docker icon indicating copy to clipboard operation
docker copied to clipboard

Warn if config folder misses original image configs or files differ

Open flortsch opened this issue 2 years ago • 4 comments

Implements a check in the Docker entrypoint that compares the special config files of this Docker image with the config files of the user folder. It prints a warning if those config files are missing or they differ from the original Docker image config files. This is a first step, as suggested by @joshtrichards in https://github.com/nextcloud/docker/issues/1533. In the long run, these files should be treated as special config files not to be messed with by the user, and they should be copied over into the user config folder on every update.

flortsch avatar Dec 13 '23 23:12 flortsch

Thanks @flortsch!

I'm thinking we should include a new paragraph in the README to refer people to with questions about what is going on here / help them know what they should do. Based on experience with the setup warnings in Nextcloud's Admin Overview, many people pay a lot of attention to these things and get frustrated if they're not able to remedy warnings (understandable reaction + remedying this is the goal!).

Ideally we include that in this PR, but it can also be in an immediately following one if you'd prefer to keep separate.

joshtrichards avatar Dec 28 '23 16:12 joshtrichards

Hey @joshtrichards! Sound good. I can extend the PR with a section in the readme and a link to it in the console output. Since you mentioned warnings in the Nextcloud admin overview: is it possible to generate such UI warnings in the entrypoint script? I think that warnings in the UI would be a nice addition to the console output which can often be overlooked.

flortsch avatar Dec 28 '23 17:12 flortsch

@joshtrichards Please check if the text and link is ok. I updated the README and added a link in the docker-entrypoint echo statement. Also, I inlined your last text feedback.

flortsch avatar Jan 12 '24 01:01 flortsch

Since you mentioned warnings in the Nextcloud admin overview: is it possible to generate such UI warnings in the entrypoint script? I think that warnings in the UI would be a nice addition to the console output which can often be overlooked.

Not currently.

joshtrichards avatar Feb 15 '24 03:02 joshtrichards

Hey @joshtrichards! I included your suggestions. Only fixed the formatting and the uninitialized variable access. Tested locally with docker-compose (fresh start + changes in existing container/volume). Missing or changed image-specific files trigger the warning & file listing. Custom config files don't trigger anything. Commits squashed and rebased on latest master.

flortsch avatar Mar 18 '24 20:03 flortsch

@J0WI What do you think about this?

I think a warning when the configs are out-of-date is a good step forward without introducing a breaking change. It's also something easy to check (or request the reporter check) in the container logs whenever a new bug comes in is unknowingly bogus due to out-of-date config files.

The other option is we take the full leap: documenting all the image config/ files (names) as being "special" and exclusive to the image. Then we update them automatically whenever needed. That'll cause it's own problems (at least in the short-term) IMO.

We can always take the bigger leap later on after we add the warning. So I think this is a good compromise.

joshtrichards avatar Jun 03 '24 14:06 joshtrichards

@joshtrichards I reverted the changes and simplified the code back to just print a one-line warning for every config file that differs. What do you think?

flortsch avatar Jun 21 '24 11:06 flortsch

Thanks, @flortsch for getting this started and hanging in there! :-)

joshtrichards avatar Jun 26 '24 01:06 joshtrichards

First sighting in the wild: https://help.nextcloud.com/t/question-on-some-warnings-on-outdated-files-since-update-to-29-0-3/196297

Maybe the docs link should get added back in :-)

joshtrichards avatar Jun 27 '24 19:06 joshtrichards

I myself got this warning today after updating to 28.0.7 :rofl: Will double-check my configs, but it seems the check fulfills its purpose :smile: If the confusion gets to big I can provide a PR with the link to the README back in place.

flortsch avatar Jun 27 '24 19:06 flortsch