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

Use RequiresRepublish in secret rotation. #1622

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 0 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -403,9 +403,7 @@ e2e-provider-deploy:
e2e-deploy-manifest:
kubectl apply -f manifest_staging/deploy/csidriver.yaml
kubectl apply -f manifest_staging/deploy/rbac-secretproviderclass.yaml
kubectl apply -f manifest_staging/deploy/rbac-secretproviderrotation.yaml
kubectl apply -f manifest_staging/deploy/rbac-secretprovidersyncing.yaml
kubectl apply -f manifest_staging/deploy/rbac-secretprovidertokenrequest.yaml
kubectl apply -f manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasses.yaml
kubectl apply -f manifest_staging/deploy/secrets-store.csi.x-k8s.io_secretproviderclasspodstatuses.yaml
kubectl apply -f manifest_staging/deploy/role-secretproviderclasses-admin.yaml
Expand Down
24 changes: 1 addition & 23 deletions cmd/secrets-store-csi-driver/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ import (

secretsstorev1 "sigs.k8s.io/secrets-store-csi-driver/apis/v1"
"sigs.k8s.io/secrets-store-csi-driver/controllers"
"sigs.k8s.io/secrets-store-csi-driver/pkg/k8s"
"sigs.k8s.io/secrets-store-csi-driver/pkg/metrics"
"sigs.k8s.io/secrets-store-csi-driver/pkg/rotation"
secretsstore "sigs.k8s.io/secrets-store-csi-driver/pkg/secrets-store"
"sigs.k8s.io/secrets-store-csi-driver/pkg/version"

Expand All @@ -39,7 +37,6 @@ import (
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
"k8s.io/klog/v2"
Expand Down Expand Up @@ -203,26 +200,7 @@ func mainErr() error {
reconciler.RunPatcher(ctx)
}()

// token request client
kubeClient := kubernetes.NewForConfigOrDie(cfg)
tokenClient := k8s.NewTokenClient(kubeClient, *driverName, 10*time.Minute)

if err = tokenClient.Run(ctx.Done()); err != nil {
klog.ErrorS(err, "failed to run token client")
return err
}

// Secret rotation
if *enableSecretRotation {
rec, err := rotation.NewReconciler(*driverName, mgr.GetCache(), scheme, *rotationPollInterval, providerClients, tokenClient)
if err != nil {
klog.ErrorS(err, "failed to initialize rotation reconciler")
return err
}
go rec.Run(ctx.Done())
}

driver := secretsstore.NewSecretsStoreDriver(*driverName, *nodeID, *endpoint, providerClients, mgr.GetClient(), mgr.GetAPIReader(), tokenClient)
driver := secretsstore.NewSecretsStoreDriver(*driverName, *nodeID, *endpoint, providerClients, mgr.GetClient(), mgr.GetAPIReader(), *enableSecretRotation, *rotationPollInterval)
driver.Run(ctx)

return nil
Expand Down
104 changes: 40 additions & 64 deletions controllers/secretproviderclasspodstatus_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,58 +295,44 @@ func (r *SecretProviderClassPodStatusReconciler) Reconcile(ctx context.Context,
errs = append(errs, fmt.Errorf("failed to validate secret object in spc %s/%s, err: %w", spc.Namespace, spc.Name, err))
continue
}
exists, err := r.secretExists(ctx, secretName, req.Namespace)
if err != nil {
klog.ErrorS(err, "failed to check if secret exists", "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spc", klog.KObj(spc), "pod", klog.KObj(pod), "spcps", klog.KObj(spcPodStatus))
// syncSecret.enabled is set to false by default in the helm chart for installing the driver in v0.0.23+
// that would result in a forbidden error, so generate a warning that can be helpful for debugging
if apierrors.IsForbidden(err) {
klog.Warning(SyncSecretForbiddenWarning)
}
errs = append(errs, fmt.Errorf("failed to check if secret %s exists, err: %w", secretName, err))
continue
}

var funcs []func() (bool, error)
secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type))

if !exists {
secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type))

