From 944f4cfa10c56248eb42352efbd2ca097149c528 Mon Sep 17 00:00:00 2001 From: Sanskar Jaiswal Date: Tue, 30 May 2023 17:54:58 +0530 Subject: [PATCH] gitrepo: Add support for specifying proxy per `GitRepository` Add `.spec.proxySecretRef.name` to the `GitRepository` API to allow referencing a secret containing the proxy settings to be used for all remote Git operations for the particular `GitRepository` object. It takes precedence over any proxy configured through enviornment variables. Signed-off-by: Sanskar Jaiswal --- Makefile | 11 +-- api/v1/gitrepository_types.go | 5 ++ api/v1/zz_generated.deepcopy.go | 5 ++ ...rce.toolkit.fluxcd.io_gitrepositories.yaml | 10 +++ docs/api/v1/source.md | 30 +++++++ docs/spec/v1/gitrepositories.md | 49 +++++++++++ .../controller/gitrepository_controller.go | 87 +++++++++++++++---- .../gitrepository_controller_test.go | 73 ++++++++++++++++ 8 files changed, 245 insertions(+), 25 deletions(-) diff --git a/Makefile b/Makefile index b29cc622f..cd89fe65e 100644 --- a/Makefile +++ b/Makefile @@ -63,11 +63,11 @@ endif all: build -build: check-deps ## Build manager binary +build: ## Build manager binary go build $(GO_STATIC_FLAGS) -o $(BUILD_DIR)/bin/manager main.go KUBEBUILDER_ASSETS?="$(shell $(ENVTEST) --arch=$(ENVTEST_ARCH) use -i $(ENVTEST_KUBERNETES_VERSION) --bin-dir=$(ENVTEST_ASSETS_DIR) -p path)" -test: install-envtest test-api check-deps ## Run all tests +test: install-envtest test-api ## Run all tests HTTPS_PROXY="" HTTP_PROXY="" \ KUBEBUILDER_ASSETS=$(KUBEBUILDER_ASSETS) \ GIT_CONFIG_GLOBAL=/dev/null \ @@ -76,7 +76,7 @@ test: install-envtest test-api check-deps ## Run all tests $(GO_TEST_ARGS) \ -coverprofile cover.out -test-ctrl: install-envtest test-api check-deps ## Run controller tests +test-ctrl: install-envtest test-api ## Run controller tests HTTPS_PROXY="" HTTP_PROXY="" \ KUBEBUILDER_ASSETS=$(KUBEBUILDER_ASSETS) \ GIT_CONFIG_GLOBAL=/dev/null \ @@ -85,11 +85,6 @@ test-ctrl: install-envtest test-api check-deps ## Run controller tests -v ./internal/controller \ -coverprofile cover.out -check-deps: -ifeq ($(shell uname -s),Darwin) - if ! command -v pkg-config &> /dev/null; then echo "pkg-config is required"; exit 1; fi -endif - test-api: ## Run api tests cd api; go test $(GO_TEST_ARGS) ./... -coverprofile cover.out diff --git a/api/v1/gitrepository_types.go b/api/v1/gitrepository_types.go index 4475acba4..f08efca24 100644 --- a/api/v1/gitrepository_types.go +++ b/api/v1/gitrepository_types.go @@ -78,6 +78,11 @@ type GitRepositorySpec struct { // +optional Verification *GitRepositoryVerification `json:"verify,omitempty"` + // ProxySecretRef specifies the Secret containing the proxy configuration + // to use while communicating with the Git server. + // +optional + ProxySecretRef *meta.LocalObjectReference `json:"proxySecretRef,omitempty"` + // Ignore overrides the set of excluded patterns in the .sourceignore format // (which is the same as .gitignore). If not provided, a default will be used, // consult the documentation for your version to find out what those are. diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 0b0fde694..23630ff9f 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -169,6 +169,11 @@ func (in *GitRepositorySpec) DeepCopyInto(out *GitRepositorySpec) { *out = new(GitRepositoryVerification) **out = **in } + if in.ProxySecretRef != nil { + in, out := &in.ProxySecretRef, &out.ProxySecretRef + *out = new(meta.LocalObjectReference) + **out = **in + } if in.Ignore != nil { in, out := &in.Ignore, &out.Ignore *out = new(string) diff --git a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml index 3097292ca..ba19ecd05 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml @@ -90,6 +90,16 @@ spec: description: Interval at which to check the GitRepository for updates. pattern: ^([0-9]+(\.[0-9]+)?(ms|s|m|h))+$ type: string + proxySecretRef: + description: ProxySecretRef specifies the Secret containing the proxy + configuration to use while communicating with the Git server. + properties: + name: + description: Name of the referent. + type: string + required: + - name + type: object recurseSubmodules: description: RecurseSubmodules enables the initialization of all submodules within the GitRepository as cloned from the URL, using their default diff --git a/docs/api/v1/source.md b/docs/api/v1/source.md index ed4862bbe..7e77455e0 100644 --- a/docs/api/v1/source.md +++ b/docs/api/v1/source.md @@ -157,6 +157,21 @@ signature(s).

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

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

