From a4f5fb4bcb5fdd4fbec551d1ff1728f760de0fb8 Mon Sep 17 00:00:00 2001 From: Soule BA Date: Thu, 29 Sep 2022 13:03:27 +0200 Subject: [PATCH] refactor makeOptions Reduce complexity by replacing the functional options with a flat out conditional logic in makeOptions. Signed-off-by: Soule BA --- controllers/ocirepository_controller.go | 45 +++++++++---------------- 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/controllers/ocirepository_controller.go b/controllers/ocirepository_controller.go index 7185b209f..3d99582d2 100644 --- a/controllers/ocirepository_controller.go +++ b/controllers/ocirepository_controller.go @@ -347,7 +347,7 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, obj *sour return sreconcile.ResultEmpty, e } - opts := makeOptions(ctx, obj, withTransport(transport), withKeychainOrAuth(keychain, auth)) + opts := makeOptions(ctx, obj, keychain, auth, transport) // Determine which artifact revision to pull url, err := r.getArtifactURL(obj, opts.craneOpts) @@ -1174,14 +1174,26 @@ func craneOptions(ctx context.Context, insecure bool) []crane.Option { return options } -func makeOptions(ctxTimeout context.Context, obj *sourcev1.OCIRepository, opts ...Option) remoteOptions { +func makeOptions(ctxTimeout context.Context, obj *sourcev1.OCIRepository, keychain authn.Keychain, auth authn.Authenticator, + transport http.RoundTripper) remoteOptions { o := remoteOptions{ craneOpts: craneOptions(ctxTimeout, obj.Spec.Insecure), verifyOpts: []remote.Option{}, } - for _, opt := range opts { - opt(&o) + if auth != nil { + // auth take precedence over keychain here as we expect the caller to set + // the auth only if it is required. + o.verifyOpts = append(o.verifyOpts, remote.WithAuth(auth)) + o.craneOpts = append(o.craneOpts, crane.WithAuth(auth)) + } else { + o.verifyOpts = append(o.verifyOpts, remote.WithAuthFromKeychain(keychain)) + o.craneOpts = append(o.craneOpts, crane.WithAuthFromKeychain(keychain)) + } + + if transport != nil { + o.craneOpts = append(o.craneOpts, crane.WithTransport(transport)) + o.verifyOpts = append(o.verifyOpts, remote.WithTransport(transport)) } return o @@ -1191,28 +1203,3 @@ type remoteOptions struct { craneOpts []crane.Option verifyOpts []remote.Option } - -type Option func(*remoteOptions) - -func withKeychainOrAuth(keychain authn.Keychain, auth authn.Authenticator) Option { - return func(o *remoteOptions) { - if auth != nil { - // auth take precedence over keychain here as we expect the caller to set - // the auth only if it is required. - o.verifyOpts = append(o.verifyOpts, remote.WithAuth(auth)) - o.craneOpts = append(o.craneOpts, crane.WithAuth(auth)) - } else { - o.verifyOpts = append(o.verifyOpts, remote.WithAuthFromKeychain(keychain)) - o.craneOpts = append(o.craneOpts, crane.WithAuthFromKeychain(keychain)) - } - } -} - -func withTransport(transport http.RoundTripper) Option { - return func(o *remoteOptions) { - if transport != nil { - o.craneOpts = append(o.craneOpts, crane.WithTransport(transport)) - o.verifyOpts = append(o.verifyOpts, remote.WithTransport(transport)) - } - } -}