var datamap map[string][]byte
if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files); err != nil {
r.generateEvent(pod, corev1.EventTypeWarning, secretCreationFailedReason, fmt.Sprintf("failed to get data in spc %s/%s for secret %s, err: %+v", req.Namespace, spcName, secretName, err))
klog.ErrorS(err, "failed to get data in spc for secret", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spcps", klog.KObj(spcPodStatus))
errs = append(errs, fmt.Errorf("failed to get data in spc %s/%s for secret %s, err: %w", req.Namespace, spcName, secretName, err))
continue
}
var datamap map[string][]byte
if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files); err != nil {
r.generateEvent(pod, corev1.EventTypeWarning, secretCreationFailedReason, fmt.Sprintf("failed to get data in spc %s/%s for secret %s, err: %+v", req.Namespace, spcName, secretName, err))
klog.ErrorS(err, "failed to get data in spc for secret", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spcps", klog.KObj(spcPodStatus))
errs = append(errs, fmt.Errorf("failed to get data in spc %s/%s for secret %s, err: %w", req.Namespace, spcName, secretName, err))
continue
}

labelsMap := make(map[string]string)
if secretObj.Labels != nil {
labelsMap = secretObj.Labels
}
annotationsMap := make(map[string]string)
if secretObj.Annotations != nil {
annotationsMap = secretObj.Annotations
}
// Set secrets-store.csi.k8s.io/managed=true label on the secret that's created and managed
// by the secrets-store-csi-driver. This label will be used to perform a filtered list watch
// only on secrets created and managed by the driver
labelsMap[SecretManagedLabel] = "true"

createFn := func() (bool, error) {
if err := r.createK8sSecret(ctx, secretName, req.Namespace, datamap, labelsMap, annotationsMap, secretType); err != nil {
klog.ErrorS(err, "failed to create Kubernetes secret", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spcps", klog.KObj(spcPodStatus))
// syncSecret.enabled is set to false by default in the helm chart for installing the driver in v0.0.23+
// that would result in a forbidden error, so generate a warning that can be helpful for debugging
if apierrors.IsForbidden(err) {
klog.Warning(SyncSecretForbiddenWarning)
}
return false, nil
labelsMap := make(map[string]string)
if secretObj.Labels != nil {
labelsMap = secretObj.Labels
}
annotationsMap := make(map[string]string)
if secretObj.Annotations != nil {
annotationsMap = secretObj.Annotations
}
// Set secrets-store.csi.k8s.io/managed=true label on the secret that's created and managed
// by the secrets-store-csi-driver. This label will be used to perform a filtered list watch
// only on secrets created and managed by the driver
labelsMap[SecretManagedLabel] = "true"

createFn := func() (bool, error) {
if err := r.createOrUpdateK8sSecret(ctx, secretName, req.Namespace, datamap, labelsMap, annotationsMap, secretType); err != nil {
klog.ErrorS(err, "failed to create Kubernetes secret", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spcps", klog.KObj(spcPodStatus))
// syncSecret.enabled is set to false by default in the helm chart for installing the driver in v0.0.23+
// that would result in a forbidden error, so generate a warning that can be helpful for debugging
if apierrors.IsForbidden(err) {
klog.Warning(SyncSecretForbiddenWarning)
}
return true, nil
return false, nil
}
funcs = append(funcs, createFn)
return true, nil
}
funcs = append(funcs, createFn)

for _, f := range funcs {
if err := wait.ExponentialBackoff(wait.Backoff{
Expand Down Expand Up @@ -410,9 +396,9 @@ func (r *SecretProviderClassPodStatusReconciler) processIfBelongsToNode(objMeta
return true
}

// createK8sSecret creates K8s secret with data from mounted files
// createOrUpdateK8sSecret creates K8s secret with data from mounted files
// If a secret with the same name already exists in the namespace of the pod, the error is nil.
func (r *SecretProviderClassPodStatusReconciler) createK8sSecret(ctx context.Context, name, namespace string, datamap map[string][]byte, labelsmap map[string]string, annotationsmap map[string]string, secretType corev1.SecretType) error {
func (r *SecretProviderClassPodStatusReconciler) createOrUpdateK8sSecret(ctx context.Context, name, namespace string, datamap map[string][]byte, labelsmap map[string]string, annotationsmap map[string]string, secretType corev1.SecretType) error {
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: namespace,
Expand All @@ -430,6 +416,13 @@ func (r *SecretProviderClassPodStatusReconciler) createK8sSecret(ctx context.Con
return nil
}
if apierrors.IsAlreadyExists(err) {
klog.InfoS("Kubernetes secret is already created", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
err := r.writer.Update(ctx, secret)
if err != nil {
klog.Errorf("Unable to update kubernetes secret", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
return err
}
klog.InfoS("successfully updated Kubernetes secret", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
return nil
}
return err
Expand Down Expand Up @@ -477,23 +470,6 @@ func (r *SecretProviderClassPodStatusReconciler) patchSecretWithOwnerRef(ctx con
return nil
}

// secretExists checks if the secret with name and namespace already exists
func (r *SecretProviderClassPodStatusReconciler) secretExists(ctx context.Context, name, namespace string) (bool, error) {
o := &corev1.Secret{}
secretKey := types.NamespacedName{
Namespace: namespace,
Name: name,
}
err := r.Client.Get(ctx, secretKey, o)
if err == nil {
return true, nil
}
if apierrors.IsNotFound(err) {
return false, nil
}
return false, err
}

// generateEvent generates an event
func (r *SecretProviderClassPodStatusReconciler) generateEvent(obj apiruntime.Object, eventType, reason, message string) {
if obj != nil {
Expand Down
33 changes: 4 additions & 29 deletions controllers/secretproviderclasspodstatus_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,31 +121,6 @@ func newReconciler(client client.Client, scheme *runtime.Scheme, nodeID string)
}
}

func TestSecretExists(t *testing.T) {
g := NewWithT(t)

scheme, err := setupScheme()
g.Expect(err).NotTo(HaveOccurred())

labels := map[string]string{"environment": "test"}
annotations := map[string]string{"kubed.appscode.com/sync": "app=test"}

initObjects := []client.Object{
newSecret("my-secret", "default", labels, annotations),
}

client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(initObjects...).Build()
reconciler := newReconciler(client, scheme, "node1")

exists, err := reconciler.secretExists(context.TODO(), "my-secret", "default")
g.Expect(exists).To(Equal(true))
g.Expect(err).NotTo(HaveOccurred())

exists, err = reconciler.secretExists(context.TODO(), "my-secret2", "default")
g.Expect(exists).To(Equal(false))
g.Expect(err).NotTo(HaveOccurred())
}

func TestPatchSecretWithOwnerRef(t *testing.T) {
g := NewWithT(t)

Expand Down Expand Up @@ -183,7 +158,7 @@ func TestPatchSecretWithOwnerRef(t *testing.T) {
g.Expect(secret.GetOwnerReferences()).To(HaveLen(1))
}

func TestCreateK8sSecret(t *testing.T) {
func TestCreateOrUpdateK8sSecret(t *testing.T) {
g := NewWithT(t)

scheme, err := setupScheme()
Expand All @@ -198,11 +173,11 @@ func TestCreateK8sSecret(t *testing.T) {
client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(initObjects...).Build()
reconciler := newReconciler(client, scheme, "node1")

// secret already exists
err = reconciler.createK8sSecret(context.TODO(), "my-secret", "default", nil, labels, annotations, corev1.SecretTypeOpaque)
// secret already exists, just update it.
err = reconciler.createOrUpdateK8sSecret(context.TODO(), "my-secret", "default", nil, labels, annotations, corev1.SecretTypeOpaque)
g.Expect(err).NotTo(HaveOccurred())

err = reconciler.createK8sSecret(context.TODO(), "my-secret2", "default", nil, labels, annotations, corev1.SecretTypeOpaque)
err = reconciler.createOrUpdateK8sSecret(context.TODO(), "my-secret2", "default", nil, labels, annotations, corev1.SecretTypeOpaque)
g.Expect(err).NotTo(HaveOccurred())
secret := &corev1.Secret{}
err = client.Get(context.TODO(), types.NamespacedName{Name: "my-secret2", Namespace: "default"}, secret)
Expand Down
22 changes: 0 additions & 22 deletions controllers/tokenrequest/tokenrequest.go

This file was deleted.

2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ require (
k8s.io/client-go v0.26.4
k8s.io/klog/v2 v2.100.1
k8s.io/mount-utils v0.26.4
k8s.io/utils v0.0.0-20221128185143-99ec85e7a448
monis.app/mlog v0.0.2
sigs.k8s.io/controller-runtime v0.14.6
)
Expand All @@ -29,6 +28,7 @@ require (
github.com/blang/semver/v4 v4.0.0 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
k8s.io/kube-openapi v0.0.0-20221012153701-172d655c2280 // indirect
k8s.io/utils v0.0.0-20221128185143-99ec85e7a448 // indirect
)

require (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ spec:
volumeLifecycleModes:
- Ephemeral
{{- if and (semverCompare ">=1.20-0" .Capabilities.KubeVersion.Version) .Values.tokenRequests }}
requiresRepublish: true
tokenRequests:
{{- toYaml .Values.tokenRequests | nindent 2 }}
{{- end }}

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

1 change: 1 addition & 0 deletions manifest_staging/deploy/csidriver.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ spec:
attachRequired: false
volumeLifecycleModes:
- Ephemeral
requiresRepublish: true
Loading