Allow the ServiceAccount to be specified for vertica server pods

api/v1/verticadb_types.go

@@ -362,6 +362,14 @@ type VerticaDBSpec struct {
 	// here are applied to the default startup probe we create. If this is
 	// omitted, we use the default probe.
 	StartupProbeOverride *corev1.Probe `json:"startupProbeOverride,omitempty"`
+
+	// +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:io.kubernetes:ServiceAccount","urn:alm:descriptor:com.tectonic.ui:advanced"}
+	// +kubebuilder:validation:Optional
+	// The name of a serviceAccount to use to run the Vertica pods. If the
+	// service account is not specified or does not exist, the operator will
+	// create one, using the specified name if provided, along with a Role and
+	// RoleBinding.
+	ServiceAccountName string `json:"serviceAccountName,omitempty"`
 }
 
 // LocalObjectReference is used instead of corev1.LocalObjectReference and behaves the same.

api/v1beta1/verticadb_conversion.go

@@ -95,6 +95,7 @@ func convertToSpec(src *VerticaDBSpec) v1.VerticaDBSpec {
 		ReadinessProbeOverride:   src.ReadinessProbeOverride,
 		LivenessProbeOverride:    src.LivenessProbeOverride,
 		StartupProbeOverride:     src.StartupProbeOverride,
+		ServiceAccountName:       src.ServiceAccountName,
 	}
 	for i := range src.ReviveOrder {
 		dst.ReviveOrder[i] = v1.SubclusterPodCount(src.ReviveOrder[i])
@@ -148,6 +149,7 @@ func convertFromSpec(src *v1.VerticaDBSpec) VerticaDBSpec {
 		ReadinessProbeOverride:   src.ReadinessProbeOverride,
 		LivenessProbeOverride:    src.LivenessProbeOverride,
 		StartupProbeOverride:     src.StartupProbeOverride,
+		ServiceAccountName:       src.ServiceAccountName,
 	}
 	for i := range src.ReviveOrder {
 		dst.ReviveOrder[i] = SubclusterPodCount(src.ReviveOrder[i])

api/v1beta1/verticadb_types.go

@@ -353,6 +353,14 @@ type VerticaDBSpec struct {
 	// here are applied to the default startup probe we create. If this is
 	// omitted, we use the default probe.
 	StartupProbeOverride *corev1.Probe `json:"startupProbeOverride,omitempty"`
+
+	// +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors={"urn:alm:descriptor:io.kubernetes:ServiceAccount","urn:alm:descriptor:com.tectonic.ui:advanced"}
+	// +kubebuilder:validation:Optional
+	// The name of a serviceAccount to use to run the Vertica pods. If the
+	// service account is not specified or does not exist, the operator will
+	// create one, using the specified name if provided, along with a Role and
+	// RoleBinding.
+	ServiceAccountName string `json:"serviceAccountName,omitempty"`
 }
 
 // LocalObjectReference is used instead of corev1.LocalObjectReference and behaves the same.

changes/unreleased/Added-20230927-155433.yaml

@@ -0,0 +1,5 @@
+kind: Added
+body: Ability to specify pre-existing serviceAccount in the VerticaDB CR
+time: 2023-09-27T15:54:33.833815-03:00
+custom:
+  Issue: "521"

config/rbac/vertica-server-role.yaml

@@ -0,0 +1,29 @@
+# (c) Copyright [2021-2023] Open Text.
+# Licensed under the Apache License, Version 2.0 (the "License");
+# You may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# The vertica pod needs to run with a ServiceAccount that is bound to this
+# role. Use this if you intend to provide the ServiceAccount name in the
+# VerticaDB CR.
+
+apiVersion: rbac.authorization.k8s.io/v1
+kind: Role
+metadata:
+  name: vertica-server-role
+rules:
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get
+  - list

pkg/builder/builder.go

@@ -521,7 +521,7 @@ func buildDepotVolume() corev1.Volume {
 }
 
 // buildPodSpec creates a PodSpec for the statefulset
-func buildPodSpec(vdb *vapi.VerticaDB, sc *vapi.Subcluster, serviceAccountName string) corev1.PodSpec {
+func buildPodSpec(vdb *vapi.VerticaDB, sc *vapi.Subcluster) corev1.PodSpec {
 	termGracePeriod := int64(0)
 	return corev1.PodSpec{
 		NodeSelector:                  sc.NodeSelector,
@@ -531,7 +531,7 @@ func buildPodSpec(vdb *vapi.VerticaDB, sc *vapi.Subcluster, serviceAccountName s
 		Containers:                    makeContainers(vdb, sc),
 		Volumes:                       buildVolumes(vdb),
 		TerminationGracePeriodSeconds: &termGracePeriod,
-		ServiceAccountName:            serviceAccountName,
+		ServiceAccountName:            vdb.Spec.ServiceAccountName,
 		SecurityContext:               buildPodSecurityPolicy(vdb),
 	}
 }
@@ -846,7 +846,7 @@ func getStorageClassName(vdb *vapi.VerticaDB) *string {
 }
 
 // BuildStsSpec builds manifest for a subclusters statefulset
-func BuildStsSpec(nm types.NamespacedName, vdb *vapi.VerticaDB, sc *vapi.Subcluster, serviceAccountName string) *appsv1.StatefulSet {
+func BuildStsSpec(nm types.NamespacedName, vdb *vapi.VerticaDB, sc *vapi.Subcluster) *appsv1.StatefulSet {
 	isControllerRef := true
 	return &appsv1.StatefulSet{
 		ObjectMeta: metav1.ObjectMeta{
@@ -866,7 +866,7 @@ func BuildStsSpec(nm types.NamespacedName, vdb *vapi.VerticaDB, sc *vapi.Subclus
 					Labels:      MakeLabelsForPodObject(vdb, sc),
 					Annotations: MakeAnnotationsForObject(vdb),
 				},
-				Spec: buildPodSpec(vdb, sc, serviceAccountName),
+				Spec: buildPodSpec(vdb, sc),
 			},
 			UpdateStrategy:      makeUpdateStrategy(vdb),
 			PodManagementPolicy: appsv1.ParallelPodManagement,
@@ -912,7 +912,7 @@ func BuildPod(vdb *vapi.VerticaDB, sc *vapi.Subcluster, podIndex int32) *corev1.
 			Labels:      MakeLabelsForPodObject(vdb, sc),
 			Annotations: MakeAnnotationsForObject(vdb),
 		},
-		Spec: buildPodSpec(vdb, sc, "test-default"),
+		Spec: buildPodSpec(vdb, sc),
 	}
 	// Setup default values for the DC table annotations.  These are normally
 	// added by the AnnotationAndLabelPodReconciler.  However, this function is for test

pkg/builder/builder_test.go

@@ -172,7 +172,7 @@ var _ = Describe("builder", func() {
 
 	It("should have the fsGroup set for the dbadmin GID", func() {
 		vdb := vapi.MakeVDB()
-		c := buildPodSpec(vdb, &vdb.Spec.Subclusters[0], "test-sa")
+		c := buildPodSpec(vdb, &vdb.Spec.Subclusters[0])
 		Expect(*c.SecurityContext.FSGroup).Should(Equal(int64(5000)))
 	})
 
@@ -193,10 +193,10 @@ var _ = Describe("builder", func() {
 				},
 			},
 		}
-		c := buildPodSpec(vdb, &vdb.Spec.Subclusters[0], "test-sa")
+		c := buildPodSpec(vdb, &vdb.Spec.Subclusters[0])
 		Expect(isPasswdIncludedInPodInfo(vdb, &c)).Should(BeFalse())
 		vdb.Spec.StartupProbeOverride = nil
-		c = buildPodSpec(vdb, &vdb.Spec.Subclusters[0], "test-sa")
+		c = buildPodSpec(vdb, &vdb.Spec.Subclusters[0])
 		Expect(isPasswdIncludedInPodInfo(vdb, &c)).Should(BeTrue())
 	})
 
@@ -216,7 +216,7 @@ var _ = Describe("builder", func() {
 				},
 			},
 		}
-		c := buildPodSpec(vdb, &vdb.Spec.Subclusters[0], "test-sa")
+		c := buildPodSpec(vdb, &vdb.Spec.Subclusters[0])
 		Expect(c.Containers[0].ReadinessProbe.Exec).Should(BeNil())
 		Expect(c.Containers[0].ReadinessProbe.GRPC).ShouldNot(BeNil())
 		Expect(c.Containers[0].LivenessProbe.Exec).Should(BeNil())
@@ -230,7 +230,7 @@ var _ = Describe("builder", func() {
 		vdb.Annotations = map[string]string{
 			vmeta.GcpGsmAnnotation: "true",
 		}
-		c := buildPodSpec(vdb, &vdb.Spec.Subclusters[0], "test-sa")
+		c := buildPodSpec(vdb, &vdb.Spec.Subclusters[0])
 		Expect(isPasswdIncludedInPodInfo(vdb, &c)).Should(BeFalse())
 	})
 
@@ -242,7 +242,7 @@ var _ = Describe("builder", func() {
 				{Name: "net.ipv4.tcp_keepalive_intvl", Value: "5"},
 			},
 		}
-		c := buildPodSpec(vdb, &vdb.Spec.Subclusters[0], "test-sa")
+		c := buildPodSpec(vdb, &vdb.Spec.Subclusters[0])
 		Expect(*c.SecurityContext.FSGroup).Should(Equal(int64(5000)))
 		Expect(len(c.SecurityContext.Sysctls)).Should(Equal(2))
 		Expect(c.SecurityContext.Sysctls[0].Name).Should(Equal("net.ipv4.tcp_keepalive_time"))
@@ -254,7 +254,7 @@ var _ = Describe("builder", func() {
 	It("should mount ssh secret for dbadmin and root", func() {
 		vdb := vapi.MakeVDB()
 		vdb.Spec.SSHSecret = "my-secret"
-		c := buildPodSpec(vdb, &vdb.Spec.Subclusters[0], "test-sa")
+		c := buildPodSpec(vdb, &vdb.Spec.Subclusters[0])
 		cnt := &c.Containers[0]
 		i, ok := getFirstSSHSecretVolumeMountIndex(cnt)
 		Expect(ok).Should(BeTrue())

pkg/controllers/vdb/obj_reconciler.go

@@ -404,13 +404,9 @@ func (o *ObjReconciler) createService(ctx context.Context, svc *corev1.Service,
 // true if any create/update was done.
 func (o *ObjReconciler) reconcileSts(ctx context.Context, sc *vapi.Subcluster) (ctrl.Result, error) {
 	nm := names.GenStsName(o.Vdb, sc)
-	saName, err := o.getServiceAccountName(ctx)
-	if err != nil {
-		return ctrl.Result{}, err
-	}
 	curSts := &appsv1.StatefulSet{}
-	expSts := builder.BuildStsSpec(nm, o.Vdb, sc, saName)
-	err = o.VRec.Client.Get(ctx, nm, curSts)
+	expSts := builder.BuildStsSpec(nm, o.Vdb, sc)
+	err := o.VRec.Client.Get(ctx, nm, curSts)
 	if err != nil && errors.IsNotFound(err) {
 		o.Log.Info("Creating statefulset", "Name", nm, "Size", expSts.Spec.Replicas, "Image", expSts.Spec.Template.Spec.Containers[0].Image)
 		err = ctrl.SetControllerReference(o.Vdb, expSts, o.VRec.Scheme)
@@ -476,23 +472,6 @@ func (o *ObjReconciler) reconcileSts(ctx context.Context, sc *vapi.Subcluster) (
 	return ctrl.Result{}, nil
 }
 
-// getServiceAccountName returns the name of the service account to use for the vertica pods
-func (o *ObjReconciler) getServiceAccountName(ctx context.Context) (string, error) {
-	rbacFinder := iter.MakeRBACFinder(o.VRec.Client, o.Vdb)
-	exists, sa, err := rbacFinder.FindServiceAccount(ctx)
-	if err != nil {
-		return "", fmt.Errorf("error during service account lookup: %w", err)
-	}
-	if exists {
-		return sa.Name, nil
-	}
-	// For test purposes, we will use the default service account. k8s ensures
-	// this always exist in the namespace.
-	const DefaultServiceAccount = "default"
-	o.Log.Info("Count not find a service account for vdb using default", "name", DefaultServiceAccount)
-	return DefaultServiceAccount, nil
-}
-
 // checkIfReadyForStsUpdate will check whether it is okay to proceed
 // with the statefulset update.  This checks if we are deleting pods/sts and if
 // what we are deleting has had proper cleanup. In the case of admintools, failure to

pkg/controllers/vdb/serviceaccount_reconciler.go

@@ -24,9 +24,12 @@ import (
 	"github.com/vertica/vertica-kubernetes/pkg/builder"
 	"github.com/vertica/vertica-kubernetes/pkg/controllers"
 	"github.com/vertica/vertica-kubernetes/pkg/iter"
+	"github.com/vertica/vertica-kubernetes/pkg/names"
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/util/retry"
 	ctrl "sigs.k8s.io/controller-runtime"
 )
 
@@ -48,6 +51,14 @@ func MakeServiceAccountReconciler(vdbrecon *VerticaDBReconciler, log logr.Logger
 // Reconcile will ensure that a serviceAccount, role and rolebindings exists for
 // the vertica pods.
 func (s *ServiceAccountReconciler) Reconcile(ctx context.Context, _ *ctrl.Request) (ctrl.Result, error) {
+	// If a serviceAccount name was specified and it exists, then we can exit
+	// this reconciler early.
+	if s.Vdb.Spec.ServiceAccountName != "" {
+		if exists, err := s.hasUserProvidedServiceAccount(ctx, s.Vdb.Spec.ServiceAccountName); exists || err != nil {
+			return ctrl.Result{}, err
+		}
+	}
+
 	rbacFinder := iter.MakeRBACFinder(s.VRec.Client, s.Vdb)
 	exists, sa, err := rbacFinder.FindServiceAccount(ctx)
 	if err != nil {
@@ -82,7 +93,8 @@ func (s *ServiceAccountReconciler) Reconcile(ctx context.Context, _ *ctrl.Reques
 			return ctrl.Result{}, fmt.Errorf("failed to create rolebinding: %w", err)
 		}
 	}
-	return ctrl.Result{}, nil
+
+	return ctrl.Result{}, s.saveServiceAccountNameInVDB(ctx, sa.Name)
 }
 
 // createServiceAccount will create a new service account to be used for the vertica pods
@@ -91,10 +103,9 @@ func (s *ServiceAccountReconciler) createServiceAccount(ctx context.Context) (*c
 	blockOwnerDeletion := false
 	sa := corev1.ServiceAccount{
 		ObjectMeta: metav1.ObjectMeta{
-			GenerateName: fmt.Sprintf("%s-sa-", s.Vdb.Name),
-			Namespace:    s.Vdb.Namespace,
-			Annotations:  builder.MakeAnnotationsForObject(s.Vdb),
-			Labels:       builder.MakeCommonLabels(s.Vdb, nil, false),
+			Namespace:   s.Vdb.Namespace,
+			Annotations: builder.MakeAnnotationsForObject(s.Vdb),
+			Labels:      builder.MakeCommonLabels(s.Vdb, nil, false),
 			OwnerReferences: []metav1.OwnerReference{
 				{
 					APIVersion:         vapi.GroupVersion.String(),
@@ -107,6 +118,12 @@ func (s *ServiceAccountReconciler) createServiceAccount(ctx context.Context) (*c
 			},
 		},
 	}
+	// Only generate a name if one wasn't already specified in the vdb
+	if s.Vdb.Spec.ServiceAccountName == "" {
+		sa.GenerateName = fmt.Sprintf("%s-sa-", s.Vdb.Name)
+	} else {
+		sa.Name = s.Vdb.Spec.ServiceAccountName
+	}
 	err := s.VRec.Client.Create(ctx, &sa)
 	if err != nil {
 		err = fmt.Errorf("failed to create serviceaccount with generated name %s for VerticaDB: %w",
@@ -138,6 +155,8 @@ func (s *ServiceAccountReconciler) createRole(ctx context.Context) (*rbacv1.Role
 			},
 		},
 		Rules: []rbacv1.PolicyRule{
+			// Any policy changes here must be kept insync with:
+			// config/samples/vertica-server-role.yaml
 			{
 				// We need to allow vertica pods to read secrets directly from
 				// the API. This will be used by the NMA and vcluster CLI to
@@ -203,3 +222,42 @@ func (s *ServiceAccountReconciler) createRoleBinding(ctx context.Context, sa *co
 	s.Log.Info("rolebinding created", "name", rolebinding.ObjectMeta.Name)
 	return nil
 }
+
+// hasUserProvidedServiceAccount will check if the given serviceAccount name
+// exists and was user provided.
+func (s *ServiceAccountReconciler) hasUserProvidedServiceAccount(ctx context.Context, saName string) (bool, error) {
+	sa := corev1.ServiceAccount{}
+	nm := names.GenNamespacedName(s.Vdb, saName)
+	if err := s.VRec.Client.Get(ctx, nm, &sa); err != nil {
+		if errors.IsNotFound(err) {
+			return false, nil
+		}
+		return false, err
+	}
+	// Check if the serviceAccount has the expected labels. If it doesn't, then
+	// we assume the service account is user provided.
+	if sa.Labels == nil {
+		return true, nil
+	}
+	expLabels := builder.MakeCommonLabels(s.Vdb, nil, false)
+	for k, v := range expLabels {
+		if sa.Labels[k] != v {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
+// saveServiceAccountNameInVDB will store the given serviceAccountName in the VerticaDB.
+func (s *ServiceAccountReconciler) saveServiceAccountNameInVDB(ctx context.Context, saName string) error {
+	nm := s.Vdb.ExtractNamespacedName()
+	return retry.RetryOnConflict(retry.DefaultBackoff, func() error {
+		if err := s.VRec.Client.Get(ctx, nm, s.Vdb); err != nil {
+			return err
+		}
+
+		s.Vdb.Spec.ServiceAccountName = saName
+		s.Log.Info("Updating serviceAccountName in VerticaDB", "name", saName)
+		return s.VRec.Client.Update(ctx, s.Vdb)
+	})
+}