Allow to disable secret auto-recreation (#1118)

* Allow to disable secret auto-recreation Co-authored-by: Alejandro Moreno <[email protected]> Signed-off-by: Jose Luis Vazquez Gonzalez <[email protected]>
bitnami-labs · Mar 6, 2023 · fc66939 · fc66939
1 parent a6b155f
commit fc66939
Show file tree

Hide file tree

Showing 9 changed files with 129 additions and 29 deletions.
diff --git a/carvel/package.yaml b/carvel/package.yaml
@@ -67,6 +67,10 @@ spec:
           type: boolean
           description: Specifies whether the Sealed Secrets controller should update the status subresource
           default: true
+        skipRecreate:
+          type: boolean
+          description: Specifies whether the Sealed Secrets controller should skip recreating removed secrets
+          default: false
         keyrenewperiod:
           type: string
           description: Specifies key renewal period. Default 30 days

diff --git a/cmd/controller/main.go b/cmd/controller/main.go
@@ -47,6 +47,8 @@ func bindControllerFlags(f *controller.Flags, fs *flag.FlagSet) {
 
 	fs.BoolVar(&f.UpdateStatus, "update-status", true, "beta: if true, the controller will update the status sub-resource whenever it processes a sealed secret")
 
+	fs.BoolVar(&f.SkipRecreate, "skip-recreate", false, "if true the controller will skip listening for managed secret changes to recreate them. This helps on limited permission environments.")
+
 	fs.DurationVar(&f.KeyRenewPeriod, "rotate-period", defaultKeyRenewPeriod, "")
 	_ = fs.MarkDeprecated("rotate-period", "please use key-renew-period instead")
 }

diff --git a/helm/sealed-secrets/README.md b/helm/sealed-secrets/README.md
@@ -92,6 +92,7 @@ The command removes all the Kubernetes components associated with the chart and
 | `createController`                                | Specifies whether the Sealed Secrets controller should be created                    | `true`                              |
 | `secretName`                                      | The name of an existing TLS secret containing the key used to encrypt secrets        | `sealed-secrets-key`                |
 | `updateStatus`                                    | Specifies whether the Sealed Secrets controller should update the status subresource | `true`                              |
+| `skipRecreate`                                   | Specifies whether the Sealed Secrets controller should skip recreating removed secrets | `false`                              |
 | `keyrenewperiod`                                  | Specifies key renewal period. Default 30 days                                        | `""`                                |
 | `rateLimit`                                       | Number of allowed sustained request per second for verify endpoint                   | `""`                                |
 | `rateLimitBurst`                                  | Number of requests allowed to exceed the rate limit per second for verify endpoint   | `""`                                |

diff --git a/helm/sealed-secrets/templates/deployment.yaml b/helm/sealed-secrets/templates/deployment.yaml
@@ -70,6 +70,9 @@ spec:
             {{- if .Values.updateStatus }}
             - --update-status
             {{- end }}
+            {{- if .Values.skipRecreate }}
+            - --skip-recreate
+            {{- end }}
             {{- if .Values.keyrenewperiod }}
             - --key-renew-period
             - {{ .Values.keyrenewperiod | quote }}

diff --git a/helm/sealed-secrets/values.yaml b/helm/sealed-secrets/values.yaml
@@ -56,6 +56,12 @@ secretName: "sealed-secrets-key"
 ## @param updateStatus Specifies whether the Sealed Secrets controller should update the status subresource
 ##
 updateStatus: true
+## @param skipRecreate Specifies whether the Sealed Secrets controller should skip recreating removed secrets
+## Setting it to false allows to optionally restore backward compatibility in low priviledge
+## environments when old versions of the controller did not require watch permissions on secrets
+## for secret re-creation.
+##
+skipRecreate: false
 ## @param keyrenewperiod Specifies key renewal period. Default 30 days
 ## e.g
 ## keyrenewperiod: "720h30m"

diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go
@@ -84,10 +84,34 @@ func NewController(clientset kubernetes.Interface, ssclientset ssclientset.Inter
 	eventBroadcaster.StartRecordingToSink(&v1.EventSinkImpl{Interface: clientset.CoreV1().Events("")})
 	recorder := eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: "sealed-secrets"})
 
