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

Add keystore.xml to overrides not defaults #474

Open
wants to merge 1 commit into
base: vNext
Choose a base branch
from
Open

Conversation

idlewis
Copy link
Member

@idlewis idlewis commented Oct 20, 2023

This prevents a keystore password conflict, if the user has copied a template server.xml (which may include keystore config + password)

First stab at addressing #427

If docker-server.sh finds certificates mounted in '/etc/x509/certs', we make the assumption that the user wants us to manage their certificates for them. This means that we are 'safe' to put our keystore config in configDropins/overrides. This will prevent the issue reported in #427, as the generated keystore/config will override the config from the example server.xml, so there will no longer be a mismatch between keystore and keystore password.

The openssl commands will overwrite an existing keystore in the default location, so creating the keystore will always suceed, even if an existing keystore is in the container image.

If the user has run configure.sh in their dockerfile, then there will be an existing keystore.xml snippet in configDropins/defaults. This will cause a warning at server startup due to multiple definitions of the keystore config. The override will win the conflict, so it would be safe to leave the defaults keystore.xml in place, but it is probably better to try and remove it.
Again, this should be safe, as it is highly unlikely that a user would have written to this specific file location and name.

A potentially 'fail' scenario would be if a user has added a keystore + config into their container image, and also mounted certificates in /etc/x509/certs. We would then overwrite their keystore, and override their keystore config.
This is an unlikely scenario as it is not best practice to put a keystore into a container image, and in addition, the user would then be unlikely to mount certificates into /etc/x509/certs.

@leochr
Copy link
Member

leochr commented Oct 27, 2023

@arturdzm please review when you get a moment. Thanks

arturdzm
arturdzm previously approved these changes Nov 2, 2023
This prevents a keystore password conflict, if the user has copied
a template server.xml (which may include keystore config + password)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants