Skip to content

Support Azure Workload Identity (WIF) in image stream registry env vars #442

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

Open
kaovilai wants to merge 1 commit into
base: oadp-dev
Choose a base branch
from
+324 −3
Open

Support Azure Workload Identity (WIF) in image stream registry env vars #442

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
97 changes: 94 additions & 3 deletions velero-plugins/imagestream/registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package imagestream

import (
"fmt"
"strings"

oadpv1alpha1 "github.com/openshift/oadp-operator/api/v1alpha1"
oadpCreds "github.com/openshift/oadp-operator/pkg/credentials"
Expand Down Expand Up @@ -60,6 +61,19 @@ const (
defaultCredentialsDirectory = "/tmp/credentials"
)

// Keys found in the Azure BSL credential secret (e.g. cloud-credentials-azure).
// The OADP operator STS flow (pkg/credentials/stsflow) provisions a secret
// containing AZURE_CLIENT_ID/AZURE_TENANT_ID/AZURE_SUBSCRIPTION_ID and no
// long-lived credentials (no storage account key, no service principal secret)
// on Azure Workload Identity (WIF) clusters.
const (
AzureCredentialsClientIDKey = "AZURE_CLIENT_ID"
AzureCredentialsTenantIDKey = "AZURE_TENANT_ID"
AzureCredentialsClientSecretKey = "AZURE_CLIENT_SECRET"
AzureCredentialsFederatedTokenFileKey = "AZURE_FEDERATED_TOKEN_FILE"
AzureCredentialsStorageAccountKeyKey = "AZURE_STORAGE_ACCOUNT_ACCESS_KEY"
)

// TODO: remove this map and just define them in each function
// creating skeleton for provider based env var map
var cloudProviderEnvVarMap = map[string][]corev1.EnvVar{
Expand Down Expand Up @@ -197,25 +211,62 @@ func getAWSRegistryEnvVars(bsl *velerov1.BackupStorageLocation) ([]corev1.EnvVar
// https://github.com/vmware-tanzu/velero/blob/5afe837f76aea4dd59b1bf2792e7802d4966f0a7/internal/credentials/file_store.go#L72
// This file is written by velero server on startup
func getBslSecretPath(bsl *velerov1.BackupStorageLocation) string {
selector := getBslSecretKeySelector(bsl)
return fmt.Sprintf("%s/%s/%s-%s", defaultCredentialsDirectory, bsl.Namespace, selector.Name, selector.Key)
}

// getBslSecretKeySelector returns the secret name and key referencing the BSL
// credentials, inheriting from OADP defaults for the provider when the BSL
// does not specify them.
func getBslSecretKeySelector(bsl *velerov1.BackupStorageLocation) *corev1.SecretKeySelector {
var secretName, secretKey string
if bsl.Spec.Credential != nil {
secretName = bsl.Spec.Credential.LocalObjectReference.Name
secretKey = bsl.Spec.Credential.Key
}
// if secretName or secretKey is not set, inherit from OADP defaults for each provider
if bsl.Spec.Credential == nil || secretName == "" {
if secretName == "" {
secretName = oadpCreds.PluginSpecificFields[oadpv1alpha1.DefaultPlugin(bsl.Spec.Provider)].SecretName
}
if bsl.Spec.Credential == nil || secretKey == "" {
if secretKey == "" {
secretKey = oadpCreds.PluginSpecificFields[oadpv1alpha1.DefaultPlugin(bsl.Spec.Provider)].PluginSecretKey
}
return fmt.Sprintf("%s/%s/%s-%s", defaultCredentialsDirectory, bsl.Namespace, secretName, secretKey)
return &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: secretName},
Key: secretKey,
}
}

func getAzureRegistryEnvVars(bsl *velerov1.BackupStorageLocation, azureEnvVars []corev1.EnvVar) ([]corev1.EnvVar, error) {
if bsl.Spec.Config == nil {
bsl.Spec.Config = make(map[string]string)
}
// On Azure Workload Identity (WIF) clusters the OADP operator does not
// create the oadp-<bsl>-azure-registry-secret (there are no long-lived
// credentials to extract), so the account key/SPN env vars below cannot be
// resolved. Set only storage type, container and account name; with no
// accountkey and no credentials parameters the openshift/docker-distribution
// azure storage driver falls through to azidentity.NewDefaultAzureCredential,
// which authenticates via WorkloadIdentityCredential using AZURE_CLIENT_ID,
// AZURE_TENANT_ID and AZURE_FEDERATED_TOKEN_FILE from the Velero pod
// environment (injected by the OADP operator via the
// azure-workload-identity-env secret).
if isAzureWorkloadIdentity(bsl) {
return []corev1.EnvVar{
{
Name: RegistryStorageEnvVarKey,
Value: Azure,
},
{
Name: RegistryStorageAzureContainerEnvVarKey,
Value: bsl.Spec.StorageType.ObjectStorage.Bucket,
},
{
Name: RegistryStorageAzureAccountnameEnvVarKey,
Value: bsl.Spec.Config[StorageAccount],
},
}, nil
}
for i := range azureEnvVars {
if azureEnvVars[i].Name == RegistryStorageAzureContainerEnvVarKey {
azureEnvVars[i].Value = bsl.Spec.StorageType.ObjectStorage.Bucket
Expand Down Expand Up @@ -262,6 +313,46 @@ func getAzureRegistryEnvVars(bsl *velerov1.BackupStorageLocation, azureEnvVars [
return azureEnvVars, nil
}

// isAzureWorkloadIdentity returns true when the BSL credential secret contains
// Azure Workload Identity (WIF/STS) credentials, i.e. no long-lived
// credentials (storage account key or service principal client secret) and
// either a federated token file reference or a client ID + tenant ID pair.
// If the credential secret cannot be read the credential type cannot be
// determined and long-lived credentials are assumed, preserving the previous
// behavior.
func isAzureWorkloadIdentity(bsl *velerov1.BackupStorageLocation) bool {
secretData, err := getSecretKeyRefData(getBslSecretKeySelector(bsl), bsl.Namespace)
if err != nil {
return false
}
creds := parseAzureCredentialsConfig(secretData)
if creds[AzureCredentialsStorageAccountKeyKey] != "" || creds[AzureCredentialsClientSecretKey] != "" {
return false
}
return creds[AzureCredentialsFederatedTokenFileKey] != "" ||
(creds[AzureCredentialsClientIDKey] != "" && creds[AzureCredentialsTenantIDKey] != "")
}

// parseAzureCredentialsConfig parses env-file style azure credentials
// (KEY=value lines, skipping blank lines, comments and [section] headers).
func parseAzureCredentialsConfig(data []byte) map[string]string {
creds := map[string]string{}
for _, line := range strings.Split(string(data), "\n") {
line = strings.TrimSpace(strings.TrimSuffix(line, "\r"))
if line == "" || strings.HasPrefix(line, "#") || strings.HasPrefix(line, "[") {
continue
}
key, value, found := strings.Cut(line, "=")
if !found {
continue
}
value = strings.TrimSpace(value)
value = strings.Trim(value, `"'`)
creds[strings.TrimSpace(key)] = value
}
return creds
}

func getGCPRegistryEnvVars(bsl *velerov1.BackupStorageLocation) ([]corev1.EnvVar, error) {
gcpEnvVars := []corev1.EnvVar{
{
Expand Down
230 changes: 230 additions & 0 deletions velero-plugins/imagestream/registry_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,15 @@ var (
"AZURE_CLIENT_SECRET=" + testClientSecret + "\n" +
"AZURE_RESOURCE_GROUP=" + testResourceGroup),
}
// Azure Workload Identity secret as created by the OADP operator STS flow
// (stsflow.CreateOrUpdateSTSAzureSecret): no storage account key, no client secret.
secretAzureWIFData = map[string][]byte{
"azurekey": []byte("\n" +
"AZURE_SUBSCRIPTION_ID=" + testSubscriptionID + "\n" +
"AZURE_TENANT_ID=" + testTenantID + "\n" +
"AZURE_CLIENT_ID=" + testClientID + "\n" +
"AZURE_CLOUD_NAME=AzurePublicCloud\n"),
}
awsRegistrySecretData = map[string][]byte{
"access_key": []byte(testBslAccessKey),
"secret_key": []byte(testBslSecretAccessKey),
Expand Down Expand Up @@ -772,6 +781,227 @@ func Test_getAzureRegistryEnvVars(t *testing.T) {
}
}

func Test_getAzureRegistryEnvVars_WorkloadIdentity(t *testing.T) {
azureLongLivedEnvVars := func(bsl *velerov1.BackupStorageLocation) []corev1.EnvVar {
registrySecretName := "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"
return []corev1.EnvVar{
{
Name: RegistryStorageEnvVarKey,
Value: Azure,
},
{
Name: RegistryStorageAzureContainerEnvVarKey,
Value: "azure-bucket",
},
{
Name: RegistryStorageAzureAccountnameEnvVarKey,
Value: "velero-azure-account",
},
{
Name: RegistryStorageAzureAccountkeyEnvVarKey,
ValueFrom: &corev1.EnvVarSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: registrySecretName},
Key: "storage_account_key",
},
},
},
{
Name: RegistryStorageAzureAADEndpointEnvVarKey,
Value: "",
},
{
Name: RegistryStorageAzureSPNClientIDEnvVarKey,
ValueFrom: &corev1.EnvVarSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: registrySecretName},
Key: "client_id_key",
},
},
},
{
Name: RegistryStorageAzureSPNClientSecretEnvVarKey,
ValueFrom: &corev1.EnvVarSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: registrySecretName},
Key: "client_secret_key",
},
},
},
{
Name: RegistryStorageAzureSPNTenantIDEnvVarKey,
ValueFrom: &corev1.EnvVarSource{
SecretKeyRef: &corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: registrySecretName},
Key: "tenant_id_key",
},
},
},
}
}
azureWIFBsl := func(credential *corev1.SecretKeySelector) *velerov1.BackupStorageLocation {
return &velerov1.BackupStorageLocation{
ObjectMeta: metav1.ObjectMeta{
Name: "test-bsl",
Namespace: "test-wif-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,
},
Credential: credential,
},
}
}
tests := []struct {
name string
bsl *velerov1.BackupStorageLocation
secret *corev1.Secret
wantRegistryContainerEnvVar []corev1.EnvVar
}{
{
name: "given azure WIF bsl, only storage, container and account name env vars are returned",
bsl: azureWIFBsl(&corev1.SecretKeySelector{
LocalObjectReference: corev1.LocalObjectReference{Name: "cloud-credentials-azure"},
Key: "azurekey",
}),
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-credentials-azure",
Namespace: "test-wif-ns",
},
Data: secretAzureWIFData,
},
wantRegistryContainerEnvVar: []corev1.EnvVar{
{
Name: RegistryStorageEnvVarKey,
Value: Azure,
},
{
Name: RegistryStorageAzureContainerEnvVarKey,
Value: "azure-bucket",
},
{
Name: RegistryStorageAzureAccountnameEnvVarKey,
Value: "velero-azure-account",
},
},
},
{
name: "given azure bsl with storage account key credentials, long-lived env vars are returned",
bsl: azureWIFBsl(nil),
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-credentials-azure",
Namespace: "test-wif-ns",
},
Data: secretAzureData,
},
},
{
name: "given azure bsl with service principal credentials, long-lived env vars are returned",
bsl: azureWIFBsl(nil),
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-credentials-azure",
Namespace: "test-wif-ns",
},
Data: secretAzureServicePrincipalData,
},
},
{
name: "given azure bsl whose credential secret is missing, long-lived env vars are returned",
bsl: azureWIFBsl(nil),
},
}
testEnv := &envtest.Environment{}
cfg, err := testEnv.Start()
if err != nil {
t.Fatal(err)
}
defer testEnv.Stop()
clients.SetInClusterConfig(cfg)
cv1c, err := corev1client.NewForConfig(cfg)
if err != nil {
t.Fatal(err)
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if tt.secret != nil {
cv1c.Namespaces().Create(context.Background(), &corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: tt.secret.Namespace,
},
}, metav1.CreateOptions{})
if _, err := cv1c.Secrets(tt.secret.Namespace).Create(context.Background(), tt.secret, metav1.CreateOptions{}); err != nil {
t.Fatal(err)
}
defer cv1c.Secrets(tt.secret.Namespace).Delete(context.Background(), tt.secret.Name, metav1.DeleteOptions{})
}
if tt.wantRegistryContainerEnvVar == nil {
tt.wantRegistryContainerEnvVar = azureLongLivedEnvVars(tt.bsl)
}

