From 0aa7038e0fd965f206c0358050a298b04982ab51 Mon Sep 17 00:00:00 2001 From: Shubham Pampattiwar Date: Tue, 4 Aug 2026 18:54:26 -0700 Subject: [PATCH] feat: use velero SA and add bound-sa-token volume for KDM controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On OpenShift STS clusters, pods authenticate to AWS using projected service account tokens via the Cloud Credential Operator. The KDM controller deployment was using a separate service account that had no IAM trust configured, and was missing the bound-sa-token projected volume needed for STS authentication. Switch the KDM controller deployment to use the velero service account (which already has IAM trust on STS clusters) and add the bound-sa-token projected volume and mount. KDM is always deployed by OADP and never standalone, so a separate SA is unnecessary. Update kustomize configs to bind KDM RBAC permissions to the velero SA. Fixes: migtools/kubevirt-datamover-controller#24 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../oadp-operator.clusterserviceversion.yaml | 252 +++++++++--------- .../kustomization.yaml | 1 - .../leader_election_role_binding.yaml | 2 +- .../metrics_auth_role_binding.yaml | 2 +- .../role_binding.yaml | 2 +- .../kubevirt_datamover_controller.go | 23 +- .../kubevirt_datamover_controller_test.go | 58 +++- 7 files changed, 201 insertions(+), 139 deletions(-) diff --git a/bundle/manifests/oadp-operator.clusterserviceversion.yaml b/bundle/manifests/oadp-operator.clusterserviceversion.yaml index 277dfebb433..ef52f7a9017 100644 --- a/bundle/manifests/oadp-operator.clusterserviceversion.yaml +++ b/bundle/manifests/oadp-operator.clusterserviceversion.yaml @@ -871,130 +871,6 @@ spec: verbs: - get serviceAccountName: non-admin-controller - - rules: - - apiGroups: - - "" - resources: - - persistentvolumeclaims - verbs: - - create - - delete - - get - - list - - patch - - update - - watch - - apiGroups: - - "" - resources: - - persistentvolumes - verbs: - - delete - - get - - list - - patch - - update - - watch - - apiGroups: - - "" - resources: - - pods - verbs: - - create - - delete - - get - - list - - watch - - apiGroups: - - "" - resources: - - pods/log - verbs: - - get - - apiGroups: - - "" - resources: - - secrets - verbs: - - get - - list - - watch - - apiGroups: - - backup.kubevirt.io - resources: - - virtualmachinebackups - - virtualmachinebackuptrackers - verbs: - - create - - delete - - get - - list - - patch - - update - - watch - - apiGroups: - - backup.kubevirt.io - resources: - - virtualmachinebackups/status - verbs: - - get - - apiGroups: - - backup.kubevirt.io - resources: - - virtualmachinebackuptrackers/status - verbs: - - get - - patch - - update - - apiGroups: - - kubevirt.io - resources: - - virtualmachines - verbs: - - get - - list - - watch - - apiGroups: - - velero.io - resources: - - backupstoragelocations - verbs: - - get - - list - - watch - - apiGroups: - - velero.io - resources: - - datadownloads - - datauploads - verbs: - - get - - list - - patch - - update - - watch - - apiGroups: - - velero.io - resources: - - datadownloads/status - - datauploads/status - verbs: - - get - - patch - - update - - apiGroups: - - authentication.k8s.io - resources: - - tokenreviews - verbs: - - create - - apiGroups: - - authorization.k8s.io - resources: - - subjectaccessreviews - verbs: - - create - serviceAccountName: oadp-kubevirt-datamover-controller-manager - rules: - apiGroups: - "" @@ -1373,6 +1249,128 @@ spec: - get - list - delete + - apiGroups: + - "" + resources: + - persistentvolumeclaims + verbs: + - create + - delete + - get + - list + - patch + - update + - watch + - apiGroups: + - "" + resources: + - persistentvolumes + verbs: + - delete + - get + - list + - patch + - update + - watch + - apiGroups: + - "" + resources: + - pods + verbs: + - create + - delete + - get + - list + - watch + - apiGroups: + - "" + resources: + - pods/log + verbs: + - get + - apiGroups: + - "" + resources: + - secrets + verbs: + - get + - list + - watch + - apiGroups: + - backup.kubevirt.io + resources: + - virtualmachinebackups + - virtualmachinebackuptrackers + verbs: + - create + - delete + - get + - list + - patch + - update + - watch + - apiGroups: + - backup.kubevirt.io + resources: + - virtualmachinebackups/status + verbs: + - get + - apiGroups: + - backup.kubevirt.io + resources: + - virtualmachinebackuptrackers/status + verbs: + - get + - patch + - update + - apiGroups: + - kubevirt.io + resources: + - virtualmachines + verbs: + - get + - list + - watch + - apiGroups: + - velero.io + resources: + - backupstoragelocations + verbs: + - get + - list + - watch + - apiGroups: + - velero.io + resources: + - datadownloads + - datauploads + verbs: + - get + - list + - patch + - update + - watch + - apiGroups: + - velero.io + resources: + - datadownloads/status + - datauploads/status + verbs: + - get + - patch + - update + - apiGroups: + - authentication.k8s.io + resources: + - tokenreviews + verbs: + - create + - apiGroups: + - authorization.k8s.io + resources: + - subjectaccessreviews + verbs: + - create - apiGroups: - build.openshift.io - migration.openshift.io @@ -1578,7 +1576,7 @@ spec: verbs: - create - patch - serviceAccountName: oadp-kubevirt-datamover-controller-manager + serviceAccountName: oadp-vm-file-restore-controller-manager - rules: - apiGroups: - "" @@ -1611,7 +1609,7 @@ spec: verbs: - create - patch - serviceAccountName: oadp-vm-file-restore-controller-manager + serviceAccountName: openshift-adp-controller-manager - rules: - apiGroups: - "" @@ -1644,7 +1642,7 @@ spec: verbs: - create - patch - serviceAccountName: openshift-adp-controller-manager + serviceAccountName: velero strategy: deployment installModes: - supported: true diff --git a/config/kubevirt-datamover-controller_rbac/kustomization.yaml b/config/kubevirt-datamover-controller_rbac/kustomization.yaml index fd12e7b6d06..483db265d1e 100644 --- a/config/kubevirt-datamover-controller_rbac/kustomization.yaml +++ b/config/kubevirt-datamover-controller_rbac/kustomization.yaml @@ -5,7 +5,6 @@ resources: # if your manager will use a service account that exists at # runtime. Be sure to update RoleBinding and ClusterRoleBinding # subjects if changing service account names. -- service_account.yaml - role.yaml - role_binding.yaml - leader_election_role.yaml diff --git a/config/kubevirt-datamover-controller_rbac/leader_election_role_binding.yaml b/config/kubevirt-datamover-controller_rbac/leader_election_role_binding.yaml index ead2445b038..ff025fe4f9d 100644 --- a/config/kubevirt-datamover-controller_rbac/leader_election_role_binding.yaml +++ b/config/kubevirt-datamover-controller_rbac/leader_election_role_binding.yaml @@ -11,5 +11,5 @@ roleRef: name: leader-election-role subjects: - kind: ServiceAccount - name: controller-manager + name: velero namespace: system diff --git a/config/kubevirt-datamover-controller_rbac/metrics_auth_role_binding.yaml b/config/kubevirt-datamover-controller_rbac/metrics_auth_role_binding.yaml index e775d67ff08..83fff4d16a9 100644 --- a/config/kubevirt-datamover-controller_rbac/metrics_auth_role_binding.yaml +++ b/config/kubevirt-datamover-controller_rbac/metrics_auth_role_binding.yaml @@ -8,5 +8,5 @@ roleRef: name: metrics-auth-role subjects: - kind: ServiceAccount - name: controller-manager + name: velero namespace: system diff --git a/config/kubevirt-datamover-controller_rbac/role_binding.yaml b/config/kubevirt-datamover-controller_rbac/role_binding.yaml index e75f2397418..83e968d9a02 100644 --- a/config/kubevirt-datamover-controller_rbac/role_binding.yaml +++ b/config/kubevirt-datamover-controller_rbac/role_binding.yaml @@ -11,5 +11,5 @@ roleRef: name: manager-role subjects: - kind: ServiceAccount - name: controller-manager + name: velero namespace: system diff --git a/internal/controller/kubevirt_datamover_controller.go b/internal/controller/kubevirt_datamover_controller.go index fc1efab5f39..1bdce1416c0 100644 --- a/internal/controller/kubevirt_datamover_controller.go +++ b/internal/controller/kubevirt_datamover_controller.go @@ -264,6 +264,11 @@ func ensureKubevirtDatamoverRequiredSpecs( Name: "tmp", MountPath: "/tmp", }, + { + Name: "bound-sa-token", + MountPath: "/var/run/secrets/openshift/serviceaccount", + ReadOnly: true, + }, }, SecurityContext: &corev1.SecurityContext{ AllowPrivilegeEscalation: ptr.To(false), @@ -329,9 +334,25 @@ func ensureKubevirtDatamoverRequiredSpecs( EmptyDir: &corev1.EmptyDirVolumeSource{}, }, }, + { + Name: "bound-sa-token", + VolumeSource: corev1.VolumeSource{ + Projected: &corev1.ProjectedVolumeSource{ + Sources: []corev1.VolumeProjection{ + { + ServiceAccountToken: &corev1.ServiceAccountTokenProjection{ + Audience: "openshift", + ExpirationSeconds: ptr.To(int64(3600)), + Path: "token", + }, + }, + }, + }, + }, + }, } deploymentObject.Spec.Template.Spec.RestartPolicy = corev1.RestartPolicyAlways - deploymentObject.Spec.Template.Spec.ServiceAccountName = kubevirtDatamoverObjectName + deploymentObject.Spec.Template.Spec.ServiceAccountName = "velero" return nil } diff --git a/internal/controller/kubevirt_datamover_controller_test.go b/internal/controller/kubevirt_datamover_controller_test.go index 2db5cb33b52..4c94f820e81 100644 --- a/internal/controller/kubevirt_datamover_controller_test.go +++ b/internal/controller/kubevirt_datamover_controller_test.go @@ -810,8 +810,8 @@ func TestEnsureKubevirtDatamoverRequiredSpecs(t *testing.T) { } // Verify service account name - if deployment.Spec.Template.Spec.ServiceAccountName != kubevirtDatamoverObjectName { - t.Errorf("serviceAccountName: expected %s, got %s", kubevirtDatamoverObjectName, deployment.Spec.Template.Spec.ServiceAccountName) + if deployment.Spec.Template.Spec.ServiceAccountName != "velero" { + t.Errorf("serviceAccountName: expected velero, got %s", deployment.Spec.Template.Spec.ServiceAccountName) } // Verify container @@ -949,6 +949,17 @@ func TestEnsureKubevirtDatamoverRequiredSpecs(t *testing.T) { t.Error("expected /tmp emptyDir volume on pod spec") } + hasBoundSATokenVolume := false + for _, v := range volumes { + if v.Name == "bound-sa-token" && v.VolumeSource.Projected != nil { + hasBoundSATokenVolume = true + break + } + } + if !hasBoundSATokenVolume { + t.Error("expected bound-sa-token projected volume on pod spec") + } + // Verify /tmp volume mount on container (only for new containers) if len(tt.existingContainers) == 0 { hasTmpMount := false @@ -961,6 +972,17 @@ func TestEnsureKubevirtDatamoverRequiredSpecs(t *testing.T) { if !hasTmpMount { t.Error("expected /tmp volumeMount on container") } + + hasBoundSATokenMount := false + for _, vm := range container.VolumeMounts { + if vm.Name == "bound-sa-token" && vm.MountPath == "/var/run/secrets/openshift/serviceaccount" && vm.ReadOnly { + hasBoundSATokenMount = true + break + } + } + if !hasBoundSATokenMount { + t.Error("expected bound-sa-token volumeMount on container") + } } // Verify template labels include control-plane @@ -1009,7 +1031,7 @@ func TestBuildKubevirtDatamoverDeployment(t *testing.T) { expectedImage: defaultKubevirtDatamoverImage, expectedEnvCount: 3, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL (empty) expectedReplicas: 1, - expectedSAName: kubevirtDatamoverObjectName, + expectedSAName: "velero", expectError: false, }, { @@ -1032,7 +1054,7 @@ func TestBuildKubevirtDatamoverDeployment(t *testing.T) { expectedImage: "custom-registry.io/kdm:v1.0", expectedEnvCount: 3, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL (empty) expectedReplicas: 1, - expectedSAName: kubevirtDatamoverObjectName, + expectedSAName: "velero", expectError: false, }, { @@ -1055,7 +1077,7 @@ func TestBuildKubevirtDatamoverDeployment(t *testing.T) { expectedImage: "env-registry.io/kdm:v2.0", expectedEnvCount: 3, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL (empty) expectedReplicas: 1, - expectedSAName: kubevirtDatamoverObjectName, + expectedSAName: "velero", expectError: false, }, { @@ -1078,7 +1100,7 @@ func TestBuildKubevirtDatamoverDeployment(t *testing.T) { expectedImage: defaultKubevirtDatamoverImage, expectedEnvCount: 4, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL, LOG_FORMAT expectedReplicas: 1, - expectedSAName: kubevirtDatamoverObjectName, + expectedSAName: "velero", expectError: false, }, { @@ -1098,7 +1120,7 @@ func TestBuildKubevirtDatamoverDeployment(t *testing.T) { expectedImage: defaultKubevirtDatamoverImage, expectedEnvCount: 3, // WATCH_NAMESPACE, DATAMOVER_IMAGE, LOG_LEVEL (empty) expectedReplicas: 1, - expectedSAName: kubevirtDatamoverObjectName, + expectedSAName: "velero", expectError: false, }, } @@ -1226,6 +1248,28 @@ func TestBuildKubevirtDatamoverDeployment(t *testing.T) { t.Error("expected /tmp volumeMount on container") } + hasBoundSATokenVolume := false + for _, v := range deployment.Spec.Template.Spec.Volumes { + if v.Name == "bound-sa-token" && v.VolumeSource.Projected != nil { + hasBoundSATokenVolume = true + break + } + } + if !hasBoundSATokenVolume { + t.Error("expected bound-sa-token projected volume on pod spec") + } + + hasBoundSATokenMount := false + for _, vm := range container.VolumeMounts { + if vm.Name == "bound-sa-token" && vm.MountPath == "/var/run/secrets/openshift/serviceaccount" && vm.ReadOnly { + hasBoundSATokenMount = true + break + } + } + if !hasBoundSATokenMount { + t.Error("expected bound-sa-token volumeMount on container") + } + // Verify labels labels := deployment.GetLabels() if labels["control-plane"] != kubevirtDatamoverControlPlaneValue {