From 33d762dc00a802979f303f20f8795f246e6dfadf Mon Sep 17 00:00:00 2001 From: Kamil Sambor Date: Fri, 26 Jan 2024 16:06:25 +0100 Subject: [PATCH] Sync refactor with tls enable patches --- controllers/placementapi_controller.go | 112 +++++++++++++++--- pkg/placement/dbsync.go | 2 - pkg/placement/deployment.go | 2 +- .../config/placement-api-config.json | 2 +- templates/placementapi/config/placement.conf | 2 +- .../placementapi_controller_test.go | 6 +- .../common/assert_sample_deployment.yaml | 6 +- .../tests/placement_deploy_tls/03-assert.yaml | 6 +- 8 files changed, 111 insertions(+), 27 deletions(-) diff --git a/controllers/placementapi_controller.go b/controllers/placementapi_controller.go index 4bcfc610..f85cc5e4 100644 --- a/controllers/placementapi_controller.go +++ b/controllers/placementapi_controller.go @@ -26,6 +26,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/kubernetes" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" @@ -51,6 +52,7 @@ import ( common_rbac "github.com/openstack-k8s-operators/lib-common/modules/common/rbac" "github.com/openstack-k8s-operators/lib-common/modules/common/secret" "github.com/openstack-k8s-operators/lib-common/modules/common/service" + "github.com/openstack-k8s-operators/lib-common/modules/common/tls" util "github.com/openstack-k8s-operators/lib-common/modules/common/util" mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1" @@ -315,7 +317,7 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request // // check for required OpenStack secret holding passwords for service/admin user and add hash to the vars map // - hash, result, _, err := ensureSecret( + hash, result, secret, err := ensureSecret( ctx, types.NamespacedName{Namespace: instance.Namespace, Name: instance.Spec.Secret}, []string{ @@ -346,7 +348,7 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request // all our input checks out so report InputReady instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage) - err = r.generateServiceConfigMaps(ctx, h, instance, &configMapVars) + err = r.generateServiceConfigMaps(ctx, h, instance, secret, &configMapVars) if err != nil { instance.Status.Conditions.Set(condition.FalseCondition( condition.ServiceConfigReadyCondition, @@ -357,6 +359,52 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrl.Result{}, err } + // TLS input validation + // + // Validate the CA cert secret if provided + if instance.Spec.TLS.CaBundleSecretName != "" { + hash, ctrlResult, err := tls.ValidateCACertSecret( + ctx, + h.GetClient(), + types.NamespacedName{ + Name: instance.Spec.TLS.CaBundleSecretName, + Namespace: instance.Namespace, + }, + ) + if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.TLSInputReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.TLSInputErrorMessage, + err.Error())) + return ctrlResult, err + } else if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil + } + + if hash != "" { + configMapVars[tls.CABundleKey] = env.SetValue(hash) + } + } + + // Validate API service certs secrets + certsHash, ctrlResult, err := instance.Spec.TLS.API.ValidateCertSecrets(ctx, h, instance.Namespace) + if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.TLSInputReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.TLSInputErrorMessage, + err.Error())) + return ctrlResult, err + } else if (ctrlResult != ctrl.Result{}) { + return ctrlResult, nil + } + configMapVars[tls.TLSHashName] = env.SetValue(certsHash) + + instance.Status.Conditions.MarkTrue(condition.TLSInputReadyCondition, condition.InputReadyMessage) + // create hash over all the different input resources to identify if any those changed // and a restart/recreate is required. // @@ -424,9 +472,10 @@ func (r *PlacementAPIReconciler) Reconcile(ctx context.Context, req ctrl.Request return ctrl.Result{}, nil } -func getServiceLabels() map[string]string { +func getServiceLabels(instance *placementv1.PlacementAPI) map[string]string { return map[string]string{ - common.AppSelector: placement.ServiceName, + common.AppSelector: placement.ServiceName, + common.OwnerSelector: instance.Name, } } @@ -451,7 +500,7 @@ func (r *PlacementAPIReconciler) ensureServiceExposed( } exportLabels := util.MergeStringMaps( - getServiceLabels(), + getServiceLabels(instance), map[string]string{ service.AnnotationEndpointKey: endpointTypeStr, }, @@ -463,7 +512,7 @@ func (r *PlacementAPIReconciler) ensureServiceExposed( Name: endpointName, Namespace: instance.Namespace, Labels: exportLabels, - Selector: getServiceLabels(), + Selector: getServiceLabels(instance), Port: service.GenericServicePort{ Name: endpointName, Port: data.Port, @@ -524,7 +573,12 @@ func (r *PlacementAPIReconciler) ensureServiceExposed( } // create service - end - // TODO: TLS, pass in https as protocol, create TLS cert + // if TLS is enabled + if instance.Spec.TLS.API.Enabled(endpointType) { + // set endpoint protocol to https + data.Protocol = ptr.To(service.ProtocolHTTPS) + } + apiEndpoints[string(endpointType)], err = svc.GetAPIEndpoint( svcOverride.EndpointURL, data.Protocol, data.Path) if err != nil { @@ -593,7 +647,7 @@ func (r *PlacementAPIReconciler) ensureKeystoneServiceUser( Secret: instance.Spec.Secret, PasswordSelector: instance.Spec.PasswordSelectors.Service, } - serviceLabels := getServiceLabels() + serviceLabels := getServiceLabels(instance) ksSvc := keystonev1.NewKeystoneService(ksSvcSpec, instance.Namespace, serviceLabels, time.Duration(10)*time.Second) _, err := ksSvc.CreateOrPatch(ctx, h) if err != nil { @@ -625,7 +679,7 @@ func (r *PlacementAPIReconciler) ensureKeystoneEndpoint( placement.ServiceName, instance.Namespace, ksEndptSpec, - getServiceLabels(), + getServiceLabels(instance), time.Duration(10)*time.Second, ) ctrlResult, err := ksEndpt.CreateOrPatch(ctx, h) @@ -733,6 +787,10 @@ func (r *PlacementAPIReconciler) initConditions( condition.RoleBindingReadyCondition, condition.InitReason, condition.RoleBindingReadyInitMessage), + condition.UnknownCondition( + condition.TLSInputReadyCondition, + condition.InitReason, + condition.InputReadyInitMessage), ) instance.Status.Conditions.Init(&cl) @@ -988,7 +1046,7 @@ func (r *PlacementAPIReconciler) ensureDbSync( serviceAnnotations map[string]string, ) (ctrl.Result, error) { Log := r.GetLogger(ctx) - serviceLabels := getServiceLabels() + serviceLabels := getServiceLabels(instance) dbSyncHash := instance.Status.Hash[placementv1.DbSyncHash] jobDef := placement.DbSyncJob(instance, serviceLabels, serviceAnnotations) dbSyncjob := job.NewJob( @@ -1037,10 +1095,20 @@ func (r *PlacementAPIReconciler) ensureDeployment( Log := r.GetLogger(ctx) Log.Info("Reconciling Service") - serviceLabels := getServiceLabels() + serviceLabels := getServiceLabels(instance) // Define a new Deployment object - deplDef := placement.Deployment(instance, inputHash, serviceLabels, serviceAnnotations) + deplDef, err := placement.Deployment(ctx, h, instance, inputHash, serviceLabels, serviceAnnotations) + + if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + condition.DeploymentReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + condition.DeploymentReadyErrorMessage, + err.Error())) + } + depl := deployment.NewDeployment( deplDef, time.Duration(5)*time.Second, @@ -1111,6 +1179,7 @@ func (r *PlacementAPIReconciler) generateServiceConfigMaps( ctx context.Context, h *helper.Helper, instance *placementv1.PlacementAPI, + ospSecret corev1.Secret, envVars *map[string]env.Setter, ) error { // @@ -1147,14 +1216,29 @@ func (r *PlacementAPIReconciler) generateServiceConfigMaps( "ServiceUser": instance.Spec.ServiceUser, "KeystoneInternalURL": keystoneInternalURL, "KeystonePublicURL": keystonePublicURL, - "PlacementPassword": instance.Spec.PasswordSelectors.Service, + "PlacementPassword": string(ospSecret.Data[instance.Spec.PasswordSelectors.Service]), "DBUser": instance.Spec.DatabaseUser, - "DBPassword": instance.Spec.PasswordSelectors.Database, + "DBPassword": string(ospSecret.Data[instance.Spec.PasswordSelectors.Database]), "DBAddress": instance.Status.DatabaseHostname, "DBName": placement.DatabaseName, "log_file": "/var/log/placement/placement-api.log", } + // create httpd vhost template parameters + httpdVhostConfig := map[string]interface{}{} + for _, endpt := range []service.Endpoint{service.EndpointInternal, service.EndpointPublic} { + endptConfig := map[string]interface{}{} + endptConfig["ServerName"] = fmt.Sprintf("placement-%s.%s.svc", endpt.String(), instance.Namespace) + endptConfig["TLS"] = false // default TLS to false, and set it bellow to true if enabled + if instance.Spec.TLS.API.Enabled(endpt) { + endptConfig["TLS"] = true + endptConfig["SSLCertificateFile"] = fmt.Sprintf("/etc/pki/tls/certs/%s.crt", endpt.String()) + endptConfig["SSLCertificateKeyFile"] = fmt.Sprintf("/etc/pki/tls/private/%s.key", endpt.String()) + } + httpdVhostConfig[endpt.String()] = endptConfig + } + templateParameters["VHosts"] = httpdVhostConfig + extraTemplates := map[string]string{ "placement.conf": "placementapi/config/placement.conf", } diff --git a/pkg/placement/dbsync.go b/pkg/placement/dbsync.go index 3170d07e..4b36444d 100644 --- a/pkg/placement/dbsync.go +++ b/pkg/placement/dbsync.go @@ -89,7 +89,5 @@ func DbSyncJob( }, } - job.Spec.Template.Spec.Volumes = getVolumes(instance.Name) - return job } diff --git a/pkg/placement/deployment.go b/pkg/placement/deployment.go index ab45eef3..592c1634 100644 --- a/pkg/placement/deployment.go +++ b/pkg/placement/deployment.go @@ -199,5 +199,5 @@ func Deployment( deployment.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector } - return deployment + return deployment, nil } diff --git a/templates/placementapi/config/placement-api-config.json b/templates/placementapi/config/placement-api-config.json index 2955d08a..36a2fefd 100644 --- a/templates/placementapi/config/placement-api-config.json +++ b/templates/placementapi/config/placement-api-config.json @@ -20,7 +20,7 @@ "perm": "0600" }, { - "source": "/var/lib/config-data/merged/ssl.conf", + "source": "/var/lib/config-data/ssl.conf", "dest": "/etc/httpd/conf.d/ssl.conf", "owner": "apache", "perm": "0644" diff --git a/templates/placementapi/config/placement.conf b/templates/placementapi/config/placement.conf index 5ee76710..aa69f5d0 100644 --- a/templates/placementapi/config/placement.conf +++ b/templates/placementapi/config/placement.conf @@ -9,7 +9,7 @@ log_file = {{ .log_file }} debug = true [placement_database] -connection = mysql+pymysql://{{ .DBUser }}:{{ .DBPassword}}@{{ .DBAddress }}/{{ .DBName }} +connection = mysql+pymysql://{{ .DBUser }}:{{ .DBPassword }}@{{ .DBAddress }}/{{ .DBName }} [api] auth_strategy = keystone diff --git a/tests/functional/placementapi_controller_test.go b/tests/functional/placementapi_controller_test.go index 462af74d..96d73b72 100644 --- a/tests/functional/placementapi_controller_test.go +++ b/tests/functional/placementapi_controller_test.go @@ -252,7 +252,9 @@ var _ = Describe("PlacementAPI controller", func() { Expect(cm.Data["placement.conf"]).Should( ContainSubstring("username = placement")) Expect(cm.Data["placement.conf"]).Should( - ContainSubstring("connection = mysql+pymysql://placement:PlacementDatabasePassword@/placement")) + ContainSubstring("password = 12345678")) + Expect(cm.Data["placement.conf"]).Should( + ContainSubstring("connection = mysql+pymysql://placement:12345678@/placement")) }) It("creates service account, role and rolebindig", func() { @@ -757,7 +759,7 @@ var _ = Describe("PlacementAPI controller", func() { Expect(container.ReadinessProbe.HTTPGet.Scheme).To(Equal(corev1.URISchemeHTTPS)) Expect(container.LivenessProbe.HTTPGet.Scheme).To(Equal(corev1.URISchemeHTTPS)) - configDataMap := th.GetConfigMap(names.ConfigMapName) + configDataMap := th.GetSecret(names.ConfigMapName) Expect(configDataMap).ShouldNot(BeNil()) Expect(configDataMap.Data).Should(HaveKey("httpd.conf")) Expect(configDataMap.Data).Should(HaveKey("ssl.conf")) diff --git a/tests/kuttl/common/assert_sample_deployment.yaml b/tests/kuttl/common/assert_sample_deployment.yaml index 454e47da..62cd65b4 100644 --- a/tests/kuttl/common/assert_sample_deployment.yaml +++ b/tests/kuttl/common/assert_sample_deployment.yaml @@ -145,10 +145,10 @@ spec: - mountPath: /usr/local/bin/container-scripts name: scripts readOnly: true - - mountPath: /var/lib/config-data/merged - name: config-data-merged + - mountPath: /var/lib/config-data/ + name: config-data - mountPath: /var/lib/kolla/config_files/config.json - name: config-data-merged + name: config-data readOnly: true subPath: placement-api-config.json - mountPath: /var/log/placement diff --git a/tests/kuttl/tests/placement_deploy_tls/03-assert.yaml b/tests/kuttl/tests/placement_deploy_tls/03-assert.yaml index 684418c3..9664d916 100644 --- a/tests/kuttl/tests/placement_deploy_tls/03-assert.yaml +++ b/tests/kuttl/tests/placement_deploy_tls/03-assert.yaml @@ -152,10 +152,10 @@ spec: - mountPath: /usr/local/bin/container-scripts name: scripts readOnly: true - - mountPath: /var/lib/config-data/merged - name: config-data-merged + - mountPath: /var/lib/config-data + name: config-data - mountPath: /var/lib/kolla/config_files/config.json - name: config-data-merged + name: config-data readOnly: true subPath: placement-api-config.json - mountPath: /var/log/placement