cm iteration logic (#169)

* cm iteration logic * cleanup * CHANGELOG
giantswarm · Oct 16, 2024 · 2664ada · 2664ada
1 parent b954712
commit 2664ada
Show file tree

Hide file tree

Showing 10 changed files with 180 additions and 105 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+- Changed the logic of setting roles by parsing `"%s-teleport-kube-agent-user-values"` configmaps to check if apps are enabled.
+- Deprecated `MC-Namespace` and `tokenRoles`
+
 ## [0.11.2] - 2024-10-11
 
 ### Fixed

diff --git a/go.mod b/go.mod
@@ -14,6 +14,7 @@ require (
 	github.com/onsi/gomega v1.34.1
 	github.com/pkg/errors v0.9.1
 	google.golang.org/grpc v1.67.1
+	gopkg.in/yaml.v2 v2.4.0
 	gopkg.in/yaml.v3 v3.0.1
 	k8s.io/api v0.25.0
 	k8s.io/apimachinery v0.25.0
@@ -107,7 +108,6 @@ require (
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240814211410-ddb44dafa142 // indirect
 	google.golang.org/protobuf v1.34.2 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
-	gopkg.in/yaml.v2 v2.4.0 // indirect
 	k8s.io/apiextensions-apiserver v0.25.0 // indirect
 	k8s.io/component-base v0.25.0 // indirect
 	k8s.io/klog/v2 v2.70.1 // indirect

diff --git a/helm/teleport-operator/templates/deployment.yaml b/helm/teleport-operator/templates/deployment.yaml
@@ -36,8 +36,6 @@ spec:
         image: "{{ .Values.registry.domain }}/{{ .Values.image.name }}:{{ .Chart.Version }}"
         args:
         - "--namespace={{ include "resource.default.namespace" . }}"
-        - "--token-roles={{ .Values.teleportOperator.roles | join "," }}"
-        - "--mc-namespace={{ .Values.teleportOperator.mcNamespace }}"
         {{- if .Values.tbot.enabled }}
         - "--tbot"
         {{- end }}

diff --git a/helm/teleport-operator/values.schema.json b/helm/teleport-operator/values.schema.json
@@ -63,19 +63,6 @@
                 }
             }
         },
