Add TLS support #23

olliewalsh · 2022-07-01T11:57:35Z

Add a TLS property to the CR to specify the the cert and CA secrets.
TLS/secretName and TLS/caSecretName seems to be the convention in other
operators.

Add secrets to rbac and fix missing patch support for pods. Fix Pod
CreateOrUpdate logic.

Trigger reconcile using a label watch as we do not own the TLS secrets.

Add volume mounts for the TLS secrets and kolla config.
Fix mariadb conf ownership, it was being silently ignored.

stuggi · 2022-07-04T07:04:50Z

templates/mariadb/bin/galera.cnf

+ssl-cert = /etc/pki/tls/certs/mariadb.crt
+ssl-cipher = !SSLv2:kEECDH:kRSA:kEDH:kPSK:+3DES:!aNULL:!eNULL:!MD5:!EXP:!RC4:!SEED:!IDEA:!DES:!SSLv3:!TLSv1
+ssl-key = /etc/pki/tls/private/mariadb.key
+ssl-ca = /etc/pki/tls/certs/mariadbca.crt


does it still start without TLS? because we do not force TLS if I read the above correct.

Yes. The client decides to initiate the SSL handshake. If the SSL files
exist on the server then it will work. There is a conf option to force ssl
by default but it does not exist in this version of mariadb
https://mariadb.com/kb/en/server-system-variables/#require_secure_transport.

I believe we can require SSL for specific accounts though.

That was when using centos8/train images. If the target is centos9/wallaby then we should have require_secure_transport as it's mariadb 10.5.16.

dprince · 2022-07-06T01:10:16Z

templates/mariadb/bin/galera.cnf

@@ -7,7 +7,7 @@ key_buffer_size = 16M

 [mysqld]
 basedir = /usr
-bind-address = 127.0.0.1
+bind-address = *


Do we need to listen on *?

Yes, loopback would only be accessible within the pod. It's the default that we have been using all along since mysql user could not read this file.

we could set the actual ip via an init container to the status.PodIP, but not sure if that is better than * as we only have a single pod interface.

keystone-operator apache uses Listen 0.0.0.0:5000, which is basically the same but IPv4 only. In fact we should change keystone to Listen 5000.

openstack-k8s-operators/keystone-operator#86

stuggi · 2022-07-07T09:58:06Z

api/v1beta1/mariadb_types.go

@@ -20,6 +20,15 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )

