Scaleway secret path (#2737)

* feat: add path support for scaleway provider

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

* feat: update scaleway testcases for path support

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

* docs: update scaleway doc to add path support

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

* fix: change func signature to make linter pass

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

---------

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

docs/provider/scaleway.md

@@ -28,7 +28,7 @@ spec:
 
 ### Referencing Secrets
 
-Secrets can be referenced by name or by id, using the prefixes `"name:"` and `"id:"` respectively.
+Secrets can be referenced by name, id or path, using the prefixes `"name:"`, `"id:"` and `"path:"` respectively.
 
 A PushSecret resource can only use a name reference.
 

pkg/provider/scaleway/client.go

@@ -113,13 +113,23 @@ func (c *client) PushSecret(ctx context.Context, value []byte, _ *apiextensionsv
 		PageSize:  scw.Uint32Ptr(1),
 	}
 	var secretName string
+	secretPath := "/"
 
 	switch scwRef.RefType {
 	case refTypeName:
 		listSecretReq.Name = &scwRef.Value
 		secretName = scwRef.Value
+	case refTypePath:
+		name, path, ok := splitNameAndPath(scwRef.Value)
+		if !ok {
+			return fmt.Errorf("ref is not a path")
+		}
+		listSecretReq.Name = &name
+		listSecretReq.Path = &path
+		secretName = name
+		secretPath = path
 	default:
-		return fmt.Errorf("secrets can only be pushed by name")
+		return fmt.Errorf("secrets can only be pushed by name or path")
 	}
 
 	var secretID string
@@ -164,6 +174,7 @@ func (c *client) PushSecret(ctx context.Context, value []byte, _ *apiextensionsv
 		secret, err := c.api.CreateSecret(&smapi.CreateSecretRequest{
 			ProjectID: c.projectID,
 			Name:      secretName,
+			Path:      &secretPath,
 		}, scw.WithContext(ctx))
 		if err != nil {
 			return err
@@ -214,8 +225,16 @@ func (c *client) DeleteSecret(ctx context.Context, remoteRef esv1beta1.PushRemot
 	switch scwRef.RefType {
 	case refTypeName:
 		listSecretReq.Name = &scwRef.Value
+	case refTypePath:
+		name, path, ok := splitNameAndPath(scwRef.Value)
+		if !ok {
+			return fmt.Errorf("ref is not a path")
+		}
+		listSecretReq.Name = &name
+		listSecretReq.Path = &path
+
 	default:
-		return fmt.Errorf("secrets can only be pushed by name")
+		return fmt.Errorf("secrets can only be deleted by name or path")
 	}
 
 	listSecrets, err := c.api.ListSecrets(listSecretReq, scw.WithContext(ctx))
@@ -286,7 +305,7 @@ func (c *client) GetAllSecrets(ctx context.Context, ref esv1beta1.ExternalSecret
 	}
 
 	if ref.Path != nil {
-		return nil, fmt.Errorf("searching by path is not supported")
+		request.Path = ref.Path
 	}
 
 	var nameMatcher *find.Matcher
@@ -356,9 +375,11 @@ func (c *client) accessSecretVersion(ctx context.Context, secretRef *scwSecretRe
 	}
 
 	// otherwise, we do a GetSecret() first to avoid transferring the secret value if it is cached
-
-	var secretID string
-	var secretRevision uint32
+	request := &smapi.ListSecretsRequest{
+		ProjectID: &c.projectID,
+		Page:      scw.Int32Ptr(1),
+		PageSize:  scw.Uint32Ptr(1),
+	}
 
 	switch secretRef.RefType {
 	case refTypeID:
@@ -370,41 +391,42 @@ func (c *client) accessSecretVersion(ctx context.Context, secretRef *scwSecretRe
 		if err != nil {
 			return nil, err
 		}
-		secretID = response.SecretID
-		secretRevision = response.Revision
+		return c.accessSpecificSecretVersion(ctx, response.SecretID, response.Revision)
 	case refTypeName:
-		request := &smapi.ListSecretsRequest{
-			ProjectID: &c.projectID,
-			Page:      scw.Int32Ptr(1),
-			PageSize:  scw.Uint32Ptr(1),
-			Name:      &secretRef.Value,
-		}
+		request.Name = &secretRef.Value
 
-		response, err := c.api.ListSecrets(request, scw.WithContext(ctx))
-		if err != nil {
-			return nil, err
+	case refTypePath:
+		name, path, ok := splitNameAndPath(secretRef.Value)
+		if !ok {
+			return nil, fmt.Errorf("ref is not a path")
 		}
 
-		if len(response.Secrets) == 0 {
-			return nil, errNoSecretForName
-		}
+		request.Name = &name
+		request.Path = &path
+	default:
+		return nil, fmt.Errorf("invalid secret reference: %q", secretRef.Value)
+	}
 
-		secretID = response.Secrets[0].ID
+	response, err := c.api.ListSecrets(request, scw.WithContext(ctx))
+	if err != nil {
+		return nil, err
+	}
 
-		secretVersion, err := c.api.GetSecretVersion(&smapi.GetSecretVersionRequest{
-			SecretID: secretID,
-			Revision: versionSpec,
-		}, scw.WithContext(ctx))
-		if err != nil {
-			return nil, err
-		}
+	if len(response.Secrets) == 0 {
+		return nil, errNoSecretForName
+	}
 
-		secretRevision = secretVersion.Revision
-	default:
-		return nil, fmt.Errorf("invalid secret reference: %q", secretRef.Value)
+	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
 	}
 
-	return c.accessSpecificSecretVersion(ctx, secretID, secretRevision)
+	return c.accessSpecificSecretVersion(ctx, secretID, secretVersion.Revision)
 }
 
 func (c *client) accessSpecificSecretVersion(ctx context.Context, secretID string, revision uint32) ([]byte, error) {
@@ -445,3 +467,19 @@ func extractJSONProperty(secretData []byte, property string) ([]byte, error) {
 
 	return jsonToSecretData(json.RawMessage(result.Raw)), nil
 }
+
+func splitNameAndPath(ref string) (name, path string, ok bool) {
+	if !strings.HasPrefix(ref, "/") {
+		return
+	}
+
+	s := strings.Split(ref, "/")
+	name = s[len(s)-1]
+	if len(s) == 2 {
+		path = "/"
+	} else {
+		path = strings.Join(s[:len(s)-1], "/")
+	}
+	ok = true
+	return
+}

pkg/provider/scaleway/client_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	esv1beta1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
+	"github.com/external-secrets/external-secrets/pkg/utils"
 )
 
 var db = buildDB(&fakeSecretAPI{
@@ -79,6 +80,16 @@ var db = buildDB(&fakeSecretAPI{
 				)},
 			},
 		},
+		{
+			name: "nested-secret",
+			path: "/subpath",
+			versions: []*fakeSecretVersion{
+				{
+					revision: 1,
+					data:     []byte("secret data"),
+				},
+			},
+		},
 	},
 })
 
@@ -143,6 +154,13 @@ func TestGetSecret(t *testing.T) {
 			},
 			response: []byte("9"),
 		},
