From 9ce8da0b37ea7fd592f860a9fb934e23ab348817 Mon Sep 17 00:00:00 2001 From: TheCodeAssassin Date: Thu, 9 May 2024 11:34:50 +0200 Subject: [PATCH] feat: making DataStore TLS configuration optional Co-authored-by: Dario Tranchitella --- .golangci.yml | 2 + Makefile | 1 + api/v1alpha1/datastore_types.go | 5 +- api/v1alpha1/indexer_datastore_usedsecret.go | 24 +- api/v1alpha1/zz_generated.deepcopy.go | 12 +- charts/kamaji/README.md | 3 +- charts/kamaji/crds/datastore.yaml | 2 - charts/kamaji/templates/datastore.yaml | 5 + charts/kamaji/values.yaml | 5 +- .../bases/kamaji.clastix.io_datastores.yaml | 7 +- config/install.yaml | 6 +- .../kamaji_v1alpha1_datastore_nats_notls.yaml | 17 + deploy/kine/nats/Makefile | 4 +- deploy/kine/nats/values-notls.yaml | 14 + docs/content/reference/api.md | 394 +++++++++--------- e2e/tcp_nats_ready_no_tls_test.go | 54 +++ internal/builders/controlplane/deployment.go | 89 ++-- internal/datastore/connection.go | 10 +- internal/datastore/datastore.go | 53 ++- internal/datastore/mysql.go | 8 +- .../datastore/datastore_certificate.go | 136 +++--- internal/webhook/handlers/ds_validate.go | 4 + 22 files changed, 507 insertions(+), 348 deletions(-) create mode 100644 config/samples/kamaji_v1alpha1_datastore_nats_notls.yaml create mode 100644 deploy/kine/nats/values-notls.yaml create mode 100644 e2e/tcp_nats_ready_no_tls_test.go diff --git a/.golangci.yml b/.golangci.yml index f1fec67..c4cde2f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -37,6 +37,8 @@ linters: - funlen - dupl - cyclop + - gocognit + - nestif # deprecated linters - deadcode - golint diff --git a/Makefile b/Makefile index 2095f1a..ba252da 100644 --- a/Makefile +++ b/Makefile @@ -151,6 +151,7 @@ datastore-nats: helm $(MAKE) NAME=bronze _datastore-nats $(MAKE) NAME=silver _datastore-nats $(MAKE) NAME=gold _datastore-nats + $(MAKE) NAME=notls _datastore-nats datastores: datastore-mysql datastore-etcd datastore-postgres datastore-nats ## Install all Kamaji DataStores with multiple drivers, and different tiers. diff --git a/api/v1alpha1/datastore_types.go b/api/v1alpha1/datastore_types.go index b54453a..171dd8a 100644 --- a/api/v1alpha1/datastore_types.go +++ b/api/v1alpha1/datastore_types.go @@ -34,7 +34,8 @@ type DataStoreSpec struct { // This value is optional. BasicAuth *BasicAuth `json:"basicAuth,omitempty"` // Defines the TLS/SSL configuration required to connect to the data store in a secure way. - TLSConfig TLSConfig `json:"tlsConfig"` + // This value is optional. + TLSConfig *TLSConfig `json:"tlsConfig,omitempty"` } // TLSConfig contains the information used to connect to the data store using a secured connection. @@ -43,7 +44,7 @@ type TLSConfig struct { // The key reference is required since etcd authentication is based on certificates, and Kamaji is responsible in creating this. CertificateAuthority CertKeyPair `json:"certificateAuthority"` // Specifies the SSL/TLS key and private key pair used to connect to the data store. - ClientCertificate ClientCertificate `json:"clientCertificate"` + ClientCertificate *ClientCertificate `json:"clientCertificate,omitempty"` } type ClientCertificate struct { diff --git a/api/v1alpha1/indexer_datastore_usedsecret.go b/api/v1alpha1/indexer_datastore_usedsecret.go index 1e481f1..45c4262 100644 --- a/api/v1alpha1/indexer_datastore_usedsecret.go +++ b/api/v1alpha1/indexer_datastore_usedsecret.go @@ -43,20 +43,22 @@ func (d *DatastoreUsedSecret) ExtractValue() client.IndexerFunc { } } - if ds.Spec.TLSConfig.CertificateAuthority.Certificate.SecretRef != nil { - res = append(res, d.namespacedName(*ds.Spec.TLSConfig.CertificateAuthority.Certificate.SecretRef)) - } + if ds.Spec.TLSConfig != nil { + if ds.Spec.TLSConfig.CertificateAuthority.Certificate.SecretRef != nil { + res = append(res, d.namespacedName(*ds.Spec.TLSConfig.CertificateAuthority.Certificate.SecretRef)) + } - if ds.Spec.TLSConfig.CertificateAuthority.PrivateKey != nil && ds.Spec.TLSConfig.CertificateAuthority.PrivateKey.SecretRef != nil { - res = append(res, d.namespacedName(*ds.Spec.TLSConfig.CertificateAuthority.PrivateKey.SecretRef)) - } + if ds.Spec.TLSConfig.CertificateAuthority.PrivateKey != nil && ds.Spec.TLSConfig.CertificateAuthority.PrivateKey.SecretRef != nil { + res = append(res, d.namespacedName(*ds.Spec.TLSConfig.CertificateAuthority.PrivateKey.SecretRef)) + } - if ds.Spec.TLSConfig.ClientCertificate.Certificate.SecretRef != nil { - res = append(res, d.namespacedName(*ds.Spec.TLSConfig.ClientCertificate.Certificate.SecretRef)) - } + if ds.Spec.TLSConfig.ClientCertificate.Certificate.SecretRef != nil { + res = append(res, d.namespacedName(*ds.Spec.TLSConfig.ClientCertificate.Certificate.SecretRef)) + } - if ds.Spec.TLSConfig.ClientCertificate.PrivateKey.SecretRef != nil { - res = append(res, d.namespacedName(*ds.Spec.TLSConfig.ClientCertificate.PrivateKey.SecretRef)) + if ds.Spec.TLSConfig.ClientCertificate.PrivateKey.SecretRef != nil { + res = append(res, d.namespacedName(*ds.Spec.TLSConfig.ClientCertificate.PrivateKey.SecretRef)) + } } return res diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 91c2eee..b1eeb99 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -525,7 +525,11 @@ func (in *DataStoreSpec) DeepCopyInto(out *DataStoreSpec) { *out = new(BasicAuth) (*in).DeepCopyInto(*out) } - in.TLSConfig.DeepCopyInto(&out.TLSConfig) + if in.TLSConfig != nil { + in, out := &in.TLSConfig, &out.TLSConfig + *out = new(TLSConfig) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DataStoreSpec. @@ -1201,7 +1205,11 @@ func (in *StorageStatus) DeepCopy() *StorageStatus { func (in *TLSConfig) DeepCopyInto(out *TLSConfig) { *out = *in in.CertificateAuthority.DeepCopyInto(&out.CertificateAuthority) - in.ClientCertificate.DeepCopyInto(&out.ClientCertificate) + if in.ClientCertificate != nil { + in, out := &in.ClientCertificate, &out.ClientCertificate + *out = new(ClientCertificate) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TLSConfig. diff --git a/charts/kamaji/README.md b/charts/kamaji/README.md index 6222e43..4b6e139 100644 --- a/charts/kamaji/README.md +++ b/charts/kamaji/README.md @@ -77,7 +77,7 @@ Here the values you can override: | datastore.driver | string | `"etcd"` | (string) The Kamaji Datastore driver, supported: etcd, MySQL, PostgreSQL (defaults=etcd). | | datastore.enabled | bool | `true` | (bool) Enable the Kamaji Datastore creation (default=true) | | datastore.endpoints | list | `[]` | (array) List of endpoints of the selected Datastore. When letting the Chart install the etcd datastore, this field is populated automatically. | -| datastore.nameOverride | string | `nil` | The Datastore name override, if empty and enabled=true defaults to `default`, if enabled=false, this is the name of the Datastore to connect to. | +| datastore.nameOverride | string | `nil` | The Datastore name override, if empty and enabled=true defaults to `default`, if enabled=false, this is the name of the Datastore to connect to. | | datastore.tlsConfig.certificateAuthority.certificate.keyPath | string | `nil` | Key of the Secret which contains the content of the certificate. | | datastore.tlsConfig.certificateAuthority.certificate.name | string | `nil` | Name of the Secret containing the CA required to establish the mandatory SSL/TLS connection to the datastore. | | datastore.tlsConfig.certificateAuthority.certificate.namespace | string | `nil` | Namespace of the Secret containing the CA required to establish the mandatory SSL/TLS connection to the datastore. | @@ -90,6 +90,7 @@ Here the values you can override: | datastore.tlsConfig.clientCertificate.privateKey.keyPath | string | `nil` | Key of the Secret which contains the content of the private key. | | datastore.tlsConfig.clientCertificate.privateKey.name | string | `nil` | Name of the Secret containing the client certificate private key required to establish the mandatory SSL/TLS connection to the datastore. | | datastore.tlsConfig.clientCertificate.privateKey.namespace | string | `nil` | Namespace of the Secret containing the client certificate private key required to establish the mandatory SSL/TLS connection to the datastore. | +| datastore.tlsConfig.enabled | bool | `true` | | | etcd.compactionInterval | int | `0` | ETCD Compaction interval (e.g. "5m0s"). (default: "0" (disabled)) | | etcd.deploy | bool | `true` | Install an etcd with enabled multi-tenancy along with Kamaji | | etcd.image | object | `{"pullPolicy":"IfNotPresent","repository":"quay.io/coreos/etcd","tag":"v3.5.6"}` | Install specific etcd image | diff --git a/charts/kamaji/crds/datastore.yaml b/charts/kamaji/crds/datastore.yaml index 142f6ba..dacda71 100644 --- a/charts/kamaji/crds/datastore.yaml +++ b/charts/kamaji/crds/datastore.yaml @@ -256,12 +256,10 @@ spec: type: object required: - certificateAuthority - - clientCertificate type: object required: - driver - endpoints - - tlsConfig type: object status: description: DataStoreStatus defines the observed state of DataStore. diff --git a/charts/kamaji/templates/datastore.yaml b/charts/kamaji/templates/datastore.yaml index b54ef99..08631b9 100644 --- a/charts/kamaji/templates/datastore.yaml +++ b/charts/kamaji/templates/datastore.yaml @@ -20,9 +20,14 @@ spec: secretReference: {{- .Values.datastore.basicAuth.passwordSecret | toYaml | nindent 8 }} {{- end }} +{{- if .Values.datastore.tlsConfig.enabled }} tlsConfig: certificateAuthority: {{- include "datastore.certificateAuthority" . | indent 6 }} + + {{- if .Values.datastore.tlsConfig.clientCertificate }} clientCertificate: {{- include "datastore.clientCertificate" . | indent 6 }} + {{- end }} +{{- end}} {{- end}} diff --git a/charts/kamaji/values.yaml b/charts/kamaji/values.yaml index 814f2aa..ca0e407 100644 --- a/charts/kamaji/values.yaml +++ b/charts/kamaji/values.yaml @@ -60,7 +60,7 @@ etcd: # -- The custom annotations to add to the PVC customAnnotations: {} # volumeType: local - + # -- (array) Kubernetes affinity rules to apply to Kamaji etcd pods tolerations: [] @@ -162,7 +162,7 @@ loggingDevel: datastore: # -- (bool) Enable the Kamaji Datastore creation (default=true) enabled: true - # -- (string) The Datastore name override, if empty and enabled=true defaults to `default`, if enabled=false, this is the name of the Datastore to connect to. + # -- (string) The Datastore name override, if empty and enabled=true defaults to `default`, if enabled=false, this is the name of the Datastore to connect to. nameOverride: # -- (string) The Kamaji Datastore driver, supported: etcd, MySQL, PostgreSQL (defaults=etcd). driver: etcd @@ -184,6 +184,7 @@ datastore: # -- The Secret key where the data is stored. keyPath: tlsConfig: + enabled: true certificateAuthority: certificate: # -- Name of the Secret containing the CA required to establish the mandatory SSL/TLS connection to the datastore. diff --git a/config/crd/bases/kamaji.clastix.io_datastores.yaml b/config/crd/bases/kamaji.clastix.io_datastores.yaml index 6e7522b..e5f8c5d 100644 --- a/config/crd/bases/kamaji.clastix.io_datastores.yaml +++ b/config/crd/bases/kamaji.clastix.io_datastores.yaml @@ -132,8 +132,9 @@ spec: minItems: 1 type: array tlsConfig: - description: Defines the TLS/SSL configuration required to connect - to the data store in a secure way. + description: |- + Defines the TLS/SSL configuration required to connect to the data store in a secure way. + This value is optional. properties: certificateAuthority: description: |- @@ -269,12 +270,10 @@ spec: type: object required: - certificateAuthority - - clientCertificate type: object required: - driver - endpoints - - tlsConfig type: object status: description: DataStoreStatus defines the observed state of DataStore. diff --git a/config/install.yaml b/config/install.yaml index 8664af4..2ee6166 100644 --- a/config/install.yaml +++ b/config/install.yaml @@ -138,7 +138,9 @@ spec: minItems: 1 type: array tlsConfig: - description: Defines the TLS/SSL configuration required to connect to the data store in a secure way. + description: |- + Defines the TLS/SSL configuration required to connect to the data store in a secure way. + This value is optional. properties: certificateAuthority: description: |- @@ -265,12 +267,10 @@ spec: type: object required: - certificateAuthority - - clientCertificate type: object required: - driver - endpoints - - tlsConfig type: object status: description: DataStoreStatus defines the observed state of DataStore. diff --git a/config/samples/kamaji_v1alpha1_datastore_nats_notls.yaml b/config/samples/kamaji_v1alpha1_datastore_nats_notls.yaml new file mode 100644 index 0000000..ebb0731 --- /dev/null +++ b/config/samples/kamaji_v1alpha1_datastore_nats_notls.yaml @@ -0,0 +1,17 @@ +apiVersion: kamaji.clastix.io/v1alpha1 +kind: DataStore +metadata: + name: nats-notls +spec: + driver: NATS + endpoints: + - notls-nats.nats-system.svc:4222 + basicAuth: + username: + content: YWRtaW4= + password: + secretReference: + name: nats-notls-config + namespace: nats-system + keyPath: password + diff --git a/deploy/kine/nats/Makefile b/deploy/kine/nats/Makefile index 7eed195..673ad26 100644 --- a/deploy/kine/nats/Makefile +++ b/deploy/kine/nats/Makefile @@ -31,7 +31,9 @@ nats-secret: --dry-run=client -o yaml | kubectl apply -f - nats-deployment: - @NAME=$(NAME) envsubst < $(ROOT_DIR)/values.yaml | $(HELM) upgrade --install $(NAME) nats/nats --create-namespace -n nats-system -f - + @VALUES_FILE=$(if $(findstring notls,$(NAME)),values-notls.yaml,values.yaml); \ + NAME=$(NAME) envsubst < $(ROOT_DIR)/$$VALUES_FILE | $(HELM) upgrade --install $(NAME) nats/nats --create-namespace -n nats-system -f - + nats-destroy: @NAME=$(NAME) envsubst < $(ROOT_DIR)/nats.yaml | kubectl -n $(NAMESPACE) delete --ignore-not-found -f - diff --git a/deploy/kine/nats/values-notls.yaml b/deploy/kine/nats/values-notls.yaml new file mode 100644 index 0000000..1a4591f --- /dev/null +++ b/deploy/kine/nats/values-notls.yaml @@ -0,0 +1,14 @@ +config: + merge: + accounts: + private: + jetstream: enabled + users: + - {user: admin, password: "password", permissions: {subscribe: [">"], publish: [">"]}} + cluster: + enabled: no + jetstream: + enabled: true + fileStore: + pvc: + size: 32Mi diff --git a/docs/content/reference/api.md b/docs/content/reference/api.md index 66cb775..ad838e6 100644 --- a/docs/content/reference/api.md +++ b/docs/content/reference/api.md @@ -100,13 +100,6 @@ DataStoreSpec defines the desired state of DataStore. No need for protocol, just bare IP/FQDN and port.
true - - tlsConfig - object - - Defines the TLS/SSL configuration required to connect to the data store in a secure way.
- - true basicAuth object @@ -115,6 +108,202 @@ No need for protocol, just bare IP/FQDN and port.
This value is optional.
false + + tlsConfig + object + + Defines the TLS/SSL configuration required to connect to the data store in a secure way. +This value is optional.
+ + false + + + + +### DataStore.spec.basicAuth + + + +In case of authentication enabled for the given data store, specifies the username and password pair. +This value is optional. + + + + + + + + + + + + + + + + + + + + + +
NameTypeDescriptionRequired
passwordobject +
+ 		true
usernameobject +
+ 		true
+ + +### DataStore.spec.basicAuth.password + + + + + + + + + + + + + + + + + + + + + + + + + +
NameTypeDescriptionRequired
contentstring + Bare content of the file, base64 encoded. +It has precedence over the SecretReference value.
+
+ Format: byte
+ 		false
secretReferenceobject +
+ 		false
+ + +### DataStore.spec.basicAuth.password.secretReference + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
NameTypeDescriptionRequired
keyPathstring + Name of the key for the given Secret reference where the content is stored. +This value is mandatory.
+ 		true
namestring + name is unique within a namespace to reference a secret resource.
+ 		false
namespacestring + namespace defines the space within which the secret name must be unique.
+ 		false
+ + +### DataStore.spec.basicAuth.username + + + + + + + + + + + + + + + + + + + + + + + + + +
NameTypeDescriptionRequired
contentstring + Bare content of the file, base64 encoded. +It has precedence over the SecretReference value.
+
+ Format: byte
+ 		false
secretReferenceobject +
+ 		false
+ + +### DataStore.spec.basicAuth.username.secretReference + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
NameTypeDescriptionRequired
keyPathstring + Name of the key for the given Secret reference where the content is stored. +This value is mandatory.
+ 		true
namestring + name is unique within a namespace to reference a secret resource.
+ 		false
namespacestring + namespace defines the space within which the secret name must be unique.
+ 		false
@@ -124,6 +313,7 @@ This value is optional.
Defines the TLS/SSL configuration required to connect to the data store in a secure way. +This value is optional. @@ -148,7 +338,7 @@ The key reference is required since etcd authentication is based on certificates - +
Specifies the SSL/TLS key and private key pair used to connect to the data store.
truefalse
@@ -493,194 +683,6 @@ It has precedence over the SecretReference value.
- - - - - - - - - - - - - - - - - - - - - - - - - -
NameTypeDescriptionRequired
keyPathstring - Name of the key for the given Secret reference where the content is stored. -This value is mandatory.
- 		true
namestring - name is unique within a namespace to reference a secret resource.
- 		false
namespacestring - namespace defines the space within which the secret name must be unique.
- 		false
- - -### DataStore.spec.basicAuth - - - -In case of authentication enabled for the given data store, specifies the username and password pair. -This value is optional. - - - - - - - - - - - - - - - - - - - - - -
NameTypeDescriptionRequired
passwordobject -
- 		true
usernameobject -
- 		true
- - -### DataStore.spec.basicAuth.password - - - - - - - - - - - - - - - - - - - - - - - - - -
NameTypeDescriptionRequired
contentstring - Bare content of the file, base64 encoded. -It has precedence over the SecretReference value.
-
- Format: byte
- 		false
secretReferenceobject -
- 		false
- - -### DataStore.spec.basicAuth.password.secretReference - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
NameTypeDescriptionRequired
keyPathstring - Name of the key for the given Secret reference where the content is stored. -This value is mandatory.
- 		true
namestring - name is unique within a namespace to reference a secret resource.
- 		false
namespacestring - namespace defines the space within which the secret name must be unique.
- 		false
- - -### DataStore.spec.basicAuth.username - - - - - - - - - - - - - - - - - - - - - - - - - -
NameTypeDescriptionRequired
contentstring - Bare content of the file, base64 encoded. -It has precedence over the SecretReference value.
-
- Format: byte
- 		false
secretReferenceobject -
- 		false
- - -### DataStore.spec.basicAuth.username.secretReference - - - - - diff --git a/e2e/tcp_nats_ready_no_tls_test.go b/e2e/tcp_nats_ready_no_tls_test.go new file mode 100644 index 0000000..f739170 --- /dev/null +++ b/e2e/tcp_nats_ready_no_tls_test.go @@ -0,0 +1,54 @@ +// Copyright 2022 Clastix Labs +// SPDX-License-Identifier: Apache-2.0 + +package e2e + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + pointer "k8s.io/utils/ptr" + + kamajiv1alpha1 "github.com/clastix/kamaji/api/v1alpha1" +) + +var _ = Describe("Deploy a TenantControlPlane resource with the NATS driver without TLS", func() { + // Fill TenantControlPlane object + tcp := &kamajiv1alpha1.TenantControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nats-notls", + Namespace: "default", + }, + Spec: kamajiv1alpha1.TenantControlPlaneSpec{ + DataStore: "nats-notls", + ControlPlane: kamajiv1alpha1.ControlPlane{ + Deployment: kamajiv1alpha1.DeploymentSpec{ + Replicas: pointer.To(int32(1)), + }, + Service: kamajiv1alpha1.ServiceSpec{ + ServiceType: "ClusterIP", + }, + }, + Kubernetes: kamajiv1alpha1.KubernetesSpec{ + Version: "v1.23.6", + Kubelet: kamajiv1alpha1.KubeletSpec{ + CGroupFS: "cgroupfs", + }, + }, + }, + } + // Create a TenantControlPlane resource into the cluster + JustBeforeEach(func() { + Expect(k8sClient.Create(context.Background(), tcp)).NotTo(HaveOccurred()) + }) + // Delete the TenantControlPlane resource after test is finished + JustAfterEach(func() { + Expect(k8sClient.Delete(context.Background(), tcp)).Should(Succeed()) + }) + // Check if TenantControlPlane resource has been created + It("Should be Ready", func() { + StatusMustEqualTo(tcp, kamajiv1alpha1.VersionReady) + }) +}) diff --git a/internal/builders/controlplane/deployment.go b/internal/builders/controlplane/deployment.go index 0af00e0..63dbe4e 100644 --- a/internal/builders/controlplane/deployment.go +++ b/internal/builders/controlplane/deployment.go @@ -805,7 +805,10 @@ func (d Deployment) removeKineContainers(podSpec *corev1.PodSpec) { podSpec.Containers = containers } - // Removing the kine init-container, if present + d.removeKineInitContainers(podSpec) +} + +func (d Deployment) removeKineInitContainers(podSpec *corev1.PodSpec) { if found, index := utilities.HasNamedContainer(podSpec.InitContainers, kineInitContainerName); found { var initContainers []corev1.Container @@ -823,45 +826,67 @@ func (d Deployment) buildKine(podSpec *corev1.PodSpec, tcp kamajiv1alpha1.Tenant return } - // Ensuring the init container required for kine is present: - // a chmod is required for kine in order to read the certificates to connect to the secured datastore. - found, index := utilities.HasNamedContainer(podSpec.InitContainers, kineInitContainerName) - if !found { - index = len(podSpec.InitContainers) - podSpec.InitContainers = append(podSpec.InitContainers, corev1.Container{}) + + // Building kine arguments, taking in consideration the user-space ones if provided. + args := map[string]string{} + + if d.DataStore.Spec.TLSConfig != nil { + // Ensuring the init container required for kine is present: + // a chmod is required for kine in order to read the certificates to connect to the secured datastore. + found, index := utilities.HasNamedContainer(podSpec.InitContainers, kineInitContainerName) + if !found { + index = len(podSpec.InitContainers) + podSpec.InitContainers = append(podSpec.InitContainers, corev1.Container{}) + } + + podSpec.InitContainers[index].Name = kineInitContainerName + podSpec.InitContainers[index].Image = d.KineContainerImage + podSpec.InitContainers[index].Command = []string{"sh"} + + podSpec.InitContainers[index].Args = []string{ + "-c", + "cp /kine/*.* /certs && chmod -R 600 /certs/*.*", + } + + podSpec.InitContainers[index].VolumeMounts = []corev1.VolumeMount{ + { + Name: dataStoreCertsVolumeName, + ReadOnly: true, + MountPath: "/kine", + }, + { + Name: kineVolumeCertName, + MountPath: "/certs", + ReadOnly: false, + }, + } + + args["--ca-file"] = "/certs/ca.crt" + + if d.DataStore.Spec.TLSConfig.ClientCertificate != nil { + args["--cert-file"] = "/certs/server.crt" + args["--key-file"] = "/certs/server.key" + } + } else { + // if no TLS configuration is provided, the kine initContainer must be removed. + d.removeKineInitContainers(podSpec) } - podSpec.InitContainers[index].Name = kineInitContainerName - podSpec.InitContainers[index].Image = d.KineContainerImage - podSpec.InitContainers[index].Command = []string{"sh"} - podSpec.InitContainers[index].Args = []string{ - "-c", - "cp /kine/*.* /certs && chmod -R 600 /certs/*.*", - } - podSpec.InitContainers[index].VolumeMounts = []corev1.VolumeMount{ - { - Name: dataStoreCertsVolumeName, - ReadOnly: true, - MountPath: "/kine", - }, - { - Name: kineVolumeCertName, - MountPath: "/certs", - ReadOnly: false, - }, - } // Kine is expecting an additional container, and it must be removed before proceeding with the additional one // in order to make this function idempotent. - found, index = utilities.HasNamedContainer(podSpec.Containers, kineContainerName) + found, index := utilities.HasNamedContainer(podSpec.Containers, kineContainerName) if !found { index = len(podSpec.Containers) podSpec.Containers = append(podSpec.Containers, corev1.Container{}) } - // Building kine arguments, taking in consideration the user-space ones if provided. - args := map[string]string{} if tcp.Spec.ControlPlane.Deployment.ExtraArgs != nil { - args = utilities.ArgsFromSliceToMap(tcp.Spec.ControlPlane.Deployment.ExtraArgs.Kine) + utilArgs := utilities.ArgsFromSliceToMap(tcp.Spec.ControlPlane.Deployment.ExtraArgs.Kine) + + // Merging the user-space arguments with the Kamaji ones. + for k, v := range utilArgs { + args[k] = v + } } switch d.DataStore.Spec.Driver { @@ -873,10 +898,6 @@ func (d Deployment) buildKine(podSpec *corev1.PodSpec, tcp kamajiv1alpha1.Tenant args["--endpoint"] = "nats://$(DB_USER):$(DB_PASSWORD)@$(DB_CONNECTION_STRING)?bucket=$(DB_SCHEMA)&noEmbed" } - args["--ca-file"] = "/certs/ca.crt" - args["--cert-file"] = "/certs/server.crt" - args["--key-file"] = "/certs/server.key" - podSpec.Containers[index].Name = kineContainerName podSpec.Containers[index].Image = d.KineContainerImage podSpec.Containers[index].Command = []string{"/bin/kine"} diff --git a/internal/datastore/connection.go b/internal/datastore/connection.go index 997ef30..8f354bd 100644 --- a/internal/datastore/connection.go +++ b/internal/datastore/connection.go @@ -21,14 +21,20 @@ func NewStorageConnection(ctx context.Context, client client.Client, ds kamajiv1 switch ds.Spec.Driver { case kamajiv1alpha1.KineMySQLDriver: - cc.TLSConfig.ServerName = cc.Endpoints[0].Host + + if ds.Spec.TLSConfig != nil { + cc.TLSConfig.ServerName = cc.Endpoints[0].Host + } + cc.Parameters = map[string][]string{ "multiStatements": {"true"}, } return NewMySQLConnection(*cc) case kamajiv1alpha1.KinePostgreSQLDriver: - cc.TLSConfig.ServerName = cc.Endpoints[0].Host + if ds.Spec.TLSConfig != nil { + cc.TLSConfig.ServerName = cc.Endpoints[0].Host + } return NewPostgreSQLConnection(*cc) case kamajiv1alpha1.EtcdDriver: diff --git a/internal/datastore/datastore.go b/internal/datastore/datastore.go index 6ed5d7e..8be1099 100644 --- a/internal/datastore/datastore.go +++ b/internal/datastore/datastore.go @@ -36,29 +36,41 @@ type ConnectionConfig struct { } func NewConnectionConfig(ctx context.Context, client client.Client, ds kamajiv1alpha1.DataStore) (*ConnectionConfig, error) { - ca, err := ds.Spec.TLSConfig.CertificateAuthority.Certificate.GetContent(ctx, client) - if err != nil { - return nil, err + var tlsConfig *tls.Config + + if ds.Spec.TLSConfig != nil { + ca, err := ds.Spec.TLSConfig.CertificateAuthority.Certificate.GetContent(ctx, client) + if err != nil { + return nil, err + } + + rootCAs := x509.NewCertPool() + if ok := rootCAs.AppendCertsFromPEM(ca); !ok { + return nil, fmt.Errorf("error create root CA for the DB connector") + } + + tlsConfig = &tls.Config{ + RootCAs: rootCAs, + } } - crt, err := ds.Spec.TLSConfig.ClientCertificate.Certificate.GetContent(ctx, client) - if err != nil { - return nil, err - } + if ds.Spec.TLSConfig != nil && ds.Spec.TLSConfig.ClientCertificate != nil { + crt, err := ds.Spec.TLSConfig.ClientCertificate.Certificate.GetContent(ctx, client) + if err != nil { + return nil, err + } - key, err := ds.Spec.TLSConfig.ClientCertificate.PrivateKey.GetContent(ctx, client) - if err != nil { - return nil, err - } + key, err := ds.Spec.TLSConfig.ClientCertificate.PrivateKey.GetContent(ctx, client) + if err != nil { + return nil, err + } - rootCAs := x509.NewCertPool() - if ok := rootCAs.AppendCertsFromPEM(ca); !ok { - return nil, fmt.Errorf("error create root CA for the DB connector") - } + certificate, err := tls.X509KeyPair(crt, key) + if err != nil { + return nil, errors.Wrap(err, "cannot retrieve x.509 key pair from the Kine Secret") + } - certificate, err := tls.X509KeyPair(crt, key) - if err != nil { - return nil, errors.Wrap(err, "cannot retrieve x.509 key pair from the Kine Secret") + tlsConfig.Certificates = []tls.Certificate{certificate} } var user, password string @@ -99,10 +111,7 @@ func NewConnectionConfig(ctx context.Context, client client.Client, ds kamajiv1a User: user, Password: password, Endpoints: eps, - TLSConfig: &tls.Config{ - RootCAs: rootCAs, - Certificates: []tls.Certificate{certificate}, - }, + TLSConfig: tlsConfig, }, nil } diff --git a/internal/datastore/mysql.go b/internal/datastore/mysql.go index ad003de..61d4978 100644 --- a/internal/datastore/mysql.go +++ b/internal/datastore/mysql.go @@ -112,12 +112,14 @@ func NewMySQLConnection(config ConnectionConfig) (Connection, error) { tlsKey := "mysql" - if err = mysql.RegisterTLSConfig(tlsKey, config.TLSConfig); err != nil { - return nil, err + if config.TLSConfig != nil { + if err = mysql.RegisterTLSConfig(tlsKey, config.TLSConfig); err != nil { + return nil, err + } + mysqlConfig.TLSConfig = tlsKey } mysqlConfig.DBName = config.DBName - mysqlConfig.TLSConfig = tlsKey parsedDSN := mysqlConfig.FormatDSN() db, err := sql.Open("mysql", parsedDSN) diff --git a/internal/resources/datastore/datastore_certificate.go b/internal/resources/datastore/datastore_certificate.go index 1d5604f..02cbdb1 100644 --- a/internal/resources/datastore/datastore_certificate.go +++ b/internal/resources/datastore/datastore_certificate.go @@ -80,79 +80,89 @@ func (r *Certificate) mutate(ctx context.Context, tenantControlPlane *kamajiv1al return func() error { logger := log.FromContext(ctx, "resource", r.GetName()) - ca, err := r.DataStore.Spec.TLSConfig.CertificateAuthority.Certificate.GetContent(ctx, r.Client) - if err != nil { - logger.Error(err, "cannot retrieve CA certificate content") + if r.DataStore.Spec.TLSConfig != nil { + ca, err := r.DataStore.Spec.TLSConfig.CertificateAuthority.Certificate.GetContent(ctx, r.Client) + if err != nil { + logger.Error(err, "cannot retrieve CA certificate content") - return err - } + return err + } - r.resource.Data["ca.crt"] = ca + r.resource.Data["ca.crt"] = ca - r.resource.SetLabels(utilities.MergeMaps( - utilities.KamajiLabels(tenantControlPlane.GetName(), r.GetName()), - map[string]string{ - constants.ControllerLabelResource: "x509", - }, - )) + r.resource.SetLabels(utilities.MergeMaps( + utilities.KamajiLabels(tenantControlPlane.GetName(), r.GetName()), + map[string]string{ + constants.ControllerLabelResource: "x509", + }, + )) - if err = ctrl.SetControllerReference(tenantControlPlane, r.resource, r.Client.Scheme()); err != nil { - logger.Error(err, "cannot set controller reference", "resource", r.GetName()) + if err = ctrl.SetControllerReference(tenantControlPlane, r.resource, r.Client.Scheme()); err != nil { + logger.Error(err, "cannot set controller reference", "resource", r.GetName()) - return err - } + return err + } - if utilities.GetObjectChecksum(r.resource) == utilities.CalculateMapChecksum(r.resource.Data) { - if r.DataStore.Spec.Driver == kamajiv1alpha1.EtcdDriver { - if isValid, _ := crypto.IsValidCertificateKeyPairBytes(r.resource.Data["server.crt"], r.resource.Data["server.key"]); isValid { - return nil + if utilities.GetObjectChecksum(r.resource) == utilities.CalculateMapChecksum(r.resource.Data) { + if r.DataStore.Spec.Driver == kamajiv1alpha1.EtcdDriver { + if isValid, _ := crypto.IsValidCertificateKeyPairBytes(r.resource.Data["server.crt"], r.resource.Data["server.key"]); isValid { + return nil + } } } + + var crt, key *bytes.Buffer + + switch r.DataStore.Spec.Driver { + case kamajiv1alpha1.EtcdDriver: + var privateKey []byte + // When dealing with the etcd storage we cannot use the basic authentication, thus the generation of a + // certificate used for authentication is mandatory, along with the CA private key. + if privateKey, err = r.DataStore.Spec.TLSConfig.CertificateAuthority.PrivateKey.GetContent(ctx, r.Client); err != nil { + logger.Error(err, "unable to retrieve CA private key content") + + return err + } + + if crt, key, err = crypto.GenerateCertificatePrivateKeyPair(crypto.NewCertificateTemplate(tenantControlPlane.Status.Storage.Setup.User), ca, privateKey); err != nil { + logger.Error(err, "unable to generate certificate and private key") + + return err + } + case kamajiv1alpha1.KineMySQLDriver, kamajiv1alpha1.KinePostgreSQLDriver, kamajiv1alpha1.KineNatsDriver: + var crtBytes, keyBytes []byte + // For the SQL drivers we just need to copy the certificate, since the basic authentication is used + // to connect to the desired schema and database. + + if r.DataStore.Spec.TLSConfig.ClientCertificate != nil { + if crtBytes, err = r.DataStore.Spec.TLSConfig.ClientCertificate.Certificate.GetContent(ctx, r.Client); err != nil { + logger.Error(err, "unable to retrieve certificate content") + + return err + } + + crt = bytes.NewBuffer(crtBytes) + + if keyBytes, err = r.DataStore.Spec.TLSConfig.ClientCertificate.PrivateKey.GetContent(ctx, r.Client); err != nil { + logger.Error(err, "unable to retrieve private key content") + + return err + } + key = bytes.NewBuffer(keyBytes) + } + default: + return fmt.Errorf("unrecognized driver for Certificate generation") + } + + if r.DataStore.Spec.TLSConfig.ClientCertificate != nil { + r.resource.Data["server.crt"] = crt.Bytes() + r.resource.Data["server.key"] = key.Bytes() + } + } else { + // set r.resource.Data to empty to allow switching from TLS to non-tls + r.resource.Data = map[string][]byte{} } - var crt, key *bytes.Buffer - - switch r.DataStore.Spec.Driver { - case kamajiv1alpha1.EtcdDriver: - var privateKey []byte - // When dealing with the etcd storage we cannot use the basic authentication, thus the generation of a - // certificate used for authentication is mandatory, along with the CA private key. - if privateKey, err = r.DataStore.Spec.TLSConfig.CertificateAuthority.PrivateKey.GetContent(ctx, r.Client); err != nil { - logger.Error(err, "unable to retrieve CA private key content") - - return err - } - - if crt, key, err = crypto.GenerateCertificatePrivateKeyPair(crypto.NewCertificateTemplate(tenantControlPlane.Status.Storage.Setup.User), ca, privateKey); err != nil { - logger.Error(err, "unable to generate certificate and private key") - - return err - } - case kamajiv1alpha1.KineMySQLDriver, kamajiv1alpha1.KinePostgreSQLDriver, kamajiv1alpha1.KineNatsDriver: - var crtBytes, keyBytes []byte - // For the SQL drivers we just need to copy the certificate, since the basic authentication is used - // to connect to the desired schema and database. - if crtBytes, err = r.DataStore.Spec.TLSConfig.ClientCertificate.Certificate.GetContent(ctx, r.Client); err != nil { - logger.Error(err, "unable to retrieve certificate content") - - return err - } - - crt = bytes.NewBuffer(crtBytes) - - if keyBytes, err = r.DataStore.Spec.TLSConfig.ClientCertificate.PrivateKey.GetContent(ctx, r.Client); err != nil { - logger.Error(err, "unable to retrieve private key content") - - return err - } - key = bytes.NewBuffer(keyBytes) - default: - return fmt.Errorf("unrecognized driver for Certificate generation") - } - - r.resource.Data["server.crt"] = crt.Bytes() - r.resource.Data["server.key"] = key.Bytes() - utilities.SetObjectChecksum(r.resource, r.resource.Data) return nil diff --git a/internal/webhook/handlers/ds_validate.go b/internal/webhook/handlers/ds_validate.go index 9f6e7e5..6bd98aa 100644 --- a/internal/webhook/handlers/ds_validate.go +++ b/internal/webhook/handlers/ds_validate.go @@ -84,6 +84,10 @@ func (d DataStoreValidation) validateBasicAuth(ctx context.Context, ds kamajiv1a } func (d DataStoreValidation) validateTLSConfig(ctx context.Context, ds kamajiv1alpha1.DataStore) error { + if ds.Spec.TLSConfig == nil && ds.Spec.Driver != kamajiv1alpha1.EtcdDriver { + return nil + } + if err := d.validateContentReference(ctx, ds.Spec.TLSConfig.CertificateAuthority.Certificate); err != nil { return fmt.Errorf("CA certificate is not valid, %w", err) }