-
Notifications
You must be signed in to change notification settings - Fork 845
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
601f7d5 to
145da27
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: traffic_ctl config ssl_multicert <old_config> <new_config>
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.
| ts.Disk.ssl_multicert_yaml.AddLines( | ||
| [ | ||
| 'dest_ip=* ssl_cert_name=passphrase.pem ssl_key_name=passphrase.key ssl_key_dialog="exec:/bin/bash -c \'echo -n passphrase\'"', | ||
| ]) |
Copilot
AI
Dec 15, 2025
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.
This test is still using legacy format syntax after changing to ssl_multicert_yaml. Lines 44-45 contain legacy format entries which should be converted to YAML format for consistency with the migration pattern.
| tr2.Disk.ssl_multicert_yaml.AddLines( | ||
| [ | ||
| 'dest_ip=* ssl_cert_name=passphrase2.pem ssl_key_name=passphrase2.key ssl_key_dialog="exec:/bin/bash -c \'echo -n passphrase\'"', | ||
| ]) |
Copilot
AI
Dec 15, 2025
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.
The second occurrence in this file also uses legacy format syntax after the migration. Lines 69-71 should be converted to YAML format.
|
|
||
| std::string | ||
| SSLMultiCertMarshaller::to_json(SSLMultiCertConfig const &config) | ||
| { |
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.
| // ssl-multicert subcommand | ||
| auto &ssl_multicert_command = | ||
| config_command.add_command("ssl-multicert", "Manage ssl_multicert configuration").require_commands(); | ||
| ssl_multicert_command.add_command("show", "Show the ssl_multicert configuration in JSON format", Command_Execute) |
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.
By default is JSON?? does it make sense to drop YAML too? should be straight forward as the YAML node is what we read from the file.?
if so, I'd suggest using mutex groups for the options.
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: