From ef270448b7903ee2a8a038bc7af9c4341a3507bb Mon Sep 17 00:00:00 2001 From: Domenico Luciani Date: Wed, 13 Sep 2023 16:18:11 +0200 Subject: [PATCH] Make the getInsecureOptions a static method as a temporary solution to remove duplications Signed-off-by: Domenico Luciani --- cmd/lifecycle/exporter.go | 17 ++--------------- cmd/lifecycle/rebaser.go | 16 +--------------- cmd/lifecycle/restorer.go | 21 ++------------------- image/registry_handler.go | 15 ++++++++++----- image/registry_handler_test.go | 30 ++++-------------------------- image/remote_handler.go | 18 +----------------- 6 files changed, 20 insertions(+), 97 deletions(-) diff --git a/cmd/lifecycle/exporter.go b/cmd/lifecycle/exporter.go index cfc39fd91..9c6ba4212 100644 --- a/cmd/lifecycle/exporter.go +++ b/cmd/lifecycle/exporter.go @@ -5,7 +5,6 @@ import ( "os" "path/filepath" "strconv" - "strings" "time" "github.com/BurntSushi/toml" @@ -342,6 +341,7 @@ func (e *exportCmd) initRemoteAppImage(analyzedMD files.Analyzed) (imgutil.Image var opts = []remote.ImageOption{ remote.FromBaseImage(e.RunImageRef), } + if e.supportsRunImageExtension() { extendedConfig, err := e.getExtendedConfig(analyzedMD.RunImage) if err != nil { @@ -357,7 +357,7 @@ func (e *exportCmd) initRemoteAppImage(analyzedMD files.Analyzed) (imgutil.Image opts = append(opts, remote.WithHistory()) } - opts = append(opts, e.getInsecureOptions(e.RunImageRef)...) + opts = append(opts, image.GetInsecureOptions(e.InsecureRegistries, e.RunImageRef)...) if analyzedMD.PreviousImageRef() != "" { cmd.DefaultLogger.Infof("Reusing layers from image '%s'", analyzedMD.PreviousImageRef()) @@ -524,16 +524,3 @@ func (e *exportCmd) hasExtendedLayers() bool { } return true } - -func (e *exportCmd) getInsecureOptions(imageRef string) []remote.ImageOption { - var opts []remote.ImageOption - if len(e.InsecureRegistries) > 0 { - cmd.DefaultLogger.Warnf("Found Insecure Registries: %+q", e.InsecureRegistries) - for _, insecureRegistry := range e.InsecureRegistries { - if strings.HasPrefix(imageRef, insecureRegistry) { - opts = append(opts, remote.WithRegistrySetting(insecureRegistry, true)) - } - } - } - return opts -} diff --git a/cmd/lifecycle/rebaser.go b/cmd/lifecycle/rebaser.go index 331d5f7dd..a3751d2e3 100644 --- a/cmd/lifecycle/rebaser.go +++ b/cmd/lifecycle/rebaser.go @@ -2,7 +2,6 @@ package main import ( "fmt" - "strings" "github.com/buildpacks/imgutil" "github.com/buildpacks/imgutil/local" @@ -170,7 +169,7 @@ func (r *rebaseCmd) setAppImage() error { remote.FromBaseImage(targetImageRef), } - opts = append(opts, r.getInsecureRegistryOptions(targetImageRef)...) + opts = append(opts, image.GetInsecureOptions(r.InsecureRegistries, targetImageRef)...) r.appImage, err = remote.NewImage( targetImageRef, @@ -210,16 +209,3 @@ func (r *rebaseCmd) setAppImage() error { return nil } - -func (r *rebaseCmd) getInsecureRegistryOptions(imageRef string) []remote.ImageOption { - var opts []remote.ImageOption - if len(r.InsecureRegistries) > 0 { - cmd.DefaultLogger.Warnf("Found Insecure Registries: %+q", r.InsecureRegistries) - for _, insecureRegistry := range r.InsecureRegistries { - if strings.HasPrefix(imageRef, insecureRegistry) { - opts = append(opts, remote.WithRegistrySetting(insecureRegistry, true)) - } - } - } - return opts -} diff --git a/cmd/lifecycle/restorer.go b/cmd/lifecycle/restorer.go index 974ca539e..7f66a25ea 100644 --- a/cmd/lifecycle/restorer.go +++ b/cmd/lifecycle/restorer.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "path/filepath" - "strings" "github.com/buildpacks/imgutil" "github.com/buildpacks/imgutil/layout" @@ -220,11 +219,8 @@ func (r *restoreCmd) pullSparse(imageRef string) (imgutil.Image, error) { return nil, fmt.Errorf("failed to create cache directory: %w", err) } - var opts = []remote.ImageOption{ - remote.FromBaseImage(imageRef), - } - - opts = append(opts, r.getInsecureRegistryOptions(imageRef)...) + var opts []remote.ImageOption + opts = append(opts, append(image.GetInsecureOptions(r.InsecureRegistries, imageRef), remote.FromBaseImage(imageRef))...) // get remote image remoteImage, err := remote.NewImage(imageRef, r.keychain, opts...) @@ -281,16 +277,3 @@ func (r *restoreCmd) restore(layerMetadata files.LayersMetadata, group buildpack } return nil } - -func (r *restoreCmd) getInsecureRegistryOptions(imageRef string) []remote.ImageOption { - var opts []remote.ImageOption - if len(r.InsecureRegistries) > 0 { - cmd.DefaultLogger.Warnf("Found Insecure Registries: %+q", r.InsecureRegistries) - for _, insecureRegistry := range r.InsecureRegistries { - if strings.HasPrefix(imageRef, insecureRegistry) { - opts = append(opts, remote.WithRegistrySetting(insecureRegistry, true)) - } - } - } - return opts -} diff --git a/image/registry_handler.go b/image/registry_handler.go index 7714ccd34..b471c4c16 100644 --- a/image/registry_handler.go +++ b/image/registry_handler.go @@ -35,7 +35,7 @@ func NewRegistryHandler(keychain authn.Keychain, insecureRegistries []string) *D // EnsureReadAccess ensures that we can read from the registry func (rv *DefaultRegistryHandler) EnsureReadAccess(imageRefs ...string) error { for _, imageRef := range imageRefs { - if err := verifyReadAccess(imageRef, rv.keychain, rv.GetInsecureOptions(imageRef)); err != nil { + if err := verifyReadAccess(imageRef, rv.keychain, GetInsecureOptions(rv.insecureRegistry, imageRef)); err != nil { return err } } @@ -45,7 +45,7 @@ func (rv *DefaultRegistryHandler) EnsureReadAccess(imageRefs ...string) error { // EnsureWriteAccess ensures that we can write to the registry func (rv *DefaultRegistryHandler) EnsureWriteAccess(imageRefs ...string) error { for _, imageRef := range imageRefs { - if err := verifyReadWriteAccess(imageRef, rv.keychain, rv.GetInsecureOptions(imageRef)); err != nil { + if err := verifyReadWriteAccess(imageRef, rv.keychain, GetInsecureOptions(rv.insecureRegistry, imageRef)); err != nil { return err } } @@ -53,10 +53,15 @@ func (rv *DefaultRegistryHandler) EnsureWriteAccess(imageRefs ...string) error { } // GetInsecureOptions returns a list of WithRegistrySetting imageOptions matching the specified imageRef prefix -func (rv *DefaultRegistryHandler) GetInsecureOptions(imageRef string) []remote.ImageOption { +/* +TODO: This is a temporary solution in order to get insecure registries in other components too +TODO: Ideally we should fix the `imgutil.options` struct visibility in order to mock and test the `remote.WithRegistrySetting` +TODO: function correctly and use the RegistryHandler everywhere it is needed. +*/ +func GetInsecureOptions(insecureRegistries []string, imageRef string) []remote.ImageOption { var opts []remote.ImageOption - if len(rv.insecureRegistry) > 0 { - for _, insecureRegistry := range rv.insecureRegistry { + if len(insecureRegistries) > 0 { + for _, insecureRegistry := range insecureRegistries { if strings.HasPrefix(imageRef, insecureRegistry) { opts = append(opts, remote.WithRegistrySetting(insecureRegistry, true)) } diff --git a/image/registry_handler_test.go b/image/registry_handler_test.go index 89b13e21f..62b1b4c92 100644 --- a/image/registry_handler_test.go +++ b/image/registry_handler_test.go @@ -3,10 +3,6 @@ package image import ( "testing" - "github.com/golang/mock/gomock" - - testmockauth "github.com/buildpacks/lifecycle/testmock/auth" - "github.com/sclevine/spec" "github.com/sclevine/spec/report" @@ -18,44 +14,26 @@ func TestRegistryHandler(t *testing.T) { } func testRegistryHandler(t *testing.T, when spec.G, it spec.S) { when("insecure registry", func() { - var ( - mockController *gomock.Controller - mockKeychain *testmockauth.MockKeychain - ) - - it.Before(func() { - mockController = gomock.NewController(t) - mockKeychain = testmockauth.NewMockKeychain(mockController) - }) - it("returns WithRegistrySetting options for the domains specified", func() { - registryHandler := NewRegistryHandler(mockKeychain, []string{"host.docker.internal"}) - - registryOptions := registryHandler.GetInsecureOptions("host.docker.internal/bar") + registryOptions := GetInsecureOptions([]string{"host.docker.internal"}, "host.docker.internal/bar") h.AssertEq(t, len(registryOptions), 1) }) it("returns WithRegistrySetting options only for the domains specified", func() { - registryHandler := NewRegistryHandler(mockKeychain, []string{"host.docker.internal", "this.is.just.a.try"}) - - registryOptions := registryHandler.GetInsecureOptions("host.docker.internal/bar") + registryOptions := GetInsecureOptions([]string{"host.docker.internal", "this.is.just.a.try"}, "host.docker.internal/bar") h.AssertEq(t, len(registryOptions), 1) }) it("returns empty options if any domain hasn't been specified and the imageRef is empty", func() { - registryHandler := NewRegistryHandler(mockKeychain, nil) - - options := registryHandler.GetInsecureOptions("") + options := GetInsecureOptions(nil, "") h.AssertEq(t, len(options), 0) }) it("returns empty options if an empty list of insecure registries has been passed but the imageRef has been passed anyway", func() { - registryHandler := NewRegistryHandler(mockKeychain, []string{}) - - options := registryHandler.GetInsecureOptions("host.docker.container") + options := GetInsecureOptions([]string{}, "host.docker.container") h.AssertEq(t, len(options), 0) }) diff --git a/image/remote_handler.go b/image/remote_handler.go index ac256d623..3561235ab 100644 --- a/image/remote_handler.go +++ b/image/remote_handler.go @@ -1,10 +1,6 @@ package image import ( - "strings" - - "github.com/buildpacks/lifecycle/cmd" - "github.com/buildpacks/imgutil" "github.com/buildpacks/imgutil/remote" "github.com/google/go-containerregistry/pkg/authn" @@ -26,7 +22,7 @@ func (h *RemoteHandler) InitImage(imageRef string) (imgutil.Image, error) { remote.FromBaseImage(imageRef), } - options = append(options, h.getInsecureRegistryOptions(imageRef)...) + options = append(options, GetInsecureOptions(h.insecureRegistries, imageRef)...) return remote.NewImage( imageRef, @@ -38,15 +34,3 @@ func (h *RemoteHandler) InitImage(imageRef string) (imgutil.Image, error) { func (h *RemoteHandler) Kind() string { return RemoteKind } -func (h *RemoteHandler) getInsecureRegistryOptions(imageRef string) []remote.ImageOption { - var opts []remote.ImageOption - if len(h.insecureRegistries) > 0 { - cmd.DefaultLogger.Warnf("Found Insecure Registries: %+q", h.insecureRegistries) - for _, insecureRegistry := range h.insecureRegistries { - if strings.HasPrefix(imageRef, insecureRegistry) { - opts = append(opts, remote.WithRegistrySetting(insecureRegistry, true)) - } - } - } - return opts -}