From 03a118a94c1684bd9800df954aa49de2aa3fcf91 Mon Sep 17 00:00:00 2001 From: Matheus Pimenta Date: Wed, 3 Jul 2024 10:38:02 +0100 Subject: [PATCH] Add proxy support for OCIRepository API Signed-off-by: Matheus Pimenta --- api/v1beta2/ocirepository_types.go | 5 + api/v1beta2/zz_generated.deepcopy.go | 5 + ...rce.toolkit.fluxcd.io_ocirepositories.yaml | 11 + docs/api/v1beta2/source.md | 30 ++ docs/spec/v1beta2/ocirepositories.md | 41 +++ .../controller/ocirepository_controller.go | 76 ++++- .../ocirepository_controller_test.go | 314 +++++++++++++++++- internal/oci/cosign/cosign_test.go | 63 ++++ internal/oci/notation/notation.go | 42 ++- internal/oci/notation/notation_test.go | 62 +++- tests/listener/listener.go | 2 +- tests/registry/registry.go | 123 +++++++ 12 files changed, 751 insertions(+), 23 deletions(-) create mode 100644 tests/registry/registry.go diff --git a/api/v1beta2/ocirepository_types.go b/api/v1beta2/ocirepository_types.go index 1e8338393..9030fab74 100644 --- a/api/v1beta2/ocirepository_types.go +++ b/api/v1beta2/ocirepository_types.go @@ -116,6 +116,11 @@ type OCIRepositorySpec struct { // +optional CertSecretRef *meta.LocalObjectReference `json:"certSecretRef,omitempty"` + // ProxySecretRef specifies the Secret containing the proxy configuration + // to use while communicating with the container registry. + // +optional + ProxySecretRef *meta.LocalObjectReference `json:"proxySecretRef,omitempty"` + // Interval at which the OCIRepository URL 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 2d0877f83..1a7c8fc79 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -799,6 +799,11 @@ func (in *OCIRepositorySpec) DeepCopyInto(out *OCIRepositorySpec) { *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_ocirepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_ocirepositories.yaml index 4e2dc576e..a6098b72a 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_ocirepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_ocirepositories.yaml @@ -131,6 +131,17 @@ spec: - azure - gcp type: string + proxySecretRef: + description: |- + ProxySecretRef specifies the Secret containing the proxy configuration + to use while communicating with the container registry. + properties: + name: + description: Name of the referent. + type: string + required: + - name + type: object ref: description: |- The OCI reference to pull and monitor for changes, diff --git a/docs/api/v1beta2/source.md b/docs/api/v1beta2/source.md index f70f6a951..10ef720f5 100644 --- a/docs/api/v1beta2/source.md +++ b/docs/api/v1beta2/source.md @@ -1235,6 +1235,21 @@ been deprecated.

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

ProxySecretRef specifies the Secret containing the proxy configuration +to use while communicating with the container registry.

+ + + + interval
@@ -3313,6 +3328,21 @@ been deprecated.

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

ProxySecretRef specifies the Secret containing the proxy configuration +to use while communicating with the container registry.

+ + + + interval
diff --git a/docs/spec/v1beta2/ocirepositories.md b/docs/spec/v1beta2/ocirepositories.md index aafd6c7fb..eb5de4c5f 100644 --- a/docs/spec/v1beta2/ocirepositories.md +++ b/docs/spec/v1beta2/ocirepositories.md @@ -330,6 +330,47 @@ data: deprecated. If you have any Secrets using these keys and specified in an OCIRepository, the controller will log a deprecation warning. +### 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 OCIRepository. +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. + +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. + +**Warning:** [Cosign](https://github.com/sigstore/cosign) *keyless* +[verification](#verification) is not supported for this API. If you +require cosign keyless verification to use a proxy you must use the +standard environment variables mentioned above. If you specify a +`proxySecretRef` the controller will simply send out the requests +needed for keyless verification without the associated object-level +proxy settings. + ### Insecure `.spec.insecure` is an optional field to allow connecting to an insecure (HTTP) diff --git a/internal/controller/ocirepository_controller.go b/internal/controller/ocirepository_controller.go index b3f2a3ea6..eaaf06474 100644 --- a/internal/controller/ocirepository_controller.go +++ b/internal/controller/ocirepository_controller.go @@ -24,6 +24,7 @@ import ( "fmt" "io" "net/http" + "net/url" "os" "path/filepath" "regexp" @@ -437,7 +438,7 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch conditions.GetObservedGeneration(obj, sourcev1.SourceVerifiedCondition) != obj.Generation || conditions.IsFalse(obj, sourcev1.SourceVerifiedCondition) { - result, err := r.verifySignature(ctx, obj, ref, keychain, auth, opts...) + result, err := r.verifySignature(ctx, obj, ref, keychain, auth, transport, opts...) if err != nil { provider := obj.Spec.Verify.Provider if obj.Spec.Verify.SecretRef == nil && obj.Spec.Verify.Provider == "cosign" { @@ -623,7 +624,10 @@ func (r *OCIRepositoryReconciler) digestFromRevision(revision string) string { // If not, when using cosign it falls back to a keyless approach for verification. // When notation is used, a trust policy is required to verify the image. // The verification result is returned as a VerificationResult and any error encountered. -func (r *OCIRepositoryReconciler) verifySignature(ctx context.Context, obj *ociv1.OCIRepository, ref name.Reference, keychain authn.Keychain, auth authn.Authenticator, opt ...remote.Option) (soci.VerificationResult, error) { +func (r *OCIRepositoryReconciler) verifySignature(ctx context.Context, obj *ociv1.OCIRepository, + ref name.Reference, keychain authn.Keychain, auth authn.Authenticator, + transport *http.Transport, opt ...remote.Option) (soci.VerificationResult, error) { + ctxTimeout, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration) defer cancel() @@ -753,6 +757,7 @@ func (r *OCIRepositoryReconciler) verifySignature(ctx context.Context, obj *ociv notation.WithInsecureRegistry(obj.Spec.Insecure), notation.WithLogger(ctrl.LoggerFrom(ctx)), notation.WithRootCertificates(certs), + notation.WithTransport(transport), } verifier, err := notation.NewNotationVerifier(defaultNotationOciOpts...) @@ -920,16 +925,40 @@ func (r *OCIRepositoryReconciler) keychain(ctx context.Context, obj *ociv1.OCIRe // transport clones the default transport from remote and when a certSecretRef is specified, // the returned transport will include the TLS client and/or CA certificates. +// If the insecure flag is set, the transport will skip the verification of the server's certificate. +// Additionally, if a proxy is specified, transport will use it. func (r *OCIRepositoryReconciler) transport(ctx context.Context, obj *ociv1.OCIRepository) (*http.Transport, error) { transport := remote.DefaultTransport.(*http.Transport).Clone() + tlsConfig, err := r.getTLSConfig(ctx, obj) + if err != nil { + return nil, err + } + if tlsConfig != nil { + transport.TLSClientConfig = tlsConfig + } + + proxyURL, err := r.getProxyURL(ctx, obj) + if err != nil { + return nil, err + } + if proxyURL != nil { + transport.Proxy = http.ProxyURL(proxyURL) + } + + return transport, nil +} + +// getTLSConfig gets the TLS configuration for the transport based on the +// specified secret reference in the OCIRepository object, or the insecure flag. +func (r *OCIRepositoryReconciler) getTLSConfig(ctx context.Context, obj *ociv1.OCIRepository) (*cryptotls.Config, error) { if obj.Spec.CertSecretRef == nil || obj.Spec.CertSecretRef.Name == "" { if obj.Spec.Insecure { - transport.TLSClientConfig = &cryptotls.Config{ + return &cryptotls.Config{ InsecureSkipVerify: true, - } + }, nil } - return transport, nil + return nil, nil } certSecretName := types.NamespacedName{ @@ -955,9 +984,42 @@ func (r *OCIRepositoryReconciler) transport(ctx context.Context, obj *ociv1.OCIR Info("warning: specifying TLS auth data via `certFile`/`keyFile`/`caFile` is deprecated, please use `tls.crt`/`tls.key`/`ca.crt` instead") } } - transport.TLSClientConfig = tlsConfig - return transport, nil + return tlsConfig, nil +} + +// getProxyURL gets the proxy configuration for the transport based on the +// specified proxy secret reference in the OCIRepository object. +func (r *OCIRepositoryReconciler) getProxyURL(ctx context.Context, obj *ociv1.OCIRepository) (*url.URL, error) { + if obj.Spec.ProxySecretRef == nil || obj.Spec.ProxySecretRef.Name == "" { + return nil, nil + } + + proxySecretName := types.NamespacedName{ + Namespace: obj.Namespace, + Name: obj.Spec.ProxySecretRef.Name, + } + var proxySecret corev1.Secret + if err := r.Get(ctx, proxySecretName, &proxySecret); err != 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.Namespace, obj.Spec.ProxySecretRef.Name) + } + 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 } // reconcileStorage ensures the current state of the storage matches the diff --git a/internal/controller/ocirepository_controller_test.go b/internal/controller/ocirepository_controller_test.go index 0e9f89885..d07917ce1 100644 --- a/internal/controller/ocirepository_controller_test.go +++ b/internal/controller/ocirepository_controller_test.go @@ -73,6 +73,7 @@ import ( serror "github.com/fluxcd/source-controller/internal/error" snotation "github.com/fluxcd/source-controller/internal/oci/notation" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" + testproxy "github.com/fluxcd/source-controller/tests/proxy" ) func TestOCIRepositoryReconciler_deleteBeforeFinalizer(t *testing.T) { @@ -963,7 +964,133 @@ func TestOCIRepository_CertSecret(t *testing.T) { return len(resultobj.Finalizers) > 0 }, timeout).Should(BeTrue()) - // Wait for the object to fail + // Wait for the object to be ready + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, &resultobj); err != nil { + return false + } + readyCondition := conditions.Get(&resultobj, meta.ReadyCondition) + if readyCondition == nil || conditions.IsUnknown(&resultobj, meta.ReadyCondition) { + return false + } + return obj.Generation == readyCondition.ObservedGeneration && + conditions.IsReady(&resultobj) == tt.expectreadyconition + }, timeout).Should(BeTrue()) + + tt.expectedstatusmessage = strings.ReplaceAll(tt.expectedstatusmessage, "", pi.url) + + readyCondition := conditions.Get(&resultobj, meta.ReadyCondition) + g.Expect(readyCondition.Message).Should(ContainSubstring(tt.expectedstatusmessage)) + + // Wait for the object to be deleted + g.Expect(testEnv.Delete(ctx, &resultobj)).To(Succeed()) + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, &resultobj); err != nil { + return apierrors.IsNotFound(err) + } + return false + }, timeout).Should(BeTrue()) + }) + } +} + +func TestOCIRepository_ProxySecret(t *testing.T) { + g := NewWithT(t) + + tmpDir := t.TempDir() + regServer, err := setupRegistryServer(ctx, tmpDir, registryOptions{}) + g.Expect(err).ToNot(HaveOccurred()) + t.Cleanup(func() { + regServer.Close() + }) + + pi, err := createPodinfoImageFromTar("podinfo-6.1.5.tar", "6.1.5", regServer.registryHost) + g.Expect(err).NotTo(HaveOccurred()) + + proxyAddr, proxyPort := testproxy.New(t) + + tests := []struct { + name string + url string + digest gcrv1.Hash + proxySecret *corev1.Secret + expectreadyconition bool + expectedstatusmessage string + }{ + { + name: "test proxied connection", + url: pi.url, + digest: pi.digest, + proxySecret: &corev1.Secret{ + Data: map[string][]byte{ + "address": []byte(fmt.Sprintf("http://%s", proxyAddr)), + }, + }, + expectreadyconition: true, + expectedstatusmessage: fmt.Sprintf("stored artifact for digest '%s'", pi.digest.String()), + }, + { + name: "test proxy connection error", + url: pi.url, + digest: pi.digest, + proxySecret: &corev1.Secret{ + Data: map[string][]byte{ + "address": []byte(fmt.Sprintf("http://localhost:%d", proxyPort+1)), + }, + }, + expectreadyconition: false, + expectedstatusmessage: "failed to pull artifact", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + ns, err := testEnv.CreateNamespace(ctx, "ocirepository-test") + g.Expect(err).ToNot(HaveOccurred()) + defer func() { g.Expect(testEnv.Delete(ctx, ns)).To(Succeed()) }() + + obj := &ociv1.OCIRepository{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "ocirepository-test-resource", + Namespace: ns.Name, + Generation: 1, + }, + Spec: ociv1.OCIRepositorySpec{ + URL: tt.url, + Interval: metav1.Duration{Duration: 60 * time.Minute}, + Reference: &ociv1.OCIRepositoryRef{Digest: tt.digest.String()}, + }, + } + + if tt.proxySecret != nil { + tt.proxySecret.ObjectMeta = metav1.ObjectMeta{ + GenerateName: "proxy-secretref", + Namespace: ns.Name, + } + + g.Expect(testEnv.CreateAndWait(ctx, tt.proxySecret)).To(Succeed()) + defer func() { g.Expect(testEnv.Delete(ctx, tt.proxySecret)).To(Succeed()) }() + + obj.Spec.ProxySecretRef = &meta.LocalObjectReference{Name: tt.proxySecret.Name} + } + + g.Expect(testEnv.Create(ctx, obj)).To(Succeed()) + + key := client.ObjectKey{Name: obj.Name, Namespace: obj.Namespace} + + resultobj := ociv1.OCIRepository{} + + // Wait for the finalizer to be set + g.Eventually(func() bool { + if err := testEnv.Get(ctx, key, &resultobj); err != nil { + return false + } + return len(resultobj.Finalizers) > 0 + }, timeout).Should(BeTrue()) + + // Wait for the object to be ready g.Eventually(func() bool { if err := testEnv.Get(ctx, key, &resultobj); err != nil { return false @@ -3511,3 +3638,188 @@ func TestOCIContentConfigChanged(t *testing.T) { }) } } + +func TestOCIRepositoryReconciler_getProxyURL(t *testing.T) { + tests := []struct { + name string + ociRepo *ociv1.OCIRepository + objects []client.Object + expectedURL string + expectedErr string + }{ + { + name: "empty proxySecretRef", + ociRepo: &ociv1.OCIRepository{ + Spec: ociv1.OCIRepositorySpec{ + ProxySecretRef: nil, + }, + }, + }, + { + name: "non-existing proxySecretRef", + ociRepo: &ociv1.OCIRepository{ + Spec: ociv1.OCIRepositorySpec{ + ProxySecretRef: &meta.LocalObjectReference{ + Name: "non-existing", + }, + }, + }, + expectedErr: "secrets \"non-existing\" not found", + }, + { + name: "missing address in proxySecretRef", + ociRepo: &ociv1.OCIRepository{ + Spec: ociv1.OCIRepositorySpec{ + ProxySecretRef: &meta.LocalObjectReference{ + Name: "dummy", + }, + }, + }, + objects: []client.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dummy", + }, + Data: map[string][]byte{}, + }, + }, + expectedErr: "invalid proxy secret '/dummy': key 'address' is missing", + }, + { + name: "invalid address in proxySecretRef", + ociRepo: &ociv1.OCIRepository{ + Spec: ociv1.OCIRepositorySpec{ + ProxySecretRef: &meta.LocalObjectReference{ + Name: "dummy", + }, + }, + }, + objects: []client.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dummy", + }, + Data: map[string][]byte{ + "address": {0x7f}, + }, + }, + }, + expectedErr: "failed to parse proxy address '\x7f': parse \"\\x7f\": net/url: invalid control character in URL", + }, + { + name: "no user, no password", + ociRepo: &ociv1.OCIRepository{ + Spec: ociv1.OCIRepositorySpec{ + ProxySecretRef: &meta.LocalObjectReference{ + Name: "dummy", + }, + }, + }, + objects: []client.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dummy", + }, + Data: map[string][]byte{ + "address": []byte("http://proxy.example.com"), + }, + }, + }, + expectedURL: "http://proxy.example.com", + }, + { + name: "user, no password", + ociRepo: &ociv1.OCIRepository{ + Spec: ociv1.OCIRepositorySpec{ + ProxySecretRef: &meta.LocalObjectReference{ + Name: "dummy", + }, + }, + }, + objects: []client.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dummy", + }, + Data: map[string][]byte{ + "address": []byte("http://proxy.example.com"), + "username": []byte("user"), + }, + }, + }, + expectedURL: "http://user:@proxy.example.com", + }, + { + name: "no user, password", + ociRepo: &ociv1.OCIRepository{ + Spec: ociv1.OCIRepositorySpec{ + ProxySecretRef: &meta.LocalObjectReference{ + Name: "dummy", + }, + }, + }, + objects: []client.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dummy", + }, + Data: map[string][]byte{ + "address": []byte("http://proxy.example.com"), + "password": []byte("password"), + }, + }, + }, + expectedURL: "http://:password@proxy.example.com", + }, + { + name: "user, password", + ociRepo: &ociv1.OCIRepository{ + Spec: ociv1.OCIRepositorySpec{ + ProxySecretRef: &meta.LocalObjectReference{ + Name: "dummy", + }, + }, + }, + objects: []client.Object{ + &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dummy", + }, + Data: map[string][]byte{ + "address": []byte("http://proxy.example.com"), + "username": []byte("user"), + "password": []byte("password"), + }, + }, + }, + expectedURL: "http://user:password@proxy.example.com", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + c := fakeclient.NewClientBuilder(). + WithScheme(testEnv.Scheme()). + WithObjects(tt.objects...). + Build() + + r := &OCIRepositoryReconciler{ + Client: c, + } + + u, err := r.getProxyURL(ctx, tt.ociRepo) + if tt.expectedErr == "" { + g.Expect(err).To(BeNil()) + } else { + g.Expect(err.Error()).To(ContainSubstring(tt.expectedErr)) + } + if tt.expectedURL == "" { + g.Expect(u).To(BeNil()) + } else { + g.Expect(u.String()).To(Equal(tt.expectedURL)) + } + }) + } +} diff --git a/internal/oci/cosign/cosign_test.go b/internal/oci/cosign/cosign_test.go index 17af9523f..f99e7d1f6 100644 --- a/internal/oci/cosign/cosign_test.go +++ b/internal/oci/cosign/cosign_test.go @@ -17,13 +17,21 @@ limitations under the License. package cosign import ( + "context" + "fmt" "net/http" + "net/url" "reflect" "testing" "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/remote" + . "github.com/onsi/gomega" "github.com/sigstore/cosign/v2/pkg/cosign" + + testproxy "github.com/fluxcd/source-controller/tests/proxy" + testregistry "github.com/fluxcd/source-controller/tests/registry" ) func TestOptions(t *testing.T) { @@ -128,3 +136,58 @@ func TestOptions(t *testing.T) { }) } } + +func TestPrivateKeyVerificationWithProxy(t *testing.T) { + g := NewWithT(t) + + registryAddr := testregistry.New(t) + + tagURL := fmt.Sprintf("%s/fluxcd/source-controller:v1.3.0", registryAddr) + ref, err := name.ParseReference(tagURL) + g.Expect(err).NotTo(HaveOccurred()) + + proxyAddr, proxyPort := testproxy.New(t) + + keys, err := cosign.GenerateKeyPair(func(b bool) ([]byte, error) { + return []byte("cosign-password"), nil + }) + g.Expect(err).NotTo(HaveOccurred()) + + tests := []struct { + name string + proxyURL *url.URL + err string + }{ + { + name: "with correct proxy", + proxyURL: &url.URL{Scheme: "http", Host: proxyAddr}, + err: "image tag not found", + }, + { + name: "with incorrect proxy", + proxyURL: &url.URL{Scheme: "http", Host: fmt.Sprintf("localhost:%d", proxyPort+1)}, + err: "connection refused", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + ctx := context.Background() + + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.Proxy = http.ProxyURL(tt.proxyURL) + + var opts []Options + opts = append(opts, WithRemoteOptions(remote.WithTransport(transport))) + opts = append(opts, WithPublicKey(keys.PublicBytes)) + + verifier, err := NewCosignVerifier(ctx, opts...) + g.Expect(err).NotTo(HaveOccurred()) + + _, err = verifier.Verify(ctx, ref) + g.Expect(err.Error()).To(ContainSubstring(tt.err)) + }) + } +} diff --git a/internal/oci/notation/notation.go b/internal/oci/notation/notation.go index 4ae63fb14..0158ffd03 100644 --- a/internal/oci/notation/notation.go +++ b/internal/oci/notation/notation.go @@ -56,6 +56,7 @@ type options struct { keychain authn.Keychain insecure bool logger logr.Logger + transport *http.Transport } // Options is a function that configures the options applied to a Verifier. @@ -118,14 +119,22 @@ func WithLogger(logger logr.Logger) Options { } } +// WithTransport is a function that returns an Options function to set the transport for the options. +func WithTransport(transport *http.Transport) Options { + return func(o *options) { + o.transport = transport + } +} + // NotationVerifier is a struct which is responsible for executing verification logic type NotationVerifier struct { - auth authn.Authenticator - keychain authn.Keychain - verifier *notation.Verifier - opts []remote.Option - insecure bool - logger logr.Logger + auth authn.Authenticator + keychain authn.Keychain + verifier *notation.Verifier + opts []remote.Option + insecure bool + logger logr.Logger + transport *http.Transport } var _ truststore.X509TrustStore = &trustStore{} @@ -181,12 +190,13 @@ func NewNotationVerifier(opts ...Options) (*NotationVerifier, error) { } return &NotationVerifier{ - auth: o.auth, - keychain: o.keychain, - verifier: &verifier, - opts: o.rOpt, - insecure: o.insecure, - logger: o.logger, + auth: o.auth, + keychain: o.keychain, + verifier: &verifier, + opts: o.rOpt, + insecure: o.insecure, + logger: o.logger, + transport: o.transport, }, nil } @@ -344,8 +354,14 @@ func (v *NotationVerifier) remoteRepo(repoUrl string) (*oras.Repository, error) } } + hc := retryhttp.DefaultClient + if v.transport != nil { + hc = &http.Client{ + Transport: retryhttp.NewTransport(v.transport), + } + } repoClient := &oauth.Client{ - Client: retryhttp.DefaultClient, + Client: hc, Header: http.Header{ "User-Agent": {"flux"}, }, diff --git a/internal/oci/notation/notation_test.go b/internal/oci/notation/notation_test.go index 16054ca06..cdd8a3872 100644 --- a/internal/oci/notation/notation_test.go +++ b/internal/oci/notation/notation_test.go @@ -17,8 +17,11 @@ limitations under the License. package notation import ( + "context" "fmt" "net/http" + "net/url" + "path" "reflect" "testing" @@ -31,6 +34,8 @@ import ( . "github.com/onsi/gomega" "github.com/fluxcd/source-controller/internal/oci" + testproxy "github.com/fluxcd/source-controller/tests/proxy" + testregistry "github.com/fluxcd/source-controller/tests/registry" ) func TestOptions(t *testing.T) { @@ -537,6 +542,61 @@ func TestRepoUrlWithDigest(t *testing.T) { } } +func TestVerificationWithProxy(t *testing.T) { + g := NewWithT(t) + + registryAddr := testregistry.New(t) + + tarFilePath := path.Join("..", "..", "controller", "testdata", "podinfo", "podinfo-6.1.5.tar") + _, err := testregistry.CreatePodinfoImageFromTar(tarFilePath, "6.1.5", registryAddr) + g.Expect(err).NotTo(HaveOccurred()) + + tagURL := fmt.Sprintf("%s/podinfo:6.1.5", registryAddr) + ref, err := name.ParseReference(tagURL) + g.Expect(err).NotTo(HaveOccurred()) + + proxyAddr, proxyPort := testproxy.New(t) + + tests := []struct { + name string + proxyURL *url.URL + err string + }{ + { + name: "with correct proxy", + proxyURL: &url.URL{Scheme: "http", Host: proxyAddr}, + err: "no signature is associated with", + }, + { + name: "with incorrect proxy", + proxyURL: &url.URL{Scheme: "http", Host: fmt.Sprintf("localhost:%d", proxyPort+1)}, + err: "connection refused", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + ctx := context.Background() + + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.Proxy = http.ProxyURL(tt.proxyURL) + + var opts []Options + opts = append(opts, WithTransport(transport)) + opts = append(opts, WithTrustPolicy(dummyPolicyDocument())) + opts = append(opts, WithInsecureRegistry(true)) + + verifier, err := NewNotationVerifier(opts...) + g.Expect(err).NotTo(HaveOccurred()) + + _, err = verifier.Verify(ctx, ref) + g.Expect(err.Error()).To(ContainSubstring(tt.err)) + }) + } +} + func dummyPolicyDocument() (policyDoc *trustpolicy.Document) { policyDoc = &trustpolicy.Document{ Version: "1.0", @@ -548,7 +608,7 @@ func dummyPolicyDocument() (policyDoc *trustpolicy.Document) { func dummyPolicyStatement() (policyStatement trustpolicy.TrustPolicy) { policyStatement = trustpolicy.TrustPolicy{ Name: "test-statement-name", - RegistryScopes: []string{"registry.acme-rockets.io/software/net-monitor"}, + RegistryScopes: []string{"*"}, SignatureVerification: trustpolicy.SignatureVerification{VerificationLevel: "strict"}, TrustStores: []string{"ca:valid-trust-store", "signingAuthority:valid-trust-store"}, TrustedIdentities: []string{"x509.subject:CN=Notation Test Root,O=Notary,L=Seattle,ST=WA,C=US"}, diff --git a/tests/listener/listener.go b/tests/listener/listener.go index f034b61fb..390008d75 100644 --- a/tests/listener/listener.go +++ b/tests/listener/listener.go @@ -32,7 +32,7 @@ import ( func New(t *testing.T) (net.Listener, string, int) { t.Helper() - lis, err := net.Listen("tcp", ":0") + lis, err := net.Listen("tcp", "localhost:0") assert.NilError(t, err) t.Cleanup(func() { lis.Close() }) diff --git a/tests/registry/registry.go b/tests/registry/registry.go new file mode 100644 index 000000000..74ee117c7 --- /dev/null +++ b/tests/registry/registry.go @@ -0,0 +1,123 @@ +/* +Copyright 2024 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package testregistry + +import ( + "context" + "fmt" + "io" + "net/url" + "strings" + "testing" + "time" + + "github.com/distribution/distribution/v3/configuration" + "github.com/distribution/distribution/v3/registry" + _ "github.com/distribution/distribution/v3/registry/storage/driver/inmemory" + "github.com/google/go-containerregistry/pkg/crane" + gcrv1 "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/mutate" + "github.com/sirupsen/logrus" + "gotest.tools/assert" + + "github.com/fluxcd/pkg/oci" + + testlistener "github.com/fluxcd/source-controller/tests/listener" +) + +func New(t *testing.T) string { + t.Helper() + + // Get a free random port and release it so the registry can use it. + listener, addr, _ := testlistener.New(t) + err := listener.Close() + assert.NilError(t, err) + + config := &configuration.Configuration{} + config.HTTP.Addr = addr + config.HTTP.DrainTimeout = time.Duration(10) * time.Second + config.Storage = map[string]configuration.Parameters{"inmemory": map[string]interface{}{}} + config.Log.AccessLog.Disabled = true + config.Log.Level = "error" + logrus.SetOutput(io.Discard) + + r, err := registry.NewRegistry(context.Background(), config) + assert.NilError(t, err) + + go r.ListenAndServe() + + return addr +} + +type PodinfoImage struct { + URL string + Tag string + Digest gcrv1.Hash +} + +func CreatePodinfoImageFromTar(tarFilePath, tag, registryURL string, opts ...crane.Option) (*PodinfoImage, error) { + // Create Image + image, err := crane.Load(tarFilePath) + if err != nil { + return nil, err + } + + image = setPodinfoImageAnnotations(image, tag) + + // url.Parse doesn't handle urls with no scheme well e.g localhost: + if !(strings.HasPrefix(registryURL, "http://") || strings.HasPrefix(registryURL, "https://")) { + registryURL = fmt.Sprintf("http://%s", registryURL) + } + + myURL, err := url.Parse(registryURL) + if err != nil { + return nil, err + } + repositoryURL := fmt.Sprintf("%s/podinfo", myURL.Host) + + // Image digest + podinfoImageDigest, err := image.Digest() + if err != nil { + return nil, err + } + + // Push image + err = crane.Push(image, repositoryURL, opts...) + if err != nil { + return nil, err + } + + // Tag the image + err = crane.Tag(repositoryURL, tag, opts...) + if err != nil { + return nil, err + } + + return &PodinfoImage{ + URL: "oci://" + repositoryURL, + Tag: tag, + Digest: podinfoImageDigest, + }, nil +} + +func setPodinfoImageAnnotations(img gcrv1.Image, tag string) gcrv1.Image { + metadata := map[string]string{ + oci.SourceAnnotation: "https://github.com/stefanprodan/podinfo", + oci.RevisionAnnotation: fmt.Sprintf("%s@sha1:b3b00fe35424a45d373bf4c7214178bc36fd7872", tag), + } + return mutate.Annotations(img, metadata).(gcrv1.Image) +}