Skip to content

Commit

Permalink
Add support for .spec.proxySecretRef for generic provider of Bucket API
Browse files Browse the repository at this point in the history
Signed-off-by: Matheus Pimenta <[email protected]>
  • Loading branch information
matheuscscp committed Jun 19, 2024
1 parent 59ad5a7 commit af1652b
Show file tree
Hide file tree
Showing 12 changed files with 242 additions and 11 deletions.
5 changes: 5 additions & 0 deletions api/v1beta2/bucket_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ type BucketSpec struct {
// +optional
CertSecretRef *meta.LocalObjectReference `json:"certSecretRef,omitempty"`

// ProxySecretRef specifies the Secret containing the proxy configuration
// to use while communicating with the Bucket server.
// +optional
ProxySecretRef *meta.LocalObjectReference `json:"proxySecretRef,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.

11 changes: 11 additions & 0 deletions config/crd/bases/source.toolkit.fluxcd.io_buckets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,17 @@ spec:
- gcp
- azure
type: string
proxySecretRef:
description: |-
ProxySecretRef specifies the Secret containing the proxy configuration
to use while communicating with the Bucket server.
properties:
name:
description: Name of the referent.
type: string
required:
- name
type: object
region:
description: Region of the Endpoint where the BucketName is located
in.
Expand Down
30 changes: 30 additions & 0 deletions docs/api/v1beta2/source.md
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,21 @@ be of type <code>Opaque</code> or <code>kubernetes.io/tls</code>.</p>
</tr>
<tr>
<td>
<code>proxySecretRef</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>ProxySecretRef specifies the Secret containing the proxy configuration
to use while communicating with the Bucket server.</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 @@ -1541,6 +1556,21 @@ be of type <code>Opaque</code> or <code>kubernetes.io/tls</code>.</p>
</tr>
<tr>
<td>
<code>proxySecretRef</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>ProxySecretRef specifies the Secret containing the proxy configuration
to use while communicating with the Bucket server.</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
35 changes: 35 additions & 0 deletions docs/spec/v1beta2/buckets.md
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,41 @@ stringData:
ca.crt: <PEM-encoded cert>
```

### Proxy secret reference

`.spec.proxySecretRef.name` is an optional field used to specify the name of a
Secret that contains the proxy settings for the object. These settings are used
for all the remote operations related to the Bucket.
The Secret can contain three keys:

- `address`, to specify the address of the proxy server. This is a required key.
- `username`, to specify the username to use if the proxy server is protected by
basic authentication. This is an optional key.
- `password`, to specify the password to use if the proxy server is protected by
basic authentication. This is an optional key.

The proxy server must be HTTP/S.

Example:

```yaml
---
apiVersion: v1
kind: Secret
metadata:
name: http-proxy
type: Opaque
stringData:
address: http://proxy.com
username: mandalorian
password: grogu
```

Proxying can also be configured in the source-controller Deployment directly by
using the standard environment variables such as `HTTPS_PROXY`, `ALL_PROXY`, etc.

`.spec.proxySecretRef.name` takes precedence over all environment variables.

### Insecure

`.spec.insecure` is an optional field to allow connecting to an insecure (HTTP)
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ require (
github.com/distribution/distribution/v3 v3.0.0-alpha.1
github.com/docker/cli v24.0.9+incompatible
github.com/docker/go-units v0.5.0
github.com/elazarl/goproxy v0.0.0-20231117061959-7cc037d33fb5
github.com/fluxcd/cli-utils v0.36.0-flux.7
github.com/fluxcd/pkg/apis/event v0.9.0
github.com/fluxcd/pkg/apis/meta v1.5.0
Expand Down
3 changes: 3 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,8 @@ github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkp
github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto=
github.com/elazarl/goproxy v0.0.0-20231117061959-7cc037d33fb5 h1:m62nsMU279qRD9PQSWD1l66kmkXzuYcnVJqL4XLeV2M=
github.com/elazarl/goproxy v0.0.0-20231117061959-7cc037d33fb5/go.mod h1:Ro8st/ElPeALwNFlcTpWmkr6IoMFfkjXAvTHpevnDsM=
github.com/elazarl/goproxy/ext v0.0.0-20190711103511-473e67f1d7d2 h1:dWB6v3RcOy03t/bUadywsbyrQwCqZeNIEX6M1OtSZOM=
github.com/elazarl/goproxy/ext v0.0.0-20190711103511-473e67f1d7d2/go.mod h1:gNh8nYJoAm43RfaxurUnxr+N1PwuFV3ZMl/efxlIlY8=
github.com/emicklei/go-restful/v3 v3.12.0 h1:y2DdzBAURM29NFF94q6RaY4vjIH1rtwDapwQtU84iWk=
github.com/emicklei/go-restful/v3 v3.12.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc=
github.com/emicklei/proto v1.12.1 h1:6n/Z2pZAnBwuhU66Gs8160B8rrrYKo7h2F2sCOnNceE=
Expand Down Expand Up @@ -831,6 +833,7 @@ github.com/redis/go-redis/v9 v9.5.1/go.mod h1:hdY0cQFCN4fnSYT6TkisLufl/4W5UIXyv0
github.com/rivo/uniseg v0.2.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
github.com/rivo/uniseg v0.4.4 h1:8TfxU8dW6PdqD27gjM8MVNuicgxIjxpm4K7x4jp8sis=
github.com/rivo/uniseg v0.4.4/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88=
github.com/rogpeppe/go-charset v0.0.0-20180617210344-2471d30d28b4/go.mod h1:qgYeAmZ5ZIpBWTGllZSQnw97Dj+woV0toclVaRGI8pc=
github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
github.com/rs/xid v1.5.0 h1:mKX4bl4iPYJtEIxp6CYiUuLQ/8DYMoz0PUdtGgMFRVc=
Expand Down
34 changes: 33 additions & 1 deletion internal/controller/bucket_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
stdtls "crypto/tls"
"errors"
"fmt"
"net/url"
"os"
"path/filepath"
"strings"
Expand Down Expand Up @@ -431,6 +432,13 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, sp *patch.Serial
return sreconcile.ResultEmpty, e
}

proxyURL, err := r.getProxyURL(ctx, obj)
if err != nil {
e := serror.NewGeneric(err, sourcev1.AuthenticationFailedReason)
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error())
return sreconcile.ResultEmpty, e
}

