From cd34911c03dec1962bf8f76848b7d2021c1a25bf 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 | 8 +-- 8 files changed, 152 insertions(+), 13 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..1e76257fa 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 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. + 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 + 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..6c6a0093a 100644 --- a/pkg/minio/minio_test.go +++ b/pkg/minio/minio_test.go @@ -145,7 +145,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(), nil /*FIXME*/) if err != nil { log.Fatalf("cannot create Minio client: %s", err) } @@ -178,19 +178,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(), nil /*FIXME*/) 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(), nil /*FIXME*/) 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 /*FIXME*/) assert.NilError(t, err) assert.Assert(t, minioClient != nil) }