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(appcontroller): Using impersonation when an application is deleted #21800

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
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
20 changes: 20 additions & 0 deletions controller/appcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import (
"sync"
"time"

"k8s.io/client-go/rest"

clustercache "github.com/argoproj/gitops-engine/pkg/cache"
"github.com/argoproj/gitops-engine/pkg/diff"
"github.com/argoproj/gitops-engine/pkg/health"
Expand Down Expand Up @@ -1222,6 +1224,24 @@ func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Applic
}
logCtx.Infof("Deleting application's resources with %s propagation policy", propagationPolicy)

impersonationEnabled, err := ctrl.settingsMgr.IsImpersonationEnabled()
if err != nil {
logCtx.Errorf("could not get impersonation feature flag: %v", err)
return err
}
if impersonationEnabled {
serviceAccountToImpersonate, err := deriveServiceAccountToImpersonate(proj, app)
if err != nil {
logCtx.Errorf("could not get service account for impersonation: %v", err)
return err
}
logCtx = logCtx.WithFields(log.Fields{"impersonationEnabled": "true", "serviceAccount": serviceAccountToImpersonate})
// set the impersonation headers.
config.Impersonate = rest.ImpersonationConfig{
UserName: serviceAccountToImpersonate,
}
}

err = kube.RunAllAsync(len(filteredObjs), func(i int) error {
obj := filteredObjs[i]
return ctrl.kubectl.DeleteResource(context.Background(), config, obj.GroupVersionKind(), obj.GetName(), obj.GetNamespace(), metav1.DeleteOptions{PropagationPolicy: &propagationPolicy})
Expand Down
139 changes: 139 additions & 0 deletions controller/appcontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"strconv"
"testing"
"time"

Expand Down Expand Up @@ -2651,3 +2652,141 @@ func TestSyncTimeout(t *testing.T) {
})
}
}

func TestFinalizeApplicationDeletionWithImpersonate(t *testing.T) {
now := metav1.Now()
type fixture struct {
application *v1alpha1.Application
controller *ApplicationController
patched *bool
}

setup := func(impersonationEnabled bool, destinationNamespace, serviceAccountName string) *fixture {
app := newFakeApp()
app.SetCascadedDeletion(v1alpha1.ResourcesFinalizerName)
app.DeletionTimestamp = &now
app.Spec.Destination.Namespace = test.FakeArgoCDNamespace
app.Status.OperationState = nil
app.Status.History = nil

project := v1alpha1.AppProject{
ObjectMeta: metav1.ObjectMeta{
Namespace: test.FakeArgoCDNamespace,
Name: "default",
},
Spec: v1alpha1.AppProjectSpec{
SourceRepos: []string{"*"},
Destinations: []v1alpha1.ApplicationDestination{
{
Server: "*",
Namespace: "*",
},
},
DestinationServiceAccounts: []v1alpha1.
ApplicationDestinationServiceAccount{
{
Server: "https://localhost:6443",
Namespace: destinationNamespace,
DefaultServiceAccount: serviceAccountName,
},
},
},
}
additionalObjs := make([]runtime.Object, 0)
if serviceAccountName != "" {
syncServiceAccount := &corev1.ServiceAccount{
ObjectMeta: metav1.ObjectMeta{
Name: serviceAccountName,
Namespace: test.FakeDestNamespace,
},
}
additionalObjs = append(additionalObjs, syncServiceAccount)
}
data := fakeData{
apps: []runtime.Object{app, &project},
manifestResponse: &apiclient.ManifestResponse{
Manifests: []string{},
Namespace: test.FakeDestNamespace,
Server: "https://localhost:6443",
Revision: "abc123",
},
managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{},
configMapData: map[string]string{
"application.sync.impersonation.enabled": strconv.FormatBool(impersonationEnabled),
},
additionalObjs: additionalObjs,
}
ctrl := newFakeController(&data, 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", "*", func(_ kubetesting.Action) (handled bool, ret runtime.Object, err error) {
patched = true
return true, &v1alpha1.Application{}, nil
})

return &fixture{
application: app,
controller: ctrl,
patched: &patched,
}
}

t.Run("sync with impersonation and no matching service account", func(t *testing.T) {
// given app sync impersonation feature is enabled with an application referring a project no matching service account
f := setup(true, test.FakeArgoCDNamespace, "")
// when
err := f.controller.finalizeApplicationDeletion(f.application, func(_ string) ([]*v1alpha1.Cluster, error) {
return []*v1alpha1.Cluster{}, nil
})

// then, app sync should fail with expected error message in operation state
require.ErrorContains(t, err, "default service account contains invalid chars ''")
require.False(t, *f.patched)
})

