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

feat: implement secret adoption #633

Merged
merged 3 commits into from
Jul 16, 2024
Merged
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
16 changes: 16 additions & 0 deletions api/v1alpha1/amaltheasession_children.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,3 +237,19 @@ func (cr *AmaltheaSession) Pod(ctx context.Context, clnt client.Client) (*v1.Pod
err := clnt.Get(ctx, key, &pod)
return &pod, err
}

// Returns the list of all the secrets used in this CR
func (cr *AmaltheaSession) AllSecrets() v1.SecretList {
secrets := v1.SecretList{}

if cr.Spec.Ingress != nil && cr.Spec.Ingress.TLSSecretName != nil {
secrets.Items = append(secrets.Items, v1.Secret{
ObjectMeta: metav1.ObjectMeta{
Namespace: cr.Namespace,
Name: *cr.Spec.Ingress.TLSSecretName,
},
})
}

return secrets
}
Comment on lines +242 to +255
Copy link
Member

Choose a reason for hiding this comment

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

There could be more.

I looked around and there is:

  • CloningConfigSecretRef
  • ConfigSecretRef
  • For data sources there could be a secret too

But it is all the same to me we can add these in when we add the functionality for cloning code and data sources. Whatever you prefer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There's also the authentication related secret however I was thinking about adding them while we are implementing the corresponding features so that if for some reason we change the definition we don't have to change the code as well.

8 changes: 8 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,11 @@ rules:
- get
- patch
- update
- apiGroups:
- ""
resources:
- secrets
verbs:
- delete
- get
- list
52 changes: 51 additions & 1 deletion internal/controller/amaltheasession_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package controller

