openshift
CNF-13731: Add HTTP01ChallengeProxy
Related to: https://github.com/openshift/enhancements/pull/1773
This is my first attempt at a FeatureGate so I'm expecting this to need a bunch of work.
Additions to API Specification and Validation:
- Added
HTTP01ChallengeProxyconfiguration to theAPIServerSpecstruct, allowing users to enable and configure the HTTP01 challenge proxy for API endpoint certificates. This includes options forDefaultDeploymentandCustomDeploymentmodes. (config/v1/types_apiserver.go, [1] [2]
Feature Gate Registration:
- Registered the new
HTTP01ChallengeProxyfeature gate infeatures.go, enabling it in theTechPreviewNoUpgradescope and providing metadata such as a contact person and enhancement proposal link. (features/features.go, features/features.goR869-R876)
Test Cases for Validation:
- Added comprehensive test cases in
HTTP01ChallengeProxy.yamlto validate the behavior of theHTTP01ChallengeProxyconfiguration. Tests cover valid configurations, invalid modes, and boundary conditions for theinternalPortfield. (config/v1/tests/apiservers.config.openshift.io/HTTP01ChallengeProxy.yaml, config/v1/tests/apiservers.config.openshift.io/HTTP01ChallengeProxy.yamlR1-R105)
@sebrandon1: This pull request references CNF-13731 which is a valid jira issue.
Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "4.20.0" version, but no target version was set.
In response to this:
Related to: https://github.com/openshift/enhancements/pull/1773
This is my first attempt at a FeatureGate so I'm expecting this to need a bunch of work.
Additions to API Specification and Validation:
- Added
HTTP01ChallengeProxyconfiguration to theAPIServerSpecstruct, allowing users to enable and configure the HTTP01 challenge proxy for API endpoint certificates. This includes options forDefaultDeploymentandCustomDeploymentmodes. (config/v1/types_apiserver.go, [1] [2]Feature Gate Registration:
- Registered the new
HTTP01ChallengeProxyfeature gate infeatures.go, enabling it in theTechPreviewNoUpgradescope and providing metadata such as a contact person and enhancement proposal link. (features/features.go, features/features.goR869-R876)Test Cases for Validation:
- Added comprehensive test cases in
HTTP01ChallengeProxy.yamlto validate the behavior of theHTTP01ChallengeProxyconfiguration. Tests cover valid configurations, invalid modes, and boundary conditions for theinternalPortfield. (config/v1/tests/apiservers.config.openshift.io/HTTP01ChallengeProxy.yaml, config/v1/tests/apiservers.config.openshift.io/HTTP01ChallengeProxy.yamlR1-R105)
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 openshift-eng/jira-lifecycle-plugin repository.
Hello @sebrandon1! Some important instructions when contributing to openshift/api: API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.
![openshift-ci[bot] avatar](https://avatars.githubusercontent.com/in/91280?v=4 "Published by a pub.dev verified publisher"){kind=small}
Proto changes appear to be unrelated, verify thinks changing them is wrong and is suggesting to change them back
Possibly running with a newer or incompatible version?
Okay I started over and re-ran it with PROTO_OPTIONAL=true make update like you suggested and it seems much more manageable now. I'll take whatever suggestions I can get 😄
Okay I started over and re-ran it with
PROTO_OPTIONAL=true make updatelike you suggested and it seems much more manageable now. I'll take whatever suggestions I can get 😄
Looks like the actual API changes are missing now
API types look good, but it looks like some of the test cases are expecting things that aren't true (optional port), so lets get those fixed up and please squash the various fixups
False positive on new required field with optional parent.
/override ci/prow/verify-crd-schema
@everettraven: Overrode contexts on behalf of everettraven: ci/prow/verify-crd-schema
In response to this:
False positive on new required field with optional parent.
/override ci/prow/verify-crd-schema
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.
Changes here LGTM, there are however some code generation issues by the looks of it
/approve /lgtm
verify-crd-schema failing on a false positive. New required fields are all parented by optional fields.
/override ci/prow/verify-crd-schema
Holding this PR on merge of enhancement proposal. @JoelSpeed Feel free to overrule me here if holding on EP merge is not necessary. /hold
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: everettraven, sebrandon1
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [everettraven]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@everettraven: Overrode contexts on behalf of everettraven: ci/prow/verify-crd-schema
In response to this:
/approve /lgtm
verify-crd-schema failing on a false positive. New required fields are all parented by optional fields.
/override ci/prow/verify-crd-schema
Holding this PR on merge of enhancement proposal. @JoelSpeed Feel free to overrule me here if holding on EP merge is not necessary. /hold
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.
Regarding whether this should merge or not, I currently haven't seen any of the listed EP approvers chime on the EP for review, nor anyone from the kube-apiserver-operator owners file. Since the EP relies on that team accepting a change into their operator, I'd expect to see at least some signal that they're happy with the direction before we merge the API
@sebrandon1: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
| Test name | Commit | Details | Required | Rerun command |
|---|---|---|---|---|
| ci/prow/e2e-aws-serial-techpreview-1of2 | 9b31febcccfa4c12df6824ce5feae84e87ead925 | link | true | /test e2e-aws-serial-techpreview-1of2 |
| ci/prow/e2e-aws-serial-1of2 | 9b31febcccfa4c12df6824ce5feae84e87ead925 | link | true | /test e2e-aws-serial-1of2 |
| ci/prow/e2e-aws-ovn-hypershift | 9b31febcccfa4c12df6824ce5feae84e87ead925 | link | true | /test e2e-aws-ovn-hypershift |
| ci/prow/verify | 9b31febcccfa4c12df6824ce5feae84e87ead925 | link | true | /test verify |
| ci/prow/e2e-upgrade-out-of-change | 9b31febcccfa4c12df6824ce5feae84e87ead925 | link | true | /test e2e-upgrade-out-of-change |
| ci/prow/verify-crdify | 9b31febcccfa4c12df6824ce5feae84e87ead925 | link | true | /test verify-crdify |
Full PR test history. Your PR dashboard.
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.
PR needs rebase.
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.
Issues go stale after 90d of inactivity.
Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.
If this issue is safe to close now please do so with /close.
/lifecycle stale
[!IMPORTANT]
Review skipped
Auto reviews are limited based on label configuration.
:no_entry_sign: Review skipped — only excluded labels are configured. (1)
- do-not-merge/work-in-progress
Please check the settings in the CodeRabbit UI or the
.coderabbit.yamlfile in this repository. To trigger a single review, invoke the@coderabbitai reviewcommand.You can disable this status message by setting the
reviews.review_statustofalsein the CodeRabbit configuration file.
Comment @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot] avatar](https://avatars.githubusercontent.com/in/347564?v=4 "Published by a pub.dev verified publisher"){kind=small}
Stale issues rot after 30d of inactivity.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.
If this issue is safe to close now please do so with /close.
/lifecycle rotten /remove-lifecycle stale