-        "teleportOperator": {
-            "type": "object",
-            "properties": {
-              "roles": {
-                "type": "array",
-                "items": {
-                  "type": "string"
-                },
-                "description": "List of roles for the Teleport operator. Example: ['kube', 'app']"
-              }
-            },
-            "required": ["roles"]
-          },
         "pod": {
             "type": "object",
             "properties": {

diff --git a/helm/teleport-operator/values.yaml b/helm/teleport-operator/values.yaml
@@ -19,10 +19,6 @@ teleport:
   teleportClusterName: test.teleport.giantswarm.io
   teleportVersion: 16.1.7
 
-teleportOperator:
-  roles:
-    - kube
-  mcNamespace: "org-giantswarm"
 
 pod:
   user:

diff --git a/internal/controller/cluster_controller.go b/internal/controller/cluster_controller.go
@@ -39,14 +39,13 @@ const identityExpirationPeriod = 20 * time.Minute
 
 // ClusterReconciler reconciles a Cluster object
 type ClusterReconciler struct {
-	Client       client.Client
-	Log          logr.Logger
-	Scheme       *runtime.Scheme
-	Teleport     *teleport.Teleport
-	IsBotEnabled bool
-	Namespace    string
-	MCNamespace  string
-	TokenRoles   []string
+	Client            client.Client
+	Log               logr.Logger
+	Scheme            *runtime.Scheme
+	Teleport          *teleport.Teleport
+	IsBotEnabled      bool
+	Namespace         string
+	lastAssignedRoles []string
 }
 
 //+kubebuilder:rbac:groups=cluster.x-k8s.io.giantswarm.io,resources=clusters,verbs=get;list;watch;create;update;patch;delete
@@ -73,13 +72,19 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 		return ctrl.Result{}, microerror.Mask(err)
 	}
 
-	roles := []string{key.RoleKube}
-	if r.MCNamespace != "" && cluster.Namespace == r.MCNamespace {
-		roles = r.TokenRoles
-	}
-
 	log.Info("Reconciling cluster", "cluster", cluster)
 
+	appsEnabled, err := r.Teleport.AreTeleportAppsEnabled(ctx, cluster.Name, cluster.Namespace)
+	if err != nil {
+		log.Error(err, "Failed to check if Teleport apps are enabled")
+		return ctrl.Result{}, microerror.Mask(err)
+	}
+
+	roles := []string{key.RoleKube}
+	if appsEnabled {
+		roles = append(roles, key.RoleApp)
+	}
+	r.lastAssignedRoles = roles
 	if r.Teleport.Identity != nil {
 		log.Info("Teleport identity", "last-read-minutes-ago", r.Teleport.Identity.Age(), "hash", r.Teleport.Identity.Hash())
 	}

diff --git a/internal/controller/cluster_controller_test.go b/internal/controller/cluster_controller_test.go
@@ -3,12 +3,15 @@ package controller
 import (
 	"context"
 	"errors"
+	"fmt"
+	"reflect"
 	"testing"
 	"time"
 
 	appv1alpha1 "github.com/giantswarm/apiextensions-application/api/v1alpha1"
 	teleportTypes "github.com/gravitational/teleport/api/types"
 	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
@@ -24,76 +27,114 @@ import (
 
 func Test_ClusterController(t *testing.T) {
 	testCases := []struct {
-		name              string
-		namespace         string
-		token             string
-		tokens            []teleportTypes.ProvisionToken
-		config            *config.Config
-		identity          *config.IdentityConfig
-		identitySecret    *corev1.Secret
-		cluster           *capi.Cluster
-		secret            *corev1.Secret
-		configMap         *corev1.ConfigMap
-		newTeleportClient func(ctx context.Context, proxyAddr, identityFile string) (teleport.Client, error)
-		expectedCluster   *capi.Cluster
-		expectedSecret    *corev1.Secret
-		expectedConfigMap *corev1.ConfigMap
-		expectedError     error
-		mcNamespace       string
-		tokenRoles        []string
+		name                string
+		namespace           string
+		token               string
+		tokens              []teleportTypes.ProvisionToken
+		config              *config.Config
+		identity            *config.IdentityConfig
+		identitySecret      *corev1.Secret
+		cluster             *capi.Cluster
+		secret              *corev1.Secret
+		configMap           *corev1.ConfigMap
+		userValuesConfigMap *corev1.ConfigMap
+		newTeleportClient   func(ctx context.Context, proxyAddr, identityFile string) (teleport.Client, error)
+		expectedCluster     *capi.Cluster
+		expectedSecret      *corev1.Secret
+		expectedConfigMap   *corev1.ConfigMap
+		expectedError       error
+		expectedRoles       []string
 	}{
 		{
-			name:              "case 0: Register cluster and create Secret, ConfigMap and App resources in case they do not exist",
-			namespace:         test.NamespaceName,
-			token:             test.TokenName,
-			config:            newConfig(),
-			identity:          newIdentity(test.LastReadValue),
-			cluster:           test.NewCluster(test.ClusterName, test.NamespaceName, []string{key.TeleportOperatorFinalizer}, time.Time{}),
+			name:      "case 0: Register cluster with apps enabled",
+			namespace: test.NamespaceName,
+			token:     test.TokenName,
+			config:    newConfig(),
+			identity:  newIdentity(test.LastReadValue),
+			cluster:   test.NewCluster(test.ClusterName, test.NamespaceName, []string{key.TeleportOperatorFinalizer}, time.Time{}),
+			userValuesConfigMap: &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      fmt.Sprintf("%s-teleport-kube-agent-user-values", test.ClusterName),
+					Namespace: test.NamespaceName,
+				},
+				Data: map[string]string{
+					"values": "apps:\n- name: test-app\n  uri: http://test-app",
+				},
+			},
+			expectedCluster:   test.NewCluster(test.ClusterName, test.NamespaceName, []string{key.TeleportOperatorFinalizer}, time.Time{}),
+			expectedSecret:    test.NewSecret(test.ClusterName, test.NamespaceName, test.TokenName),
+			expectedConfigMap: test.NewConfigMap(test.ClusterName, test.AppName, test.NamespaceName, test.TokenName, []string{key.RoleKube, key.RoleApp}),
+			expectedRoles:     []string{key.RoleKube, key.RoleApp},
+		},
+		{
+			name:      "case 1: Register cluster without apps",
+			namespace: test.NamespaceName,
+			token:     test.TokenName,
+			config:    newConfig(),
+			identity:  newIdentity(test.LastReadValue),
+			cluster:   test.NewCluster(test.ClusterName, test.NamespaceName, []string{key.TeleportOperatorFinalizer}, time.Time{}),
+			userValuesConfigMap: &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      fmt.Sprintf("%s-teleport-kube-agent-user-values", test.ClusterName),
+					Namespace: test.NamespaceName,
+				},
+				Data: map[string]string{
+					"values": "someOtherConfig: true",
+				},
+			},
 			expectedCluster:   test.NewCluster(test.ClusterName, test.NamespaceName, []string{key.TeleportOperatorFinalizer}, time.Time{}),
 			expectedSecret:    test.NewSecret(test.ClusterName, test.NamespaceName, test.TokenName),
 			expectedConfigMap: test.NewConfigMap(test.ClusterName, test.AppName, test.NamespaceName, test.TokenName, []string{key.RoleKube}),
-			tokenRoles:        []string{key.RoleKube, key.RoleApp},
+			expectedRoles:     []string{key.RoleKube},
 		},
 		{
-			name:              "case 1: Register cluster in MC namespace and create Secret, ConfigMap with TokenRoles",
+			name:              "case 2: Register cluster without user values ConfigMap",
 			namespace:         test.NamespaceName,
 			token:             test.TokenName,
 			config:            newConfig(),
 			identity:          newIdentity(test.LastReadValue),
 			cluster:           test.NewCluster(test.ClusterName, test.NamespaceName, []string{key.TeleportOperatorFinalizer}, time.Time{}),
 			expectedCluster:   test.NewCluster(test.ClusterName, test.NamespaceName, []string{key.TeleportOperatorFinalizer}, time.Time{}),
 			expectedSecret:    test.NewSecret(test.ClusterName, test.NamespaceName, test.TokenName),
-			expectedConfigMap: test.NewConfigMap(test.ClusterName, test.AppName, test.NamespaceName, test.TokenName, []string{key.RoleKube, key.RoleApp}),
-			mcNamespace:       test.NamespaceName,
-			tokenRoles:        []string{key.RoleKube, key.RoleApp},
+			expectedConfigMap: test.NewConfigMap(test.ClusterName, test.AppName, test.NamespaceName, test.TokenName, []string{key.RoleKube}),
+			expectedRoles:     []string{key.RoleKube},
 		},
 		{
-			name:              "case 2: Update Secret and ConfigMap resources in case join token changes",
-			namespace:         test.NamespaceName,
-			token:             test.NewTokenName,
-			config:            newConfig(),
-			identity:          newIdentity(test.LastReadValue),
-			cluster:           test.NewCluster(test.ClusterName, test.NamespaceName, []string{key.TeleportOperatorFinalizer}, time.Time{}),
-			secret:            test.NewSecret(test.ClusterName, test.NamespaceName, test.TokenName),
-			configMap:         test.NewConfigMap(test.ClusterName, test.AppName, test.NamespaceName, test.TokenName, []string{key.RoleKube}),
+			name:      "case 3: Update Secret and ConfigMap resources in case join token changes",
+			namespace: test.NamespaceName,
+			token:     test.NewTokenName,
+			config:    newConfig(),
+			identity:  newIdentity(test.LastReadValue),
+			cluster:   test.NewCluster(test.ClusterName, test.NamespaceName, []string{key.TeleportOperatorFinalizer}, time.Time{}),
+			secret:    test.NewSecret(test.ClusterName, test.NamespaceName, test.TokenName),
+			configMap: test.NewConfigMap(test.ClusterName, test.AppName, test.NamespaceName, test.TokenName, []string{key.RoleKube}),
+			userValuesConfigMap: &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      fmt.Sprintf("%s-teleport-kube-agent-user-values", test.ClusterName),
+					Namespace: test.NamespaceName,
+				},
+				Data: map[string]string{
+					"values": "apps:\n- name: test-app\n  uri: http://test-app",
+				},
+			},
 			expectedCluster:   test.NewCluster(test.ClusterName, test.NamespaceName, []string{key.TeleportOperatorFinalizer}, time.Time{}),
 			expectedSecret:    test.NewSecret(test.ClusterName, test.NamespaceName, test.NewTokenName),