import (
"context"
"errors"
"time"

appsv1 "k8s.io/api/apps/v1"
Expand All @@ -28,6 +29,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/log"

amaltheadevv1alpha1 "github.com/SwissDataScienceCenter/amalthea/api/v1alpha1"
Expand All @@ -47,9 +49,13 @@ const (
typeDegradedAmaltheaSession = "Degraded"
)

// finalizers
const secretCleanupFinalizerName = "amalthea.dev/secrets-finalizer"

//+kubebuilder:rbac:groups=amalthea.dev,resources=amaltheasessions,verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups=amalthea.dev,resources=amaltheasessions/status,verbs=get;update;patch
//+kubebuilder:rbac:groups=amalthea.dev,resources=amaltheasessions/finalizers,verbs=update
//+kubebuilder:rbac:groups=core,resources=secrets,verbs=get;list;delete

// Reconcile is part of the main kubernetes reconciliation loop which aims to
// move the current state of the cluster closer to the desired state.
Expand Down Expand Up @@ -77,7 +83,17 @@ func (r *AmaltheaSessionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
return ctrl.Result{}, err
}

if amaltheasession.GetDeletionTimestamp() != nil {
if amaltheasession.GetDeletionTimestamp() == nil {
// The object is not being deleted, so if it does not have our finalizer,
// then lets add the finalizer and update the object. This is equivalent
// to registering our finalizer.
if amaltheasession.Spec.AdoptSecrets && !controllerutil.ContainsFinalizer(amaltheasession, secretCleanupFinalizerName) {
controllerutil.AddFinalizer(amaltheasession, secretCleanupFinalizerName)
if err := r.Update(ctx, amaltheasession); err != nil {
return ctrl.Result{}, err
}
}
} else {
amaltheasession.Status.State = amaltheadevv1alpha1.NotReady
err := r.Status().Update(ctx, amaltheasession)
if err != nil {
Expand All @@ -92,6 +108,20 @@ func (r *AmaltheaSessionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
return ctrl.Result{}, err
}
}

// The object is being deleted
if controllerutil.ContainsFinalizer(amaltheasession, secretCleanupFinalizerName) {
if err := r.deleteSecrets(ctx, amaltheasession); err != nil {
return ctrl.Result{}, err
}

// remove our finalizer from the list and update it.
controllerutil.RemoveFinalizer(amaltheasession, secretCleanupFinalizerName)
if err := r.Update(ctx, amaltheasession); err != nil {
return ctrl.Result{}, err
}
}

return ctrl.Result{Requeue: true}, nil
}

Expand Down Expand Up @@ -135,6 +165,26 @@ func (r *AmaltheaSessionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
return ctrl.Result{Requeue: true, RequeueAfter: time.Second * 10}, nil
}

func (r *AmaltheaSessionReconciler) deleteSecrets(ctx context.Context, cr *amaltheadevv1alpha1.AmaltheaSession) error {
if !cr.Spec.AdoptSecrets {
log := log.FromContext(ctx)
log.Info("Secret deletion finalizer called while not adopting secrets, doing nothing")
return nil
}

// create an initial empty error list
error_list := errors.Join(nil, nil)
for _, item := range cr.AllSecrets().Items {
err := r.Delete(ctx, &item)
if err != nil {
if !apierrors.IsNotFound(err) {
error_list = errors.Join(error_list, err)
}
}
}
return error_list
}

// SetupWithManager sets up the controller with the Manager.
func (r *AmaltheaSessionReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
Expand Down
87 changes: 87 additions & 0 deletions internal/controller/amaltheasession_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

amaltheadevv1alpha1 "github.com/SwissDataScienceCenter/amalthea/api/v1alpha1"
Expand Down Expand Up @@ -74,6 +75,7 @@ var _ = Describe("AmaltheaSession Controller", func() {
By("Cleanup the specific resource instance AmaltheaSession")
Expect(k8sClient.Delete(ctx, resource)).To(Succeed())
})

It("should successfully reconcile the resource", func() {
By("Reconciling the created resource")
controllerReconciler := &AmaltheaSessionReconciler{
Expand Down Expand Up @@ -130,6 +132,7 @@ var _ = Describe("AmaltheaSession Controller", func() {
By("Cleanup the specific resource instance AmaltheaSession")
Expect(k8sClient.Delete(ctx, resource)).To(Succeed())
})

It("should successfully reconcile the resource", func() {
By("Reconciling the created resource")
controllerReconciler := &AmaltheaSessionReconciler{
Expand All @@ -145,4 +148,88 @@ var _ = Describe("AmaltheaSession Controller", func() {
// Example: If you expect a certain status condition after reconciliation, verify it here.
})
})

Context("Adopting secrets", func() {
const resourceName = "test-resource"
const secretName = "test-secret"

ctx := context.Background()

typeNamespacedName := types.NamespacedName{
Name: resourceName,
Namespace: "default", // TODO(user):Modify as needed
}
amaltheasession := &amaltheadevv1alpha1.AmaltheaSession{}

secretNamespacedName := types.NamespacedName{
Name: secretName,
Namespace: "default", // TODO(user):Modify as needed
}
secret := &corev1.Secret{}

BeforeEach(func() {
tlsSecretName := secretName

amaltheasession = &amaltheadevv1alpha1.AmaltheaSession{
ObjectMeta: metav1.ObjectMeta{
Name: resourceName,
Namespace: "default",
},
Spec: amaltheadevv1alpha1.AmaltheaSessionSpec{
Session: amaltheadevv1alpha1.Session{
Image: "debian:bookworm-slim",
Port: 8000,
},
Ingress: &amaltheadevv1alpha1.Ingress{
Host: "test.com",
TLSSecretName: &tlsSecretName,
},
AdoptSecrets: false,
},
}

err := k8sClient.Get(ctx, secretNamespacedName, secret)
if err != nil && errors.IsNotFound(err) {
secret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
Namespace: "default",
},
}
Expect(k8sClient.Create(ctx, secret)).To(Succeed())
}
})

AfterEach(func() {
Expect(k8sClient.Delete(ctx, secret)).To(Succeed())
})

DescribeTable("Manage secrets",
func(adoptSecrets bool) {
By("Ensuring the session has the correct configuration")
amaltheasession.Spec.AdoptSecrets = adoptSecrets
err := k8sClient.Get(ctx, typeNamespacedName, amaltheasession)
if err != nil && errors.IsNotFound(err) {
Expect(k8sClient.Create(ctx, amaltheasession)).To(Succeed())
}

actual := amaltheadevv1alpha1.AmaltheaSession{}
Expect(k8sClient.Get(ctx, typeNamespacedName, &actual)).To(Succeed())
Expect(actual.Spec.AdoptSecrets).To(Equal(adoptSecrets))

By("Deleting the session")
Expect(k8sClient.Delete(ctx, &actual)).To(Succeed())

By("Checking the secret existence matches expectation")
err = k8sClient.Get(ctx, secretNamespacedName, secret)
if adoptSecrets {
Expect(errors.IsNotFound(err))
} else {
Expect(err).NotTo(HaveOccurred())
}
},
Entry("When secrets are adopted", true),
Entry("When secrets are not adopted", false),
)
})
})
Loading