Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 [Portainer] Not possible to perform Initial Setup #1587

Open
IPTN opened this issue Sep 27, 2024 · 12 comments
Open

🐛 [Portainer] Not possible to perform Initial Setup #1587

IPTN opened this issue Sep 27, 2024 · 12 comments
Labels
bug Something isn't working stale Element will be closed automatically

Comments

@IPTN
Copy link

IPTN commented Sep 27, 2024

Description

Currently it is not possible to perform Initial Setup of Portainer which prevents restoring from backup.

"password": "str?",

This line is preventing removing the password from the addon configuration options, which is the way to cause a full database reset and allow initial setup.
password: define admin password. If kept blank, will allow manual restore of previous backup. At least 12 characters.

# Define option
if bashio::config.has_value 'password' ; then
echo -n "${PASSWORD:-empty}" > /data/portainer_password
options+=(--admin-password-file /data/portainer_password)
bashio::log.info "... password set to $PASSWORD"
else
echo -n "${PASSWORD:-empty}" > /data/portainer_password
bashio::log.info "... starting without predefined password"
fi

This behavior of not allowing matches HA documentation for the options and schema fields, which results in this behavior
image
When simply removing the password field from the addon options it will be repopulated to the default value of 'homeassistant'.

"password": "homeassistant",

Removing the line first referenced from schema in config.json should result in the intended behavior of allowing not setting a password in the addon options.

Willing to submit a PR for this change if requested, but it would obviously be a trivial one.

Reproduction steps

1. Remove password from addon options
2. Password option is auto populated to the default value of 'homeassistant'

Addon Logs

Starting...
/etc/cont-init.d/00-banner.sh: executing

-----------------------------------------------------------
 Add-on: Portainer
 Manage your Docker environment with ease
-----------------------------------------------------------
 Add-on version: 2.21.1
 You are running the latest version of this add-on.
 System: Home Assistant OS 13.1
 Home Assistant Core: 2024.9.3
 Home Assistant Supervisor: 2024.09.1
-----------------------------------------------------------
 Please, share the above information when looking for help
 or support in, e.g., GitHub, forums
-----------------------------------------------------------
 Provided by: https://github.com/alexbelgium/hassio-addons 
-----------------------------------------------------------
/etc/cont-init.d/00-global_var.sh: executing
certfile='fullchain.pem'
keyfile='privkey.pem'
password='homeassistant'
ssl='false'
/etc/cont-init.d/01-config_yaml.sh: executing
/etc/cont-init.d/01-custom_script.sh: executing
/etc/cont-init.d/30-nginx.sh: executing
/etc/cont-init.d/31-portainer.sh: executing
 
Starting the upstream container
 
s6-rc: info: service s6rc-oneshot-runner: starting
s6-rc: info: service s6rc-oneshot-runner successfully started
s6-rc: info: service base-addon-banner: starting

-----------------------------------------------------------
 Add-on: Portainer
 Manage your Docker environment with ease
-----------------------------------------------------------
 Add-on version: 2.21.1
 You are running the latest version of this add-on.
 System: Home Assistant OS 13.1
 Home Assistant Core: 2024.9.3
 Home Assistant Supervisor: 2024.09.1
-----------------------------------------------------------
 Please, share the above information when looking for help
 or support in, e.g., GitHub, forums or the Discord chat.
