migrate experimental-peer-skip-client-san-verification flag to feature gate #19225
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wodeyoulai The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Hi @wodeyoulai. Thanks for your PR. I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
server/features/etcd_features.go
Outdated
| // PeerSkipClientSanVerification enables to skip the verification of Subject Alternative Name (SAN) field in client certificates during peer TLS communication | ||
| // owner: @wodeyoulai | ||
| // alpha: v3.6 | ||
| // main PR: https://github.com/etcd-io/etcd/pull/14120 |
There was a problem hiding this comment.
I don't think this PR is correct.
Can you find the original PR that added this flag and change the owner to that author? Thanks!
There was a problem hiding this comment.
Sorry.I've found the original author. I will update the PR with the correct owner shortly.
|
/ok-to-test |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
... and 18 files with indirect coverage changes @@ Coverage Diff @@
## main #19225 +/- ##
==========================================
- Coverage 68.85% 68.82% -0.04%
==========================================
Files 420 420
Lines 35693 35696 +3
==========================================
- Hits 24577 24566 -11
- Misses 9692 9704 +12
- Partials 1424 1426 +2 Continue to review full report in Codecov by Sentry.
|
|
Please signoff the commit. |
read https://github.com/etcd-io/etcd/pull/19225/checks?check_run_id=35810123123 |
|
We might not need to migrate this flag, we just need to rename it to |
|
@ahrtr Understand. If you have decided to keep both flags , I can close this PR. |
|
@wodeyoulai are you still working on this PR? |
|
/retest |
|
@wodeyoulai are you still working on this PR? |
I will add the remaining tests. |
wodeyoulai
force-pushed
the
feat_gate
branch
2 times, most recently
from
6e251ee to
6ea1dc9
Compare
|
Please
|
…p-client-san-verification rename --experimental-peer-skip-client-san-verification to --peer-skip-client-san-verification Signed-off-by: wodeyoulai <106662970@qq.com>
|
@wodeyoulai: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
@ahrtr I encountered conflicts in tests after rebasing. Given the complexity, I'd like to pass this PR to someone else. Thank you for your help. |
|
| type securityConfig struct { | ||
| PeerSkipClientSanVerification bool `json:"peer-skip-client-san-verification"` | ||
| } |
There was a problem hiding this comment.
| type securityConfig struct { | |
| PeerSkipClientSanVerification bool `json:"peer-skip-client-san-verification"` | |
| } |
|
@wodeyoulai You did a really great job! Just need to remove |
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.
issue #19062