Skip to content

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

Closed
damdo wants to merge 1 commit into
openshift:mainfrom
damdo:remove-azure-workload-identity
Closed

damdo wants to merge 1 commit into
openshift:mainfrom
damdo:remove-azure-workload-identity

Conversation

@damdo

@damdo damdo commented Sep 2, 2026

Copy link
Copy Markdown
Member

The Azure File CSI driver operator no longer depends on the AzureWorkloadIdentity feature gate. The operator now configures the Azure workload identity flag unconditionally in both the controller and node operands, and removes the feature-gate accessor and related startup plumbing.

This must land before openshift/api#3018 so this component no longer references the feature gate when the API definition is removed.

Related: openshift/api#3018

Note: I removed the FeatureGate accessor as it was not in use anymore after the AzureWorkloadIdentity feature gate removal

Summary by CodeRabbit

  • New Features

    • Azure Workload Identity is now consistently enabled for Azure file CSI controller and node components.
  • Bug Fixes

    • Improved reliability by removing configuration-dependent behavior that could prevent Azure Workload Identity from being enabled.
    • Simplified operator startup configuration to ensure consistent component setup.

@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 Azure File CSI driver operator no longer depends on the AzureWorkloadIdentity feature gate. The operator now configures the Azure workload identity flag unconditionally in both the controller and node operands, and removes the feature-gate accessor and related startup plumbing.

This must land before openshift/api#3018 so this component no longer references the feature gate when the API definition is removed.

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.

@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

@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 214eb255-43b9-49bc-af55-e079ed47f926

📥 Commits

Reviewing files that changed from the base of the PR and between bb236b6 and 9443df8.

📒 Files selected for processing (3)
  • assets/controller.yaml
  • assets/node.yaml
  • pkg/operator/starter.go

Included review availability: Your plan provides up to 12 included reviews per hour; 9 remain after this review.


Walkthrough

The controller and node assets now enable Azure Workload Identity unconditionally. The operator no longer initializes feature gates or replaces the workload identity setting at runtime.

Changes

Azure Workload Identity Configuration

Layer / File(s) Summary
Enable workload identity in assets
assets/controller.yaml, assets/node.yaml
The credential-injection containers now use true instead of ENABLE_AZURE_WORKLOAD_IDENTITY.
Remove runtime workload identity replacement
pkg/operator/starter.go
RunOperator no longer initializes feature gates or replaces workload identity configuration. Asset replacement only configures the cloud controller manager operator image.

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

Merge Risk: ⚪ Minimal · up to 9443d

