Add support for multiple sds configs in DownstramTlsContext#24900
Add support for multiple sds configs in DownstramTlsContext#24900ggreenway merged 4 commits intoenvoyproxy:mainfrom
Conversation
Remove max_items=2 validation rules from sds configs proto. Add test cases to verify that multiple sds configs is allowed and works with SNI-based cert selection. Fixes envoyproxy#24824 Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
| } | ||
|
|
||
| TEST_F(ServerContextConfigImplTest, MultiSdsConfig) { | ||
| TEST_F(ServerContextConfigImplTest, SdsConfigNoName) { |
There was a problem hiding this comment.
The old test does not make sense. The validation failure is caused by not setting name instread of dual configs.
| "00:5B:C1:3E:33:8A:B9:2D:04:2C:B1:3F:0A"; | ||
| constexpr char TEST_SERVER_CERT_NOT_BEFORE[] = "Apr 7 16:46:35 2022 GMT"; | ||
| constexpr char TEST_SERVER_CERT_NOT_AFTER[] = "Apr 6 16:46:35 2024 GMT"; | ||
| constexpr char TEST_SERVER_CERT_256_HASH[] = |
There was a problem hiding this comment.
regenerating this header file causes CI failure since it overrides the old content, TEST_SERVER_CERT_NOT_AFTER is not generated but other test still replies on it.
I checked that servercert_info.h was first introduced by #21428.
@daixiang0 How did you generate this file, I use generate_info_header in certs.sh, but the output is different.
There was a problem hiding this comment.
From a comment (https://github.com/envoyproxy/envoy/pull/21428/files#r884045881) it sounds like it was supposed to be generated, but it seems like that isn't the case for some reason. If you'd like you can manually add NOT_BEFORE and NOT_AFTER for the new correct values, and file an issue to fix so that they're generated automatically like they should be. Or if you'd like to fix it, that would be great.
There was a problem hiding this comment.
I see, the command in certs.sh was probably modified downstream to generate header info but not got committed and upstreamed.
There was a problem hiding this comment.
regenerating this header file causes CI failure since it overrides the old content, TEST_SERVER_CERT_NOT_AFTER is not generated but other test still replies on it.
I checked that servercert_info.h was first introduced by #21428. @daixiang0 How did you generate this file, I use generate_info_header in certs.sh, but the output is different.
When run the script, it will update all contents, so you can add value and update all.
There was a problem hiding this comment.
@daixiang0 yes, but we should add changes to certs.sh otherwise when other people run certs.sh the old content is not generated.
Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
|
/retest |
|
Retrying Azure Pipelines: |
|
Network issue, wait for a moment then retest. |
|
/retest |
|
Retrying Azure Pipelines: |
…xy#24900) Remove max_items=2 validation rules from sds configs proto. Add test cases to verify that multiple sds configs is allowed and works with SNI-based cert selection. Fixes envoyproxy#24824 Signed-off-by: Luyao Zhong <luyao.zhong@intel.com>
4 participants
Remove max_items=2 validation rules from sds configs proto. Add test cases to verify that multiple sds configs is allowed and works with SNI-based cert selection.
Fixes #24824
Signed-off-by: Luyao Zhong luyao.zhong@intel.com