feat: remove rbac when using argocd cluster rm (#8969)

Signed-off-by: Daniel Helfand <[email protected]>

cmd/argocd/commands/cluster.go

@@ -12,6 +12,7 @@ import (
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 
 	"github.com/argoproj/argo-cd/v2/cmd/argocd/commands/headless"
@@ -53,7 +54,7 @@ func NewClusterCommand(clientOpts *argocdclient.ClientOptions, pathOpts *clientc
 	command.AddCommand(NewClusterAddCommand(clientOpts, pathOpts))
 	command.AddCommand(NewClusterGetCommand(clientOpts))
 	command.AddCommand(NewClusterListCommand(clientOpts))
-	command.AddCommand(NewClusterRemoveCommand(clientOpts))
+	command.AddCommand(NewClusterRemoveCommand(clientOpts, pathOpts))
 	command.AddCommand(NewClusterRotateAuthCommand(clientOpts))
 	return command
 }
@@ -76,21 +77,9 @@ func NewClusterAddCommand(clientOpts *argocdclient.ClientOptions, pathOpts *clie
 				cmdutil.PrintKubeContexts(configAccess)
 				os.Exit(1)
 			}
-			config, err := configAccess.GetStartingConfig()
-			errors.CheckError(err)
 			contextName := args[0]
-			clstContext := config.Contexts[contextName]
-			if clstContext == nil {
-				log.Fatalf("Context %s does not exist in kubeconfig", contextName)
-			}
-
-			overrides := clientcmd.ConfigOverrides{
-				Context: *clstContext,
-			}
-			clientConfig := clientcmd.NewDefaultClientConfig(*config, &overrides)
-			conf, err := clientConfig.ClientConfig()
+			conf, err := getRestConfig(pathOpts, contextName)
 			errors.CheckError(err)
-
 			managerBearerToken := ""
 			var awsAuthConf *argoappv1.AWSAuthConfig
 			var execProviderConf *argoappv1.ExecProviderConfig
@@ -171,6 +160,30 @@ func NewClusterAddCommand(clientOpts *argocdclient.ClientOptions, pathOpts *clie
 	return command
 }
 
+func getRestConfig(pathOpts *clientcmd.PathOptions, ctxName string) (*rest.Config, error) {
+	config, err := pathOpts.GetStartingConfig()
+	if err != nil {
+		return nil, err
+	}
+
+	clstContext := config.Contexts[ctxName]
+	if clstContext == nil {
+		return nil, fmt.Errorf("Context %s does not exist in kubeconfig", ctxName)
+	}
+
+	overrides := clientcmd.ConfigOverrides{
+		Context: *clstContext,
+	}
+
+	clientConfig := clientcmd.NewDefaultClientConfig(*config, &overrides)
+	conf, err := clientConfig.ClientConfig()
+	if err != nil {
+		return nil, err
+	}
+
+	return conf, nil
+}
+
 // NewClusterGetCommand returns a new instance of an `argocd cluster get` command
 func NewClusterGetCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command {
 	var (
@@ -245,7 +258,7 @@ func printClusterDetails(clusters []argoappv1.Cluster) {
 }
 
 // NewClusterRemoveCommand returns a new instance of an `argocd cluster rm` command
-func NewClusterRemoveCommand(clientOpts *argocdclient.ClientOptions) *cobra.Command {
+func NewClusterRemoveCommand(clientOpts *argocdclient.ClientOptions, pathOpts *clientcmd.PathOptions) *cobra.Command {
 	var command = &cobra.Command{
 		Use:   "rm SERVER/NAME",
 		Short: "Remove cluster credentials",
@@ -259,16 +272,27 @@ argocd cluster rm cluster-name`,
 			conn, clusterIf := headless.NewClientOrDie(clientOpts, c).NewClusterClientOrDie()
 			defer io.Close(conn)
 
-			// clientset, err := kubernetes.NewForConfig(conf)
-			// errors.CheckError(err)
+			for _, clusterSelector := range args {
+				clusterQuery := getQueryBySelector(clusterSelector)
+
+				// get the cluster name to use as context to delete RBAC on cluster
+				clst, err := clusterIf.Get(context.Background(), clusterQuery)
+				errors.CheckError(err)
+
+				// remove cluster
+				_, err = clusterIf.Delete(context.Background(), clusterQuery)
+				errors.CheckError(err)
+				fmt.Printf("Cluster '%s' removed\n", clusterSelector)
+
+				// remove RBAC from cluster
+				conf, err := getRestConfig(pathOpts, clst.Name)
+				errors.CheckError(err)
+
+				clientset, err := kubernetes.NewForConfig(conf)
+				errors.CheckError(err)
 
-			for _, clusterName := range args {
-				// TODO(jessesuen): find the right context and remove manager RBAC artifacts
-				// err := clusterauth.UninstallClusterManagerRBAC(clientset)
-				// errors.CheckError(err)
-				_, err := clusterIf.Delete(context.Background(), getQueryBySelector(clusterName))
+				err = clusterauth.UninstallClusterManagerRBAC(clientset)
 				errors.CheckError(err)
-				fmt.Printf("Cluster '%s' removed\n", clusterName)
 			}
 		},
 	}

cmd/argocd/commands/cluster_test.go

@@ -3,10 +3,12 @@ package commands
 import (
 	"testing"
 
+	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
-	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
+	"k8s.io/client-go/rest"
+	"k8s.io/client-go/tools/clientcmd"
 )
 
 func Test_getQueryBySelector(t *testing.T) {
@@ -44,3 +46,63 @@ func Test_printClusterTable(t *testing.T) {
 		},
 	})
 }
+
+func Test_getRestConfig(t *testing.T) {
+	type args struct {
+		pathOpts *clientcmd.PathOptions
+		ctxName  string
+	}
+	pathOpts := &clientcmd.PathOptions{
+		GlobalFile:   "./testdata/config",
+		LoadingRules: clientcmd.NewDefaultClientConfigLoadingRules(),
+	}
+	tests := []struct {
+		name        string
+		args        args
+		expected    *rest.Config
+		wantErr     bool
+		expectedErr string
+	}{
+		{
+			"Load config for context successfully",
+			args{
+				pathOpts,
+				"argocd2.example.com:443",
+			},
+			&rest.Config{Host: "argocd2.example.com:443"},
+			false,
+			"",
+		},
+		{
+			"Load config for current-context successfully",
+			args{
+				pathOpts,
+				"localhost:8080",
+			},
+			&rest.Config{Host: "localhost:8080"},
+			false,
+			"",
+		},
+		{
+			"Context not found",
+			args{
+				pathOpts,
+				"not-exist",
+			},
+			nil,
+			true,
+			"Context not-exist does not exist in kubeconfig",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got, err := getRestConfig(tt.args.pathOpts, tt.args.ctxName); err == nil {
+				require.Equal(t, got, tt.expected)
+			} else if tt.wantErr {
+				require.Equal(t, err.Error(), tt.expectedErr)
+			} else {
+				t.Errorf("An unexpected error occurred during test %s:\n%s", tt.name, err.Error())
+			}
+		})
+	}
+}

cmd/argocd/commands/testdata/config

@@ -1,25 +1,31 @@
+clusters:
+- cluster:
+    server: argocd1.example.com:443
+  name: argocd1.example.com:443
+- cluster:
+    server: argocd2.example.com:443
+  name: argocd2.example.com:443
+- cluster:
+    server: localhost:8080
+  name: localhost:8080
 contexts:
-- name: argocd1.example.com:443
-  server: argocd1.example.com:443
-  user: argocd1.example.com:443
-- name: argocd2.example.com:443
-  server: argocd2.example.com:443
-  user: argocd2.example.com:443
-- name: localhost:8080
-  server: localhost:8080
-  user: localhost:8080
-current-context: localhost:8080
-servers:
-- server: argocd1.example.com:443
-- server: argocd2.example.com:443
-- plain-text: true
-  server: localhost:8080
-users:
-- auth-token: vErrYS3c3tReFRe$hToken
+- context:
+    server: argocd1.example.com:443
+    user: argocd1.example.com:443
+    cluster: argocd1.example.com:443
   name: argocd1.example.com:443
-  refresh-token: vErrYS3c3tReFRe$hToken
-- auth-token: vErrYS3c3tReFRe$hToken
+- context:
+    server: argocd2.example.com:443
+    user: argocd2.example.com:443
+    cluster: argocd2.example.com:443
   name: argocd2.example.com:443
-  refresh-token: vErrYS3c3tReFRe$hToken
-- auth-token: vErrYS3c3tReFRe$hToken
-  name: localhost:8080
+- context:
+    server: localhost:8080
+    user: localhost:8080
+    cluster: localhost:8080
+  name: localhost:8080
+current-context: localhost:8080
+users:
+- name:  argocd1.example.com:443
+- name:  argocd2.example.com:443
+- name:  localhost:8080

test/e2e/cluster_test.go

@@ -197,6 +197,11 @@ func TestClusterDeleteDenied(t *testing.T) {
 				Action:   "create",
 				Scope:    ProjectName + "/*",
 			},
+			{
+				Resource: "clusters",
+				Action:   "get",
+				Scope:    ProjectName + "/*",
+			},
 		}, "org-admin")
 
 	// Attempt to remove cluster creds by name
@@ -227,3 +232,75 @@ func TestClusterDeleteDenied(t *testing.T) {
 			assert.True(t, strings.Contains(err.Error(), "PermissionDenied desc = permission denied: clusters, delete"))
 		})
 }
+
+func TestClusterDelete(t *testing.T) {
+	accountFixture.Given(t).
+		Name("default").
+		When().
+		Create().
+		Login().
+		SetPermissions([]fixture.ACL{
+			{
+				Resource: "clusters",
+				Action:   "create",
+				Scope:    ProjectName + "/*",
+			},
+			{
+				Resource: "clusters",
+				Action:   "get",
+				Scope:    ProjectName + "/*",
+			},
+			{
+				Resource: "clusters",
+				Action:   "delete",
+				Scope:    ProjectName + "/*",
+			},
+		}, "org-admin")
+
+	clstAction := clusterFixture.
+		GivenWithSameState(t).
+		Name("default").
+		Project(ProjectName).
+		Upsert(true).
+		Server(KubernetesInternalAPIServerAddr).
+		When().
+		CreateWithRBAC()
+
+	// Check that RBAC is created
+	_, err := fixture.Run("", "kubectl", "get", "serviceaccount", "argocd-manager", "-n", "kube-system")
+	if err != nil {
+		t.Errorf("Expected no error from not finding serviceaccount argocd-manager but got:\n%s", err.Error())
+	}
+
+	_, err = fixture.Run("", "kubectl", "get", "clusterrole", "argocd-manager-role")
+	if err != nil {
+		t.Errorf("Expected no error from not finding clusterrole argocd-manager-role but got:\n%s", err.Error())
+	}
+
+	_, err = fixture.Run("", "kubectl", "get", "clusterrolebinding", "argocd-manager-role-binding")
+	if err != nil {
+		t.Errorf("Expected no error from not finding clusterrole argocd-manager-role but got:\n%s", err.Error())
+	}
+
+	clstAction.DeleteByName().
+		Then().
+		AndCLIOutput(func(output string, err error) {
+			assert.Equal(t, "Cluster 'default' removed", output)
+		})
+
+	// Check that RBAC is removed after delete
+	output, err := fixture.Run("", "kubectl", "get", "serviceaccount", "argocd-manager", "-n", "kube-system")
+	if err == nil {
+		t.Errorf("Expected error from not finding serviceaccount argocd-manager but got:\n%s", output)
+	}
+
+	output, err = fixture.Run("", "kubectl", "get", "clusterrole", "argocd-manager-role")
+	if err == nil {
+		t.Errorf("Expected error from not finding clusterrole argocd-manager-role but got:\n%s", output)
+	}
+
+	output, err = fixture.Run("", "kubectl", "get", "clusterrolebinding", "argocd-manager-role-binding")
+	if err == nil {
+		t.Errorf("Expected error from not finding clusterrole argocd-manager-role but got:\n%s", output)
+	}
+}

test/e2e/fixture/app/consequences.go

@@ -24,7 +24,7 @@ func (c *Consequences) Expect(e Expectation) *Consequences {
 	c.context.t.Helper()
 	var message string
 	var state state
-	timeout := time.Duration(15) * time.Second
+	timeout := time.Duration(30) * time.Second
 	for start := time.Now(); time.Since(start) < timeout; time.Sleep(3 * time.Second) {
 		state, message = e(c)
 		switch state {

test/e2e/fixture/cluster/actions.go

@@ -7,6 +7,9 @@ import (
 	"log"
 
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
+	"github.com/argoproj/argo-cd/v2/util/clusterauth"
+	"k8s.io/client-go/kubernetes"
+	"k8s.io/client-go/tools/clientcmd"
 
 	clusterpkg "github.com/argoproj/argo-cd/v2/pkg/apiclient/cluster"
 	"github.com/argoproj/argo-cd/v2/test/e2e/fixture"
@@ -63,6 +66,30 @@ func (a *Actions) Create(args ...string) *Actions {
 	return a
 }
 
+func (a *Actions) CreateWithRBAC(args ...string) *Actions {
+	pathOpts := clientcmd.NewDefaultPathOptions()
+	config, err := pathOpts.GetStartingConfig()
+	if err != nil {
+		a.lastError = err
+		return a
+	}
+	clientConfig := clientcmd.NewDefaultClientConfig(*config, &clientcmd.ConfigOverrides{})
+	conf, err := clientConfig.ClientConfig()
+	if err != nil {
+		a.lastError = err
+		return a
+	}
+	client := kubernetes.NewForConfigOrDie(conf)
+
+	_, err = clusterauth.InstallClusterManagerRBAC(client, "kube-system", []string{})
+	if err != nil {
+		a.lastError = err
+		return a
+	}
+
+	return a.Create()
+}
+
 func (a *Actions) List() *Actions {
 	a.context.t.Helper()
 	a.runCli("cluster", "list")