Add .certSecretRef for Bucket API

Signed-off-by: Matheus Pimenta <matheuscscp@gmail.com>
2024-05-06 13:08:18 +01:00 · 2024-05-06 13:08:18 +01:00 · bc16734391
parent b41c6532a7
commit bc16734391
9 changed files with 304 additions and 15 deletions
--- a/api/v1beta2/bucket_types.go
+++ b/api/v1beta2/bucket_types.go
@ -83,6 +83,23 @@ type BucketSpec struct {
 	// +optional
 	SecretRef *meta.LocalObjectReference `json:"secretRef,omitempty"`

+	// CertSecretRef can be given the name of a Secret containing
+	// either or both of
+	//
+	// - a PEM-encoded client certificate (`tls.crt`) and private
+	// key (`tls.key`);
+	// - a PEM-encoded CA certificate (`ca.crt`)
+	//
+	// and whichever are supplied, will be used for connecting to the
+	// bucket. The client cert and key are useful if you are
+	// authenticating with a certificate; the CA cert is useful if
+	// you are using a self-signed server certificate. The Secret must
+	// be of type `Opaque` or `kubernetes.io/tls`.
+	//
+	// This field is only supported for the `generic` provider.
+	// +optional
+	CertSecretRef *meta.LocalObjectReference `json:"certSecretRef,omitempty"`
+
 	// Interval at which the Bucket Endpoint is checked for updates.
 	// This interval is approximate and may be subject to jitter to ensure
 	// efficient use of resources.
--- a/api/v1beta2/zz_generated.deepcopy.go
+++ b/api/v1beta2/zz_generated.deepcopy.go
@ -123,6 +123,11 @@ func (in *BucketSpec) DeepCopyInto(out *BucketSpec) {
 		*out = new(meta.LocalObjectReference)
 		**out = **in
 	}
+	if in.CertSecretRef != nil {
+		in, out := &in.CertSecretRef, &out.CertSecretRef
+		*out = new(meta.LocalObjectReference)
+		**out = **in
+	}
 	out.Interval = in.Interval
 	if in.Timeout != nil {
 		in, out := &in.Timeout, &out.Timeout
--- a/config/crd/bases/source.toolkit.fluxcd.io_buckets.yaml
+++ b/config/crd/bases/source.toolkit.fluxcd.io_buckets.yaml
@ -329,6 +329,32 @@ spec:
              bucketName:
                description: BucketName is the name of the object storage bucket.
                type: string
+              certSecretRef:
+                description: |-
+                  CertSecretRef can be given the name of a Secret containing
+                  either or both of
+
+
+                  - a PEM-encoded client certificate (`tls.crt`) and private
+                  key (`tls.key`);
+                  - a PEM-encoded CA certificate (`ca.crt`)
+
+
+                  and whichever are supplied, will be used for connecting to the
+                  bucket. The client cert and key are useful if you are
+                  authenticating with a certificate; the CA cert is useful if
+                  you are using a self-signed server certificate. The Secret must
+                  be of type `Opaque` or `kubernetes.io/tls`.
+
+
+                  This field is only supported for the `generic` provider.
+                properties:
+                  name:
+                    description: Name of the referent.
+                    type: string
+                required:
+                - name
+                type: object
              endpoint:
                description: Endpoint is the object storage address the BucketName
                  is located at.
--- a/docs/api/v1beta2/source.md
+++ b/docs/api/v1beta2/source.md
@ -165,6 +165,32 @@ for the Bucket.</p>
 </tr>
 <tr>
 <td>
+<code>certSecretRef</code><br>
+<em>
+<a href="https://pkg.go.dev/github.com/fluxcd/pkg/apis/meta#LocalObjectReference">
+github.com/fluxcd/pkg/apis/meta.LocalObjectReference
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>CertSecretRef can be given the name of a Secret containing
+either or both of</p>
+<ul>
+<li>a PEM-encoded client certificate (<code>tls.crt</code>) and private
+key (<code>tls.key</code>);</li>
+<li>a PEM-encoded CA certificate (<code>ca.crt</code>)</li>
+</ul>
+<p>and whichever are supplied, will be used for connecting to the
+bucket. The client cert and key are useful if you are
+authenticating with a certificate; the CA cert is useful if
+you are using a self-signed server certificate. The Secret must
+be of type <code>Opaque</code> or <code>kubernetes.io/tls</code>.</p>
+<p>This field is only supported for the <code>generic</code> provider.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>interval</code><br>
 <em>
 <a href="https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#Duration">
@ -1489,6 +1515,32 @@ for the Bucket.</p>
 </tr>
 <tr>
 <td>
+<code>certSecretRef</code><br>
+<em>
+<a href="https://pkg.go.dev/github.com/fluxcd/pkg/apis/meta#LocalObjectReference">
+github.com/fluxcd/pkg/apis/meta.LocalObjectReference
+</a>
+</em>
+</td>
+<td>
+<em>(Optional)</em>
+<p>CertSecretRef can be given the name of a Secret containing
+either or both of</p>
+<ul>
+<li>a PEM-encoded client certificate (<code>tls.crt</code>) and private
+key (<code>tls.key</code>);</li>
+<li>a PEM-encoded CA certificate (<code>ca.crt</code>)</li>
+</ul>
+<p>and whichever are supplied, will be used for connecting to the
+bucket. The client cert and key are useful if you are
+authenticating with a certificate; the CA cert is useful if
+you are using a self-signed server certificate. The Secret must
+be of type <code>Opaque</code> or <code>kubernetes.io/tls</code>.</p>
+<p>This field is only supported for the <code>generic</code> provider.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>interval</code><br>
 <em>
 <a href="https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#Duration">
--- a/docs/spec/v1beta2/buckets.md
+++ b/docs/spec/v1beta2/buckets.md
@ -763,6 +763,67 @@ See [Provider](#provider) for more (provider specific) examples.

 See [Provider](#provider) for more (provider specific) examples.

+### Cert secret reference
+
+`.spec.certSecretRef.name` is an optional field to specify a secret containing
+TLS certificate data. The secret can contain the following keys:
+
+* `tls.crt` and `tls.key`, to specify the client certificate and private key used
+for TLS client authentication. These must be used in conjunction, i.e.
+specifying one without the other will lead to an error.
+* `ca.crt`, to specify the CA certificate used to verify the server, which is
+required if the server is using a self-signed certificate.
+
+If the server is using a self-signed certificate and has TLS client
+authentication enabled, all three values are required.
+
+The Secret should be of type `Opaque` or `kubernetes.io/tls`. All the files in
+the Secret are expected to be [PEM-encoded][pem-encoding]. Assuming you have
+three files; `client.key`, `client.crt` and `ca.crt` for the client private key,
+client certificate and the CA certificate respectively, you can generate the
+required Secret using the `flux create secret tls` command:
+
+```sh
+flux create secret tls minio-tls --tls-key-file=client.key --tls-crt-file=client.crt --ca-crt-file=ca.crt
+```
+
+If TLS client authentication is not required, you can generate the secret with:
+
+```sh
+flux create secret tls minio-tls --ca-crt-file=ca.crt
+```
+
+This API is only supported for the `generic` [provider](#provider).
+
+Example usage:
+
+```yaml
+---
+apiVersion: source.toolkit.fluxcd.io/v1beta2
+kind: Bucket
+metadata:
+  name: example
+  namespace: example
+spec:
+  interval: 5m
+  bucketName: example
+  provider: generic
+  endpoint: minio.example.com
+  certSecretRef:
+    name: minio-tls
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: minio-tls
+  namespace: example
+type: kubernetes.io/tls # or Opaque
+stringData:
+  tls.crt: <PEM-encoded cert>
+  tls.key: <PEM-encoded key>
+  ca.crt: <PEM-encoded cert>
+```
+
 ### Insecure

 `.spec.insecure` is an optional field to allow connecting to an insecure (HTTP)
--- a/internal/controller/bucket_controller.go
+++ b/internal/controller/bucket_controller.go
@ -18,6 +18,7 @@ package controller

 import (
 	"context"
+	stdtls "crypto/tls"
 	"errors"
 	"fmt"
 	"os"
@ -57,6 +58,7 @@ import (
 	"github.com/fluxcd/source-controller/internal/index"
 	sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
 	"github.com/fluxcd/source-controller/internal/reconcile/summarize"
+	"github.com/fluxcd/source-controller/internal/tls"
 	"github.com/fluxcd/source-controller/pkg/azure"
 	"github.com/fluxcd/source-controller/pkg/gcp"
 	"github.com/fluxcd/source-controller/pkg/minio"
@ -421,7 +423,7 @@ func (r *BucketReconciler) reconcileStorage(ctx context.Context, sp *patch.Seria
 // the provider. If this fails, it records v1beta2.FetchFailedCondition=True on
 // the object and returns early.
 func (r *BucketReconciler) reconcileSource(ctx context.Context, sp *patch.SerialPatcher, obj *bucketv1.Bucket, index *index.Digester, dir string) (sreconcile.Result, error) {
-	secret, err := r.getBucketSecret(ctx, obj)
+	secret, err := r.getSecret(ctx, obj.Spec.SecretRef, obj.GetNamespace())
 	if err != nil {
 		e := serror.NewGeneric(err, sourcev1.AuthenticationFailedReason)
 		conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error())
@ -460,7 +462,13 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, sp *patch.Serial
 			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error())
 			return sreconcile.ResultEmpty, e
 		}
-		if provider, err = minio.NewClient(obj, secret); err != nil {
+		tlsConfig, err := r.getTLSConfig(ctx, obj)
+		if err != nil {
+			e := serror.NewGeneric(err, sourcev1.AuthenticationFailedReason)
+			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error())
+			return sreconcile.ResultEmpty, e
+		}
+		if provider, err = minio.NewClient(obj, secret, tlsConfig); err != nil {
 			e := serror.NewGeneric(err, "ClientError")
 			conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error())
 			return sreconcile.ResultEmpty, e
@ -663,15 +671,15 @@ func (r *BucketReconciler) garbageCollect(ctx context.Context, obj *bucketv1.Buc
 	return nil
 }

-// getBucketSecret attempts to fetch the Secret reference if specified on the
-// obj. It returns any client error.
-func (r *BucketReconciler) getBucketSecret(ctx context.Context, obj *bucketv1.Bucket) (*corev1.Secret, error) {
-	if obj.Spec.SecretRef == nil {
+// getSecret attempts to fetch a Secret reference if specified. It returns any client error.
+func (r *BucketReconciler) getSecret(ctx context.Context, secretRef *meta.LocalObjectReference,
+	namespace string) (*corev1.Secret, error) {
+	if secretRef == nil {
 		return nil, nil
 	}
 	secretName := types.NamespacedName{
-		Namespace: obj.GetNamespace(),
-		Name:      obj.Spec.SecretRef.Name,
+		Namespace: namespace,
+		Name:      secretRef.Name,
 	}
 	secret := &corev1.Secret{}
 	if err := r.Get(ctx, secretName, secret); err != nil {
@ -680,6 +688,21 @@ func (r *BucketReconciler) getBucketSecret(ctx context.Context, obj *bucketv1.Bu
 	return secret, nil
 }

+func (r *BucketReconciler) getTLSConfig(ctx context.Context, obj *bucketv1.Bucket) (*stdtls.Config, error) {
+	certSecret, err := r.getSecret(ctx, obj.Spec.CertSecretRef, obj.GetNamespace())
+	if err != nil || certSecret == nil {
+		return nil, err
+	}
+	tlsConfig, _, err := tls.KubeTLSClientConfigFromSecret(*certSecret, obj.Spec.Endpoint)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create TLS config: %w", err)
+	}
+	if tlsConfig == nil {
+		return nil, fmt.Errorf("certificate secret does not contain any TLS configuration")
+	}
+	return tlsConfig, nil
+}
+
 // eventLogf records events, and logs at the same time.
 //
 // This log is different from the debug log in the EventRecorder, in the sense
--- a/internal/controller/bucket_controller_test.go
+++ b/internal/controller/bucket_controller_test.go
@ -510,6 +510,47 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
 				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
 			},
 		},
+		{
+			name:       "Observes non-existing certSecretRef",
+			bucketName: "dummy",
+			beforeFunc: func(obj *bucketv1.Bucket) {
+				obj.Spec.CertSecretRef = &meta.LocalObjectReference{
+					Name: "dummy",
+				}
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
+				conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
+			},
+			wantErr:     true,
+			assertIndex: index.NewDigester(),
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret '/dummy': secrets \"dummy\" not found"),
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
+				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
+			},
+		},
+		{
+			name:       "Observes invalid certSecretRef",
+			bucketName: "dummy",
+			secret: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "dummy",
+				},
+			},
+			beforeFunc: func(obj *bucketv1.Bucket) {
+				obj.Spec.CertSecretRef = &meta.LocalObjectReference{
+					Name: "dummy",
+				}
+				conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
+				conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
+			},
+			wantErr:     true,
+			assertIndex: index.NewDigester(),
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
+				*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
+				*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "certificate secret does not contain any TLS configuration"),
+			},
+		},
 		{
 			name:       "Observes non-existing bucket name",
 			bucketName: "dummy",
--- a/pkg/minio/minio.go
+++ b/pkg/minio/minio.go
@ -18,6 +18,7 @@ package minio

 import (
 	"context"
+	"crypto/tls"
 	"errors"
 	"fmt"

@ -36,7 +37,7 @@ type MinioClient struct {
 }

 // NewClient creates a new Minio storage client.
-func NewClient(bucket *sourcev1.Bucket, secret *corev1.Secret) (*MinioClient, error) {
+func NewClient(bucket *sourcev1.Bucket, secret *corev1.Secret, tlsConfig *tls.Config) (*MinioClient, error) {
 	opt := minio.Options{
 		Region: bucket.Spec.Region,
 		Secure: !bucket.Spec.Insecure,
@ -60,6 +61,18 @@ func NewClient(bucket *sourcev1.Bucket, secret *corev1.Secret) (*MinioClient, er
 		opt.Creds = credentials.NewIAM("")
 	}

+	if opt.Secure && tlsConfig != nil {
+		// Use the default minio transport, but override the TLS config.
+		secure := false // true causes the TLS config to be defined internally, but here we have our own so we just pass false.
+		transport, err := minio.DefaultTransport(secure)
+		if err != nil {
+			// The error returned here is always nil, but we keep the check for future compatibility.
+			return nil, fmt.Errorf("failed to create default minio transport: %w", err)
+		}
+		transport.TLSClientConfig = tlsConfig.Clone()
+		opt.Transport = transport
+	}
+
 	client, err := minio.New(bucket.Spec.Endpoint, &opt)
 	if err != nil {
 		return nil, err
--- a/pkg/minio/minio_test.go
+++ b/pkg/minio/minio_test.go
@ -18,6 +18,9 @@ package minio

 import (
 	"context"
+	"crypto/tls"
+	"crypto/x509"
+	"errors"
 	"fmt"
 	"log"
 	"os"
@ -48,7 +51,7 @@ const (
 var (
 	// testMinioVersion is the version (image tag) of the Minio server image
 	// used to test against.
-	testMinioVersion = "RELEASE.2022-12-12T19-27-27Z"
+	testMinioVersion = "RELEASE.2024-05-07T06-41-25Z"
 	// testMinioRootUser is the root user of the Minio server.
 	testMinioRootUser = "fluxcd"
 	// testMinioRootPassword is the root password of the Minio server.
@ -59,6 +62,8 @@ var (
 	// testMinioClient is the Minio client used to test against, it is set
 	// by TestMain after booting the Minio server.
 	testMinioClient *MinioClient
+	// testTLSConfig is the TLS configuration used to connect to the Minio server.
+	testTLSConfig *tls.Config
 )

 var (
@ -115,6 +120,14 @@ func TestMain(m *testing.M) {
 		log.Fatalf("could not connect to docker: %s", err)
 	}

+	// Load a private key and certificate from a self-signed CA for the Minio server and
+	// a client TLS configuration to connect to the Minio server.
+	var serverCert, serverKey string
+	serverCert, serverKey, testTLSConfig, err = loadServerCertAndClientTLSConfig()
+	if err != nil {
+		log.Fatalf("could not load server cert and client TLS config: %s", err)
+	}
+
 	// Pull the image, create a container based on it, and run it
 	resource, err := pool.RunWithOptions(&dockertest.RunOptions{
 		Repository: "minio/minio",
@ -128,6 +141,10 @@ func TestMain(m *testing.M) {
 			"MINIO_ROOT_PASSWORD=" + testMinioRootPassword,
 		},
 		Cmd: []string{"server", "/data", "--console-address", ":9001"},
+		Mounts: []string{
+			fmt.Sprintf("%s:/root/.minio/certs/public.crt", serverCert),
+			fmt.Sprintf("%s:/root/.minio/certs/private.key", serverKey),
+		},
 	}, func(config *docker.HostConfig) {
 		config.AutoRemove = true
 	})
@ -145,7 +162,7 @@ func TestMain(m *testing.M) {
 	testMinioAddress = fmt.Sprintf("127.0.0.1:%v", resource.GetPort("9000/tcp"))

 	// Construct a Minio client using the address of the Minio server.
-	testMinioClient, err = NewClient(bucketStub(bucket, testMinioAddress), secret.DeepCopy())
+	testMinioClient, err = NewClient(bucketStub(bucket, testMinioAddress), secret.DeepCopy(), testTLSConfig)
 	if err != nil {
 		log.Fatalf("cannot create Minio client: %s", err)
 	}
@ -178,19 +195,19 @@ func TestMain(m *testing.M) {
 }

 func TestNewClient(t *testing.T) {
-	minioClient, err := NewClient(bucketStub(bucket, testMinioAddress), secret.DeepCopy())
+	minioClient, err := NewClient(bucketStub(bucket, testMinioAddress), secret.DeepCopy(), testTLSConfig)
 	assert.NilError(t, err)
 	assert.Assert(t, minioClient != nil)
 }

 func TestNewClientEmptySecret(t *testing.T) {
-	minioClient, err := NewClient(bucketStub(bucket, testMinioAddress), emptySecret.DeepCopy())
+	minioClient, err := NewClient(bucketStub(bucket, testMinioAddress), emptySecret.DeepCopy(), testTLSConfig)
 	assert.NilError(t, err)
 	assert.Assert(t, minioClient != nil)
 }

 func TestNewClientAwsProvider(t *testing.T) {
-	minioClient, err := NewClient(bucketStub(bucketAwsProvider, testMinioAddress), nil)
+	minioClient, err := NewClient(bucketStub(bucketAwsProvider, testMinioAddress), nil, nil)
 	assert.NilError(t, err)
 	assert.Assert(t, minioClient != nil)
 }
@ -295,7 +312,7 @@ func TestValidateSecret(t *testing.T) {
 func bucketStub(bucket sourcev1.Bucket, endpoint string) *sourcev1.Bucket {
 	b := bucket.DeepCopy()
 	b.Spec.Endpoint = endpoint
-	b.Spec.Insecure = true
+	b.Spec.Insecure = false
 	return b
 }

@ -351,3 +368,37 @@ func getObjectFile() string {
 	  timeout: 30s
 	`
 }
+
+func loadServerCertAndClientTLSConfig() (serverCert string, serverKey string, clientConf *tls.Config, err error) {
+	const certsDir = "../../internal/controller/testdata/certs"
+	clientConf = &tls.Config{}
+
+	serverCert, err = filepath.Abs(filepath.Join(certsDir, "server.pem"))
+	if err != nil {
+		return "", "", nil, fmt.Errorf("failed to get server cert path: %w", err)
+	}
+	serverKey, err = filepath.Abs(filepath.Join(certsDir, "server-key.pem"))
+	if err != nil {
+		return "", "", nil, fmt.Errorf("failed to get server key path: %w", err)
+	}
+
+	b, err := os.ReadFile(filepath.Join(certsDir, "ca.pem"))
+	if err != nil {
+		return "", "", nil, fmt.Errorf("failed to load CA: %w", err)
+	}
+	caPool := x509.NewCertPool()
+	if !caPool.AppendCertsFromPEM(b) {
+		return "", "", nil, errors.New("failed to append CA to pool")
+	}
+	clientConf.RootCAs = caPool
+
+	clientCert := filepath.Join(certsDir, "client.pem")
+	clientKey := filepath.Join(certsDir, "client-key.pem")
+	client, err := tls.LoadX509KeyPair(clientCert, clientKey)
+	if err != nil {
+		return "", "", nil, fmt.Errorf("failed to load client cert and key: %w", err)
+	}
+	clientConf.Certificates = []tls.Certificate{client}
+
+	return
+}