feat: add controller class on VaultDynamicSecret resources (#2287)

* feat: add generator for vaultdynamicsecret

* Added controllerClass on VaultDynamicSecret

* Added controllerClass on VaultDynamicSecret

Signed-off-by: rdeepc <12953177+rdeepc@users.noreply.github.com>

* Fixed lint

Signed-off-by: rdeepc <12953177+rdeepc@users.noreply.github.com>

* Fixed hack bash

Signed-off-by: rdeepc <12953177+rdeepc@users.noreply.github.com>

* feat: Implemented generator controller class support

- Controller class support in VaultDynamicSecret
- Controller class support in Fake

Signed-off-by: rdeepc <12953177+rdeepc@users.noreply.github.com>

* feat: Implemented Generator controller class check

Signed-off-by: rdeepc <12953177+rdeepc@users.noreply.github.com>

* feat: Implemented Generator controller class check

Signed-off-by: rdeepc <dpr0413@gmail.com>

* feat: Implemented Generator controller class check

Signed-off-by: rdeepc <dpr0413@gmail.com>

* feat: hoist controller class check to the top

The generator controller class check should be at the very top of the
reconcile function just like the other secretStore class check.

Otherwise we would return an error and as a result set the status field on the es
resource - which is undesirable. The controller should completely
ignore the resource instead.

Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>

---------

Signed-off-by: rdeepc <12953177+rdeepc@users.noreply.github.com>
Signed-off-by: rdeepc <dpr0413@gmail.com>
Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>
Co-authored-by: Frederic Mereu <frederic.mereu@gaming1.com>
Co-authored-by: Moritz Johner <beller.moritz@googlemail.com>

apis/externalsecrets/v1alpha1/secretstore_types.go

@@ -21,8 +21,8 @@ import (
 
 // SecretStoreSpec defines the desired state of SecretStore.
 type SecretStoreSpec struct {
-	// Used to select the correct KES controller (think: ingress.ingressClassName)
-	// The KES controller is instantiated with a specific controller name and filters ES based on this property
+	// Used to select the correct ESO controller (think: ingress.ingressClassName)
+	// The ESO controller is instantiated with a specific controller name and filters ES based on this property
 	// +optional
 	Controller string `json:"controller"`
 

apis/externalsecrets/v1beta1/secretstore_types.go

@@ -21,8 +21,8 @@ import (
 
 // SecretStoreSpec defines the desired state of SecretStore.
 type SecretStoreSpec struct {
-	// Used to select the correct KES controller (think: ingress.ingressClassName)
-	// The KES controller is instantiated with a specific controller name and filters ES based on this property
+	// Used to select the correct ESO controller (think: ingress.ingressClassName)
+	// The ESO controller is instantiated with a specific controller name and filters ES based on this property
 	// +optional
 	Controller string `json:"controller"`
 

apis/generators/v1alpha1/generator_fake.go

@@ -20,6 +20,11 @@ import (
 
 // FakeSpec contains the static data.
 type FakeSpec struct {
+	// 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
+	// +optional
+	Controller string `json:"controller"`
+
 	// Data defines the static data returned
 	// by this generator.
 	Data map[string]string `json:"data,omitempty"`

apis/generators/v1alpha1/generator_types.go

@@ -0,0 +1,21 @@
+/*
+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 v1alpha1
+
+type ControllerClassResource struct {
+	Spec struct {
+		ControllerClass string `json:"controller"`
+	} `json:"spec"`
+}

apis/generators/v1alpha1/generator_vault.go

@@ -22,6 +22,11 @@ import (
 )
 
 type VaultDynamicSecretSpec struct {
+	// 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
+	// +optional
+	Controller string `json:"controller"`
+
 	// Vault API method to use (GET/POST/other)
 	Method string `json:"method,omitempty"`
 

apis/generators/v1alpha1/zz_generated.deepcopy.go

@@ -266,6 +266,22 @@ func (in *AzureACRWorkloadIdentityAuth) DeepCopy() *AzureACRWorkloadIdentityAuth
 }
 
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
+func (in *ControllerClassResource) DeepCopyInto(out *ControllerClassResource) {
+	*out = *in
+	out.Spec = in.Spec
+}
+
+// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ControllerClassResource.
+func (in *ControllerClassResource) DeepCopy() *ControllerClassResource {
+	if in == nil {
+		return nil
+	}
+	out := new(ControllerClassResource)
+	in.DeepCopyInto(out)
+	return out
+}
+
+// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *ECRAuthorizationToken) DeepCopyInto(out *ECRAuthorizationToken) {
 	*out = *in
 	out.TypeMeta = in.TypeMeta

config/crds/bases/external-secrets.io_clustersecretstores.yaml

@@ -47,8 +47,8 @@ spec:
             description: SecretStoreSpec defines the desired state of SecretStore.
             properties:
               controller:
-                description: 'Used to select the correct KES controller (think: ingress.ingressClassName)
-                  The KES controller is instantiated with a specific controller name
+                description: 'Used to select the correct ESO controller (think: ingress.ingressClassName)
+                  The ESO controller is instantiated with a specific controller name
                   and filters ES based on this property'
                 type: string
               provider:
@@ -1645,8 +1645,8 @@ spec:
                   type: object
                 type: array
               controller:
-                description: 'Used to select the correct KES controller (think: ingress.ingressClassName)
-                  The KES controller is instantiated with a specific controller name
+                description: 'Used to select the correct ESO controller (think: ingress.ingressClassName)
+                  The ESO controller is instantiated with a specific controller name
                   and filters ES based on this property'
                 type: string
               provider:

config/crds/bases/external-secrets.io_secretstores.yaml

@@ -47,8 +47,8 @@ spec:
             description: SecretStoreSpec defines the desired state of SecretStore.
             properties:
               controller:
-                description: 'Used to select the correct KES controller (think: ingress.ingressClassName)
-                  The KES controller is instantiated with a specific controller name
+                description: 'Used to select the correct ESO controller (think: ingress.ingressClassName)
+                  The ESO controller is instantiated with a specific controller name
                   and filters ES based on this property'
                 type: string
               provider:
@@ -1645,8 +1645,8 @@ spec:
                   type: object
                 type: array
               controller:
-                description: 'Used to select the correct KES controller (think: ingress.ingressClassName)
-                  The KES controller is instantiated with a specific controller name
+                description: 'Used to select the correct ESO controller (think: ingress.ingressClassName)
+                  The ESO controller is instantiated with a specific controller name
                   and filters ES based on this property'
                 type: string
               provider:

config/crds/bases/generators.external-secrets.io_fakes.yaml

@@ -38,6 +38,11 @@ spec:
           spec:
             description: FakeSpec contains the static data.
             properties:
+              controller:
+                description: '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
               data:
                 additionalProperties:
                   type: string

config/crds/bases/generators.external-secrets.io_vaultdynamicsecrets.yaml

@@ -35,6 +35,11 @@ spec:
             type: object
           spec:
             properties:
+              controller:
+                description: '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
               method:
                 description: Vault API method to use (GET/POST/other)
                 type: string

deploy/charts/external-secrets/tests/__snapshot__/crds_test.yaml.snap

@@ -54,7 +54,7 @@ should match snapshot of default values:
                   description: SecretStoreSpec defines the desired state of SecretStore.
                   properties:
                     controller:
-                      description: 'Used to select the correct KES controller (think: ingress.ingressClassName) The KES controller is instantiated with a specific controller name and filters ES based on this property'
+                      description: 'Used to select the correct ESO controller (think: ingress.ingressClassName) The ESO controller is instantiated with a specific controller name and filters ES based on this property'
                       type: string
                     provider:
                       description: Used to configure the provider. Only one provider may be set
@@ -1216,7 +1216,7 @@ should match snapshot of default values:
                         type: object
                       type: array
                     controller:
-                      description: 'Used to select the correct KES controller (think: ingress.ingressClassName) The KES controller is instantiated with a specific controller name and filters ES based on this property'
+                      description: 'Used to select the correct ESO controller (think: ingress.ingressClassName) The ESO controller is instantiated with a specific controller name and filters ES based on this property'
                       type: string
                     provider:
                       description: Used to configure the provider. Only one provider may be set

deploy/crds/bundle.yaml

@@ -498,7 +498,7 @@ spec:
               description: SecretStoreSpec defines the desired state of SecretStore.
               properties:
                 controller:
-                  description: 'Used to select the correct KES controller (think: ingress.ingressClassName) The KES controller is instantiated with a specific controller name and filters ES based on this property'
+                  description: 'Used to select the correct ESO controller (think: ingress.ingressClassName) The ESO controller is instantiated with a specific controller name and filters ES based on this property'
                   type: string
                 provider:
                   description: Used to configure the provider. Only one provider may be set
@@ -1660,7 +1660,7 @@ spec:
                     type: object
                   type: array
                 controller:
-                  description: 'Used to select the correct KES controller (think: ingress.ingressClassName) The KES controller is instantiated with a specific controller name and filters ES based on this property'
+                  description: 'Used to select the correct ESO controller (think: ingress.ingressClassName) The ESO controller is instantiated with a specific controller name and filters ES based on this property'
                   type: string
                 provider:
                   description: Used to configure the provider. Only one provider may be set
@@ -4052,7 +4052,7 @@ spec:
               description: SecretStoreSpec defines the desired state of SecretStore.
               properties:
                 controller:
-                  description: 'Used to select the correct KES controller (think: ingress.ingressClassName) The KES controller is instantiated with a specific controller name and filters ES based on this property'
+                  description: 'Used to select the correct ESO controller (think: ingress.ingressClassName) The ESO controller is instantiated with a specific controller name and filters ES based on this property'
                   type: string
                 provider:
                   description: Used to configure the provider. Only one provider may be set
@@ -5214,7 +5214,7 @@ spec:
                     type: object
                   type: array
                 controller:
-                  description: 'Used to select the correct KES controller (think: ingress.ingressClassName) The KES controller is instantiated with a specific controller name and filters ES based on this property'
+                  description: 'Used to select the correct ESO controller (think: ingress.ingressClassName) The ESO controller is instantiated with a specific controller name and filters ES based on this property'
                   type: string
                 provider:
                   description: Used to configure the provider. Only one provider may be set
@@ -7031,6 +7031,9 @@ spec:
             spec:
               description: FakeSpec contains the static data.
               properties:
+                controller:
+                  description: '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
                 data:
                   additionalProperties:
                     type: string
@@ -7270,6 +7273,9 @@ spec:
               type: object
             spec:
               properties:
+                controller:
+                  description: '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
                 method:
                   description: Vault API method to use (GET/POST/other)
                   type: string

docs/api/spec.md

@@ -1470,8 +1470,8 @@ string
 </td>
 <td>
 <em>(Optional)</em>
-<p>Used to select the correct KES controller (think: ingress.ingressClassName)
-The KES controller is instantiated with a specific controller name and filters ES based on this property</p>
+<p>Used to select the correct ESO controller (think: ingress.ingressClassName)
+The ESO controller is instantiated with a specific controller name and filters ES based on this property</p>
 </td>
 </tr>
 <tr>
@@ -4211,8 +4211,8 @@ string
 </td>
 <td>
 <em>(Optional)</em>
-<p>Used to select the correct KES controller (think: ingress.ingressClassName)
-The KES controller is instantiated with a specific controller name and filters ES based on this property</p>
+<p>Used to select the correct ESO controller (think: ingress.ingressClassName)
+The ESO controller is instantiated with a specific controller name and filters ES based on this property</p>
 </td>
 </tr>
 <tr>
@@ -4722,8 +4722,8 @@ string
 </td>
 <td>
 <em>(Optional)</em>
-<p>Used to select the correct KES controller (think: ingress.ingressClassName)
-The KES controller is instantiated with a specific controller name and filters ES based on this property</p>
+<p>Used to select the correct ESO controller (think: ingress.ingressClassName)
+The ESO controller is instantiated with a specific controller name and filters ES based on this property</p>
 </td>
 </tr>
 <tr>

pkg/controllers/externalsecret/externalsecret_controller.go

@@ -458,6 +458,21 @@ func shouldSkipUnmanagedStore(ctx context.Context, namespace string, r *Reconcil
 		if ref.SourceRef != nil && ref.SourceRef.SecretStoreRef != nil {
 			storeList = append(storeList, *ref.SourceRef.SecretStoreRef)
 		}
+
+		// verify that generator's controllerClass matches
+		if ref.SourceRef != nil && ref.SourceRef.GeneratorRef != nil {
+			genDef, err := r.getGeneratorDefinition(ctx, namespace, ref.SourceRef)
+			if err != nil {
+				return false, err
+			}
+			skipGenerator, err := shouldSkipGenerator(r, genDef)
+			if err != nil {
+				return false, err
+			}
+			if skipGenerator {
+				return true, nil
+			}
+		}
 	}
 
 	for _, ref := range storeList {

pkg/controllers/externalsecret/externalsecret_controller_secret.go

@@ -16,6 +16,7 @@ package externalsecret
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 
@@ -229,3 +230,13 @@ func (r *Reconciler) handleFindAllSecrets(ctx context.Context, externalSecret *e
 	}
 	return secretMap, err
 }
+
+func shouldSkipGenerator(r *Reconciler, generatorDef *apiextensions.JSON) (bool, error) {
+	var genControllerClass genv1alpha1.ControllerClassResource
+	err := json.Unmarshal(generatorDef.Raw, &genControllerClass)
+	if err != nil {
+		return false, err
+	}
+	var controllerClass = genControllerClass.Spec.ControllerClass
+	return controllerClass != "" && controllerClass != r.ControllerClass, nil
+}

pkg/controllers/externalsecret/externalsecret_controller_test.go

@@ -16,6 +16,7 @@ package externalsecret
 import (
 	"bytes"
 	"context"
+	"encoding/json"
 	"fmt"
 	"os"
 	"strconv"
@@ -26,6 +27,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	dto "github.com/prometheus/client_model/go"
 	v1 "k8s.io/api/core/v1"
+	apiextensions "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -487,6 +489,33 @@ var _ = Describe("ExternalSecret controller", func() {
 		}
 	}
 
+	ignoreMismatchControllerForGeneratorRef := func(tc *testCase) {
+		const secretKey = "somekey"
+		const secretVal = "someValue"
+
+		fakeGenerator := &genv1alpha1.Fake{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "mytestfake2",
+				Namespace: ExternalSecretNamespace,
+			},
+			Spec: genv1alpha1.FakeSpec{
+				Data: map[string]string{
+					secretKey: secretVal,
+				},
+				Controller: "fakeControllerClass",
+			},
+		}
+
+		fakeGeneratorJSON, _ := json.Marshal(fakeGenerator)
+
+		Expect(shouldSkipGenerator(
+			&Reconciler{
+				ControllerClass: "default",
+			},
+			&apiextensions.JSON{Raw: fakeGeneratorJSON},
+		)).To(BeTrue())
+	}
+
 	syncWithMultipleSecretStores := func(tc *testCase) {
 		Expect(k8sClient.Create(context.Background(), &esv1beta1.SecretStore{
 			ObjectMeta: metav1.ObjectMeta{
@@ -1970,6 +1999,7 @@ var _ = Describe("ExternalSecret controller", func() {
 		Entry("should not update unchanged secret using creationPolicy=Merge", mergeWithSecretNoChange),
 		Entry("should not delete pre-existing secret with creationPolicy=Orphan", createSecretPolicyOrphan),
 		Entry("should sync with generatorRef", syncWithGeneratorRef),
+		Entry("should not process generatorRef with mismatching controller field", ignoreMismatchControllerForGeneratorRef),
 		Entry("should sync with multiple secret stores via sourceRef", syncWithMultipleSecretStores),
 		Entry("should sync with template", syncWithTemplate),
 		Entry("should sync with template engine v2", syncWithTemplateV2),