-			expectedConfigMap: test.NewConfigMap(test.ClusterName, test.AppName, test.NamespaceName, test.NewTokenName, []string{key.RoleKube}),
-			tokenRoles:        []string{key.RoleKube, key.RoleApp},
+			expectedConfigMap: test.NewConfigMap(test.ClusterName, test.AppName, test.NamespaceName, test.NewTokenName, []string{key.RoleKube, key.RoleApp}),
+			expectedRoles:     []string{key.RoleKube, key.RoleApp},
 		},
 		{
-			name:       "case 3: Deregister cluster and delete resources in case the cluster is deleted",
-			namespace:  test.NamespaceName,
-			token:      test.TokenName,
-			config:     newConfig(),
-			identity:   newIdentity(test.LastReadValue),
-			cluster:    test.NewCluster(test.ClusterName, test.NamespaceName, []string{key.TeleportOperatorFinalizer}, time.Now()),
-			secret:     test.NewSecret(test.ClusterName, test.NamespaceName, test.TokenName),
-			configMap:  test.NewConfigMap(test.ClusterName, test.AppName, test.NamespaceName, test.TokenName, []string{key.RoleKube}),
-			tokenRoles: []string{key.RoleKube, key.RoleApp},
+			name:          "case 4: Deregister cluster and delete resources in case the cluster is deleted",
+			namespace:     test.NamespaceName,
+			token:         test.TokenName,
+			config:        newConfig(),
+			identity:      newIdentity(test.LastReadValue),
+			cluster:       test.NewCluster(test.ClusterName, test.NamespaceName, []string{key.TeleportOperatorFinalizer}, time.Now()),
+			secret:        test.NewSecret(test.ClusterName, test.NamespaceName, test.TokenName),
+			configMap:     test.NewConfigMap(test.ClusterName, test.AppName, test.NamespaceName, test.TokenName, []string{key.RoleKube}),
+			expectedRoles: []string{key.RoleKube},
 		},
 		{
-			name:           "case 4: Reconnect to Teleport when credentials are rotated",
+			name:           "case 5: Reconnect to Teleport when credentials are rotated",
 			namespace:      test.NamespaceName,
 			token:          test.NewTokenName,
 			config:         newConfig(),
@@ -102,16 +143,25 @@ func Test_ClusterController(t *testing.T) {
 			secret:         test.NewSecret(test.ClusterName, test.NamespaceName, test.TokenName),
 			identitySecret: test.NewIdentitySecret(test.NamespaceName, test.IdentityFileValue),
 			configMap:      test.NewConfigMap(test.ClusterName, test.AppName, test.NamespaceName, test.TokenName, []string{key.RoleKube}),
+			userValuesConfigMap: &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      fmt.Sprintf("%s-teleport-kube-agent-user-values", test.ClusterName),
+					Namespace: test.NamespaceName,
+				},
+				Data: map[string]string{
+					"values": "apps:\n- name: test-app\n  uri: http://test-app",
+				},
+			},
 			newTeleportClient: func(ctx context.Context, proxyAddr, identityFile string) (teleport.Client, error) {
 				return test.NewTeleportClient(test.FakeTeleportClientConfig{Tokens: nil}), nil
 			},
 			expectedCluster:   test.NewCluster(test.ClusterName, test.NamespaceName, []string{key.TeleportOperatorFinalizer}, time.Time{}),
 			expectedSecret:    test.NewSecret(test.ClusterName, test.NamespaceName, test.NewTokenName),
