-
Notifications
You must be signed in to change notification settings - Fork 583
TLS curves configuration feature gate #2594
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: master
Are you sure you want to change the base?
TLS curves configuration feature gate #2594
Conversation
📝 WalkthroughWalkthroughAdds a new public feature gate Changes
Estimated code review effortMedium — verify metadata and placement across manifests and docs. Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to data retention organization setting 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (5)
🧰 Additional context used🧬 Code graph analysis (1)features/features.go (1)
🔇 Additional comments (5)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
|
Hello @richardsonnick! Some important instructions when contributing to openshift/api: |
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
features/features.go
Outdated
| mustRegister() | ||
|
|
||
| FeatureGateTLSCurvesConfiguration = newFeatureGate("TLSCurvesConfiguration"). | ||
| reportProblemsToJiraComponent("kube-apiserver"). |
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.
Why should problems be reported to this component?
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.
To clarify, I'm asking because I don't see any stakeholders from the team responsible for this component represented in the enhancement proposal as reviewers or having signed off on being on the hook for bugs and issues related to this feature.
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.
Hey @everettraven,
Sorry for the confusion, I'm working with @davidesalerno to add supporting docs for the new feature introduced here: #2583
We decided Networking was the best component.
|
@richardsonnick Looks like this may need a rebase and it looks like you are missing the generated file updates here. Running |
524928d to
b1e173a
Compare
Rebased and updated generated files. Thanks for the review |
everettraven
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.
/lgtm
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: everettraven The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
As discussed with @richardsonnick I need to verify if Jira component and contact person values are the right ones. Should we also add some tests (see here)? /hold |
|
Tests are not required for new fates, only new fields. When you add the API tests will be important |
b1e173a to
e39035c
Compare
|
New changes are detected. LGTM label has been removed. |
|
Rebased + codegen |
|
@richardsonnick: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
Adds feature gate for TLS curves configuration in API config.
Enhancement PR: openshift/enhancements#1894
Implementation PR: #2583