Browse Source

fix(barbican): treat find.name.regexp as a regexp, not an exact match (#6614)

GetAllSecrets passed ref.Name.RegExp straight into Barbican's list API as
an exact name filter, so a value like ^db-.* only matched a secret named
literally ^db-.* and never behaved as a regular expression. Every other
ESO provider treats find.name.regexp as a regexp.

Compile the pattern and filter the listed secret names client-side with
MatchString, dropping the server-side exact-name filter that cannot express
a regexp. Add a test that lists db-a, db-b and web-a from a fake Barbican
endpoint and asserts ^db- returns only the two db-* secrets.

Fixes #6533

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Co-authored-by: Jean-Philippe Evrard <jean-philippe.evrard+rochepub@external.roche.com>
Arpit Jain 4 days ago
parent
commit
68826f87b4
2 changed files with 85 additions and 8 deletions
  1. 16 8
      providers/v1/barbican/client.go
  2. 69 0
      providers/v1/barbican/client_test.go

+ 16 - 8
providers/v1/barbican/client.go

@@ -22,6 +22,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"regexp"
 	"strings"
 
 	"github.com/gophercloud/gophercloud/v2"
@@ -54,11 +55,15 @@ func (c *Client) GetAllSecrets(ctx context.Context, ref esapi.ExternalSecretFind
 		return nil, fmt.Errorf(errClientMissingField, errors.New("name and/or regexp"))
 	}
 
-	opts := secrets.ListOpts{
-		Name: ref.Name.RegExp,
+	// Barbican's list API only supports exact-name matching, so we can't push
+	// the regexp down to the server. List everything and match client-side, the
+	// same way the other ESO providers treat find.name.regexp.
+	nameMatcher, err := regexp.Compile(ref.Name.RegExp)
+	if err != nil {
+		return nil, fmt.Errorf(errClientGeneric, fmt.Errorf("invalid name regexp %q: %w", ref.Name.RegExp, err))
 	}
 
-	allPages, err := secrets.List(c.keyManager, opts).AllPages(ctx)
+	allPages, err := secrets.List(c.keyManager, secrets.ListOpts{}).AllPages(ctx)
 	if err != nil {
 		return nil, fmt.Errorf(errClientListAllSecrets, err)
 	}
@@ -68,20 +73,23 @@ func (c *Client) GetAllSecrets(ctx context.Context, ref esapi.ExternalSecretFind
 		return nil, fmt.Errorf(errClientExtractSecrets, err)
 	}
 
-	if len(allSecrets) == 0 {
-		return nil, fmt.Errorf(errClientGeneric, errors.New("no secrets found"))
-	}
-
 	var secretsMap = make(map[string][]byte)
 
-	// return a secret map with all found secrets.
 	for _, secret := range allSecrets {
+		if !nameMatcher.MatchString(secret.Name) {
+			continue
+		}
 		secretUUID := extractUUIDFromRef(secret.SecretRef)
 		secretsMap[secretUUID], err = secrets.GetPayload(ctx, c.keyManager, secretUUID, nil).Extract()
 		if err != nil {
 			return nil, fmt.Errorf(errClientGetSecretPayload, fmt.Errorf("failed to get secret payload for secret %s: %w", secretUUID, err))
 		}
 	}
+
+	if len(secretsMap) == 0 {
+		return nil, fmt.Errorf(errClientGeneric, errors.New("no secrets found"))
+	}
+
 	return secretsMap, nil
 }
 

+ 69 - 0
providers/v1/barbican/client_test.go

@@ -18,9 +18,14 @@ package barbican
 
 import (
 	"context"
+	"encoding/json"
+	"fmt"
+	"net/http"
 	"testing"
 
 	"github.com/gophercloud/gophercloud/v2"
+	th "github.com/gophercloud/gophercloud/v2/testhelper"
+	thclient "github.com/gophercloud/gophercloud/v2/testhelper/client"
 	"github.com/stretchr/testify/assert"
 
 	esv1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1"
@@ -207,3 +212,67 @@ func TestGetAllSecretsValidation(t *testing.T) {
 		})
 	}
 }
+
+func TestGetAllSecretsRegexpMatch(t *testing.T) {
+	type fakeSecret struct {
+		name    string
+		uuid    string
+		payload string
+	}
+	all := []fakeSecret{
+		{name: "db-a", uuid: "11111111-1111-1111-1111-111111111111", payload: "payload-db-a"},
+		{name: "db-b", uuid: "22222222-2222-2222-2222-222222222222", payload: "payload-db-b"},
+		{name: "web-a", uuid: "33333333-3333-3333-3333-333333333333", payload: "payload-web-a"},
+	}
+
+	fakeServer := th.SetupHTTP()
+	defer fakeServer.Teardown()
+
+	// Barbican's list endpoint only does exact-name matching, so mirror that
+	// here: honor the ?name= query with a literal comparison, like the real
+	// service does. A regexp value therefore matches nothing server-side, which
+	// is exactly the reported bug.
+	fakeServer.Mux.HandleFunc("/secrets", func(w http.ResponseWriter, r *http.Request) {
+		nameFilter := r.URL.Query().Get("name")
+		type listed struct {
+			Name      string `json:"name"`
+			SecretRef string `json:"secret_ref"`
+		}
+		var out []listed
+		for _, s := range all {
+			if nameFilter != "" && s.name != nameFilter {
+				continue
+			}
+			out = append(out, listed{
+				Name:      s.name,
+				SecretRef: fmt.Sprintf("http://barbican.example.com/v1/secrets/%s", s.uuid),
+			})
+		}
+		body, _ := json.Marshal(map[string]any{"secrets": out, "total": len(out)})
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write(body)
+	})
+
+	for _, s := range all {
+		payload := s.payload
+		fakeServer.Mux.HandleFunc("/secrets/"+s.uuid+"/payload", func(w http.ResponseWriter, _ *http.Request) {
+			w.Header().Set("Content-Type", "text/plain")
+			w.WriteHeader(http.StatusOK)
+			_, _ = w.Write([]byte(payload))
+		})
+	}
+
+	client := &Client{keyManager: thclient.ServiceClient(fakeServer)}
+
+	result, err := client.GetAllSecrets(context.Background(), esv1.ExternalSecretFind{
+		Name: &esv1.FindName{RegExp: "^db-"},
+	})
+	assert.NoError(t, err)
+	// Only the db-* secrets should match the pattern, not web-a and not just a
+	// literal secret named "^db-".
+	assert.Len(t, result, 2)
+	assert.Equal(t, []byte("payload-db-a"), result["11111111-1111-1111-1111-111111111111"])
+	assert.Equal(t, []byte("payload-db-b"), result["22222222-2222-2222-2222-222222222222"])
+	assert.NotContains(t, result, "33333333-3333-3333-3333-333333333333")
+}