From aaf53e65b79dac959a5110c516a78da6a9a5a865 Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Mon, 6 May 2024 13:08:18 +0100 Subject: [PATCH] Add .certSecretRef for Bucket API Signed-off-by: Matheus Pimenta --- api/v1/condition_types.go | 5 ++ api/v1beta2/bucket_types.go | 17 +++++ api/v1beta2/zz_generated.deepcopy.go | 5 ++ .../source.toolkit.fluxcd.io_buckets.yaml | 26 ++++++++ docs/api/v1beta2/source.md | 52 +++++++++++++++ internal/controller/bucket_controller.go | 37 ++++++++--- pkg/minio/minio.go | 15 ++++- pkg/minio/minio_test.go | 63 +++++++++++++++++-- 8 files changed, 205 insertions(+), 15 deletions(-) diff --git a/api/v1/condition_types.go b/api/v1/condition_types.go index 72c7e67a2..ba9bc8f8f 100644 --- a/api/v1/condition_types.go +++ b/api/v1/condition_types.go @@ -71,6 +71,11 @@ const ( // required fields, or the provided credentials do not match. AuthenticationFailedReason string = "AuthenticationFailed" + // CertificateFailedReason signals that a problem occurred while + // fetching the certificate Secret, or the expected fields are missing + // or invalid. + CertificateFailedReason string = "CertificateFailed" + // VerificationError signals that the Source's verification // check failed. VerificationError string = "VerificationError" diff --git a/api/v1beta2/bucket_types.go b/api/v1beta2/bucket_types.go index 5d3d9c7d0..a1060431e 100644 --- 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. diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index 12cda6cb0..1611af57c 100644 --- 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 diff --git a/config/crd/bases/source.toolkit.fluxcd.io_buckets.yaml b/config/crd/bases/source.toolkit.fluxcd.io_buckets.yaml index de096bf51..49ff85c0a 100644 --- 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. diff --git a/docs/api/v1beta2/source.md b/docs/api/v1beta2/source.md index 439c81afd..0866e76fa 100644 --- a/docs/api/v1beta2/source.md +++ b/docs/api/v1beta2/source.md @@ -165,6 +165,32 @@ for the Bucket.

+certSecretRef
+ + +github.com/fluxcd/pkg/apis/meta.LocalObjectReference + + + + +(Optional) +

CertSecretRef can be given the name of a Secret containing +either or both of

+ +

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.

+ + + + interval
@@ -1489,6 +1515,32 @@ for the Bucket.

+certSecretRef
+ + +github.com/fluxcd/pkg/apis/meta.LocalObjectReference + + + + +(Optional) +

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.

+ + + + interval
diff --git a/internal/controller/bucket_controller.go b/internal/controller/bucket_controller.go index f12319e62..7a973b0fc 100644 --- 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,9 @@ 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) + objNamespace := obj.GetNamespace() + + secret, err := r.getSecret(ctx, obj.Spec.SecretRef, objNamespace) if err != nil { e := serror.NewGeneric(err, sourcev1.AuthenticationFailedReason) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error()) @@ -429,6 +433,23 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, sp *patch.Serial return sreconcile.ResultEmpty, e } + // Fetch and validate certificate secret if specified on the object. + certSecret, err := r.getSecret(ctx, obj.Spec.CertSecretRef, objNamespace) + if err != nil { + e := serror.NewGeneric(err, sourcev1.CertificateFailedReason) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error()) + return sreconcile.ResultEmpty, e + } + var tlsConfig *stdtls.Config + if certSecret != nil { + tlsConfig, _, err = tls.KubeTLSClientConfigFromSecret(*certSecret, obj.Spec.Endpoint) + if err != nil { + e := serror.NewGeneric(err, sourcev1.CertificateFailedReason) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error()) + return sreconcile.ResultEmpty, e + } + } + // Construct provider client var provider BucketProvider switch obj.Spec.Provider { @@ -460,7 +481,7 @@ 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 { + 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 +684,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 { diff --git a/pkg/minio/minio.go b/pkg/minio/minio.go index 7343f753e..61a30ded4 100644 --- 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 diff --git a/pkg/minio/minio_test.go b/pkg/minio/minio_test.go index 40eb3deee..a0b25b938 100644 --- 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 +}