UNCLASSIFIED

Commit e6380b06 authored by abrichards's avatar abrichards
Browse files

Merge branch 'dev-jvb-719-delete-group' into 'master'

feat: Add delete feature to groups controller

See merge request !58
parents 0f5ed144 df3af1e0
...@@ -65,7 +65,6 @@ var _ = BeforeSuite(func() { ...@@ -65,7 +65,6 @@ var _ = BeforeSuite(func() {
err = fortifyv1alpha1.AddToScheme(scheme.Scheme) err = fortifyv1alpha1.AddToScheme(scheme.Scheme)
Expect(err).NotTo(HaveOccurred()) Expect(err).NotTo(HaveOccurred())
//+kubebuilder:scaffold:scheme //+kubebuilder:scaffold:scheme
k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
......
...@@ -26,6 +26,7 @@ import ( ...@@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime" ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"strconv" "strconv"
"time" "time"
apisGitlab "valkyrie.dso.mil/valkyrie-api/apis/gitlab" apisGitlab "valkyrie.dso.mil/valkyrie-api/apis/gitlab"
...@@ -50,6 +51,11 @@ const ( ...@@ -50,6 +51,11 @@ const (
errorCreatingGroupVariableInGitlab = "Error creating group variable in Gitlab" errorCreatingGroupVariableInGitlab = "Error creating group variable in Gitlab"
errorUpdatingGroupVariableInGitlab = "Error updating group variable in Gitlab" errorUpdatingGroupVariableInGitlab = "Error updating group variable in Gitlab"
errorWhileUpdatingGroupState = "Error while updating group state in status" 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 // Group Status
...@@ -67,6 +73,8 @@ const ( ...@@ -67,6 +73,8 @@ const (
const valkyrie = "valkyrie" 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 // 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. // with credentials in the secret provided. It will then check the state of the Group, and create it if necessary.
type GroupReconciler struct { type GroupReconciler struct {
...@@ -119,6 +127,42 @@ func (r *GroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl ...@@ -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 // See if the Group Exists
var gitlabGroup *gitlab.Group var gitlabGroup *gitlab.Group
found, gitlabGroup, err := r.groupExists(group) found, gitlabGroup, err := r.groupExists(group)
...@@ -240,6 +284,23 @@ func (r *GroupReconciler) processProjects(ctx context.Context, group *gitlabv1al ...@@ -240,6 +284,23 @@ func (r *GroupReconciler) processProjects(ctx context.Context, group *gitlabv1al
return nil 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) { func (r *GroupReconciler) getProject(ctx context.Context, group *gitlabv1alpha1.Group, spec gitlabv1alpha1.ProjectSpec) (*gitlabv1alpha1.Project, bool, error) {
var project gitlabv1alpha1.Project var project gitlabv1alpha1.Project
projectName := types.NamespacedName{ projectName := types.NamespacedName{
...@@ -400,3 +461,39 @@ func (r *GroupReconciler) updateState(ctx context.Context, group *gitlabv1alpha1 ...@@ -400,3 +461,39 @@ func (r *GroupReconciler) updateState(ctx context.Context, group *gitlabv1alpha1
} }
return nil 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
}
...@@ -12,7 +12,7 @@ import ( ...@@ -12,7 +12,7 @@ import (
"strconv" "strconv"
"testing" "testing"
"time" "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" gitlabv1alpha1 "valkyrie.dso.mil/valkyrie-api/apis/gitlab/v1alpha1"
gitlabClient "valkyrie.dso.mil/valkyrie-api/clients/gitlab" gitlabClient "valkyrie.dso.mil/valkyrie-api/clients/gitlab"
) )
...@@ -23,6 +23,12 @@ const ( ...@@ -23,6 +23,12 @@ const (
greenGroupName = "greenGroup" greenGroupName = "greenGroup"
) )
const (
toDeleteGreenGroupID = 3
toDeleteGreenGroupNamespace = "default"
toDeleteGreenGroupName = "greenDelete"
)
func getGreenGroupControllerRequest() ctrl.Request { func getGreenGroupControllerRequest() ctrl.Request {
return ctrl.Request{ return ctrl.Request{
NamespacedName: types.NamespacedName{ NamespacedName: types.NamespacedName{
...@@ -32,6 +38,15 @@ func getGreenGroupControllerRequest() ctrl.Request { ...@@ -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 { func getGreenGroup() *gitlabv1alpha1.Group {
returnGroup := &gitlabv1alpha1.Group{ returnGroup := &gitlabv1alpha1.Group{
TypeMeta: metav1.TypeMeta{}, TypeMeta: metav1.TypeMeta{},
...@@ -86,13 +101,19 @@ func getGroupControllerWithMocksInGreenTestState() (*GroupReconciler, *MockLogge ...@@ -86,13 +101,19 @@ func getGroupControllerWithMocksInGreenTestState() (*GroupReconciler, *MockLogge
clientMock := MockClient{statusWriter: &MockStatusWriter{}} clientMock := MockClient{statusWriter: &MockStatusWriter{}}
clientMock.expectedObjects = make(map[client.ObjectKey]client.Object) clientMock.expectedObjects = make(map[client.ObjectKey]client.Object)
greenGroup := getGreenGroup() greenGroup := getGreenGroup()
toDeleteGroup := getToDeleteGroup()
clientMock.expectedObjects[types.NamespacedName{ clientMock.expectedObjects[types.NamespacedName{
Namespace: greenGroupNamespace, Namespace: greenGroupNamespace,
Name: greenGroupName, Name: greenGroupName,
}] = greenGroup }] = greenGroup
clientMock.expectedObjects[types.NamespacedName{
Namespace: toDeleteGreenGroupNamespace,
Name: toDeleteGreenGroupName,
}] = toDeleteGroup
expectedGitlabGroups := make(map[int]*gitlab.Group) expectedGitlabGroups := make(map[int]*gitlab.Group)
expectedGitlabGroups[greenGroupID] = getGreenGitlabGroup() expectedGitlabGroups[greenGroupID] = getGreenGitlabGroup()
expectedGitlabGroups[toDeleteGreenGroupID] = getGreentoDeleteGitlabGroup()
gitlabClientMock := MockGitlabClient{ gitlabClientMock := MockGitlabClient{
expectedGroups: expectedGitlabGroups, expectedGroups: expectedGitlabGroups,
} }
...@@ -111,7 +132,18 @@ func getGroupControllerWithMocksInGreenTestState() (*GroupReconciler, *MockLogge ...@@ -111,7 +132,18 @@ func getGroupControllerWithMocksInGreenTestState() (*GroupReconciler, *MockLogge
EnvironmentScope: &environmentScope, 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{ configurationMock := MockClientConfiguration{
GitlabClient: &gitlabClientMock, GitlabClient: &gitlabClientMock,
...@@ -127,6 +159,50 @@ func getGroupControllerWithMocksInGreenTestState() (*GroupReconciler, *MockLogge ...@@ -127,6 +159,50 @@ func getGroupControllerWithMocksInGreenTestState() (*GroupReconciler, *MockLogge
return &sut, &loggerMock, &configurationMock, &clientMock, &gitlabClientMock 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 { func getGreenGitlabGroup() *gitlab.Group {
return &gitlab.Group{ return &gitlab.Group{
ID: greenGroupID, ID: greenGroupID,
...@@ -136,9 +212,19 @@ func getGreenGitlabGroup() *gitlab.Group { ...@@ -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() { var _ = Describe("Reconcile", func() {
getGroupControllerWithMocksInGreenTestState() getGroupControllerWithMocksInGreenTestState()
Context("green state", func() { Context("green state", func() {
// Create Update Operations
sut, _, _, _, _ := getGroupControllerWithMocksInGreenTestState() sut, _, _, _, _ := getGroupControllerWithMocksInGreenTestState()
result, err := sut.Reconcile(context.TODO(), getGreenGroupControllerRequest()) result, err := sut.Reconcile(context.TODO(), getGreenGroupControllerRequest())
...@@ -148,6 +234,36 @@ var _ = Describe("Reconcile", func() { ...@@ -148,6 +234,36 @@ var _ = Describe("Reconcile", func() {
It("should not return an error", func() { It("should not return an error", func() {
Expect(err).To(BeNil()) 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() { Context("supports client configuration mocking", func() {
sut, _, _, _, _ := getGroupControllerWithMocksInGreenTestState() sut, _, _, _, _ := getGroupControllerWithMocksInGreenTestState()
...@@ -156,7 +272,7 @@ var _ = Describe("Reconcile", func() { ...@@ -156,7 +272,7 @@ var _ = Describe("Reconcile", func() {
sut.gitlabClientConfiguration = nil sut.gitlabClientConfiguration = nil
sut.Reconcile(context.TODO(), getGreenGroupControllerRequest()) sut.Reconcile(context.TODO(), getGreenGroupControllerRequest())
It("should use the default client configuration implementation", func() { 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() { Context("the API Object isn't found", func() {
...@@ -344,12 +460,22 @@ var _ = Describe("Reconcile", func() { ...@@ -344,12 +460,22 @@ var _ = Describe("Reconcile", func() {
sut, logger, _, kubernetesClient, _ := getGroupControllerWithMocksInGreenTestState() sut, logger, _, kubernetesClient, _ := getGroupControllerWithMocksInGreenTestState()
kubernetesClient.createFunction = func(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { 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 { 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() { It("should requeue the object", func() {
Expect(result).To(Equal(ctrl.Result{Requeue: true})) Expect(result).To(Equal(ctrl.Result{Requeue: true}))
}) })
......
...@@ -437,6 +437,7 @@ type MockGitlabClient struct { ...@@ -437,6 +437,7 @@ type MockGitlabClient struct {
updateGroupVariableFunction func(groupID int, key string, options gitlab.UpdateGroupVariableOptions) (*gitlab.GroupVariable, int, error) 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) updateProjectVariableFunction func(groupID int, key string, options gitlab.UpdateProjectVariableOptions) (*gitlab.ProjectVariable, int, error)
getProjectVariableFunction func(projectID int, key string) (*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) { 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 ...@@ -614,6 +615,7 @@ func (m *MockGitlabClient) DeleteProjectVariable(projectID int, key string, wait
if foundVariable != nil { if foundVariable != nil {
delete(m.expectedProjectVariables, key) delete(m.expectedProjectVariables, key)
} }
return 200, nil return 200, nil
...@@ -682,7 +684,17 @@ func (m *MockGitlabClient) AddGroupMember(groupID *int, userID *int, accessLevel ...@@ -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) { 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) { func (m *MockGitlabClient) GetProject(projectID int) (*gitlab.Project, int, error) {
......
...@@ -67,7 +67,6 @@ var _ = BeforeSuite(func() { ...@@ -67,7 +67,6 @@ var _ = BeforeSuite(func() {
err = gitlabv1alpha1.AddToScheme(scheme.Scheme) err = gitlabv1alpha1.AddToScheme(scheme.Scheme)
Expect(err).NotTo(HaveOccurred()) Expect(err).NotTo(HaveOccurred())
//+kubebuilder:scaffold:scheme //+kubebuilder:scaffold:scheme
k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment