Browse Source

fix: use a separate parameter for GET calls in VaultDynamicSecrets (#6275)

Gergely Bräutigam 1 month ago
parent
commit
d225e48c6f

+ 7 - 1
apis/generators/v1alpha1/types_vault.go

@@ -33,9 +33,15 @@ type VaultDynamicSecretSpec struct {
 	// Vault API method to use (GET/POST/other)
 	Method string `json:"method,omitempty"`
 
-	// Parameters to pass to Vault for write and Get calls. GET calls only support string value types.
+	// Parameters to pass to Vault write (for non-GET methods)
 	Parameters *apiextensions.JSON `json:"parameters,omitempty"`
 
+	// GetParameters are query-string parameters passed to Vault on GET calls.
+	// Each key may map to multiple values, matching HTTP query-string semantics.
+	// Ignored for non-GET methods; use Parameters for write bodies.
+	// +optional
+	GetParameters map[string][]string `json:"getParameters,omitempty"`
+
 	// Result type defines which data is returned from the generator.
 	// By default, it is the "data" section of the Vault API response.
 	// When using e.g. /auth/token/create the "data" section is empty but

+ 16 - 0
apis/generators/v1alpha1/zz_generated.deepcopy.go

@@ -1860,6 +1860,22 @@ func (in *VaultDynamicSecretSpec) DeepCopyInto(out *VaultDynamicSecretSpec) {
 		*out = new(v1.JSON)
 		(*in).DeepCopyInto(*out)
 	}
+	if in.GetParameters != nil {
+		in, out := &in.GetParameters, &out.GetParameters
+		*out = make(map[string][]string, len(*in))
+		for key, val := range *in {
+			var outVal []string
+			if val == nil {
+				(*out)[key] = nil
+			} else {
+				inVal := (*in)[key]
+				in, out := &inVal, &outVal
+				*out = make([]string, len(*in))
+				copy(*out, *in)
+			}
+			(*out)[key] = outVal
+		}
+	}
 	if in.RetrySettings != nil {
 		in, out := &in.RetrySettings, &out.RetrySettings
 		*out = new(externalsecretsv1.SecretStoreRetrySettings)

+ 12 - 2
config/crds/bases/generators.external-secrets.io_clustergenerators.yaml

@@ -1174,12 +1174,22 @@ spec:
                           Used to select the correct ESO controller (think: ingress.ingressClassName)
                           The ESO controller is instantiated with a specific controller name and filters VDS based on this property
                         type: string
+                      getParameters:
+                        additionalProperties:
+                          items:
+                            type: string
+                          type: array
+                        description: |-
+                          GetParameters are query-string parameters passed to Vault on GET calls.
+                          Each key may map to multiple values, matching HTTP query-string semantics.
+                          Ignored for non-GET methods; use Parameters for write bodies.
+                        type: object
                       method:
                         description: Vault API method to use (GET/POST/other)
                         type: string
                       parameters:
-                        description: Parameters to pass to Vault for write and Get
-                          calls. GET calls only support string value types.
+                        description: Parameters to pass to Vault write (for non-GET
+                          methods)
                         x-kubernetes-preserve-unknown-fields: true
                       path:
                         description: Vault path to obtain the dynamic secret from

+ 11 - 2
config/crds/bases/generators.external-secrets.io_vaultdynamicsecrets.yaml

@@ -54,12 +54,21 @@ spec:
                   Used to select the correct ESO controller (think: ingress.ingressClassName)
                   The ESO controller is instantiated with a specific controller name and filters VDS based on this property
                 type: string
+              getParameters:
+                additionalProperties:
+                  items:
+                    type: string
+                  type: array
+                description: |-
+                  GetParameters are query-string parameters passed to Vault on GET calls.
+                  Each key may map to multiple values, matching HTTP query-string semantics.
+                  Ignored for non-GET methods; use Parameters for write bodies.
+                type: object
               method:
                 description: Vault API method to use (GET/POST/other)
                 type: string
               parameters:
-                description: Parameters to pass to Vault for write and Get calls.
-                  GET calls only support string value types.
+                description: Parameters to pass to Vault write (for non-GET methods)
                 x-kubernetes-preserve-unknown-fields: true
               path:
                 description: Vault path to obtain the dynamic secret from

+ 22 - 2
deploy/crds/bundle.yaml

@@ -25891,11 +25891,21 @@ spec:
                             Used to select the correct ESO controller (think: ingress.ingressClassName)
                             The ESO controller is instantiated with a specific controller name and filters VDS based on this property
                           type: string
+                        getParameters:
+                          additionalProperties:
+                            items:
+                              type: string
+                            type: array
+                          description: |-
+                            GetParameters are query-string parameters passed to Vault on GET calls.
+                            Each key may map to multiple values, matching HTTP query-string semantics.
+                            Ignored for non-GET methods; use Parameters for write bodies.
+                          type: object
                         method:
                           description: Vault API method to use (GET/POST/other)
                           type: string
                         parameters:
-                          description: Parameters to pass to Vault for write and Get calls. GET calls only support string value types.
+                          description: Parameters to pass to Vault write (for non-GET methods)
                           x-kubernetes-preserve-unknown-fields: true
                         path:
                           description: Vault path to obtain the dynamic secret from
@@ -28581,11 +28591,21 @@ spec:
                     Used to select the correct ESO controller (think: ingress.ingressClassName)
                     The ESO controller is instantiated with a specific controller name and filters VDS based on this property
                   type: string
+                getParameters:
+                  additionalProperties:
+                    items:
+                      type: string
+                    type: array
+                  description: |-
+                    GetParameters are query-string parameters passed to Vault on GET calls.
+                    Each key may map to multiple values, matching HTTP query-string semantics.
+                    Ignored for non-GET methods; use Parameters for write bodies.
+                  type: object
                 method:
                   description: Vault API method to use (GET/POST/other)
                   type: string
                 parameters:
-                  description: Parameters to pass to Vault for write and Get calls. GET calls only support string value types.
+                  description: Parameters to pass to Vault write (for non-GET methods)
                   x-kubernetes-preserve-unknown-fields: true
                 path:
                   description: Vault path to obtain the dynamic secret from

+ 16 - 0
docs/api/generator/vault.md

@@ -14,12 +14,28 @@ are stored into the resulting Secret in JSON format. The generator exposes `data
 section of the response from Vault API by default. To adjust the behaviour, use
 `resultType` key.
 
+### Passing parameters
+
+- `parameters` is a JSON body sent on write methods (POST, PUT, etc.) and
+  supports arbitrary nested JSON. It is **ignored** on `GET` and `LIST`.
+- `getParameters` is a `map[string][]string` sent as the query string on `GET`
+  calls. Each key may map to multiple values, matching HTTP query-string
+  semantics. It is ignored for non-GET methods.
+
 ## Example manifest
 
+Write method (POST) with a JSON body:
+
 ```yaml
 {% include 'generator-vault.yaml' %}
 ```
 
+GET method with query-string parameters:
+
+```yaml
+{% include 'generator-vault-get.yaml' %}
+```
+
 Example `ExternalSecret` that references the Vault generator:
 ```yaml
 {% include 'generator-vault-example.yaml' %}

+ 30 - 2
docs/api/spec.md

@@ -28548,7 +28548,21 @@ k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.JSON
 </em>
 </td>
 <td>
-<p>Parameters to pass to Vault for write and Get calls. GET calls only support string value types.</p>
+<p>Parameters to pass to Vault write (for non-GET methods)</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>getParameters</code></br>
+<em>
+map[string][]string
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>GetParameters are query-string parameters passed to Vault on GET calls.
+Each key may map to multiple values, matching HTTP query-string semantics.
+Ignored for non-GET methods; use Parameters for write bodies.</p>
 </td>
 </tr>
 <tr>
@@ -28701,7 +28715,21 @@ k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1.JSON
 </em>
 </td>
 <td>
-<p>Parameters to pass to Vault for write and Get calls. GET calls only support string value types.</p>
+<p>Parameters to pass to Vault write (for non-GET methods)</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>getParameters</code></br>
+<em>
+map[string][]string
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>GetParameters are query-string parameters passed to Vault on GET calls.
+Each key may map to multiple values, matching HTTP query-string semantics.
+Ignored for non-GET methods; use Parameters for write bodies.</p>
 </td>
 </tr>
 <tr>

+ 1 - 15
generators/v1/vault/vault.go

@@ -116,21 +116,7 @@ func (g *Generator) fetchVaultSecret(ctx context.Context, res *genv1alpha1.Vault
 	)
 
 	if res.Spec.Method == "" || res.Spec.Method == "GET" {
-		var raw map[string]any
-		if res.Spec.Parameters != nil {
-			if err := json.Unmarshal(res.Spec.Parameters.Raw, &raw); err != nil {
-				return nil, fmt.Errorf("failed to parse parameters for GET call: %w", err)
-			}
-		}
-		params := make(map[string][]string, len(raw))
-		for k, v := range raw {
-			s, ok := v.(string)
-			if !ok {
-				return nil, fmt.Errorf("unsupported type for GET parameter %q: %T", k, v)
-			}
-			params[k] = []string{s}
-		}
-		result, err = cl.Logical().ReadWithDataWithContext(ctx, res.Spec.Path, params)
+		result, err = cl.Logical().ReadWithDataWithContext(ctx, res.Spec.Path, res.Spec.GetParameters)
 	} else if res.Spec.Method == "LIST" {
 		result, err = cl.Logical().ListWithContext(ctx, res.Spec.Path)
 	} else if res.Spec.Method == "DELETE" {

+ 56 - 20
generators/v1/vault/vault_test.go

@@ -418,8 +418,17 @@ func TestVaultDynamicSecretGetParameters(t *testing.T) {
 		ObjectMeta: metav1.ObjectMeta{Name: "testing", Namespace: "testing"},
 		Secrets:    []corev1.ObjectReference{{Name: "test"}},
 	}
-	spec := func(params string) *apiextensions.JSON {
-		return &apiextensions.JSON{Raw: []byte(`apiVersion: generators.external-secrets.io/v1alpha1
+
+	t.Run("ForwardsGetParameters", func(t *testing.T) {
+		var got map[string][]string
+		clientFn := fake.ModifiableClientWithLoginMock(func(cl *fake.VaultClient) {
+			cl.MockLogical.ReadWithDataWithContextFn = func(_ context.Context, _ string, data map[string][]string) (*vaultapi.Secret, error) {
+				got = data
+				return &vaultapi.Secret{Data: map[string]any{"key": "value"}}, nil
+			}
+		})
+		c := &provider.Provider{NewVaultClient: clientFn}
+		spec := &apiextensions.JSON{Raw: []byte(`apiVersion: generators.external-secrets.io/v1alpha1
 kind: VaultDynamicSecret
 spec:
   provider:
@@ -429,40 +438,67 @@ spec:
         serviceAccountRef:
           name: "testing"
   method: GET
-  parameters:
-    ` + params + `
+  getParameters:
+    scope:
+      - "applied-permissions/user"
+    tag:
+      - "prod"
+      - "blue"
   path: "github/token/example"`)}
-	}
+		_, _, err := (&Generator{}).generate(context.Background(),
+			c, spec,
+			clientfake.NewClientBuilder().WithObjects(sa).Build(),
+			utilfake.NewCreateTokenMock().WithToken("ok"), "testing")
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		want := map[string][]string{
+			"scope": {"applied-permissions/user"},
+			"tag":   {"prod", "blue"},
+		}
+		if diff := cmp.Diff(want, got); diff != "" {
+			t.Errorf("forwarded params mismatch:\n%s", diff)
+		}
+	})
 
-	t.Run("ForwardsStringParams", func(t *testing.T) {
+	t.Run("IgnoresParametersOnGET", func(t *testing.T) {
 		var got map[string][]string
+		called := false
 		clientFn := fake.ModifiableClientWithLoginMock(func(cl *fake.VaultClient) {
 			cl.MockLogical.ReadWithDataWithContextFn = func(_ context.Context, _ string, data map[string][]string) (*vaultapi.Secret, error) {
+				called = true
 				got = data
 				return &vaultapi.Secret{Data: map[string]any{"key": "value"}}, nil
 			}
 		})
 		c := &provider.Provider{NewVaultClient: clientFn}
+		spec := &apiextensions.JSON{Raw: []byte(`apiVersion: generators.external-secrets.io/v1alpha1
+kind: VaultDynamicSecret
+spec:
+  provider:
+    auth:
+      kubernetes:
+        role: test
+        serviceAccountRef:
+          name: "testing"
+  method: GET
+  parameters:
+    ttl: 60
+    nested:
+      key: "value"
+  path: "github/token/example"`)}
 		_, _, err := (&Generator{}).generate(context.Background(),
-			c, spec(`scope: "applied-permissions/user"`),
+			c, spec,
 			clientfake.NewClientBuilder().WithObjects(sa).Build(),
 			utilfake.NewCreateTokenMock().WithToken("ok"), "testing")
 		if err != nil {
-			t.Fatalf("unexpected error: %v", err)
+			t.Fatalf("Parameters on GET should be ignored, got error: %v", err)
 		}
-		if diff := cmp.Diff(map[string][]string{"scope": {"applied-permissions/user"}}, got); diff != "" {
-			t.Errorf("forwarded params mismatch:\n%s", diff)
+		if !called {
+			t.Fatal("expected ReadWithDataWithContext to be called")
 		}
-	})
-
-	t.Run("RejectsNonStringParams", func(t *testing.T) {
-		c := &provider.Provider{NewVaultClient: fake.ClientWithLoginMock}
-		_, _, err := (&Generator{}).generate(context.Background(),
-			c, spec(`ttl: 60`),
-			clientfake.NewClientBuilder().WithObjects(sa).Build(),
-			utilfake.NewCreateTokenMock().WithToken("ok"), "testing")
-		if err == nil || err.Error() != `unsupported type for GET parameter "ttl": float64` {
-			t.Errorf("want unsupported-type error, got: %v", err)
+		if got != nil {
+			t.Errorf("expected nil params on GET when only Parameters is set, got: %v", got)
 		}
 	})
 }

+ 1 - 0
tests/__snapshot__/clustergenerator-v1alpha1.yaml

@@ -177,6 +177,7 @@ spec:
     vaultDynamicSecretSpec:
       allowEmptyResponse: false
       controller: string
+      getParameters: {}
       method: string
       parameters: 
       path: string

+ 1 - 0
tests/__snapshot__/vaultdynamicsecret-v1alpha1.yaml

@@ -4,6 +4,7 @@ metadata: {}
 spec:
   allowEmptyResponse: false
   controller: string
+  getParameters: {}
   method: string
   parameters: 
   path: string