+
+// TLSSpec defines the TLS options
+type TLSSpec struct {


not sure how static this is. is this a generic type we want to add to lib-common as its used by all operators?

stuggi · 2022-07-07T10:10:21Z

controllers/mariadb_controller.go

+		tlsSecretHasLabel := tlsSecret.Labels[mariaDBReconcileLabel] == instance.Name
+		if !tlsSecretHasLabel {
+			if tlsSecret.Labels == nil {
+				tlsSecret.Labels = make(map[string]string)
+			}
+			tlsSecret.Labels[mariaDBReconcileLabel] = instance.Name


could also use labels.Merge

if value, ok := tlsSecret.Labels[mariaDBReconcileLabel]; !ok || value != instance.Name { tlsSecret.GetObjectMeta().SetLabels(labels.Merge(tlsSecret.GetObjectMeta().GetLabels(), tlsSecret.Labels[mariaDBReconcileLabel] = instance.Name) ... update secret }

stuggi · 2022-07-07T10:11:32Z

controllers/mariadb_controller.go

+				if k8s_errors.IsConflict(err) {
+					return ctrl.Result{Requeue: true}, err
+				}
+				if k8s_errors.IsNotFound(err) {
+					return ctrl.Result{Requeue: true}, err
+				}


could do this in one if as the return ctrl is the same?

stuggi · 2022-07-07T10:12:19Z

controllers/mariadb_controller.go

+		if !caSecretHasLabel {
+			if caSecret.Labels == nil {
+				caSecret.Labels = make(map[string]string)
+			}
+			caSecret.Labels[mariaDBReconcileLabel] = instance.Name


same as above, could use labels.Merge

openshift-ci · 2023-04-24T14:51:02Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: olliewalsh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [olliewalsh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Add a TLS property to the CR to specify the the cert and CA secrets. TLS/secretName and TLS/caSecretName seems to be the convention in other operators. Add secrets to rbac and fix missing patch support for pods. Fix Pod CreateOrUpdate logic. Trigger reconcile using a label watch as we do not own the TLS secrets. Add volume mounts for the TLS secrets and kolla config. Fix mariadb conf ownership, it was being silently ignored.

openshift-merge-robot · 2023-05-11T14:45:40Z

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci · 2023-08-08T05:58:46Z

@olliewalsh: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/images	`a1c76ba`	link	true	`/test images`
ci/prow/unit	`a1c76ba`	link	true	`/test unit`
ci/prow/precommit-check	`a1c76ba`	link	true	`/test precommit-check`
ci/prow/mariadb-operator-build-deploy-kuttl	`a1c76ba`	link	true	`/test mariadb-operator-build-deploy-kuttl`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

olliewalsh · 2024-01-12T10:11:10Z

Closing this initial POC PR. See #119 instead

olliewalsh requested review from dprince, abays and stuggi July 1, 2022 11:57

olliewalsh force-pushed the tls branch 2 times, most recently from b47d2b4 to f73c3ea Compare July 1, 2022 20:45

stuggi reviewed Jul 4, 2022

View reviewed changes

dprince reviewed Jul 6, 2022

View reviewed changes

stuggi reviewed Jul 7, 2022

View reviewed changes

openshift-merge-robot added the needs-rebase label Aug 23, 2022

olliewalsh marked this pull request as draft April 24, 2023 14:50

openshift-ci bot added the do-not-merge/work-in-progress label Apr 24, 2023

olliewalsh force-pushed the tls branch from f73c3ea to 3d2e73e Compare April 24, 2023 14:50

openshift-ci bot added the approved label Apr 24, 2023

openshift-merge-robot removed the needs-rebase label Apr 24, 2023

olliewalsh force-pushed the tls branch 5 times, most recently from e6bf838 to 2adb965 Compare May 11, 2023 14:45

openshift-merge-robot added the needs-rebase label May 11, 2023

olliewalsh force-pushed the tls branch 2 times, most recently from db27379 to 63c7b03 Compare May 11, 2023 15:05

Enable mTLS

a1c76ba

olliewalsh force-pushed the tls branch from 63c7b03 to a1c76ba Compare May 11, 2023 15:09

dciabrin mentioned this pull request Sep 7, 2023

WIP direct TLS connection to database service openstack-k8s-operators/keystone-operator#306

Closed

olliewalsh closed this Jan 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add TLS support #23

Add TLS support #23

olliewalsh commented Jul 1, 2022

stuggi Jul 4, 2022

olliewalsh Jul 4, 2022

olliewalsh Aug 19, 2022

dprince Jul 6, 2022

olliewalsh Jul 6, 2022

stuggi Jul 7, 2022

olliewalsh Aug 19, 2022 •

edited

Loading

stuggi Aug 19, 2022

stuggi Jul 7, 2022

stuggi Jul 7, 2022

stuggi Jul 7, 2022

stuggi Jul 7, 2022

openshift-ci bot commented Apr 24, 2023

openshift-merge-robot commented May 11, 2023

openshift-ci bot commented Aug 8, 2023

olliewalsh commented Jan 12, 2024

Add TLS support #23

Add TLS support #23

Conversation

olliewalsh commented Jul 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olliewalsh Aug 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci bot commented Apr 24, 2023

openshift-merge-robot commented May 11, 2023

openshift-ci bot commented Aug 8, 2023

olliewalsh commented Jan 12, 2024

olliewalsh Aug 19, 2022 •

edited

Loading