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/prevent appproject deletion #21319

Closed
46 changes: 46 additions & 0 deletions controller/appcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,32 @@
gvk.Kind == application.ApplicationKind
}

// JAY-KULKARNI-FUNCTION
func (ctrl *ApplicationController) isAppProjectWithExistingApplications(obj *unstructured.Unstructured) (bool, error) {
if obj.GetKind() != "AppProject" {
return false, nil
}
origJSON, err := json.Marshal(obj)
if err != nil {
return false, err
}

origProj := &appv1.AppProject{}
if err = json.Unmarshal(origJSON, &origProj); err != nil {
return false, err
}

if err := ctrl.addProjectFinalizer(origProj); err != nil {
log.Warnf("Failed to add finalizer for project: %v", err)
}

if err := ctrl.finalizeProjectDeletion(origProj); err != nil {
log.Errorf("Failed to finalize project deletion: %v", err)
}

return origProj.HasFinalizer(), nil
}

func (ctrl *ApplicationController) newAppProjCache(name string) *appProjCache {
return &appProjCache{name: name, ctrl: ctrl}
}
Expand Down Expand Up @@ -1130,6 +1156,21 @@
return err
}

func (ctrl *ApplicationController) addProjectFinalizer(proj *appv1.AppProject, stage ...string) error {

Check failure on line 1159 in controller/appcontroller.go

View workflow job for this annotation

GitHub Actions / Lint Go code

`(*ApplicationController).addProjectFinalizer` - `stage` is unused (unparam)
if proj.HasFinalizer() {
return nil
}
proj.SetProjectFinalizer()
var patch []byte
patch, _ = json.Marshal(map[string]interface{}{

Check failure on line 1165 in controller/appcontroller.go

View workflow job for this annotation

GitHub Actions / Lint Go code

use-any: since Go 1.18 'interface{}' can be replaced by 'any' (revive)
"metadata": map[string]interface{}{

Check failure on line 1166 in controller/appcontroller.go

View workflow job for this annotation

GitHub Actions / Lint Go code

use-any: since Go 1.18 'interface{}' can be replaced by 'any' (revive)
"finalizers": proj.Finalizers,
},
})
_, err := ctrl.applicationClientset.ArgoprojV1alpha1().AppProjects(ctrl.namespace).Patch(context.Background(), proj.Name, types.MergePatchType, patch, metav1.PatchOptions{})
return err
}

// shouldBeDeleted returns whether a given resource obj should be deleted on cascade delete of application app
func (ctrl *ApplicationController) shouldBeDeleted(app *appv1.Application, obj *unstructured.Unstructured) bool {
return !kube.IsCRD(obj) && !isSelfReferencedApp(app, kube.GetObjectRef(obj)) &&
Expand Down Expand Up @@ -1228,6 +1269,11 @@
logCtx.Infof("Resource %v requires manual confirmation to delete", k)
return nil
}
isAppProjectWithExistingApplications, err := ctrl.isAppProjectWithExistingApplications(objsMap[k])
if isAppProjectWithExistingApplications {
logCtx.Infof("Unable to delete project %v", objsMap[k].GetName())
return err
}
}
}

