directly create SA if does not already exist

controllers/nstemplateset/client.go

@@ -2,6 +2,7 @@ package nstemplateset
 
 import (
 	"context"
+	"strings"
 
 	toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
 	applycl "github.com/codeready-toolchain/toolchain-common/pkg/client"
@@ -39,25 +40,32 @@ func (c APIClient) ApplyToolchainObjects(logger logr.Logger, toolchainObjects []
 		// Special handling of ServiceAccounts is required because if a ServiceAccount is reapplied when it already exists, it causes Kubernetes controllers to
 		// automatically create new Secrets for the ServiceAccounts. After enough time the number of Secrets created will hit the Secrets quota and then no new
 		// Secrets can be created. To prevent this from happening, we fetch the already existing SA, update labels and annotations only, and then call update using the same object (keeping the refs to secrets).
-		if object.GetObjectKind().GroupVersionKind().Kind == "ServiceAccount" {
+		if strings.EqualFold(object.GetObjectKind().GroupVersionKind().Kind, "ServiceAccount") {
+			logger.Info("the object is a ServiceAccount so we do the special handling for it...", "object_namespace", object.GetNamespace(), "object_name", object.GetObjectKind().GroupVersionKind().Kind+"/"+object.GetName())
 			sa := &v1.ServiceAccount{}
-			err := applyClient.Client.Get(context.TODO(), runtimeclient.ObjectKeyFromObject(object), sa)
+			err := applyClient.Get(context.TODO(), runtimeclient.ObjectKeyFromObject(object), sa)
 			if err != nil && !errors.IsNotFound(err) {
 				return anyApplied, err
 			}
 			// update labels and annotations for service account
-			if err == nil {
-				logger.Info("the object is a ServiceAccount and already exists - updating labels and annotations...")
+			if err != nil {
+				logger.Info("the ServiceAccount does not exists - creating...")
+				applycl.MergeLabels(object, newLabels)
+				if err := applyClient.Create(context.TODO(), object); err != nil {
+					return anyApplied, err
+				}
+			} else {
+				logger.Info("the ServiceAccount already exists - updating labels and annotations...")
 				applycl.MergeLabels(sa, newLabels)                    // add new labels to existing one
 				applycl.MergeLabels(sa, object.GetLabels())           // add new labels from template
 				applycl.MergeAnnotations(sa, object.GetAnnotations()) // add new annotations from template
 				err = applyClient.Update(context.TODO(), sa)
 				if err != nil {
 					return anyApplied, err
 				}
-				anyApplied = true
-				continue
 			}
+			anyApplied = true
+			continue
 		}
 		logger.Info("applying object", "object_namespace", object.GetNamespace(), "object_name", object.GetObjectKind().GroupVersionKind().Kind+"/"+object.GetName())
 		_, err := applyClient.Apply([]runtimeclient.Object{object}, newLabels)

controllers/nstemplateset/client_test.go

@@ -156,16 +156,25 @@ func TestApplyToolchainObjects(t *testing.T) {
 				"existing_label": "new_value", // existing label value was updated
 				"foo":            "bar",       // existing label was preserved
 			}
-			apiClient, fakeClient := prepareAPIClient(t)
-			_, err := client.NewApplyClient(fakeClient).Apply(copyObjects(sa), additionalLabel)
-			require.NoError(t, err)
+			originalSA := sa.DeepCopy()
+			client.MergeLabels(originalSA, additionalLabel)
+			apiClient, fakeClient := prepareAPIClient(t, originalSA)
+			called := false
+			fakeClient.MockGet = func(ctx context.Context, key runtimeclient.ObjectKey, obj runtimeclient.Object, opts ...runtimeclient.GetOption) error {
+				if key.Name == "appstudio-user-sa" {
+					require.False(t, called, "should be called only once for SA")
+					called = true
+				}
+				return fakeClient.Client.Get(ctx, key, obj, opts...)
+			}
 
 			// when
 			changed, err := apiClient.ApplyToolchainObjects(logger, copyObjects(newSaObject), newlabels)
 
 			// then
 			require.NoError(t, err)
 			assert.True(t, changed)
+			fakeClient.MockGet = nil
 			actualSA := &corev1.ServiceAccount{}
 			AssertObject(t, fakeClient, sa.Namespace, sa.Name, actualSA, func() {
 				// check that the Secrets field is still there
@@ -191,11 +200,20 @@ func TestApplyToolchainObjects(t *testing.T) {
 			apiClient, fakeClient := prepareAPIClient(t)
 			_, err := client.NewApplyClient(fakeClient).Apply(copyObjects(sa), additionalLabel)
 			require.NoError(t, err)
+			called := false
+			fakeClient.MockGet = func(ctx context.Context, key runtimeclient.ObjectKey, obj runtimeclient.Object, opts ...runtimeclient.GetOption) error {
+				if key.Name == "appstudio-user-sa" {
+					require.False(t, called, "should be called only once for SA")
+					called = true
+				}
+				return fakeClient.Client.Get(ctx, key, obj, opts...)
+			}
 			changed, err := apiClient.ApplyToolchainObjects(logger, copyObjects(newSaObject), additionalLabel)
 
 			// then
 			require.NoError(t, err)
 			assert.True(t, changed)
+			fakeClient.MockGet = nil
 			actualSA := &corev1.ServiceAccount{}
 			AssertObject(t, fakeClient, sa.Namespace, sa.Name, actualSA, func() {
 				// check that the Secrets field is still there
@@ -233,13 +251,22 @@ func TestApplyToolchainObjects(t *testing.T) {
 		apiClient, fakeClient := prepareAPIClient(t)
 		_, err := client.NewApplyClient(fakeClient).Apply(copyObjects(devNs, role), additionalLabel)
 		require.NoError(t, err)
+		called := false
+		fakeClient.MockGet = func(ctx context.Context, key runtimeclient.ObjectKey, obj runtimeclient.Object, opts ...runtimeclient.GetOption) error {
+			if key.Name == "appstudio-user-sa" {
+				require.False(t, called, "should be called only once for SA")
+				called = true
+			}
+			return fakeClient.Client.Get(ctx, key, obj, opts...)
+		}
 
 		// when
 		changed, err := apiClient.ApplyToolchainObjects(logger, copyObjects(sa), additionalLabel)
 
 		// then
 		require.NoError(t, err)
 		assert.True(t, changed)
+		fakeClient.MockGet = nil
 		assertObjects(t, fakeClient, false)
 	})
 }