Merge pull request #275 from vshn/psql_tls

Add possibility to disable TLS in VSHNPostgreSQL
vshn · Nov 29, 2024 · 210c3ba · 210c3ba
2 parents aa73d55 + 3e9b0aa
commit 210c3ba
Show file tree

Hide file tree

Showing 13 changed files with 200 additions and 15 deletions.
diff --git a/apis/vshn/v1/dbaas_vshn_postgresql.go b/apis/vshn/v1/dbaas_vshn_postgresql.go
@@ -160,6 +160,9 @@ type VSHNPostgreSQLServiceSpec struct {
 
 	// Access defines additional users and databases for this instance.
 	Access []VSHNAccess `json:"access,omitempty"`
+
+	// TLS settings for the instance.
+	TLS VSHNPostgreSQLTLS `json:"tls,omitempty"`
 }
 
 // VSHNDBaaSPostgresExtension contains the name of a single extension.
@@ -226,6 +229,14 @@ type VSHNPostgreSQLEncryption struct {
 	Enabled bool `json:"enabled,omitempty"`
 }
 
+// VSHNPostgreSQLTLS contains TLS specific parameters
+type VSHNPostgreSQLTLS struct {
+	// Enabled specifies if the instance should use TLS for the instance.
+	// This change takes effect immediately and does not require a restart of the database.
+	// +kubebuilder:default=true
+	Enabled bool `json:"enabled,omitempty"`
+}
+
 // VSHNPostgreSQLStatus reflects the observed state of a VSHNPostgreSQL.
 type VSHNPostgreSQLStatus struct {
 	// InstanceNamespace contains the name of the namespace where the instance resides

diff --git a/apis/vshn/v1/zz_generated.deepcopy.go b/apis/vshn/v1/zz_generated.deepcopy.go
diff --git a/crds/vshn.appcat.vshn.io_vshnkeycloaks.yaml b/crds/vshn.appcat.vshn.io_vshnkeycloaks.yaml
@@ -9888,6 +9888,16 @@ spec:
                                     - besteffort
                                     - guaranteed
                                   type: string
+                                tls:
+                                  description: TLS settings for the instance.
+                                  properties:
+                                    enabled:
+                                      default: true
+                                      description: |-
+                                        Enabled specifies if the instance should use TLS for the instance.
+                                        This change takes effect immediately and does not require a restart of the database.
+                                      type: boolean
+                                  type: object
                                 vacuumEnabled:
                                   default: false
                                   type: boolean

diff --git a/crds/vshn.appcat.vshn.io_vshnnextclouds.yaml b/crds/vshn.appcat.vshn.io_vshnnextclouds.yaml
@@ -9870,6 +9870,16 @@ spec:
                                     - besteffort
                                     - guaranteed
                                   type: string
+                                tls:
+                                  description: TLS settings for the instance.
+                                  properties:
+                                    enabled:
+                                      default: true
+                                      description: |-
+                                        Enabled specifies if the instance should use TLS for the instance.
+                                        This change takes effect immediately and does not require a restart of the database.
+                                      type: boolean
+                                  type: object
                                 vacuumEnabled:
                                   default: false
                                   type: boolean

diff --git a/crds/vshn.appcat.vshn.io_vshnpostgresqls.yaml b/crds/vshn.appcat.vshn.io_vshnpostgresqls.yaml
@@ -5078,6 +5078,16 @@ spec:
                             - besteffort
                             - guaranteed
                           type: string
+                        tls:
+                          description: TLS settings for the instance.
+                          properties:
+                            enabled:
+                              default: true
+                              description: |-
+                                Enabled specifies if the instance should use TLS for the instance.
+                                This change takes effect immediately and does not require a restart of the database.
+                              type: boolean
+                          type: object
                         vacuumEnabled:
                           default: false
                           type: boolean

diff --git a/crds/vshn.appcat.vshn.io_xvshnkeycloaks.yaml b/crds/vshn.appcat.vshn.io_xvshnkeycloaks.yaml
@@ -11678,6 +11678,16 @@ spec:
                                 - besteffort
                                 - guaranteed
                                 type: string
+                              tls:
+                                description: TLS settings for the instance.
+                                properties:
+                                  enabled:
+                                    default: true
+                                    description: |-
+                                      Enabled specifies if the instance should use TLS for the instance.
+                                      This change takes effect immediately and does not require a restart of the database.
+                                    type: boolean
+                                type: object
                               vacuumEnabled:
                                 default: false
                                 type: boolean

diff --git a/crds/vshn.appcat.vshn.io_xvshnnextclouds.yaml b/crds/vshn.appcat.vshn.io_xvshnnextclouds.yaml
@@ -11663,6 +11663,16 @@ spec:
                                 - besteffort
                                 - guaranteed
                                 type: string
+                              tls:
+                                description: TLS settings for the instance.
+                                properties:
+                                  enabled:
+                                    default: true
+                                    description: |-
+                                      Enabled specifies if the instance should use TLS for the instance.
+                                      This change takes effect immediately and does not require a restart of the database.
+                                    type: boolean
+                                type: object
                               vacuumEnabled:
                                 default: false
                                 type: boolean

diff --git a/crds/vshn.appcat.vshn.io_xvshnpostgresqls.yaml b/crds/vshn.appcat.vshn.io_xvshnpostgresqls.yaml
@@ -5769,6 +5769,16 @@ spec:
                         - besteffort
                         - guaranteed
                         type: string
+                      tls:
+                        description: TLS settings for the instance.
+                        properties:
+                          enabled:
+                            default: true
+                            description: |-
+                              Enabled specifies if the instance should use TLS for the instance.
+                              This change takes effect immediately and does not require a restart of the database.
+                            type: boolean
+                        type: object
                       vacuumEnabled:
                         default: false
                         type: boolean

diff --git a/pkg/comp-functions/functions/vshnpostgres/postgresql_deploy.go b/pkg/comp-functions/functions/vshnpostgres/postgresql_deploy.go
@@ -406,17 +406,6 @@ func createSgCluster(ctx context.Context, comp *vshnv1.VSHNPostgreSQL, svc *runt
 			InitialData: initialData,
 			Postgres: sgv1.SGClusterSpecPostgres{
 				Version: comp.Spec.Parameters.Service.MajorVersion,
-				Ssl: &sgv1.SGClusterSpecPostgresSsl{
-					Enabled: ptr.To(true),
-					CertificateSecretKeySelector: &sgv1.SGClusterSpecPostgresSslCertificateSecretKeySelector{
-						Name: certificateSecretName,
-						Key:  "tls.crt",
-					},
-					PrivateKeySecretKeySelector: &sgv1.SGClusterSpecPostgresSslPrivateKeySecretKeySelector{
-						Name: certificateSecretName,
-						Key:  "tls.key",
-					},
-				},
 			},
 			Pods: sgv1.SGClusterSpecPods{
 				PersistentVolume: sgv1.SGClusterSpecPodsPersistentVolume{
@@ -443,6 +432,23 @@ func createSgCluster(ctx context.Context, comp *vshnv1.VSHNPostgreSQL, svc *runt
 		sgCluster.Spec.Pods.DisableConnectionPooling = ptr.To(true)
 	}
 
+	TLSSettings := &sgv1.SGClusterSpecPostgresSsl{
+		Enabled: &comp.Spec.Parameters.Service.TLS.Enabled,
+	}
+
+	if comp.Spec.Parameters.Service.TLS.Enabled {
+		TLSSettings.CertificateSecretKeySelector = &sgv1.SGClusterSpecPostgresSslCertificateSecretKeySelector{
+			Name: certificateSecretName,
+			Key:  "tls.crt",
+		}
+		TLSSettings.PrivateKeySecretKeySelector = &sgv1.SGClusterSpecPostgresSslPrivateKeySecretKeySelector{
+			Name: certificateSecretName,
+			Key:  "tls.key",
+		}
+	}
+
+	sgCluster.Spec.Postgres.Ssl = TLSSettings
+
 	configureReplication(comp, sgCluster)
 
 	err = svc.SetDesiredKubeObjectWithName(sgCluster, comp.GetName()+"-cluster", "cluster", runtime.KubeOptionAddRefs(backupRef))

diff --git a/pkg/sliexporter/probes/manager.go b/pkg/sliexporter/probes/manager.go
@@ -158,7 +158,7 @@ func (m Manager) sendProbe(ctx context.Context, p Prober) {
 		"maintenance":  strconv.FormatBool(m.maintenanceStatus.IsMaintenanceRunning()),
 	})
 	if err != nil {
-		l.Error(err, "failed to instanciate prmetheus histogram")
+		l.Error(err, "failed to instanciate prometheus histogram")
 		return
 	}
 

diff --git a/pkg/sliexporter/probes/postgresql.go b/pkg/sliexporter/probes/postgresql.go
@@ -21,6 +21,7 @@ type PostgreSQL struct {
 	Namespace     string
 	Organization  string
 	HighAvailable bool
+	TLSEnabled    bool
 	ServiceLevel  string
 }
 
@@ -97,7 +98,13 @@ func NewFailingPostgreSQL(service, name, namespace string) (*PostgreSQL, error)
 }
 
 // PGWithCA adds the provided CA to the rootCAs of the pgxpool.
-func PGWithCA(ca []byte) func(*pgxpool.Config) error {
+func PGWithCA(ca []byte, tlsEnabled bool) func(*pgxpool.Config) error {
+	if !tlsEnabled {
+		return func(conf *pgxpool.Config) error {
+			return nil
+		}
+	}
+
 	return func(conf *pgxpool.Config) error {
 		if conf.ConnConfig.TLSConfig == nil {
 			conf.ConnConfig.TLSConfig = &tls.Config{
@@ -116,6 +123,7 @@ func PGWithCA(ca []byte) func(*pgxpool.Config) error {
 		if !conf.ConnConfig.TLSConfig.RootCAs.AppendCertsFromPEM(ca) {
 			return errors.New("cannot append root CA certificates")
 		}
+
 		return nil
 	}
 }
diff --git a/pkg/sliexporter/probes/postgresql_test.go b/pkg/sliexporter/probes/postgresql_test.go
@@ -2,10 +2,17 @@ package probes
 
 import (
 	"context"
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
 	"fmt"
+	"math/big"
 	"testing"
 	"time"
 
+	"github.com/jackc/pgx/v5/pgxpool"
 	"github.com/ory/dockertest"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -89,3 +96,72 @@ func TestPostgreSQL_Fail(t *testing.T) {
 		assert.NoError(t, p.Close())
 	})
 }
+
+func TestPGWithCA(t *testing.T) {
+	t.Parallel()
+	// Generate a private key
+	privateKey, err := rsa.GenerateKey(rand.Reader, 2048)
+	if err != nil {
+		fmt.Println("Failed to generate private key:", err)
+		t.FailNow()
+		return
+	}
+
+	// Create a template for the certificate
+	template := x509.Certificate{
+		SerialNumber: big.NewInt(1),
+		DNSNames:     []string{"localhost"},
+		Subject: pkix.Name{
+			Organization: []string{"Example Corp"},
+		},
+		NotBefore:             time.Now(),
+		NotAfter:              time.Now().Add(365 * 24 * time.Hour), // Valid for one year
+		KeyUsage:              x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
+		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+		BasicConstraintsValid: true,
+	}
+
+	// Create the certificate
+	certBytes, err := x509.CreateCertificate(rand.Reader, &template, &template, &privateKey.PublicKey, privateKey)
+	if err != nil {
+		fmt.Println("Failed to create certificate:", err)
+		t.FailNow()
+		return
+	}
+
+	pembytes := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: certBytes})
+
+	op := PGWithCA(pembytes, true)
+
+	conf, err := pgxpool.ParseConfig("postgresql://foo:bar@localhost:5432/buzz?sslmode=disable")
+	require.NoError(t, err)
+
+	err = op(conf)
+	assert.NoError(t, err)
+	assert.NotNil(t, conf.ConnConfig.TLSConfig)
+	assert.NotNil(t, conf.ConnConfig.TLSConfig.RootCAs)
+}
+
+func TestPGWithCA_DisabledTLS(t *testing.T) {
+	t.Parallel()
+	op := PGWithCA(nil, false)
+
+	conf, err := pgxpool.ParseConfig("postgresql://foo:bar@localhost:5432/buzz?sslmode=disable")
+	require.NoError(t, err)
+
+	err = op(conf)
+	assert.NoError(t, err)
+	assert.Nil(t, conf.ConnConfig.TLSConfig)
+}
+
+func TestPGWithCA_InvalidCA(t *testing.T) {
+	t.Parallel()
+	op := PGWithCA(nil, true)
+
+	conf, err := pgxpool.ParseConfig("postgresql://foo:bar@localhost:5432/buzz?sslmode=disable")
+	require.NoError(t, err)
+
+	err = op(conf)
+	assert.Error(t, err)
+	assert.Equal(t, "got nil CA", err.Error())
+}
diff --git a/pkg/sliexporter/vshnpostgresql_controller/vshnpostgresql_controller.go b/pkg/sliexporter/vshnpostgresql_controller/vshnpostgresql_controller.go
@@ -114,19 +114,27 @@ func (r VSHNPostgreSQLReconciler) fetchProberFor(ctx context.Context, obj slirec
 		ha = false
 	}
 
+	sslmode := "verify-ca"
+	if !inst.Spec.Parameters.Service.TLS.Enabled {
+		sslmode = "disable"
+	}
+
 	probe, err := r.PostgreDialer(vshnpostgresqlsServiceKey, inst.GetName(), inst.GetLabels()[slireconciler.ClaimNamespaceLabel],
 		fmt.Sprintf(
-			"postgresql://%s:%s@%s:%s/%s?sslmode=verify-ca",
+			"postgresql://%s:%s@%s:%s/%s?sslmode=%s",
 			credSecret.Data["POSTGRESQL_USER"],
 			credSecret.Data["POSTGRESQL_PASSWORD"],
 			credSecret.Data["POSTGRESQL_HOST"],
 			credSecret.Data["POSTGRESQL_PORT"],
 			credSecret.Data["POSTGRESQL_DB"],
+			sslmode,
 		), org, string(sla), ha,
-		probes.PGWithCA(credSecret.Data["ca.crt"]))
+		probes.PGWithCA(credSecret.Data["ca.crt"], inst.Spec.Parameters.Service.TLS.Enabled),
+	)
 	if err != nil {
 		return nil, err
 	}
+
 	return probe, nil
 }