Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to disable secret auto-recreation #1118

Merged
merged 6 commits into from
Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions carvel/package.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions cmd/controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
1 change: 1 addition & 0 deletions helm/sealed-secrets/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | `""` |
Expand Down
3 changes: 3 additions & 0 deletions helm/sealed-secrets/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
6 changes: 6 additions & 0 deletions helm/sealed-secrets/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ secretName: "sealed-secrets-key"
## @param updateStatus Specifies whether the Sealed Secrets controller should update the status subresource
##
updateStatus: true
## @param skip-recreate Specifies whether the Sealed Secrets controller should skip recreating removed secrets
josvazg marked this conversation as resolved.
Show resolved Hide resolved
## Setting it to false allows to optionally restore backward compatibility in low priviledge
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed this small typo ^^
This message is talking about the case in which skipRecreate is actually set to true

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed per #1371

## 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"
Expand Down
54 changes: 36 additions & 18 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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())
}
}

Expand All @@ -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())
}
}

Expand Down
57 changes: 57 additions & 0 deletions pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,19 @@
package controller

import (
"context"
"crypto/rand"
"errors"
"fmt"
"testing"

ssv1alpha1 "github.com/bitnami-labs/sealed-secrets/pkg/apis/sealedsecrets/v1alpha1"
ssinformers "github.com/bitnami-labs/sealed-secrets/pkg/client/informers/externalversions"
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) {
Expand Down Expand Up @@ -45,3 +53,52 @@ 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()
sinformer := initSecretInformerFactory(clientset, ns, tweakopts, false)
if sinformer == nil {
t.Fatalf("sinformer %v want non nil", sinformer)
}
ssinformer := ssinformers.NewFilteredSharedInformerFactory(ssc, 0, ns, tweakopts)
keyRegistry := testKeyRegister(t, context.Background(), clientset, ns)

_, got := NewController(clientset, ssc, ssinformer, sinformer, keyRegistry)
if got != nil {
t.Fatalf("got %v want %v", got, nil)
}
}

func TestSkipReleaseConfigDoesSkipIt(t *testing.T) {
josvazg marked this conversation as resolved.
Show resolved Hide resolved
ns := "some-namespace"
var tweakopts func(*metav1.ListOptions)
clientset := fake.NewSimpleClientset()
ssc := ssfake.NewSimpleClientset()
sinformer := initSecretInformerFactory(clientset, ns, tweakopts, true)
if sinformer != nil {
t.Fatalf("sinformer %v want nil", sinformer)
}
ssinformer := ssinformers.NewFilteredSharedInformerFactory(ssc, 0, ns, tweakopts)
keyRegistry := testKeyRegister(t, context.Background(), clientset, ns)

_, got := NewController(clientset, ssc, ssinformer, sinformer, keyRegistry)
if got != nil {
t.Fatalf("got %v want %v", got, nil)
}
}

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
}
2 changes: 1 addition & 1 deletion pkg/controller/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 9 additions & 1 deletion pkg/controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type Flags struct {
RateLimitBurst int
OldGCBehavior bool
UpdateStatus bool
SkipRecreate bool
}

func initKeyPrefix(keyPrefix string) (string, error) {
Expand Down Expand Up @@ -226,7 +227,7 @@ func Main(f *Flags, version string) error {
}
if ns != namespace {
ssinf = ssinformers.NewFilteredSharedInformerFactory(ssclientset, 0, ns, tweakopts)
sinf = informers.NewFilteredSharedInformerFactory(clientset, 0, ns, tweakopts)
sinf = initSecretInformerFactory(clientset, ns, tweakopts, f.SkipRecreate)
josvazg marked this conversation as resolved.
Show resolved Hide resolved
ctlr, err = NewController(clientset, ssclientset, ssinf, sinf, keyRegistry)
if err != nil {
return err
Expand Down Expand Up @@ -255,3 +256,10 @@ func Main(f *Flags, version string) error {

return server.Shutdown(context.Background())
}

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)
}