+ + + + ignore
string @@ -593,6 +608,21 @@ signature(s).

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

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

+ + + + ignore
string diff --git a/docs/spec/v1/gitrepositories.md b/docs/spec/v1/gitrepositories.md index 0630901ea..4992f24d7 100644 --- a/docs/spec/v1/gitrepositories.md +++ b/docs/spec/v1/gitrepositories.md @@ -439,6 +439,55 @@ GitRepository, and changes to the resource or in the Git repository will not result in a new Artifact. When the field is set to `false` or removed, it will resume. +### 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 remote Git operations related to the GitRepository. +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 either HTTP/S or SOCKS5. You can use a SOCKS5 proxy +with a HTTP/S Git repository url. + +Examples: + +```yaml +--- +apiVersion: v1 +kind: Secret +metadata: + name: http-proxy +type: Opaque +stringData: + address: http://proxy.com + username: mandalorian + password: grogu +``` + +```yaml +--- +apiVersion: v1 +kind: Secret +metadata: + name: ssh-proxy +type: Opaque +stringData: + address: socks5://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. + ### Recurse submodules `.spec.recurseSubmodules` is an optional field to enable the initialization of diff --git a/internal/controller/gitrepository_controller.go b/internal/controller/gitrepository_controller.go index d620c82fb..c3fb38880 100644 --- a/internal/controller/gitrepository_controller.go +++ b/internal/controller/gitrepository_controller.go @@ -28,6 +28,7 @@ import ( securejoin "github.com/cyphar/filepath-securejoin" "github.com/fluxcd/pkg/runtime/logger" + "github.com/go-git/go-git/v5/plumbing/transport" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -473,24 +474,19 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch conditions.Delete(obj, sourcev1.SourceVerifiedCondition) } - var authData map[string][]byte - if obj.Spec.SecretRef != nil { - // Attempt to retrieve secret - name := types.NamespacedName{ - Namespace: obj.GetNamespace(), - Name: obj.Spec.SecretRef.Name, - } - var secret corev1.Secret - if err := r.Client.Get(ctx, name, &secret); err != nil { + var proxyOpts *transport.ProxyOptions + if obj.Spec.ProxySecretRef != nil { + var err error + proxyOpts, err = r.getProxyOpts(ctx, obj.Spec.ProxySecretRef.Name, obj.GetNamespace()) + if err != nil { e := serror.NewGeneric( - fmt.Errorf("failed to get secret '%s': %w", name.String(), err), + fmt.Errorf("failed to configure proxy options: %w", err), sourcev1.AuthenticationFailedReason, ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) // Return error as the world as observed may change return sreconcile.ResultEmpty, e } - authData = secret.Data } u, err := url.Parse(obj.Spec.URL) @@ -503,14 +499,14 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch return sreconcile.ResultEmpty, e } - // Configure authentication strategy to access the source - authOpts, err := git.NewAuthOptions(*u, authData) + authOpts, err := r.getAuthOpts(ctx, obj, *u) if err != nil { e := serror.NewGeneric( fmt.Errorf("failed to configure authentication options: %w", err), sourcev1.AuthenticationFailedReason, ) conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) + // Return error as the world as observed may change return sreconcile.ResultEmpty, e } @@ -536,7 +532,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch // Persist the ArtifactSet. *includes = *artifacts - c, err := r.gitCheckout(ctx, obj, authOpts, dir, true) + c, err := r.gitCheckout(ctx, obj, authOpts, proxyOpts, dir, true) if err != nil { return sreconcile.ResultEmpty, err } @@ -578,7 +574,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch // If we can't skip the reconciliation, checkout again without any // optimization. - c, err := r.gitCheckout(ctx, obj, authOpts, dir, false) + c, err := r.gitCheckout(ctx, obj, authOpts, proxyOpts, dir, false) if err != nil { return sreconcile.ResultEmpty, err } @@ -606,6 +602,60 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch return sreconcile.ResultSuccess, nil } +// getProxyOpts fetches the secret containing the proxy settings, constructs a +// transport.ProxyOptions object using those settings and then returns it. +func (r *GitRepositoryReconciler) getProxyOpts(ctx context.Context, proxySecretName, + proxySecretNamespace string) (*transport.ProxyOptions, error) { + proxyData, err := r.getSecretData(ctx, proxySecretName, proxySecretNamespace) + if err != nil { + return nil, fmt.Errorf("failed to get proxy secret '%s/%s': %w", proxySecretNamespace, proxySecretName, err) + } + address, ok := proxyData["address"] + if !ok { + return nil, fmt.Errorf("invalid proxy secret '%s/%s': key 'address' is missing", proxySecretNamespace, proxySecretName) + } + + proxyOpts := &transport.ProxyOptions{ + URL: string(address), + Username: string(proxyData["username"]), + Password: string(proxyData["password"]), + } + return proxyOpts, nil +} + +// getAuthOpts fetches the secret containing the auth options (if specified), +// constructs a git.AuthOptions object using those options along with the provided +// URL and returns it. +func (r *GitRepositoryReconciler) getAuthOpts(ctx context.Context, obj *sourcev1.GitRepository, u url.URL) (*git.AuthOptions, error) { + var authData map[string][]byte + if obj.Spec.SecretRef != nil { + var err error + authData, err = r.getSecretData(ctx, obj.Spec.SecretRef.Name, obj.GetNamespace()) + if err != nil { + return nil, fmt.Errorf("failed to get secret '%s/%s': %w", obj.GetNamespace(), obj.Spec.SecretRef.Name, err) + } + } + + // Configure authentication strategy to access the source + authOpts, err := git.NewAuthOptions(u, authData) + if err != nil { + return nil, err + } + return authOpts, nil +} + +func (r *GitRepositoryReconciler) getSecretData(ctx context.Context, name, namespace string) (map[string][]byte, error) { + key := types.NamespacedName{ + Namespace: namespace, + Name: name, + } + var secret corev1.Secret + if err := r.Client.Get(ctx, key, &secret); err != nil { + return nil, err + } + return secret.Data, nil +} + // reconcileArtifact archives a new Artifact to the Storage, if the current // (Status) data on the object does not match the given. // @@ -776,8 +826,8 @@ func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context, sp *patc // gitCheckout builds checkout options with the given configurations and // performs a git checkout. -func (r *GitRepositoryReconciler) gitCheckout(ctx context.Context, - obj *sourcev1.GitRepository, authOpts *git.AuthOptions, dir string, optimized bool) (*git.Commit, error) { +func (r *GitRepositoryReconciler) gitCheckout(ctx context.Context, obj *sourcev1.GitRepository, + authOpts *git.AuthOptions, proxyOpts *transport.ProxyOptions, dir string, optimized bool) (*git.Commit, error) { // Configure checkout strategy. cloneOpts := repository.CloneConfig{ RecurseSubmodules: obj.Spec.RecurseSubmodules, @@ -807,6 +857,9 @@ func (r *GitRepositoryReconciler) gitCheckout(ctx context.Context, if authOpts.Transport == git.HTTP { clientOpts = append(clientOpts, gogit.WithInsecureCredentialsOverHTTP()) } + if proxyOpts != nil { + clientOpts = append(clientOpts, gogit.WithProxy(*proxyOpts)) + } gitReader, err := gogit.NewClient(dir, authOpts, clientOpts...) if err != nil { diff --git a/internal/controller/gitrepository_controller_test.go b/internal/controller/gitrepository_controller_test.go index aa9b30700..8b452daa6 100644 --- a/internal/controller/gitrepository_controller_test.go +++ b/internal/controller/gitrepository_controller_test.go @@ -33,6 +33,7 @@ import ( "github.com/go-git/go-git/v5/config" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" + "github.com/go-git/go-git/v5/plumbing/transport" "github.com/go-git/go-git/v5/storage/memory" . "github.com/onsi/gomega" sshtestdata "golang.org/x/crypto/ssh/testdata" @@ -1619,6 +1620,78 @@ func TestGitRepositoryReconciler_verifyCommitSignature(t *testing.T) { } } +func TestGitRepositoryReconciler_getProxyOpts(t *testing.T) { + invalidProxy := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "invalid-proxy", + Namespace: "default", + }, + Data: map[string][]byte{ + "url": []byte("https://example.com"), + }, + } + validProxy := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid-proxy", + Namespace: "default", + }, + Data: map[string][]byte{ + "address": []byte("https://example.com"), + "username": []byte("user"), + "password": []byte("pass"), + }, + } + + clientBuilder := fakeclient.NewClientBuilder(). + WithScheme(testEnv.GetScheme()). + WithObjects(invalidProxy, validProxy) + + r := &GitRepositoryReconciler{ + Client: clientBuilder.Build(), + } + + tests := []struct { + name string + secret string + err string + proxyOpts *transport.ProxyOptions + }{ + { + name: "non-existent secret", + secret: "non-existent", + err: "failed to get proxy secret 'default/non-existent': ", + }, + { + name: "invalid proxy secret", + secret: "invalid-proxy", + err: "invalid proxy secret 'default/invalid-proxy': key 'address' is missing", + }, + { + name: "valid proxy secret", + secret: "valid-proxy", + proxyOpts: &transport.ProxyOptions{ + URL: "https://example.com", + Username: "user", + Password: "pass", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + opts, err := r.getProxyOpts(context.TODO(), tt.secret, "default") + if opts != nil { + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(opts).To(Equal(tt.proxyOpts)) + } else { + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring(tt.err)) + } + }) + } +} + func TestGitRepositoryReconciler_ConditionsUpdate(t *testing.T) { g := NewWithT(t)