From adf11bddc762d8f49d6fccb92bf047ec066cff42 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Thu, 20 Jul 2023 23:33:21 -0400 Subject: [PATCH] add sts cred secret handling Signed-off-by: Tiger Kaovilai --- go.mod | 2 +- velero-plugins/clients/clients.go | 4 + velero-plugins/clients/mock/mock.go | 7 + velero-plugins/imagestream/registry.go | 108 ++++++--- velero-plugins/imagestream/registry_test.go | 256 ++++++++++++++------ velero-plugins/imagestream/shared.go | 8 +- 6 files changed, 264 insertions(+), 121 deletions(-) create mode 100644 velero-plugins/clients/mock/mock.go diff --git a/go.mod b/go.mod index 3d7ea6be..04755bcf 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/bombsimon/logrusr/v3 v3.0.0 github.com/containers/image/v5 v5.26.1 github.com/go-logr/logr v1.2.4 - github.com/google/go-cmp v0.5.9 // indirect + github.com/google/go-cmp v0.5.9 github.com/hashicorp/go-hclog v1.0.0 // indirect github.com/kaovilai/udistribution v0.0.10-oadp-1.2 github.com/mitchellh/mapstructure v1.5.0 // indirect diff --git a/velero-plugins/clients/clients.go b/velero-plugins/clients/clients.go index 41bbe144..e3e33e38 100644 --- a/velero-plugins/clients/clients.go +++ b/velero-plugins/clients/clients.go @@ -34,6 +34,10 @@ var buildClientError error var inClusterConfig *rest.Config +func SetInClusterConfig(config *rest.Config) { + inClusterConfig = config +} + func GetInClusterConfig() (*rest.Config, error) { if inClusterConfig != nil { return inClusterConfig, nil diff --git a/velero-plugins/clients/mock/mock.go b/velero-plugins/clients/mock/mock.go new file mode 100644 index 00000000..cc5a4e53 --- /dev/null +++ b/velero-plugins/clients/mock/mock.go @@ -0,0 +1,7 @@ +package mock + +import "k8s.io/client-go/rest" + +func MockInClusterConfig() (*rest.Config, error) { + return nil, nil +} \ No newline at end of file diff --git a/velero-plugins/imagestream/registry.go b/velero-plugins/imagestream/registry.go index 38dc9f55..055917d3 100644 --- a/velero-plugins/imagestream/registry.go +++ b/velero-plugins/imagestream/registry.go @@ -2,6 +2,8 @@ package imagestream import ( "errors" + "regexp" + "strings" "github.com/openshift/oadp-operator/pkg/credentials" @@ -13,14 +15,15 @@ import ( // Registry Env var keys const ( // AWS registry env vars - RegistryStorageEnvVarKey = "REGISTRY_STORAGE" - RegistryStorageS3AccesskeyEnvVarKey = "REGISTRY_STORAGE_S3_ACCESSKEY" - RegistryStorageS3BucketEnvVarKey = "REGISTRY_STORAGE_S3_BUCKET" - RegistryStorageS3RegionEnvVarKey = "REGISTRY_STORAGE_S3_REGION" - RegistryStorageS3SecretkeyEnvVarKey = "REGISTRY_STORAGE_S3_SECRETKEY" - RegistryStorageS3RegionendpointEnvVarKey = "REGISTRY_STORAGE_S3_REGIONENDPOINT" - RegistryStorageS3RootdirectoryEnvVarKey = "REGISTRY_STORAGE_S3_ROOTDIRECTORY" - RegistryStorageS3SkipverifyEnvVarKey = "REGISTRY_STORAGE_S3_SKIPVERIFY" + RegistryStorageEnvVarKey = "REGISTRY_STORAGE" + RegistryStorageS3AccesskeyEnvVarKey = "REGISTRY_STORAGE_S3_ACCESSKEY" + RegistryStorageS3BucketEnvVarKey = "REGISTRY_STORAGE_S3_BUCKET" + RegistryStorageS3RegionEnvVarKey = "REGISTRY_STORAGE_S3_REGION" + RegistryStorageS3SecretkeyEnvVarKey = "REGISTRY_STORAGE_S3_SECRETKEY" + RegistryStorageS3CredentialsConfigPathEnvVarKey = "REGISTRY_STORAGE_S3_CREDENTIALSCONFIGPATH" + RegistryStorageS3RegionendpointEnvVarKey = "REGISTRY_STORAGE_S3_REGIONENDPOINT" + RegistryStorageS3RootdirectoryEnvVarKey = "REGISTRY_STORAGE_S3_ROOTDIRECTORY" + RegistryStorageS3SkipverifyEnvVarKey = "REGISTRY_STORAGE_S3_SKIPVERIFY" // Azure registry env vars RegistryStorageAzureContainerEnvVarKey = "REGISTRY_STORAGE_AZURE_CONTAINER" RegistryStorageAzureAccountnameEnvVarKey = "REGISTRY_STORAGE_AZURE_ACCOUNTNAME" @@ -35,7 +38,7 @@ const ( RegistryStorageGCSRootdirectory = "REGISTRY_STORAGE_GCS_ROOTDIRECTORY" ) -// provider specific object storage +// provider specific object storage config const ( S3 = "s3" Azure = "azure" @@ -50,6 +53,12 @@ const ( InsecureSkipTLSVerify = "insecureSkipTLSVerify" StorageAccount = "storageAccount" ResourceGroup = "resourceGroup" + enableSharedConfig = "enableSharedConfig" +) + +// secret data keys +const ( + webIdentityTokenFile = "web_identity_token_file" ) // TODO: remove this map and just define them in each function @@ -120,25 +129,25 @@ type azureCredentials struct { } func getRegistryEnvVars(bsl *velerov1.BackupStorageLocation) ([]corev1.EnvVar, error) { - envVar := []corev1.EnvVar{} + var envVars []corev1.EnvVar provider := bsl.Spec.Provider var err error switch provider { case AWSProvider: - envVar, err = getAWSRegistryEnvVars(bsl) + envVars, err = getAWSRegistryEnvVars(bsl) case AzureProvider: - envVar, err = getAzureRegistryEnvVars(bsl, cloudProviderEnvVarMap[AzureProvider]) + envVars, err = getAzureRegistryEnvVars(bsl, cloudProviderEnvVarMap[AzureProvider]) case GCPProvider: - envVar, err = getGCPRegistryEnvVars(bsl, cloudProviderEnvVarMap[GCPProvider]) + envVars, err = getGCPRegistryEnvVars(bsl, cloudProviderEnvVarMap[GCPProvider]) default: return nil, errors.New("unsupported provider") } if err != nil { return nil, err } - return envVar, nil + return envVars, nil } func getAWSRegistryEnvVars(bsl *velerov1.BackupStorageLocation) ([]corev1.EnvVar, error) { @@ -154,40 +163,67 @@ func getAWSRegistryEnvVars(bsl *velerov1.BackupStorageLocation) ([]corev1.EnvVar Value: S3, }, { - Name: RegistryStorageS3AccesskeyEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"}, - Key: "access_key", - }, - }, - }, - { - Name: RegistryStorageS3BucketEnvVarKey, + Name: RegistryStorageS3BucketEnvVarKey, Value: bsl.Spec.StorageType.ObjectStorage.Bucket, }, { - Name: RegistryStorageS3RegionEnvVarKey, + Name: RegistryStorageS3RegionEnvVarKey, Value: bslSpecRegion, }, { - Name: RegistryStorageS3SecretkeyEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"}, - Key: "secret_key", - }, - }, - }, - { - Name: RegistryStorageS3RegionendpointEnvVarKey, + Name: RegistryStorageS3RegionendpointEnvVarKey, Value: bsl.Spec.Config[S3URL], }, { - Name: RegistryStorageS3SkipverifyEnvVarKey, + Name: RegistryStorageS3SkipverifyEnvVarKey, Value: bsl.Spec.Config[InsecureSkipTLSVerify], }, } + // if credential is sts, then add sts specific env vars + if bsl.Spec.Config[enableSharedConfig] == "true" { + secretData, err := getSecretKeyRefData(bsl.Spec.Credential, bsl.Namespace) + if err != nil { + return nil, errors.Join(err, errors.New("error getting secret data from bsl for sts cred")) + } + // get web_identity_token_file from secret data + splitString := strings.Split(string(secretData), "\n") + RegExWebIdentity, err := regexp.Compile(webIdentityTokenFile) + if err != nil { + return nil, errors.Join(err, errors.New("error compiling regex for web_identity_token_file")) + } + tokenFilePath := "/init" + for _, line := range splitString { + if lineIsTokenFile := RegExWebIdentity.MatchString(line); lineIsTokenFile { + // split line by "=" + tokenFilePath = strings.TrimSpace(strings.Split(line, "=")[1]) + break + } + } + awsEnvs = append(awsEnvs, corev1.EnvVar{ + Name: RegistryStorageS3CredentialsConfigPathEnvVarKey, + Value: tokenFilePath, + }) + } else { + awsEnvs = append(awsEnvs, + corev1.EnvVar { + Name: RegistryStorageS3AccesskeyEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"}, + Key: "access_key", + }, + }, + }, + corev1.EnvVar { + Name: RegistryStorageS3SecretkeyEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + bsl.Name + "-" + bsl.Spec.Provider + "-registry-secret"}, + Key: "secret_key", + }, + }, + }) + } return awsEnvs, nil } diff --git a/velero-plugins/imagestream/registry_test.go b/velero-plugins/imagestream/registry_test.go index fc2dc592..4fab016e 100644 --- a/velero-plugins/imagestream/registry_test.go +++ b/velero-plugins/imagestream/registry_test.go @@ -1,45 +1,35 @@ package imagestream import ( + "context" "reflect" "testing" - oadpv1alpha1 "github.com/openshift/oadp-operator/api/v1alpha1" + "github.com/google/go-cmp/cmp" + "github.com/konveyor/openshift-velero-plugin/velero-plugins/clients" velerov1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/client-go/kubernetes/scheme" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + "sigs.k8s.io/controller-runtime/pkg/envtest" ) -func getSchemeForFakeClientForRegistry() (*runtime.Scheme, error) { - err := oadpv1alpha1.AddToScheme(scheme.Scheme) - if err != nil { - return nil, err - } - - err = velerov1.AddToScheme(scheme.Scheme) - if err != nil { - return nil, err - } - - return scheme.Scheme, nil -} - const ( - testProfile = "someProfile" - testAccessKey = "someAccessKey" - testSecretAccessKey = "someSecretAccessKey" - testStoragekey = "someStorageKey" - testCloudName = "someCloudName" - testBslProfile = "bslProfile" - testBslAccessKey = "bslAccessKey" - testBslSecretAccessKey = "bslSecretAccessKey" - testSubscriptionID = "someSubscriptionID" - testTenantID = "someTenantID" - testClientID = "someClientID" - testClientSecret = "someClientSecret" - testResourceGroup = "someResourceGroup" + testProfile = "someProfile" + testAccessKey = "someAccessKey" + testSecretAccessKey = "someSecretAccessKey" + testStoragekey = "someStorageKey" + testCloudName = "someCloudName" + testBslProfile = "bslProfile" + testBslAccessKey = "bslAccessKey" + testBslSecretAccessKey = "bslSecretAccessKey" + testBslRoleArn = "bslRoleArn" + testBslWebIdentityTokenFile = "/var/run/secrets/openshift/serviceaccount/token" + testSubscriptionID = "someSubscriptionID" + testTenantID = "someTenantID" + testClientID = "someClientID" + testClientSecret = "someClientSecret" + testResourceGroup = "someResourceGroup" ) var ( @@ -124,6 +114,11 @@ var ( "access_key": []byte(testBslAccessKey), "secret_key": []byte(testBslSecretAccessKey), } + awsStsRegistrySecretData = map[string][]byte{ + "cloud": []byte(`role_arn=testBslRoleArn +web_identity_token_file=`+testBslWebIdentityTokenFile+` +`), + } azureRegistrySecretData = map[string][]byte{ "client_id_key": []byte(""), "client_secret_key": []byte(""), @@ -195,7 +190,68 @@ func Test_getAWSRegistryEnvVars(t *testing.T) { }, wantProfile: "test-profile", matchProfile: true, - }, { + }, + { + name: "given aws sts bsl, appropriate env var for the container are returned", + bsl: &velerov1.BackupStorageLocation{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-bsl", + Namespace: "test-ns", + }, + Spec: velerov1.BackupStorageLocationSpec{ + Provider: AWSProvider, + StorageType: velerov1.StorageType{ + ObjectStorage: &velerov1.ObjectStorageLocation{ + Bucket: "aws-bucket", + }, + }, + Config: map[string]string{ + Region: "aws-region", + Profile: "test-profile", + enableSharedConfig: "true", + }, + Credential: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "cloud-credentials-sts", + }, + Key: "cloud", + }, + }, + }, + secret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cloud-credentials-sts", + Namespace: "test-ns", + }, + Data: awsStsRegistrySecretData, + }, + wantProfile: "test-profile", + wantRegistryContainerEnvVar: []corev1.EnvVar{ + { + Name: RegistryStorageEnvVarKey, + Value: S3, + }, + { + Name: RegistryStorageS3BucketEnvVarKey, + Value: "aws-bucket", + }, + { + Name: RegistryStorageS3RegionEnvVarKey, Value: "aws-region", + }, + { + Name: RegistryStorageS3RegionendpointEnvVarKey, + }, + { + Name: RegistryStorageS3SkipverifyEnvVarKey, + }, + { + Name: RegistryStorageS3CredentialsConfigPathEnvVarKey, Value: testBslWebIdentityTokenFile, + }, + }, + + matchProfile: true, + }, + { name: "given aws profile in bsl, appropriate env var for the container are returned", bsl: &velerov1.BackupStorageLocation{ ObjectMeta: metav1.ObjectMeta{ @@ -233,7 +289,8 @@ func Test_getAWSRegistryEnvVars(t *testing.T) { }, wantProfile: testBslProfile, matchProfile: true, - }, { + }, + { name: "given missing aws profile in bsl, env var should not match", bsl: &velerov1.BackupStorageLocation{ ObjectMeta: metav1.ObjectMeta{ @@ -273,54 +330,68 @@ func Test_getAWSRegistryEnvVars(t *testing.T) { matchProfile: false, }, } + testEnv := & envtest.Environment{} + cfg, err := testEnv.Start() + if err != nil { + t.Fatal(err) + } + defer testEnv.Stop() + clients.SetInClusterConfig(cfg) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - tt.wantRegistryContainerEnvVar = []corev1.EnvVar{ - { - Name: RegistryStorageEnvVarKey, - Value: S3, - }, - { - Name: RegistryStorageS3AccesskeyEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, - Key: "access_key", - }, + cv1c, err := corev1client.NewForConfig(cfg) + if err != nil { + t.Fatal(err) + } + if tt.secret != nil { + cv1c.Namespaces().Create(context.Background(), &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: tt.secret.Namespace, }, - }, - { - Name: RegistryStorageS3BucketEnvVarKey, - Value: "aws-bucket", - }, - { - Name: RegistryStorageS3RegionEnvVarKey, - Value: "aws-region", - }, - { - Name: RegistryStorageS3SecretkeyEnvVarKey, - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, - Key: "secret_key", - }, + }, metav1.CreateOptions{}) + cv1c.Secrets(tt.secret.Namespace).Create(context.Background(), tt.secret, metav1.CreateOptions{}) + } + if tt.registrySecret != nil { + cv1c.Namespaces().Create(context.Background(), &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: tt.registrySecret.Namespace, }, - }, - { - Name: RegistryStorageS3RegionendpointEnvVarKey, - Value: "https://sr-url-aws-domain.com", - }, - { - Name: RegistryStorageS3SkipverifyEnvVarKey, - Value: "false", - }, + }, metav1.CreateOptions{}) + cv1c.Secrets(tt.registrySecret.Namespace).Create(context.Background(), tt.registrySecret, metav1.CreateOptions{}) } - if tt.wantProfile == testBslProfile { + defer func() { + // envtest quirks is we cannot cleanup via namespace deletion + // https://github.com/kubernetes-sigs/controller-runtime/issues/880#issuecomment-615517902 + // so delete the secret + if tt.secret != nil { + cv1c.Secrets(tt.secret.Namespace).Delete(context.Background(), tt.secret.Name, metav1.DeleteOptions{}) + } + if tt.registrySecret != nil { + cv1c.Secrets(tt.registrySecret.Namespace).Delete(context.Background(), tt.registrySecret.Name, metav1.DeleteOptions{}) + } + }() + if tt.wantRegistryContainerEnvVar == nil { tt.wantRegistryContainerEnvVar = []corev1.EnvVar{ { Name: RegistryStorageEnvVarKey, Value: S3, }, + { + Name: RegistryStorageS3BucketEnvVarKey, + Value: "aws-bucket", + }, + { + Name: RegistryStorageS3RegionEnvVarKey, + Value: "aws-region", + }, + { + Name: RegistryStorageS3RegionendpointEnvVarKey, + Value: "https://sr-url-aws-domain.com", + }, + { + Name: RegistryStorageS3SkipverifyEnvVarKey, + Value: "false", + }, { Name: RegistryStorageS3AccesskeyEnvVarKey, ValueFrom: &corev1.EnvVarSource{ @@ -330,14 +401,6 @@ func Test_getAWSRegistryEnvVars(t *testing.T) { }, }, }, - { - Name: RegistryStorageS3BucketEnvVarKey, - Value: "aws-bucket", - }, - { - Name: RegistryStorageS3RegionEnvVarKey, - Value: "aws-region", - }, { Name: RegistryStorageS3SecretkeyEnvVarKey, ValueFrom: &corev1.EnvVarSource{ @@ -347,6 +410,22 @@ func Test_getAWSRegistryEnvVars(t *testing.T) { }, }, }, + } + } + if tt.wantProfile == testBslProfile { + tt.wantRegistryContainerEnvVar = []corev1.EnvVar{ + { + Name: RegistryStorageEnvVarKey, + Value: S3, + }, + { + Name: RegistryStorageS3BucketEnvVarKey, + Value: "aws-bucket", + }, + { + Name: RegistryStorageS3RegionEnvVarKey, + Value: "aws-region", + }, { Name: RegistryStorageS3RegionendpointEnvVarKey, Value: "https://sr-url-aws-domain.com", @@ -355,6 +434,24 @@ func Test_getAWSRegistryEnvVars(t *testing.T) { Name: RegistryStorageS3SkipverifyEnvVarKey, Value: "false", }, + { + Name: RegistryStorageS3AccesskeyEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, + Key: "access_key", + }, + }, + }, + { + Name: RegistryStorageS3SecretkeyEnvVarKey, + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: "oadp-" + tt.bsl.Name + "-" + tt.bsl.Spec.Provider + "-registry-secret"}, + Key: "secret_key", + }, + }, + }, } } @@ -366,7 +463,7 @@ func Test_getAWSRegistryEnvVars(t *testing.T) { } if tt.matchProfile && !reflect.DeepEqual(tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) { - t.Errorf("expected registry container env var to be %#v, got %#v", tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar) + t.Errorf("expected registry container env var has diff %s", cmp.Diff(tt.wantRegistryContainerEnvVar, gotRegistryContainerEnvVar)) } }) } @@ -463,7 +560,6 @@ func Test_getAzureRegistryEnvVars(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - tt.wantRegistryContainerEnvVar = []corev1.EnvVar{ { Name: RegistryStorageEnvVarKey, diff --git a/velero-plugins/imagestream/shared.go b/velero-plugins/imagestream/shared.go index 9813fd3d..f7eedbfe 100644 --- a/velero-plugins/imagestream/shared.go +++ b/velero-plugins/imagestream/shared.go @@ -70,12 +70,12 @@ func GetUdistributionTransportForLocation(uid k8stypes.UID, location, namespace log.Info("Getting registry envs for udistribution transport") envs, err := GetRegistryEnvsForLocation(location, namespace) if err != nil { - return nil, errors.New(fmt.Sprintf("errors getting registryenv: %v", err)) + return nil, fmt.Errorf("errors getting registryenv: %v", err) } log.Info("Creating udistribution transport") ut, err := udistribution.NewTransportFromNewConfig("", envs) if err != nil { - return nil, errors.New(fmt.Sprintf("errors creating new udistribution transport from config: %v", err)) + return nil, fmt.Errorf("errors creating new udistribution transport from config: %v", err) } log.Info("Got udistribution transport") common.BackupUidMap[uid].Ut = ut @@ -91,12 +91,12 @@ func GetUdistributionKey(location, namespace string) string { func GetRegistryEnvsForLocation(location string, namespace string) ([]string, error) { bsl, err := common.GetBackupStorageLocation(location, namespace) if err != nil { - return nil, errors.New(fmt.Sprintf("errors getting bsl: %v", err)) + return nil, fmt.Errorf("errors getting bsl: %v", err) } envVars, err := getRegistryEnvVars(bsl) if err != nil { - return nil, errors.New(fmt.Sprintf("errors getting registry env vars: %v", err)) + return nil, fmt.Errorf("errors getting registry env vars: %v", err) } return coreV1EnvVarArrToStringArr(envVars, bsl.Namespace), nil }