From 707f441929b5544cdf8c1473a7a12a2816fa1003 Mon Sep 17 00:00:00 2001 From: Libo Yu Date: Tue, 4 Feb 2025 09:29:37 -0500 Subject: [PATCH] address PR comments --- api/v1/verticadb_types.go | 2 +- api/v1/verticadb_webhook.go | 21 +++++++++++++++ pkg/builder/builder.go | 22 ++++++++++++++++ .../vdb/nmacertconfigmapgen_reconciler.go | 24 +---------------- .../vdb/nmaservercertgen_reconciler.go | 6 ++--- .../vdb/tlsservercertgen_reconciler.go | 26 +++++++++---------- pkg/controllers/vdb/verticadb_controller.go | 4 +-- 7 files changed, 62 insertions(+), 43 deletions(-) diff --git a/api/v1/verticadb_types.go b/api/v1/verticadb_types.go index 275333e90..6558ba16f 100644 --- 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 index d1098edd1..dc89f6677 100644 --- 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 index a628176a1..c67522e16 100644 --- 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 index 716fde534..eeaf81fbf 100644 --- 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 index bb70bf6b4..5d8d01ffb 100644 --- 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 index 210b8e5e0..efbaeaad8 100644 --- 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 index e9b3db739..db92fa601 100644 --- 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),