+		"secret in path": {
+			ref: esv1beta1.ExternalSecretDataRemoteRef{
+				Key:     "path:/subpath/nested-secret",
+				Version: "latest",
+			},
+			response: []byte("secret data"),
+		},
 		"non existing secret id should yield NoSecretErr": {
 			ref: esv1beta1.ExternalSecretDataRemoteRef{
 				Key: "id:730aa98d-ec0c-4426-8202-b11aeec8ea1e",
@@ -238,6 +256,20 @@ func TestPushSecret(t *testing.T) {
 		assert.Equal(t, data, db.secret(secretName).versions[0].data)
 	})
 
+	t.Run("secret created in path", func(t *testing.T) {
+		ctx := context.Background()
+		c := newTestClient()
+		data := []byte("some secret data in path")
+		secretPath := "/folder"
+		secretName := "secret-in-path"
+
+		pushErr := c.PushSecret(ctx, data, nil, pushRemoteRef("path:"+secretPath+"/"+secretName))
+		assert.NoError(t, pushErr)
+		assert.Len(t, db.secret(secretName).versions, 1)
+		assert.Equal(t, data, db.secret(secretName).versions[0].data)
+		assert.Equal(t, secretPath, db.secret(secretName).path)
+	})
+
 	t.Run("by invalid secret ref is an error", func(t *testing.T) {
 		ctx := context.Background()
 		c := newTestClient()
@@ -339,6 +371,14 @@ func TestGetAllSecrets(t *testing.T) {
 				db.secrets[1].name: db.secrets[1].mustGetVersion("latest").data,
 			},
 		},
+		"find secrets by path": {
+			ref: esv1beta1.ExternalSecretFind{
+				Path: utils.Ptr("/subpath"),
+			},
+			response: map[string][]byte{
+				db.secret("nested-secret").name: db.secret("nested-secret").mustGetVersion("latest_enabled").data,
+			},
+		},
 	}
 
 	for tcName, tc := range testCases {
@@ -361,6 +401,7 @@ func TestDeleteSecret(t *testing.T) {
 	c := newTestClient()
 
 	secret := db.secrets[0]
+	byPath := db.secret("nested-secret")
 
 	testCases := map[string]struct {
 		ref esv1beta1.PushRemoteRef
@@ -370,6 +411,10 @@ func TestDeleteSecret(t *testing.T) {
 			ref: pushRemoteRef("name:" + secret.name),
 			err: nil,
 		},
+		"Delete by path": {
+			ref: pushRemoteRef("path:" + byPath.path + "/" + byPath.name),
+			err: nil,
+		},
 		"Secret Not Found": {
 			ref: pushRemoteRef("name:not-a-secret"),
 			err: nil,
@@ -388,3 +433,43 @@ func TestDeleteSecret(t *testing.T) {
 		})
 	}
 }
+
+func TestSplitNameAndPath(t *testing.T) {
+	type test struct {
+		in   string
+		name string
+		path string
+		ok   bool
+	}
+
+	tests := []test{
+		{
+			in:   "/foo",
+			name: "foo",
+			path: "/",
+			ok:   true,
+		},
+		{
+			in:   "",
+			name: "",
+			path: "",
+		},
+		{
+			in:   "/foo/bar",
+			name: "bar",
+			path: "/foo",
+			ok:   true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.in, func(t *testing.T) {
+			name, path, ok := splitNameAndPath(tc.in)
+			assert.Equal(t, tc.ok, ok, "bad ref")
+			if tc.ok {
+				assert.Equal(t, tc.name, name, "wrong name")
+				assert.Equal(t, tc.path, path, "wrong path")
+			}
+		})
+	}
+}

pkg/provider/scaleway/fake_secret_api_test.go

@@ -36,6 +36,7 @@ type fakeSecret struct {
 	versions []*fakeSecretVersion
 	tags     []string
 	status   string
+	path     string
 }
 
 var _ secretAPI = (*fakeSecretAPI)(nil)
@@ -55,6 +56,10 @@ func buildDB(f *fakeSecretAPI) *fakeSecretAPI {
 			secret.id = uuid.NewString()
 		}
 
+		if secret.path == "" {
+			secret.path = "/"
+		}
+
 		sort.Slice(secret.versions, func(i, j int) bool {
 			return secret.versions[i].revision < secret.versions[j].revision
 		})
@@ -166,6 +171,7 @@ func (f *fakeSecretAPI) GetSecret(request *smapi.GetSecretRequest, _ ...scw.Requ
 		Status:       smapi.SecretStatus(secret.status),
 		Tags:         secret.tags,
 		VersionCount: uint32(len(secret.versions)),
+		Path:         secret.path,
 	}, nil
 }
 
@@ -246,41 +252,43 @@ func (f *fakeSecretAPI) DisableSecretVersion(request *smapi.DisableSecretVersion
 	}, nil
 }
 
