diff --git a/docs/provider/ibm-secrets-manager.md b/docs/provider/ibm-secrets-manager.md index 900d00e5d..44df2c3c1 100644 --- a/docs/provider/ibm-secrets-manager.md +++ b/docs/provider/ibm-secrets-manager.md @@ -197,7 +197,7 @@ Below example creates a kubernetes secret based on ID of the secret in Secrets M {% include 'ibm-external-secret.yaml' %} ``` -Alternatively, secret name can be specified instead of secret ID. +Alternatively, secret name can be specified instead of secret ID. However, note that ESO makes an additional call to fetch the relevant secret ID for the specified secret name. ```yaml {% include 'ibm-external-secret-by-name.yaml' %} diff --git a/pkg/provider/ibm/provider.go b/pkg/provider/ibm/provider.go index 4d1e72030..b123c2c97 100644 --- a/pkg/provider/ibm/provider.go +++ b/pkg/provider/ibm/provider.go @@ -227,8 +227,14 @@ func getImportCertSecret(ibm *providerIBM, secretName *string, ref esv1beta1.Ext if err != nil { return nil, err } - if val, ok := secMap[ref.Property]; ok { + val, ok := secMap[ref.Property] + if ok { return []byte(val.(string)), nil + } else if ref.Property == privateKeyConst { + // we want to return an empty string in case the secret doesn't contain a private key + // this is to ensure that secret of type 'kubernetes.io/tls' gets created as expected, even with an empty private key + fmt.Printf("warn: %s is empty for secret %s\n", privateKeyConst, *secretName) + return []byte(""), nil } return nil, fmt.Errorf("key %s does not exist in secret %s", ref.Property, ref.Key) } @@ -418,6 +424,7 @@ func (ibm *providerIBM) GetSecretMap(_ context.Context, ref esv1beta1.ExternalSe } secretMap := make(map[string][]byte) + secMapBytes := make(map[string][]byte) response, err := getSecretData(ibm, &secretName, secretType) if err != nil { return nil, err @@ -430,36 +437,42 @@ func (ibm *providerIBM) GetSecretMap(_ context.Context, ref esv1beta1.ExternalSe if ref.MetadataPolicy == esv1beta1.ExternalSecretMetadataPolicyFetch { secretMap = populateSecretMap(secretMap, secMap) } + secMapBytes = populateSecretMap(secMapBytes, secMap) switch secretType { case sm.Secret_SecretType_Arbitrary: - secretMap[arbitraryConst] = []byte(fmt.Sprintf("%v", secMap[payloadConst])) + secretMap[arbitraryConst] = secMapBytes[payloadConst] return secretMap, nil case sm.Secret_SecretType_UsernamePassword: - secretMap[usernameConst] = []byte(fmt.Sprintf("%v", secMap[usernameConst])) - secretMap[passwordConst] = []byte(fmt.Sprintf("%v", secMap[passwordConst])) + secretMap[usernameConst] = secMapBytes[usernameConst] + secretMap[passwordConst] = secMapBytes[passwordConst] return secretMap, nil case sm.Secret_SecretType_IamCredentials: - secretMap[apikeyConst] = []byte(fmt.Sprintf("%v", secMap[smAPIKeyConst])) + secretMap[apikeyConst] = secMapBytes[smAPIKeyConst] return secretMap, nil case sm.Secret_SecretType_ImportedCert: - secretMap[certificateConst] = []byte(fmt.Sprintf("%v", secMap[certificateConst])) - secretMap[intermediateConst] = []byte(fmt.Sprintf("%v", secMap[intermediateConst])) - secretMap[privateKeyConst] = []byte(fmt.Sprintf("%v", secMap[privateKeyConst])) + secretMap[certificateConst] = secMapBytes[certificateConst] + secretMap[intermediateConst] = secMapBytes[intermediateConst] + if v, ok := secMapBytes[privateKeyConst]; ok { + secretMap[privateKeyConst] = v + } else { + fmt.Printf("warn: %s is empty for secret %s\n", privateKeyConst, secretName) + secretMap[privateKeyConst] = []byte("") + } return secretMap, nil case sm.Secret_SecretType_PublicCert: - secretMap[certificateConst] = []byte(fmt.Sprintf("%v", secMap[certificateConst])) - secretMap[intermediateConst] = []byte(fmt.Sprintf("%v", secMap[intermediateConst])) - secretMap[privateKeyConst] = []byte(fmt.Sprintf("%v", secMap[privateKeyConst])) + secretMap[certificateConst] = secMapBytes[certificateConst] + secretMap[intermediateConst] = secMapBytes[intermediateConst] + secretMap[privateKeyConst] = secMapBytes[privateKeyConst] return secretMap, nil case sm.Secret_SecretType_PrivateCert: - secretMap[certificateConst] = []byte(fmt.Sprintf("%v", secMap[certificateConst])) - secretMap[privateKeyConst] = []byte(fmt.Sprintf("%v", secMap[privateKeyConst])) + secretMap[certificateConst] = secMapBytes[certificateConst] + secretMap[privateKeyConst] = secMapBytes[privateKeyConst] return secretMap, nil case sm.Secret_SecretType_Kv: diff --git a/pkg/provider/ibm/provider_test.go b/pkg/provider/ibm/provider_test.go index 5074505f6..81f9dfd27 100644 --- a/pkg/provider/ibm/provider_test.go +++ b/pkg/provider/ibm/provider_test.go @@ -340,6 +340,22 @@ func TestIBMSecretManagerGetSecret(t *testing.T) { } setSecretCert := funcSetCertSecretTest(importedCert, "good case: imported_cert type with property", sm.Secret_SecretType_ImportedCert, true) + // good case: imported_cert type without a private_key + importedCertNoPvtKey := func(smtc *secretManagerTestCase) { + secret := &sm.ImportedCertificate{ + SecretType: utilpointer.To(sm.Secret_SecretType_ImportedCert), + Name: utilpointer.To("testyname"), + ID: utilpointer.To(secretUUID), + Certificate: utilpointer.To(secretCertificate), + } + smtc.name = "good case: imported cert without private key" + smtc.apiInput.ID = utilpointer.To(secretUUID) + smtc.apiOutput = secret + smtc.ref.Key = "imported_cert/" + secretUUID + smtc.ref.Property = "private_key" + smtc.expectedSecret = "" + } + // bad case: imported_cert type without property badSecretCert := funcSetCertSecretTest(importedCert, "bad case: imported_cert type without property", sm.Secret_SecretType_ImportedCert, false) @@ -493,6 +509,7 @@ func TestIBMSecretManagerGetSecret(t *testing.T) { makeValidSecretManagerTestCaseCustom(setSecretKVWithOutKey), makeValidSecretManagerTestCaseCustom(badSecretKV), makeValidSecretManagerTestCaseCustom(badSecretCert), + makeValidSecretManagerTestCaseCustom(importedCertNoPvtKey), makeValidSecretManagerTestCaseCustom(setSecretPublicCert), makeValidSecretManagerTestCaseCustom(badSecretPublicCert), makeValidSecretManagerTestCaseCustom(setSecretPrivateCert), @@ -500,16 +517,16 @@ func TestIBMSecretManagerGetSecret(t *testing.T) { } sm := providerIBM{} - for k, v := range successCases { + for _, v := range successCases { t.Run(v.name, func(t *testing.T) { sm.IBMClient = v.mockClient sm.cache = NewCache(10, 1*time.Minute) out, err := sm.GetSecret(context.Background(), *v.ref) if !ErrorContains(err, v.expectError) { - t.Errorf("[%d] unexpected error: %s, expected: '%s'", k, err.Error(), v.expectError) + t.Errorf("unexpected error:\n%s, expected:\n'%s'", err.Error(), v.expectError) } if string(out) != v.expectedSecret { - t.Errorf("[%d] unexpected secret: expected %s, got %s", k, v.expectedSecret, string(out)) + t.Errorf("unexpected secret: expected:\n%s\ngot:\n%s", v.expectedSecret, string(out)) } }) } @@ -779,6 +796,29 @@ func TestGetSecretMap(t *testing.T) { } } + // good case: imported_cert without a private_key + setimportedCertWithNoPvtKey := func(smtc *secretManagerTestCase) { + secret := &sm.ImportedCertificate{ + CreatedBy: utilpointer.To("testCreatedBy"), + CreatedAt: &strfmt.DateTime{}, + Downloaded: utilpointer.To(false), + Labels: []string{"abc", "def", "xyz"}, + LocksTotal: utilpointer.To(int64(20)), + Certificate: utilpointer.To(secretCertificate), + Intermediate: utilpointer.To(secretIntermediate), + } + smtc.name = "good case: imported_cert without private key" + smtc.apiInput.ID = utilpointer.To(secretUUID) + smtc.apiOutput = secret + smtc.ref.Key = "imported_cert/" + secretUUID + + smtc.expectedData = map[string][]byte{ + "certificate": []byte(secretCertificate), + "intermediate": []byte(secretIntermediate), + "private_key": []byte(""), + } + } + // good case: public_cert with metadata setPublicCertWithMetadata := func(smtc *secretManagerTestCase) { secret := &sm.PublicCertificate{ @@ -992,6 +1032,7 @@ func TestGetSecretMap(t *testing.T) { makeValidSecretManagerTestCaseCustom(badSecretKVWithUnknownProperty), makeValidSecretManagerTestCaseCustom(setSecretPublicCert), makeValidSecretManagerTestCaseCustom(setSecretPrivateCert), + makeValidSecretManagerTestCaseCustom(setimportedCertWithNoPvtKey), makeValidSecretManagerTestCaseCustom(setSecretIamWithMetadata), makeValidSecretManagerTestCaseCustom(setArbitraryWithMetadata), makeValidSecretManagerTestCaseCustom(setSecretUserPassWithMetadata), @@ -1003,15 +1044,15 @@ func TestGetSecretMap(t *testing.T) { } sm := providerIBM{} - for k, v := range successCases { + for _, v := range successCases { t.Run(v.name, func(t *testing.T) { sm.IBMClient = v.mockClient out, err := sm.GetSecretMap(context.Background(), *v.ref) if !ErrorContains(err, v.expectError) { - t.Errorf(" unexpected error: %s, expected: '%s'", err.Error(), v.expectError) + t.Errorf("unexpected error: %s, expected: '%s'", err.Error(), v.expectError) } if err == nil && !reflect.DeepEqual(out, v.expectedData) { - t.Errorf("[%d] unexpected secret data: expected %+v, got %v", k, v.expectedData, out) + t.Errorf("unexpected secret data: expected:\n%+v\ngot:\n%+v", v.expectedData, out) } }) }