-----------------------------------------------------------
s6-rc: info: service base-addon-banner successfully started
s6-rc: info: service fix-attrs: starting
s6-rc: info: service base-addon-log-level: starting
s6-rc: info: service fix-attrs successfully started
s6-rc: info: service base-addon-log-level successfully started
s6-rc: info: service legacy-cont-init: starting
s6-rc: info: service legacy-cont-init successfully started
s6-rc: info: service legacy-services: starting
services-up: info: copying legacy longrun nginx (no readiness notification)
services-up: info: copying legacy longrun portainer (no readiness notification)
[20:50:55] INFO: Starting Portainer...
[16:50:55] INFO: Waiting for port 9000 to open...
s6-rc: info: service legacy-services successfully started
[20:50:55] INFO: ... password set to homeassistant
[20:50:55] INFO: ... non-addon containers hidden
[20:50:55] INFO: ... portainer launched
2024/09/27 08:50PM INF github.com/portainer/portainer/api/cmd/portainer/main.go:370 > encryption key file not present | filename=portainer
2024/09/27 08:50PM INF github.com/portainer/portainer/api/cmd/portainer/main.go:393 > proceeding without encryption key |
2024/09/27 08:50PM INF github.com/portainer/portainer/api/database/boltdb/db.go:125 > loading PortainerDB | filename=portainer.db
2024/09/27 08:50PM INF github.com/portainer/portainer/api/internal/ssl/ssl.go:80 > no cert files found, generating self signed SSL certificates |
2024/09/27 08:50PM INF github.com/portainer/portainer/api/cmd/portainer/main.go:542 > created admin user with the given password. |
2024/09/27 08:50PM INF github.com/portainer/portainer/api/chisel/service.go:228 > generated a new Chisel private key file | private-key=/data/chisel/private-key.pem
2024/09/27 20:50:55 server: Reverse tunnelling enabled
2024/09/27 20:50:55 server: Fingerprint tzcmzQBRytTsOCwh1eWhV+EEsd7KC4sDyJzrp5ZbRf4=
2024/09/27 20:50:55 server: Listening on http://0.0.0.0:8000
2024/09/27 08:50PM INF github.com/portainer/portainer/api/cmd/portainer/main.go:655 > starting Portainer
2024/09/27 08:50PM INF github.com/portainer/portainer/api/http/server.go:370 > starting HTTPS server | bind_address=:9443
2024/09/27 08:50PM INF github.com/portainer/portainer/api/http/server.go:354 > starting HTTP server | bind_address=0.0.0.0:9000
[16:50:56] INFO: Starting NGinx...

Architecture

No response

OS

No response

@IPTN IPTN added the bug Something isn't working label Sep 27, 2024
@alexbelgium
Copy link
Owner

Hi, thanks I see your point. I'll check tomorrow (an update will also likely come this night as a new version exists)

@alexbelgium
Copy link
Owner

The issue is that if there is no password it is less convenient for novice users for whom it would be easier to have the password set by default in the HA options. Perhaps I could add a specific boolean that would be automatically reset to false on boot, allowing to circumvent the whole password feature for one single boot...

@IPTN
Copy link
Author

IPTN commented Sep 27, 2024

Based on the documentation for addon configuration, removing the password field from schema but keeping it in options with the current default value should result in what I understand to be the desired behavior documented in the README. No other changes should be necessary unless you are looking to change functionality.

That change will make the password field optional while still retaining the default value.

I tried this change locally and it did not work as I expected. It did allow reaching the Initial Setup screen for Portainer.... because password was no longer being used from options at all.

With password in options and not schema:

  1. The UI for the addon options page in HA did not show the field.
  2. But the yaml editor is correctly populated with the default value.
  3. When editing password through YAML it behaves as I had expected - Initially populated with the default value, is now nullable without an error, these changes propogate correctly to addons.json in Supervisor.
  4. Nothing else about it works, password is never passed to the addon whether populated or not, which obviously breaks the functionality of setting the password with the addon options.

At this point I figured that the documentation was incorrect. There was a relevant line showing in Supervisor logs WARNING (MainThread) [supervisor.addons.options] Option 'password' does not exist in the schema for Portainer (db21ed7f_portainer).

