add Azure Workload Identity support to kubevirt-datamover controller - #2362
add Azure Workload Identity support to kubevirt-datamover controller#2362sseago wants to merge 1 commit into
Conversation
…oller Co-authored-by: aider (gemini/gemini-3.1-pro-preview) <aider@aider.chat>
WalkthroughThe KubeVirt DataMover controller now supports Azure Workload Identity STS configuration. It adds the configured Secret reference when all required Azure environment variables are present, during both container creation and reconciliation. ChangesAzure Workload Identity support
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@internal/controller/kubevirt_datamover_controller.go`:
- Around line 183-196: Update the KubeVirt DataMover Pod template construction
alongside the Azure workload identity envFrom setup to include an annotation
derived from the referenced Azure Secret’s resource version or content hash.
Ensure this annotation changes whenever the Secret changes, triggering
Deployment rollout and fresh environment values in existing Pods; add coverage
for Azure identity rotation.
🪄 Autofix
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: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: f8995c80-9d6b-4396-ae53-79d4b276049b
📒 Files selected for processing (1)
internal/controller/kubevirt_datamover_controller.go
| // Add Azure workload identity environment variables if configured | ||
| var envFrom []corev1.EnvFromSource | ||
| azureClientID := os.Getenv(stsflow.ClientIDEnvKey) | ||
| if azureClientID != "" && os.Getenv(stsflow.TenantIDEnvKey) != "" && os.Getenv(stsflow.SubscriptionIDEnvKey) != "" { | ||
| envFrom = append(envFrom, corev1.EnvFromSource{ | ||
| SecretRef: &corev1.SecretEnvSource{ | ||
| LocalObjectReference: corev1.LocalObjectReference{ | ||
| Name: stsflow.AzureWorkloadIdentitySecretName, | ||
| }, | ||
| }, | ||
| }) | ||
| r.Log.Info("Added Azure workload identity secret reference to KubeVirt DataMover container") | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 \
'ReconcileAzureWorkloadIdentitySecret|AzureWorkloadIdentitySecretName|AZURE_(CLIENT_ID|TENANT_ID|FEDERATED_TOKEN_FILE)|EnvFrom|ResourceVersion|resource-version|revision|sha256' \
--glob '*.go' .Repository: openshift/oadp-operator
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- target controller outline ---'
ast-grep outline internal/controller/kubevirt_datamover_controller.go
echo '--- target controller relevant symbols ---'
rg -n -C 12 'AzureWorkloadIdentitySecretName|ClientIDEnvKey|TenantIDEnvKey|SubscriptionIDEnvKey|EnvFrom|DataMover|Deployment|Reconcile' internal/controller/kubevirt_datamover_controller.go
echo '--- STS reconciler symbols and secret updates ---'
rg -n -C 14 'ReconcileAzureWorkloadIdentitySecret|CreateOrUpdateSTSSecretWithClientsAndWait|AzureWorkloadIdentitySecretName|ClientIDEnvKey|TenantIDEnvKey|SubscriptionIDEnvKey|FEDERATED_TOKEN' pkg/credentials/stsflow internal/controller
echo '--- target controller tests and callers ---'
rg -n -C 10 'KubeVirt|DataMover|AzureWorkloadIdentity|EnvFrom|SecretRef' internal/controller/*test.goRepository: openshift/oadp-operator
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- target deployment construction ---'
sed -n '183,220p' internal/controller/kubevirt_datamover_controller.go
sed -n '260,340p' internal/controller/kubevirt_datamover_controller.go
echo '--- Azure STS controller ---'
wc -l internal/controller/stsflow.go
cat -n internal/controller/stsflow.go
echo '--- STS secret update implementation ---'
sed -n '85,145p' pkg/credentials/stsflow/stsflow.go
sed -n '224,330p' pkg/credentials/stsflow/stsflow.go
echo '--- all call sites for Azure STS reconciliation and KubeVirt reconciliation ---'
rg -n -C 8 'ReconcileAzureWorkloadIdentitySecret|STSStandardizedFlow|CreateOrUpdateSTSSecretWithClients|ReconcileKubevirtDatamoverController' --glob '*.go' .Repository: openshift/oadp-operator
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- controller setup and owned-resource watches ---'
sed -n '55,135p' internal/controller/dataprotectionapplication_controller.go
rg -n -C 8 'Owns\(|For\(|Complete\(|SetupWithManager|Secret' internal/controller/dataprotectionapplication_controller.go
echo '--- KubeVirt controller tests for Azure envFrom and template changes ---'
rg -n -C 12 'CLIENTID|TENANTID|SUBSCRIPTIONID|AZURE|EnvFrom|Annotations|PodTemplate|ResourceVersion' internal/controller/kubevirt_datamover_controller_test.go
echo '--- source-level invariant probe ---'
python3 - <<'PY'
from pathlib import Path
p = Path("internal/controller/kubevirt_datamover_controller.go").read_text()
start = p.index("func ensureKubevirtDatamoverRequiredSpecs")
end = p.index("\nfunc ", start + 5)
fn = p[start:end]
checks = {
"reads operator Azure environment": 'os.Getenv(stsflow.ClientIDEnvKey)' in fn,
"references Azure Secret through EnvFrom": 'Name: stsflow.AzureWorkloadIdentitySecretName' in fn,
"assigns EnvFrom to manager": 'kubevirtDatamoverContainer.EnvFrom = envFrom' in fn,
"reads Secret resource version": '.ResourceVersion' in fn,
"gets Secret from Kubernetes API": 'r.Get(' in fn,
"adds Secret content revision": 'sha256' in fn or 'revision' in fn,
}
for name, value in checks.items():
print(f"{name}: {value}")
PYRepository: openshift/oadp-operator
Length of output: 25295
🌐 Web query:
Kubernetes official documentation Secret environment variables not updated automatically running container
💡 Result:
In Kubernetes, environment variables injected into a container from a Secret (or ConfigMap) are not updated automatically when the underlying Secret is modified [1][2]. These environment variables are only set when the Pod is created [1][3]. Because they are part of the process environment, they cannot be changed without terminating and recreating the container or the Pod [1][4]. If you need to update the values used by your application, you have the following options: 1. Perform a rolling update: The standard way to propagate changes is to trigger a rollout of the Deployment [1][4]. This can be done by running: kubectl rollout restart deployment This command replaces existing Pods with new ones that will pull the updated Secret values [1][5]. 2. Use Volumes instead of environment variables: If your application can be configured to read values from files, mounting the Secret as a volume is an alternative [6][2]. When a Secret is mounted as a volume, Kubernetes automatically updates the projected files (after a brief delay for kubelet synchronization), which allows many applications to detect the change and reload the configuration without requiring a Pod restart [4][2]. Note: Simply updating the Secret does not affect running Pods [1][4]. If you do not perform a rollout or recreate the Pods, the containers will continue to use the stale environment variable values from their initial startup [1][5].
Citations:
- 1: https://kubernetes.io/docs/tutorials/configuration/updating-configuration-via-a-configmap/
- 2: https://kubernetes.io/docs/concepts/configuration/configmap/
- 3: https://kubernetes.io/docs/tasks/inject-data-application/define-environment-variable-container/
- 4: https://stackoverflow.com/questions/37945800/update-kubernetes-secrets-doesnt-update-running-container-env-vars
- 5: Secret referenced with envFrom in a Deployment not reloaded after rollout restart kubernetes/kubernetes#130769
- 6: https://kubernetes.io/docs/concepts/configuration/secret/
Add a rollout trigger for Azure Secret changes.
envFrom values are set only when the Pod starts. Secret updates enqueue DPA reconciliation, but the Deployment template keeps the same SecretRef, so existing KubeVirt DataMover Pods retain stale Azure identity values. Add a Secret resource-version or content-hash annotation to the Pod template and test Azure identity rotation.
🤖 Prompt for 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.
In `@internal/controller/kubevirt_datamover_controller.go` around lines 183 - 196,
Update the KubeVirt DataMover Pod template construction alongside the Azure
workload identity envFrom setup to include an annotation derived from the
referenced Azure Secret’s resource version or content hash. Ensure this
annotation changes whenever the Secret changes, triggering Deployment rollout
and fresh environment values in existing Pods; add coverage for Azure identity
rotation.
Source: MCP tools
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: shubham-pampattiwar, sseago, weshayutin 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 |
|
/cherry-pick oadp-1.6 |
|
@shubham-pampattiwar: once the present PR merges, I will cherry-pick it on top of 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 kubernetes-sigs/prow repository. |
|
/retest-required |
|
@sseago: The following tests 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. |
4 participants
Why the changes were made
For Azure Workload Identity, the same AZURE env vars we're adding to the Velero deployment also need to be added to the KDM controller.
migtools/kubevirt-datamover-controller#98
How to test the changes made
In an Azure STS cluster, the following env vars should be added to the KDM controller
Summary by CodeRabbit