From df3af1e07b79eebec0c453e26ebf67640b61d586 Mon Sep 17 00:00:00 2001 From: Jason van Brackel Date: Tue, 20 Jul 2021 17:28:47 -0400 Subject: [PATCH] feat: Add delete feature to groups controller --- controllers/fortify/suite_test.go | 1 - controllers/gitlab/group_controller.go | 97 ++++++++++++ controllers/gitlab/group_controller_test.go | 138 +++++++++++++++++- controllers/gitlab/mocks_test.go | 14 +- controllers/gitlab/suite_test.go | 1 - .../controllers/group_controller_test.go | 1 - 6 files changed, 242 insertions(+), 10 deletions(-) diff --git a/controllers/fortify/suite_test.go b/controllers/fortify/suite_test.go index 55e6164..f73cae2 100644 --- a/controllers/fortify/suite_test.go +++ b/controllers/fortify/suite_test.go @@ -65,7 +65,6 @@ var _ = BeforeSuite(func() { err = fortifyv1alpha1.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) - //+kubebuilder:scaffold:scheme k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) diff --git a/controllers/gitlab/group_controller.go b/controllers/gitlab/group_controller.go index 86da4e8..6c6da8f 100644 --- a/controllers/gitlab/group_controller.go +++ b/controllers/gitlab/group_controller.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "strconv" "time" apisGitlab "valkyrie.dso.mil/valkyrie-api/apis/gitlab" @@ -50,6 +51,11 @@ const ( errorCreatingGroupVariableInGitlab = "Error creating group variable in Gitlab" errorUpdatingGroupVariableInGitlab = "Error updating group variable in Gitlab" errorWhileUpdatingGroupState = "Error while updating group state in status" + errorAddingFinalizer = "Error while adding a finalizer" + errorRemovingFinalizer = "Error while removing a finalizer" + errorWhileDeletingGroup = "Error while deleting group" + errorWhileDeletingProjectsFromGroup = "Error while deleting project from group" + errorRunningGroupFinalizer = "Error running group deleting finalizer" ) // Group Status @@ -67,6 +73,8 @@ const ( const valkyrie = "valkyrie" +const groupFinalizerName = "valkyrie.dso.mil/valkyrie-api/apis/gitlab/group/finalizer" + // GroupReconciler reconciles the state of a Gitlab Group, for each reconciliation loop it will log in to Gitlab // with credentials in the secret provided. It will then check the state of the Group, and create it if necessary. type GroupReconciler struct { @@ -119,6 +127,42 @@ func (r *GroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl } } + // Delete the Group if required + // name of our custom finalizer + + // examine DeletionTimestamp to determine if object is under deletion + if group.ObjectMeta.DeletionTimestamp.IsZero() { + // 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 + // registering our finalizer. + if !containsString(group.GetFinalizers(), groupFinalizerName) { + controllerutil.AddFinalizer(group, groupFinalizerName) + if err = r.Update(ctx, group); err != nil { + r.Log.Error(err, errorAddingFinalizer) + return ctrl.Result{}, err + } + } + } else { + // The object is being deleted + if containsString(group.GetFinalizers(), groupFinalizerName) { + // our finalizer is present, so lets handle any external dependency + if err = r.deleteProjectsAndGitlabGroup(ctx, group); err != nil { + r.Log.Error(err, errorRunningGroupFinalizer) + return ctrl.Result{}, err + } + + // remove our finalizer from the list and update it. + controllerutil.RemoveFinalizer(group, groupFinalizerName) + if err = r.Update(ctx, group); err != nil { + r.Log.Error(err, errorRemovingFinalizer) + return ctrl.Result{}, err + } + } + + // Stop reconciliation as the item is being deleted + return ctrl.Result{}, nil + } + // See if the Group Exists var gitlabGroup *gitlab.Group found, gitlabGroup, err := r.groupExists(group) @@ -240,6 +284,23 @@ func (r *GroupReconciler) processProjects(ctx context.Context, group *gitlabv1al return nil } +func (r *GroupReconciler) deleteProjects(ctx context.Context, group *gitlabv1alpha1.Group, specs []gitlabv1alpha1.ProjectSpec) error { + for _, projectSpec := range specs { + // if it's not found we'll consider it gone and not worry about it. + if project, foundProject, err := r.getProject(ctx, group, projectSpec); err != nil { + r.Log.Error(err, errorWhileDeletingProjectsFromGroup, "name", projectSpec.Name) + return err + } else if foundProject { + project.ObjectMeta.DeletionTimestamp = &metav1.Time{Time: time.Now()} + if _, err = r.updateProject(ctx, 0, projectSpec, project); err != nil { + r.Log.Error(err, errorWhileDeletingProjectsFromGroup, "project", project) + return err + } + } + } + return nil +} + func (r *GroupReconciler) getProject(ctx context.Context, group *gitlabv1alpha1.Group, spec gitlabv1alpha1.ProjectSpec) (*gitlabv1alpha1.Project, bool, error) { var project gitlabv1alpha1.Project projectName := types.NamespacedName{ @@ -400,3 +461,39 @@ func (r *GroupReconciler) updateState(ctx context.Context, group *gitlabv1alpha1 } return nil } + +func (r *GroupReconciler) deleteProjectsAndGitlabGroup(ctx context.Context, group *gitlabv1alpha1.Group) error { + var id int + var err error + + if id, err = strconv.Atoi(group.Annotations[annotationKeyID]); err != nil { + r.Log.Error(err, errorWhileDeletingGroup) + return err + } + if err = r.updateState(ctx, group, errorWhileDeletingGroup); err != nil { + return err + } + + var statusCode int + if statusCode, err = r.gitlabClient.DeleteGroup(id, 0, 0); err != nil { + r.Log.Error(err, errorWhileDeletingGroup, "statusCode", statusCode) + return err + } + + if err = r.deleteProjects(ctx, group, group.Spec.ProjectSpecs); err != nil { + r.Log.Error(err, errorWhileDeletingProjectsFromGroup) + return err + } + + return nil +} + +// Helper functions to check and remove string from a slice of strings. +func containsString(slice []string, s string) bool { + for _, item := range slice { + if item == s { + return true + } + } + return false +} diff --git a/controllers/gitlab/group_controller_test.go b/controllers/gitlab/group_controller_test.go index 0865a03..dba425d 100755 --- a/controllers/gitlab/group_controller_test.go +++ b/controllers/gitlab/group_controller_test.go @@ -12,7 +12,7 @@ import ( "strconv" "testing" "time" - gitlab2 "valkyrie.dso.mil/valkyrie-api/apis/gitlab" + apisGitlab "valkyrie.dso.mil/valkyrie-api/apis/gitlab" gitlabv1alpha1 "valkyrie.dso.mil/valkyrie-api/apis/gitlab/v1alpha1" gitlabClient "valkyrie.dso.mil/valkyrie-api/clients/gitlab" ) @@ -23,6 +23,12 @@ const ( greenGroupName = "greenGroup" ) +const ( + toDeleteGreenGroupID = 3 + toDeleteGreenGroupNamespace = "default" + toDeleteGreenGroupName = "greenDelete" +) + func getGreenGroupControllerRequest() ctrl.Request { return ctrl.Request{ NamespacedName: types.NamespacedName{ @@ -32,6 +38,15 @@ func getGreenGroupControllerRequest() ctrl.Request { } } +func getGreenGroupControllerDeleteRequest() ctrl.Request { + return ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: toDeleteGreenGroupNamespace, + Name: toDeleteGreenGroupName, + }, + } +} + func getGreenGroup() *gitlabv1alpha1.Group { returnGroup := &gitlabv1alpha1.Group{ TypeMeta: metav1.TypeMeta{}, @@ -86,13 +101,19 @@ func getGroupControllerWithMocksInGreenTestState() (*GroupReconciler, *MockLogge clientMock := MockClient{statusWriter: &MockStatusWriter{}} clientMock.expectedObjects = make(map[client.ObjectKey]client.Object) greenGroup := getGreenGroup() + toDeleteGroup := getToDeleteGroup() clientMock.expectedObjects[types.NamespacedName{ Namespace: greenGroupNamespace, Name: greenGroupName, }] = greenGroup + clientMock.expectedObjects[types.NamespacedName{ + Namespace: toDeleteGreenGroupNamespace, + Name: toDeleteGreenGroupName, + }] = toDeleteGroup expectedGitlabGroups := make(map[int]*gitlab.Group) expectedGitlabGroups[greenGroupID] = getGreenGitlabGroup() + expectedGitlabGroups[toDeleteGreenGroupID] = getGreentoDeleteGitlabGroup() gitlabClientMock := MockGitlabClient{ expectedGroups: expectedGitlabGroups, } @@ -111,7 +132,18 @@ func getGroupControllerWithMocksInGreenTestState() (*GroupReconciler, *MockLogge EnvironmentScope: &environmentScope, } - gitlabClientMock.AddGroupVariable(greenGroupID, options) + _, _, _ = gitlabClientMock.AddGroupVariable(greenGroupID, options) + + options = gitlab.CreateGroupVariableOptions{ + Key: &key, + Value: &toDeleteGroup.Spec.ManifestRepositoryURL, + VariableType: &variableType, + Protected: &protected, + Masked: &masked, + EnvironmentScope: &environmentScope, + } + + _, _, _ = gitlabClientMock.AddGroupVariable(toDeleteGreenGroupID, options) configurationMock := MockClientConfiguration{ GitlabClient: &gitlabClientMock, @@ -127,6 +159,50 @@ func getGroupControllerWithMocksInGreenTestState() (*GroupReconciler, *MockLogge return &sut, &loggerMock, &configurationMock, &clientMock, &gitlabClientMock } +func getToDeleteGroup() *gitlabv1alpha1.Group { + toDeleteGroup := &gitlabv1alpha1.Group{ + TypeMeta: metav1.TypeMeta{}, + ObjectMeta: metav1.ObjectMeta{ + Annotations: make(map[string]string), + }, + Spec: gitlabv1alpha1.GroupSpec{ + FullPath: "/greenDeleteGroup", + Name: "greenDeleteGroup", + Description: "a green Group to Delete", + GitlabCredentialsName: "greenCredentialsName", + KustomizeProductionPath: "/kustomize/prod/path", + KustomizeStagingPath: "/kustomize/staging/path", + ManifestRepositoryURL: "https://gitserver/manifest/repo/url", + ProjectSpecs: make([]gitlabv1alpha1.ProjectSpec, 0), + }, + Status: gitlabv1alpha1.GroupStatus{ + CreatedTime: metav1.Time{ + Time: time.Now(), + }, + LastUpdatedTime: metav1.Time{ + Time: time.Now(), + }, + State: "", + }, + } + toDeleteGroup.Spec.Name = toDeleteGreenGroupName + toDeleteGroup.Namespace = toDeleteGreenGroupNamespace + toDeleteGroup.Spec.FullPath = "/greenDeleteGroup" + toDeleteGroup.Spec.ProjectSpecs = append(toDeleteGroup.Spec.ProjectSpecs, gitlabv1alpha1.ProjectSpec{ + GroupID: toDeleteGreenGroupID, + Name: "A Green Project to Delete", + FullPath: "/greenGroup/greenProjectToDelete", + GitlabCredentialsName: "greenCredentialsName", + ImpactLevel: "2", + StorageTypes: nil, + VirtualService: "", + ServicePort: 0, + Language: "", + }) + toDeleteGroup.ObjectMeta.Annotations[annotationKeyID] = strconv.Itoa(toDeleteGreenGroupID) + return toDeleteGroup +} + func getGreenGitlabGroup() *gitlab.Group { return &gitlab.Group{ ID: greenGroupID, @@ -136,9 +212,19 @@ func getGreenGitlabGroup() *gitlab.Group { } } +func getGreentoDeleteGitlabGroup() *gitlab.Group { + return &gitlab.Group{ + ID: toDeleteGreenGroupID, + Path: "/greenDelete", + Name: toDeleteGreenGroupName, + Description: "a green Group to Delete", + } +} + var _ = Describe("Reconcile", func() { getGroupControllerWithMocksInGreenTestState() Context("green state", func() { + // Create Update Operations sut, _, _, _, _ := getGroupControllerWithMocksInGreenTestState() result, err := sut.Reconcile(context.TODO(), getGreenGroupControllerRequest()) @@ -148,6 +234,36 @@ var _ = Describe("Reconcile", func() { It("should not return an error", func() { Expect(err).To(BeNil()) }) + + // Update Operation (Add Finalizer) + ctx := context.TODO() + req := getGreenGroupControllerDeleteRequest() + result, err = sut.Reconcile(ctx, req) + + It("should not request a requeue", func() { + Expect(result).To(Equal(ctrl.Result{})) + }) + It("should not return an error", func() { + Expect(err).To(BeNil()) + }) + + // Finish the Delete + group := sut.Client.(*MockClient).expectedObjects[getGreenGroupControllerDeleteRequest().NamespacedName].(*gitlabv1alpha1.Group) + group.SetDeletionTimestamp(&metav1.Time{Time: time.Now()}) + group.Finalizers = append(group.Finalizers, groupFinalizerName) + result, err = sut.Reconcile(context.TODO(), getGreenGroupControllerDeleteRequest()) + + It("should not request a requeue", func() { + Expect(result).To(Equal(ctrl.Result{})) + }) + It("should not return an error", func() { + Expect(err).To(BeNil()) + }) + It("should delete the Group from gitlab, that's supposed to be deleted.", func() { + group, found := sut.gitlabClient.(*MockGitlabClient).expectedGroups[toDeleteGreenGroupID] + Expect(found).To(BeFalse()) + Expect(group).To(BeNil()) + }) }) Context("supports client configuration mocking", func() { sut, _, _, _, _ := getGroupControllerWithMocksInGreenTestState() @@ -156,7 +272,7 @@ var _ = Describe("Reconcile", func() { sut.gitlabClientConfiguration = nil sut.Reconcile(context.TODO(), getGreenGroupControllerRequest()) It("should use the default client configuration implementation", func() { - Expect(sut.gitlabClientConfiguration).To(BeAssignableToTypeOf(gitlab2.ClientConfigurationImpl{})) + Expect(sut.gitlabClientConfiguration).To(BeAssignableToTypeOf(apisGitlab.ClientConfigurationImpl{})) }) }) Context("the API Object isn't found", func() { @@ -344,12 +460,22 @@ var _ = Describe("Reconcile", func() { sut, logger, _, kubernetesClient, _ := getGroupControllerWithMocksInGreenTestState() kubernetesClient.createFunction = func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { - return &MockError{message: "processProjects Fails"} + switch obj.(type) { + case *gitlabv1alpha1.Project: + return &MockError{message: "processProjects Fails"} + } + return nil } kubernetesClient.UpdateFunction = func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return &MockError{message: "processProjects Fails"} + switch obj.(type) { + case *gitlabv1alpha1.Project: + return &MockError{message: "processProjects Fails"} + } + return nil } - result, err := sut.Reconcile(context.TODO(), getGreenGroupControllerRequest()) + ctx := context.TODO() + req := getGreenGroupControllerRequest() + result, err := sut.Reconcile(ctx, req) It("should requeue the object", func() { Expect(result).To(Equal(ctrl.Result{Requeue: true})) }) diff --git a/controllers/gitlab/mocks_test.go b/controllers/gitlab/mocks_test.go index 0c1393f..dce556f 100755 --- a/controllers/gitlab/mocks_test.go +++ b/controllers/gitlab/mocks_test.go @@ -437,6 +437,7 @@ type MockGitlabClient struct { updateGroupVariableFunction func(groupID int, key string, options gitlab.UpdateGroupVariableOptions) (*gitlab.GroupVariable, int, error) updateProjectVariableFunction func(groupID int, key string, options gitlab.UpdateProjectVariableOptions) (*gitlab.ProjectVariable, int, error) getProjectVariableFunction func(projectID int, key string) (*gitlab.ProjectVariable, int, error) + deleteGroupFunction func(groupID int, waitInterval int, waitCount int) (int, error) } func (m *MockGitlabClient) GetGroupVariable(groupID int, key string) (*gitlab.GroupVariable, int, error) { @@ -614,6 +615,7 @@ func (m *MockGitlabClient) DeleteProjectVariable(projectID int, key string, wait if foundVariable != nil { delete(m.expectedProjectVariables, key) + } return 200, nil @@ -682,7 +684,17 @@ func (m *MockGitlabClient) AddGroupMember(groupID *int, userID *int, accessLevel } func (m *MockGitlabClient) DeleteGroup(groupID int, waitInterval int, waitCount int) (int, error) { - panic("implement me") + if m.deleteGroupFunction != nil { + return m.deleteGroupFunction(groupID, waitInterval, waitCount) + } + + if m.expectedGroups == nil { + m.expectedGroups = make(map[int]*gitlab.Group) + } + + delete(m.expectedGroups, groupID) + + return groupID, nil } func (m *MockGitlabClient) GetProject(projectID int) (*gitlab.Project, int, error) { diff --git a/controllers/gitlab/suite_test.go b/controllers/gitlab/suite_test.go index 593a551..0bd5660 100644 --- a/controllers/gitlab/suite_test.go +++ b/controllers/gitlab/suite_test.go @@ -67,7 +67,6 @@ var _ = BeforeSuite(func() { err = gitlabv1alpha1.AddToScheme(scheme.Scheme) Expect(err).NotTo(HaveOccurred()) - //+kubebuilder:scaffold:scheme k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) diff --git a/integration-tests/controllers/group_controller_test.go b/integration-tests/controllers/group_controller_test.go index 083d62a..2d32936 100755 --- a/integration-tests/controllers/group_controller_test.go +++ b/integration-tests/controllers/group_controller_test.go @@ -1,2 +1 @@ package controllers - -- GitLab