Tests for SetSecret passing: refactored SetSecret

Signed-off-by: Dom Meddick <dom.meddick@engineerbetter.com>
Co-authored-by: Lilly Daniell <lilly.daniell@engineerbetter.com>
Co-authored-by: Adrienne Galloway <adrienne.galloway@engineerbetter.com>
Co-authored-by: Will Young <will.young@engineerbetter.com>
Co-authored-by: Amr Fawzy <amr.fawzy@container-solutions.com>
Co-authored-by: Gustavo Carvalho <gustavo.carvalho@containersolutions.com>

pkg/provider/vault/vault.go

@@ -364,7 +364,7 @@ func (c *connector) ValidateStore(store esv1beta1.GenericStore) error {
 func (v *client) SetSecret(ctx context.Context, value []byte, remoteRef esv1beta1.PushRemoteRef) error {
 	label := make(map[string]string)
 	label["managed-by"] = "external-secrets"
-
+	// remoteRef.GetRemoteKey() -> "foo"
 	secretRequest := vault.Secret{
 		Data: map[string]interface{}{remoteRef.GetRemoteKey(): string(value)},
 		Auth: &vault.SecretAuth{Metadata: label},
@@ -375,38 +375,22 @@ func (v *client) SetSecret(ctx context.Context, value []byte, remoteRef esv1beta
 	// Retrieve the secret map from vault and convert the secret value in string form.
 	vaultSecret, err := v.GetSecretMap(ctx, esv1beta1.ExternalSecretDataRemoteRef{Key: path})
 	vaultSecretValue := string(vaultSecret[remoteRef.GetRemoteKey()])
+
 	// Retrieve the secret value to be pushed and convert it to string form.
 	secretToPush := secretRequest.Data
-	pushSecretValue := fmt.Sprintf("%v", secretToPush[remoteRef.GetRemoteKey()])
+	pushSecretValue := string(value)
 
 	if vaultSecretValue == pushSecretValue {
 		return nil
 	}
 
-	// If error is nil this will error out
-	if err != nil {
-		stringError := err.Error()
-		if stringError == "secret not found" {
-			_, err = v.logical.WriteWithContext(ctx, path, secretToPush)
-			if err != nil {
-				return err
-			}
-		}
-	} else {
-		_, err = v.logical.WriteWithContext(ctx, path, secretToPush)
-		if err != nil {
-			return err
-		}
-	}
-
-	manager := secretRequest.Auth.Metadata
-	fmt.Println(manager["managed-by"])
-
-	if manager["managed-by"] != "external-secrets" {
-		return fmt.Errorf("secret %v is not managed by external secrets", remoteRef.GetRemoteKey())
+	// If error is not of type secret not found, we should error
+	if err != nil && !strings.Contains(err.Error(), "secret not found") {
+		return err
 	}
-
-	return nil
+	// Otherwise, create or update the version.
+	_, err = v.logical.WriteWithContext(ctx, path, secretToPush)
+	return err
 }
 
 // GetAllSecrets gets multiple secrets from the provider and loads into a kubernetes secret.

pkg/provider/vault/vault_test.go

@@ -1409,64 +1409,107 @@ func (f fakeRef) GetRemoteKey() string {
 	return f.key
 }
 
-func TestSetSecret(t *testing.T) {
-	path := "secret"
 
-	// Testing for when SetSecret returns an error
-	client1 := client{
+
+
+func TestSetSecretUpdateSecretNotFound(t *testing.T) {
+	path := "secret"
+	secretData := map[string]interface{}{
+		"data": map[string]interface{}{
+			"fake key": "fake value",
+		},
+	}
+	f := fake.Logical{
+		ReadWithDataWithContextFn: fake.NewReadWithContextFn(nil, fmt.Errorf("secret not found")),
+	}
+	f.WriteWithContextFn = fake.WriteChangingReadContext(secretData, f)
+	client := client{
 		store: &esv1beta1.VaultProvider{
 			Path: &path,
 		},
-		logical: fake.Logical{
-			WriteWithContextFn: fake.NewWriteWithContextFn(nil, fmt.Errorf("error")),
-		},
+		logical: f,
 	}
 	ref := fakeRef{key: "I'm a key"}
 
-	err := client1.SetSecret(context.Background(), []byte("HI"), ref)
-
-	assert.Equal(t, err.Error(), "error")
+	err := client.SetSecret(context.Background(), []byte("HI"), ref)
+	assert.Equal(t, err, nil)
+}
 
-	// Testing for when SetSecret returns nil
-	client2 := client{
+func TestSetSecretUpdateSecretNotFoundWithError(t *testing.T) {
+	path := "secret"
+	f := fake.Logical{
+		ReadWithDataWithContextFn: fake.NewReadWithContextFn(nil, fmt.Errorf("secret not found")),
+	}
+	f.WriteWithContextFn = fake.NewWriteWithContextFn(nil, fmt.Errorf("no permissions"))
+	client := client{
 		store: &esv1beta1.VaultProvider{
 			Path: &path,
 		},
-		logical: fake.Logical{
-			WriteWithContextFn: fake.NewWriteWithContextFn(nil, nil),
+		logical: f,
+	}
+	ref := fakeRef{key: "I'm a key"}
+
+	err := client.SetSecret(context.Background(), []byte("HI"), ref)
+	assert.Equal(t, err.Error(), "no permissions")
+}
+func TestSetSecretEqualsPushSecret(t *testing.T){
+	path := "secret"
+	f := fake.Logical{
+		ReadWithDataWithContextFn: fake.NewReadWithContextFn(map[string]interface{}{
+			"key": "fake value",
+		}, nil),
+	}
+	f.WriteWithContextFn = fake.NewWriteWithContextFn(nil, nil)
+	client := client{
+		store: &esv1beta1.VaultProvider{
+			Path: &path,
 		},
+		logical: f,
 	}
+	ref := fakeRef{key: "key"}
+
+	err := client.SetSecret(context.Background(), []byte("fake value"), ref)
 
-	err = client2.SetSecret(context.Background(), []byte("HI"), ref)
 
 	assert.Equal(t, err, nil)
 }
 
-// func TestSetSecretUpdate(t *testing.T) {
-// 	path := "secret"
-// 	secretData := map[string]interface{}{
-// 		"data": map[string]interface{}{
-// 			"fake key": "fake value",
-// 		},
-// 	}
-// 	f := fake.Logical{
-// 		ReadWithDataWithContextFn: fake.NewReadWithContextFn(nil, nil),
-// 	}
-// 	f.WriteWithContextFn = fake.WriteChangingReadContext(secretData, f)
-// 	client := client{
-// 		store: &esv1beta1.VaultProvider{
-// 			Path: &path,
-// 		},
-// 		logical: f,
-// 	}
-// 	ref := fakeRef{key: "I'm a key"}
-
-// 	client.SetSecret(context.Background(), []byte("HI"), ref)
-// 	f.WriteWithContextFn = fake.WriteChangingReadContext(secretData, f)
-// 	err := client.SetSecret(context.Background(), []byte("HI"), ref)
-
-// 	assert.Equal(t, err, "cannot push - secret already exists")
-// }
+func TestSetSecretEqualsPushSecretWithError(t *testing.T){
+	path := "secret"
+	f := fake.Logical{
+		ReadWithDataWithContextFn: fake.NewReadWithContextFn(map[string]interface{}{
+			"key": "wrong-key",
+		}, nil),
+	}
+	f.WriteWithContextFn = fake.NewWriteWithContextFn(nil, fmt.Errorf("boom"))
+	client := client{
+		store: &esv1beta1.VaultProvider{
+			Path: &path,
+		},
+		logical: f,
+	}
+	ref := fakeRef{key: "key"}
+
+	err := client.SetSecret(context.Background(), []byte("fake value"), ref)
+	assert.Equal(t, err.Error(), "boom")
+}
+func TestSetSecretErrorReadingSecret(t *testing.T){
+	path := "secret"
+	f := fake.Logical{
+		ReadWithDataWithContextFn: fake.NewReadWithContextFn(nil, fmt.Errorf("you shall not pass")),
+	}
+	f.WriteWithContextFn = fake.NewWriteWithContextFn(nil, nil)
+	client := client{
+		store: &esv1beta1.VaultProvider{
+			Path: &path,
+		},
+		logical: f,
+	}
+	ref := fakeRef{key: "key"}
+
+	err := client.SetSecret(context.Background(), []byte("fake value"), ref)
+	assert.ErrorContains(t,err,"you shall not pass")
+}
 
 // Above test pushing same exact secret twice.
 // It will also