Refacto scaleway provider (#2667)

* replace API calls by ListSecrets

Scalewaw will deprecate soon the Secret Manager endpoints *ByName, the
idea is to use the ListSecrets instead and to filter by name

Signed-off-by: Florent Viel <fviel@scaleway.com>

* allow to filter by name for the fake scw secret api

Signed-off-by: Florent Viel <fviel@scaleway.com>

* improve scaleway fake api

Signed-off-by: Florent Viel <fviel@scaleway.com>

the fake implementation of the ListSecrets for the Scaleway API was too
naive and returns too many secrets based on the input filters

* scaleway: fix calls to the ListSecrets endpoint

Signed-off-by: Florent Viel <fviel@scaleway.com>

* scaleway: fix lint issues

Signed-off-by: Florent Viel <fviel@scaleway.com>

---------

Signed-off-by: Florent Viel <fviel@scaleway.com>

pkg/provider/scaleway/client.go

@@ -43,6 +43,7 @@ type client struct {
 const (
 	refTypeName = "name"
 	refTypeID   = "id"
+	refTypePath = "path"
 )
 
 type scwSecretRef struct {
@@ -66,23 +67,6 @@ func decodeScwSecretRef(key string) (*scwSecretRef, error) {
 	}, nil
 }
 
-func (c *client) getSecretByName(ctx context.Context, name string) (*smapi.Secret, error) {
-	request := smapi.GetSecretByNameRequest{
-		SecretName: name,
-	}
-
-	response, err := c.api.GetSecretByName(&request, scw.WithContext(ctx))
-	if err != nil {
-		//nolint:errorlint
-		if _, isErrNotFound := err.(*scw.ResourceNotFoundError); isErrNotFound {
-			return nil, errNoSecretForName
-		}
-		return nil, err
-	}
-
-	return response, nil
-}
-
 func (c *client) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretDataRemoteRef) ([]byte, error) {
 	scwRef, err := decodeScwSecretRef(ref.Key)
 	if err != nil {
@@ -99,6 +83,8 @@ func (c *client) GetSecret(ctx context.Context, ref esv1beta1.ExternalSecretData
 		//nolint:errorlint
 		if _, isNotFoundErr := err.(*scw.ResourceNotFoundError); isNotFoundErr {
 			return nil, esv1beta1.NoSecretError{}
+		} else if errors.Is(err, errNoSecretForName) {
+			return nil, esv1beta1.NoSecretError{}
 		}
 		return nil, err
 	}
@@ -121,41 +107,49 @@ func (c *client) PushSecret(ctx context.Context, value []byte, _ *apiextensionsv
 		return err
 	}
 
-	if scwRef.RefType != refTypeName {
-		return fmt.Errorf("secrets can only be pushed by name")
+	listSecretReq := &smapi.ListSecretsRequest{
+		ProjectID: &c.projectID,
+		Page:      scw.Int32Ptr(1),
+		PageSize:  scw.Uint32Ptr(1),
 	}
-	secretName := scwRef.Value
+	var secretName string
 
-	// First, we do a GetSecretVersion() to resolve the secret id and the last revision number.
+	switch scwRef.RefType {
+	case refTypeName:
+		listSecretReq.Name = &scwRef.Value
+		secretName = scwRef.Value
+	default:
+		return fmt.Errorf("secrets can only be pushed by name")
+	}
 
 	var secretID string
-	secretExists := false
 	existingSecretVersion := int64(-1)
 
-	secretVersion, err := c.api.GetSecretVersionByName(&smapi.GetSecretVersionByNameRequest{
-		SecretName: secretName,
-		Revision:   "latest",
-	}, scw.WithContext(ctx))
+	// list secret by ref
+	listSecrets, err := c.api.ListSecrets(listSecretReq, scw.WithContext(ctx))
 	if err != nil {
-		//nolint:errorlint
-		if notFoundErr, ok := err.(*scw.ResourceNotFoundError); ok {
-			if notFoundErr.Resource == "secret_version" {
-				secretExists = true
+		return err
+	}
+
+	// secret exists
+	if len(listSecrets.Secrets) > 0 {
+		secretID = listSecrets.Secrets[0].ID
+
+		// get the latest version
+		secretVersion, err := c.api.GetSecretVersion(&smapi.GetSecretVersionRequest{
+			SecretID: secretID,
+			Revision: "latest",
+		}, scw.WithContext(ctx))
+		if err != nil {
+			var errNotNound *scw.ResourceNotFoundError
+			if !errors.As(err, &errNotNound) {
+				return err
 			}
 		} else {
-			return err
+			existingSecretVersion = int64(secretVersion.Revision)
 		}
-	} else {
-		secretExists = true
-		existingSecretVersion = int64(secretVersion.Revision)
-	}
 
-	if secretExists {
 		if existingSecretVersion != -1 {
-			// If the secret exists, we can fetch its last value to see if we have any change to make.
-
-			secretID = secretVersion.SecretID
-
 			data, err := c.accessSpecificSecretVersion(ctx, secretID, secretVersion.Revision)
 			if err != nil {
 				return err
@@ -165,22 +159,8 @@ func (c *client) PushSecret(ctx context.Context, value []byte, _ *apiextensionsv
 				// No change to push.
 				return nil
 			}
-		} else {
-			// If the secret exists but has no versions, we need an additional GetSecret() to resolve the secret id.
-			// This may happen if a push was interrupted.
-
-			secret, err := c.api.GetSecretByName(&smapi.GetSecretByNameRequest{
-				SecretName: secretName,
-			}, scw.WithContext(ctx))
-			if err != nil {
-				return err
-			}
-
-			secretID = secret.ID
 		}
 	} else {
-		// If the secret does not exist, we need to create it.
-
 		secret, err := c.api.CreateSecret(&smapi.CreateSecretRequest{
 			ProjectID: c.projectID,
 			Name:      secretName,
@@ -206,7 +186,7 @@ func (c *client) PushSecret(ctx context.Context, value []byte, _ *apiextensionsv
 
 	c.cache.Put(secretID, createSecretVersionResponse.Revision, value)
 
-	if secretExists && existingSecretVersion != -1 {
+	if existingSecretVersion != -1 {
 		_, err := c.api.DisableSecretVersion(&smapi.DisableSecretVersionRequest{
 			SecretID: secretID,
 			Revision: fmt.Sprintf("%d", existingSecretVersion),
@@ -225,21 +205,30 @@ func (c *client) DeleteSecret(ctx context.Context, remoteRef esv1beta1.PushRemot
 		return err
 	}
 
-	if scwRef.RefType != refTypeName {
+	listSecretReq := &smapi.ListSecretsRequest{
+		ProjectID: &c.projectID,
+		Page:      scw.Int32Ptr(1),
+		PageSize:  scw.Uint32Ptr(1),
+	}
+
+	switch scwRef.RefType {
+	case refTypeName:
+		listSecretReq.Name = &scwRef.Value
+	default:
 		return fmt.Errorf("secrets can only be pushed by name")
 	}
-	secretName := scwRef.Value
 
-	secret, err := c.getSecretByName(ctx, secretName)
+	listSecrets, err := c.api.ListSecrets(listSecretReq, scw.WithContext(ctx))
 	if err != nil {
-		if errors.Is(err, errNoSecretForName) {
-			return nil
-		}
 		return err
 	}
 
+	if len(listSecrets.Secrets) == 0 {
+		return nil
+	}
+
 	request := smapi.DeleteSecretRequest{
-		SecretID: secret.ID,
+		SecretID: listSecrets.Secrets[0].ID,
 	}
 
 	err = c.api.DeleteSecret(&request, scw.WithContext(ctx))
@@ -254,12 +243,10 @@ func (c *client) Validate() (esv1beta1.ValidationResult, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
-	page := int32(1)
-	pageSize := uint32(0)
 	_, err := c.api.ListSecrets(&smapi.ListSecretsRequest{
 		ProjectID: &c.projectID,
-		Page:      &page,
-		PageSize:  &pageSize,
+		Page:      scw.Int32Ptr(1),
+		PageSize:  scw.Uint32Ptr(0),
 	}, scw.WithContext(ctx))
 	if err != nil {
 		return esv1beta1.ValidationResultError, nil
@@ -294,11 +281,9 @@ func (c *client) GetSecretMap(ctx context.Context, ref esv1beta1.ExternalSecretD
 func (c *client) GetAllSecrets(ctx context.Context, ref esv1beta1.ExternalSecretFind) (map[string][]byte, error) {
 	request := smapi.ListSecretsRequest{
 		ProjectID: &c.projectID,
-		Page:      new(int32),
-		PageSize:  new(uint32),
+		Page:      scw.Int32Ptr(1),
+		PageSize:  scw.Uint32Ptr(50),
 	}
-	*request.Page = 1
-	*request.PageSize = 50
 
 	if ref.Path != nil {
 		return nil, fmt.Errorf("searching by path is not supported")
@@ -388,16 +373,33 @@ func (c *client) accessSecretVersion(ctx context.Context, secretRef *scwSecretRe
 		secretID = response.SecretID
 		secretRevision = response.Revision
 	case refTypeName:
-		request := smapi.GetSecretVersionByNameRequest{
-			SecretName: secretRef.Value,
-			Revision:   versionSpec,
+		request := &smapi.ListSecretsRequest{
+			ProjectID: &c.projectID,
+			Page:      scw.Int32Ptr(1),
+			PageSize:  scw.Uint32Ptr(1),
+			Name:      &secretRef.Value,
 		}
-		response, err := c.api.GetSecretVersionByName(&request, scw.WithContext(ctx))
+
+		response, err := c.api.ListSecrets(request, scw.WithContext(ctx))
 		if err != nil {
 			return nil, err
 		}
-		secretID = response.SecretID
-		secretRevision = response.Revision
+
+		if len(response.Secrets) == 0 {
+			return nil, errNoSecretForName
+		}
+
+		secretID = response.Secrets[0].ID
+
+		secretVersion, err := c.api.GetSecretVersion(&smapi.GetSecretVersionRequest{
+			SecretID: secretID,
+			Revision: versionSpec,
+		}, scw.WithContext(ctx))
+		if err != nil {
+			return nil, err
+		}
+
+		secretRevision = secretVersion.Revision
 	default:
 		return nil, fmt.Errorf("invalid secret reference: %q", secretRef.Value)
 	}

pkg/provider/scaleway/fake_secret_api_test.go

@@ -38,6 +38,8 @@ type fakeSecret struct {
 	status   string
 }
 
+var _ secretAPI = (*fakeSecretAPI)(nil)
+
 type fakeSecretAPI struct {
 	secrets        []*fakeSecret
 	_secretsByID   map[string]*fakeSecret
@@ -148,19 +150,6 @@ func (f *fakeSecretAPI) getSecretByID(secretID string) (*fakeSecret, error) {
 	return secret, nil
 }
 
-func (f *fakeSecretAPI) getSecretByName(secretName string) (*fakeSecret, error) {
-	secret, foundSecret := f._secretsByName[secretName]
-
-	if !foundSecret {
-		return nil, &scw.ResourceNotFoundError{
-			Resource:   "secret",
-			ResourceID: secretName,
-		}
-	}
-
-	return secret, nil
-}
-
 func (f *fakeSecretAPI) GetSecret(request *smapi.GetSecretRequest, _ ...scw.RequestOption) (*smapi.Secret, error) {
 	if request.Region != "" {
 		panic("explicit region in request is not supported")
@@ -180,25 +169,6 @@ func (f *fakeSecretAPI) GetSecret(request *smapi.GetSecretRequest, _ ...scw.Requ
 	}, nil
 }
 
-func (f *fakeSecretAPI) GetSecretByName(request *smapi.GetSecretByNameRequest, _ ...scw.RequestOption) (*smapi.Secret, error) {
-	if request.Region != "" {
-		panic("explicit region in request is not supported")
-	}
-
-	secret, err := f.getSecretByName(request.SecretName)
-	if err != nil {
-		return nil, err
-	}
-
-	return &smapi.Secret{
-		ID:           secret.id,
-		Name:         secret.name,
-		Status:       smapi.SecretStatus(secret.status),
-		Tags:         secret.tags,
-		VersionCount: uint32(len(secret.versions)),
-	}, nil
-}
-
 func (f *fakeSecretAPI) GetSecretVersion(request *smapi.GetSecretVersionRequest, _ ...scw.RequestOption) (*smapi.SecretVersion, error) {
 	if request.Region != "" {
 		panic("explicit region in request is not supported")
@@ -224,31 +194,6 @@ func (f *fakeSecretAPI) GetSecretVersion(request *smapi.GetSecretVersionRequest,
 	}, nil
 }
 
-func (f *fakeSecretAPI) GetSecretVersionByName(request *smapi.GetSecretVersionByNameRequest, _ ...scw.RequestOption) (*smapi.SecretVersion, error) {
-	if request.Region != "" {
-		panic("explicit region in request is not supported")
-	}
-
-	secret, err := f.getSecretByName(request.SecretName)
-	if err != nil {
-		return nil, err
-	}
-
-	version, ok := secret.getVersion(request.Revision)
-	if !ok {
-		return nil, &scw.ResourceNotFoundError{
-			Resource:   "secret_version",
-			ResourceID: request.Revision,
-		}
-	}
-
-	return &smapi.SecretVersion{
-		SecretID: secret.id,
-		Revision: uint32(version.revision),
-		Status:   smapi.SecretVersionStatus(secret.status),
-	}, nil
-}
-
 func (f *fakeSecretAPI) AccessSecretVersion(request *smapi.AccessSecretVersionRequest, _ ...scw.RequestOption) (*smapi.AccessSecretVersionResponse, error) {
 	if request.Region != "" {
 		panic("explicit region in request is not supported")
@@ -302,22 +247,40 @@ func (f *fakeSecretAPI) DisableSecretVersion(request *smapi.DisableSecretVersion
 }
 
 func matchListSecretFilter(secret *fakeSecret, filter *smapi.ListSecretsRequest) bool {
-	for _, requiredTag := range filter.Tags {
-		found := false
-
-		for _, secretTag := range secret.tags {
-			if requiredTag == secretTag {
-				found = true
-				break
+	var matchTag, matchName, found bool
+
+	if filter.Tags != nil {
+		matchTag = true
+		for _, requiredTag := range filter.Tags {
+			for _, secretTag := range secret.tags {
+				if requiredTag == secretTag {
+					found = true
+					break
+				}
 			}
 		}
+	}
 
-		if !found {
-			return false
+	if filter.Name != nil {
+		matchName = true
+		if *filter.Name == secret.name {
+			found = true
 		}
 	}
 
-	return true
+	if matchTag && found {
+		return true
+	}
+
+	if matchName && found {
+		return true
+	}
+
+	if !matchName && !matchTag {
+		return true
+	}
+
+	return false
 }
 
 func (f *fakeSecretAPI) ListSecrets(request *smapi.ListSecretsRequest, _ ...scw.RequestOption) (*smapi.ListSecretsResponse, error) {

pkg/provider/scaleway/secret_api.go

@@ -20,9 +20,7 @@ import (
 
 type secretAPI interface {
 	GetSecret(req *smapi.GetSecretRequest, opts ...scw.RequestOption) (*smapi.Secret, error)
-	GetSecretByName(req *smapi.GetSecretByNameRequest, opts ...scw.RequestOption) (*smapi.Secret, error)
 	GetSecretVersion(req *smapi.GetSecretVersionRequest, opts ...scw.RequestOption) (*smapi.SecretVersion, error)
-	GetSecretVersionByName(req *smapi.GetSecretVersionByNameRequest, opts ...scw.RequestOption) (*smapi.SecretVersion, error)
 	AccessSecretVersion(request *smapi.AccessSecretVersionRequest, option ...scw.RequestOption) (*smapi.AccessSecretVersionResponse, error)
 	DisableSecretVersion(request *smapi.DisableSecretVersionRequest, option ...scw.RequestOption) (*smapi.SecretVersion, error)
 	ListSecrets(request *smapi.ListSecretsRequest, option ...scw.RequestOption) (*smapi.ListSecretsResponse, error)