-			expectedConfigMap: test.NewConfigMap(test.ClusterName, test.AppName, test.NamespaceName, test.NewTokenName, []string{key.RoleKube}),
-			tokenRoles:        []string{key.RoleKube, key.RoleApp},
+			expectedConfigMap: test.NewConfigMap(test.ClusterName, test.AppName, test.NamespaceName, test.NewTokenName, []string{key.RoleKube, key.RoleApp}),
+			expectedRoles:     []string{key.RoleKube, key.RoleApp},
 		},
 		{
-			name:      "case 5: Return an error in case reconnection to Teleport fails after the credentials are rotated",
+			name:      "case 6: Return an error in case reconnection to Teleport fails after the credentials are rotated",
 			namespace: test.NamespaceName,
 			token:     test.TokenName,
 			config:    newConfig(),
@@ -123,7 +173,7 @@ func Test_ClusterController(t *testing.T) {
 				return nil, errors.New("simulated error")
 			},
 			expectedError: errors.New("secrets \"identity-output\" not found"),
-			tokenRoles:    []string{key.RoleKube, key.RoleApp},
+			expectedRoles: []string{key.RoleKube},
 		},
 	}
 
@@ -142,6 +192,9 @@ func Test_ClusterController(t *testing.T) {
 			if tc.identitySecret != nil {
 				runtimeObjects = append(runtimeObjects, tc.identitySecret)
 			}
+			if tc.userValuesConfigMap != nil {
+				runtimeObjects = append(runtimeObjects, tc.userValuesConfigMap)
+			}
 
 			newTeleportClient := teleport.NewClient
 			if tc.newTeleportClient != nil {
@@ -166,13 +219,12 @@ func Test_ClusterController(t *testing.T) {
 				Namespace:    tc.namespace,
 				Teleport:     teleport.New(tc.namespace, tc.config, test.NewMockTokenGenerator(tc.token)),
 				IsBotEnabled: false,
-				TokenRoles:   tc.tokenRoles,
-				MCNamespace:  tc.mcNamespace,
 			}
 			controller.Teleport.TeleportClient = test.NewTeleportClient(test.FakeTeleportClientConfig{
 				Tokens: tc.tokens,
 			})
 			controller.Teleport.Identity = tc.identity
+			controller.Teleport.Client = ctrlClient
 
 			req := ctrl.Request{
 				NamespacedName: types.NamespacedName{
@@ -245,10 +297,21 @@ func Test_ClusterController(t *testing.T) {
 			if err != nil {
 				t.Fatalf("unexpected error %v", err)
 			}
+
+			// Check if the roles were set correctly
+			// This assumes you've added a method to expose the last assigned roles for testing
+			assignedRoles := controller.GetLastAssignedRoles()
+			if !reflect.DeepEqual(assignedRoles, tc.expectedRoles) {
+				t.Errorf("Expected roles %v, but got %v", tc.expectedRoles, assignedRoles)
+			}
 		})
 	}
 }
 
+func (r *ClusterReconciler) GetLastAssignedRoles() []string {
+	return r.lastAssignedRoles
+}
+
 func newConfig() *config.Config {
 	return &config.Config{
 		AppCatalog:            test.AppCatalog,