+	ssInformer, err := watchSealedSecrets(ssinformer, queue)
+	if err != nil {
+		return nil, err
+	}
+
+	var sInformer cache.SharedIndexInformer
+	if sinformer != nil {
+		sInformer, err = watchSecrets(sinformer, ssclientset, queue)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	return &Controller{
+		ssInformer:  ssInformer,
+		sInformer:   sInformer,
+		queue:       queue,
+		sclient:     clientset.CoreV1(),
+		ssclient:    ssclientset.BitnamiV1alpha1(),
+		recorder:    recorder,
+		keyRegistry: keyRegistry,
+	}, nil
+}
+
+func watchSealedSecrets(ssinformer ssinformer.SharedInformerFactory, queue workqueue.RateLimitingInterface) (cache.SharedIndexInformer, error) {
 	ssInformer := ssinformer.Bitnami().V1alpha1().
 		SealedSecrets().
 		Informer()
-
 	_, err := ssInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{
 		AddFunc: func(obj interface{}) {
 			key, err := cache.MetaNamespaceKeyFunc(obj)
@@ -114,9 +138,12 @@ func NewController(clientset kubernetes.Interface, ssclientset ssclientset.Inter
 	if err != nil {
 		return nil, fmt.Errorf("could not add event handler to sealed secrets informer: %w", err)
 	}
+	return ssInformer, nil
+}
 
+func watchSecrets(sinformer informers.SharedInformerFactory, ssclientset ssclientset.Interface, queue workqueue.RateLimitingInterface) (cache.SharedIndexInformer, error) {
 	sInformer := sinformer.Core().V1().Secrets().Informer()
-	_, err = sInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{
+	_, err := sInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{
 		DeleteFunc: func(obj interface{}) {
 			skey, err := cache.DeletionHandlingMetaNamespaceKeyFunc(obj)
 			if err != nil {
@@ -154,16 +181,7 @@ func NewController(clientset kubernetes.Interface, ssclientset ssclientset.Inter
 	if err != nil {
 		return nil, fmt.Errorf("could not add event handler to secrets informer: %w", err)
 	}
-
-	return &Controller{
-		ssInformer:  ssInformer,
-		sInformer:   sInformer,
-		queue:       queue,
-		sclient:     clientset.CoreV1(),
-		ssclient:    ssclientset.BitnamiV1alpha1(),
-		recorder:    recorder,
-		keyRegistry: keyRegistry,
-	}, nil
+	return sInformer, nil
 }
 
 // HasSynced returns true once this controller has completed an
@@ -192,7 +210,7 @@ func (c *Controller) Run(stopCh <-chan struct{}) {
 	go c.sInformer.Run(stopCh)
 
 	if !cache.WaitForCacheSync(stopCh, c.HasSynced) {
-		utilruntime.HandleError(fmt.Errorf("Timed out waiting for caches to sync"))
+		utilruntime.HandleError(fmt.Errorf("timed out waiting for caches to sync"))
 		return
 	}
 
@@ -417,7 +435,7 @@ func (c *Controller) AttemptUnseal(content []byte) (bool, error) {
 		}
 		return true, nil
 	default:
-		return false, fmt.Errorf("Unexpected resource type: %s", s.GetObjectKind().GroupVersionKind().String())
+		return false, fmt.Errorf("unexpected resource type: %s", s.GetObjectKind().GroupVersionKind().String())
 	}
 }
 
@@ -434,20 +452,20 @@ func (c *Controller) Rotate(content []byte) ([]byte, error) {
 	case *ssv1alpha1.SealedSecret:
 		secret, err := c.attemptUnseal(s)
 		if err != nil {
-			return nil, fmt.Errorf("Error decrypting secret. %v", err)
+			return nil, fmt.Errorf("error decrypting secret. %v", err)
 		}
 		latestPrivKey := c.keyRegistry.latestPrivateKey()
 		resealedSecret, err := ssv1alpha1.NewSealedSecret(scheme.Codecs, &latestPrivKey.PublicKey, secret)
 		if err != nil {
-			return nil, fmt.Errorf("Error creating new sealed secret. %v", err)
+			return nil, fmt.Errorf("error creating new sealed secret. %v", err)
 		}
 		data, err := json.Marshal(resealedSecret)
 		if err != nil {
-			return nil, fmt.Errorf("Error marshalling new secret to json. %v", err)
+			return nil, fmt.Errorf("error marshalling new secret to json. %v", err)
 		}
 		return data, nil
 	default:
-		return nil, fmt.Errorf("Unexpected resource type: %s", s.GetObjectKind().GroupVersionKind().String())
+		return nil, fmt.Errorf("unexpected resource type: %s", s.GetObjectKind().GroupVersionKind().String())
 	}
 }
 

diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go
@@ -1,11 +1,18 @@
 package controller
 
 import (
+	"context"
+	"crypto/rand"
 	"errors"
 	"fmt"
 	"testing"
 
 	ssv1alpha1 "github.com/bitnami-labs/sealed-secrets/pkg/apis/sealedsecrets/v1alpha1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/kubernetes/fake"
+
+	ssfake "github.com/bitnami-labs/sealed-secrets/pkg/client/clientset/versioned/fake"
 )
 
 func TestConvert2SealedSecretBadType(t *testing.T) {
@@ -45,3 +52,54 @@ func TestConvert2SealedSecretPassThrough(t *testing.T) {
 		t.Fatalf("got %v want %v", got, want)
 	}
 }
+
+func TestDefaultConfigDoesNotSkipRecreate(t *testing.T) {
+	ns := "some-namespace"
+	var tweakopts func(*metav1.ListOptions)
+	clientset := fake.NewSimpleClientset()
+	ssc := ssfake.NewSimpleClientset()
+	keyRegistry := testKeyRegister(t, context.Background(), clientset, ns)
+
+	got, err := prepareController(clientset, ns, tweakopts, &Flags{SkipRecreate: false}, ssc, keyRegistry)
+	if err != nil {
+		t.Fatalf("err %v want %v", got, nil)
+	}
+	if got == nil {
+		t.Fatalf("ctrl %v want non nil", got)
+	}
+	if got.sInformer == nil {
+		t.Fatalf("sInformer %v want non nil", got.sInformer)
+	}
+}
+
+func TestSkipRecreateConfigDoesSkipIt(t *testing.T) {
+	ns := "some-namespace"
+	var tweakopts func(*metav1.ListOptions)
+	clientset := fake.NewSimpleClientset()
+	ssc := ssfake.NewSimpleClientset()
+	keyRegistry := testKeyRegister(t, context.Background(), clientset, ns)
+
+	got, err := prepareController(clientset, ns, tweakopts, &Flags{SkipRecreate: true}, ssc, keyRegistry)
+	if err != nil {
+		t.Fatalf("err %v want %v", got, nil)
+	}
+	if got == nil {
+		t.Fatalf("ctrl %v want non nil", got)
+	}
+	if got.sInformer != nil {
+		t.Fatalf("sInformer %v want nil", got.sInformer)
+	}
+}
+
+func testKeyRegister(t *testing.T, ctx context.Context, clientset kubernetes.Interface, ns string) *KeyRegistry {
+	t.Helper()
+
+	keyLabel := SealedSecretsKeyLabel
+	prefix := "test-keys"
+	testKeySize := 4096
+	keyRegistry, err := initKeyRegistry(ctx, clientset, rand.Reader, ns, prefix, keyLabel, testKeySize)
+	if err != nil {
+		t.Fatalf("failed to provision key registry: %v", err)
+	}
+	return keyRegistry
+}
diff --git a/pkg/controller/keys.go b/pkg/controller/keys.go
@@ -21,7 +21,7 @@ const SealedSecretsKeyLabel = "sealedsecrets.bitnami.com/sealed-secrets-key"
 
 var (
 	// ErrPrivateKeyNotRSA is returned when the private key is not a valid RSA key.
-	ErrPrivateKeyNotRSA = errors.New("Private key is not an RSA key")
+	ErrPrivateKeyNotRSA = errors.New("private key is not an RSA key")
 )
 
 func generatePrivateKeyAndCert(keySize int, validFor time.Duration, cn string) (*rsa.PrivateKey, *x509.Certificate, error) {

diff --git a/pkg/controller/main.go b/pkg/controller/main.go
@@ -23,6 +23,7 @@ import (
 	"k8s.io/client-go/informers"
 
 	ssv1alpha1 "github.com/bitnami-labs/sealed-secrets/pkg/apis/sealedsecrets/v1alpha1"
+	"github.com/bitnami-labs/sealed-secrets/pkg/client/clientset/versioned"
 	sealedsecrets "github.com/bitnami-labs/sealed-secrets/pkg/client/clientset/versioned"
 	ssinformers "github.com/bitnami-labs/sealed-secrets/pkg/client/informers/externalversions"
 )
@@ -48,6 +49,7 @@ type Flags struct {
 	RateLimitBurst       int
 	OldGCBehavior        bool
 	UpdateStatus         bool
+	SkipRecreate         bool
 }
 
 func initKeyPrefix(keyPrefix string) (string, error) {
@@ -195,9 +197,7 @@ func Main(f *Flags, version string) error {
 		}
 	}
 
-	sinformer := informers.NewFilteredSharedInformerFactory(clientset, 0, namespace, tweakopts)
-	ssinformer := ssinformers.NewFilteredSharedInformerFactory(ssclientset, 0, namespace, tweakopts)
-	controller, err := NewController(clientset, ssclientset, ssinformer, sinformer, keyRegistry)
+	controller, err := prepareController(clientset, namespace, tweakopts, f, ssclientset, keyRegistry)
 	if err != nil {
 		return err
 	}
@@ -212,10 +212,6 @@ func Main(f *Flags, version string) error {
 	if f.AdditionalNamespaces != "" {
 		addNS := removeDuplicates(strings.Split(f.AdditionalNamespaces, ","))
 
-		var ssinf ssinformers.SharedInformerFactory
-		var sinf informers.SharedInformerFactory
-		var ctlr *Controller
-
 		for _, ns := range addNS {
 			if _, err := clientset.CoreV1().Namespaces().Get(ctx, ns, metav1.GetOptions{}); err != nil {
 				if errors.IsNotFound(err) {
@@ -225,9 +221,7 @@ func Main(f *Flags, version string) error {
 				return err
 			}
 			if ns != namespace {
-				ssinf = ssinformers.NewFilteredSharedInformerFactory(ssclientset, 0, ns, tweakopts)
-				sinf = informers.NewFilteredSharedInformerFactory(clientset, 0, ns, tweakopts)
-				ctlr, err = NewController(clientset, ssclientset, ssinf, sinf, keyRegistry)
+				ctlr, err := prepareController(clientset, ns, tweakopts, f, ssclientset, keyRegistry)
 				if err != nil {
 					return err
 				}
@@ -255,3 +249,17 @@ func Main(f *Flags, version string) error {
 
 	return server.Shutdown(context.Background())
 }
+
+func prepareController(clientset kubernetes.Interface, namespace string, tweakopts func(*metav1.ListOptions), f *Flags, ssclientset versioned.Interface, keyRegistry *KeyRegistry) (*Controller, error) {
+	sinformer := initSecretInformerFactory(clientset, namespace, tweakopts, f.SkipRecreate)
+	ssinformer := ssinformers.NewFilteredSharedInformerFactory(ssclientset, 0, namespace, tweakopts)
+	controller, err := NewController(clientset, ssclientset, ssinformer, sinformer, keyRegistry)
+	return controller, err
+}
+
+func initSecretInformerFactory(clientset kubernetes.Interface, ns string, tweakopts func(*metav1.ListOptions), skipRecreate bool) informers.SharedInformerFactory {
+	if skipRecreate {
+		return nil
+	}
+	return informers.NewFilteredSharedInformerFactory(clientset, 0, ns, tweakopts)
+}