From af1652b79463993717478013fda8d61005fa7b96 Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Tue, 4 Jun 2024 17:41:14 +0100 Subject: [PATCH] Add support for .spec.proxySecretRef for generic provider of Bucket API Signed-off-by: Matheus Pimenta --- api/v1beta2/bucket_types.go | 5 +++ api/v1beta2/zz_generated.deepcopy.go | 5 +++ .../source.toolkit.fluxcd.io_buckets.yaml | 11 +++++ docs/api/v1beta2/source.md | 30 ++++++++++++++ docs/spec/v1beta2/buckets.md | 35 ++++++++++++++++ go.mod | 1 + go.sum | 3 ++ internal/controller/bucket_controller.go | 34 ++++++++++++++- internal/controller/bucket_controller_test.go | 41 +++++++++++++++++++ pkg/minio/minio.go | 33 ++++++++++++--- pkg/minio/minio_test.go | 22 ++++++++-- pkg/testing/http_proxy.go | 33 +++++++++++++++ 12 files changed, 242 insertions(+), 11 deletions(-) create mode 100644 pkg/testing/http_proxy.go diff --git a/api/v1beta2/bucket_types.go b/api/v1beta2/bucket_types.go index a1060431e..52d0b5aff 100644 --- a/api/v1beta2/bucket_types.go +++ b/api/v1beta2/bucket_types.go @@ -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. diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index 1611af57c..b62bafecb 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -128,6 +128,11 @@ func (in *BucketSpec) DeepCopyInto(out *BucketSpec) { *out = new(meta.LocalObjectReference) **out = **in } + if in.ProxySecretRef != nil { + in, out := &in.ProxySecretRef, &out.ProxySecretRef + *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 49ff85c0a..992fab966 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_buckets.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_buckets.yaml @@ -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. diff --git a/docs/api/v1beta2/source.md b/docs/api/v1beta2/source.md index 0866e76fa..9410662ab 100644 --- a/docs/api/v1beta2/source.md +++ b/docs/api/v1beta2/source.md @@ -191,6 +191,21 @@ be of type Opaque or kubernetes.io/tls.

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

ProxySecretRef specifies the Secret containing the proxy configuration +to use while communicating with the Bucket server.

+ + + + interval
@@ -1541,6 +1556,21 @@ be of type Opaque or kubernetes.io/tls.

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

ProxySecretRef specifies the Secret containing the proxy configuration +to use while communicating with the Bucket server.

+ + + + interval
diff --git a/docs/spec/v1beta2/buckets.md b/docs/spec/v1beta2/buckets.md index 81ae7d224..0128d44cb 100644 --- a/docs/spec/v1beta2/buckets.md +++ b/docs/spec/v1beta2/buckets.md @@ -824,6 +824,41 @@ stringData: ca.crt: ``` +### 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) diff --git a/go.mod b/go.mod index 82990c75c..b8330eb4a 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/go.sum b/go.sum index 8083e29f5..75843a2a7 100644 --- a/go.sum +++ b/go.sum @@ -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= @@ -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= diff --git a/internal/controller/bucket_controller.go b/internal/controller/bucket_controller.go index 45705e9b3..c12febf4e 100644 --- a/internal/controller/bucket_controller.go +++ b/internal/controller/bucket_controller.go @@ -21,6 +21,7 @@ import ( stdtls "crypto/tls" "errors" "fmt" + "net/url" "os" "path/filepath" "strings" @@ -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 { @@ -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 @@ -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 diff --git a/internal/controller/bucket_controller_test.go b/internal/controller/bucket_controller_test.go index b17ce534e..29ea0aae0 100644 --- a/internal/controller/bucket_controller_test.go +++ b/internal/controller/bucket_controller_test.go @@ -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", diff --git a/pkg/minio/minio.go b/pkg/minio/minio.go index 61a30ded4..9abc548e0 100644 --- a/pkg/minio/minio.go +++ b/pkg/minio/minio.go @@ -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" @@ -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, @@ -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} + }) + } + + 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 } diff --git a/pkg/minio/minio_test.go b/pkg/minio/minio_test.go index a0b25b938..912f225e4 100644 --- a/pkg/minio/minio_test.go +++ b/pkg/minio/minio_test.go @@ -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 ( @@ -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) } @@ -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) } @@ -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() diff --git a/pkg/testing/http_proxy.go b/pkg/testing/http_proxy.go new file mode 100644 index 000000000..ee7fe394c --- /dev/null +++ b/pkg/testing/http_proxy.go @@ -0,0 +1,33 @@ +package pkgtesting + +import ( + "context" + "net" + "net/http" + "net/url" + "testing" + + "github.com/elazarl/goproxy" + "gotest.tools/assert" +) + +// NewHTTPProxy starts an HTTP proxy server in a random port and returns the +// URL of the proxy server and a function to stop the server. +func NewHTTPProxy(t *testing.T) (*url.URL, func()) { + listener, err := net.Listen("tcp", ":0") + assert.NilError(t, err, "could not start proxy server") + + addr := listener.Addr().String() + proxy := goproxy.NewProxyHttpServer() + proxy.Verbose = true + server := &http.Server{ + Addr: addr, + Handler: proxy, + } + + go server.Serve(listener) + return &url.URL{Scheme: "http", Host: addr}, func() { + server.Shutdown(context.Background()) + listener.Close() + } +}