Support TLS connections for galera #119

dciabrin · 2023-06-28T15:28:00Z

No description provided.

openshift-ci · 2023-06-28T15:28:05Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

dciabrin · 2023-07-04T12:42:09Z

/retest

dciabrin · 2023-08-23T07:35:33Z

/retest

controllers/galera_controller.go

pkg/mariadb/database.go

dciabrin · 2023-10-16T13:53:44Z

/retest

gibizer

overall looks good to me

gibizer · 2023-10-17T07:46:43Z

controllers/mariadbdatabase_controller.go

+		// useTLS = (dbGalera.Spec.TLS.SecretName != "")
+		useTLS = false


Do we need to switch to the commented out code to be able to test this?

I updated it slightly to enforce TLS for users based on TLS property "TLS.Service.DisableNonTLSListeners". I also added a kuttl test for that feature

stuggi · 2023-10-24T08:38:16Z

controllers/galera_controller.go

+		Watches(&source.Kind{Type: &corev1.Secret{}}, handler.EnqueueRequestsFromMapFunc(
+			func(o client.Object) []reconcile.Request {
+				labels := o.GetLabels()
+
+				reconcileCR, hasLabel := labels[mariaDBReconcileLabel]
+				if !hasLabel {
+					return []reconcile.Request{}
+				}
+
+				return []reconcile.Request{
+					{NamespacedName: types.NamespacedName{
+						Name:      reconcileCR,
+						Namespace: o.GetNamespace(),
+					}},
+				}
+			},
+		)).


is the label being set somewhere ? you consider using it to watch the tls cert secrets? for this case we intend that the service operators add themselves as an additional owner (not controller) of the secret. then is should only be required to add Owns(&corev1.Secret{}).

See my remark below

stuggi · 2023-10-24T08:38:33Z

controllers/galera_controller.go


 	mariadbv1 "github.com/openstack-k8s-operators/mariadb-operator/api/v1beta1"
 	mariadb "github.com/openstack-k8s-operators/mariadb-operator/pkg/mariadb"
 )

+// Label used in a k8s secret to reference its corresponding galera CR
+const mariaDBReconcileLabel = "mariadb-ref"


is it added somewhere to an object?

No the idea was to rely on the templating ability of cert-manager, as e.g. https://github.com/openstack-k8s-operators/mariadb-operator/pull/119/files#diff-c73cb33ebf19bd8e12d56f29679841e5abcd55b1fb58b07ec416749e8ca2d30bR47-R49
The drawback is that any Secret won't work if it's not labeled.
I'm good with adding the label automatically if this is a pattern that we're going to follow everywhere in the project.

lets wait with this. I should have an example hopefully tomorrow or on thu to show what I mean

I just implemented the 2nd pattern in this PR, that uses cache indexing. This approach allows us to reconcile and restart galera when a secret is manually deleted to forcibly regenerate a certificate in cert-monger.

controllers/galera_controller.go

dciabrin · 2023-11-09T14:15:04Z

/retest

dciabrin · 2023-11-17T14:01:59Z

Fixed unit tests in the last push

pkg/mariadb/volumes.go

api/bases/mariadb.openstack.org_galeras.yaml

api/v1beta1/galera_types.go

config/crd/bases/mariadb.openstack.org_galeras.yaml

stuggi · 2024-01-25T15:57:07Z

controllers/galera_controller.go

+			err.Error()))
+		return ctrl.Result{}, fmt.Errorf("error calculating input hash: %w", err)
+	}
+	instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage)


in the other operators we use TLSInputReady condition for TLS cert secrets. maybe want to also have this here to be in sync

Ack I'll update that

pkg/mariadb/volumes.go

olliewalsh · 2024-01-25T16:04:26Z

api/v1beta1/galera_types.go

+	// TLS settings for MySQL service and internal Galera replication
+	TLS tls.SimpleService `json:"tls,omitempty"`
+	// +kubebuilder:validation:Optional
+	// When TLS is used, only allow connections to the DB over TLS


nit: maybe s/used/configured/

olliewalsh · 2024-01-25T16:06:40Z

controllers/galera_controller.go