+type secretFilter func(*fakeSecret) bool
+
 func matchListSecretFilter(secret *fakeSecret, filter *smapi.ListSecretsRequest) bool {
-	var matchTag, matchName, found bool
+	filters := make([]secretFilter, 0)
 
 	if filter.Tags != nil {
-		matchTag = true
-		for _, requiredTag := range filter.Tags {
-			for _, secretTag := range secret.tags {
-				if requiredTag == secretTag {
-					found = true
-					break
+		filters = append(filters, func(fs *fakeSecret) bool {
+			for _, requiredTag := range filter.Tags {
+				for _, secretTag := range fs.tags {
+					if requiredTag == secretTag {
+						return true
+					}
 				}
 			}
-		}
+			return false
+		})
 	}
 
 	if filter.Name != nil {
-		matchName = true
-		if *filter.Name == secret.name {
-			found = true
-		}
+		filters = append(filters, func(fs *fakeSecret) bool {
+			return *filter.Name == fs.name
+		})
 	}
 
-	if matchTag && found {
-		return true
+	if filter.Path != nil {
+		filters = append(filters, func(fs *fakeSecret) bool {
+			return *filter.Path == fs.path
+		})
 	}
 
-	if matchName && found {
-		return true
-	}
+	match := true
 
-	if !matchName && !matchTag {
-		return true
+	for _, filterFn := range filters {
+		match = match && filterFn(secret)
 	}
 
-	return false
+	return match
 }
 
 func (f *fakeSecretAPI) ListSecrets(request *smapi.ListSecretsRequest, _ ...scw.RequestOption) (*smapi.ListSecretsResponse, error) {
@@ -333,6 +341,7 @@ func (f *fakeSecretAPI) ListSecrets(request *smapi.ListSecretsRequest, _ ...scw.
 			Status:       smapi.SecretStatus(secret.status),
 			Tags:         secret.tags,
 			VersionCount: uint32(len(secret.versions)),
+			Path:         secret.path,
 		})
 	}
 
@@ -344,10 +353,16 @@ func (f *fakeSecretAPI) CreateSecret(request *smapi.CreateSecretRequest, _ ...sc
 		panic("explicit region in request is not supported")
 	}
 
+	path := "/"
+	if request.Path != nil {
+		path = *request.Path
+	}
+
 	secret := &fakeSecret{
 		id:     uuid.NewString(),
 		name:   request.Name,
 		status: "ready",
+		path:   path,
 	}
 
 	f.secrets = append(f.secrets, secret)
@@ -360,6 +375,7 @@ func (f *fakeSecretAPI) CreateSecret(request *smapi.CreateSecretRequest, _ ...sc
 		Name:         secret.name,
 		Status:       smapi.SecretStatus(secret.status),
 		VersionCount: 0,
+		Path:         secret.path,
 	}, nil
 }