Browse Source

Convert IBM auth struct fields to pointers (#2669)

* Convert SecretStore IBMAuth contents to struct pointers

Signed-off-by: akurata <akurata100@gmail.com>

* update ibm provider logic + tests

Signed-off-by: akurata <akurata100@gmail.com>

* refactor ibm provider validatestore to reduce complexity

Signed-off-by: akurata <akurata100@gmail.com>

* update ibm provider container auth profile check to return an error for a missing profile def

Signed-off-by: akurata <akurata100@gmail.com>

---------

Signed-off-by: akurata <akurata100@gmail.com>
Co-authored-by: Alex Kurata <alexander.kurata@ibm.com>
Alexander Kurata 2 years ago
parent
commit
e85b76f1d3

+ 2 - 2
apis/externalsecrets/v1beta1/secretstore_ibm_types.go

@@ -31,8 +31,8 @@ type IBMProvider struct {
 // +kubebuilder:validation:MinProperties=1
 // +kubebuilder:validation:MinProperties=1
 // +kubebuilder:validation:MaxProperties=1
 // +kubebuilder:validation:MaxProperties=1
 type IBMAuth struct {
 type IBMAuth struct {
-	SecretRef     IBMAuthSecretRef     `json:"secretRef,omitempty"`
-	ContainerAuth IBMAuthContainerAuth `json:"containerAuth,omitempty"`
+	SecretRef     *IBMAuthSecretRef     `json:"secretRef,omitempty"`
+	ContainerAuth *IBMAuthContainerAuth `json:"containerAuth,omitempty"`
 }
 }
 
 
 type IBMAuthSecretRef struct {
 type IBMAuthSecretRef struct {

+ 10 - 2
apis/externalsecrets/v1beta1/zz_generated.deepcopy.go

@@ -1413,8 +1413,16 @@ func (in *GitlabSecretRef) DeepCopy() *GitlabSecretRef {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *IBMAuth) DeepCopyInto(out *IBMAuth) {
 func (in *IBMAuth) DeepCopyInto(out *IBMAuth) {
 	*out = *in
 	*out = *in
-	in.SecretRef.DeepCopyInto(&out.SecretRef)
-	out.ContainerAuth = in.ContainerAuth
+	if in.SecretRef != nil {
+		in, out := &in.SecretRef, &out.SecretRef
+		*out = new(IBMAuthSecretRef)
+		(*in).DeepCopyInto(*out)
+	}
+	if in.ContainerAuth != nil {
+		in, out := &in.ContainerAuth, &out.ContainerAuth
+		*out = new(IBMAuthContainerAuth)
+		**out = **in
+	}
 }
 }
 
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IBMAuth.
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IBMAuth.

+ 39 - 16
pkg/provider/ibm/provider.go

@@ -69,8 +69,10 @@ const (
 var contextTimeout = time.Minute * 2
 var contextTimeout = time.Minute * 2
 
 
 // https://github.com/external-secrets/external-secrets/issues/644
 // https://github.com/external-secrets/external-secrets/issues/644
-var _ esv1beta1.SecretsClient = &providerIBM{}
-var _ esv1beta1.Provider = &providerIBM{}
+var (
+	_ esv1beta1.SecretsClient = &providerIBM{}
+	_ esv1beta1.Provider      = &providerIBM{}
+)
 
 
 type SecretManagerClient interface {
 type SecretManagerClient interface {
 	GetSecretWithContext(ctx context.Context, getSecretOptions *sm.GetSecretOptions) (result sm.SecretIntf, response *core.DetailedResponse, err error)
 	GetSecretWithContext(ctx context.Context, getSecretOptions *sm.GetSecretOptions) (result sm.SecretIntf, response *core.DetailedResponse, err error)
@@ -580,20 +582,25 @@ func (ibm *providerIBM) ValidateStore(store esv1beta1.GenericStore) error {
 	}
 	}
 
 
 	containerRef := ibmSpec.Auth.ContainerAuth
 	containerRef := ibmSpec.Auth.ContainerAuth
-	secretKeyRef := ibmSpec.Auth.SecretRef.SecretAPIKey
-	if utils.IsNil(containerRef.Profile) || (containerRef.Profile == "") {
-		// proceed with API Key Auth validation
-		err := utils.ValidateSecretSelector(store, secretKeyRef)
-		if err != nil {
-			return err
-		}
-		if secretKeyRef.Name == "" {
-			return fmt.Errorf("secretAPIKey.name cannot be empty")
+	secretRef := ibmSpec.Auth.SecretRef
+
+	missingContainerRef := utils.IsNil(containerRef)
+	missingSecretRef := utils.IsNil(secretRef)
+
+	if missingContainerRef == missingSecretRef {
+		// since both are equal, if one is missing assume both are missing
+		if missingContainerRef {
+			return fmt.Errorf("missing auth method")
 		}
 		}
-		if secretKeyRef.Key == "" {
-			return fmt.Errorf("secretAPIKey.key cannot be empty")
+		return fmt.Errorf("too many auth methods defined")
+	}
+
+	if !missingContainerRef {
+		// catch undefined container auth profile
+		if containerRef.Profile == "" {
+			return fmt.Errorf("container auth profile cannot be empty")
 		}
 		}
-	} else {
+
 		// proceed with container auth
 		// proceed with container auth
 		if containerRef.TokenLocation == "" {
 		if containerRef.TokenLocation == "" {
 			containerRef.TokenLocation = "/var/run/secrets/tokens/vault-token"
 			containerRef.TokenLocation = "/var/run/secrets/tokens/vault-token"
@@ -601,7 +608,22 @@ func (ibm *providerIBM) ValidateStore(store esv1beta1.GenericStore) error {
 		if _, err := os.Open(containerRef.TokenLocation); err != nil {
 		if _, err := os.Open(containerRef.TokenLocation); err != nil {
 			return fmt.Errorf("cannot read container auth token %s. %w", containerRef.TokenLocation, err)
 			return fmt.Errorf("cannot read container auth token %s. %w", containerRef.TokenLocation, err)
 		}
 		}
+		return nil
+	}
+
+	// proceed with API Key Auth validation
+	secretKeyRef := secretRef.SecretAPIKey
+	err := utils.ValidateSecretSelector(store, secretKeyRef)
+	if err != nil {
+		return err
+	}
+	if secretKeyRef.Name == "" {
+		return fmt.Errorf("secretAPIKey.name cannot be empty")
 	}
 	}
+	if secretKeyRef.Key == "" {
+		return fmt.Errorf("secretAPIKey.key cannot be empty")
+	}
+
 	return nil
 	return nil
 }
 }
 
 
@@ -623,9 +645,10 @@ func (ibm *providerIBM) NewClient(ctx context.Context, store esv1beta1.GenericSt
 
 
 	var err error
 	var err error
 	var secretsManager *sm.SecretsManagerV2
 	var secretsManager *sm.SecretsManagerV2
-	containerAuthProfile := iStore.store.Auth.ContainerAuth.Profile
-	if containerAuthProfile != "" {
+	containerAuth := iStore.store.Auth.ContainerAuth
+	if !utils.IsNil(containerAuth) && containerAuth.Profile != "" {
 		// container-based auth
 		// container-based auth
+		containerAuthProfile := iStore.store.Auth.ContainerAuth.Profile
 		containerAuthToken := iStore.store.Auth.ContainerAuth.TokenLocation
 		containerAuthToken := iStore.store.Auth.ContainerAuth.TokenLocation
 		containerAuthEndpoint := iStore.store.Auth.ContainerAuth.IAMEndpoint
 		containerAuthEndpoint := iStore.store.Auth.ContainerAuth.IAMEndpoint
 
 

+ 29 - 14
pkg/provider/ibm/provider_test.go

@@ -167,18 +167,20 @@ func TestValidateStore(t *testing.T) {
 	}
 	}
 	url := "my-url"
 	url := "my-url"
 	store.Spec.Provider.IBM.ServiceURL = &url
 	store.Spec.Provider.IBM.ServiceURL = &url
-	var nilProfile esv1beta1.IBMAuthContainerAuth
-	store.Spec.Provider.IBM.Auth.ContainerAuth = nilProfile
 	err = p.ValidateStore(store)
 	err = p.ValidateStore(store)
 	if err == nil {
 	if err == nil {
 		t.Errorf(errExpectedErr)
 		t.Errorf(errExpectedErr)
-	} else if err.Error() != "secretAPIKey.name cannot be empty" {
-		t.Errorf("KeySelector test failed: expected secret name is required, got %v", err)
+	} else if err.Error() != "missing auth method" {
+		t.Errorf("KeySelector test failed: expected missing auth method, got %v", err)
 	}
 	}
-	store.Spec.Provider.IBM.Auth.SecretRef.SecretAPIKey.Name = "foo"
-	store.Spec.Provider.IBM.Auth.SecretRef.SecretAPIKey.Key = "bar"
 	ns := "ns-one"
 	ns := "ns-one"
-	store.Spec.Provider.IBM.Auth.SecretRef.SecretAPIKey.Namespace = &ns
+	store.Spec.Provider.IBM.Auth.SecretRef = &esv1beta1.IBMAuthSecretRef{
+		SecretAPIKey: v1.SecretKeySelector{
+			Name:      "foo",
+			Key:       "bar",
+			Namespace: &ns,
+		},
+	}
 	err = p.ValidateStore(store)
 	err = p.ValidateStore(store)
 	if err == nil {
 	if err == nil {
 		t.Errorf(errExpectedErr)
 		t.Errorf(errExpectedErr)
@@ -187,10 +189,21 @@ func TestValidateStore(t *testing.T) {
 	}
 	}
 
 
 	// add container auth test
 	// add container auth test
-	store.Spec.Provider.IBM = &esv1beta1.IBMProvider{}
-	store.Spec.Provider.IBM.ServiceURL = &url
-	store.Spec.Provider.IBM.Auth.ContainerAuth.Profile = "Trusted IAM Profile"
-	store.Spec.Provider.IBM.Auth.ContainerAuth.TokenLocation = "/a/path/to/nowhere/that/should/exist"
+	store = &esv1beta1.SecretStore{
+		Spec: esv1beta1.SecretStoreSpec{
+			Provider: &esv1beta1.SecretStoreProvider{
+				IBM: &esv1beta1.IBMProvider{
+					ServiceURL: &url,
+					Auth: esv1beta1.IBMAuth{
+						ContainerAuth: &esv1beta1.IBMAuthContainerAuth{
+							Profile:       "Trusted IAM Profile",
+							TokenLocation: "/a/path/to/nowhere/that/should/exist",
+						},
+					},
+				},
+			},
+		},
+	}
 	err = p.ValidateStore(store)
 	err = p.ValidateStore(store)
 	expected := "cannot read container auth token"
 	expected := "cannot read container auth token"
 	if !ErrorContains(err, expected) {
 	if !ErrorContains(err, expected) {
@@ -697,7 +710,8 @@ func TestGetSecretMap(t *testing.T) {
 		smtc.apiOutput = secret
 		smtc.apiOutput = secret
 		smtc.ref.Key = secretUUID
 		smtc.ref.Key = secretUUID
 		smtc.ref.MetadataPolicy = esv1beta1.ExternalSecretMetadataPolicyFetch
 		smtc.ref.MetadataPolicy = esv1beta1.ExternalSecretMetadataPolicyFetch
-		smtc.expectedData = map[string][]byte{"arbitrary": []byte(payload),
+		smtc.expectedData = map[string][]byte{
+			"arbitrary":       []byte(payload),
 			"created_at":      []byte(timeValue),
 			"created_at":      []byte(timeValue),
 			"created_by":      []byte(*secret.CreatedBy),
 			"created_by":      []byte(*secret.CreatedBy),
 			"crn":             []byte(nilValue),
 			"crn":             []byte(nilValue),
@@ -728,7 +742,8 @@ func TestGetSecretMap(t *testing.T) {
 		smtc.apiOutput = secret
 		smtc.apiOutput = secret
 		smtc.ref.Key = iamCredentialsSecret + secretUUID
 		smtc.ref.Key = iamCredentialsSecret + secretUUID
 		smtc.ref.MetadataPolicy = esv1beta1.ExternalSecretMetadataPolicyFetch
 		smtc.ref.MetadataPolicy = esv1beta1.ExternalSecretMetadataPolicyFetch
-		smtc.expectedData = map[string][]byte{"api_key": []byte(secretAPIKey),
+		smtc.expectedData = map[string][]byte{
+			"api_key":         []byte(secretAPIKey),
 			"apikey":          []byte(secretAPIKey),
 			"apikey":          []byte(secretAPIKey),
 			"created_at":      []byte(timeValue),
 			"created_at":      []byte(timeValue),
 			"created_by":      []byte(*secret.CreatedBy),
 			"created_by":      []byte(*secret.CreatedBy),
@@ -1099,7 +1114,7 @@ func TestValidRetryInput(t *testing.T) {
 			Provider: &esv1beta1.SecretStoreProvider{
 			Provider: &esv1beta1.SecretStoreProvider{
 				IBM: &esv1beta1.IBMProvider{
 				IBM: &esv1beta1.IBMProvider{
 					Auth: esv1beta1.IBMAuth{
 					Auth: esv1beta1.IBMAuth{
-						SecretRef: esv1beta1.IBMAuthSecretRef{
+						SecretRef: &esv1beta1.IBMAuthSecretRef{
 							SecretAPIKey: v1.SecretKeySelector{
 							SecretAPIKey: v1.SecretKeySelector{
 								Name: "fake-secret",
 								Name: "fake-secret",
 								Key:  "fake-key",
 								Key:  "fake-key",