diff options
author | Valentin Rothberg <rothberg@redhat.com> | 2021-10-22 16:07:26 +0200 |
---|---|---|
committer | Valentin Rothberg <rothberg@redhat.com> | 2021-10-27 14:36:25 +0200 |
commit | ff31f2264da1550be97055865dea639eb0882327 (patch) | |
tree | f3b3c2f81c1825ac9bbce41e9e16279fccc23604 | |
parent | 979b6312286b4bd993d7be0413e1e95c4a0bad56 (diff) | |
download | podman-ff31f2264da1550be97055865dea639eb0882327.tar.gz podman-ff31f2264da1550be97055865dea639eb0882327.tar.bz2 podman-ff31f2264da1550be97055865dea639eb0882327.zip |
container create: fix --tls-verify parsing
Make sure that the value is only set if specified on the CLI. c/image
already defaults to true but if set in the system context, we'd skip
settings in the registries.conf.
Fixes: #11933
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
-rw-r--r-- | cmd/podman/common/create.go | 8 | ||||
-rw-r--r-- | cmd/podman/containers/create.go | 7 | ||||
-rw-r--r-- | go.mod | 2 | ||||
-rw-r--r-- | go.sum | 5 | ||||
-rw-r--r-- | pkg/domain/entities/pods.go | 3 | ||||
-rw-r--r-- | test/e2e/push_test.go | 4 | ||||
-rw-r--r-- | test/e2e/run_test.go | 6 | ||||
-rw-r--r-- | test/registries.conf | 6 | ||||
-rw-r--r-- | vendor/github.com/containers/common/libimage/filters.go | 32 | ||||
-rw-r--r-- | vendor/github.com/containers/common/libimage/image.go | 36 | ||||
-rw-r--r-- | vendor/github.com/containers/common/libimage/search.go | 2 | ||||
-rw-r--r-- | vendor/github.com/containers/common/pkg/config/config.go | 2 | ||||
-rw-r--r-- | vendor/github.com/containers/common/pkg/flag/flag.go | 174 | ||||
-rw-r--r-- | vendor/modules.txt | 3 |
14 files changed, 258 insertions, 32 deletions
diff --git a/cmd/podman/common/create.go b/cmd/podman/common/create.go index 6270bad16..fbc8fb8ab 100644 --- a/cmd/podman/common/create.go +++ b/cmd/podman/common/create.go @@ -5,6 +5,7 @@ import ( "github.com/containers/common/pkg/auth" "github.com/containers/common/pkg/completion" + commonFlag "github.com/containers/common/pkg/flag" "github.com/containers/podman/v3/cmd/podman/registry" "github.com/containers/podman/v3/libpod/define" "github.com/containers/podman/v3/pkg/domain/entities" @@ -589,12 +590,9 @@ func DefineCreateFlags(cmd *cobra.Command, cf *entities.ContainerCreateOptions, ) _ = cmd.RegisterFlagCompletionFunc(timeoutFlagName, completion.AutocompleteNone) - // Flag for TLS verification, so that `run` and `create` commands can make use of it. - // Make sure to use `=` while using this flag i.e `--tls-verify=false/true` - tlsVerifyFlagName := "tls-verify" - createFlags.BoolVar( + commonFlag.OptionalBoolFlag(createFlags, &cf.TLSVerify, - tlsVerifyFlagName, true, + "tls-verify", "Require HTTPS and verify certificates when contacting registries for pulling images", ) diff --git a/cmd/podman/containers/create.go b/cmd/podman/containers/create.go index 3be426ae2..d35c1a192 100644 --- a/cmd/podman/containers/create.go +++ b/cmd/podman/containers/create.go @@ -303,6 +303,11 @@ func PullImage(imageName string, cliVals entities.ContainerCreateOptions) (strin } } + skipTLSVerify := types.OptionalBoolUndefined + if cliVals.TLSVerify.Present() { + skipTLSVerify = types.NewOptionalBool(!cliVals.TLSVerify.Value()) + } + pullReport, pullErr := registry.ImageEngine().Pull(registry.GetContext(), imageName, entities.ImagePullOptions{ Authfile: cliVals.Authfile, Quiet: cliVals.Quiet, @@ -311,7 +316,7 @@ func PullImage(imageName string, cliVals entities.ContainerCreateOptions) (strin Variant: cliVals.Variant, SignaturePolicy: cliVals.SignaturePolicy, PullPolicy: pullPolicy, - SkipTLSVerify: types.NewOptionalBool(!cliVals.TLSVerify), // If Flag changed for TLS Verification + SkipTLSVerify: skipTLSVerify, }) if pullErr != nil { return "", pullErr @@ -12,7 +12,7 @@ require ( github.com/containernetworking/cni v1.0.1 github.com/containernetworking/plugins v1.0.1 github.com/containers/buildah v1.23.1 - github.com/containers/common v0.46.1-0.20211008123044-d846f5aaec0e + github.com/containers/common v0.46.1-0.20211026130826-7abfd453c86f github.com/containers/conmon v2.0.20+incompatible github.com/containers/image/v5 v5.16.1 github.com/containers/ocicrypt v1.1.2 @@ -258,8 +258,8 @@ github.com/containernetworking/plugins v1.0.1/go.mod h1:QHCfGpaTwYTbbH+nZXKVTxNB github.com/containers/buildah v1.23.1 h1:Tpc9DsRuU+0Oofewpxb6OJVNQjCu7yloN/obUqzfDTY= github.com/containers/buildah v1.23.1/go.mod h1:4WnrN0yrA7ab0ppgunixu2WM1rlD2rG8QLJAKbEkZlQ= github.com/containers/common v0.44.2/go.mod h1:7sdP4vmI5Bm6FPFxb3lvAh1Iktb6tiO1MzjUzhxdoGo= -github.com/containers/common v0.46.1-0.20211008123044-d846f5aaec0e h1:lYazDued7KBcMq5IJzRIbX47SSLRg/yYxvM/P9LaVhE= -github.com/containers/common v0.46.1-0.20211008123044-d846f5aaec0e/go.mod h1:ggZks97KCmjBcHvNTCyLc17SqdjSYoeexW7rnRt9H9Y= +github.com/containers/common v0.46.1-0.20211026130826-7abfd453c86f h1:jFFIV8QvsPgwkJHh3tjfREFRwSeMq5M8lt8vklkZaOk= +github.com/containers/common v0.46.1-0.20211026130826-7abfd453c86f/go.mod h1:pVvmLTLCOZE300e4rex/QDmpnRmEM/5aZ/YfCkkjgZo= github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg= github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I= github.com/containers/image/v5 v5.16.0/go.mod h1:XgTpfAPLRGOd1XYyCU5cISFr777bLmOerCSpt/v7+Q4= @@ -336,7 +336,6 @@ github.com/docker/distribution v2.7.1+incompatible h1:a5mlkVzth6W5A4fOsS3D2EO5BU github.com/docker/distribution v2.7.1+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= github.com/docker/docker v1.4.2-0.20190924003213-a8608b5b67c7/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= github.com/docker/docker v20.10.8+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= -github.com/docker/docker v20.10.9+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= github.com/docker/docker v20.10.10+incompatible h1:GKkP0T7U4ks6X3lmmHKC2QDprnpRJor2Z5a8m62R9ZM= github.com/docker/docker v20.10.10+incompatible/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= github.com/docker/docker-credential-helpers v0.6.3/go.mod h1:WRaJzqw3CTB9bk10avuGsjVBZsD05qeibJ1/TYlvc0Y= diff --git a/pkg/domain/entities/pods.go b/pkg/domain/entities/pods.go index a2f36eaa6..3d8579acf 100644 --- a/pkg/domain/entities/pods.go +++ b/pkg/domain/entities/pods.go @@ -5,6 +5,7 @@ import ( "strings" "time" + commonFlag "github.com/containers/common/pkg/flag" "github.com/containers/podman/v3/libpod/define" "github.com/containers/podman/v3/pkg/specgen" "github.com/containers/podman/v3/pkg/util" @@ -241,7 +242,7 @@ type ContainerCreateOptions struct { Sysctl []string Systemd string Timeout uint - TLSVerify bool + TLSVerify commonFlag.OptionalBool TmpFS []string TTY bool Timezone string diff --git a/test/e2e/push_test.go b/test/e2e/push_test.go index b7e8309fb..7b35acd35 100644 --- a/test/e2e/push_test.go +++ b/test/e2e/push_test.go @@ -146,7 +146,7 @@ var _ = Describe("Podman push", func() { session = podmanTest.Podman([]string{"logs", "registry"}) session.WaitWithDefaultTimeout() - push := podmanTest.Podman([]string{"push", "--format=v2s2", "--creds=podmantest:test", ALPINE, "localhost:5000/tlstest"}) + push := podmanTest.Podman([]string{"push", "--tls-verify=true", "--format=v2s2", "--creds=podmantest:test", ALPINE, "localhost:5000/tlstest"}) push.WaitWithDefaultTimeout() Expect(push).To(ExitWithError()) @@ -163,7 +163,7 @@ var _ = Describe("Podman push", func() { if !IsRemote() { // remote does not support --cert-dir - push = podmanTest.Podman([]string{"push", "--creds=podmantest:test", "--cert-dir=fakedir", ALPINE, "localhost:5000/certdirtest"}) + push = podmanTest.Podman([]string{"push", "--tls-verify=true", "--creds=podmantest:test", "--cert-dir=fakedir", ALPINE, "localhost:5000/certdirtest"}) push.WaitWithDefaultTimeout() Expect(push).To(ExitWithError()) } diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 8502879ff..2e0f626d0 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -188,6 +188,12 @@ var _ = Describe("Podman run", func() { run.WaitWithDefaultTimeout() Expect(run).Should(Exit(0)) Expect(podmanTest.NumberOfContainers()).To(Equal(3)) + + // Now registries.conf will be consulted where localhost:5000 + // is set to be insecure. + run = podmanTest.Podman([]string{"run", ALPINE}) + run.WaitWithDefaultTimeout() + Expect(run).Should(Exit(0)) }) It("podman run a container with a --rootfs", func() { diff --git a/test/registries.conf b/test/registries.conf index 0559c9e52..8e4671760 100644 --- a/test/registries.conf +++ b/test/registries.conf @@ -15,3 +15,9 @@ location="mirror.gcr.io" [[registry]] prefix="docker.io/library" location="quay.io/libpod" + +# For testing #11933 to make sure that registries.conf is consulted unless +# --tls-verify is used during container creation. +[[registry]] +location="localhost:5000" +insecure=true diff --git a/vendor/github.com/containers/common/libimage/filters.go b/vendor/github.com/containers/common/libimage/filters.go index 833f940cc..521af5d06 100644 --- a/vendor/github.com/containers/common/libimage/filters.go +++ b/vendor/github.com/containers/common/libimage/filters.go @@ -50,6 +50,18 @@ func filterImages(images []*Image, filters []filterFunc) ([]*Image, error) { func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOptions) ([]filterFunc, error) { logrus.Tracef("Parsing image filters %s", options.Filters) + var tree *layerTree + getTree := func() (*layerTree, error) { + if tree == nil { + t, err := r.layerTree() + if err != nil { + return nil, err + } + tree = t + } + return tree, nil + } + filterFuncs := []filterFunc{} for _, filter := range options.Filters { var key, value string @@ -93,7 +105,11 @@ func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOp if err != nil { return nil, errors.Wrapf(err, "non-boolean value %q for dangling filter", value) } - filterFuncs = append(filterFuncs, filterDangling(ctx, dangling)) + t, err := getTree() + if err != nil { + return nil, err + } + filterFuncs = append(filterFuncs, filterDangling(ctx, dangling, t)) case "id": filterFuncs = append(filterFuncs, filterID(value)) @@ -103,7 +119,11 @@ func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOp if err != nil { return nil, errors.Wrapf(err, "non-boolean value %q for intermediate filter", value) } - filterFuncs = append(filterFuncs, filterIntermediate(ctx, intermediate)) + t, err := getTree() + if err != nil { + return nil, err + } + filterFuncs = append(filterFuncs, filterIntermediate(ctx, intermediate, t)) case "label": filterFuncs = append(filterFuncs, filterLabel(ctx, value)) @@ -221,9 +241,9 @@ func filterContainers(value string, fn IsExternalContainerFunc) filterFunc { } // filterDangling creates a dangling filter for matching the specified value. -func filterDangling(ctx context.Context, value bool) filterFunc { +func filterDangling(ctx context.Context, value bool, tree *layerTree) filterFunc { return func(img *Image) (bool, error) { - isDangling, err := img.IsDangling(ctx) + isDangling, err := img.isDangling(ctx, tree) if err != nil { return false, err } @@ -241,9 +261,9 @@ func filterID(value string) filterFunc { // filterIntermediate creates an intermediate filter for images. An image is // considered to be an intermediate image if it is dangling (i.e., no tags) and // has no children (i.e., no other image depends on it). -func filterIntermediate(ctx context.Context, value bool) filterFunc { +func filterIntermediate(ctx context.Context, value bool, tree *layerTree) filterFunc { return func(img *Image) (bool, error) { - isIntermediate, err := img.IsIntermediate(ctx) + isIntermediate, err := img.isIntermediate(ctx, tree) if err != nil { return false, err } diff --git a/vendor/github.com/containers/common/libimage/image.go b/vendor/github.com/containers/common/libimage/image.go index 00a2d620e..bf3310da2 100644 --- a/vendor/github.com/containers/common/libimage/image.go +++ b/vendor/github.com/containers/common/libimage/image.go @@ -128,10 +128,16 @@ func (i *Image) IsReadOnly() bool { // IsDangling returns true if the image is dangling, that is an untagged image // without children. func (i *Image) IsDangling(ctx context.Context) (bool, error) { + return i.isDangling(ctx, nil) +} + +// isDangling returns true if the image is dangling, that is an untagged image +// without children. If tree is nil, it will created for this invocation only. +func (i *Image) isDangling(ctx context.Context, tree *layerTree) (bool, error) { if len(i.Names()) > 0 { return false, nil } - children, err := i.getChildren(ctx, false) + children, err := i.getChildren(ctx, false, tree) if err != nil { return false, err } @@ -141,10 +147,17 @@ func (i *Image) IsDangling(ctx context.Context) (bool, error) { // IsIntermediate returns true if the image is an intermediate image, that is // an untagged image with children. func (i *Image) IsIntermediate(ctx context.Context) (bool, error) { + return i.isIntermediate(ctx, nil) +} + +// isIntermediate returns true if the image is an intermediate image, that is +// an untagged image with children. If tree is nil, it will created for this +// invocation only. +func (i *Image) isIntermediate(ctx context.Context, tree *layerTree) (bool, error) { if len(i.Names()) > 0 { return false, nil } - children, err := i.getChildren(ctx, false) + children, err := i.getChildren(ctx, false, tree) if err != nil { return false, err } @@ -189,7 +202,7 @@ func (i *Image) Parent(ctx context.Context) (*Image, error) { // HasChildren returns indicates if the image has children. func (i *Image) HasChildren(ctx context.Context) (bool, error) { - children, err := i.getChildren(ctx, false) + children, err := i.getChildren(ctx, false, nil) if err != nil { return false, err } @@ -198,7 +211,7 @@ func (i *Image) HasChildren(ctx context.Context) (bool, error) { // Children returns the image's children. func (i *Image) Children(ctx context.Context) ([]*Image, error) { - children, err := i.getChildren(ctx, true) + children, err := i.getChildren(ctx, true, nil) if err != nil { return nil, err } @@ -206,13 +219,16 @@ func (i *Image) Children(ctx context.Context) ([]*Image, error) { } // getChildren returns a list of imageIDs that depend on the image. If all is -// false, only the first child image is returned. -func (i *Image) getChildren(ctx context.Context, all bool) ([]*Image, error) { - tree, err := i.runtime.layerTree() - if err != nil { - return nil, err +// false, only the first child image is returned. If tree is nil, it will be +// created for this invocation only. +func (i *Image) getChildren(ctx context.Context, all bool, tree *layerTree) ([]*Image, error) { + if tree == nil { + t, err := i.runtime.layerTree() + if err != nil { + return nil, err + } + tree = t } - return tree.children(ctx, i, all) } diff --git a/vendor/github.com/containers/common/libimage/search.go b/vendor/github.com/containers/common/libimage/search.go index 7e20e4331..ece81531a 100644 --- a/vendor/github.com/containers/common/libimage/search.go +++ b/vendor/github.com/containers/common/libimage/search.go @@ -244,7 +244,7 @@ func (r *Runtime) searchImageInRegistry(ctx context.Context, term, registry stri name = index + "/library/" + results[i].Name } params := SearchResult{ - Index: index, + Index: registry, Name: name, Description: description, Official: official, diff --git a/vendor/github.com/containers/common/pkg/config/config.go b/vendor/github.com/containers/common/pkg/config/config.go index d5be77edd..45230703d 100644 --- a/vendor/github.com/containers/common/pkg/config/config.go +++ b/vendor/github.com/containers/common/pkg/config/config.go @@ -574,7 +574,7 @@ func readConfigFromFile(path string, config *Config) error { } keys := meta.Undecoded() if len(keys) > 0 { - logrus.Warningf("Failed to decode the keys %q from %q.", keys, path) + logrus.Debugf("Failed to decode the keys %q from %q.", keys, path) } return nil diff --git a/vendor/github.com/containers/common/pkg/flag/flag.go b/vendor/github.com/containers/common/pkg/flag/flag.go new file mode 100644 index 000000000..52eb50da0 --- /dev/null +++ b/vendor/github.com/containers/common/pkg/flag/flag.go @@ -0,0 +1,174 @@ +package flag + +import ( + "strconv" + + "github.com/spf13/pflag" +) + +// OptionalBool is a boolean with a separate presence flag and value. +type OptionalBool struct { + present bool + value bool +} + +// Present returns the bool's presence flag. +func (ob *OptionalBool) Present() bool { + return ob.present +} + +// Present returns the bool's value. Should only be used if Present() is true. +func (ob *OptionalBool) Value() bool { + return ob.value +} + +// optionalBool is a cli.Generic == flag.Value implementation equivalent to +// the one underlying flag.Bool, except that it records whether the flag has been set. +// This is distinct from optionalBool to (pretend to) force callers to use +// optionalBoolFlag +type optionalBoolValue OptionalBool + +// OptionalBoolFlag creates new flag for an optional in the specified flag with +// the specified name and usage. +func OptionalBoolFlag(fs *pflag.FlagSet, p *OptionalBool, name, usage string) *pflag.Flag { + flag := fs.VarPF(internalNewOptionalBoolValue(p), name, "", usage) + flag.NoOptDefVal = "true" + flag.DefValue = "false" + return flag +} + +// WARNING: Do not directly use this method to define optionalBool flag. +// Caller should use optionalBoolFlag +func internalNewOptionalBoolValue(p *OptionalBool) pflag.Value { + p.present = false + return (*optionalBoolValue)(p) +} + +// Set parses the string to a bool and sets it. +func (ob *optionalBoolValue) Set(s string) error { + v, err := strconv.ParseBool(s) + if err != nil { + return err + } + ob.value = v + ob.present = true + return nil +} + +// String returns the string representation of the string. +func (ob *optionalBoolValue) String() string { + if !ob.present { + return "" // This is, sadly, not round-trip safe: --flag is interpreted as --flag=true + } + return strconv.FormatBool(ob.value) +} + +// Type returns the type. +func (ob *optionalBoolValue) Type() string { + return "bool" +} + +// IsBoolFlag indicates that it's a bool flag. +func (ob *optionalBoolValue) IsBoolFlag() bool { + return true +} + +// OptionalString is a string with a separate presence flag. +type OptionalString struct { + present bool + value string +} + +// Present returns the strings's presence flag. +func (os *OptionalString) Present() bool { + return os.present +} + +// Present returns the string's value. Should only be used if Present() is true. +func (os *OptionalString) Value() string { + return os.value +} + +// optionalString is a cli.Generic == flag.Value implementation equivalent to +// the one underlying flag.String, except that it records whether the flag has been set. +// This is distinct from optionalString to (pretend to) force callers to use +// newoptionalString +type optionalStringValue OptionalString + +// NewOptionalStringValue returns a pflag.Value fo the string. +func NewOptionalStringValue(p *OptionalString) pflag.Value { + p.present = false + return (*optionalStringValue)(p) +} + +// Set sets the string. +func (ob *optionalStringValue) Set(s string) error { + ob.value = s + ob.present = true + return nil +} + +// String returns the string if present. +func (ob *optionalStringValue) String() string { + if !ob.present { + return "" // This is, sadly, not round-trip safe: --flag= is interpreted as {present:true, value:""} + } + return ob.value +} + +// Type returns the string type. +func (ob *optionalStringValue) Type() string { + return "string" +} + +// OptionalInt is a int with a separate presence flag. +type OptionalInt struct { + present bool + value int +} + +// Present returns the int's presence flag. +func (oi *OptionalInt) Present() bool { + return oi.present +} + +// Present returns the int's value. Should only be used if Present() is true. +func (oi *OptionalInt) Value() int { + return oi.value +} + +// optionalInt is a cli.Generic == flag.Value implementation equivalent to +// the one underlying flag.Int, except that it records whether the flag has been set. +// This is distinct from optionalInt to (pretend to) force callers to use +// newoptionalIntValue +type optionalIntValue OptionalInt + +// NewOptionalIntValue returns the pflag.Value of the int. +func NewOptionalIntValue(p *OptionalInt) pflag.Value { + p.present = false + return (*optionalIntValue)(p) +} + +// Set parses the string to an int and sets it. +func (ob *optionalIntValue) Set(s string) error { + v, err := strconv.ParseInt(s, 0, strconv.IntSize) + if err != nil { + return err + } + ob.value = int(v) + ob.present = true + return nil +} + +// String returns the string representation of the int. +func (ob *optionalIntValue) String() string { + if !ob.present { + return "" // If the value is not present, just return an empty string, any other value wouldn't make sense. + } + return strconv.Itoa(int(ob.value)) +} + +// Type returns the int's type. +func (ob *optionalIntValue) Type() string { + return "int" +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 6891f6ab1..8ea5dc5ee 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -95,7 +95,7 @@ github.com/containers/buildah/pkg/rusage github.com/containers/buildah/pkg/sshagent github.com/containers/buildah/pkg/util github.com/containers/buildah/util -# github.com/containers/common v0.46.1-0.20211008123044-d846f5aaec0e +# github.com/containers/common v0.46.1-0.20211026130826-7abfd453c86f github.com/containers/common/libimage github.com/containers/common/libimage/manifests github.com/containers/common/pkg/apparmor @@ -108,6 +108,7 @@ github.com/containers/common/pkg/completion github.com/containers/common/pkg/config github.com/containers/common/pkg/defaultnet github.com/containers/common/pkg/filters +github.com/containers/common/pkg/flag github.com/containers/common/pkg/manifests github.com/containers/common/pkg/parse github.com/containers/common/pkg/report |