Browse Source

Support glob for namespaces condition in ClusterSecretStore (#2920)

* feat(ClusterSecretStore): Support glob for conditions.namespaces

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* feat(ClusterSecretStore): Fix diff

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* feat(ClusterSecretStore): Fix code smell

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* feat(ClusterSecretStore): First code review

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* feat(ClusterSecretStore): Second code review

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* feat(ClusterSecretStore): Generate

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* feat(ClusterSecretStore): Fix Sonar method complexity

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>

* addressed comments

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

* renamed namedspacesregexes because it sounded funny

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

---------

Signed-off-by: gmuselli <geoffrey.muselli@gmail.com>
Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Geoffrey MUSELLI 1 year ago
parent
commit
f74e08546c

+ 3 - 0
apis/externalsecrets/v1beta1/provider_schema.go

@@ -73,6 +73,9 @@ func GetProvider(s GenericStore) (Provider, error) {
 	}
 	spec := s.GetSpec()
 	if spec == nil {
+		// Note, this condition can never be reached, because
+		// the Spec is not a pointer in Kubernetes. It will
+		// always exist.
 		return nil, fmt.Errorf("no spec found in %#v", s)
 	}
 	storeName, err := getProviderName(spec.Provider)

+ 5 - 0
apis/externalsecrets/v1beta1/secretstore_types.go

@@ -50,7 +50,12 @@ type ClusterSecretStoreCondition struct {
 	NamespaceSelector *metav1.LabelSelector `json:"namespaceSelector,omitempty"`
 
 	// Choose namespaces by name
+	// +optional
 	Namespaces []string `json:"namespaces,omitempty"`
+
+	// Choose namespaces by using regex matching
+	// +optional
+	NamespaceRegexes []string `json:"namespaceRegexes,omitempty"`
 }
 
 // SecretStoreProvider contains the provider-specific configuration.

+ 20 - 0
apis/externalsecrets/v1beta1/secretstore_validator.go

@@ -16,7 +16,9 @@ package v1beta1
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"regexp"
 
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -54,9 +56,27 @@ func (r *GenericStoreValidator) ValidateDelete(_ context.Context, _ runtime.Obje
 }
 
 func validateStore(store GenericStore) (admission.Warnings, error) {
+	if err := validateConditions(store); err != nil {
+		return nil, err
+	}
+
 	provider, err := GetProvider(store)
 	if err != nil {
 		return nil, err
 	}
+
 	return provider.ValidateStore(store)
 }
+
+func validateConditions(store GenericStore) error {
+	var errs error
+	for ci, condition := range store.GetSpec().Conditions {
+		for ri, r := range condition.NamespaceRegexes {
+			if _, err := regexp.Compile(r); err != nil {
+				errs = errors.Join(errs, fmt.Errorf("failed to compile %dth namespace regex in %dth condition: %w", ri, ci, err))
+			}
+		}
+	}
+
+	return errs
+}

+ 150 - 0
apis/externalsecrets/v1beta1/secretstore_validator_test.go

@@ -0,0 +1,150 @@
+/*
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1beta1
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+)
+
+// ValidationProvider is a simple provider that we can use without cyclic import.
+type ValidationProvider struct {
+	Provider
+}
+
+func (v *ValidationProvider) ValidateStore(_ GenericStore) (admission.Warnings, error) {
+	return nil, nil
+}
+
+func TestValidateSecretStore(t *testing.T) {
+	tests := []struct {
+		name      string
+		obj       *SecretStore
+		mock      func()
+		assertErr func(t *testing.T, err error)
+	}{
+		{
+			name: "valid regex",
+			obj: &SecretStore{
+				Spec: SecretStoreSpec{
+					Conditions: []ClusterSecretStoreCondition{
+						{
+							NamespaceRegexes: []string{`.*`},
+						},
+					},
+					Provider: &SecretStoreProvider{
+						AWS: &AWSProvider{},
+					},
+				},
+			},
+			mock: func() {
+				ForceRegister(&ValidationProvider{}, &SecretStoreProvider{
+					AWS: &AWSProvider{},
+				})
+			},
+			assertErr: func(t *testing.T, err error) {
+				require.NoError(t, err)
+			},
+		},
+		{
+			name: "invalid regex",
+			obj: &SecretStore{
+				Spec: SecretStoreSpec{
+					Conditions: []ClusterSecretStoreCondition{
+						{
+							NamespaceRegexes: []string{`\1`},
+						},
+					},
+					Provider: &SecretStoreProvider{
+						AWS: &AWSProvider{},
+					},
+				},
+			},
+			mock: func() {
+				ForceRegister(&ValidationProvider{}, &SecretStoreProvider{
+					AWS: &AWSProvider{},
+				})
+			},
+			assertErr: func(t *testing.T, err error) {
+				assert.EqualError(t, err, "failed to compile 0th namespace regex in 0th condition: error parsing regexp: invalid escape sequence: `\\1`")
+			},
+		},
+		{
+			name: "multiple errors",
+			obj: &SecretStore{
+				Spec: SecretStoreSpec{
+					Conditions: []ClusterSecretStoreCondition{
+						{
+							NamespaceRegexes: []string{`\1`, `\2`},
+						},
+					},
+					Provider: &SecretStoreProvider{
+						AWS: &AWSProvider{},
+					},
+				},
+			},
+			mock: func() {
+				ForceRegister(&ValidationProvider{}, &SecretStoreProvider{
+					AWS: &AWSProvider{},
+				})
+			},
+			assertErr: func(t *testing.T, err error) {
+				assert.EqualError(t, err, "failed to compile 0th namespace regex in 0th condition: error parsing regexp: invalid escape sequence: `\\1`\nfailed to compile 1th namespace regex in 0th condition: error parsing regexp: invalid escape sequence: `\\2`")
+			},
+		},
+		{
+			name: "secret store must have only a single backend",
+			obj: &SecretStore{
+				Spec: SecretStoreSpec{
+					Provider: &SecretStoreProvider{
+						AWS:   &AWSProvider{},
+						GCPSM: &GCPSMProvider{},
+					},
+				},
+			},
+			assertErr: func(t *testing.T, err error) {
+				assert.EqualError(t, err, "store error for : secret stores must only have exactly one backend specified, found 2")
+			},
+		},
+		{
+			name: "no registered store backend",
+			obj: &SecretStore{
+				Spec: SecretStoreSpec{
+					Conditions: []ClusterSecretStoreCondition{
+						{
+							Namespaces: []string{"default"},
+						},
+					},
+				},
+			},
+			assertErr: func(t *testing.T, err error) {
+				assert.EqualError(t, err, "store error for : secret stores must only have exactly one backend specified, found 0")
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.mock != nil {
+				tt.mock()
+			}
+
+			_, err := validateStore(tt.obj)
+			tt.assertErr(t, err)
+		})
+	}
+}

+ 5 - 0
apis/externalsecrets/v1beta1/zz_generated.deepcopy.go

@@ -682,6 +682,11 @@ func (in *ClusterSecretStoreCondition) DeepCopyInto(out *ClusterSecretStoreCondi
 		*out = make([]string, len(*in))
 		copy(*out, *in)
 	}
+	if in.NamespaceRegexes != nil {
+		in, out := &in.NamespaceRegexes, &out.NamespaceRegexes
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 }
 
 // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ClusterSecretStoreCondition.

+ 5 - 0
config/crds/bases/external-secrets.io_clustersecretstores.yaml

@@ -1646,6 +1646,11 @@ spec:
                     ClusterSecretStoreCondition describes a condition by which to choose namespaces to process ExternalSecrets in
                     for a ClusterSecretStore instance.
                   properties:
+                    namespaceRegexes:
+                      description: Choose namespaces by using regex matching
+                      items:
+                        type: string
+                      type: array
                     namespaceSelector:
                       description: Choose namespace using a labelSelector
                       properties:

+ 5 - 0
config/crds/bases/external-secrets.io_secretstores.yaml

@@ -1646,6 +1646,11 @@ spec:
                     ClusterSecretStoreCondition describes a condition by which to choose namespaces to process ExternalSecrets in
                     for a ClusterSecretStore instance.
                   properties:
+                    namespaceRegexes:
+                      description: Choose namespaces by using regex matching
+                      items:
+                        type: string
+                      type: array
                     namespaceSelector:
                       description: Choose namespace using a labelSelector
                       properties:

+ 10 - 0
deploy/crds/bundle.yaml

@@ -2203,6 +2203,11 @@ spec:
                       ClusterSecretStoreCondition describes a condition by which to choose namespaces to process ExternalSecrets in
                       for a ClusterSecretStore instance.
                     properties:
+                      namespaceRegexes:
+                        description: Choose namespaces by using regex matching
+                        items:
+                          type: string
+                        type: array
                       namespaceSelector:
                         description: Choose namespace using a labelSelector
                         properties:
@@ -7689,6 +7694,11 @@ spec:
                       ClusterSecretStoreCondition describes a condition by which to choose namespaces to process ExternalSecrets in
                       for a ClusterSecretStore instance.
                     properties:
+                      namespaceRegexes:
+                        description: Choose namespaces by using regex matching
+                        items:
+                          type: string
+                        type: array
                       namespaceSelector:
                         description: Choose namespace using a labelSelector
                         properties:

+ 13 - 0
docs/api/spec.md

@@ -1861,9 +1861,22 @@ Kubernetes meta/v1.LabelSelector
 </em>
 </td>
 <td>
+<em>(Optional)</em>
 <p>Choose namespaces by name</p>
 </td>
 </tr>
+<tr>
+<td>
+<code>namespaceRegexes</code></br>
+<em>
+[]string
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>Choose namespaces by using regex matching</p>
+</td>
+</tr>
 </tbody>
 </table>
 <h3 id="external-secrets.io/v1beta1.ConjurAPIKey">ConjurAPIKey

+ 6 - 1
docs/snippets/full-cluster-secret-store.yaml

@@ -142,7 +142,7 @@ spec:
 
   # Conditions about namespaces in which the ClusterSecretStore is usable for ExternalSecrets
   conditions:
-    # Options are namespaceSelector, or namespaces
+    # Options are namespaceSelector, namespaces or namespacesRegex
     - namespaceSelector:
         matchLabels:
           my.namespace.io/some-label: "value" # Only namespaces with that label will work
@@ -151,6 +151,11 @@ spec:
         - "namespace-a"
         - "namespace-b"
 
+    # Namespace regex is helpful for namespace naming convention or when an external tool auto generate namespaces with prefix
+    - namespacesRegex:
+        - "namespace-a-.*" # All namespaces prefixed by namespace-a- will work
+        - "namespace-b-.*" # All namespaces prefixed by namespace-b- will work
+
     # conditions needs only one of the conditions to meet for the CSS to be usable in the namespace.
 
 status:

+ 13 - 0
pkg/controllers/secretstore/client_manager.go

@@ -17,6 +17,7 @@ package secretstore
 import (
 	"context"
 	"fmt"
+	"regexp"
 	"strings"
 
 	"github.com/go-logr/logr"
@@ -245,6 +246,18 @@ func (m *Manager) shouldProcessSecret(store esv1beta1.GenericStore, ns string) (
 				return true, nil
 			}
 		}
+
+		for _, reg := range condition.NamespaceRegexes {
+			match, err := regexp.MatchString(reg, ns)
+			if err != nil {
+				// Should not happen since store validation already verified the regexes.
+				return false, fmt.Errorf("failed to compile regex %v: %w", reg, err)
+			}
+
+			if match {
+				return true, nil
+			}
+		}
 	}
 
 	return false, nil

+ 90 - 0
pkg/controllers/secretstore/client_manager_test.go

@@ -310,6 +310,96 @@ func TestManagerGet(t *testing.T) {
 	}
 }
 
+func TestShouldProcessSecret(t *testing.T) {
+	scheme := runtime.NewScheme()
+	_ = clientgoscheme.AddToScheme(scheme)
+	_ = esv1beta1.AddToScheme(scheme)
+	_ = apiextensionsv1.AddToScheme(scheme)
+
+	testNamespace := "test-a"
+	testCases := []struct {
+		name       string
+		conditions []esv1beta1.ClusterSecretStoreCondition
+		namespace  *corev1.Namespace
+		wantErr    string
+		want       bool
+	}{
+		{
+			name: "processes a regex condition",
+			conditions: []esv1beta1.ClusterSecretStoreCondition{
+				{
+					NamespaceRegexes: []string{`test-*`},
+				},
+			},
+			namespace: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: testNamespace,
+				},
+			},
+			want: true,
+		},
+		{
+			name: "process multiple regexes",
+			conditions: []esv1beta1.ClusterSecretStoreCondition{
+				{
+					NamespaceRegexes: []string{`nope`, `test-*`},
+				},
+			},
+			namespace: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: testNamespace,
+				},
+			},
+			want: true,
+		},
+		{
+			name: "shouldn't process if nothing matches",
+			conditions: []esv1beta1.ClusterSecretStoreCondition{
+				{
+					NamespaceRegexes: []string{`nope`},
+				},
+			},
+			namespace: &corev1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: testNamespace,
+				},
+			},
+			want: false,
+		},
+	}
+
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			fakeSpec := esv1beta1.SecretStoreSpec{
+				Conditions: tt.conditions,
+			}
+
+			defaultStore := &esv1beta1.ClusterSecretStore{
+				TypeMeta: metav1.TypeMeta{Kind: esv1beta1.ClusterSecretStoreKind},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "foo",
+					Namespace: tt.namespace.Name,
+				},
+				Spec: fakeSpec,
+			}
+
+			client := fakeclient.NewClientBuilder().WithScheme(scheme).WithObjects(defaultStore, tt.namespace).Build()
+			clientMap := make(map[clientKey]*clientVal)
+			mgr := &Manager{
+				log:             logr.Discard(),
+				client:          client,
+				enableFloodgate: true,
+				clientMap:       clientMap,
+			}
+
+			got, err := mgr.shouldProcessSecret(defaultStore, tt.namespace.Name)
+			require.NoError(t, err)
+
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
 type WrapProvider struct {
 	newClientFunc func(
 		context.Context,

+ 1 - 1
pkg/controllers/secretstore/common_test.go

@@ -44,7 +44,7 @@ var _ = Describe("SecretStore reconcile", func() {
 		Expect(k8sClient.Delete(context.Background(), test.store)).ToNot(HaveOccurred())
 	})
 
-	// a invalid provider config should be reflected
+	// an invalid provider config should be reflected
 	// in the store status condition
 	invalidProvider := func(tc *testCase) {
 		tc.assert = func() {