Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Insecure Registries #2077

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions internal/build/lifecycle_execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,12 @@
flags = append(flags, "-uid", strconv.Itoa(l.opts.UID))
}

if l.platformAPI.AtLeast("0.13") {
for _, reg := range l.opts.InsecureRegistries {
flags = append(flags, "-insecure-registry", reg)
}

Check warning on line 334 in internal/build/lifecycle_execution.go

View check run for this annotation

Codecov / codecov/patch

internal/build/lifecycle_execution.go#L332-L334

Added lines #L332 - L334 were not covered by tests
}

if l.opts.PreviousImage != "" {
if l.opts.Image == nil {
return errors.New("image can't be nil")
Expand Down Expand Up @@ -481,6 +487,12 @@
flags = append(flags, "-uid", strconv.Itoa(l.opts.UID))
}

if l.platformAPI.AtLeast("0.13") {
for _, reg := range l.opts.InsecureRegistries {
flags = append(flags, "-insecure-registry", reg)
}

Check warning on line 493 in internal/build/lifecycle_execution.go

View check run for this annotation

Codecov / codecov/patch

internal/build/lifecycle_execution.go#L491-L493

Added lines #L491 - L493 were not covered by tests
}

// for kaniko
kanikoCacheBindOp := NullOp()
if (l.platformAPI.AtLeast("0.10") && l.hasExtensionsForBuild()) ||
Expand Down Expand Up @@ -586,6 +598,12 @@
flags = append(flags, "-uid", strconv.Itoa(l.opts.UID))
}

if l.platformAPI.AtLeast("0.13") {
for _, reg := range l.opts.InsecureRegistries {
flags = append(flags, "-insecure-registry", reg)
}

Check warning on line 604 in internal/build/lifecycle_execution.go

View check run for this annotation

Codecov / codecov/patch

internal/build/lifecycle_execution.go#L602-L604

Added lines #L602 - L604 were not covered by tests
}

if l.opts.PreviousImage != "" {
if l.opts.Image == nil {
return errors.New("image can't be nil")
Expand Down Expand Up @@ -795,6 +813,12 @@
flags = append(flags, "-uid", strconv.Itoa(l.opts.UID))
}

if l.platformAPI.AtLeast("0.13") {
for _, reg := range l.opts.InsecureRegistries {
flags = append(flags, "-insecure-registry", reg)
}

Check warning on line 819 in internal/build/lifecycle_execution.go

View check run for this annotation

Codecov / codecov/patch

internal/build/lifecycle_execution.go#L817-L819

Added lines #L817 - L819 were not covered by tests
}

cacheBindOp := NullOp()
switch buildCache.Type() {
case cache.Image:
Expand Down
1 change: 1 addition & 0 deletions internal/build/lifecycle_executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ type LifecycleOptions struct {
SBOMDestinationDir string
CreationTime *time.Time
Keychain authn.Keychain
InsecureRegistries []string
}

func NewLifecycleExecutor(logger logging.Logger, docker DockerClient) *LifecycleExecutor {
Expand Down
3 changes: 3 additions & 0 deletions internal/commands/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ type BuildFlags struct {
DateTime string
PreBuildpacks []string
PostBuildpacks []string
InsecureRegistries []string
}

var macAddressRegex = regexp.MustCompile(`^([0-9A-Fa-f]{2}[:-]){5}([0-9A-Fa-f]{2})$`)
Expand Down Expand Up @@ -203,6 +204,7 @@ func Build(logger logging.Logger, cfg config.Config, packClient PackClient) *cob
PreviousInputImage: inputPreviousImage,
LayoutRepoDir: cfg.LayoutRepositoryDir,
},
InsecureRegistries: flags.InsecureRegistries,
}); err != nil {
return errors.Wrap(err, "failed to build")
}
Expand Down Expand Up @@ -236,6 +238,7 @@ func buildCommandFlags(cmd *cobra.Command, buildFlags *BuildFlags, cfg config.Co
cmd.Flags().StringVarP(&buildFlags.AppPath, "path", "p", "", "Path to app dir or zip-formatted file (defaults to current working directory)")
cmd.Flags().StringSliceVarP(&buildFlags.Buildpacks, "buildpack", "b", nil, "Buildpack to use. One of:\n a buildpack by id and version in the form of '<buildpack>@<version>',\n path to a buildpack directory (not supported on Windows),\n path/URL to a buildpack .tar or .tgz file, or\n a packaged buildpack image name in the form of '<hostname>/<repo>[:<tag>]'"+stringSliceHelp("buildpack"))
cmd.Flags().StringSliceVarP(&buildFlags.Extensions, "extension", "", nil, "Extension to use. One of:\n an extension by id and version in the form of '<extension>@<version>',\n path to an extension directory (not supported on Windows),\n path/URL to an extension .tar or .tgz file, or\n a packaged extension image name in the form of '<hostname>/<repo>[:<tag>]'"+stringSliceHelp("extension"))
cmd.Flags().StringSliceVarP(&buildFlags.InsecureRegistries, "insecure-registry", "", nil, "List of insecure registries")
cmd.Flags().StringVarP(&buildFlags.Builder, "builder", "B", cfg.DefaultBuilder, "Builder image")
cmd.Flags().Var(&buildFlags.Cache, "cache",
`Cache options used to define cache techniques for build process.
Expand Down
2 changes: 1 addition & 1 deletion internal/commands/rebase.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func Rebase(logger logging.Logger, cfg config.Config, pack PackClient) *cobra.Co
cmd.Flags().StringVar(&opts.PreviousImage, "previous-image", "", "Image to rebase. Set to a particular tag reference, digest reference, or (when performing a daemon build) image ID. Use this flag in combination with <image-name> to avoid replacing the original image.")
cmd.Flags().StringVar(&opts.ReportDestinationDir, "report-output-dir", "", "Path to export build report.toml.\nOmitting the flag yield no report file.")
cmd.Flags().BoolVar(&opts.Force, "force", false, "Perform rebase operation without target validation (only available for API >= 0.12)")

cmd.Flags().StringSliceVarP(&opts.InsecureRegistries, "insecure-registry", "", nil, "List of insecure registries")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, I will add something similar to the --force flag.

cmd.Flags().StringSliceVarP(&opts.InsecureRegistries, "insecure-registry", "", nil, "List of insecure registries (only available for API >= 0.13)")

AddHelpFlag(cmd, "rebase")
return cmd
}
3 changes: 3 additions & 0 deletions pkg/client/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ type BuildOptions struct {

// Configuration to export to OCI layout format
LayoutConfig *LayoutConfig

InsecureRegistries []string
}

func (b *BuildOptions) Layout() bool {
Expand Down Expand Up @@ -555,6 +557,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error {
CreationTime: opts.CreationTime,
Layout: opts.Layout(),
Keychain: c.keychain,
InsecureRegistries: opts.InsecureRegistries,
}

switch {
Expand Down
16 changes: 12 additions & 4 deletions pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,10 @@
lifecycleExecutor LifecycleExecutor
buildpackDownloader BuildpackDownloader

experimental bool
registryMirrors map[string]string
version string
experimental bool
registryMirrors map[string]string
version string
insecureRegistries []string
jjbustamante marked this conversation as resolved.
Show resolved Hide resolved
}

// Option is a type of function that mutate settings on the client.
Expand Down Expand Up @@ -187,6 +188,13 @@
}
}

// WithInsecureRegistries sets insecure registry to pull images from.
func WithInsecureRegistries(insecureRegistries []string) Option {
return func(c *Client) {
c.insecureRegistries = insecureRegistries
}

Check warning on line 195 in pkg/client/client.go

View check run for this annotation

Codecov / codecov/patch

pkg/client/client.go#L192-L195

Added lines #L192 - L195 were not covered by tests
}

// WithKeychain sets keychain of credentials to image registries
func WithKeychain(keychain authn.Keychain) Option {
return func(c *Client) {
Expand Down Expand Up @@ -231,7 +239,7 @@
}

if client.imageFetcher == nil {
client.imageFetcher = image.NewFetcher(client.logger, client.docker, image.WithRegistryMirrors(client.registryMirrors), image.WithKeychain(client.keychain))
client.imageFetcher = image.NewFetcher(client.logger, client.docker, image.WithRegistryMirrors(client.registryMirrors), image.WithKeychain(client.keychain), image.WithInsecureRegistries(client.insecureRegistries))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is a misunderstood with this code, for what I can see, client.insecureRegistries is always going to be empty. because this option is never called when we initialized the client.

Usually, the client is being initialized with configuration we save in the pack configuration file, see pack config command. In this case, the insecure-registries are configured at runtime when the user invokes the pack build or pack rebase commands, but at that point in time, the client was already initialized and the image fetcher was already created.

I think we need a way to passthrough the insecure-registries to the Fetch method at runtime

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prashantrewar Do you have a chance to take a look at this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I apologize @jjbustamante, right now I am busy with something, on the weekend I will try to fix it.

}

if client.imageFactory == nil {
Expand Down
7 changes: 7 additions & 0 deletions pkg/client/rebase.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,16 @@
// validated (will not have any effect if API < 0.12).
Force bool

InsecureRegistries []string

// Image reference to use as the previous image for rebase.
PreviousImage string
}

// Rebase updates the run image layers in an app image.
// This operation mutates the image specified in opts.
func (c *Client) Rebase(ctx context.Context, opts RebaseOptions) error {
var flags = []string{"rebase"}
imageRef, err := c.parseTagReference(opts.RepoName)
if err != nil {
return errors.Wrapf(err, "invalid image name '%s'", opts.RepoName)
Expand Down Expand Up @@ -121,6 +124,10 @@
return err
}

for _, reg := range opts.InsecureRegistries {
flags = append(flags, "-insecure-registry", reg)
}

Check warning on line 129 in pkg/client/rebase.go

View check run for this annotation

Codecov / codecov/patch

pkg/client/rebase.go#L128-L129

Added lines #L128 - L129 were not covered by tests

c.logger.Infof("Rebasing %s on run image %s", style.Symbol(appImage.Name()), style.Symbol(baseImage.Name()))
rebaser := &phase.Rebaser{Logger: c.logger, PlatformAPI: build.SupportedPlatformAPIVersions.Latest(), Force: opts.Force}
report, err := rebaser.Rebase(appImage, baseImage, opts.RepoName, nil)
Expand Down
36 changes: 27 additions & 9 deletions pkg/image/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@
}
}

// WithInsecureRegistries supply your own insecure registries.
func WithInsecureRegistries(insecureRegistries []string) FetcherOption {
return func(c *Fetcher) {
c.insecureRegistries = insecureRegistries
}

Check warning on line 49 in pkg/image/fetcher.go

View check run for this annotation

Codecov / codecov/patch

pkg/image/fetcher.go#L46-L49

Added lines #L46 - L49 were not covered by tests
}

func WithKeychain(keychain authn.Keychain) FetcherOption {
return func(c *Fetcher) {
c.keychain = keychain
Expand All @@ -54,17 +61,19 @@
}

type Fetcher struct {
docker DockerClient
logger logging.Logger
registryMirrors map[string]string
keychain authn.Keychain
docker DockerClient
logger logging.Logger
registryMirrors map[string]string
keychain authn.Keychain
insecureRegistries []string
}

type FetchOptions struct {
Daemon bool
Platform string
PullPolicy PullPolicy
LayoutOption LayoutOption
Daemon bool
Platform string
PullPolicy PullPolicy
LayoutOption LayoutOption
InsecureRegistries []string
}

func NewFetcher(logger logging.Logger, docker DockerClient, opts ...FetcherOption) *Fetcher {
Expand Down Expand Up @@ -137,7 +146,16 @@
}

func (f *Fetcher) fetchRemoteImage(name string) (imgutil.Image, error) {
image, err := remote.NewImage(name, f.keychain, remote.FromBaseImage(name))
var options []remote.ImageOption

if len(f.insecureRegistries) > 0 {
for _, registry := range f.insecureRegistries {
options = append(options, remote.WithRegistrySetting(registry, true))
}

Check warning on line 154 in pkg/image/fetcher.go

View check run for this annotation

Codecov / codecov/patch

pkg/image/fetcher.go#L152-L154

Added lines #L152 - L154 were not covered by tests
}

image, err := remote.NewImage(name, f.keychain, append(options, remote.FromBaseImage(name))...)

if err != nil {
return nil, err
}
Expand Down
Loading