Expand Down
164 changes: 164 additions & 0 deletions controller/appcontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,27 @@
}
`

var fakeAppProjectDeveloper = `
{
"apiVersion": "argoproj.io/v1alpha1",
"kind": "AppProject",
"metadata": {
"name": "developer",
"namespace": "fake-argocd-ns",
},
"spec": {
"description": "Developer Project",
"destinations": [
{
"server": "*",
"namespace": "*"
}
],
"sourceRepos": [ '*' ],
}
}
`

func newFakeApp() *v1alpha1.Application {
return createFakeApp(fakeApp)
}
Expand Down Expand Up @@ -584,6 +605,15 @@
return serviceAccount
}

func newFakeAppProject() map[string]interface{} {

Check failure on line 608 in controller/appcontroller_test.go

View workflow job for this annotation

GitHub Actions / Lint Go code

use-any: since Go 1.18 'interface{}' can be replaced by 'any' (revive)
var appProject map[string]interface{}

Check failure on line 609 in controller/appcontroller_test.go

View workflow job for this annotation

GitHub Actions / Lint Go code

use-any: since Go 1.18 'interface{}' can be replaced by 'any' (revive)
err := yaml.Unmarshal([]byte(fakeAppProjectDeveloper), &appProject)
if err != nil {
panic(err)
}
return appProject
}

func TestAutoSync(t *testing.T) {
app := newFakeApp()
ctrl := newFakeController(&fakeData{apps: []runtime.Object{app}}, nil)
Expand Down Expand Up @@ -1143,6 +1173,140 @@
// finalizer is not removed
assert.False(t, patched)
})

t.Run("DeveloperAppProject_IsNotDeleted", func(t *testing.T) {
defaultProjectApp := newFakeApp()
defaultProjectApp.Spec.Destination.Namespace = test.FakeArgoCDNamespace
defaultProjectApp.SetFinalizers([]string{v1alpha1.ResourcesFinalizerName})
defaultProjectApp.Spec.Project = "default"
defaultProjectApp.Name = "defaultProjectApp"
defaultProjectApp.DeletionTimestamp = &testTimestamp
////////// CREATE MANAGED PROJECT APP //////////
developerProjectApp := newFakeApp()
developerProjectApp.Spec.Destination.Namespace = test.FakeArgoCDNamespace
developerProjectApp.SetFinalizers([]string{v1alpha1.ResourcesFinalizerName})
developerProjectApp.Spec.Project = "developer"
developerProjectApp.Name = "developerProjectApp"
developerProjectApp.DeletionTimestamp = &testTimestamp

////////// CREATE PROJECTS //////////
liveRole := &unstructured.Unstructured{Object: newFakeRole()}
liveServiceAccount := &unstructured.Unstructured{Object: newFakeServiceAccount()}
liveAppProjectDeveloper := &unstructured.Unstructured{Object: newFakeAppProject()}

////////// MARSHALLING //////////
developerProject := &v1alpha1.AppProject{}
origJSON, err := json.Marshal(liveAppProjectDeveloper)
if err != nil {
panic(err)
}
if err = json.Unmarshal(origJSON, &developerProject); err != nil {
panic(err)
}

ctrl := newFakeController(&fakeData{
manifestResponses: []*apiclient.ManifestResponse{{
Manifests: []string{fakeRole, fakeServiceAccount, fakeAppProjectDeveloper},
}},
apps: []runtime.Object{defaultProjectApp, developerProjectApp, &defaultProj, developerProject},
managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{
kube.GetResourceKey(liveRole): liveRole,
kube.GetResourceKey(liveServiceAccount): liveServiceAccount,
kube.GetResourceKey(liveAppProjectDeveloper): liveAppProjectDeveloper,
},
}, nil)
patched := false
fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset)
defaultReactor := fakeAppCs.ReactionChain[0]
fakeAppCs.ReactionChain = nil
fakeAppCs.AddReactor("get", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
return defaultReactor.React(action)
})
fakeAppCs.AddReactor("patch", "applications", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
patched = true
return true, &v1alpha1.Application{}, nil
})
fakeAppCs.AddReactor("patch", "appprojects", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
patched = true
return true, &v1alpha1.AppProject{}, nil
})

err = ctrl.finalizeApplicationDeletion(defaultProjectApp, func(project string) ([]*v1alpha1.Cluster, error) {
return []*v1alpha1.Cluster{}, nil
})
require.NoError(t, err)
require.Len(t, ctrl.kubectl.(*MockKubectl).DeletedResources, 0)

Check failure on line 1238 in controller/appcontroller_test.go

View workflow job for this annotation

GitHub Actions / Lint Go code

empty: use require.Empty (testifylint)
deletedResources := []string{}
for _, res := range ctrl.kubectl.(*MockKubectl).DeletedResources {
deletedResources = append(deletedResources, res.Name)

Check failure on line 1241 in controller/appcontroller_test.go

View workflow job for this annotation

GitHub Actions / Lint Go code

SA4010: this result of append is never used, except maybe in other appends (staticcheck)
}
assert.True(t, patched)
})

t.Run("ProjectApp_IsDeleted", func(t *testing.T) {
defaultProjectApp := newFakeApp()
defaultProjectApp.Spec.Destination.Namespace = test.FakeArgoCDNamespace
defaultProjectApp.Spec.Project = "default"
defaultProjectApp.SetFinalizers([]string{v1alpha1.ResourcesFinalizerName})
defaultProjectApp.Name = "defaultProjectApp"

////////// CREATE PROJECTS //////////
liveRole := &unstructured.Unstructured{Object: newFakeRole()}
liveServiceAccount := &unstructured.Unstructured{Object: newFakeServiceAccount()}
liveAppProjectDeveloper := &unstructured.Unstructured{Object: newFakeAppProject()}
liveAppProjectDeveloper.SetName("developer")
////////// MARSHALLING //////////
developerProject := &v1alpha1.AppProject{}
origJSON, err := json.Marshal(liveAppProjectDeveloper)
if err != nil {
panic(err)
}
if err = json.Unmarshal(origJSON, &developerProject); err != nil {
panic(err)
}

defaultProjectApp.DeletionTimestamp = &testTimestamp
ctrl := newFakeController(&fakeData{
manifestResponses: []*apiclient.ManifestResponse{{
Manifests: []string{fakeRole, fakeServiceAccount, fakeAppProjectDeveloper},
}},
apps: []runtime.Object{defaultProjectApp, &defaultProj, developerProject},
managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{
kube.GetResourceKey(liveRole): liveRole,
kube.GetResourceKey(liveServiceAccount): liveServiceAccount,
kube.GetResourceKey(liveAppProjectDeveloper): liveAppProjectDeveloper,
},
}, nil)

patched := false
fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset)
defaultReactor := fakeAppCs.ReactionChain[0]
fakeAppCs.ReactionChain = nil
fakeAppCs.AddReactor("get", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
return defaultReactor.React(action)
})
fakeAppCs.AddReactor("patch", "applications", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
patched = true
return true, &v1alpha1.Application{}, nil
})
fakeAppCs.AddReactor("patch", "appprojects", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
patched = true
return true, &v1alpha1.AppProject{}, nil
})

err = ctrl.finalizeApplicationDeletion(defaultProjectApp, func(project string) ([]*v1alpha1.Cluster, error) {
return []*v1alpha1.Cluster{}, nil
})
require.NoError(t, err)
require.Len(t, ctrl.kubectl.(*MockKubectl).DeletedResources, 3)
deletedResources := []string{}
for _, res := range ctrl.kubectl.(*MockKubectl).DeletedResources {
deletedResources = append(deletedResources, res.Name)
}
expectedNames := []string{"hook-serviceaccount", "developer", "hook-role"}
require.ElementsMatch(t, expectedNames, deletedResources, "Deleted resources should match expected names")
assert.True(t, patched)
})
}

// TestNormalizeApplication verifies we normalize an application during reconciliation
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/application/v1alpha1/app_project_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,10 @@ func (proj *AppProject) RemoveFinalizer() {
setFinalizer(&proj.ObjectMeta, ResourcesFinalizerName, false)
}

func (proj *AppProject) SetProjectFinalizer(stage ...string) {
setFinalizer(&proj.ObjectMeta, strings.Join(append([]string{ResourcesFinalizerName}, stage...), "/"), true)
}

func globMatch(pattern string, val string, allowNegation bool, separators ...rune) bool {
if allowNegation && isDenyPattern(pattern) {
return !glob.Match(pattern[1:], val, separators...)
Expand Down
Loading