Skip to content

Commit

Permalink
Sync refactor with tls enable patches
Browse files Browse the repository at this point in the history
  • Loading branch information
mrkisaolamb committed Jan 30, 2024
1 parent 5daf412 commit 69401dc
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 53 deletions.
112 changes: 98 additions & 14 deletions controllers/placementapi_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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,
Expand All @@ -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.
//
Expand Down Expand Up @@ -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,
}
}

Expand All @@ -451,7 +500,7 @@ func (r *PlacementAPIReconciler) ensureServiceExposed(
}

exportLabels := util.MergeStringMaps(
getServiceLabels(),
getServiceLabels(instance),
map[string]string{
service.AnnotationEndpointKey: endpointTypeStr,
},
Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
//
Expand Down Expand Up @@ -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",
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/placement/dbsync.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,5 @@ func DbSyncJob(
},
}

job.Spec.Template.Spec.Volumes = getVolumes(instance.Name)

return job
}
2 changes: 1 addition & 1 deletion pkg/placement/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,5 +199,5 @@ func Deployment(
deployment.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
}

return deployment
return deployment, nil
}
2 changes: 1 addition & 1 deletion templates/placementapi/config/placement-api-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion templates/placementapi/config/placement.conf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions tests/functional/placementapi_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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"))
Expand Down
20 changes: 4 additions & 16 deletions tests/kuttl/common/assert_sample_deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -229,19 +229,7 @@ spec:
type: ClusterIP
---
apiVersion: v1
kind: ConfigMap
metadata:
labels:
placement.openstack.org/name: placement
name: placement-scripts
ownerReferences:
- blockOwnerDeletion: true
controller: true
kind: PlacementAPI
name: placement
---
apiVersion: v1
kind: ConfigMap
kind: Secret
metadata:
labels:
placement.openstack.org/name: placement
Expand Down
14 changes: 1 addition & 13 deletions tests/kuttl/common/errors_cleanup_placement.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -70,19 +70,7 @@ spec:
type: ClusterIP
---
apiVersion: v1
kind: ConfigMap
metadata:
labels:
placement.openstack.org/name: placement
name: placement-scripts
ownerReferences:
- blockOwnerDeletion: true
controller: true
kind: PlacementAPI
name: placement
---
apiVersion: v1
kind: ConfigMap
kind: Secret
metadata:
labels:
placement.openstack.org/name: placement
Expand Down
6 changes: 3 additions & 3 deletions tests/kuttl/tests/placement_deploy_tls/03-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 69401dc

Please sign in to comment.