I found the relevant HA code:
https://github.com/home-assistant/supervisor/blob/d09460a97170f239f0247f3a87692b39d45b09f6/supervisor/addons/options.py#L83-L92
There is that log line (I didn't actually look at the Supervisor log until I saw it here). And the comment indicates that this is how it is intended to work.

The other part of the code that is important to the functionality that is trying to be achieved with the password field is here:
https://github.com/home-assistant/supervisor/blob/d09460a97170f239f0247f3a87692b39d45b09f6/supervisor/addons/options.py#L256-L261
It looks like it should work as currently implemented given the check for it being defined in the schema as a string ending in '?'. This matches the documentation.

Well the problem is in the edge case as always. It never actually makes it to that point because of an earlier validation called just before it checks for missing options:
https://github.com/home-assistant/supervisor/blob/d09460a97170f239f0247f3a87692b39d45b09f6/supervisor/addons/options.py#L116-L120
You'll notice it doesn't actually check if the schema field is optional when throwing the error for a null value; despite the comment seemingly indicating it should only be applied to required fields.

The last piece is pretty obvious. It is not possible to just remove the field from the addon options (which I guess is how all optional fields currently work) because it is populated with the default value before the schema validation is even performed:
https://github.com/home-assistant/supervisor/blob/d09460a97170f239f0247f3a87692b39d45b09f6/supervisor/addons/addon.py#L963-L973

To circle back around, the documentation was not clear enough to me on the several times I read it before looking at the actual implementation.
The options dictionary contains all available options and their default value. Set the default value to null or define the data type in the schema dictionary to make an option mandatory. This way the option needs to be given by the user before the add-on can start. ~ To make an option truly optional (without default value), the schema dictionary needs to be used. Put a ? at the end of the data type and do not define any default value in the options dictionary. If any default value is given, the option becomes a required value. ~ The schema looks like options but describes how we should validate the user input.
It's not obvious that all fields used need to be defined in schema, and the documentation on optional fields is not well organized/is written in seperate paragraphs with somewhat conflicting information and with differing terminology.

The defining part for this use case is "If any default value is given, the option becomes a required value.", which didn't really sink in until going down this rabbit hole.

T.L.D.R.
So in summary, it is not currently possible to have an optional field with a default value because you fields can never be set to None/null regardless of optional status and in spite of comments in the code indicating otherwise. (And I am of the opinion that documentation on addon options and schema is not clear enough, especially as regards to optional fields).

^ None of which is a bug inside of your addon, with the small caveat that the optional flag for password won't work with a default value. Right back where I started:

"password": "str?",

@alexbelgium
Copy link
Owner

Now you are a very comprehensive person in your search and documentation!

My slight concern in removing the default value is that it makes things less "beginner-proof" as the password will need to be set by them though an action.

Would a middle ground : typing "blank" in the password field be an acceptable workaround? And I'll update the documentation as such. This would allow both the use case of starting without a password (resetting the database), and setting a default value for beginners to have a ready-to-use add-on.

Second option for me is to remove the default option tag but make a big warning in the log that the password was not set in the options

@IPTN
Copy link
Author

IPTN commented Sep 28, 2024

I understand the intention behind the feature, but I would like to point out that when starting Portainer without setting the password results in the Initial Setup being shown to the user immediately on first startup.
image

So my prefence would be to make use of the Portainers built-in onboarding.

One downside to relying on Portainers Initial Setup exclusively is that a beginner user has no way of easily retrieving or reseting the password outside of Portainer. The easiest resolution for such a user in that situation would likely be to simply reinstall the addon and perform initial setup again. This obviously results in their configuration being lost, but that is not materially different than the current implementation that wipes when the password is changed through the password field. Still, this could be addressed by changing to a reset_password field which when set results in the current behavior of a password change and is then cleared with bashio.

The other caveat with this approach is that there is a 5 minute window to complete the Initial Setup. If this timeout is reached it results in a rather unhelpful page that only displays '404: Not Found'. It is easily resolvable though by looking at the log: INF github.com/portainer/portainer/api/adminmonitor/admin_monitor.go:62 > the Portainer instance timed out for security purposes, to re-enable your Portainer instance, you will need to restart Portainer |.
The reason for the very unhelpful page shown is due to Portainer redirecting to `http://{homeassistant.local:8123}/timeout.html', which obviously doesn't work due to the reverse proxy. Unfortunately, I don't think this is easily mitigated to properly serve the intended timeout page or a replacement, but the log message is clear.

The alternative middle ground you suggested would certainly work. However, for a feature targeted at beginners I believe it has some downsides.

First, that it is not obvious that changing the password in this manner will reset Portainer, which is currently mitigated with the backup and log messages indicating such after the fact (this behaviour should probably be included with the documentation change if you decide to go with this approach). I would argue, however, that a beginner who needs this feature would not easily be able to restore to the backup inside the container.

The other pitfall that exists is that the password is kept in the config directly and currently is being logged:
The check to not log if the field contains 'PASS' is not met with the current fieldname:

if bashio::config.false "verbose" || [[ "${KEYS}" == *"PASS"* ]]; then
bashio::log.blue "${KEYS}=******"
else
bashio::log.blue "$line"

But, it is intentional behaviour for this addon in particular.
bashio::log.info "... password set to $PASSWORD"

Since the container is reversed proxied behind the HA interface, and therefore a valid HA login, it is not immediately abusable if leaked. But for a beginner it would be fairly easy to accidently leak either through sharing their config (unlikely to be using secrets.yaml) or sharing their log which could be a problem if it is a credential they are reusing.

Regardless, my understanding is that this addon specifically meant for more advanced users due to the potential for footguns: The description provided with the addon (and I believe the stated reason for its removal from the community addons).
It is obviously your decision to make the addon as accessible as possible (especially with adding guard rails), so feel free to ignore my 2cents; But, a user who is unable to complete first time setup (which IMO is a fairly low barrier to entry) without this feature might be better served not using this addon until they are more experienced.

Thanks for your work maintaining this addon and being so responsive!

Copy link
Contributor

github-actions bot commented Oct 6, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Element will be closed automatically label Oct 6, 2024
@alexbelgium
Copy link
Owner

Argh, I forgot about this one, sorry - I had seen your text and instead of answering directly, thought that it warranted a bit more reflection. So here it comes :

Indeed I see your point - I usually try to make things as easy as possible and indeed in this case perhaps it is not warranted.

"One downside to relying on Portainers Initial Setup exclusively is that a beginner user has no way of easily retrieving or reseting the password outside of Portainer." > I agree, either they can uninstall or reinstall, or simply setting the addon in the addon options does this too.

"The other caveat with this approach is that there is a 5 minute window to complete the Initial Setup" > now this is a bigger issue. I can put a warning at the bottom of the log.

I'll try to push a version with the new logic, and hopefully will have time to try it before other people update ! Worst case is that all users would loose their databases which would not be the best outcome.

thanks for your very comprehensive descriptions!

alexbelgium added a commit that referenced this issue Oct 6, 2024
alexbelgium added a commit that referenced this issue Oct 6, 2024
@IPTN
Copy link
Author

IPTN commented Oct 6, 2024

No worries!

With the changes in the commits that I see, I don't believe that it should cause anyone who has already set the addon up previously to reset their database unless they change or clear the password from their options (which was already how it worked). I should have some time later today to test though.

I noticed that you changed the addon script to not log the password set in the options, however it would still be logged by the shared global_var script. Changing this to be case insensitive should be enough to prevent logging it there as well:

if bashio::config.false "verbose" || [[ "${KEYS}" == *"PASS"* ]]; then
bashio::log.blue "${KEYS}=******"
else
bashio::log.blue "$line"

@alexbelgium
Copy link
Owner

EDIT : ok, I've rolled back the update. As I feared, changing the default setting removed my password and resetted my database. Therefore, for user continuity I can't remove "password": "homeassistant", from the options: block.

@alexbelgium
Copy link
Owner

It seems that HA considers that if a default value was set in the addon options and removed in a further update, it should delete it from the user config (which of course seems like a very bad idea ;-) )

Then I'll have to use the second option : manually type "empty" in password to trigger the startup without password ...

@IPTN
Copy link
Author

IPTN commented Oct 6, 2024

It seems that HA considers that if a default value was set in the addon options and removed in a further update, it should delete it from the user config (which of course seems like a very bad idea ;-) )

Then I'll have to use the second option : manually type "empty" in password to trigger the startup without password ...

Ah, yeah if HA clears it then it would result in a wipe. What happens if, when adding the logic to check for the magic string "empty", you also change the default to "empty". Does a different default override a value a user had set previously? If not that would allow the onboarding process to be the new default.

Another approach would be to use a migration process to a new option such as "password_override" where you could manage existing users while still removing the default. This would be more work though so not necessarily worth it.

alexbelgium added a commit that referenced this issue Oct 6, 2024
alexbelgium added a commit that referenced this issue Oct 6, 2024
@alexbelgium
Copy link
Owner

alexbelgium commented Oct 6, 2024

I've tried setting default to "empty" and it resets again my database. So I think I'll keep the default to "homeassistant" and allow initial boot by setting the option to "empty", and write that in the README

alexbelgium added a commit that referenced this issue Oct 6, 2024
alexbelgium added a commit that referenced this issue Oct 6, 2024
alexbelgium added a commit that referenced this issue Oct 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working stale Element will be closed automatically
Projects
None yet
Development

No branches or pull requests

2 participants