From 1b9ccebbc5ae8a7f11f8f975178cdee85bf6d0c7 Mon Sep 17 00:00:00 2001 From: kkb0318 Date: Thu, 11 Apr 2024 20:59:17 +0900 Subject: [PATCH] secret deletion --- internal/controller/irsasetup_controller.go | 25 ++++++++++- .../controller/irsasetup_controller_test.go | 44 ++++++++++++------- internal/controller/suite_test.go | 24 ++++++++++ internal/handler/handler.go | 3 +- internal/handler/kubernetes.go | 13 ++++++ internal/kubernetes/apply.go | 25 +---------- internal/kubernetes/delete.go | 24 +++------- internal/kubernetes/unstructured.go | 26 +++++++++++ internal/manifests/keys_secret.go | 5 --- internal/selfhosted/oidc.go | 2 + internal/selfhosted/oidc/id_provider.go | 5 +++ .../selfhosted/oidc/id_provider_discovery.go | 6 +++ internal/selfhosted/selfhosted.go | 18 ++++++++ 13 files changed, 155 insertions(+), 65 deletions(-) create mode 100644 internal/kubernetes/unstructured.go diff --git a/internal/controller/irsasetup_controller.go b/internal/controller/irsasetup_controller.go index 44df1d6..8bc0e3c 100644 --- a/internal/controller/irsasetup_controller.go +++ b/internal/controller/irsasetup_controller.go @@ -77,7 +77,10 @@ func (r *IRSASetupReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } if !obj.DeletionTimestamp.IsZero() { - err = r.reconcileDelete(ctx, obj) + err = r.reconcileDelete(ctx, obj, kubeClient) + if err == nil { + log.Info("successfully deleted") + } return ctrl.Result{}, err } @@ -94,7 +97,25 @@ func (r *IRSASetupReconciler) reconcile(ctx context.Context, obj *irsav1alpha1.I return err } -func (r *IRSASetupReconciler) reconcileDelete(ctx context.Context, obj *irsav1alpha1.IRSASetup) error { +func (r *IRSASetupReconciler) reconcileDelete(ctx context.Context, obj *irsav1alpha1.IRSASetup, kubeClient *kubernetes.KubernetesClient) error { + factory, err := newOIDCIdpFactory(ctx, obj, nil, r.AwsClient) + if err != nil { + return err + } + secret, err := manifests.NewSecretBuilder().Build("name", "default") + if err != nil { + return err + } + kubeHandler := handler.NewKubernetesHandler(kubeClient) + kubeHandler.Append(secret) + err = selfhosted.Delete(ctx, factory) + if err != nil { + return err + } + err = kubeHandler.DeleteAll(ctx) + if err != nil { + return err + } return nil } diff --git a/internal/controller/irsasetup_controller_test.go b/internal/controller/irsasetup_controller_test.go index 856781b..915ac5b 100644 --- a/internal/controller/irsasetup_controller_test.go +++ b/internal/controller/irsasetup_controller_test.go @@ -38,13 +38,16 @@ var _ = Describe("IRSASetup Controller", func() { Context("When reconciling a resource", func() { const resourceName = "test-resource" - ctx := context.Background() - typeNamespacedName := types.NamespacedName{ Name: resourceName, - Namespace: "default", // TODO(user):Modify as needed + Namespace: "default", + } + irsasetup := &irsav1alpha1.IRSASetup{ + ObjectMeta: metav1.ObjectMeta{ + Name: resourceName, + Namespace: "default", + }, } - irsasetup := &irsav1alpha1.IRSASetup{} BeforeEach(func() { By("creating the custom resource for the Kind IRSASetup") @@ -69,17 +72,12 @@ var _ = Describe("IRSASetup Controller", func() { } }) - AfterEach(func() { - // TODO(user): Cleanup logic after each test, like removing the resource instance. - resource := &irsav1alpha1.IRSASetup{} - err := k8sClient.Get(ctx, typeNamespacedName, resource) - Expect(err).NotTo(HaveOccurred()) - - By("Cleanup the specific resource instance IRSASetup") - Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) - }) It("should successfully reconcile the resource", func() { awsClient := newMockAwsClient() + expected := []types.NamespacedName{ + {Name: "name", Namespace: "default"}, + } + By("Reconciling the created resource") controllerReconciler := &IRSASetupReconciler{ Client: k8sClient, @@ -91,8 +89,24 @@ var _ = Describe("IRSASetup Controller", func() { NamespacedName: typeNamespacedName, }) Expect(err).NotTo(HaveOccurred()) - // TODO(user): Add more specific assertions depending on your controller's reconciliation logic. - // Example: If you expect a certain status condition after reconciliation, verify it here. + _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + for _, expect := range expected { + checkExist(expect, newSecret) + } + By("removing the custom resource for the Kind") + Eventually(func() error { + return k8sClient.Delete(ctx, irsasetup) + }, timeout).Should(Succeed()) + _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).To(Not(HaveOccurred())) + for _, expect := range expected { + checkNoExist(expect, newSecret) + } }) }) }) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index a4d5ba5..966c15b 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -21,12 +21,16 @@ import ( "path/filepath" "runtime" "testing" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -43,6 +47,8 @@ var ( cfg *rest.Config k8sClient client.Client testEnv *envtest.Environment + timeout = time.Second * 10 + ctx = ctrl.SetupSignalHandler() ) func TestControllers(t *testing.T) { @@ -89,3 +95,21 @@ var _ = AfterSuite(func() { err := testEnv.Stop() Expect(err).NotTo(HaveOccurred()) }) + +func checkExist(expected types.NamespacedName, newFunc func() client.Object) { + Eventually(func() error { + found := newFunc() + return k8sClient.Get(ctx, expected, found) + }, timeout).Should(Succeed()) +} + +func checkNoExist(expected types.NamespacedName, newFunc func() client.Object) { + Eventually(func() error { + found := newFunc() + return k8sClient.Get(ctx, expected, found) + }, timeout).Should(Not(Succeed())) +} + +func newSecret() client.Object { + return &corev1.Secret{} +} diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 639dfc3..a33488d 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -4,13 +4,12 @@ import ( "context" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "sigs.k8s.io/controller-runtime/pkg/client" ) type KubernetesClient interface { Apply(ctx context.Context, obj client.Object) error - Delete(ctx context.Context, obj *unstructured.Unstructured, opts DeleteOptions) error + Delete(ctx context.Context, obj client.Object, opts DeleteOptions) error } // DeleteOptions contains options for delete requests. diff --git a/internal/handler/kubernetes.go b/internal/handler/kubernetes.go index e8fa60e..b639e65 100644 --- a/internal/handler/kubernetes.go +++ b/internal/handler/kubernetes.go @@ -3,6 +3,7 @@ package handler import ( "context" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -31,3 +32,15 @@ func (k *KubernetesHandler) ApplyAll(ctx context.Context) error { } return nil } + +func (k *KubernetesHandler) DeleteAll(ctx context.Context) error { + for _, obj := range k.objs { + err := k.client.Delete(ctx, obj, DeleteOptions{ + DeletionPropagation: metav1.DeletePropagationBackground, + }) + if err != nil { + return err + } + } + return nil +} diff --git a/internal/kubernetes/apply.go b/internal/kubernetes/apply.go index 3ea57bf..4050216 100644 --- a/internal/kubernetes/apply.go +++ b/internal/kubernetes/apply.go @@ -3,11 +3,7 @@ package kubernetes import ( "context" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/client/apiutil" ) func NewKubernetesClient(c client.Client, owner Owner) (*KubernetesClient, error) { @@ -22,19 +18,10 @@ func (h KubernetesClient) Apply(ctx context.Context, obj client.Object) error { client.ForceOwnership, client.FieldOwner(h.owner.Field), } - gvk, err := apiutil.GVKForObject(obj, h.client.Scheme()) + u, err := h.toUnstructured(obj) if err != nil { return err } - - u := &unstructured.Unstructured{} - unstructured, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) - if err != nil { - return err - } - u.Object = unstructured - u.SetGroupVersionKind(gvk) - u.SetManagedFields(nil) err = h.client.Patch(ctx, u, client.Apply, opts...) if err != nil { return err @@ -48,18 +35,10 @@ func (h KubernetesClient) PatchStatus(ctx context.Context, obj client.Object) er FieldManager: h.owner.Field, }, } - gvk, err := apiutil.GVKForObject(obj, h.client.Scheme()) + u, err := h.toUnstructured(obj) if err != nil { return err } - u := &unstructured.Unstructured{} - unstructured, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) - if err != nil { - return err - } - u.Object = unstructured - u.SetGroupVersionKind(gvk) - u.SetManagedFields(nil) return h.client.Status().Patch(ctx, u, client.Apply, opts) } diff --git a/internal/kubernetes/delete.go b/internal/kubernetes/delete.go index aaf3bd8..c535bb7 100644 --- a/internal/kubernetes/delete.go +++ b/internal/kubernetes/delete.go @@ -14,27 +14,15 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ) -func (h *KubernetesClient) DeleteAll(ctx context.Context, resources []*unstructured.Unstructured, opts handler.DeleteOptions) error { - if !h.cleanup { - return nil - } - for _, r := range resources { - err := h.Delete(ctx, r, opts) - if err != nil { - return err - } - } - return nil -} - // Delete deletes the given object (not found errors are ignored). -func (h *KubernetesClient) Delete(ctx context.Context, object *unstructured.Unstructured, opts handler.DeleteOptions) error { - if !h.cleanup { - return nil +func (h *KubernetesClient) Delete(ctx context.Context, obj client.Object, opts handler.DeleteOptions) error { + u, err := h.toUnstructured(obj) + if err != nil { + return err } existingObject := &unstructured.Unstructured{} - existingObject.SetGroupVersionKind(object.GroupVersionKind()) - err := h.client.Get(ctx, client.ObjectKeyFromObject(object), existingObject) + existingObject.SetGroupVersionKind(u.GroupVersionKind()) + err = h.client.Get(ctx, client.ObjectKeyFromObject(u), existingObject) if err != nil { if !errors.IsNotFound(err) { return fmt.Errorf("failed to delete: %w", err) diff --git a/internal/kubernetes/unstructured.go b/internal/kubernetes/unstructured.go new file mode 100644 index 0000000..fd77d15 --- /dev/null +++ b/internal/kubernetes/unstructured.go @@ -0,0 +1,26 @@ +package kubernetes + +import ( + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" +) + +func (h KubernetesClient) toUnstructured(obj client.Object) (*unstructured.Unstructured, error) { + gvk, err := apiutil.GVKForObject(obj, h.client.Scheme()) + if err != nil { + return nil, err + } + + u := &unstructured.Unstructured{} + unstructured, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) + if err != nil { + return nil, err + } + u.Object = unstructured + u.SetGroupVersionKind(gvk) + u.SetManagedFields(nil) + return u, nil +} diff --git a/internal/manifests/keys_secret.go b/internal/manifests/keys_secret.go index 9b74bb9..14f06ba 100644 --- a/internal/manifests/keys_secret.go +++ b/internal/manifests/keys_secret.go @@ -1,8 +1,6 @@ package manifests import ( - "fmt" - "github.com/kkb0318/irsa-manager/internal/selfhosted" corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -29,9 +27,6 @@ func (b *SecretBuilder) WithSSHKey(keyPair selfhosted.KeyPair) *SecretBuilder { } func (b *SecretBuilder) Build(name, ns string) (*corev1.Secret, error) { - if b.data == nil { - return nil, fmt.Errorf("Secret.Data is empty") - } secret := &corev1.Secret{ ObjectMeta: v1.ObjectMeta{ Name: name, diff --git a/internal/selfhosted/oidc.go b/internal/selfhosted/oidc.go index 8c77fa7..ab84cae 100644 --- a/internal/selfhosted/oidc.go +++ b/internal/selfhosted/oidc.go @@ -11,6 +11,7 @@ type OIDCIdP interface { Create(ctx context.Context) (string, error) IsUpdate() (bool, error) Update(ctx context.Context) error + Delete(ctx context.Context) error } type OIDCIdPDiscoveryContents interface { @@ -22,6 +23,7 @@ type OIDCIdPDiscoveryContents interface { type OIDCIdPDiscovery interface { CreateStorage(ctx context.Context) error Upload(ctx context.Context, o OIDCIdPDiscoveryContents) error + DeleteStorage(ctx context.Context) error } type OIDCIdPFactory interface { diff --git a/internal/selfhosted/oidc/id_provider.go b/internal/selfhosted/oidc/id_provider.go index 58262e0..1ed3b0b 100644 --- a/internal/selfhosted/oidc/id_provider.go +++ b/internal/selfhosted/oidc/id_provider.go @@ -32,3 +32,8 @@ func (a *AwsIdP) Update(ctx context.Context) error { func (a *AwsIdP) IsUpdate() (bool, error) { return false, nil } + +func (a *AwsIdP) Delete(ctx context.Context) error { + // TODO: + return nil +} diff --git a/internal/selfhosted/oidc/id_provider_discovery.go b/internal/selfhosted/oidc/id_provider_discovery.go index 2e0844e..bcc8b7d 100644 --- a/internal/selfhosted/oidc/id_provider_discovery.go +++ b/internal/selfhosted/oidc/id_provider_discovery.go @@ -59,3 +59,9 @@ func (s *S3IdPDiscovery) Upload(ctx context.Context, o selfhosted.OIDCIdPDiscove } return nil } + +// DeleteStorage delete an S3 bucket +func (s *S3IdPDiscovery) DeleteStorage(ctx context.Context) error { + // TODO: + return nil +} diff --git a/internal/selfhosted/selfhosted.go b/internal/selfhosted/selfhosted.go index 635c7b4..af639ea 100644 --- a/internal/selfhosted/selfhosted.go +++ b/internal/selfhosted/selfhosted.go @@ -24,3 +24,21 @@ func Execute(ctx context.Context, factory OIDCIdPFactory) error { } return nil } + +func Delete(ctx context.Context, factory OIDCIdPFactory) error { + issuerMeta := factory.IssuerMeta() + discovery := factory.IdPDiscovery() + idp, err := factory.IdP(issuerMeta) + if err != nil { + return err + } + err = discovery.DeleteStorage(ctx) + if err != nil { + return err + } + err = idp.Delete(ctx) + if err != nil { + return err + } + return nil +}