Browse Source

Checking nil value when parsing secret values. Added tests

Martin Montes 4 years ago
parent
commit
394c4257f1
2 changed files with 93 additions and 9 deletions
  1. 2 3
      pkg/provider/vault/vault.go
  2. 91 6
      pkg/provider/vault/vault_test.go

+ 2 - 3
pkg/provider/vault/vault.go

@@ -219,14 +219,13 @@ func (v *client) readSecret(ctx context.Context, path, version string) (map[stri
 
 
 	byteMap := make(map[string][]byte, len(secretData))
 	byteMap := make(map[string][]byte, len(secretData))
 	for k, v := range secretData {
 	for k, v := range secretData {
-		if v == nil {
-			continue
-		}
 		switch t := v.(type) {
 		switch t := v.(type) {
 		case string:
 		case string:
 			byteMap[k] = []byte(t)
 			byteMap[k] = []byte(t)
 		case []byte:
 		case []byte:
 			byteMap[k] = t
 			byteMap[k] = t
+		case nil:
+			byteMap[k] = []byte(nil)
 		default:
 		default:
 			return nil, errors.New(errSecretFormat)
 			return nil, errors.New(errSecretFormat)
 		}
 		}

+ 91 - 6
pkg/provider/vault/vault_test.go

@@ -41,7 +41,7 @@ const (
 	secretDataString = "some-creds"
 	secretDataString = "some-creds"
 )
 )
 
 
-func makeValidSecretStore() *esv1alpha1.SecretStore {
+func makeValidSecretStoreWithVersion(v esv1alpha1.VaultKVStoreVersion) *esv1alpha1.SecretStore {
 	return &esv1alpha1.SecretStore{
 	return &esv1alpha1.SecretStore{
 		ObjectMeta: metav1.ObjectMeta{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "vault-store",
 			Name:      "vault-store",
@@ -52,7 +52,7 @@ func makeValidSecretStore() *esv1alpha1.SecretStore {
 				Vault: &esv1alpha1.VaultProvider{
 				Vault: &esv1alpha1.VaultProvider{
 					Server:  "vault.example.com",
 					Server:  "vault.example.com",
 					Path:    "secret",
 					Path:    "secret",
-					Version: esv1alpha1.VaultKVStoreV2,
+					Version: v,
 					Auth: esv1alpha1.VaultAuth{
 					Auth: esv1alpha1.VaultAuth{
 						Kubernetes: &esv1alpha1.VaultKubernetesAuth{
 						Kubernetes: &esv1alpha1.VaultKubernetesAuth{
 							Path: "kubernetes",
 							Path: "kubernetes",
@@ -68,6 +68,10 @@ func makeValidSecretStore() *esv1alpha1.SecretStore {
 	}
 	}
 }
 }
 
 
+func makeValidSecretStore() *esv1alpha1.SecretStore {
+	return makeValidSecretStoreWithVersion(esv1alpha1.VaultKVStoreV2)
+}
+
 func makeValidSecretStoreWithCerts() *esv1alpha1.SecretStore {
 func makeValidSecretStoreWithCerts() *esv1alpha1.SecretStore {
 	return &esv1alpha1.SecretStore{
 	return &esv1alpha1.SecretStore{
 		ObjectMeta: metav1.ObjectMeta{
 		ObjectMeta: metav1.ObjectMeta{
@@ -136,11 +140,15 @@ func newVaultResponse(data *vault.Secret) *vault.Response {
 	}
 	}
 }
 }
 
 
-func newVaultTokenIDResponse(token string) *vault.Response {
+func newVaultResponseWithData(data map[string]interface{}) *vault.Response {
 	return newVaultResponse(&vault.Secret{
 	return newVaultResponse(&vault.Secret{
-		Data: map[string]interface{}{
-			"id": token,
-		},
+		Data: data,
+	})
+}
+
+func newVaultTokenIDResponse(token string) *vault.Response {
+	return newVaultResponseWithData(map[string]interface{}{
+		"id": token,
 	})
 	})
 }
 }
 
 
@@ -494,6 +502,15 @@ func vaultTest(t *testing.T, name string, tc testCase) {
 
 
 func TestGetSecretMap(t *testing.T) {
 func TestGetSecretMap(t *testing.T) {
 	errBoom := errors.New("boom")
 	errBoom := errors.New("boom")
+	secret := map[string]interface{}{
+		"access_key":    "access_key",
+		"access_secret": "access_secret",
+	}
+	secretWithNilVal := map[string]interface{}{
+		"access_key":    "access_key",
+		"access_secret": "access_secret",
+		"token":         nil,
+	}
 
 
 	type args struct {
 	type args struct {
 		store   *esv1alpha1.VaultProvider
 		store   *esv1alpha1.VaultProvider
@@ -512,6 +529,74 @@ func TestGetSecretMap(t *testing.T) {
 		args   args
 		args   args
 		want   want
 		want   want
 	}{
 	}{
+		"ReadSecretKV1": {
+			reason: "Should map the secret even if it has a nil value",
+			args: args{
+				store: makeValidSecretStoreWithVersion(esv1alpha1.VaultKVStoreV1).Spec.Provider.Vault,
+				vClient: &fake.VaultClient{
+					MockNewRequest: fake.NewMockNewRequestFn(&vault.Request{}),
+					MockRawRequestWithContext: fake.NewMockRawRequestWithContextFn(
+						newVaultResponseWithData(secret), nil,
+					),
+				},
+			},
+			want: want{
+				err: nil,
+			},
+		},
+		"ReadSecretKV2": {
+			reason: "Should map the secret even if it has a nil value",
+			args: args{
+				store: makeValidSecretStoreWithVersion(esv1alpha1.VaultKVStoreV2).Spec.Provider.Vault,
+				vClient: &fake.VaultClient{
+					MockNewRequest: fake.NewMockNewRequestFn(&vault.Request{}),
+					MockRawRequestWithContext: fake.NewMockRawRequestWithContextFn(
+						newVaultResponseWithData(
+							map[string]interface{}{
+								"data": secret,
+							},
+						), nil,
+					),
+				},
+			},
+			want: want{
+				err: nil,
+			},
+		},
+		"ReadSecretWithNilValueKV1": {
+			reason: "Should map the secret even if it has a nil value",
+			args: args{
+				store: makeValidSecretStoreWithVersion(esv1alpha1.VaultKVStoreV1).Spec.Provider.Vault,
+				vClient: &fake.VaultClient{
+					MockNewRequest: fake.NewMockNewRequestFn(&vault.Request{}),
+					MockRawRequestWithContext: fake.NewMockRawRequestWithContextFn(
+						newVaultResponseWithData(secretWithNilVal), nil,
+					),
+				},
+			},
+			want: want{
+				err: nil,
+			},
+		},
+		"ReadSecretWithNilValueKV2": {
+			reason: "Should map the secret even if it has a nil value",
+			args: args{
+				store: makeValidSecretStoreWithVersion(esv1alpha1.VaultKVStoreV2).Spec.Provider.Vault,
+				vClient: &fake.VaultClient{
+					MockNewRequest: fake.NewMockNewRequestFn(&vault.Request{}),
+					MockRawRequestWithContext: fake.NewMockRawRequestWithContextFn(
+						newVaultResponseWithData(
+							map[string]interface{}{
+								"data": secretWithNilVal,
+							},
+						), nil,
+					),
+				},
+			},
+			want: want{
+				err: nil,
+			},
+		},
 		"ReadSecretError": {
 		"ReadSecretError": {
 			reason: "Should return error if vault client fails to read secret.",
 			reason: "Should return error if vault client fails to read secret.",
 			args: args{
 			args: args{