Refactor the PushSecret interface (external-secrets#2859)

Signed-off-by: shuheiktgw <s-kitagawa@mercari.com>

apis/externalsecrets/v1alpha1/pushsecret_types.go

@@ -105,6 +105,22 @@ type PushSecretData struct {
 	Metadata *apiextensionsv1.JSON `json:"metadata,omitempty"`
 }
 
+func (d PushSecretData) GetMetadata() *apiextensionsv1.JSON {
+	return d.Metadata
+}
+
+func (d PushSecretData) GetSecretKey() string {
+	return d.Match.SecretKey
+}
+
+func (d PushSecretData) GetRemoteKey() string {
+	return d.Match.RemoteRef.RemoteKey
+}
+
+func (d PushSecretData) GetProperty() string {
+	return d.Match.RemoteRef.Property
+}
+
 // PushSecretConditionType indicates the condition of the PushSecret.
 type PushSecretConditionType string
 

apis/externalsecrets/v1beta1/provider.go

@@ -18,7 +18,6 @@ import (
 	"context"
 
 	corev1 "k8s.io/api/core/v1"
-	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -70,10 +69,10 @@ type SecretsClient interface {
 	GetSecret(ctx context.Context, ref ExternalSecretDataRemoteRef) ([]byte, error)
 
 	// PushSecret will write a single secret into the provider
-	PushSecret(ctx context.Context, value []byte, typed corev1.SecretType, metadata *apiextensionsv1.JSON, remoteRef PushRemoteRef) error
+	PushSecret(ctx context.Context, secret *corev1.Secret, data PushSecretData) error
 
 	// DeleteSecret will delete the secret from a provider
-	DeleteSecret(ctx context.Context, remoteRef PushRemoteRef) error
+	DeleteSecret(ctx context.Context, remoteRef PushSecretRemoteRef) error
 
 	// Validate checks if the client is configured correctly
 	// and is able to retrieve secrets from the provider.

apis/externalsecrets/v1beta1/provider_schema_test.go

@@ -19,7 +19,6 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	corev1 "k8s.io/api/core/v1"
-	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -37,12 +36,12 @@ func (p *PP) NewClient(_ context.Context, _ GenericStore, _ client.Client, _ str
 }
 
 // PushSecret writes a single secret into a provider.
-func (p *PP) PushSecret(_ context.Context, _ []byte, _ corev1.SecretType, _ *apiextensionsv1.JSON, _ PushRemoteRef) error {
+func (p *PP) PushSecret(_ context.Context, _ *corev1.Secret, _ PushSecretData) error {
 	return nil
 }
 
 // DeleteSecret deletes a single secret from a provider.
-func (p *PP) DeleteSecret(_ context.Context, _ PushRemoteRef) error {
+func (p *PP) DeleteSecret(_ context.Context, _ PushSecretRemoteRef) error {
 	return nil
 }
 

apis/externalsecrets/v1beta1/pushsecret_interfaces.go

@@ -13,13 +13,28 @@ limitations under the License.
 */
 package v1beta1
 
+import apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+
+// +kubebuilder:object:root=false
+// +kubebuilder:object:generate:false
+// +k8s:deepcopy-gen:interfaces=nil
+// +k8s:deepcopy-gen=nil
+
+// PushSecretData is an interface to allow using v1alpha1.PushSecretData content in Provider registered in v1beta1.
+type PushSecretData interface {
+	GetMetadata() *apiextensionsv1.JSON
+	GetSecretKey() string
+	GetRemoteKey() string
+	GetProperty() string
+}
+
 // +kubebuilder:object:root=false
 // +kubebuilder:object:generate:false
 // +k8s:deepcopy-gen:interfaces=nil
 // +k8s:deepcopy-gen=nil
 
-// This interface is to allow using v1alpha1 content in Provider registered in v1beta1.
-type PushRemoteRef interface {
+// PushSecretRemoteRef is an interface to allow using v1alpha1.PushSecretRemoteRef in Provider registered in v1beta1.
+type PushSecretRemoteRef interface {
 	GetRemoteKey() string
 	GetProperty() string
 }

docs/api/spec.md

@@ -4664,10 +4664,15 @@ External Secrets meta/v1.SecretKeySelector
 <p>
 <p>Provider is a common interface for interacting with secret backends.</p>
 </p>
-<h3 id="external-secrets.io/v1beta1.PushRemoteRef">PushRemoteRef
+<h3 id="external-secrets.io/v1beta1.PushSecretData">PushSecretData
 </h3>
 <p>
-<p>This interface is to allow using v1alpha1 content in Provider registered in v1beta1.</p>
+<p>PushSecretData is an interface to allow using v1alpha1.PushSecretData content in Provider registered in v1beta1.</p>
+</p>
+<h3 id="external-secrets.io/v1beta1.PushSecretRemoteRef">PushSecretRemoteRef
+</h3>
+<p>
+<p>PushSecretRemoteRef is an interface to allow using v1alpha1.PushSecretRemoteRef in Provider registered in v1beta1.</p>
 </p>
 <h3 id="external-secrets.io/v1beta1.ScalewayProvider">ScalewayProvider
 </h3>

pkg/controllers/pushsecret/pushsecret_controller.go

@@ -275,21 +275,20 @@ func (r *Reconciler) PushSecretToProviders(ctx context.Context, stores map[esapi
 			Name: store.GetName(),
 			Kind: ref.Kind,
 		}
-		client, err := mgr.Get(ctx, storeRef, ps.GetNamespace(), nil)
+		secretClient, err := mgr.Get(ctx, storeRef, ps.GetNamespace(), nil)
 		if err != nil {
 			return out, fmt.Errorf("could not get secrets client for store %v: %w", store.GetName(), err)
 		}
 		for _, data := range ps.Spec.Data {
-			secretValue, ok := secret.Data[data.Match.SecretKey]
-			if !ok {
+			if _, ok := secret.Data[data.Match.SecretKey]; !ok {
 				return out, fmt.Errorf("secret key %v does not exist", data.Match.SecretKey)
 			}
 
-			err := client.PushSecret(ctx, secretValue, secret.Type, data.Metadata, data.Match.RemoteRef)
+			err := secretClient.PushSecret(ctx, secret, data)
 			if err != nil {
 				return out, fmt.Errorf(errSetSecretFailed, data.Match.SecretKey, store.GetName(), err)
 			}
-			out[storeKey][statusRef(data.Match.RemoteRef)] = data
+			out[storeKey][statusRef(data)] = data
 		}
 	}
 	return out, nil
@@ -423,7 +422,7 @@ func getPushSecretCondition(status esapi.PushSecretStatus, condType esapi.PushSe
 	return nil
 }
 
-func statusRef(ref v1beta1.PushRemoteRef) string {
+func statusRef(ref v1beta1.PushSecretData) string {
 	if ref.GetProperty() != "" {
 		return ref.GetRemoteKey() + "/" + ref.GetProperty()
 	}

pkg/controllers/secretstore/client_manager_test.go

@@ -340,11 +340,11 @@ type MockFakeClient struct {
 	closeCalled bool
 }
 
-func (c *MockFakeClient) PushSecret(_ context.Context, _ []byte, _ corev1.SecretType, _ *apiextensionsv1.JSON, _ esv1beta1.PushRemoteRef) error {
+func (c *MockFakeClient) PushSecret(_ context.Context, _ *corev1.Secret, _ esv1beta1.PushSecretData) error {
 	return nil
 }
 
-func (c *MockFakeClient) DeleteSecret(_ context.Context, _ esv1beta1.PushRemoteRef) error {
+func (c *MockFakeClient) DeleteSecret(_ context.Context, _ esv1beta1.PushSecretRemoteRef) error {
 	return nil
 }
 

pkg/provider/akeyless/akeyless.go

@@ -30,7 +30,6 @@ import (
 	"github.com/akeylesslabs/akeyless-go/v3"
 	"github.com/tidwall/gjson"
 	corev1 "k8s.io/api/core/v1"
-	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/client-go/kubernetes"
 	typedcorev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -228,11 +227,11 @@ func (a *Akeyless) Validate() (esv1beta1.ValidationResult, error) {
 	return esv1beta1.ValidationResultReady, nil
 }
 
-func (a *Akeyless) PushSecret(_ context.Context, _ []byte, _ corev1.SecretType, _ *apiextensionsv1.JSON, _ esv1beta1.PushRemoteRef) error {
+func (a *Akeyless) PushSecret(_ context.Context, _ *corev1.Secret, _ esv1beta1.PushSecretData) error {
 	return fmt.Errorf("not implemented")
 }
 
-func (a *Akeyless) DeleteSecret(_ context.Context, _ esv1beta1.PushRemoteRef) error {
+func (a *Akeyless) DeleteSecret(_ context.Context, _ esv1beta1.PushSecretRemoteRef) error {
 	return fmt.Errorf("not implemented")
 }
 

pkg/provider/alibaba/kms.go

@@ -26,7 +26,6 @@ import (
 	"github.com/avast/retry-go/v4"
 	"github.com/tidwall/gjson"
 	corev1 "k8s.io/api/core/v1"
-	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	"k8s.io/apimachinery/pkg/types"
 	kclient "sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -59,11 +58,11 @@ type SMInterface interface {
 	Endpoint() string
 }
 
-func (kms *KeyManagementService) PushSecret(_ context.Context, _ []byte, _ corev1.SecretType, _ *apiextensionsv1.JSON, _ esv1beta1.PushRemoteRef) error {
+func (kms *KeyManagementService) PushSecret(_ context.Context, _ *corev1.Secret, _ esv1beta1.PushSecretData) error {
 	return fmt.Errorf("not implemented")
 }
 
-func (kms *KeyManagementService) DeleteSecret(_ context.Context, _ esv1beta1.PushRemoteRef) error {
+func (kms *KeyManagementService) DeleteSecret(_ context.Context, _ esv1beta1.PushSecretRemoteRef) error {
 	return fmt.Errorf("not implemented")
 }
 

pkg/provider/aws/parameterstore/parameterstore.go

@@ -27,7 +27,6 @@ import (
 	"github.com/aws/aws-sdk-go/service/ssm"
 	"github.com/tidwall/gjson"
 	corev1 "k8s.io/api/core/v1"
-	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	utilpointer "k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 
@@ -95,7 +94,7 @@ func (pm *ParameterStore) getTagsByName(ctx aws.Context, ref *ssm.GetParameterOu
 	return data.TagList, nil
 }
 
-func (pm *ParameterStore) DeleteSecret(ctx context.Context, remoteRef esv1beta1.PushRemoteRef) error {
+func (pm *ParameterStore) DeleteSecret(ctx context.Context, remoteRef esv1beta1.PushSecretRemoteRef) error {
 	secretName := remoteRef.GetRemoteKey()
 	secretValue := ssm.GetParameterInput{
 		Name: &secretName,
@@ -132,12 +131,13 @@ func (pm *ParameterStore) DeleteSecret(ctx context.Context, remoteRef esv1beta1.
 	return nil
 }
 
-func (pm *ParameterStore) PushSecret(ctx context.Context, value []byte, _ corev1.SecretType, _ *apiextensionsv1.JSON, remoteRef esv1beta1.PushRemoteRef) error {
+func (pm *ParameterStore) PushSecret(ctx context.Context, secret *corev1.Secret, data esv1beta1.PushSecretData) error {
 	parameterType := "String"
 	overwrite := true
 
+	value := secret.Data[data.GetSecretKey()]
 	stringValue := string(value)
-	secretName := remoteRef.GetRemoteKey()
+	secretName := data.GetRemoteKey()
 
 	secretRequest := ssm.PutParameterInput{
 		Name:      &secretName,

pkg/provider/aws/parameterstore/parameterstore_test.go

@@ -24,11 +24,13 @@ import (
 	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/ssm"
 	"github.com/google/go-cmp/cmp"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
 	fakeps "github.com/external-secrets/external-secrets/pkg/provider/aws/parameterstore/fake"
 	"github.com/external-secrets/external-secrets/pkg/provider/aws/util"
+	"github.com/external-secrets/external-secrets/pkg/provider/testing/fake"
 )
 
 const (
@@ -47,18 +49,6 @@ type parameterstoreTestCase struct {
 	expectedData   map[string][]byte
 }
 
-type fakeRef struct {
-	key string
-}
-
-func (f fakeRef) GetRemoteKey() string {
-	return f.key
-}
-
-func (f fakeRef) GetProperty() string {
-	return ""
-}
-
 func makeValidParameterStoreTestCase() *parameterstoreTestCase {
 	return &parameterstoreTestCase{
 		fakeClient:     &fakeps.Client{},
@@ -247,7 +237,7 @@ func TestDeleteSecret(t *testing.T) {
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			ref := fakeRef{key: "fake-key"}
+			ref := fake.PushSecretData{RemoteKey: "fake-key"}
 			ps := ParameterStore{
 				client: &tc.args.client,
 			}
@@ -273,7 +263,13 @@ func TestDeleteSecret(t *testing.T) {
 func TestPushSecret(t *testing.T) {
 	invalidParameters := errors.New(ssm.ErrCodeInvalidParameters)
 	alreadyExistsError := errors.New(ssm.ErrCodeAlreadyExistsException)
+	fakeSecretKey := "fakeSecretKey"
 	fakeValue := "fakeValue"
+	fakeSecret := &corev1.Secret{
+		Data: map[string][]byte{
+			fakeSecretKey: []byte(fakeValue),
+		},
+	}
 
 	managedByESO := ssm.Tag{
 		Key:   &managedBy,
@@ -431,11 +427,11 @@ func TestPushSecret(t *testing.T) {
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			ref := fakeRef{key: "fake-key"}
+			psd := fake.PushSecretData{SecretKey: "fake-secret-key", RemoteKey: "fake-key"}
 			ps := ParameterStore{
 				client: &tc.args.client,
 			}
-			err := ps.PushSecret(context.TODO(), []byte(fakeValue), "", nil, ref)
+			err := ps.PushSecret(context.TODO(), fakeSecret, psd)
 
 			// Error nil XOR tc.want.err nil
 			if ((err == nil) || (tc.want.err == nil)) && !((err == nil) && (tc.want.err == nil)) {

pkg/provider/aws/secretsmanager/secretsmanager.go

@@ -32,7 +32,6 @@ import (
 	"github.com/tidwall/gjson"
 	"github.com/tidwall/sjson"
 	corev1 "k8s.io/api/core/v1"
-	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	utilpointer "k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 
@@ -157,7 +156,7 @@ func (sm *SecretsManager) fetch(ctx context.Context, ref esv1beta1.ExternalSecre
 	return secretOut, nil
 }
 
-func (sm *SecretsManager) DeleteSecret(ctx context.Context, remoteRef esv1beta1.PushRemoteRef) error {
+func (sm *SecretsManager) DeleteSecret(ctx context.Context, remoteRef esv1beta1.PushSecretRemoteRef) error {
 	secretName := remoteRef.GetRemoteKey()
 	secretValue := awssm.GetSecretValueInput{
 		SecretId: &secretName,
@@ -193,8 +192,9 @@ func (sm *SecretsManager) DeleteSecret(ctx context.Context, remoteRef esv1beta1.
 	return err
 }
 
-func (sm *SecretsManager) PushSecret(ctx context.Context, value []byte, _ corev1.SecretType, _ *apiextensionsv1.JSON, remoteRef esv1beta1.PushRemoteRef) error {
-	secretName := remoteRef.GetRemoteKey()
+func (sm *SecretsManager) PushSecret(ctx context.Context, secret *corev1.Secret, psd esv1beta1.PushSecretData) error {
+	secretName := psd.GetRemoteKey()
+	value := secret.Data[psd.GetSecretKey()]
 	managedBy := managedBy
 	externalSecrets := externalSecrets
 	externalSecretsTag := []*awssm.Tag{
@@ -215,12 +215,12 @@ func (sm *SecretsManager) PushSecret(ctx context.Context, value []byte, _ corev1
 	awsSecret, err := sm.client.GetSecretValueWithContext(ctx, &secretValue)
 	metrics.ObserveAPICall(constants.ProviderAWSSM, constants.CallAWSSMGetSecretValue, err)
 
-	if remoteRef.GetProperty() != "" {
+	if psd.GetProperty() != "" {
 		currentSecret := sm.retrievePayload(awsSecret)
 		if currentSecret != "" && !gjson.Valid(currentSecret) {
-			return errors.New("PushSecret for aws secrets manager with a remoteRef property requires a json secret")
+			return errors.New("PushSecret for aws secrets manager with a pushSecretData property requires a json secret")
 		}
-		value, _ = sjson.SetBytes([]byte(currentSecret), remoteRef.GetProperty(), value)
+		value, _ = sjson.SetBytes([]byte(currentSecret), psd.GetProperty(), value)
 	}
 
 	var aerr awserr.Error