OCPBUGS-105398: refactor: remove Azure workload identity feature gate - #1575
openshift-merge-bot[bot] merged 1 commit into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughAzure workload identity selection now depends on whether Merge Risk: ⚪ Minimal · up to Azure workload identity authentication is selected when no client secret is configured, with obsolete feature-gate wiring removed. No current merge-readiness risk is identified. 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
Full details: Stable And Deterministic Test NamesExplanation The pull request changes only Azure authentication/configuration plumbing and documentation. The parent-to-HEAD diff contains no test files and no added or removed Ginkgo declarations ( Full details: Test Structure And QualityExplanation PASS: The pull request changes no test files and introduces no Ginkgo Full details: Single Node Openshift (Sno) Test CompatibilityExplanation The pull request changes six non-test files only. The patch adds no Ginkgo declarations and modifies no *_test.go, test/, or tests-extension/ files. Therefore it introduces no new e2e test with a multi-node assumption. Full details: Topology-Aware Scheduling CompatibilityExplanation PASS — The commit changes Azure authentication and removes feature-gate plumbing only. The complete diff changes six files and adds no anti-affinity, topology spread, node selector or affinity, toleration, replica, update-strategy, or PDB settings. The existing replica wiring remains unchanged, and no deployment manifest changes are present. Therefore, the PR introduces no topology-sensitive scheduling constraint covered by this check. Full details: Ote Binary Stdout ContractExplanation PASS. The pull request changes Azure credential selection and removes feature-gate plumbing. It adds no Full details: Ipv6 And Disconnected Network Test CompatibilityExplanation PASS — The pull request changes six non-test files only. The diff adds no Ginkgo declarations, e2e tests, IPv4 assumptions, or external connectivity requirements. The custom check is therefore not applicable. Full details: No-Weak-CryptoExplanation PASS: The PR changes Azure credential selection and removes feature-gate plumbing. The added code only checks whether Full details: Container-PrivilegesExplanation PASS: The pull request changes only Go and documentation files. It does not change any manifest, Dockerfile, or container security setting. The existing operator deployments set Full details: No-Sensitive-Data-In-LogsExplanation No sensitive-data logging was introduced. The diff changes Azure credential selection and removes feature-gate plumbing, but it adds no logging calls and does not pass client secrets, federated token paths, credentials, or token values to logs. Existing DNS record and zone logging is unchanged from the parent revision.
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@damdo: This pull request references Jira Issue OCPBUGS-105398, which is valid. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
/retest |
|
/assign @Miciah @davidesalerno |
|
/retest |
|
Thanks! /approve @damdo, the PR description is detailed, but the commit message is missing a body. Would you mind copying the description and a reference to https://redhat.atlassian.net/browse/OCPBUGS-105398 into the commit message? Also, your PR might as well delete the reference to the featuregate in cluster-ingress-operator/AGENTS.md Line 330 in 8e5b7c7 |
Removes the AzureWorkloadIdentity feature-gate dependency from the cluster ingress operator. The feature is GA and enabled by default, so Azure workload identity is now used unconditionally when no client secret is available. This change removes the feature-gate observation and Azure DNS configuration plumbing, including the provider/client boolean parameter. https://redhat.atlassian.net/browse/OCPBUGS-105398 openshift/api#3018
8e5b7c7 to
823f9d5
Compare
|
@Miciah Thanks for the review and approval. I’ve updated the commit message with the PR description and the OCPBUGS-105398 reference, and removed the stale |
|
/retest |
|
/pipeline required |
|
Scheduling tests matching the |
|
/retest |
4 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest-required |
|
/retest |
1 similar comment
|
/retest |
|
Thanks! /approve |
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Miciah 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 |
|
/retest-required |
1 similar comment
|
/retest-required |
| // if Azure workload identity is not used. | ||
| ClientSecret string | ||
| // FederatedTokenFile is the path to a file containing a workload | ||
| // identity token. If FederatedTokenFile is specified and |
There was a problem hiding this comment.
This wording looks directionally right after removing the feature gate, but it may be clearer to keep the relationship to FederatedTokenFile explicit. As written, “If no client secret is available” could read as though workload identity is used unconditionally, even though this field specifically represents the token file path. Consider something like: “FederatedTokenFile is the path to a file containing a workload identity token. If no client secret is available, this token file is used for Azure workload identity.”
There was a problem hiding this comment.
I will address this in a follow-up, I prefer not to lose all the green ci passes I have here :)
davidesalerno
left a comment
There was a problem hiding this comment.
just one small nit about wording on a comment
|
/retest |
|
/verified by ci |
|
@damdo: This PR has been marked as verified by 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. |
|
/label docs-approved /label px-approved |
|
@damdo: 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. |
|
@damdo: Jira Issue OCPBUGS-105398: Some pull requests linked via external trackers have merged: The following pull request, linked via external tracker, has not merged:
All associated pull requests must be merged or unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with Jira Issue OCPBUGS-105398 has not been moved to the MODIFIED state. This PR is marked as verified. If the remaining PRs listed above are marked as verified before merging, the issue will automatically be moved to VERIFIED after all of the changes from the PRs are available in an accepted nightly payload. 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. |
What does this PR do?
Removes the
AzureWorkloadIdentityfeature-gate dependency from thecluster ingress operator.
The feature is GA and enabled by default, so the previously gated Azure
workload identity behavior is now unconditional when no client secret is
available.
Changes include:
Why is this needed?
This must merge before
openshift/api#3018, which removes the feature-gatedefinition. Remaining
.Enabled()consumers would panic once the gate isremoved from the cluster-served feature-gate list.
Related: