address PR comments

vertica · Feb 4, 2025 · 707f441 · 707f441
1 parent 31a7e50
commit 707f441
Show file tree

Hide file tree

Showing 7 changed files with 62 additions and 43 deletions.
diff --git a/api/v1/verticadb_types.go b/api/v1/verticadb_types.go
@@ -294,7 +294,7 @@ type VerticaDBSpec struct {
 	// tls.crt and ca.crt.  To store this secret outside of Kubernetes, you can
 	// use a secret path reference prefix, such as gsm://. Everything after the
 	// prefix is the name of the secret in the service you are storing.
-	ClientServerTLSSecret string `json:"clientTLSSecret,omitempty"`
+	ClientServerTLSSecret string `json:"clientServerTLSSecret,omitempty"`
 
 	// +operator-sdk:csv:customresourcedefinitions:type=spec,xDescriptors="urn:alm:descriptor:com.tectonic.ui:hidden"
 	// +kubebuilder:default:=verify_ca

diff --git a/api/v1/verticadb_webhook.go b/api/v1/verticadb_webhook.go
@@ -227,6 +227,7 @@ func (v *VerticaDB) validateVerticaDBSpec() field.ErrorList {
 	allErrs = v.isServiceTypeValid(allErrs)
 	allErrs = v.hasDuplicateScName(allErrs)
 	allErrs = v.hasValidVolumeName(allErrs)
+	allErrs = v.hasValidClientServerTLSMode(allErrs)
 	allErrs = v.hasValidVolumeMountName(allErrs)
 	allErrs = v.hasValidKerberosSetup(allErrs)
 	allErrs = v.hasValidTemporarySubclusterRouting(allErrs)
@@ -669,6 +670,26 @@ func (v *VerticaDB) hasDuplicateScName(allErrs field.ErrorList) field.ErrorList
 	return allErrs
 }
 
+func (v *VerticaDB) hasValidClientServerTLSMode(allErrs field.ErrorList) field.ErrorList {
+	var tlsModes []string = []string{"prefer", "require", "verify_ca", "verify_full"}
+	if v.Spec.ClientServerTLSMode != "" {
+		TLSMode := strings.ToLower(v.Spec.ClientServerTLSMode)
+		validMode := false
+		for _, mode := range tlsModes {
+			if mode == TLSMode {
+				validMode = true
+			}
+		}
+		if !validMode {
+			err := field.Invalid(field.NewPath("spec").Child("clientservertlssecret"), v.Spec.ClientServerTLSMode, "invalid tls mode")
+			allErrs = append(allErrs, err)
+		}
+	} else {
+		v.Spec.ClientServerTLSMode = "verify_ca"
+	}
+	return allErrs
+}
+
 func (v *VerticaDB) hasValidVolumeName(allErrs field.ErrorList) field.ErrorList {
 	for i := range v.Spec.Volumes {
 		vol := v.Spec.Volumes[i]

diff --git a/pkg/builder/builder.go b/pkg/builder/builder.go
@@ -1862,3 +1862,25 @@ func GetTarballName(cmd []string) string {
 	}
 	return ""
 }
+
+// BuildNMATLSConfigMap builds a configmap with tls secret name it.
+// The configmap will be mapped to two environmental variables in NMA pod
+func BuildNMATLSConfigMap(vdb *vapi.VerticaDB) *corev1.ConfigMap {
+	secretMap := map[string]string{
+		NMASecretNamespaceEnv: vdb.ObjectMeta.Namespace,
+		NMASecretNameEnv:      vdb.Spec.NMATLSSecret,
+	}
+	tlsConfigMap := &corev1.ConfigMap{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "ConfigMap",
+			APIVersion: "v1",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:            vapi.NMATLSConfigMapName,
+			Namespace:       vdb.Namespace,
+			OwnerReferences: []metav1.OwnerReference{vdb.GenerateOwnerReference()},
+		},
+		Data: secretMap,
+	}
+	return tlsConfigMap
+}
diff --git a/pkg/controllers/vdb/nmacertconfigmapgen_reconciler.go b/pkg/controllers/vdb/nmacertconfigmapgen_reconciler.go
@@ -24,7 +24,6 @@ import (
 	"github.com/vertica/vertica-kubernetes/pkg/controllers"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 )
@@ -57,7 +56,7 @@ func (h *NMACertConfigMapGenReconciler) Reconcile(ctx context.Context, _ *ctrl.R
 	configMap := &corev1.ConfigMap{}
 	err := h.VRec.Client.Get(ctx, configMapName, configMap)
 	if errors.IsNotFound(err) {
-		configMap = h.buildTLSConfigMap(h.Vdb)
+		configMap = builder.BuildNMATLSConfigMap(h.Vdb)
 		err = h.VRec.Client.Create(ctx, configMap)
 		return ctrl.Result{}, err
 	}
@@ -83,24 +82,3 @@ func (h *NMACertConfigMapGenReconciler) tlsSecretsReady(ctx context.Context) boo
 	}
 	return true
 }
-
-// buildTLSConfigMap return a ConfigMap. Key is the json file name and value is the json file content
-func (h *NMACertConfigMapGenReconciler) buildTLSConfigMap(vdb *vapi.VerticaDB) *corev1.ConfigMap {
-	secretMap := map[string]string{
-		builder.NMASecretNamespaceEnv: vdb.ObjectMeta.Namespace,
-		builder.NMASecretNameEnv:      vdb.Spec.NMATLSSecret,
-	}
-	tlsConfigMap := &corev1.ConfigMap{
-		TypeMeta: metav1.TypeMeta{
-			Kind:       "ConfigMap",
-			APIVersion: "v1",
-		},
-		ObjectMeta: metav1.ObjectMeta{
-			Name:            vapi.NMATLSConfigMapName,
-			Namespace:       vdb.Namespace,
-			OwnerReferences: []metav1.OwnerReference{h.Vdb.GenerateOwnerReference()},
-		},
-		Data: secretMap,
-	}
-	return tlsConfigMap
-}
diff --git a/pkg/controllers/vdb/nmaservercertgen_reconciler.go b/pkg/controllers/vdb/nmaservercertgen_reconciler.go
@@ -35,7 +35,7 @@ import (
 )
 
 // NMACertGenReconciler will create a secret that has TLS credentials.  This
-// secret will be used to authenticate with the http server.
+// secret will be used to authenticate with the https server.
 type NMACertGenReconciler struct {
 	VRec *VerticaDBReconciler
 	Vdb  *vapi.VerticaDB // Vdb is the CRD we are acting on.
@@ -46,11 +46,11 @@ func MakeNMACertGenReconciler(vdbrecon *VerticaDBReconciler, log logr.Logger, vd
 	return &NMACertGenReconciler{
 		VRec: vdbrecon,
 		Vdb:  vdb,
-		Log:  log.WithName("HTTPServerCertGenReconciler"),
+		Log:  log.WithName("NMACertGenReconciler"),
 	}
 }
 
-// Reconcile will create a TLS secret for the http server if one is missing
+// Reconcile will create a TLS secret for the https server if one is missing
 func (h *NMACertGenReconciler) Reconcile(ctx context.Context, _ *ctrl.Request) (ctrl.Result, error) {
 	// If the secret name is set, check that it exists.
 	if h.Vdb.Spec.NMATLSSecret != "" {

diff --git a/pkg/controllers/vdb/tlsservercertgen_reconciler.go b/pkg/controllers/vdb/tlsservercertgen_reconciler.go
@@ -35,8 +35,8 @@ import (
 )
 
 const (
-	HTTPSTLSSecret  = "HTTPSTLSSecret" // #nosec G101
-	ClientTLSSecret = "ClientTLSSecret"
+	ClientServerTLSSecret = "ClientServerTLSSecret"
+	NMATLSSecret          = "NMATLSSecret"
 )
 
 // TLSServerCertGenReconciler will create a secret that has TLS credentials.  This
@@ -51,15 +51,15 @@ func MakeTLSServerCertGenReconciler(vdbrecon *VerticaDBReconciler, log logr.Logg
 	return &TLSServerCertGenReconciler{
 		VRec: vdbrecon,
 		Vdb:  vdb,
-		Log:  log.WithName("HTTPSServerCertGenReconciler"),
+		Log:  log.WithName("TLSServerCertGenReconciler"),
 	}
 }
 
 // Reconcile will create a TLS secret for the http server if one is missing
 func (h *TLSServerCertGenReconciler) Reconcile(ctx context.Context, _ *ctrl.Request) (ctrl.Result, error) {
 	secretFieldNameMap := map[string]string{
-		HTTPSTLSSecret:  h.Vdb.Spec.HTTPSTLSSecret,
-		ClientTLSSecret: h.Vdb.Spec.ClientServerTLSSecret,
+		ClientServerTLSSecret: h.Vdb.Spec.ClientServerTLSSecret,
+		NMATLSSecret:          h.Vdb.Spec.NMATLSSecret,
 	}
 	err := error(nil)
 	result := ctrl.Result{}
@@ -91,7 +91,7 @@ func (h *TLSServerCertGenReconciler) reconcileOneSecret(secretFieldName, secretN
 			h.Log.Info(secretName+" is set but doesn't exist. Will recreate the secret.", "name", nm)
 		} else if err != nil {
 			return ctrl.Result{},
-				fmt.Errorf("failed while attempting to read the tls secret %s: %w", h.Vdb.Spec.HTTPSTLSSecret, err)
+				fmt.Errorf("failed while attempting to read the tls secret %s: %w", secretName, err)
 		} else {
 			// Secret is filled in and exists. We can exit.
 			return ctrl.Result{}, err
@@ -109,7 +109,7 @@ func (h *TLSServerCertGenReconciler) reconcileOneSecret(secretFieldName, secretN
 	if err != nil {
 		return ctrl.Result{}, err
 	}
-	h.Log.Info("created certificate and secret for https service")
+	h.Log.Info("created certificate and secret for %s", secret.Name)
 	return ctrl.Result{}, h.setSecretNameInVDB(ctx, secretFieldName, secret.ObjectMeta.Name)
 }
 
@@ -142,10 +142,10 @@ func (h *TLSServerCertGenReconciler) createSecret(secretFieldName, secretName st
 	// the name already present is the case where the name was filled in but the
 	// secret didn't exist.
 	if secretName == "" {
-		if secretFieldName == HTTPSTLSSecret {
-			secret.GenerateName = fmt.Sprintf("%s-https-tls-", h.Vdb.Name)
+		if secretFieldName == NMATLSSecret {
+			secret.GenerateName = fmt.Sprintf("%s-nma-tls-", h.Vdb.Name)
 		} else {
-			secret.GenerateName = fmt.Sprintf("%s-client-tls-", h.Vdb.Name)
+			secret.GenerateName = fmt.Sprintf("%s-clientserver-tls-", h.Vdb.Name)
 		}
 	} else {
 		secret.Name = secretName
@@ -162,10 +162,10 @@ func (h *TLSServerCertGenReconciler) setSecretNameInVDB(ctx context.Context, sec
 		if err := h.VRec.Client.Get(ctx, nm, h.Vdb); err != nil {
 			return err
 		}
-		if secretFieldName == HTTPSTLSSecret {
-			h.Vdb.Spec.HTTPSTLSSecret = secretName
-		} else {
+		if secretFieldName == ClientServerTLSSecret {
 			h.Vdb.Spec.ClientServerTLSSecret = secretName
+		} else if secretFieldName == NMATLSSecret {
+			h.Vdb.Spec.NMATLSSecret = secretName
 		}
 		return h.VRec.Client.Update(ctx, h.Vdb)
 	})

diff --git a/pkg/controllers/vdb/verticadb_controller.go b/pkg/controllers/vdb/verticadb_controller.go
@@ -184,9 +184,7 @@ func (r *VerticaDBReconciler) constructActors(log logr.Logger, vdb *vapi.Vertica
 		// Handle upgrade actions for any k8s objects created in prior versions
 		// of the operator.
 		MakeUpgradeOperatorReconciler(r, log, vdb),
-		// Create a TLS secret for the NMA service
-		MakeNMACertGenReconciler(r, log, vdb),
-		// use the same TLS secret used by the NMA service for https service
+		// use the TLS secrets used by the NMA service, https service and clientserver
 		MakeTLSServerCertGenReconciler(r, log, vdb),
 		// Create a ConfigMap to store secret names for all tls certs
 		MakeNMACertConfigMapGenReconciler(r, log, vdb),