Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add .spec.certSecretRef to Bucket API #1475

Merged
merged 1 commit into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions api/v1beta2/bucket_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions api/v1beta2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 26 additions & 0 deletions config/crd/bases/source.toolkit.fluxcd.io_buckets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
52 changes: 52 additions & 0 deletions docs/api/v1beta2/source.md
Original file line number Diff line number Diff line change
Expand Up @@ -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">
Expand Down Expand Up @@ -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">
Expand Down
61 changes: 61 additions & 0 deletions docs/spec/v1beta2/buckets.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
39 changes: 31 additions & 8 deletions internal/controller/bucket_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package controller

import (
"context"
stdtls "crypto/tls"
"errors"
"fmt"
"os"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down
41 changes: 41 additions & 0 deletions internal/controller/bucket_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
15 changes: 14 additions & 1 deletion pkg/minio/minio.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package minio

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

Expand All @@ -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,
Expand All @@ -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
Expand Down
Loading