fix: create serviceaccount token for v1.24 clusters (#9546)

* fix: create serviceaccount token for v1.24 clusters Signed-off-by: Daniel Helfand <helfand.4@gmail.com> * change create to get in err Signed-off-by: Daniel Helfand <helfand.4@gmail.com>
argoproj · Jul 13, 2022 · 98aadc7 · 98aadc7
1 parent efdec28
commit 98aadc7
Show file tree

Hide file tree

Showing 8 changed files with 396 additions and 40 deletions.
diff --git a/cmd/argocd/commands/admin/cluster.go b/cmd/argocd/commands/admin/cluster.go
@@ -609,7 +609,7 @@ func GenerateToken(clusterOpts cmdutil.ClusterOptions, conf *rest.Config) (strin
 	clientset, err := kubernetes.NewForConfig(conf)
 	errors.CheckError(err)
 
-	bearerToken, err := clusterauth.GetServiceAccountBearerToken(clientset, clusterOpts.SystemNamespace, clusterOpts.ServiceAccount)
+	bearerToken, err := clusterauth.GetServiceAccountBearerToken(clientset, clusterOpts.SystemNamespace, clusterOpts.ServiceAccount, common.BearerTokenTimeout)
 	if err != nil {
 		return "", err
 	}

diff --git a/cmd/argocd/commands/cluster.go b/cmd/argocd/commands/cluster.go
@@ -113,7 +113,7 @@ func NewClusterAddCommand(clientOpts *argocdclient.ClientOptions, pathOpts *clie
 				clientset, err := kubernetes.NewForConfig(conf)
 				errors.CheckError(err)
 				if clusterOpts.ServiceAccount != "" {
-					managerBearerToken, err = clusterauth.GetServiceAccountBearerToken(clientset, clusterOpts.SystemNamespace, clusterOpts.ServiceAccount)
+					managerBearerToken, err = clusterauth.GetServiceAccountBearerToken(clientset, clusterOpts.SystemNamespace, clusterOpts.ServiceAccount, common.BearerTokenTimeout)
 				} else {
 					isTerminal := isatty.IsTerminal(os.Stdout.Fd()) || isatty.IsCygwinTerminal(os.Stdout.Fd())
 
@@ -123,7 +123,7 @@ func NewClusterAddCommand(clientOpts *argocdclient.ClientOptions, pathOpts *clie
 							os.Exit(1)
 						}
 					}
-					managerBearerToken, err = clusterauth.InstallClusterManagerRBAC(clientset, clusterOpts.SystemNamespace, clusterOpts.Namespaces)
+					managerBearerToken, err = clusterauth.InstallClusterManagerRBAC(clientset, clusterOpts.SystemNamespace, clusterOpts.Namespaces, common.BearerTokenTimeout)
 				}
 				errors.CheckError(err)
 			}

diff --git a/common/common.go b/common/common.go
@@ -212,6 +212,12 @@ const (
 	CacheVersion = "1.8.3"
 )
 
+// Constants used by util/clusterauth package
+const (
+	ClusterAuthRequestTimeout = 10 * time.Second
+	BearerTokenTimeout        = 30 * time.Second
+)
+
 const (
 	DefaultGitRetryMaxDuration time.Duration = time.Second * 5        // 5s
 	DefaultGitRetryDuration    time.Duration = time.Millisecond * 250 // 0.25s

diff --git a/go.mod b/go.mod
@@ -203,7 +203,7 @@ require (
 	gopkg.in/square/go-jose.v2 v2.2.2 // indirect
 	gopkg.in/warnings.v0 v0.1.2 // indirect
 	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect
-	k8s.io/apiserver v0.23.1 // indirect
+	k8s.io/apiserver v0.23.1
 	k8s.io/cli-runtime v0.23.1 // indirect
 	k8s.io/component-base v0.23.1 // indirect
 	k8s.io/component-helpers v0.23.1 // indirect

diff --git a/test/e2e/cluster_test.go b/test/e2e/cluster_test.go
@@ -182,3 +182,123 @@ func TestClusterURLInRestAPI(t *testing.T) {
 	require.NoError(t, err)
 	assert.Equal(t, map[string]string{"test": "val"}, cluster.Labels)
 }
+
+func TestClusterDeleteDenied(t *testing.T) {
+	accountFixture.Given(t).
+		Name("test").
+		When().
+		Create().
+		Login().
+		SetPermissions([]fixture.ACL{
+			{
+				Resource: "clusters",
+				Action:   "create",
+				Scope:    ProjectName + "/*",
+			},
+			{
+				Resource: "clusters",
+				Action:   "get",
+				Scope:    ProjectName + "/*",
+			},
+		}, "org-admin")
+
+	// Attempt to remove cluster creds by name
+	clusterFixture.
+		GivenWithSameState(t).
+		Project(ProjectName).
+		Upsert(true).
+		Server(KubernetesInternalAPIServerAddr).
+		When().
+		Create().
+		DeleteByName().
+		Then().
+		AndCLIOutput(func(output string, err error) {
+			assert.True(t, strings.Contains(err.Error(), "PermissionDenied desc = permission denied: clusters, delete"))
+		})
+
+	// Attempt to remove cluster creds by server
+	clusterFixture.
+		GivenWithSameState(t).
+		Project(ProjectName).
+		Upsert(true).
+		Server(KubernetesInternalAPIServerAddr).
+		When().
+		Create().
+		DeleteByServer().
+		Then().
+		AndCLIOutput(func(output string, err error) {
+			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)
+	}
+}
diff --git a/test/e2e/fixture/cluster/actions.go b/test/e2e/fixture/cluster/actions.go
@@ -6,7 +6,11 @@ import (
 	"fmt"
 	"log"
 
+	"github.com/argoproj/argo-cd/v2/common"
 	"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 +67,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{}, common.BearerTokenTimeout)
+	if err != nil {
+		a.lastError = err
+		return a
+	}
+
+	return a.Create()
+}
+
 func (a *Actions) List() *Actions {
 	a.context.t.Helper()
 	a.runCli("cluster", "list")
@@ -75,6 +103,20 @@ func (a *Actions) Get() *Actions {
 	return a
 }
 
+func (a *Actions) DeleteByName() *Actions {
+	a.context.t.Helper()
+
+	a.runCli("cluster", "rm", a.context.name)
+	return a
+}
+
+func (a *Actions) DeleteByServer() *Actions {
+	a.context.t.Helper()
+
+	a.runCli("cluster", "rm", a.context.server)
+	return a
+}
+
 func (a *Actions) Then() *Consequences {
 	a.context.t.Helper()
 	return &Consequences{a.context, a}