gotRegistryContainerEnvVar, gotErr := getAzureRegistryEnvVars(tt.bsl, cloudProviderEnvVarMap[AzureProvider])

if gotErr != nil {
t.Errorf("getAzureRegistryEnvVars() gotErr = %v", gotErr)
return
}
if !reflect.DeepEqual(tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) {
t.Errorf("expected registry container env var has diff %s", cmp.Diff(tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar))
}
})
}
}

func Test_parseAzureCredentialsConfig(t *testing.T) {
tests := []struct {
name string
data []byte
want map[string]string
}{
{
name: "operator sts flow azurekey format",
data: secretAzureWIFData["azurekey"],
want: map[string]string{
"AZURE_SUBSCRIPTION_ID": testSubscriptionID,
"AZURE_TENANT_ID": testTenantID,
"AZURE_CLIENT_ID": testClientID,
"AZURE_CLOUD_NAME": "AzurePublicCloud",
},
},
{
name: "section headers, comments, quotes, spaces and CRLF are handled",
data: []byte("[default]\r\n# a comment\nAZURE_CLIENT_ID = \"" + testClientID + "\"\r\nAZURE_TENANT_ID='" + testTenantID + "'\nAZURE_CLIENT_SECRET=" + testClientSecret + "=with=equals\nnot-a-key-value\n"),
want: map[string]string{
"AZURE_CLIENT_ID": testClientID,
"AZURE_TENANT_ID": testTenantID,
"AZURE_CLIENT_SECRET": testClientSecret + "=with=equals",
},
},
{
name: "empty data",
data: nil,
want: map[string]string{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := parseAzureCredentialsConfig(tt.data); !reflect.DeepEqual(tt.want, got) {
t.Errorf("parseAzureCredentialsConfig() has diff %s", cmp.Diff(tt.want, got))
}
})
}
}

func Test_getGCPRegistryEnvVars(t *testing.T) {
tests := []struct {
name string
Expand Down