OCPBUGS-105398: refactor: remove Azure workload identity feature gate - #266
OCPBUGS-105398: refactor: remove Azure workload identity feature gate#266damdo wants to merge 2 commits into
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@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. |
|
No actionable comments were generated in the recent review. 🎉 Summary by CodeRabbit
WalkthroughThe controller no longer initializes feature gates or passes them to cloud provider construction. Azure selects workload identity from client-secret and token-file configuration. ChangesCloud provider feature-gate removal
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change removes the obsolete Azure workload identity feature-gate dependency and selects workload identity from configured credentials. No concrete merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
Full details: Stable And Deterministic Test NamesExplanation PASS: The PR changes only controller, cloud-provider, module, and vendored dependency files. The diff contains no test-like paths and no changed Ginkgo title calls such as Full details: Test Structure And QualityExplanation PASS: The pull request changes only controller/provider source files and module metadata. The diff contains no added or modified *_test.go files, no Ginkgo test changes, and no new cluster operations or waits. Existing test-quality issues, if any, are pre-existing and are not attributable to this pull request. Full details: Microshift Test CompatibilityExplanation PASS: The diff from origin/main to HEAD changes only controller source, cloud-provider source, and Go dependency/vendor files. It adds no *_test.go files and no Ginkgo test constructs. Therefore, the MicroShift test compatibility check is not applicable. Full details: Single Node Openshift (Sno) Test CompatibilityExplanation The pull request adds no Ginkgo e2e tests. The direct diff for commit f2eec1a changes only three production Go files, with no *_test.go, e2e, or spec files and no added Describe, Context, When, It, or related constructs. The SNO multi-node test compatibility check is therefore not applicable. Full details: Topology-Aware Scheduling CompatibilityExplanation PASS: The pull request changes controller startup and Azure credential selection only. The focused diff changes feature-gate imports, client construction, and the Azure workload-identity condition; it adds no Deployment or other workload manifest and no affinity, topology spread, node selector/affinity, toleration, replica-count, rollout, or PDB constraint. The repository’s non-vendor file inventory also contains no deployment-manifest surface. Therefore, the topology-aware scheduling failure conditions are not introduced. Full details: Ote Binary Stdout ContractExplanation PASS. The pull request does not introduce an OTE binary or OTE JSON stdout path. The only executable is the cloud-network-config-controller deployment binary, and the functional diff removes feature-gate code and a klog message from main(); it adds no stdout writes or logging setup changes. Existing klog calls and the Ginkgo RunSpecs test are unchanged and cannot cause a pull-request failure under the causality rule. Full details: Ipv6 And Disconnected Network Test CompatibilityExplanation PASS: The check is not applicable. The cumulative diff from main to the PR tip changes implementation and dependency files only. It adds or modifies no *_test.go files and no added Ginkgo declarations such as It(), Describe(), Context(), or When(). Full details: No-Weak-CryptoExplanation PASS: The PR introduces no weak cryptography. The baseline-to-tip diff adds no MD5, SHA-1, DES, 3DES, RC4, Blowfish, ECB, custom crypto, or constant-time comparison changes. The only credential-related addition checks whether the federated token file path is non-empty before selecting Azure workload identity; it does not compare token contents or implement cryptography. The dependency changes remove modules and add no cryptographic implementation. Full details: Container-PrivilegesExplanation No explicit container-privilege failure was introduced. The pull request changes only Go source and dependency metadata; it does not modify the repository Dockerfile or any project-owned Kubernetes manifest. Added diff lines contain none of Full details: No-Sensitive-Data-In-LogsExplanation No sensitive-data logging was introduced. The PR adds only a constructor call, struct initialization, and the workload-identity condition. The changed authentication path logs only the fixed message "Using workload identity authentication". The PR removes the prior feature-gate log and does not log passwords, tokens, API keys, PII, session IDs, hostnames, or customer data.
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: damdo 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 |
|
/retest |
1 similar comment
|
/retest |
|
/assign @mattedallo |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
@damdo: The following test 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. |
The cloud network config controller no longer depends on the AzureWorkloadIdentity feature gate when selecting Azure workload identity authentication. The gate is GA and default-on, so workload identity behavior is now unconditional whenever federated token credentials are configured.
This must land before openshift/api#3018 so the controller no longer references the feature gate as it is removed from the API feature definitions.
Note: I removed the FeatureGate accessor as it was not in use anymore after the AzureWorkloadIdentity feature gate removal