Skip to content

OADP-6675: Add Azure workload identity support for image registry #1952

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: oadp-dev
Choose a base branch
from
Draft

OADP-6675: Add Azure workload identity support for image registry #1952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions internal/controller/nodeagent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,8 +555,7 @@ func (r *DataProtectionApplicationReconciler) customizeNodeAgentDaemonset(ds *ap
nodeAgentContainer.Env = common.AppendUniqueEnvVars(nodeAgentContainer.Env, proxy.ReadProxyVarsFromEnv())

// Add Azure workload identity environment variables if configured
azureClientID := os.Getenv(stsflow.ClientIDEnvKey)
if azureClientID != "" && os.Getenv(stsflow.TenantIDEnvKey) != "" && os.Getenv(stsflow.SubscriptionIDEnvKey) != "" {
if stsflow.AzureIsWorkloadIdentity() {
// Use envFrom to reference the secret containing Azure workload identity env vars
if nodeAgentContainer.EnvFrom == nil {
nodeAgentContainer.EnvFrom = []corev1.EnvFromSource{}
Expand Down
68 changes: 43 additions & 25 deletions internal/controller/registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
oadpv1alpha1 "github.com/openshift/oadp-operator/api/v1alpha1"
"github.com/openshift/oadp-operator/pkg/common"
"github.com/openshift/oadp-operator/pkg/credentials"
"github.com/openshift/oadp-operator/pkg/credentials/stsflow"
)

// Registry Env var keys
Expand All @@ -36,13 +37,13 @@ const (
RegistryStorageS3RootdirectoryEnvVarKey = "REGISTRY_STORAGE_S3_ROOTDIRECTORY"
RegistryStorageS3SkipverifyEnvVarKey = "REGISTRY_STORAGE_S3_SKIPVERIFY"
// Azure registry env vars
RegistryStorageAzureContainerEnvVarKey = "REGISTRY_STORAGE_AZURE_CONTAINER"
RegistryStorageAzureAccountnameEnvVarKey = "REGISTRY_STORAGE_AZURE_ACCOUNTNAME"
RegistryStorageAzureAccountkeyEnvVarKey = "REGISTRY_STORAGE_AZURE_ACCOUNTKEY"
RegistryStorageAzureSPNClientIDEnvVarKey = "REGISTRY_STORAGE_AZURE_SPN_CLIENT_ID"
RegistryStorageAzureSPNClientSecretEnvVarKey = "REGISTRY_STORAGE_AZURE_SPN_CLIENT_SECRET"
RegistryStorageAzureSPNTenantIDEnvVarKey = "REGISTRY_STORAGE_AZURE_SPN_TENANT_ID"
RegistryStorageAzureAADEndpointEnvVarKey = "REGISTRY_STORAGE_AZURE_AAD_ENDPOINT"
RegistryStorageAzureContainerEnvVarKey = "REGISTRY_STORAGE_AZURE_CONTAINER"
RegistryStorageAzureAccountnameEnvVarKey = "REGISTRY_STORAGE_AZURE_ACCOUNTNAME"
RegistryStorageAzureAccountkeyEnvVarKey = "REGISTRY_STORAGE_AZURE_ACCOUNTKEY"
RegistryStorageAzureCredentialsTypeEnvVarKey = "REGISTRY_STORAGE_AZURE_CREDENTIALS_TYPE"
RegistryStorageAzureCredentialsClientIDEnvVarKey = "REGISTRY_STORAGE_AZURE_CREDENTIALS_CLIENTID"
RegistryStorageAzureCredentialsSecretEnvVarKey = "REGISTRY_STORAGE_AZURE_CREDENTIALS_SECRET"
RegistryStorageAzureCredentialsTenantIDEnvVarKey = "REGISTRY_STORAGE_AZURE_CREDENTIALS_TENANTID"
// GCP registry env vars
RegistryStorageGCSBucket = "REGISTRY_STORAGE_GCS_BUCKET"
RegistryStorageGCSKeyfile = "REGISTRY_STORAGE_GCS_KEYFILE"
Expand Down Expand Up @@ -117,19 +118,19 @@ var cloudProviderEnvVarMap = map[string][]corev1.EnvVar{
Value: "",
},
{
Name: RegistryStorageAzureAADEndpointEnvVarKey,
Value: "",
Name: RegistryStorageAzureCredentialsTypeEnvVarKey,
Value: "", // Will be set dynamically based on auth method
},
{
Name: RegistryStorageAzureSPNClientIDEnvVarKey,
Name: RegistryStorageAzureCredentialsClientIDEnvVarKey,
Value: "",
},
{
Name: RegistryStorageAzureSPNClientSecretEnvVarKey,
Name: RegistryStorageAzureCredentialsSecretEnvVarKey,
Value: "",
},
{
Name: RegistryStorageAzureSPNTenantIDEnvVarKey,
Name: RegistryStorageAzureCredentialsTenantIDEnvVarKey,
Value: "",
},
},
Expand Down Expand Up @@ -784,6 +785,16 @@ func (r *DataProtectionApplicationReconciler) populateAWSRegistrySecret(bsl *vel
}

func (r *DataProtectionApplicationReconciler) populateAzureRegistrySecret(bsl *velerov1.BackupStorageLocation, registrySecret *corev1.Secret) error {
// Check if Azure workload identity is configured
isWorkloadIdentity := stsflow.AzureIsWorkloadIdentity()

// Determine authentication type
var credentialsType string
if isWorkloadIdentity {
credentialsType = "default_credentials"
r.Log.Info("Azure workload identity detected, using default_credentials for registry")
}

// Check for secret name
secretName, secretKey, _ := r.getSecretNameAndKey(bsl.Spec.Config, bsl.Spec.Credential, oadpv1alpha1.DefaultPluginMicrosoftAzure)

Expand All @@ -794,25 +805,31 @@ func (r *DataProtectionApplicationReconciler) populateAzureRegistrySecret(bsl *v
return err
}

// parse the secret and get azure storage account key
// parse the secret and get azure credentials
azcreds, err := r.parseAzureSecret(secret, secretKey)
if err != nil {
r.Log.Info(fmt.Sprintf("Error parsing provider secret %s for backupstoragelocation %s/%s", secretName, bsl.Namespace, bsl.Name))
return err
}

if len(bsl.Spec.Config["storageAccountKeyEnvVar"]) != 0 {
if azcreds.strorageAccountKey == "" {
r.Log.Info("Expecting storageAccountKeyEnvVar value set present in the credentials")
return errors.New("no strorageAccountKey value present in credentials file")
}
} else {
if len(azcreds.subscriptionID) == 0 &&
len(azcreds.tenantID) == 0 &&
len(azcreds.clientID) == 0 &&
len(azcreds.clientSecret) == 0 &&
len(azcreds.resourceGroup) == 0 {
return errors.New("error finding service principal parameters for the supplied Azure credential")
// Determine credentials type based on available credentials
if credentialsType == "" {
if len(bsl.Spec.Config["storageAccountKeyEnvVar"]) != 0 {
if azcreds.strorageAccountKey == "" {
r.Log.Info("Expecting storageAccountKeyEnvVar value set present in the credentials")
return errors.New("no strorageAccountKey value present in credentials file")
}
credentialsType = "shared_key"
} else if azcreds.clientID != "" && azcreds.clientSecret != "" && azcreds.tenantID != "" {
credentialsType = "client_secret"
} else {
if len(azcreds.subscriptionID) == 0 &&
len(azcreds.tenantID) == 0 &&
len(azcreds.clientID) == 0 &&
len(azcreds.clientSecret) == 0 &&
len(azcreds.resourceGroup) == 0 {
return errors.New("error finding service principal parameters for the supplied Azure credential")
}
}
}

Expand All @@ -823,6 +840,7 @@ func (r *DataProtectionApplicationReconciler) populateAzureRegistrySecret(bsl *v
"client_id_key": []byte(azcreds.clientID),
"client_secret_key": []byte(azcreds.clientSecret),
"resource_group_key": []byte(azcreds.resourceGroup),
"credentials_type": []byte(credentialsType),
}

return nil
Expand Down
180 changes: 174 additions & 6 deletions internal/controller/registry_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"k8s.io/client-go/tools/record"

oadpv1alpha1 "github.com/openshift/oadp-operator/api/v1alpha1"
"github.com/openshift/oadp-operator/pkg/credentials/stsflow"
)

func getSchemeForFakeClientForRegistry() (*runtime.Scheme, error) {
Expand Down Expand Up @@ -121,7 +122,6 @@ var (
}
secretAzureServicePrincipalData = map[string][]byte{
"cloud": []byte("[default]" + "\n" +
"AZURE_STORAGE_ACCOUNT_ACCESS_KEY=" + testStoragekey + "\n" +
"AZURE_CLOUD_NAME=" + testCloudName + "\n" +
"AZURE_SUBSCRIPTION_ID=" + testSubscriptionID + "\n" +
"AZURE_TENANT_ID=" + testTenantID + "\n" +
Expand All @@ -140,6 +140,7 @@ var (
"storage_account_key": []byte(testStoragekey),
"subscription_id_key": []byte(""),
"tenant_id_key": []byte(""),
"credentials_type": []byte("shared_key"),
}
azureRegistrySPSecretData = map[string][]byte{
"client_id_key": []byte(testClientID),
Expand Down Expand Up @@ -440,9 +441,11 @@ func TestDPAReconciler_populateAzureRegistrySecret(t *testing.T) {
azureSecret *corev1.Secret
dpa *oadpv1alpha1.DataProtectionApplication
wantErr bool
wantSecretData map[string][]byte
envVars map[string]string
}{
{
name: "Given Velero CR and bsl instance, appropriate registry secret is updated for azure case",
name: "Given Velero CR and bsl instance with storage account key, appropriate registry secret is updated",
wantErr: false,
bsl: &velerov1.BackupStorageLocation{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -483,10 +486,175 @@ func TestDPAReconciler_populateAzureRegistrySecret(t *testing.T) {
Namespace: "test-ns",
},
},
wantSecretData: azureRegistrySecretData,
},
{
name: "Azure workload identity detected - uses default_credentials",
wantErr: false,
envVars: map[string]string{
stsflow.ClientIDEnvKey: "test-client-id",
stsflow.TenantIDEnvKey: "test-tenant-id",
stsflow.SubscriptionIDEnvKey: "test-subscription-id",
},
bsl: &velerov1.BackupStorageLocation{
ObjectMeta: metav1.ObjectMeta{
Name: "test-bsl-workload",
Namespace: "test-ns",
},
Spec: velerov1.BackupStorageLocationSpec{
Provider: AzureProvider,
StorageType: velerov1.StorageType{
ObjectStorage: &velerov1.ObjectStorageLocation{
Bucket: "azure-bucket",
},
},
Config: map[string]string{
StorageAccount: "velero-azure-account",
ResourceGroup: testResourceGroup,
},
},
},
dpa: &oadpv1alpha1.DataProtectionApplication{
ObjectMeta: metav1.ObjectMeta{
Name: "Velero-test-CR",
Namespace: "test-ns",
},
},
azureSecret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-credentials-azure",
Namespace: "test-ns",
},
Data: secretAzureServicePrincipalData,
},
registrySecret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "oadp-test-bsl-workload-azure-registry-secret",
Namespace: "test-ns",
},
},
wantSecretData: map[string][]byte{
"client_id_key": []byte(testClientID),
"client_secret_key": []byte(testClientSecret),
"resource_group_key": []byte(testResourceGroup),
"storage_account_key": []byte(""),
"subscription_id_key": []byte(testSubscriptionID),
"tenant_id_key": []byte(testTenantID),
"credentials_type": []byte("default_credentials"),
},
},
{
name: "Service principal credentials - uses client_secret",
wantErr: false,
bsl: &velerov1.BackupStorageLocation{
ObjectMeta: metav1.ObjectMeta{
Name: "test-bsl-sp",
Namespace: "test-ns",
},
Spec: velerov1.BackupStorageLocationSpec{
Provider: AzureProvider,
StorageType: velerov1.StorageType{
ObjectStorage: &velerov1.ObjectStorageLocation{
Bucket: "azure-bucket",
},
},
Config: map[string]string{
StorageAccount: "velero-azure-account",
ResourceGroup: testResourceGroup,
},
},
},
dpa: &oadpv1alpha1.DataProtectionApplication{
ObjectMeta: metav1.ObjectMeta{
Name: "Velero-test-CR",
Namespace: "test-ns",
},
},
azureSecret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-credentials-azure",
Namespace: "test-ns",
},
Data: secretAzureServicePrincipalData,
},
registrySecret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "oadp-test-bsl-sp-azure-registry-secret",
Namespace: "test-ns",
},
},
wantSecretData: map[string][]byte{
"client_id_key": []byte(testClientID),
"client_secret_key": []byte(testClientSecret),
"resource_group_key": []byte(testResourceGroup),
"storage_account_key": []byte(""),
"subscription_id_key": []byte(testSubscriptionID),
"tenant_id_key": []byte(testTenantID),
"credentials_type": []byte("client_secret"),
},
},
{
name: "Partial workload identity env vars - falls back to service principal",
wantErr: false,
envVars: map[string]string{
stsflow.ClientIDEnvKey: "test-client-id",
stsflow.TenantIDEnvKey: "test-tenant-id",
// Missing SUBSCRIPTIONID
},
bsl: &velerov1.BackupStorageLocation{
ObjectMeta: metav1.ObjectMeta{
Name: "test-bsl-partial",
Namespace: "test-ns",
},
Spec: velerov1.BackupStorageLocationSpec{
Provider: AzureProvider,
StorageType: velerov1.StorageType{
ObjectStorage: &velerov1.ObjectStorageLocation{
Bucket: "azure-bucket",
},
},
Config: map[string]string{
StorageAccount: "velero-azure-account",
ResourceGroup: testResourceGroup,
},
},
},
dpa: &oadpv1alpha1.DataProtectionApplication{
ObjectMeta: metav1.ObjectMeta{
Name: "Velero-test-CR",
Namespace: "test-ns",
},
},
azureSecret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-credentials-azure",
Namespace: "test-ns",
},
Data: secretAzureServicePrincipalData,
},
registrySecret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "oadp-test-bsl-partial-azure-registry-secret",
Namespace: "test-ns",
},
},
wantSecretData: map[string][]byte{
"client_id_key": []byte(testClientID),
"client_secret_key": []byte(testClientSecret),
"resource_group_key": []byte(testResourceGroup),
"storage_account_key": []byte(""),
"subscription_id_key": []byte(testSubscriptionID),
"tenant_id_key": []byte(testTenantID),
"credentials_type": []byte("client_secret"),
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Set environment variables for test
for key, value := range tt.envVars {
t.Setenv(key, value)
}
fakeClient, err := getFakeClientFromObjects(tt.azureSecret, tt.dpa)
if err != nil {
t.Errorf("error in creating fake client, likely programmer error")
Expand All @@ -510,13 +678,13 @@ func TestDPAReconciler_populateAzureRegistrySecret(t *testing.T) {
oadpv1alpha1.OadpOperatorLabel: "True",
},
},
Data: azureRegistrySecretData,
Data: tt.wantSecretData,
}
if err := r.populateAzureRegistrySecret(tt.bsl, tt.registrySecret); (err != nil) != tt.wantErr {
t.Errorf("populateAzureRegistrySecret() error = %v, wantErr %v", err, tt.wantErr)
}
if !reflect.DeepEqual(tt.registrySecret.Data, wantRegistrySecret.Data) {
t.Errorf("expected azure registry secret to be %#v, got %#v", tt.registrySecret, wantRegistrySecret.Data)
if !tt.wantErr && !reflect.DeepEqual(tt.registrySecret.Data, wantRegistrySecret.Data) {
t.Errorf("expected azure registry secret to be %#v, got %#v", wantRegistrySecret.Data, tt.registrySecret.Data)
}
})
}
Expand Down Expand Up @@ -683,7 +851,7 @@ func TestDPAReconciler_parseAzureSecret(t *testing.T) {
},
secretKey: "cloud",
wantCreds: azureCredentials{
strorageAccountKey: testStoragekey,
strorageAccountKey: "",
subscriptionID: testSubscriptionID,
tenantID: testTenantID,
clientID: testClientID,
Expand Down
7 changes: 4 additions & 3 deletions internal/controller/stsflow.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,16 @@ import (
// eliminating the need for temporary credential files as used by AWS/GCP providers.
func (r *DataProtectionApplicationReconciler) ReconcileAzureWorkloadIdentitySecret(log logr.Logger) (bool, error) {
dpa := r.dpa
azureClientID := os.Getenv(stsflow.ClientIDEnvKey)

// Only create secret if Azure workload identity environment variables are present
azureTenantID := os.Getenv(stsflow.TenantIDEnvKey)
if azureClientID == "" || azureTenantID == "" || os.Getenv(stsflow.SubscriptionIDEnvKey) == "" {
if !stsflow.AzureIsWorkloadIdentity() {
// No Azure workload identity configured, nothing to do
return true, nil
}

azureClientID := os.Getenv(stsflow.ClientIDEnvKey)
azureTenantID := os.Getenv(stsflow.TenantIDEnvKey)

secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: stsflow.AzureWorkloadIdentitySecretName,
Expand Down
Loading