Skip to content

OCPBUGS-105398: refactor: remove Azure workload identity feature gate - #266

Open
damdo wants to merge 2 commits into
openshift:mainfrom
damdo:remove-azure-workload-identity
Open

OCPBUGS-105398: refactor: remove Azure workload identity feature gate#266
damdo wants to merge 2 commits into
openshift:mainfrom
damdo:remove-azure-workload-identity

Conversation

@damdo

@damdo damdo commented Sep 2, 2026

Copy link
Copy Markdown
Member

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

@openshift-merge-bot

Copy link
Copy Markdown
Contributor

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: LGTM mode

@openshift-ci-robot openshift-ci-robot added jira/severity-low Referenced Jira bug's severity is low for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. labels Sep 2, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@damdo: This pull request references Jira Issue OCPBUGS-105398, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (5.1.0) matches configured target version for branch (5.1.0)
  • bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

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.

Related: openshift/api#3018

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.

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉


Summary by CodeRabbit

  • Authentication
    • Azure cloud authentication uses workload identity when no client secret is provided and a federated token file is configured.
    • Workload identity selection no longer depends on a separate feature setting.
  • Simplification
    • Cloud provider startup uses the available platform configuration directly, reducing configuration requirements during initialization.
    • Startup no longer waits for feature-gate configuration before initializing cloud provider services.

Walkthrough

The controller no longer initializes feature gates or passes them to cloud provider construction. Azure selects workload identity from client-secret and token-file configuration.

Changes

Cloud provider feature-gate removal

Layer / File(s) Summary
Cloud provider authentication changes
pkg/cloudprovider/cloudprovider.go, pkg/cloudprovider/azure.go
NewCloudProviderClient no longer accepts feature gates. Azure selects federated authentication when no client secret is configured and a token file is present.
Controller startup cleanup
cmd/cloud-network-config-controller/main.go, go.mod
The controller no longer initializes or waits for feature gates. Unused feature-gate-related dependencies were removed.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to d153e

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)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (14 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed 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 It, Describe, Context,…
Test Structure And Quality ✅ Passed 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 …
Microshift Test Compatibility ✅ Passed 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 …
Single Node Openshift (Sno) Test Compatibility ✅ Passed 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, o…
Topology-Aware Scheduling Compatibility ✅ Passed 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…
Ote Binary Stdout Contract ✅ Passed 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-g…
Ipv6 And Disconnected Network Test Compatibility ✅ Passed 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…
No-Weak-Crypto ✅ Passed 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-relate…
Container-Privileges ✅ Passed 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 ma…
No-Sensitive-Data-In-Logs ✅ Passed 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 "…
Description check ✅ Passed The description clearly explains removal of the AzureWorkloadIdentity feature gate and the resulting unconditional workload identity behavior.
Title check ✅ Passed The title clearly identifies the refactor that removes the Azure workload identity feature gate.
Full details: Stable And Deterministic Test Names

Explanation

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 It, Describe, Context, or When. Therefore, the PR introduces no unstable or overly specific test name.

Full details: Test Structure And Quality

Explanation

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 Compatibility

Explanation

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 Compatibility

Explanation

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 Compatibility

Explanation

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 Contract

Explanation

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 Compatibility

Explanation

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-Crypto

Explanation

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-Privileges

Explanation

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 privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation. The Dockerfile is unchanged and declares no privilege settings.

Full details: No-Sensitive-Data-In-Logs

Explanation

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.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 2, 2026
@openshift-ci

openshift-ci Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci

openshift-ci Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: damdo
Once this PR has been reviewed and has the lgtm label, please assign jcaamano for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@damdo
damdo marked this pull request as ready for review September 2, 2026 15:52
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 2, 2026
@damdo

damdo commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

/retest

1 similar comment
@damdo

damdo commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

/retest

@damdo

damdo commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

/assign @mattedallo

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

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.

@openshift-ci

openshift-ci Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

@damdo: The following test 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/unit d153ebe link true /test unit

Full PR test history. Your PR dashboard.

Details

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/severity-low Referenced Jira bug's severity is low for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants