Skip to content

Commit

Permalink
Make the getInsecureOptions a static method as a temporary solution t…
Browse files Browse the repository at this point in the history
…o remove duplications

Signed-off-by: Domenico Luciani <[email protected]>
  • Loading branch information
Domenico Luciani committed Sep 13, 2023
1 parent c62c7c2 commit 2457621
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 97 deletions.
17 changes: 2 additions & 15 deletions cmd/lifecycle/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"os"
"path/filepath"
"strconv"
"strings"
"time"

"github.com/BurntSushi/toml"
Expand Down Expand Up @@ -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 {
Expand All @@ -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())
Expand Down Expand Up @@ -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
}
16 changes: 1 addition & 15 deletions cmd/lifecycle/rebaser.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package main

import (
"fmt"
"strings"

"github.com/buildpacks/imgutil"
"github.com/buildpacks/imgutil/local"
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
}
21 changes: 2 additions & 19 deletions cmd/lifecycle/restorer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"os"
"path/filepath"
"strings"

"github.com/buildpacks/imgutil"
"github.com/buildpacks/imgutil/layout"
Expand Down Expand Up @@ -227,11 +226,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...)
Expand Down Expand Up @@ -292,16 +288,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
}
15 changes: 10 additions & 5 deletions image/registry_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand All @@ -45,18 +45,23 @@ 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
}
}
return nil
}

// 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))
}
Expand Down
30 changes: 4 additions & 26 deletions image/registry_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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)
})
Expand Down
18 changes: 1 addition & 17 deletions image/remote_handler.go
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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,
Expand All @@ -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
}

0 comments on commit 2457621

Please sign in to comment.