Skip to content
This repository has been archived by the owner on Jun 13, 2021. It is now read-only.

Commit

Permalink
Remove --insecure-registries flag
Browse files Browse the repository at this point in the history
The `--insecure-registries` flag is removed to have a consistent UX with
Docker CLI.

Instead of this flag, the list of insecure registries is retrieved from
the engine. See https://docs.docker.com/registry/insecure/ to configure
your engine to allow communication to an insecure registry.

Signed-off-by: Yves Brissaud <yves.brissaud@docker.com>
  • Loading branch information
eunomie committed Sep 26, 2019
1 parent 5d97b59 commit 56ce605
Show file tree
Hide file tree
Showing 9 changed files with 48 additions and 46 deletions.
26 changes: 13 additions & 13 deletions e2e/pushpull_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func TestPushArchs(t *testing.T) {
t.Run(testCase.name, func(t *testing.T) {
cmd := info.configuredCmd
ref := info.registryAddress + "/test/push-pull:1"
args := []string{"app", "push", "--tag", ref, "--insecure-registries=" + info.registryAddress}
args := []string{"app", "push", "--tag", ref}
args = append(args, testCase.args...)
args = append(args, filepath.Join("testdata", "push-pull", "push-pull.dockerapp"))
cmd.Command = dockerCli.Command(args...)
Expand Down Expand Up @@ -191,10 +191,10 @@ func TestPushInstall(t *testing.T) {
runWithDindSwarmAndRegistry(t, func(info dindSwarmAndRegistryInfo) {
cmd := info.configuredCmd
ref := info.registryAddress + "/test/push-pull"
cmd.Command = dockerCli.Command("app", "push", "--tag", ref, "--insecure-registries="+info.registryAddress, filepath.Join("testdata", "push-pull", "push-pull.dockerapp"))
cmd.Command = dockerCli.Command("app", "push", "--tag", ref, filepath.Join("testdata", "push-pull", "push-pull.dockerapp"))
icmd.RunCmd(cmd).Assert(t, icmd.Success)

cmd.Command = dockerCli.Command("app", "install", "--insecure-registries="+info.registryAddress, ref, "--name", t.Name())
cmd.Command = dockerCli.Command("app", "install", ref, "--name", t.Name())
icmd.RunCmd(cmd).Assert(t, icmd.Success)
cmd.Command = dockerCli.Command("service", "ls")
assert.Check(t, cmp.Contains(icmd.RunCmd(cmd).Assert(t, icmd.Success).Combined(), ref))
Expand All @@ -206,16 +206,16 @@ func TestPushPullInstall(t *testing.T) {
cmd := info.configuredCmd
ref := info.registryAddress + "/test/push-pull"
tag := ":v.0.0.1"
cmd.Command = dockerCli.Command("app", "push", "--tag", ref+tag, "--insecure-registries="+info.registryAddress, filepath.Join("testdata", "push-pull", "push-pull.dockerapp"))
cmd.Command = dockerCli.Command("app", "push", "--tag", ref+tag, filepath.Join("testdata", "push-pull", "push-pull.dockerapp"))
icmd.RunCmd(cmd).Assert(t, icmd.Success)
cmd.Command = dockerCli.Command("app", "pull", ref+tag, "--insecure-registries="+info.registryAddress)
cmd.Command = dockerCli.Command("app", "pull", ref+tag)
icmd.RunCmd(cmd).Assert(t, icmd.Success)

// stop the registry
info.stopRegistry()

// install without --pull should succeed (rely on local store)
cmd.Command = dockerCli.Command("app", "install", "--insecure-registries="+info.registryAddress, ref+tag, "--name", t.Name())
cmd.Command = dockerCli.Command("app", "install", ref+tag, "--name", t.Name())
icmd.RunCmd(cmd).Assert(t, icmd.Success)
cmd.Command = dockerCli.Command("service", "ls")
assert.Check(t, cmp.Contains(icmd.RunCmd(cmd).Assert(t, icmd.Success).Combined(), ref))
Expand All @@ -228,7 +228,7 @@ func TestPushPullInstall(t *testing.T) {
})

// install with --pull should fail (registry is stopped)
cmd.Command = dockerCli.Command("app", "install", "--pull", "--insecure-registries="+info.registryAddress, ref, "--name", t.Name()+"2")
cmd.Command = dockerCli.Command("app", "install", "--pull", ref, "--name", t.Name()+"2")
assert.Check(t, cmp.Contains(icmd.RunCmd(cmd).Assert(t, icmd.Expected{ExitCode: 1}).Combined(), "failed to resolve bundle manifest"))
})
}
Expand All @@ -249,10 +249,10 @@ func TestPushInstallBundle(t *testing.T) {
// push it and install to check it is available
t.Run("push-bundle", func(t *testing.T) {
name := strings.Replace(t.Name(), "/", "_", 1)
cmd.Command = dockerCli.Command("app", "push", "--insecure-registries="+info.registryAddress, "--tag", ref, bundleFile)
cmd.Command = dockerCli.Command("app", "push", "--tag", ref, bundleFile)
icmd.RunCmd(cmd).Assert(t, icmd.Success)

cmd.Command = dockerCli.Command("app", "install", "--insecure-registries="+info.registryAddress, ref, "--name", name)
cmd.Command = dockerCli.Command("app", "install", ref, "--name", name)
icmd.RunCmd(cmd).Assert(t, icmd.Success)
cmd.Command = dockerCli.Command("service", "ls")
assert.Check(t, cmp.Contains(icmd.RunCmd(cmd).Assert(t, icmd.Success).Combined(), ref))
Expand All @@ -269,10 +269,10 @@ func TestPushInstallBundle(t *testing.T) {
t.Run("push-ref", func(t *testing.T) {
name := strings.Replace(t.Name(), "/", "_", 1)
ref2 := info.registryAddress + "/test/push-ref"
cmd.Command = dockerCli.Command("app", "push", "--insecure-registries="+info.registryAddress, "--tag", ref2, ref+":latest")
cmd.Command = dockerCli.Command("app", "push", "--tag", ref2, ref+":latest")
icmd.RunCmd(cmd).Assert(t, icmd.Success)

cmd.Command = dockerCli.Command("app", "install", "--insecure-registries="+info.registryAddress, ref2, "--name", name)
cmd.Command = dockerCli.Command("app", "install", ref2, "--name", name)
icmd.RunCmd(cmd).Assert(t, icmd.Success)
cmd.Command = dockerCli.Command("service", "ls")
assert.Check(t, cmp.Contains(icmd.RunCmd(cmd).Assert(t, icmd.Success).Combined(), ref2))
Expand All @@ -288,12 +288,12 @@ func TestPushInstallBundle(t *testing.T) {
cmd2.Command = dockerCli.Command("app", "bundle", "--tag", ref2, "-o", bundleFile, filepath.Join("testdata", "push-pull", "push-pull.dockerapp"))
icmd.RunCmd(cmd2).Assert(t, icmd.Success)
// Push the app without tagging it explicitly
cmd2.Command = dockerCli.Command("app", "push", "--insecure-registries="+info.registryAddress, ref2)
cmd2.Command = dockerCli.Command("app", "push", ref2)
icmd.RunCmd(cmd2).Assert(t, icmd.Success)
// remove the bundle from the bundle store to be sure it won't be used instead of registry
cleanup2()
// install from the registry
cmd.Command = dockerCli.Command("app", "install", "--insecure-registries="+info.registryAddress, ref2, "--name", name)
cmd.Command = dockerCli.Command("app", "install", ref2, "--name", name)
icmd.RunCmd(cmd).Assert(t, icmd.Success)
cmd.Command = dockerCli.Command("service", "ls")
assert.Check(t, cmp.Contains(icmd.RunCmd(cmd).Assert(t, icmd.Success).Combined(), ref))
Expand Down
8 changes: 4 additions & 4 deletions internal/commands/cnab.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ func loadBundleFromFile(filename string) (*bundle.Bundle, error) {
//resolveBundle looks for a CNAB bundle which can be in a Docker App Package format or
// a bundle stored locally or in the bundle store. It returns a built or found bundle,
// a reference to the bundle if it is found in the bundlestore, and an error.
func resolveBundle(dockerCli command.Cli, bundleStore appstore.BundleStore, name string, pullRef bool, insecureRegistries []string) (*bundle.Bundle, string, error) {
func resolveBundle(dockerCli command.Cli, bundleStore appstore.BundleStore, name string, pullRef bool) (*bundle.Bundle, string, error) {
// resolution logic:
// - if there is a docker-app package in working directory, or an http:// / https:// prefix, use packager.Extract result
// - the name has a .json or .cnab extension and refers to an existing file or web resource: load the bundle
Expand Down Expand Up @@ -286,7 +286,7 @@ func resolveBundle(dockerCli command.Cli, bundleStore appstore.BundleStore, name
return nil, "", errors.Wrap(err, name)
}
tagRef := reference.TagNameOnly(ref)
bndl, err := bundleStore.LookupOrPullBundle(tagRef, pullRef, dockerCli.ConfigFile(), insecureRegistries)
bndl, err := bundleStore.LookupOrPullBundle(tagRef, pullRef, dockerCli.ConfigFile(), insecureRegistries(dockerCli))
return bndl, tagRef.String(), err
}
return nil, "", fmt.Errorf("could not resolve bundle %q", name)
Expand Down Expand Up @@ -346,7 +346,7 @@ func isDockerHostLocal(host string) bool {
}

func prepareCustomAction(actionName string, dockerCli command.Cli, appname string, stdout io.Writer,
registryOpts registryOptions, pullOpts pullOptions, paramsOpts parametersOptions) (*action.RunCustom, *appstore.Installation, *bytes.Buffer, error) {
pullOpts pullOptions, paramsOpts parametersOptions) (*action.RunCustom, *appstore.Installation, *bytes.Buffer, error) {
s, err := appstore.NewApplicationStore(config.Dir())
if err != nil {
return nil, nil, nil, err
Expand All @@ -356,7 +356,7 @@ func prepareCustomAction(actionName string, dockerCli command.Cli, appname strin
return nil, nil, nil, err
}
driverImpl, errBuf := prepareDriver(dockerCli, bindMount{}, stdout)
bundle, ref, err := resolveBundle(dockerCli, bundleStore, appname, pullOpts.pull, registryOpts.insecureRegistries)
bundle, ref, err := resolveBundle(dockerCli, bundleStore, appname, pullOpts.pull)
if err != nil {
return nil, nil, nil, err
}
Expand Down
4 changes: 1 addition & 3 deletions internal/commands/inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (

type inspectOptions struct {
parametersOptions
registryOptions
pullOptions
}

Expand All @@ -27,14 +26,13 @@ func inspectCmd(dockerCli command.Cli) *cobra.Command {
},
}
opts.parametersOptions.addFlags(cmd.Flags())
opts.registryOptions.addFlags(cmd.Flags())
opts.pullOptions.addFlags(cmd.Flags())
return cmd
}

func runInspect(dockerCli command.Cli, appname string, opts inspectOptions) error {
defer muteDockerCli(dockerCli)()
action, installation, errBuf, err := prepareCustomAction(internal.ActionInspectName, dockerCli, appname, nil, opts.registryOptions, opts.pullOptions, opts.parametersOptions)
action, installation, errBuf, err := prepareCustomAction(internal.ActionInspectName, dockerCli, appname, nil, opts.pullOptions, opts.parametersOptions)
if err != nil {
return err
}
Expand Down
4 changes: 1 addition & 3 deletions internal/commands/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
type installOptions struct {
parametersOptions
credentialOptions
registryOptions
pullOptions
orchestrator string
kubeNamespace string
Expand Down Expand Up @@ -59,7 +58,6 @@ func installCmd(dockerCli command.Cli) *cobra.Command {
}
opts.parametersOptions.addFlags(cmd.Flags())
opts.credentialOptions.addFlags(cmd.Flags())
opts.registryOptions.addFlags(cmd.Flags())
opts.pullOptions.addFlags(cmd.Flags())
cmd.Flags().StringVar(&opts.orchestrator, "orchestrator", "", "Orchestrator to install on (swarm, kubernetes)")
cmd.Flags().StringVar(&opts.kubeNamespace, "namespace", "default", "Kubernetes namespace to install into")
Expand All @@ -81,7 +79,7 @@ func runInstall(dockerCli command.Cli, appname string, opts installOptions) erro
return err
}

bndl, ref, err := resolveBundle(dockerCli, bundleStore, appname, opts.pull, opts.insecureRegistries)
bndl, ref, err := resolveBundle(dockerCli, bundleStore, appname, opts.pull)
if err != nil {
return err
}
Expand Down
8 changes: 3 additions & 5 deletions internal/commands/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,19 @@ import (
)

func pullCmd(dockerCli command.Cli) *cobra.Command {
var opts registryOptions
cmd := &cobra.Command{
Use: "pull NAME:TAG [OPTIONS]",
Short: "Pull an application package from a registry",
Example: `$ docker app pull docker/app-example:0.1.0`,
Args: cli.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
return runPull(dockerCli, args[0], opts)
return runPull(dockerCli, args[0])
},
}
opts.addFlags(cmd.Flags())
return cmd
}

func runPull(dockerCli command.Cli, name string, opts registryOptions) error {
func runPull(dockerCli command.Cli, name string) error {
appstore, err := store.NewApplicationStore(config.Dir())
if err != nil {
return err
Expand All @@ -42,7 +40,7 @@ func runPull(dockerCli command.Cli, name string, opts registryOptions) error {
if err != nil {
return errors.Wrap(err, name)
}
bndl, err := bundleStore.LookupOrPullBundle(reference.TagNameOnly(ref), true, dockerCli.ConfigFile(), opts.insecureRegistries)
bndl, err := bundleStore.LookupOrPullBundle(reference.TagNameOnly(ref), true, dockerCli.ConfigFile(), insecureRegistries(dockerCli))
if err != nil {
return errors.Wrap(err, name)
}
Expand Down
6 changes: 2 additions & 4 deletions internal/commands/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ const ( // Docker specific annotations and values
)

type pushOptions struct {
registry registryOptions
tag string
platforms []string
allPlatforms bool
Expand All @@ -66,7 +65,6 @@ func pushCmd(dockerCli command.Cli) *cobra.Command {
flags.StringVarP(&opts.tag, "tag", "t", "", "Target registry reference (default: <name>:<version> from metadata)")
flags.StringSliceVar(&opts.platforms, "platform", []string{"linux/amd64"}, "For multi-arch service images, push the specified platforms")
flags.BoolVar(&opts.allPlatforms, "all-platforms", false, "If present, push all platforms")
opts.registry.addFlags(flags)
return cmd
}

Expand Down Expand Up @@ -102,7 +100,7 @@ func resolveReferenceAndBundle(dockerCli command.Cli, name string) (*bundle.Bund
return nil, "", err
}

bndl, ref, err := resolveBundle(dockerCli, bundleStore, name, false, nil)
bndl, ref, err := resolveBundle(dockerCli, bundleStore, name, false)
if err != nil {
return nil, "", err
}
Expand Down Expand Up @@ -136,7 +134,7 @@ func pushInvocationImage(dockerCli command.Cli, retag retagResult) error {
}

func pushBundle(dockerCli command.Cli, opts pushOptions, bndl *bundle.Bundle, retag retagResult) error {
resolver := remotes.CreateResolver(dockerCli.ConfigFile(), opts.registry.insecureRegistries...)
resolver := remotes.CreateResolver(dockerCli.ConfigFile(), insecureRegistries(dockerCli)...)
var display fixupDisplay = &plainDisplay{out: os.Stdout}
if term.IsTerminal(os.Stdout.Fd()) {
display = &interactiveDisplay{out: os.Stdout}
Expand Down
4 changes: 1 addition & 3 deletions internal/commands/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (

type renderOptions struct {
parametersOptions
registryOptions
pullOptions

formatDriver string
Expand All @@ -32,7 +31,6 @@ func renderCmd(dockerCli command.Cli) *cobra.Command {
},
}
opts.parametersOptions.addFlags(cmd.Flags())
opts.registryOptions.addFlags(cmd.Flags())
opts.pullOptions.addFlags(cmd.Flags())
cmd.Flags().StringVarP(&opts.renderOutput, "output", "o", "-", "Output file")
cmd.Flags().StringVar(&opts.formatDriver, "formatter", "yaml", "Configure the output format (yaml|json)")
Expand All @@ -53,7 +51,7 @@ func runRender(dockerCli command.Cli, appname string, opts renderOptions) error
w = f
}

action, installation, errBuf, err := prepareCustomAction(internal.ActionRenderName, dockerCli, appname, w, opts.registryOptions, opts.pullOptions, opts.parametersOptions)
action, installation, errBuf, err := prepareCustomAction(internal.ActionRenderName, dockerCli, appname, w, opts.pullOptions, opts.parametersOptions)
if err != nil {
return err
}
Expand Down
30 changes: 22 additions & 8 deletions internal/commands/root.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package commands

import (
"context"
"fmt"
"io/ioutil"
"os"

"github.com/docker/app/internal"
"github.com/sirupsen/logrus"

"github.com/docker/app/internal/store"
"github.com/docker/cli/cli/command"
Expand Down Expand Up @@ -159,18 +161,30 @@ func (o *credentialOptions) CredentialSetOpts(dockerCli command.Cli, credentialS
}
}

type registryOptions struct {
insecureRegistries []string
}

func (o *registryOptions) addFlags(flags *pflag.FlagSet) {
flags.StringSliceVar(&o.insecureRegistries, "insecure-registries", nil, "Use HTTP instead of HTTPS when pulling from/pushing to those registries")
}

type pullOptions struct {
pull bool
}

func (o *pullOptions) addFlags(flags *pflag.FlagSet) {
flags.BoolVar(&o.pull, "pull", false, "Pull the bundle")
}

func insecureRegistries(dockerCli command.Cli) (registries []string) {
registries = []string{}

info, err := dockerCli.Client().Info(context.Background())
if err != nil {
logrus.Debugf("could not get docker info: %v", err)
return
}

for _, reg := range info.RegistryConfig.IndexConfigs {
if !reg.Secure {
registries = append(registries, reg.Name)
}
}

logrus.Debugf("insecure registries: %v", registries)

return
}
4 changes: 1 addition & 3 deletions internal/commands/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
type upgradeOptions struct {
parametersOptions
credentialOptions
registryOptions
pullOptions
bundleOrDockerApp string
}
Expand All @@ -31,7 +30,6 @@ func upgradeCmd(dockerCli command.Cli) *cobra.Command {
}
opts.parametersOptions.addFlags(cmd.Flags())
opts.credentialOptions.addFlags(cmd.Flags())
opts.registryOptions.addFlags(cmd.Flags())
opts.pullOptions.addFlags(cmd.Flags())
cmd.Flags().StringVar(&opts.bundleOrDockerApp, "app-name", "", "Override the installation with another Application Package")

Expand All @@ -57,7 +55,7 @@ func runUpgrade(dockerCli command.Cli, installationName string, opts upgradeOpti
}

if opts.bundleOrDockerApp != "" {
b, _, err := resolveBundle(dockerCli, bundleStore, opts.bundleOrDockerApp, opts.pull, opts.insecureRegistries)
b, _, err := resolveBundle(dockerCli, bundleStore, opts.bundleOrDockerApp, opts.pull)
if err != nil {
return err
}
Expand Down

0 comments on commit 56ce605

Please sign in to comment.