// Construct provider client
var provider BucketProvider
switch obj.Spec.Provider {
Expand Down Expand Up @@ -468,7 +476,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, tlsConfig); err != nil {
if provider, err = minio.NewClient(obj, secret, tlsConfig, proxyURL); err != nil {
e := serror.NewGeneric(err, "ClientError")
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Error())
return sreconcile.ResultEmpty, e
Expand Down Expand Up @@ -703,6 +711,30 @@ func (r *BucketReconciler) getTLSConfig(ctx context.Context, obj *bucketv1.Bucke
return tlsConfig, nil
}

func (r *BucketReconciler) getProxyURL(ctx context.Context, obj *bucketv1.Bucket) (*url.URL, error) {
namespace := obj.GetNamespace()
proxySecret, err := r.getSecret(ctx, obj.Spec.ProxySecretRef, namespace)
if err != nil || proxySecret == nil {
return nil, err
}
proxyData := proxySecret.Data
address, ok := proxyData["address"]
if !ok {
return nil, fmt.Errorf("invalid proxy secret '%s/%s': key 'address' is missing",
obj.Spec.ProxySecretRef.Name, namespace)
}
proxyURL, err := url.Parse(string(address))
if err != nil {
return nil, fmt.Errorf("failed to parse proxy address '%s': %w", address, err)
}
user, hasUser := proxyData["username"]
password, hasPassword := proxyData["password"]
if hasUser || hasPassword {
proxyURL.User = url.UserPassword(string(user), string(password))
}
return proxyURL, 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 @@ -551,6 +551,47 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "certificate secret does not contain any TLS configuration"),
},
},
{
name: "Observes non-existing proxySecretRef",
bucketName: "dummy",
beforeFunc: func(obj *bucketv1.Bucket) {
obj.Spec.ProxySecretRef = &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 proxySecretRef",
bucketName: "dummy",
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "dummy",
},
},
beforeFunc: func(obj *bucketv1.Bucket) {
obj.Spec.ProxySecretRef = &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, "invalid proxy secret 'dummy/': key 'address' is missing"),
},
},
{
name: "Observes non-existing bucket name",
bucketName: "dummy",
Expand Down
33 changes: 27 additions & 6 deletions pkg/minio/minio.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"crypto/tls"
"errors"
"fmt"
"net/http"
"net/url"

"github.com/minio/minio-go/v7"
"github.com/minio/minio-go/v7/pkg/credentials"
Expand All @@ -37,7 +39,9 @@ type MinioClient struct {
}

// NewClient creates a new Minio storage client.
func NewClient(bucket *sourcev1.Bucket, secret *corev1.Secret, tlsConfig *tls.Config) (*MinioClient, error) {
func NewClient(bucket *sourcev1.Bucket, secret *corev1.Secret,
tlsConfig *tls.Config, proxyURL *url.URL) (*MinioClient, error) {

opt := minio.Options{
Region: bucket.Spec.Region,
Secure: !bucket.Spec.Insecure,
Expand All @@ -61,15 +65,32 @@ func NewClient(bucket *sourcev1.Bucket, secret *corev1.Secret, tlsConfig *tls.Co
opt.Creds = credentials.NewIAM("")
}

var transportOpts []func(*http.Transport)

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)
transportOpts = append(transportOpts, func(t *http.Transport) {
t.TLSClientConfig = tlsConfig.Clone()
})
} else if !opt.Secure {
transportOpts = append(transportOpts, func(t *http.Transport) {
t.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}

Check failure

Code scanning / CodeQL

Disabled TLS certificate check High

InsecureSkipVerify should not be used in production code.
})
}

if proxyURL != nil {
transportOpts = append(transportOpts, func(t *http.Transport) {
t.Proxy = http.ProxyURL(proxyURL)
})
}

if len(transportOpts) > 0 {
transport, err := minio.DefaultTransport(true /*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()
for _, opt := range transportOpts {
opt(transport)
}
opt.Transport = transport
}

Expand Down
22 changes: 18 additions & 4 deletions pkg/minio/minio_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
"github.com/fluxcd/pkg/sourceignore"

sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
pkgtesting "github.com/fluxcd/source-controller/pkg/testing"
)

const (
Expand Down Expand Up @@ -162,7 +163,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(), testTLSConfig)
testMinioClient, err = NewClient(bucketStub(bucket, testMinioAddress), secret.DeepCopy(), testTLSConfig, nil)
if err != nil {
log.Fatalf("cannot create Minio client: %s", err)
}
Expand Down Expand Up @@ -195,19 +196,19 @@ func TestMain(m *testing.M) {
}

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

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

func TestNewClientAwsProvider(t *testing.T) {
minioClient, err := NewClient(bucketStub(bucketAwsProvider, testMinioAddress), nil, nil)
minioClient, err := NewClient(bucketStub(bucketAwsProvider, testMinioAddress), nil, nil, nil)
assert.NilError(t, err)
assert.Assert(t, minioClient != nil)
}
Expand All @@ -234,6 +235,19 @@ func TestFGetObject(t *testing.T) {
assert.NilError(t, err)
}

func TestNewClientAndFGetObjectWithProxy(t *testing.T) {
proxyURL, closeProxy := pkgtesting.NewHTTPProxy(t)
defer closeProxy()
minioClient, err := NewClient(bucketStub(bucket, testMinioAddress), secret.DeepCopy(), testTLSConfig, proxyURL)
assert.NilError(t, err)
assert.Assert(t, minioClient != nil)
ctx := context.Background()
tempDir := t.TempDir()
path := filepath.Join(tempDir, sourceignore.IgnoreFile)
_, err = minioClient.FGetObject(ctx, bucketName, objectName, path)
assert.NilError(t, err)
}

func TestFGetObjectNotExists(t *testing.T) {
ctx := context.Background()
tempDir := t.TempDir()
Expand Down
Loading

0 comments on commit af1652b

Please sign in to comment.