t.Run("sync with impersonation and empty service account match", func(t *testing.T) {
// given app sync impersonation feature is enabled with an application referring a project matching service account that is an empty string
f := setup(true, test.FakeArgoCDNamespace, "")
// when
err := f.controller.finalizeApplicationDeletion(f.application, func(_ string) ([]*v1alpha1.Cluster, error) {
return []*v1alpha1.Cluster{}, nil
})

// then, app sync should fail with expected error message in operation state
require.ErrorContains(t, err, "default service account contains invalid chars ''")
require.False(t, *f.patched)
})

t.Run("sync with impersonation and matching sa", func(t *testing.T) {
// given app sync impersonation feature is enabled with an application referring a project matching service account
f := setup(true, test.FakeArgoCDNamespace, "test-sa")

// when
err := f.controller.finalizeApplicationDeletion(f.application, func(_ string) ([]*v1alpha1.Cluster, error) {
return []*v1alpha1.Cluster{}, nil
})

// then, app sync should fail with expected error message in operation state
require.NoError(t, err)
require.True(t, *f.patched)
})

t.Run("sync without impersonation", func(t *testing.T) {
// given app sync impersonation feature is disabled with an application referring a project matching service account
f := setup(false, test.FakeDestNamespace, "")

// when
err := f.controller.finalizeApplicationDeletion(f.application, func(_ string) ([]*v1alpha1.Cluster, error) {
return []*v1alpha1.Cluster{}, nil
})

// then, app sync should fail with expected error message in operation state
require.NoError(t, err)
require.True(t, *f.patched)
})
}
83 changes: 83 additions & 0 deletions test/e2e/app_deletion_test.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
package e2e

import (
"fmt"
"strings"
"testing"

corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"

. "github.com/argoproj/gitops-engine/pkg/sync/common"
"github.com/stretchr/testify/assert"

. "github.com/argoproj/argo-cd/v3/pkg/apis/application/v1alpha1"
. "github.com/argoproj/argo-cd/v3/test/e2e/fixture"
. "github.com/argoproj/argo-cd/v3/test/e2e/fixture/app"
"github.com/argoproj/argo-cd/v3/test/e2e/fixture/project"
"github.com/argoproj/argo-cd/v3/util/errors"
)

Expand Down Expand Up @@ -83,3 +89,80 @@ func TestDeletingAppByLabelWait(t *testing.T) {
// delete is successful
Expect(DoesNotExistNow())
}

func TestDeletingAppWithImpersonation(t *testing.T) {
projectCtx := project.Given(t)
appCtx := Given(t)

projectCtx.
Name("impersonation-test").
SourceNamespaces([]string{"*"}).
SourceRepositories([]string{"*"}).
Destination("*,*").
DestinationServiceAccounts([]string{fmt.Sprintf("%s,%s,%s", KubernetesInternalAPIServerAddr, "*", "default-sa")}).
When().
Create().
Then().
Expect()

appCtx.
Project("impersonation-test").
Path(guestbookPath).
When().
CreateApp("--label=foo=bar").
WithImpersonationEnabled("default-sa", []rbacv1.PolicyRule{
{
APIGroups: []string{"*"},
Resources: []string{"*"},
Verbs: []string{"*"},
},
}).
Sync().
Then().
Expect(SyncStatusIs(SyncStatusCodeSynced)).
When().
DeleteBySelectorWithWait("foo=bar").
Then().
Expect(DoesNotExist())
}

func TestDeletingAppWithImpersonationMissingDeletePermission(t *testing.T) {
projectName := "impersonation-missing-delete-permission"
serviceAccountName := "default-sa"

projectCtx := project.Given(t)
appCtx := Given(t)

projectCtx.
Name(projectName).
SourceNamespaces([]string{"*"}).
SourceRepositories([]string{"*"}).
Destination("*,*").
DestinationServiceAccounts([]string{fmt.Sprintf("%s,%s,%s", KubernetesInternalAPIServerAddr, "*", serviceAccountName)}).
When().
Create().
Then().
Expect()

appCtx.
Project(projectName).
Path(guestbookPath).
When().
CreateApp("--label=foo=bar").
WithImpersonationEnabled(serviceAccountName, []rbacv1.PolicyRule{
{
APIGroups: []string{"*"},
Resources: []string{"*"},
Verbs: []string{"get", "list", "watch", "create", "update", "patch"},
},
}).
Sync().
Then().
Expect(SyncStatusIs(SyncStatusCodeSynced)).
When().
Delete(false).
Then().
Expect(DoesNotExist()).
ExpectConsistently(DoesNotExist(), WaitDuration, TimeoutDuration).
ExpectConsistently(Pod(func(p corev1.Pod) bool { return strings.HasPrefix(p.Name, "guestbook-ui") }), WaitDuration, TimeoutDuration)
}