Rename [database].ssl* options to [database].tls* to support pymongo 4
#6250
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…l_certfile pymongo 4 will ignore the ssl_keyfile/ssl_certfile options. For consistency in st2.conf, this uses snake_case not the mongo camelCase option name. This also adds tls_certificate_key_file_password. We did not support ssl_pem_passphrase before, so there was nothing to migrate.
This needed to be a different option (instead of just renaming) because the option type is changing from str+choices to a bool. For consistency in st2.conf, this uses snake_case not the mongo camelCase option name.
For consistency in st2.conf, this uses snake_case not the mongo camelCase option name.
…tname For consistency in st2.conf, this uses snake_case not the mongo camelCase option name.
Not sure if this wasn't available before, or why it wasn't used. Try and see.
And use fix the sample default for python_binary to use python3.
pull-request-size
bot
added
the
size/XL
cognifloyd
requested review from
guzzijones,
mamercad,
nzlosh,
rush-skills and
winem
cognifloyd
commented
Sep 24, 2024
|
|
||
| result = await Get( | ||
| FallibleProcessResult, | ||
| ProcessResult, |
Member
Author
There was a problem hiding this comment.
This change prevents the conf/st2.conf.sample from being replaced if tools/config_gen.py exits with an error. It also makes pants report the error by printing stdout from the process, which includes the traceback.
cognifloyd
commented
Sep 24, 2024
guzzijones
approved these changes
Sep 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
pymongo 4 removes deprecated names for database connection options that were renamed some time ago. We even have a TODO about updating our usage of the deprecated names in the code:
st2/st2common/st2common/models/db/__init__.py
Lines 445 to 447 in 7d70498
One change in particular could not be supported without breaking backwards compatibility: The files for
ssl_keyfileandssl_certfilemust be concatenated and passed as one file in the newtls_certificate_key_fileoption.Why update to pymongo 4 (in a follow-up PR)? We need to update pymongo to ensure we're using a version that tests with and officially supports our target MongoDB version(s) (we are planning on using MongoDB 7; see #6246 and #6236). The pymongo4 upgrade guide has details on the option naming migration.
Each commit touches one option or aspect of this migration, so it will be useful to review each commit.
Since we're using newer
oslo.confignow, we can also be more explicit about deprecations when defining the options. So, a few of the commits make use of those newer features to improve ourst2.conf.samplefile.In summary, these options were migrated:
ssl->tlsssl_keyfile+ssl_certfile->tls_certificate_key_file(files must be concatenated)tls_certificate_key_file_passwordssl_cert_reqs->tls_allow_invalid_certificates(from a string opt to a bool opt)ssl_ca_certs->tls_ca_filessl_match_hostnames->tls_allow_invalid_hostnames(inverted meaning)Note: In #6246, I initially developed this using mongo's camelCase naming convention in st2.conf. After discussing with @nzlosh I went back to using snake_case to be consistent with the rest of the st2.conf options.