@@ -97,7 +120,8 @@ func buildGcommURI(instance *mariadbv1.Galera) string {
 	res := []string{}

 	for i := 0; i < replicas; i++ {
-		res = append(res, basename+"-"+strconv.Itoa(i)+"."+basename)
+		// Generate Gcomm with FQDN for TLS validation


nit: it not the FQDN, that would have .cluster.local on the end (logic looks fine, just the comment)

dciabrin · 2024-01-29T14:45:21Z

PR rebased to add TLS support on the new MariaDBAccount CR as well

Ability to specify a certificate and a CA to be used for galera cluster communication (GCOMM, SST). Updates to the certificate used for galera automatically triggers a rolling restart of the galera pods, without service disruption. The original volume mount names is changed to accomodate the TLS subpath mounts. They now follow the naming convention found in other Openstack services. When the Galera CR is configured to use TLS, the mariadbdatabase CR creates DB users that still allow connection to the DB without using TLS. This is because Openstack clients currently cannot be configured to connect via TLS or via plain TCP. This specific part will be addressed in a subsequent commit.

stuggi

/lgtm

openshift-ci · 2024-01-31T15:21:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dciabrin, stuggi

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~~ [dciabrin,stuggi]

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

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

openshift-ci bot added the approved label Jun 28, 2023

dciabrin force-pushed the galera-tls branch 2 times, most recently from 59dacf0 to 68e9f09 Compare July 3, 2023 10:48

dciabrin marked this pull request as ready for review July 3, 2023 12:12

openshift-ci bot removed the do-not-merge/work-in-progress label Jul 3, 2023

openshift-ci bot requested review from abays and olliewalsh July 3, 2023 12:13

openshift-merge-robot added the needs-rebase label Jul 6, 2023

dciabrin force-pushed the galera-tls branch from 68e9f09 to e00f80d Compare August 16, 2023 15:32

openshift-merge-robot removed the needs-rebase label Aug 16, 2023

dciabrin force-pushed the galera-tls branch 2 times, most recently from 8b7c1d4 to e5aad20 Compare August 22, 2023 15:47

dciabrin force-pushed the galera-tls branch from e5aad20 to 529b8b3 Compare September 5, 2023 09:03

dciabrin force-pushed the galera-tls branch 3 times, most recently from 65753bc to fa266f5 Compare October 6, 2023 09:11

dciabrin mentioned this pull request Oct 6, 2023

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

Closed

gibizer reviewed Oct 9, 2023

View reviewed changes

controllers/galera_controller.go Outdated Show resolved Hide resolved

controllers/galera_controller.go Outdated Show resolved Hide resolved

pkg/mariadb/database.go Outdated Show resolved Hide resolved

dciabrin force-pushed the galera-tls branch from fa266f5 to 3a628b3 Compare October 12, 2023 09:10

dciabrin force-pushed the galera-tls branch from 3a628b3 to 7f267e2 Compare October 16, 2023 15:02

gibizer reviewed Oct 17, 2023

View reviewed changes

dciabrin force-pushed the galera-tls branch 4 times, most recently from 09009ce to d2b38a3 Compare October 23, 2023 15:10

stuggi reviewed Oct 24, 2023

View reviewed changes

dciabrin force-pushed the galera-tls branch from d2b38a3 to e4f24a0 Compare November 9, 2023 13:31

dciabrin force-pushed the galera-tls branch from e4f24a0 to b9b67ca Compare November 17, 2023 12:15

dciabrin force-pushed the galera-tls branch from b9b67ca to 62522b2 Compare January 8, 2024 16:20

Deydra71 self-requested a review January 10, 2024 08:04

olliewalsh mentioned this pull request Jan 12, 2024

Add TLS support #23

Closed

olliewalsh reviewed Jan 12, 2024

View reviewed changes

pkg/mariadb/volumes.go Show resolved Hide resolved

dciabrin force-pushed the galera-tls branch 3 times, most recently from 33334b4 to a7c9d70 Compare January 23, 2024 21:39

Deydra71 reviewed Jan 25, 2024

View reviewed changes

api/bases/mariadb.openstack.org_galeras.yaml Show resolved Hide resolved

Deydra71 reviewed Jan 25, 2024

View reviewed changes

api/v1beta1/galera_types.go Show resolved Hide resolved

Deydra71 reviewed Jan 25, 2024

View reviewed changes

config/crd/bases/mariadb.openstack.org_galeras.yaml Show resolved Hide resolved

dciabrin force-pushed the galera-tls branch from a7c9d70 to 16a7312 Compare January 25, 2024 10:52

stuggi reviewed Jan 25, 2024

View reviewed changes

olliewalsh reviewed Jan 25, 2024

View reviewed changes

dciabrin force-pushed the galera-tls branch from 16a7312 to 6ce22eb Compare January 25, 2024 21:29

openshift-merge-robot added the needs-rebase label Jan 25, 2024

dciabrin force-pushed the galera-tls branch from 6ce22eb to 8c341db Compare January 29, 2024 13:58

dciabrin force-pushed the galera-tls branch from 8c341db to dd0b722 Compare January 31, 2024 13:32

openshift-merge-robot removed the needs-rebase label Jan 31, 2024

stuggi approved these changes Jan 31, 2024

View reviewed changes

openshift-ci bot assigned stuggi Jan 31, 2024

openshift-ci bot added the lgtm label Jan 31, 2024

openshift-merge-bot bot merged commit 7c0df7d into openstack-k8s-operators:main Jan 31, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support TLS connections for galera #119

Support TLS connections for galera #119

dciabrin commented Jun 28, 2023

openshift-ci bot commented Jun 28, 2023

dciabrin commented Jul 4, 2023

dciabrin commented Aug 23, 2023

dciabrin commented Oct 16, 2023

gibizer left a comment

gibizer Oct 17, 2023

dciabrin Oct 24, 2023

stuggi Oct 24, 2023

dciabrin Oct 24, 2023

stuggi Oct 24, 2023

dciabrin Oct 24, 2023

stuggi Oct 24, 2023

dciabrin Nov 9, 2023

dciabrin commented Nov 9, 2023

dciabrin commented Nov 17, 2023

stuggi Jan 25, 2024

dciabrin Jan 25, 2024

olliewalsh Jan 25, 2024

dciabrin Jan 25, 2024

olliewalsh Jan 25, 2024

dciabrin Jan 25, 2024

dciabrin commented Jan 29, 2024

stuggi left a comment

openshift-ci bot commented Jan 31, 2024

		// useTLS = (dbGalera.Spec.TLS.SecretName != "")
		useTLS = false

Support TLS connections for galera #119

Support TLS connections for galera #119

Conversation

dciabrin commented Jun 28, 2023

openshift-ci bot commented Jun 28, 2023

dciabrin commented Jul 4, 2023

dciabrin commented Aug 23, 2023

dciabrin commented Oct 16, 2023

gibizer left a 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

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

dciabrin commented Nov 9, 2023

dciabrin commented Nov 17, 2023

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

dciabrin commented Jan 29, 2024

stuggi left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Jan 31, 2024