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

+ +

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/docs/spec/v1beta2/buckets.md b/docs/spec/v1beta2/buckets.md index 14d6a0d08..81ae7d224 100644 --- 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: + tls.key: + ca.crt: +``` + ### Insecure `.spec.insecure` is an optional field to allow connecting to an insecure (HTTP) diff --git a/internal/controller/bucket_controller.go b/internal/controller/bucket_controller.go index f12319e62..45705e9b3 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,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 diff --git a/internal/controller/bucket_controller_test.go b/internal/controller/bucket_controller_test.go index 2dd23dd20..b17ce534e 100644 --- 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", 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 +}