This localized refactor removes an obsolete feature gate and enables the existing Azure workload identity configuration unconditionally; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 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 1 functions across 1 files. (2 skipped: 2 … 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 pull request changes two static YAML arguments and removes feature-gate startup code. The diff adds no Ginkgo test declarations or test titles. The repository contains no *_test.go files, …
Test Structure And Quality ✅ Passed PASS — The pull request changes only assets/controller.yaml, assets/node.yaml, and pkg/operator/starter.go. The repository contains no Ginkgo test code or Go test files, and no changed path is a…
Microshift Test Compatibility ✅ Passed PASS: The pull request adds no Ginkgo e2e tests or other test constructs. The committed diff changes only two YAML assets and pkg/operator/starter.go, so the MicroShift test compatibility check is n…
Single Node Openshift (Sno) Test Compatibility ✅ Passed PASS: The pull request changes only two asset YAML files and pkg/operator/starter.go. The diff adds no Ginkgo e2e tests or other test files, so the SNO multi-node compatibility check is not applicable…
Topology-Aware Scheduling Compatibility ✅ Passed PASS: The pull request does not introduce a topology-related scheduling constraint. The exact diff changes only the workload identity argument in the controller and node manifests and removes feature-…
Ote Binary Stdout Contract ✅ Passed The pull request adds no process-level stdout writes. The changed lines set YAML arguments and remove feature-gate startup code. The only project-level logging calls are the existing klog.Info calls…
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No new Ginkgo e2e tests were added. The pull request changes only two operand YAML arguments and removes feature-gate startup code from pkg/operator/starter.go; no changed path is a test file. Exist…
No-Weak-Crypto ✅ Passed PASS — The pull request changes only the workload-identity flag and removes feature-gate startup code. The changed lines introduce no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom crypto, or secret…
Container-Privileges ✅ Passed PASS: The pull request does not introduce any listed container privilege setting. The only manifest changes replace the Azure workload identity argument with true; hostNetwork: true and the two ex…
No-Sensitive-Data-In-Logs ✅ Passed PASS: The pull request introduces no logging. The diff only changes two asset arguments to --enable-azure-workload-identity=true and removes feature-gate startup code. The remaining starter.go log…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: removing the Azure workload identity feature gate from the operator.
Full details: Docstring Coverage

Explanation

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 1 functions across 1 files. (2 skipped: 2 unsupported.)

Full details: Stable And Deterministic Test Names

Explanation

PASS. The pull request changes two static YAML arguments and removes feature-gate startup code. The diff adds no Ginkgo test declarations or test titles. The repository contains no *_test.go files, so the check is not triggered.

Full details: Test Structure And Quality

Explanation

PASS — The pull request changes only assets/controller.yaml, assets/node.yaml, and pkg/operator/starter.go. The repository contains no Ginkgo test code or Go test files, and no changed path is a test path. Therefore, this check has no applicable test structure, cleanup, timeout, or assertion-message issue to flag.

Full details: Microshift Test Compatibility

Explanation

PASS: The pull request adds no Ginkgo e2e tests or other test constructs. The committed diff changes only two YAML assets and pkg/operator/starter.go, so the MicroShift test compatibility check is not applicable.

Full details: Single Node Openshift (Sno) Test Compatibility

Explanation

PASS: The pull request changes only two asset YAML files and pkg/operator/starter.go. The diff adds no Ginkgo e2e tests or other test files, so the SNO multi-node compatibility check is not applicable.

Full details: Topology-Aware Scheduling Compatibility

Explanation

PASS: The pull request does not introduce a topology-related scheduling constraint. The exact diff changes only the workload identity argument in the controller and node manifests and removes feature-gate startup plumbing. Scheduling fields are unchanged: the controller retains preferred anti-affinity with maxUnavailable: 1, the node DaemonSet retains maxUnavailable: 10%, and no new topology spread, replica-count, PDB, node-selector, or toleration logic was added. The existing controller node-role.kubernetes.io/master selector is pre-existing and is not caused by this pull request.

Full details: Ote Binary Stdout Contract

Explanation

The pull request adds no process-level stdout writes. The changed lines set YAML arguments and remove feature-gate startup code. The only project-level logging calls are the existing klog.Info calls in pkg/operator/starter.go; they were present in the parent revision and remain unchanged. The repository has no OTE/Ginkgo suite setup or fmt.Print calls.

Full details: Ipv6 And Disconnected Network Test Compatibility

Explanation

No new Ginkgo e2e tests were added. The pull request changes only two operand YAML arguments and removes feature-gate startup code from pkg/operator/starter.go; no changed path is a test file. Existing 127.0.0.1 values in assets/controller.yaml are unchanged asset configuration, not new tests.

Full details: No-Weak-Crypto

Explanation

PASS — The pull request changes only the workload-identity flag and removes feature-gate startup code. The changed lines introduce no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom crypto, or secret/token comparisons. The existing generic TLS cipher-suite arguments are unchanged.

Full details: Container-Privileges

Explanation

PASS: The pull request does not introduce any listed container privilege setting. The only manifest changes replace the Azure workload identity argument with true; hostNetwork: true and the two existing privileged: true settings in assets/node.yaml are unchanged from the parent revision. No hostPID, hostIPC, SYS_ADMIN, or allowPrivilegeEscalation: true setting exists in the repository. The Go changes remove feature-gate startup code and do not add manifest privileges.

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

Explanation

PASS: The pull request introduces no logging. The diff only changes two asset arguments to --enable-azure-workload-identity=true and removes feature-gate startup code. The remaining starter.go log messages are fixed informational messages (Starting the informers and Starting controllerset) and do not include passwords, tokens, API keys, PII, hostnames, or customer data. The removed feature-gate log reported feature names only.

  • 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.

@damdo
damdo marked this pull request as ready for review September 2, 2026 15:53
@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
@openshift-ci
openshift-ci Bot requested review from mpatlasov and tsmetana September 2, 2026 15:53
@damdo

damdo commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

/assign @jsafrane

@jsafrane

Copy link
Copy Markdown
Contributor

/lgtm
/approve

@jsafrane

Copy link
Copy Markdown
Contributor

wrong repo :-)
/lgtm cancel

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Sep 16, 2026
@openshift-ci

openshift-ci Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: damdo, jsafrane

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

The pull request process is described 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

@openshift-ci openshift-ci Bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed lgtm Indicates that a PR is ready to be merged. labels Sep 16, 2026
@damdo

damdo commented Sep 16, 2026

Copy link
Copy Markdown
Member Author

Closing this in favour of changes directly in openshift/csi-operator

@damdo damdo closed this Sep 16, 2026
@openshift-ci-robot

Copy link
Copy Markdown

@damdo: This pull request references Jira Issue OCPBUGS-105398. The bug has been updated to no longer refer to the pull request using the external bug tracker.

Details

In response to this:

The Azure File CSI driver operator no longer depends on the AzureWorkloadIdentity feature gate. The operator now configures the Azure workload identity flag unconditionally in both the controller and node operands, and removes the feature-gate accessor and related startup plumbing.

This must land before openshift/api#3018 so this component no longer references the feature gate when the API definition is removed.

Related: openshift/api#3018

Note: I removed the FeatureGate accessor as it was not in use anymore after the AzureWorkloadIdentity feature gate removal

Summary by CodeRabbit

  • New Features

  • Azure Workload Identity is now consistently enabled for Azure file CSI controller and node components.

  • Bug Fixes

  • Improved reliability by removing configuration-dependent behavior that could prevent Azure Workload Identity from being enabled.

  • Simplified operator startup configuration to ensure consistent component setup.

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.

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

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. 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.

4 participants