-
Notifications
You must be signed in to change notification settings - Fork 848
ssl_multicert.config -> ssl_multicert.yaml #12755
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
base: 11-Dev
Are you sure you want to change the base?
ssl_multicert.config -> ssl_multicert.yaml #12755
Conversation
9b6767b to
4cb0241
Compare
145da27 to
72e817d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 198 out of 198 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
tests/gold_tests/tls/tls_check_dual_cert_selection.test.py:56
- The test is still using legacy format syntax in AddLines() after changing from ssl_multicert_config to ssl_multicert_yaml. Lines 53-56 contain legacy format entries like 'ssl_cert_name=signed-foo-ec.pem,signed-foo.pem' which should be converted to YAML format to match the migration pattern used consistently in other test files.
tests/gold_tests/tls/tls_check_cert_select_plugin.test.py:52 - Similar to tls_check_dual_cert_selection.test.py, this test is still using legacy format syntax after changing to ssl_multicert_yaml. Lines 49-52 should be converted to YAML format to be consistent with the migration pattern.
tests/gold_tests/tls/tls_check_cert_selection_reload.test.py:46 - This test is still using legacy format syntax in AddLines() after changing to ssl_multicert_yaml. Lines 44-46 contain legacy format entries which should be converted to YAML format for consistency.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3ad74ce to
63866b1
Compare
cmcfarlen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like the concept of having parsing code and conversions in one place. It's really clean to do the conversion natively with traffic_ctl. I do think the runtime code should also use this code to load config as well so there isn't code duplication.
Replace the ssl_multicert.config format with YAML format, following the pattern established by sni.yaml. The new ssl_multicert.yaml uses a top-level 'ssl_multicert' key containing a sequence of certificate entries. This also supports config conversion via: traffic_ctl config ssl_multicert <old_config> <new_config>
This also makes yaml the default traffic_ctl config ssl_multicert show format
63866b1 to
408bf48
Compare
408bf48 to
f551253
Compare
5424730 to
380ab35
Compare
bff0d7f to
b87c368
Compare
Replace the ssl_multicert.config format with YAML format, following the pattern established by sni.yaml. The new ssl_multicert.yaml uses a top-level 'ssl_multicert' key containing a sequence of certificate entries.
This also supports config conversion via:
Community Decisions
traffic_ctl config convertper @cmcfarlen 's suggestion.At least for ssl_multicert, either of these options are possible from an implementation standpoint.
@cmcfarlen had an interesting suggestion to bake the config conversion logic into traffic_ctl. Maybe have something like: