CNTRLPLANE-3745: Add AuthenticationComponentProxy API for component-scoped proxy#2909
CNTRLPLANE-3745: Add AuthenticationComponentProxy API for component-scoped proxy#2909tchap wants to merge 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@tchap: This pull request references CNTRLPLANE-3745 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 story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Hello @tchap! Some important instructions when contributing to openshift/api: |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@operator/v1/types_authentication.go`:
- Around line 58-60: The field comments for the authentication proxy settings
are missing documentation for the minimum validation constraints. Update the
comments on the `httpProxy`, `httpsProxy`, and `noProxy` fields in
`types_authentication.go` so they explicitly reflect `MinLength=1` and
`MinItems=1` alongside the existing max constraints; use brief wording like
“must be non-empty” for the string fields and “must contain at least one entry”
for the list field. Keep the existing `MaxLength`/`MaxItems` descriptions intact
and ensure the comment text matches the validation markers on each field.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 65e9d68b-295c-4ffe-9bea-83347ececc28
⛔ Files ignored due to path filters (8)
openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**,!**/zz_generated*openapi/openapi.jsonis excluded by!openapi/**operator/v1/zz_generated.crd-manifests/0000_50_authentication_01_authentications-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.deepcopy.gois excluded by!**/zz_generated*operator/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/zz_generated*operator/v1/zz_generated.featuregated-crd-manifests/authentications.operator.openshift.io/AuthenticationComponentProxy.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**operator/v1/zz_generated.model_name.gois excluded by!**/zz_generated*operator/v1/zz_generated.swagger_doc_generated.gois excluded by!**/zz_generated*
📒 Files selected for processing (2)
operator/v1/types_authentication.gopayload-manifests/crds/0000_50_authentication_01_authentications-CustomNoUpgrade.crd.yaml
8a0f0c6 to
ce3fd4e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@operator/v1/tests/authentications.operator.openshift.io/AuthenticationComponentProxy.yaml`:
- Around line 107-146: Add mirrored validation coverage in
AuthenticationComponentProxy.yaml for the untested proxy branches. The current
cases only exercise httpProxy scheme/path/query/fragment failures; extend the
same style of tests for httpsProxy and add cases for “must be a valid URL” and
“must contain a hostname” on both proxy fields. Use the existing Authentication
proxy validation entries as the place to add these new negative tests so
regressions in generated schema validation are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 1956187e-f346-4508-8fb1-046bbc04d08f
⛔ Files ignored due to path filters (8)
openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**,!**/zz_generated*openapi/openapi.jsonis excluded by!openapi/**operator/v1/zz_generated.crd-manifests/0000_50_authentication_01_authentications-CustomNoUpgrade.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1/zz_generated.deepcopy.gois excluded by!**/zz_generated*operator/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/zz_generated*operator/v1/zz_generated.featuregated-crd-manifests/authentications.operator.openshift.io/AuthenticationComponentProxy.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**operator/v1/zz_generated.model_name.gois excluded by!**/zz_generated*operator/v1/zz_generated.swagger_doc_generated.gois excluded by!**/zz_generated*
📒 Files selected for processing (3)
operator/v1/tests/authentications.operator.openshift.io/AuthenticationComponentProxy.yamloperator/v1/types_authentication.gopayload-manifests/crds/0000_50_authentication_01_authentications-CustomNoUpgrade.crd.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- payload-manifests/crds/0000_50_authentication_01_authentications-CustomNoUpgrade.crd.yaml
- operator/v1/types_authentication.go
3e9fdf7 to
5857480
Compare
68ef608 to
8c2e4fa
Compare
Add a feature-gated proxy field to operator.openshift.io/v1 AuthenticationSpec, allowing component-scoped proxy configuration for the OAuth server and cluster authentication operator without requiring a cluster-wide proxy. The API includes httpProxy, httpsProxy, noProxy and trustedCA. Register the AuthenticationComponentProxy feature gate in features.go, scoped to SelfManaged clusters only (TechPreviewNoUpgrade, DevPreviewNoUpgrade).
|
@tchap: all tests passed! 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. |
Add a feature-gated proxy field to
operator.openshift.io/v1AuthenticationSpec, allowing component-scoped proxy configuration for the OAuth server and cluster authentication operator without requiring a cluster-wide proxy.The API includes
httpProxy,httpsProxy,noProxyandtrustedCA.Gated behind the
AuthenticationComponentProxyfeature gate.The relevant EP: proxy-support-for-integrated-auth-stack.md