From 04897ee64411cfe33035c37009e5b99c273409b2 Mon Sep 17 00:00:00 2001 From: Ashley Cui Date: Thu, 13 Jul 2023 09:04:40 -0400 Subject: [PATCH 01/34] Vendor c/common v0.55.2 Signed-off-by: Ashley Cui --- go.mod | 2 +- go.sum | 4 +- .../containers/common/libimage/filters.go | 6 +- .../containers/common/libimage/image.go | 66 +++++--- .../common/libimage/manifest_list.go | 5 + .../containers/common/libimage/normalize.go | 10 +- .../containers/common/libimage/pull.go | 2 +- .../containers/common/libimage/runtime.go | 142 +++++++++++------- .../containers/common/version/version.go | 2 +- vendor/modules.txt | 2 +- 10 files changed, 153 insertions(+), 88 deletions(-) diff --git a/go.mod b/go.mod index 3733076a9e..ded7774663 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/containernetworking/cni v1.1.2 github.com/containernetworking/plugins v1.3.0 github.com/containers/buildah v1.31.0 - github.com/containers/common v0.55.1 + github.com/containers/common v0.55.2 github.com/containers/conmon v2.0.20+incompatible github.com/containers/image/v5 v5.26.1 github.com/containers/libhvee v0.0.5 diff --git a/go.sum b/go.sum index 3dd94f199e..be99fbeb89 100644 --- a/go.sum +++ b/go.sum @@ -241,8 +241,8 @@ github.com/containernetworking/plugins v1.3.0 h1:QVNXMT6XloyMUoO2wUOqWTC1hWFV62Q github.com/containernetworking/plugins v1.3.0/go.mod h1:Pc2wcedTQQCVuROOOaLBPPxrEXqqXBFt3cZ+/yVg6l0= github.com/containers/buildah v1.31.0 h1:NgVtEyTsR7e/XLTSJElbInnGPjdDGNHqLKADPHzaUGg= github.com/containers/buildah v1.31.0/go.mod h1:tcgXcGhqw3kw49RapUS7tskEhxKLG4eVFJKA/QzgwNU= -github.com/containers/common v0.55.1 h1:sOlcIxEYXoR3OSHufew7CuSeOWr7a2jHGYw3r+xKA1k= -github.com/containers/common v0.55.1/go.mod h1:ZKPllYOZ2xj2rgWRdnHHVvWg6ru4BT28En8mO8DMMPk= +github.com/containers/common v0.55.2 h1:Cd+vmkUPDrPvL2v4Te1Wew6SIZdn4/XiyiBRT9IbcGg= +github.com/containers/common v0.55.2/go.mod h1:ZKPllYOZ2xj2rgWRdnHHVvWg6ru4BT28En8mO8DMMPk= 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.26.1 h1:8y3xq8GO/6y8FR+nAedHPsAFiAtOrab9qHTBpbqaX8g= diff --git a/vendor/github.com/containers/common/libimage/filters.go b/vendor/github.com/containers/common/libimage/filters.go index ff50321b77..995f89c784 100644 --- a/vendor/github.com/containers/common/libimage/filters.go +++ b/vendor/github.com/containers/common/libimage/filters.go @@ -394,10 +394,12 @@ func filterID(value string) filterFunc { } } -// filterDigest creates an digest filter for matching the specified value. +// filterDigest creates a digest filter for matching the specified value. func filterDigest(value string) filterFunc { + // TODO: return an error if value is not a digest + // if _, err := digest.Parse(value); err != nil {...} return func(img *Image) (bool, error) { - return string(img.Digest()) == value, nil + return img.hasDigest(value), nil } } diff --git a/vendor/github.com/containers/common/libimage/image.go b/vendor/github.com/containers/common/libimage/image.go index da4ff8b7a4..dc47030c56 100644 --- a/vendor/github.com/containers/common/libimage/image.go +++ b/vendor/github.com/containers/common/libimage/image.go @@ -144,6 +144,9 @@ func (i *Image) ID() string { // possibly many digests that we have stored for the image, so many // applications are better off using the entire list returned by Digests(). func (i *Image) Digest() digest.Digest { + // TODO: we return the image digest or the one of the manifest list + // which can lead to issues depending on the callers' assumptions. + // Hence, deprecate in favor of Digest_s_. return i.storageImage.Digest } @@ -154,6 +157,18 @@ func (i *Image) Digests() []digest.Digest { return i.storageImage.Digests } +// hasDigest returns whether the specified value matches any digest of the +// image. +func (i *Image) hasDigest(value string) bool { + // TODO: change the argument to a typed digest.Digest + for _, d := range i.Digests() { + if string(d) == value { + return true + } + } + return false +} + // IsReadOnly returns whether the image is set read only. func (i *Image) IsReadOnly() bool { return i.storageImage.ReadOnly @@ -656,6 +671,8 @@ func (i *Image) NamedTaggedRepoTags() ([]reference.NamedTagged, error) { // NamedRepoTags returns the repotags associated with the image as a // slice of reference.Named. func (i *Image) NamedRepoTags() ([]reference.Named, error) { + // FIXME: the NamedRepoTags name is a bit misleading as it can return + // repo@digest values if that’s how an image was pulled. var repoTags []reference.Named for _, name := range i.Names() { parsed, err := reference.Parse(name) @@ -669,32 +686,37 @@ func (i *Image) NamedRepoTags() ([]reference.Named, error) { return repoTags, nil } -// inRepoTags looks for the specified name/tag pair in the image's repo tags. -func (i *Image) inRepoTags(namedTagged reference.NamedTagged) (reference.Named, error) { +// inRepoTags looks for the specified name/tag in the image's repo tags. If +// `ignoreTag` is set, only the repo must match and the tag is ignored. +func (i *Image) inRepoTags(namedTagged reference.NamedTagged, ignoreTag bool) (reference.Named, error) { repoTags, err := i.NamedRepoTags() if err != nil { return nil, err } - pairs, err := ToNameTagPairs(repoTags) - if err != nil { - return nil, err - } - name := namedTagged.Name() tag := namedTagged.Tag() - for _, pair := range pairs { - if tag != pair.Tag { - continue + for _, r := range repoTags { + if !ignoreTag { + var repoTag string + tagged, isTagged := r.(reference.NamedTagged) + if isTagged { + repoTag = tagged.Tag() + } + if !isTagged || tag != repoTag { + continue + } } - if !strings.HasSuffix(pair.Name, name) { + + repoName := r.Name() + if !strings.HasSuffix(repoName, name) { continue } - if len(pair.Name) == len(name) { // full match - return pair.named, nil + if len(repoName) == len(name) { // full match + return r, nil } - if pair.Name[len(pair.Name)-len(name)-1] == '/' { // matches at repo - return pair.named, nil + if repoName[len(repoName)-len(name)-1] == '/' { // matches at repo + return r, nil } } @@ -806,6 +828,9 @@ type HasDifferentDigestOptions struct { // containers-auth.json(5) file to use when authenticating against // container registries. AuthFilePath string + // Allow contacting registries over HTTP, or HTTPS with failed TLS + // verification. Note that this does not affect other TLS connections. + InsecureSkipTLSVerify types.OptionalBool } // HasDifferentDigest returns true if the image specified by `remoteRef` has a @@ -831,8 +856,15 @@ func (i *Image) HasDifferentDigest(ctx context.Context, remoteRef types.ImageRef sys.VariantChoice = inspectInfo.Variant } - if options != nil && options.AuthFilePath != "" { - sys.AuthFilePath = options.AuthFilePath + if options != nil { + if options.AuthFilePath != "" { + sys.AuthFilePath = options.AuthFilePath + } + if options.InsecureSkipTLSVerify != types.OptionalBoolUndefined { + sys.DockerInsecureSkipTLSVerify = options.InsecureSkipTLSVerify + sys.OCIInsecureSkipTLSVerify = options.InsecureSkipTLSVerify == types.OptionalBoolTrue + sys.DockerDaemonInsecureSkipTLSVerify = options.InsecureSkipTLSVerify == types.OptionalBoolTrue + } } return i.hasDifferentDigestWithSystemContext(ctx, remoteRef, sys) diff --git a/vendor/github.com/containers/common/libimage/manifest_list.go b/vendor/github.com/containers/common/libimage/manifest_list.go index 3a75709e01..0223fb3557 100644 --- a/vendor/github.com/containers/common/libimage/manifest_list.go +++ b/vendor/github.com/containers/common/libimage/manifest_list.go @@ -217,6 +217,11 @@ func (i *Image) getManifestList() (manifests.List, error) { // image index (OCI). This information may be critical to make certain // execution paths more robust (e.g., suppress certain errors). func (i *Image) IsManifestList(ctx context.Context) (bool, error) { + // FIXME: due to `ImageDigestBigDataKey` we'll always check the + // _last-written_ manifest which is causing issues for multi-arch image + // pulls. + // + // See https://github.com/containers/common/pull/1505#discussion_r1242677279. ref, err := i.StorageReference() if err != nil { return false, err diff --git a/vendor/github.com/containers/common/libimage/normalize.go b/vendor/github.com/containers/common/libimage/normalize.go index bb3cdbc7c0..9619b1a0d1 100644 --- a/vendor/github.com/containers/common/libimage/normalize.go +++ b/vendor/github.com/containers/common/libimage/normalize.go @@ -100,22 +100,22 @@ func ToNameTagPairs(repoTags []reference.Named) ([]NameTagPair, error) { // normalizeTaggedDigestedString strips the tag off the specified string iff it // is tagged and digested. Note that the tag is entirely ignored to match // Docker behavior. -func normalizeTaggedDigestedString(s string) (string, error) { +func normalizeTaggedDigestedString(s string) (string, reference.Named, error) { // Note that the input string is not expected to be parseable, so we // return it verbatim in error cases. ref, err := reference.Parse(s) if err != nil { - return "", err + return "", nil, err } named, ok := ref.(reference.Named) if !ok { - return s, nil + return s, nil, nil } named, err = normalizeTaggedDigestedNamed(named) if err != nil { - return "", err + return "", nil, err } - return named.String(), nil + return named.String(), named, nil } // normalizeTaggedDigestedNamed strips the tag off the specified named diff --git a/vendor/github.com/containers/common/libimage/pull.go b/vendor/github.com/containers/common/libimage/pull.go index 188ecb5eff..296d003a84 100644 --- a/vendor/github.com/containers/common/libimage/pull.go +++ b/vendor/github.com/containers/common/libimage/pull.go @@ -86,7 +86,7 @@ func (r *Runtime) Pull(ctx context.Context, name string, pullPolicy config.PullP // Docker compat: strip off the tag iff name is tagged and digested // (e.g., fedora:latest@sha256...). In that case, the tag is stripped // off and entirely ignored. The digest is the sole source of truth. - normalizedName, normalizeError := normalizeTaggedDigestedString(name) + normalizedName, _, normalizeError := normalizeTaggedDigestedString(name) if normalizeError != nil { return nil, normalizeError } diff --git a/vendor/github.com/containers/common/libimage/runtime.go b/vendor/github.com/containers/common/libimage/runtime.go index 95da83bb99..7707d2e3bd 100644 --- a/vendor/github.com/containers/common/libimage/runtime.go +++ b/vendor/github.com/containers/common/libimage/runtime.go @@ -16,6 +16,7 @@ import ( "github.com/containers/storage" deepcopy "github.com/jinzhu/copier" jsoniter "github.com/json-iterator/go" + "github.com/opencontainers/go-digest" "github.com/sirupsen/logrus" ) @@ -239,7 +240,7 @@ func (r *Runtime) LookupImage(name string, options *LookupImageOptions) (*Image, // Docker compat: strip off the tag iff name is tagged and digested // (e.g., fedora:latest@sha256...). In that case, the tag is stripped // off and entirely ignored. The digest is the sole source of truth. - normalizedName, err := normalizeTaggedDigestedString(name) + normalizedName, possiblyUnqualifiedNamedReference, err := normalizeTaggedDigestedString(name) if err != nil { return nil, "", err } @@ -259,7 +260,7 @@ func (r *Runtime) LookupImage(name string, options *LookupImageOptions) (*Image, // If the name clearly refers to a local image, try to look it up. if byFullID || byDigest { - img, err := r.lookupImageInLocalStorage(originalName, name, options) + img, err := r.lookupImageInLocalStorage(originalName, name, nil, options) if err != nil { return nil, "", err } @@ -297,7 +298,7 @@ func (r *Runtime) LookupImage(name string, options *LookupImageOptions) (*Image, } for _, candidate := range candidates { - img, err := r.lookupImageInLocalStorage(name, candidate.String(), options) + img, err := r.lookupImageInLocalStorage(name, candidate.String(), candidate, options) if err != nil { return nil, "", err } @@ -308,7 +309,7 @@ func (r *Runtime) LookupImage(name string, options *LookupImageOptions) (*Image, // The specified name may refer to a short ID. Note that this *must* // happen after the short-name expansion as done above. - img, err := r.lookupImageInLocalStorage(name, name, options) + img, err := r.lookupImageInLocalStorage(name, name, nil, options) if err != nil { return nil, "", err } @@ -316,21 +317,51 @@ func (r *Runtime) LookupImage(name string, options *LookupImageOptions) (*Image, return img, name, err } - return r.lookupImageInDigestsAndRepoTags(name, options) + return r.lookupImageInDigestsAndRepoTags(name, possiblyUnqualifiedNamedReference, options) } // lookupImageInLocalStorage looks up the specified candidate for name in the // storage and checks whether it's matching the system context. -func (r *Runtime) lookupImageInLocalStorage(name, candidate string, options *LookupImageOptions) (*Image, error) { +func (r *Runtime) lookupImageInLocalStorage(name, candidate string, namedCandidate reference.Named, options *LookupImageOptions) (*Image, error) { logrus.Debugf("Trying %q ...", candidate) - img, err := r.store.Image(candidate) - if err != nil && !errors.Is(err, storage.ErrImageUnknown) { - return nil, err - } - if img == nil { - return nil, nil + + var err error + var img *storage.Image + var ref types.ImageReference + + // FIXME: the lookup logic for manifest lists needs improvement. + // See https://github.com/containers/common/pull/1505#discussion_r1242677279 + // for details. + + // For images pulled by tag, Image.Names does not currently contain a + // repo@digest value, so such an input would not match directly in + // c/storage. + if namedCandidate != nil { + namedCandidate = reference.TagNameOnly(namedCandidate) + ref, err = storageTransport.Transport.NewStoreReference(r.store, namedCandidate, "") + if err != nil { + return nil, err + } + img, err = storageTransport.Transport.GetStoreImage(r.store, ref) + if err != nil { + if errors.Is(err, storage.ErrImageUnknown) { + return nil, nil + } + return nil, err + } + // NOTE: we must reparse the reference another time below since + // an ordinary image may have resolved into a per-platform image + // without any regard to options.{Architecture,OS,Variant}. + } else { + img, err = r.store.Image(candidate) + if err != nil { + if errors.Is(err, storage.ErrImageUnknown) { + return nil, nil + } + return nil, err + } } - ref, err := storageTransport.Transport.ParseStoreReference(r.store, img.ID) + ref, err = storageTransport.Transport.ParseStoreReference(r.store, img.ID) if err != nil { return nil, err } @@ -417,76 +448,71 @@ func (r *Runtime) lookupImageInLocalStorage(name, candidate string, options *Loo // lookupImageInDigestsAndRepoTags attempts to match name against any image in // the local containers storage. If name is digested, it will be compared // against image digests. Otherwise, it will be looked up in the repo tags. -func (r *Runtime) lookupImageInDigestsAndRepoTags(name string, options *LookupImageOptions) (*Image, string, error) { - // Until now, we've tried very hard to find an image but now it is time - // for limbo. If the image includes a digest that we couldn't detect - // verbatim in the storage, we must have a look at all digests of all - // images. Those may change over time (e.g., via manifest lists). - // Both Podman and Buildah want us to do that dance. - allImages, err := r.ListImages(context.Background(), nil, nil) - if err != nil { - return nil, "", err - } +func (r *Runtime) lookupImageInDigestsAndRepoTags(name string, possiblyUnqualifiedNamedReference reference.Named, options *LookupImageOptions) (*Image, string, error) { + originalName := name // we may change name below - ref, err := reference.Parse(name) // Warning! This is not ParseNormalizedNamed - if err != nil { - return nil, "", err - } - named, isNamed := ref.(reference.Named) - if !isNamed { - return nil, "", fmt.Errorf("%s: %w", name, storage.ErrImageUnknown) + if possiblyUnqualifiedNamedReference == nil { + return nil, "", fmt.Errorf("%s: %w", originalName, storage.ErrImageUnknown) } - digested, isDigested := named.(reference.Digested) + // In case of a digested reference, we strip off the digest and require + // any image matching the repo/tag to also match the specified digest. + var requiredDigest digest.Digest + digested, isDigested := possiblyUnqualifiedNamedReference.(reference.Digested) if isDigested { - logrus.Debug("Looking for image with matching recorded digests") - digest := digested.Digest() - for _, image := range allImages { - for _, d := range image.Digests() { - if d != digest { - continue - } - // Also make sure that the matching image fits all criteria (e.g., manifest list). - if _, err := r.lookupImageInLocalStorage(name, image.ID(), options); err != nil { - return nil, "", err - } - return image, name, nil - - } - } - return nil, "", fmt.Errorf("%s: %w", name, storage.ErrImageUnknown) + requiredDigest = digested.Digest() + possiblyUnqualifiedNamedReference = reference.TrimNamed(possiblyUnqualifiedNamedReference) + name = possiblyUnqualifiedNamedReference.String() } if !shortnames.IsShortName(name) { - return nil, "", fmt.Errorf("%s: %w", name, storage.ErrImageUnknown) + return nil, "", fmt.Errorf("%s: %w", originalName, storage.ErrImageUnknown) } - named = reference.TagNameOnly(named) // Make sure to add ":latest" if needed - namedTagged, isNammedTagged := named.(reference.NamedTagged) - if !isNammedTagged { - // NOTE: this should never happen since we already know it's - // not a digested reference. - return nil, "", fmt.Errorf("%s: %w (could not cast to tagged)", name, storage.ErrImageUnknown) + // Docker compat: make sure to add the "latest" tag if needed. The tag + // will be ignored if we're looking for a digest match. + possiblyUnqualifiedNamedReference = reference.TagNameOnly(possiblyUnqualifiedNamedReference) + namedTagged, isNamedTagged := possiblyUnqualifiedNamedReference.(reference.NamedTagged) + if !isNamedTagged { + // NOTE: this should never happen since we already stripped off + // the digest. + return nil, "", fmt.Errorf("%s: %w (could not cast to tagged)", originalName, storage.ErrImageUnknown) + } + + allImages, err := r.ListImages(context.Background(), nil, nil) + if err != nil { + return nil, "", err } for _, image := range allImages { - named, err := image.inRepoTags(namedTagged) + named, err := image.inRepoTags(namedTagged, isDigested) if err != nil { return nil, "", err } if named == nil { continue } - img, err := r.lookupImageInLocalStorage(name, named.String(), options) + img, err := r.lookupImageInLocalStorage(name, named.String(), named, options) if err != nil { return nil, "", err } if img != nil { - return img, named.String(), err + if isDigested { + if !img.hasDigest(requiredDigest.String()) { + continue + } + named = reference.TrimNamed(named) + canonical, err := reference.WithDigest(named, requiredDigest) + if err != nil { + return nil, "", fmt.Errorf("building canonical reference with digest %q and matched %q: %w", requiredDigest.String(), named.String(), err) + } + return img, canonical.String(), nil + } + return img, named.String(), nil } } - return nil, "", fmt.Errorf("%s: %w", name, storage.ErrImageUnknown) + return nil, "", fmt.Errorf("%s: %w", originalName, storage.ErrImageUnknown) } // ResolveName resolves the specified name. If the name resolves to a local diff --git a/vendor/github.com/containers/common/version/version.go b/vendor/github.com/containers/common/version/version.go index 9a370a898c..71d6aafa90 100644 --- a/vendor/github.com/containers/common/version/version.go +++ b/vendor/github.com/containers/common/version/version.go @@ -1,4 +1,4 @@ package version // Version is the version of the build. -const Version = "0.55.1" +const Version = "0.55.2" diff --git a/vendor/modules.txt b/vendor/modules.txt index 9eef7140bd..c05134f485 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -128,7 +128,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.55.1 +# github.com/containers/common v0.55.2 ## explicit; go 1.18 github.com/containers/common/libimage github.com/containers/common/libimage/define From ed56187594c4a2d24ca9e22ae1d80c598a61e32f Mon Sep 17 00:00:00 2001 From: danishprakash Date: Tue, 6 Jun 2023 21:31:21 +0530 Subject: [PATCH 02/34] play.go: remove volumes on down -f * add e2e test Signed-off-by: danishprakash --- pkg/domain/infra/abi/play.go | 11 +++++++++++ test/e2e/play_kube_test.go | 13 +++++++++++++ 2 files changed, 24 insertions(+) diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index c19178b68e..7e2570fc5d 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -1343,6 +1343,17 @@ func (ic *ContainerEngine) PlayKubeDown(ctx context.Context, body io.Reader, opt return nil, fmt.Errorf("unable to read YAML as Kube Pod: %w", err) } podNames = append(podNames, podYAML.ObjectMeta.Name) + + for _, vol := range podYAML.Spec.Volumes { + switch vs := vol.VolumeSource; { + case vs.PersistentVolumeClaim != nil: + volumeNames = append(volumeNames, vs.PersistentVolumeClaim.ClaimName) + case vs.ConfigMap != nil: + volumeNames = append(volumeNames, vs.ConfigMap.Name) + case vs.Secret != nil: + volumeNames = append(volumeNames, vs.Secret.SecretName) + } + } case "Deployment": var deploymentYAML v1apps.Deployment diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go index 990ee0f342..c6991e27d4 100644 --- a/test/e2e/play_kube_test.go +++ b/test/e2e/play_kube_test.go @@ -4984,6 +4984,19 @@ spec: exec.WaitWithDefaultTimeout() Expect(exec).Should(Exit(0)) Expect(exec.OutputToString()).Should(Equal("hi")) + + teardown := podmanTest.Podman([]string{"kube", "down", "--force", kubeYaml}) + teardown.WaitWithDefaultTimeout() + Expect(teardown).Should(Exit(0)) + Expect(teardown.OutputToString()).Should(ContainSubstring("testvol")) + + // kube down --force should remove volumes + // specified in the manifest but not externally + // created volumes, testvol1 in this case + checkVol := podmanTest.Podman([]string{"volume", "ls", "--format", "{{.Name}}"}) + checkVol.WaitWithDefaultTimeout() + Expect(checkVol).Should(Exit(0)) + Expect(checkVol.OutputToString()).To(Equal("testvol1")) }) It("podman play kube with hostPath subpaths", func() { From 62fc35c0702bf1411f0760f25b2214607aaff2d9 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Fri, 23 Jun 2023 07:55:12 -0400 Subject: [PATCH 03/34] Trim whitespace from unit files while parsing Fixes: https://github.com/containers/podman/issues/18979 Signed-off-by: Daniel J Walsh --- .pre-commit-config.yaml | 2 +- Makefile | 2 +- pkg/systemd/parser/unitfile.go | 2 +- test/e2e/quadlet/remap-keep-id2.container | 3 ++- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ca023d18a1..0f31ebc5fe 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -12,7 +12,7 @@ repos: - id: end-of-file-fixer exclude: test/buildah-bud/buildah-tests.diff - id: trailing-whitespace - exclude: test/buildah-bud/buildah-tests.diff + exclude: test/buildah-bud/buildah-tests.diff|test/e2e/quadlet/remap-keep-id2.container - id: mixed-line-ending - id: check-byte-order-marker - id: check-executables-have-shebangs diff --git a/Makefile b/Makefile index e42e453215..badca1e6df 100644 --- a/Makefile +++ b/Makefile @@ -253,7 +253,7 @@ help: ## (Default) Print listing of key targets with their descriptions .PHONY: .gitvalidation .gitvalidation: @echo "Validating vs commit '$(call err_if_empty,EPOCH_TEST_COMMIT)'" - GIT_CHECK_EXCLUDE="./vendor:./test/tools/vendor:docs/make.bat:test/buildah-bud/buildah-tests.diff" ./test/tools/build/git-validation -run DCO,short-subject,dangling-whitespace -range $(EPOCH_TEST_COMMIT)..$(HEAD) + GIT_CHECK_EXCLUDE="./vendor:./test/tools/vendor:docs/make.bat:test/buildah-bud/buildah-tests.diff:test/e2e/quadlet/remap-keep-id2.container" ./test/tools/build/git-validation -run DCO,short-subject,dangling-whitespace -range $(EPOCH_TEST_COMMIT)..$(HEAD) .PHONY: lint lint: golangci-lint diff --git a/pkg/systemd/parser/unitfile.go b/pkg/systemd/parser/unitfile.go index 56fa1888fb..963909f9d8 100644 --- a/pkg/systemd/parser/unitfile.go +++ b/pkg/systemd/parser/unitfile.go @@ -373,7 +373,7 @@ func (p *UnitFileParser) flushPendingComments(toComment bool) { func nextLine(data string, afterPos int) (string, string) { rest := data[afterPos:] if i := strings.Index(rest, "\n"); i >= 0 { - return data[:i+afterPos], data[i+afterPos+1:] + return strings.TrimSpace(data[:i+afterPos]), data[i+afterPos+1:] } return data, "" } diff --git a/test/e2e/quadlet/remap-keep-id2.container b/test/e2e/quadlet/remap-keep-id2.container index e382f65418..e35af2ba29 100644 --- a/test/e2e/quadlet/remap-keep-id2.container +++ b/test/e2e/quadlet/remap-keep-id2.container @@ -2,6 +2,7 @@ [Container] Image=localhost/imagename -RemapUsers=keep-id +# The added three spaces to keep-id are necessary for test of trimwhitespace +RemapUsers=keep-id RemapUid=200 RemapGid=210 From f701899918065ef1ae9451fb6129d18ffe44925c Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 23 Jun 2023 13:43:36 +0200 Subject: [PATCH 04/34] make image listing more resilient Handle more TOCTOUs operating on listed images. Also pull in containers/common/pull/1520 and containers/common/pull/1522 which do the same on the internal layer tree. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2216700 Signed-off-by: Valentin Rothberg --- libpod/container_internal.go | 4 +- pkg/domain/infra/abi/images_list.go | 93 ++++++++++++++++------------- test/system/010-images.bats | 36 +++++++++++ 3 files changed, 89 insertions(+), 44 deletions(-) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 556ab86b52..441642a0f3 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -2101,7 +2101,7 @@ func (c *Container) postDeleteHooks(ctx context.Context) error { Stderr: &stderr, PostKillTimeout: exec.DefaultPostKillTimeout, } - hookErr, err := exec.RunWithOptions(ctx, opts) + hookErr, err := exec.RunWithOptions(ctx, opts) //nolint:staticcheck if err != nil { logrus.Warnf("Container %s: poststop hook %d: %v", c.ID(), i, err) if hookErr != err { @@ -2234,7 +2234,7 @@ func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (map[s Config: config, PostKillTimeout: exec.DefaultPostKillTimeout, } - hookErr, err := exec.RuntimeConfigFilterWithOptions(ctx, opts) + hookErr, err := exec.RuntimeConfigFilterWithOptions(ctx, opts) //nolint:staticcheck if err != nil { logrus.Warnf("Container %s: precreate hook: %v", c.ID(), err) if hookErr != nil && hookErr != err { diff --git a/pkg/domain/infra/abi/images_list.go b/pkg/domain/infra/abi/images_list.go index d9661721a3..b980dcc001 100644 --- a/pkg/domain/infra/abi/images_list.go +++ b/pkg/domain/infra/abi/images_list.go @@ -28,54 +28,63 @@ func (ir *ImageEngine) List(ctx context.Context, opts entities.ImageListOptions) summaries := []*entities.ImageSummary{} for _, img := range images { - repoDigests, err := img.RepoDigests() - if err != nil { - return nil, fmt.Errorf("getting repoDigests from image %q: %w", img.ID(), err) - } + summary, err := func() (*entities.ImageSummary, error) { + repoDigests, err := img.RepoDigests() + if err != nil { + return nil, fmt.Errorf("getting repoDigests from image %q: %w", img.ID(), err) + } - if img.ListData.IsDangling == nil { // Sanity check - return nil, fmt.Errorf("%w: ListData.IsDangling is nil but should not", define.ErrInternal) - } - isDangling := *img.ListData.IsDangling - parentID := "" - if img.ListData.Parent != nil { - parentID = img.ListData.Parent.ID() - } + if img.ListData.IsDangling == nil { // Sanity check + return nil, fmt.Errorf("%w: ListData.IsDangling is nil but should not be", define.ErrInternal) + } + isDangling := *img.ListData.IsDangling + parentID := "" + if img.ListData.Parent != nil { + parentID = img.ListData.Parent.ID() + } - e := entities.ImageSummary{ - ID: img.ID(), - Created: img.Created().Unix(), - Dangling: isDangling, - Digest: string(img.Digest()), - RepoDigests: repoDigests, - History: img.NamesHistory(), - Names: img.Names(), - ReadOnly: img.IsReadOnly(), - SharedSize: 0, - RepoTags: img.Names(), // may include tags and digests - ParentId: parentID, - } - e.Labels, err = img.Labels(ctx) - if err != nil { - return nil, fmt.Errorf("retrieving label for image %q: you may need to remove the image to resolve the error: %w", img.ID(), err) - } + s := &entities.ImageSummary{ + ID: img.ID(), + Created: img.Created().Unix(), + Dangling: isDangling, + Digest: string(img.Digest()), + RepoDigests: repoDigests, + History: img.NamesHistory(), + Names: img.Names(), + ReadOnly: img.IsReadOnly(), + SharedSize: 0, + RepoTags: img.Names(), // may include tags and digests + ParentId: parentID, + } + s.Labels, err = img.Labels(ctx) + if err != nil { + return nil, fmt.Errorf("retrieving label for image %q: you may need to remove the image to resolve the error: %w", img.ID(), err) + } - ctnrs, err := img.Containers() - if err != nil { - return nil, fmt.Errorf("retrieving containers for image %q: you may need to remove the image to resolve the error: %w", img.ID(), err) - } - e.Containers = len(ctnrs) + ctnrs, err := img.Containers() + if err != nil { + return nil, fmt.Errorf("retrieving containers for image %q: you may need to remove the image to resolve the error: %w", img.ID(), err) + } + s.Containers = len(ctnrs) - sz, err := img.Size() + sz, err := img.Size() + if err != nil { + return nil, fmt.Errorf("retrieving size of image %q: you may need to remove the image to resolve the error: %w", img.ID(), err) + } + s.Size = sz + // This is good enough for now, but has to be + // replaced later with correct calculation logic + s.VirtualSize = sz + return s, nil + }() if err != nil { - return nil, fmt.Errorf("retrieving size of image %q: you may need to remove the image to resolve the error: %w", img.ID(), err) + if libimage.ErrorIsImageUnknown(err) { + // The image may have been (partially) removed in the meantime + continue + } + return nil, err } - e.Size = sz - // This is good enough for now, but has to be - // replaced later with correct calculation logic - e.VirtualSize = sz - - summaries = append(summaries, &e) + summaries = append(summaries, summary) } return summaries, nil } diff --git a/test/system/010-images.bats b/test/system/010-images.bats index 78b83a0e69..4c59bcbc93 100644 --- a/test/system/010-images.bats +++ b/test/system/010-images.bats @@ -363,4 +363,40 @@ EOF run_podman --root $imstore/root rmi --all } +@test "podman images with concurrent removal" { + skip_if_remote "following test is not supported for remote clients" + local count=5 + + # First build $count images + for i in $(seq --format '%02g' 1 $count); do + cat >$PODMAN_TMPDIR/Containerfile < Date: Mon, 26 Jun 2023 17:09:55 +0200 Subject: [PATCH 05/34] specgen, rootless: raise error with --device-cgroup-rule we were silently ignoring --device-cgroup-rule in rootless mode. Make sure an error is returned if the user tries to use it. Closes: https://github.com/containers/podman/issues/18698 Signed-off-by: Giuseppe Scrivano --- pkg/specgen/generate/oci_linux.go | 3 +++ test/system/030-run.bats | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/specgen/generate/oci_linux.go b/pkg/specgen/generate/oci_linux.go index ddba5f7175..3d210bffdf 100644 --- a/pkg/specgen/generate/oci_linux.go +++ b/pkg/specgen/generate/oci_linux.go @@ -255,6 +255,9 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt s.HostDeviceList = userDevices // set the devices cgroup when not running in a user namespace + if isRootless && len(s.DeviceCgroupRule) > 0 { + return nil, fmt.Errorf("device cgroup rules are not supported in rootless mode or in a user namespace") + } if !inUserNS && !s.Privileged { for _, dev := range s.DeviceCgroupRule { g.AddLinuxResourcesDevice(true, dev.Type, dev.Major, dev.Minor, dev.Access) diff --git a/test/system/030-run.bats b/test/system/030-run.bats index 85ff2b2b71..463d7fd255 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -785,7 +785,11 @@ EOF } @test "podman run --device-cgroup-rule tests" { - skip_if_rootless "cannot add devices in rootless mode" + if is_rootless; then + run_podman 125 run --device-cgroup-rule="b 7:* rmw" --rm $IMAGE + is "$output" "Error: device cgroup rules are not supported in rootless mode or in a user namespace" + return + fi run_podman run --device-cgroup-rule="b 7:* rmw" --rm $IMAGE run_podman run --device-cgroup-rule="c 7:* rmw" --rm $IMAGE From f654f7cc7b1a0fbbdeb9cbbfe80414b840e21956 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Mon, 26 Jun 2023 17:21:44 +0200 Subject: [PATCH 06/34] specgen: honor --device-cgroup-rule with a new user namespace Signed-off-by: Giuseppe Scrivano --- pkg/specgen/generate/oci_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/specgen/generate/oci_linux.go b/pkg/specgen/generate/oci_linux.go index 3d210bffdf..7b1090a9bc 100644 --- a/pkg/specgen/generate/oci_linux.go +++ b/pkg/specgen/generate/oci_linux.go @@ -258,7 +258,7 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt if isRootless && len(s.DeviceCgroupRule) > 0 { return nil, fmt.Errorf("device cgroup rules are not supported in rootless mode or in a user namespace") } - if !inUserNS && !s.Privileged { + if !isRootless && !s.Privileged { for _, dev := range s.DeviceCgroupRule { g.AddLinuxResourcesDevice(true, dev.Type, dev.Major, dev.Minor, dev.Access) } From 47e6ce19cd7c3ac457e141479ba5ab192de0aa24 Mon Sep 17 00:00:00 2001 From: Fang-Pen Lin Date: Fri, 16 Jun 2023 13:55:00 -0700 Subject: [PATCH 07/34] Pass in correct cwd value for hooks exe Signed-off-by: Fang-Pen Lin --- libpod/container_internal.go | 35 ++++++++++++++++++------------- libpod/container_internal_test.go | 11 ++++++++++ 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 441642a0f3..3188f1b40b 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -2094,14 +2094,17 @@ func (c *Container) postDeleteHooks(ctx context.Context) error { hook := hook logrus.Debugf("container %s: invoke poststop hook %d, path %s", c.ID(), i, hook.Path) var stderr, stdout bytes.Buffer - opts := exec.RunOptions{ - Hook: &hook, - State: state, - Stdout: &stdout, - Stderr: &stderr, - PostKillTimeout: exec.DefaultPostKillTimeout, - } - hookErr, err := exec.RunWithOptions(ctx, opts) //nolint:staticcheck + hookErr, err := exec.RunWithOptions( + ctx, + exec.RunOptions{ + Hook: &hook, + Dir: c.bundlePath(), + State: state, + Stdout: &stdout, + Stderr: &stderr, + PostKillTimeout: exec.DefaultPostKillTimeout, + }, + ) if err != nil { logrus.Warnf("Container %s: poststop hook %d: %v", c.ID(), i, err) if hookErr != err { @@ -2229,12 +2232,16 @@ func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (map[s return nil, err } } - opts := exec.RuntimeConfigFilterOptions{ - Hooks: allHooks["precreate"], - Config: config, - PostKillTimeout: exec.DefaultPostKillTimeout, - } - hookErr, err := exec.RuntimeConfigFilterWithOptions(ctx, opts) //nolint:staticcheck + + hookErr, err := exec.RuntimeConfigFilterWithOptions( + ctx, + exec.RuntimeConfigFilterOptions{ + Hooks: allHooks["precreate"], + Dir: c.bundlePath(), + Config: config, + PostKillTimeout: exec.DefaultPostKillTimeout, + }, + ) if err != nil { logrus.Warnf("Container %s: precreate hook: %v", c.ID(), err) if hookErr != nil && hookErr != err { diff --git a/libpod/container_internal_test.go b/libpod/container_internal_test.go index b1e2bc1483..1c096d1d16 100644 --- a/libpod/container_internal_test.go +++ b/libpod/container_internal_test.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "testing" "github.com/containers/storage/pkg/idtools" @@ -142,6 +143,7 @@ func TestPostDeleteHooks(t *testing.T) { statePath := filepath.Join(dir, "state") copyPath := filepath.Join(dir, "copy") + cwdPath := filepath.Join(dir, "cwd") c := Container{ runtime: &Runtime{}, config: &ContainerConfig{ @@ -169,6 +171,10 @@ func TestPostDeleteHooks(t *testing.T) { Path: hookPath, Args: []string{"sh", "-c", fmt.Sprintf("cp %s %s", statePath, copyPath)}, }, + rspec.Hook{ + Path: hookPath, + Args: []string{"sh", "-c", fmt.Sprintf("pwd >%s", cwdPath)}, + }, }, }, }, @@ -188,6 +194,11 @@ func TestPostDeleteHooks(t *testing.T) { assert.Regexp(t, stateRegexp, string(content)) }) } + content, err := os.ReadFile(cwdPath) + if err != nil { + t.Fatal(err) + } + assert.Equal(t, strings.TrimSuffix(string(content), "\n"), dir) } func init() { From e30197e6f82f1db3845f02a751514ef3cfdf9d5f Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Fri, 23 Jun 2023 11:33:01 -0400 Subject: [PATCH 08/34] Fix up podmansh man page Signed-off-by: Daniel J Walsh --- docs/source/markdown/podmansh.1.md | 38 ++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/docs/source/markdown/podmansh.1.md b/docs/source/markdown/podmansh.1.md index 8a4b9b03df..0fe043325a 100644 --- a/docs/source/markdown/podmansh.1.md +++ b/docs/source/markdown/podmansh.1.md @@ -18,23 +18,27 @@ The user is confined to the container environment via all of the security mechan Systemd will automatically create the container when the user session is started. Systemd will take down the container when all connections to the user session are removed. This means users can log in to the system multiple times, with each session connected to the same container. +Administrators can use volumes to expose specific host data from the host system to the user, without the user being exposed to other parts of the system. + NOTE: This feature is currently a Tech Preview. Changes can be expected in upcoming versions. ## Setup -Modify user login session using usermod +Create user login session using useradd while running as root. ``` -# usermod -s /usr/bin/podmansh testu -# grep testu /etc/passwd -testu:x:4004:4004::/home/testu:/usr/bin/podmansh +# useradd -s /usr/bin/podmansh lockedu +# grep lockedu /etc/passwd +lockedu:x:4008:4008::/home/lockedu:/usr/bin/podmansh ``` -Now create a podman Quadlet file that looks something like one of the following. +Create a Podman Quadlet file that looks something like one of the following. Fully locked down container, no access to host OS. ``` -sudo cat > /etc/containers/systemd/users/podmansh.container << _EOF +# UID=$(id -u lockedu) +# mkdir -p /etc/containers/systemd/users/${UID} +# cat > /etc/containers/systemd/users/${UID}/podmansh.container << _EOF [Unit] Description=The podmansh container After=local-fs.target @@ -44,7 +48,7 @@ Image=registry.fedoraproject.org/fedora ContainerName=podmansh RemapUsers=keep-id RunInit=yes -DropCapabilities=all +DropCapability=all NoNewPrivileges=true Exec=sleep infinity @@ -54,13 +58,19 @@ RequiredBy=default.target _EOF ``` -Users inside of this Quadlet are allowed to become root within the user namespace, and able to read/write content in their homedirectory which is mounted from a subdir `data` of the hosts users account. +Alternatively, while running as root, create a Quadlet where the user is allowed to become root within the user namespace. They can also permanently read/write content from their home directory which is volume mounted from the actual host's users account, rather than being inside of the container. ``` -sudo cat > /etc/containers/systemd/users/podmansh.container << _EOF +# useradd -s /usr/bin/podmansh confinedu +# grep confinedu /etc/passwd +confinedu:x:4009:4009::/home/confinedu:/usr/bin/podmansh +# UID=$(id -u confinedu) +# mkdir -p /etc/containers/systemd/users/${UID} +# cat > /etc/containers/systemd/users/${UID}/podmansh.container << _EOF [Unit] Description=The podmansh container After=local-fs.target +ExecStartPre=-/bin/mkdir -p %h/data [Container] Image=registry.fedoraproject.org/fedora @@ -79,11 +89,15 @@ RequiredBy=default.target _EOF ``` -Users inside this container will be allowed to execute containers with SELinux -separate and able to read and write content in the $HOME/data directory. +Another example, while running as root, create a Quadlet where the users inside this container are allowed to execute containers with SELinux separation and able to read and write content in the $HOME/data directory. ``` -sudo cat > /etc/containers/systemd/users/podmansh.container << _EOF +# useradd -s /usr/bin/podmansh fullu +# grep fullu /etc/passwd +fullu:x:4010:4010::/home/fullu:/usr/bin/podmansh +# UID=$(id -u fullu) +# mkdir -p /etc/containers/systemd/users/${UID} +# cat > /etc/containers/systemd/users/${UID}/podmansh.container << _EOF [Unit] Description=The podmansh container After=local-fs.target From a3598ff617b3f13a0e968bd62162b024366b5754 Mon Sep 17 00:00:00 2001 From: Doug Rabson Date: Tue, 29 Nov 2022 15:53:34 +0000 Subject: [PATCH 09/34] pkg/specgen: Add support for Linux emulation on FreeBSD This is limited to images that don't depend on complex cgroup or capability setups but does cover enough functionality to be useful. [NO NEW TESTS NEEDED] Signed-off-by: Doug Rabson --- pkg/specgen/generate/oci_freebsd.go | 52 ++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/pkg/specgen/generate/oci_freebsd.go b/pkg/specgen/generate/oci_freebsd.go index 56245b5796..282165f3f1 100644 --- a/pkg/specgen/generate/oci_freebsd.go +++ b/pkg/specgen/generate/oci_freebsd.go @@ -4,6 +4,7 @@ package generate import ( "context" + "fmt" "strings" "github.com/containers/common/libimage" @@ -17,7 +18,11 @@ import ( // SpecGenToOCI returns the base configuration for the container. func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runtime, rtc *config.Config, newImage *libimage.Image, mounts []spec.Mount, pod *libpod.Pod, finalCmd []string, compatibleOptions *libpod.InfraInherit) (*spec.Spec, error) { - g, err := generate.New("freebsd") + if s.ImageOS != "freebsd" && s.ImageOS != "linux" { + return nil, fmt.Errorf("unsupported image OS: %s", s.ImageOS) + } + + g, err := generate.New(s.ImageOS) if err != nil { return nil, err } @@ -49,6 +54,51 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt return nil, err } + // Linux emulatioon + if s.ImageOS == "linux" { + var mounts []spec.Mount + for _, m := range configSpec.Mounts { + switch m.Destination { + case "/proc": + m.Type = "linprocfs" + m.Options = []string{"nodev"} + mounts = append(mounts, m) + continue + case "/sys": + m.Type = "linsysfs" + m.Options = []string{"nodev"} + mounts = append(mounts, m) + continue + case "/dev", "/dev/pts", "/dev/shm", "/dev/mqueue": + continue + } + } + mounts = append(mounts, + spec.Mount{ + Destination: "/dev", + Type: "devfs", + Source: "devfs", + Options: []string{ + "ruleset=4", + "rule=path shm unhide mode 1777", + }, + }, + spec.Mount{ + Destination: "/dev/fd", + Type: "fdescfs", + Source: "fdesc", + Options: []string{}, + }, + spec.Mount{ + Destination: "/dev/shm", + Type: "tmpfs", + Source: "shm", + Options: []string{"notmpcopyup"}, + }, + ) + configSpec.Mounts = mounts + } + // BIND MOUNTS configSpec.Mounts = SupersedeUserMounts(mounts, configSpec.Mounts) // Process mounts to ensure correct options From ee6329374dc7340f44ffbbc39d7cf5d6c6561e87 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Tue, 27 Jun 2023 08:40:10 -0400 Subject: [PATCH 10/34] Fix readonly=false failure There was a huge cut and paste of mount options which were not constent in parsing tmpfs, bind and volume mounts. Consolidated into a single function to guarantee all parse the same. Fixes: https://github.com/containers/podman/issues/18995 Signed-off-by: Daniel J Walsh --- pkg/specgenutil/volumes.go | 365 +++++++++++++++---------------------- test/system/060-mount.bats | 16 ++ 2 files changed, 158 insertions(+), 223 deletions(-) diff --git a/pkg/specgenutil/volumes.go b/pkg/specgenutil/volumes.go index c70206addf..c77710ffea 100644 --- a/pkg/specgenutil/volumes.go +++ b/pkg/specgenutil/volumes.go @@ -236,22 +236,39 @@ func Mounts(mountFlag []string) (map[string]spec.Mount, map[string]*specgen.Name return finalMounts, finalNamedVolumes, finalImageVolumes, nil } -// Parse a single bind mount entry from the --mount flag. -func getBindMount(args []string) (spec.Mount, error) { - newMount := spec.Mount{ - Type: define.TypeBind, - } - - var setSource, setDest, setRORW, setSuid, setDev, setExec, setRelabel, setOwnership bool +func parseMountOptions(mountType string, args []string) (*spec.Mount, error) { + var setTmpcopyup, setRORW, setSuid, setDev, setExec, setRelabel, setOwnership bool + mnt := spec.Mount{} for _, val := range args { kv := strings.SplitN(val, "=", 2) switch kv[0] { case "bind-nonrecursive": - newMount.Options = append(newMount.Options, "bind") + if mountType != define.TypeBind { + return nil, fmt.Errorf("%q option not supported for %q mount types", kv[0], mountType) + } + mnt.Options = append(mnt.Options, "bind") + case "bind-propagation": + if mountType != define.TypeBind { + return nil, fmt.Errorf("%q option not supported for %q mount types", kv[0], mountType) + } + if len(kv) == 1 { + return nil, fmt.Errorf("%v: %w", kv[0], errOptionArg) + } + mnt.Options = append(mnt.Options, kv[1]) + case "consistency": + // Often used on MACs and mistakenly on Linux platforms. + // Since Docker ignores this option so shall we. + continue + case "idmap": + if len(kv) > 1 { + mnt.Options = append(mnt.Options, fmt.Sprintf("idmap=%s", kv[1])) + } else { + mnt.Options = append(mnt.Options, "idmap") + } case "readonly", "ro", "rw": if setRORW { - return newMount, fmt.Errorf("cannot pass 'readonly', 'ro', or 'rw' options more than once: %w", errOptionArg) + return nil, fmt.Errorf("cannot pass 'readonly', 'ro', or 'rw' mnt.Options more than once: %w", errOptionArg) } setRORW = true // Can be formatted as one of: @@ -266,121 +283,162 @@ func getBindMount(args []string) (spec.Mount, error) { } switch len(kv) { case 1: - newMount.Options = append(newMount.Options, kv[0]) + mnt.Options = append(mnt.Options, kv[0]) case 2: switch strings.ToLower(kv[1]) { case "true": - newMount.Options = append(newMount.Options, kv[0]) + mnt.Options = append(mnt.Options, kv[0]) case "false": // Set the opposite only for rw // ro's opposite is the default if kv[0] == "rw" { - newMount.Options = append(newMount.Options, "ro") + mnt.Options = append(mnt.Options, "ro") } - default: - return newMount, fmt.Errorf("'readonly', 'ro', or 'rw' must be set to true or false, instead received %q: %w", kv[1], errOptionArg) } - default: - return newMount, fmt.Errorf("badly formatted option %q: %w", val, errOptionArg) } - case "nosuid", "suid": - if setSuid { - return newMount, fmt.Errorf("cannot pass 'nosuid' and 'suid' options more than once: %w", errOptionArg) - } - setSuid = true - newMount.Options = append(newMount.Options, kv[0]) case "nodev", "dev": if setDev { - return newMount, fmt.Errorf("cannot pass 'nodev' and 'dev' options more than once: %w", errOptionArg) + return nil, fmt.Errorf("cannot pass 'nodev' and 'dev' mnt.Options more than once: %w", errOptionArg) } setDev = true - newMount.Options = append(newMount.Options, kv[0]) + mnt.Options = append(mnt.Options, kv[0]) case "noexec", "exec": if setExec { - return newMount, fmt.Errorf("cannot pass 'noexec' and 'exec' options more than once: %w", errOptionArg) + return nil, fmt.Errorf("cannot pass 'noexec' and 'exec' mnt.Options more than once: %w", errOptionArg) } setExec = true - newMount.Options = append(newMount.Options, kv[0]) - case "shared", "rshared", "private", "rprivate", "slave", "rslave", "unbindable", "runbindable", "Z", "z": - newMount.Options = append(newMount.Options, kv[0]) - case "bind-propagation": - if len(kv) == 1 { - return newMount, fmt.Errorf("%v: %w", kv[0], errOptionArg) + mnt.Options = append(mnt.Options, kv[0]) + case "nosuid", "suid": + if setSuid { + return nil, fmt.Errorf("cannot pass 'nosuid' and 'suid' mnt.Options more than once: %w", errOptionArg) + } + setSuid = true + mnt.Options = append(mnt.Options, kv[0]) + case "relabel": + if setRelabel { + return nil, fmt.Errorf("cannot pass 'relabel' option more than once: %w", errOptionArg) + } + setRelabel = true + if len(kv) != 2 { + return nil, fmt.Errorf("%s mount option must be 'private' or 'shared': %w", kv[0], util.ErrBadMntOption) + } + switch kv[1] { + case "private": + mnt.Options = append(mnt.Options, "Z") + case "shared": + mnt.Options = append(mnt.Options, "z") + default: + return nil, fmt.Errorf("%s mount option must be 'private' or 'shared': %w", kv[0], util.ErrBadMntOption) } - newMount.Options = append(newMount.Options, kv[1]) + case "shared", "rshared", "private", "rprivate", "slave", "rslave", "unbindable", "runbindable", "Z", "z": + mnt.Options = append(mnt.Options, kv[0]) case "src", "source": + if mountType == define.TypeTmpfs { + return nil, fmt.Errorf("%q option not supported for %q mount types", kv[0], mountType) + } + if mnt.Source != "" { + return nil, fmt.Errorf("cannot pass %q option more than once: %w", kv[0], errOptionArg) + } if len(kv) == 1 { - return newMount, fmt.Errorf("%v: %w", kv[0], errOptionArg) + return nil, fmt.Errorf("%v: %w", kv[0], errOptionArg) } if len(kv[1]) == 0 { - return newMount, fmt.Errorf("host directory cannot be empty: %w", errOptionArg) + return nil, fmt.Errorf("host directory cannot be empty: %w", errOptionArg) } - newMount.Source = kv[1] - setSource = true + mnt.Source = kv[1] case "target", "dst", "destination": + if mnt.Destination != "" { + return nil, fmt.Errorf("cannot pass %q option more than once: %w", kv[0], errOptionArg) + } if len(kv) == 1 { - return newMount, fmt.Errorf("%v: %w", kv[0], errOptionArg) + return nil, fmt.Errorf("%v: %w", kv[0], errOptionArg) } if err := parse.ValidateVolumeCtrDir(kv[1]); err != nil { - return newMount, err + return nil, err } - newMount.Destination = unixPathClean(kv[1]) - setDest = true - case "relabel": - if setRelabel { - return newMount, fmt.Errorf("cannot pass 'relabel' option more than once: %w", errOptionArg) + mnt.Destination = unixPathClean(kv[1]) + case "tmpcopyup", "notmpcopyup": + if mountType != define.TypeTmpfs { + return nil, fmt.Errorf("%q option not supported for %q mount types", kv[0], mountType) } - setRelabel = true - if len(kv) != 2 { - return newMount, fmt.Errorf("%s mount option must be 'private' or 'shared': %w", kv[0], util.ErrBadMntOption) + if setTmpcopyup { + return nil, fmt.Errorf("cannot pass 'tmpcopyup' and 'notmpcopyup' mnt.Options more than once: %w", errOptionArg) } - switch kv[1] { - case "private": - newMount.Options = append(newMount.Options, "Z") - case "shared": - newMount.Options = append(newMount.Options, "z") - default: - return newMount, fmt.Errorf("%s mount option must be 'private' or 'shared': %w", kv[0], util.ErrBadMntOption) + setTmpcopyup = true + mnt.Options = append(mnt.Options, kv[0]) + case "tmpfs-mode": + if mountType != define.TypeTmpfs { + return nil, fmt.Errorf("%q option not supported for %q mount types", kv[0], mountType) + } + if len(kv) == 1 { + return nil, fmt.Errorf("%v: %w", kv[0], errOptionArg) } + mnt.Options = append(mnt.Options, fmt.Sprintf("mode=%s", kv[1])) + case "tmpfs-size": + if mountType != define.TypeTmpfs { + return nil, fmt.Errorf("%q option not supported for %q mount types", kv[0], mountType) + } + if len(kv) == 1 { + return nil, fmt.Errorf("%v: %w", kv[0], errOptionArg) + } + mnt.Options = append(mnt.Options, fmt.Sprintf("size=%s", kv[1])) case "U", "chown": if setOwnership { - return newMount, fmt.Errorf("cannot pass 'U' or 'chown' option more than once: %w", errOptionArg) + return nil, fmt.Errorf("cannot pass 'U' or 'chown' option more than once: %w", errOptionArg) } ok, err := validChownFlag(val) if err != nil { - return newMount, err + return nil, err } if ok { - newMount.Options = append(newMount.Options, "U") + mnt.Options = append(mnt.Options, "U") } setOwnership = true - case "idmap": - if len(kv) > 1 { - newMount.Options = append(newMount.Options, fmt.Sprintf("idmap=%s", kv[1])) - } else { - newMount.Options = append(newMount.Options, "idmap") + case "volume-label": + if mountType != define.TypeVolume { + return nil, fmt.Errorf("%q option not supported for %q mount types", kv[0], mountType) } - case "consistency": - // Often used on MACs and mistakenly on Linux platforms. - // Since Docker ignores this option so shall we. - continue + return nil, fmt.Errorf("the --volume-label option is not presently implemented") + case "volume-opt": + if mountType != define.TypeVolume { + return nil, fmt.Errorf("%q option not supported for %q mount types", kv[0], mountType) + } + mnt.Options = append(mnt.Options, val) default: - return newMount, fmt.Errorf("%s: %w", kv[0], util.ErrBadMntOption) + return nil, fmt.Errorf("%s: %w", kv[0], util.ErrBadMntOption) } } + if len(mnt.Destination) == 0 { + return nil, errNoDest + } + return &mnt, nil +} - if !setDest { +// Parse a single bind mount entry from the --mount flag. +func getBindMount(args []string) (spec.Mount, error) { + newMount := spec.Mount{ + Type: define.TypeBind, + } + var err error + mnt, err := parseMountOptions(newMount.Type, args) + if err != nil { + return newMount, err + } + + if len(mnt.Destination) == 0 { return newMount, errNoDest } - if !setSource { - newMount.Source = newMount.Destination + if len(mnt.Source) == 0 { + mnt.Source = mnt.Destination } - options, err := parse.ValidateVolumeOpts(newMount.Options) + options, err := parse.ValidateVolumeOpts(mnt.Options) if err != nil { return newMount, err } + newMount.Source = mnt.Source + newMount.Destination = mnt.Destination newMount.Options = options return newMount, nil } @@ -392,87 +450,16 @@ func getTmpfsMount(args []string) (spec.Mount, error) { Source: define.TypeTmpfs, } - var setDest, setRORW, setSuid, setDev, setExec, setTmpcopyup, setOwnership bool - - for _, val := range args { - kv := strings.SplitN(val, "=", 2) - switch kv[0] { - case "tmpcopyup", "notmpcopyup": - if setTmpcopyup { - return newMount, fmt.Errorf("cannot pass 'tmpcopyup' and 'notmpcopyup' options more than once: %w", errOptionArg) - } - setTmpcopyup = true - newMount.Options = append(newMount.Options, kv[0]) - case "ro", "rw": - if setRORW { - return newMount, fmt.Errorf("cannot pass 'ro' and 'rw' options more than once: %w", errOptionArg) - } - setRORW = true - newMount.Options = append(newMount.Options, kv[0]) - case "nosuid", "suid": - if setSuid { - return newMount, fmt.Errorf("cannot pass 'nosuid' and 'suid' options more than once: %w", errOptionArg) - } - setSuid = true - newMount.Options = append(newMount.Options, kv[0]) - case "nodev", "dev": - if setDev { - return newMount, fmt.Errorf("cannot pass 'nodev' and 'dev' options more than once: %w", errOptionArg) - } - setDev = true - newMount.Options = append(newMount.Options, kv[0]) - case "noexec", "exec": - if setExec { - return newMount, fmt.Errorf("cannot pass 'noexec' and 'exec' options more than once: %w", errOptionArg) - } - setExec = true - newMount.Options = append(newMount.Options, kv[0]) - case "tmpfs-mode": - if len(kv) == 1 { - return newMount, fmt.Errorf("%v: %w", kv[0], errOptionArg) - } - newMount.Options = append(newMount.Options, fmt.Sprintf("mode=%s", kv[1])) - case "tmpfs-size": - if len(kv) == 1 { - return newMount, fmt.Errorf("%v: %w", kv[0], errOptionArg) - } - newMount.Options = append(newMount.Options, fmt.Sprintf("size=%s", kv[1])) - case "src", "source": - return newMount, fmt.Errorf("source is not supported with tmpfs mounts") - case "target", "dst", "destination": - if len(kv) == 1 { - return newMount, fmt.Errorf("%v: %w", kv[0], errOptionArg) - } - if err := parse.ValidateVolumeCtrDir(kv[1]); err != nil { - return newMount, err - } - newMount.Destination = unixPathClean(kv[1]) - setDest = true - case "U", "chown": - if setOwnership { - return newMount, fmt.Errorf("cannot pass 'U' or 'chown' option more than once: %w", errOptionArg) - } - ok, err := validChownFlag(val) - if err != nil { - return newMount, err - } - if ok { - newMount.Options = append(newMount.Options, "U") - } - setOwnership = true - case "consistency": - // Often used on MACs and mistakenly on Linux platforms. - // Since Docker ignores this option so shall we. - continue - default: - return newMount, fmt.Errorf("%s: %w", kv[0], util.ErrBadMntOption) - } + var err error + mnt, err := parseMountOptions(newMount.Type, args) + if err != nil { + return newMount, err } - - if !setDest { + if len(mnt.Destination) == 0 { return newMount, errNoDest } - + newMount.Destination = mnt.Destination + newMount.Options = mnt.Options return newMount, nil } @@ -517,84 +504,16 @@ func getDevptsMount(args []string) (spec.Mount, error) { func getNamedVolume(args []string) (*specgen.NamedVolume, error) { newVolume := new(specgen.NamedVolume) - var setDest, setRORW, setSuid, setDev, setExec, setOwnership bool - - for _, val := range args { - kv := strings.SplitN(val, "=", 2) - switch kv[0] { - case "volume-opt": - newVolume.Options = append(newVolume.Options, val) - case "ro", "rw": - if setRORW { - return nil, fmt.Errorf("cannot pass 'ro' and 'rw' options more than once: %w", errOptionArg) - } - setRORW = true - newVolume.Options = append(newVolume.Options, kv[0]) - case "nosuid", "suid": - if setSuid { - return nil, fmt.Errorf("cannot pass 'nosuid' and 'suid' options more than once: %w", errOptionArg) - } - setSuid = true - newVolume.Options = append(newVolume.Options, kv[0]) - case "nodev", "dev": - if setDev { - return nil, fmt.Errorf("cannot pass 'nodev' and 'dev' options more than once: %w", errOptionArg) - } - setDev = true - newVolume.Options = append(newVolume.Options, kv[0]) - case "noexec", "exec": - if setExec { - return nil, fmt.Errorf("cannot pass 'noexec' and 'exec' options more than once: %w", errOptionArg) - } - setExec = true - newVolume.Options = append(newVolume.Options, kv[0]) - case "volume-label": - return nil, fmt.Errorf("the --volume-label option is not presently implemented") - case "src", "source": - if len(kv) == 1 { - return nil, fmt.Errorf("%v: %w", kv[0], errOptionArg) - } - newVolume.Name = kv[1] - case "target", "dst", "destination": - if len(kv) == 1 { - return nil, fmt.Errorf("%v: %w", kv[0], errOptionArg) - } - if err := parse.ValidateVolumeCtrDir(kv[1]); err != nil { - return nil, err - } - newVolume.Dest = unixPathClean(kv[1]) - setDest = true - case "idmap": - if len(kv) > 1 { - newVolume.Options = append(newVolume.Options, fmt.Sprintf("idmap=%s", kv[1])) - } else { - newVolume.Options = append(newVolume.Options, "idmap") - } - case "U", "chown": - if setOwnership { - return newVolume, fmt.Errorf("cannot pass 'U' or 'chown' option more than once: %w", errOptionArg) - } - ok, err := validChownFlag(val) - if err != nil { - return newVolume, err - } - if ok { - newVolume.Options = append(newVolume.Options, "U") - } - setOwnership = true - case "consistency": - // Often used on MACs and mistakenly on Linux platforms. - // Since Docker ignores this option so shall we. - continue - default: - return nil, fmt.Errorf("%s: %w", kv[0], util.ErrBadMntOption) - } + mnt, err := parseMountOptions(define.TypeVolume, args) + if err != nil { + return nil, err } - - if !setDest { + if len(mnt.Destination) == 0 { return nil, errNoDest } - + newVolume.Options = mnt.Options + newVolume.Name = mnt.Source + newVolume.Dest = mnt.Destination return newVolume, nil } diff --git a/test/system/060-mount.bats b/test/system/060-mount.bats index dd8aa3e97b..30fe10e410 100644 --- a/test/system/060-mount.bats +++ b/test/system/060-mount.bats @@ -84,6 +84,22 @@ load helpers is "$output" "" "podman image mount, no args, after umount" } +@test "podman run --mount ro=false " { + local volpath=/path/in/container + local stdopts="type=volume,destination=$volpath" + + # Variations on a theme (not by Paganini). All of these should fail. + for varopt in readonly readonly=true ro=true ro rw=false;do + run_podman 1 run --rm -q --mount $stdopts,$varopt $IMAGE touch $volpath/a + is "$output" "touch: $volpath/a: Read-only file system" "with $varopt" + done + + # All of these should pass + for varopt in rw rw=true ro=false readonly=false;do + run_podman run --rm -q --mount $stdopts,$varopt $IMAGE touch $volpath/a + done +} + @test "podman run --mount image" { skip_if_rootless "too hard to test rootless" From fc8f229bdea9d7840df4b1086f6cc14bbddd74e5 Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Wed, 28 Jun 2023 08:19:36 -0400 Subject: [PATCH 11/34] Remove 'inspecting object' from inspect errors This is just useless noise and gets us closer to what Docker returns. Signed-off-by: Daniel J Walsh --- cmd/podman/inspect/inspect.go | 6 +++--- test/system/700-play.bats | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/podman/inspect/inspect.go b/cmd/podman/inspect/inspect.go index 22b2c90554..575080163d 100644 --- a/cmd/podman/inspect/inspect.go +++ b/cmd/podman/inspect/inspect.go @@ -185,16 +185,16 @@ func (i *inspector) inspect(namesOrIDs []string) error { err = rpt.Execute(data) } if err != nil { - errs = append(errs, fmt.Errorf("printing inspect output: %w", err)) + errs = append(errs, err) } if len(errs) > 0 { if len(errs) > 1 { for _, err := range errs[1:] { - fmt.Fprintf(os.Stderr, "error inspecting object: %v\n", err) + fmt.Fprintf(os.Stderr, "%v\n", err) } } - return fmt.Errorf("inspecting object: %w", errs[0]) + return errs[0] } return nil } diff --git a/test/system/700-play.bats b/test/system/700-play.bats index 3ae158270a..62bfe24787 100644 --- a/test/system/700-play.bats +++ b/test/system/700-play.bats @@ -266,7 +266,7 @@ EOF run_podman kube down $PODMAN_TMPDIR/test.yaml run_podman 125 inspect test_pod-test - is "$output" ".*Error: inspecting object: no such object: \"test_pod-test\"" + is "$output" ".*Error: no such object: \"test_pod-test\"" run_podman pod rm -a run_podman rm -a } @@ -454,7 +454,7 @@ _EOF run_podman kube down $PODMAN_TMPDIR/test.yaml run_podman 125 inspect test_pod-test - is "$output" ".*Error: inspecting object: no such object: \"test_pod-test\"" + is "$output" ".*Error: no such object: \"test_pod-test\"" run_podman pod rm -a run_podman rm -a } @@ -477,7 +477,7 @@ _EOF is "$output" "true" run_podman kube down $SERVER/testpod.yaml run_podman 125 inspect test_pod-test - is "$output" ".*Error: inspecting object: no such object: \"test_pod-test\"" + is "$output" ".*Error: no such object: \"test_pod-test\"" run_podman pod rm -a -f run_podman rm -a -f -t0 From c8cfcc25349042aa7fb94fd4234fd01d9726f65d Mon Sep 17 00:00:00 2001 From: Doug Rabson Date: Mon, 12 Jun 2023 14:26:09 +0100 Subject: [PATCH 12/34] pkg/specgen: add support for 'podman run --init' on FreeBSD This adds define.BindOptions to declare the mount options for bind-like mounts (nullfs on FreeBSD). Note: this mirrors identical declarations in buildah and it may be preferable to use buildah's copies throughout podman. [NO NEW TESTS NEEDED] Signed-off-by: Doug Rabson --- libpod/define/mount_freebsd.go | 5 +++++ libpod/define/mount_linux.go | 5 +++++ pkg/specgen/generate/storage.go | 2 +- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/libpod/define/mount_freebsd.go b/libpod/define/mount_freebsd.go index e080c9ec6c..3150ffbe7a 100644 --- a/libpod/define/mount_freebsd.go +++ b/libpod/define/mount_freebsd.go @@ -6,3 +6,8 @@ const ( // TypeBind is the type for mounting host dir TypeBind = "nullfs" ) + +var ( + // Mount potions for bind + BindOptions = []string{} +) diff --git a/libpod/define/mount_linux.go b/libpod/define/mount_linux.go index 5ef8489052..126e0d6c77 100644 --- a/libpod/define/mount_linux.go +++ b/libpod/define/mount_linux.go @@ -6,3 +6,8 @@ const ( // TypeBind is the type for mounting host dir TypeBind = "bind" ) + +var ( + // Mount potions for bind + BindOptions = []string{"bind"} +) diff --git a/pkg/specgen/generate/storage.go b/pkg/specgen/generate/storage.go index ffaaa0e1fb..52d7427520 100644 --- a/pkg/specgen/generate/storage.go +++ b/pkg/specgen/generate/storage.go @@ -363,7 +363,7 @@ func addContainerInitBinary(s *specgen.SpecGenerator, path string) (spec.Mount, Destination: define.ContainerInitPath, Type: define.TypeBind, Source: path, - Options: []string{define.TypeBind, "ro"}, + Options: append(define.BindOptions, "ro"), } if path == "" { From cf5c4c9ee6a9c2a88c7254a324a2eeb36ce02f04 Mon Sep 17 00:00:00 2001 From: Fabian Wiesel Date: Thu, 29 Jun 2023 12:28:05 +0200 Subject: [PATCH 13/34] [CI:DOCS] Document support of pod security context IDs With PR #14167, the pod-level security Context ID are supported, while the markdown says it isn't. This patch fixes it. ``` None ``` Signed-off-by: Fabian Wiesel --- docs/kubernetes_support.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/kubernetes_support.md b/docs/kubernetes_support.md index 7be15d77a6..741d14e8ed 100644 --- a/docs/kubernetes_support.md +++ b/docs/kubernetes_support.md @@ -52,18 +52,18 @@ Note: **N/A** means that the option cannot be supported in a single-node Podman | shareProcessNamespace | ✅ | | serviceAccountName | no | | automountServiceAccountToken | no | -| securityContext\.runAsUser | no | +| securityContext\.runAsUser | ✅ | | securityContext\.runAsNonRoot | no | -| securityContext\.runAsGroup | no | -| securityContext\.supplementalGroups | no | +| securityContext\.runAsGroup | ✅ | +| securityContext\.supplementalGroups | ✅ | | securityContext\.fsGroup | no | | securityContext\.fsGroupChangePolicy | no | | securityContext\.seccompProfile\.type | no | | securityContext\.seccompProfile\.localhostProfile | no | -| securityContext\.seLinuxOptions\.level | no | -| securityContext\.seLinuxOptions\.role | no | -| securityContext\.seLinuxOptions\.type | no | -| securityContext\.seLinuxOptions\.user | no | +| securityContext\.seLinuxOptions\.level | ✅ | +| securityContext\.seLinuxOptions\.role | ✅ | +| securityContext\.seLinuxOptions\.type | ✅ | +| securityContext\.seLinuxOptions\.user | ✅ | | securityContext\.sysctls\.name | no | | securityContext\.sysctls\.value | no | | securityContext\.windowsOptions\.gmsaCredentialSpec | no | From 662cca7cc3102ed5f50746e48910967952cd7479 Mon Sep 17 00:00:00 2001 From: Doug Rabson Date: Sun, 2 Jul 2023 14:05:17 +0100 Subject: [PATCH 14/34] pkg/specgen: properly identify image OS on FreeBSD When working on Linux emulation on FreeBSD, I assumed that SpecGenerator.ImageOS was always populated from the image's OS value but in fact, this value comes from the CLI --os flag if set, otherwise "". This broke running FreeBSD native containers unless --os=freebsd was also set. Fix the problem by getting the value from the image itself. This is a strong incentive for me to complete a stalled project to enable podman system tests on FreeBSD. [NO NEW TESTS NEEDED] Signed-off-by: Doug Rabson --- pkg/specgen/generate/oci_freebsd.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/pkg/specgen/generate/oci_freebsd.go b/pkg/specgen/generate/oci_freebsd.go index 282165f3f1..9d1c266214 100644 --- a/pkg/specgen/generate/oci_freebsd.go +++ b/pkg/specgen/generate/oci_freebsd.go @@ -18,11 +18,17 @@ import ( // SpecGenToOCI returns the base configuration for the container. func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runtime, rtc *config.Config, newImage *libimage.Image, mounts []spec.Mount, pod *libpod.Pod, finalCmd []string, compatibleOptions *libpod.InfraInherit) (*spec.Spec, error) { - if s.ImageOS != "freebsd" && s.ImageOS != "linux" { - return nil, fmt.Errorf("unsupported image OS: %s", s.ImageOS) + inspectData, err := newImage.Inspect(ctx, nil) + if err != nil { + return nil, err + } + imageOs := inspectData.Os + + if imageOs != "freebsd" && imageOs != "linux" { + return nil, fmt.Errorf("unsupported image OS: %s", imageOs) } - g, err := generate.New(s.ImageOS) + g, err := generate.New(imageOs) if err != nil { return nil, err } @@ -55,7 +61,7 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt } // Linux emulatioon - if s.ImageOS == "linux" { + if imageOs == "linux" { var mounts []spec.Mount for _, m := range configSpec.Mounts { switch m.Destination { From 2ef2a671e9bce76b3f2629f841158963e8d9d132 Mon Sep 17 00:00:00 2001 From: Simon Brakhane Date: Thu, 6 Jul 2023 11:06:34 +0200 Subject: [PATCH 15/34] bugfix: do not try to parse empty ranges An empty range caused a panic as parseOptionIDs tried to check further down for an @ at index 0 without taking into account that the splitted out string could be empty. Signed-off-by: Simon Brakhane --- libpod/container_internal_common.go | 4 ++++ libpod/container_internal_test.go | 3 +++ 2 files changed, 7 insertions(+) diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index 7dfceac8a6..87794eeba7 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -65,6 +65,10 @@ func parseOptionIDs(ctrMappings []idtools.IDMap, option string) ([]idtools.IDMap for i, m := range ranges { var v idtools.IDMap + if m == "" { + return nil, fmt.Errorf("invalid empty range for %q", option) + } + relative := false if m[0] == '@' { relative = true diff --git a/libpod/container_internal_test.go b/libpod/container_internal_test.go index 1c096d1d16..4fb95e95e4 100644 --- a/libpod/container_internal_test.go +++ b/libpod/container_internal_test.go @@ -70,6 +70,9 @@ func TestParseOptionIDs(t *testing.T) { _, err = parseOptionIDs(idMap, "@10000-20000-2") assert.NotNil(t, err) + + _, err = parseOptionIDs(idMap, "100-200-3###400-500-6") + assert.NotNil(t, err) } func TestParseIDMapMountOption(t *testing.T) { From a312553fcb6a5a82c4fc4dece02b44f40439e31d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20F=20Bj=C3=B6rklund?= Date: Thu, 6 Jul 2023 14:51:06 +0200 Subject: [PATCH 16/34] Use bytes size consistently instead of human size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously podman was using "MB" and "GB" (binary) for input but "MB" and "GB" (decimal) for output, which was causing confusion. Signed-off-by: Anders F Björklund --- cmd/podman/machine/init.go | 4 ++-- cmd/podman/machine/list.go | 4 ++-- cmd/podman/machine/set.go | 4 ++-- docs/source/markdown/podman-machine-init.1.md.in | 4 ++-- pkg/machine/e2e/config_init_test.go | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cmd/podman/machine/init.go b/cmd/podman/machine/init.go index 5ebaa14f7e..a8da908a1b 100644 --- a/cmd/podman/machine/init.go +++ b/cmd/podman/machine/init.go @@ -61,7 +61,7 @@ func init() { flags.Uint64Var( &initOpts.DiskSize, diskSizeFlagName, cfg.ContainersConfDefaultsRO.Machine.DiskSize, - "Disk size in GB", + "Disk size in GiB", ) _ = initCmd.RegisterFlagCompletionFunc(diskSizeFlagName, completion.AutocompleteNone) @@ -70,7 +70,7 @@ func init() { flags.Uint64VarP( &initOpts.Memory, memoryFlagName, "m", cfg.ContainersConfDefaultsRO.Machine.Memory, - "Memory in MB", + "Memory in MiB", ) _ = initCmd.RegisterFlagCompletionFunc(memoryFlagName, completion.AutocompleteNone) diff --git a/cmd/podman/machine/list.go b/cmd/podman/machine/list.go index 6d9088b922..0198c61140 100644 --- a/cmd/podman/machine/list.go +++ b/cmd/podman/machine/list.go @@ -216,8 +216,8 @@ func toHumanFormat(vms []*machine.ListResponse) ([]*entities.ListReporter, error response.Created = units.HumanDuration(time.Since(vm.CreatedAt)) + " ago" response.VMType = vm.VMType response.CPUs = vm.CPUs - response.Memory = units.HumanSize(float64(vm.Memory)) - response.DiskSize = units.HumanSize(float64(vm.DiskSize)) + response.Memory = units.BytesSize(float64(vm.Memory)) + response.DiskSize = units.BytesSize(float64(vm.DiskSize)) humanResponses = append(humanResponses, response) } diff --git a/cmd/podman/machine/set.go b/cmd/podman/machine/set.go index e621dcd488..25cc2d1de3 100644 --- a/cmd/podman/machine/set.go +++ b/cmd/podman/machine/set.go @@ -61,7 +61,7 @@ func init() { flags.Uint64Var( &setFlags.DiskSize, diskSizeFlagName, 0, - "Disk size in GB", + "Disk size in GiB", ) _ = setCmd.RegisterFlagCompletionFunc(diskSizeFlagName, completion.AutocompleteNone) @@ -70,7 +70,7 @@ func init() { flags.Uint64VarP( &setFlags.Memory, memoryFlagName, "m", 0, - "Memory in MB", + "Memory in MiB", ) _ = setCmd.RegisterFlagCompletionFunc(memoryFlagName, completion.AutocompleteNone) diff --git a/docs/source/markdown/podman-machine-init.1.md.in b/docs/source/markdown/podman-machine-init.1.md.in index 4dfd1aa95d..80d0a70258 100644 --- a/docs/source/markdown/podman-machine-init.1.md.in +++ b/docs/source/markdown/podman-machine-init.1.md.in @@ -36,7 +36,7 @@ Number of CPUs. #### **--disk-size**=*number* -Size of the disk for the guest VM in GB. +Size of the disk for the guest VM in GiB. #### **--help** @@ -57,7 +57,7 @@ Defaults to `testing`. #### **--memory**, **-m**=*number* -Memory (in MB). +Memory (in MiB). Note: 1024MiB = 1GiB. #### **--now** diff --git a/pkg/machine/e2e/config_init_test.go b/pkg/machine/e2e/config_init_test.go index 8339f39d6f..51dd397159 100644 --- a/pkg/machine/e2e/config_init_test.go +++ b/pkg/machine/e2e/config_init_test.go @@ -7,11 +7,11 @@ import ( type initMachine struct { /* --cpus uint Number of CPUs (default 1) - --disk-size uint Disk size in GB (default 100) + --disk-size uint Disk size in GiB (default 100) --ignition-path string Path to ignition file --username string Username of the remote user (default "core" for FCOS, "user" for Fedora) --image-path string Path to bootable image (default "testing") - -m, --memory uint Memory in MB (default 2048) + -m, --memory uint Memory in MiB (default 2048) --now Start machine now --rootful Whether this machine should prefer rootful container execution --timezone string Set timezone (default "local") From dd7dbb826158c8cc86b898ce54e3b22a94ed7762 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Sj=C3=B6lund?= Date: Sat, 8 Jul 2023 18:17:02 +0200 Subject: [PATCH 17/34] [CI:DOCS] podman-system-service.1.md: document systemd usage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Regarding "The command does not support more than one listening socket for the API service." See this Podman source code: (a permalink into the main branch as of 2 July 2023) https://github.com/containers/podman/blob/539be58163a1730af0d84b39fcde585983cd9925/cmd/podman/system/service_abi.go#L48-L50 Move up the paragraph "The REST API provided ...". Move up the sentence "Note: The default systemd ...". Signed-off-by: Erik Sjölund --- .../markdown/podman-system-service.1.md | 101 ++++++++++++++++-- 1 file changed, 92 insertions(+), 9 deletions(-) diff --git a/docs/source/markdown/podman-system-service.1.md b/docs/source/markdown/podman-system-service.1.md index ddf96a6eeb..d039f48ccb 100644 --- a/docs/source/markdown/podman-system-service.1.md +++ b/docs/source/markdown/podman-system-service.1.md @@ -7,26 +7,69 @@ podman\-system\-service - Run an API service **podman system service** [*options*] ## DESCRIPTION -The **podman system service** command creates a listening service that answers API calls for Podman. You may -optionally provide an endpoint for the API in URI form. For example, *unix:///tmp/foobar.sock* or *tcp://localhost:8080*. +The **podman system service** command creates a listening service that answers API calls for Podman. +The command is available on Linux systems and is usually executed in systemd services. +The command is not available when the Podman command is executed directly on a Windows or macOS +host or in other situations where the Podman command is accessing a remote Podman API service. + +The REST API provided by **podman system service** is split into two parts: a compatibility layer offering support for the Docker v1.40 API, and a Podman-native Libpod layer. +Documentation for the latter is available at *https://docs.podman.io/en/latest/_static/api.html*. +Both APIs are versioned, but the server does not reject requests with an unsupported version set. + +### Run the command in a systemd service + +The command **podman system service** supports systemd socket activation. +When the command is run in a systemd service, the API service can therefore be provided on demand. +If the systemd service is not already running, it will be activated as soon as +a client connects to the listening socket. Systemd then executes the +**podman system service** command. +After some time of inactivity, as defined by the __--time__ option, the command terminates. +Systemd sets the _podman.service_ state as inactive. At this point there is no +**podman system service** process running. No unnecessary compute resources are wasted. +As soon as another client connects, systemd activates the systemd service again. + +The systemd unit files that declares the Podman API service for users are + +* _/usr/lib/systemd/user/podman.service_ +* _/usr/lib/systemd/user/podman.socket_ + +In the file _podman.socket_ the path of the listening Unix socket is defined by + +``` +ListenStream=%t/podman/podman.sock +``` + +The path contains the systemd specifier `%t` which systemd expands to the value of the environment variable +`XDG_RUNTIME_DIR` (see systemd specifiers in the **systemd.unit(5)** man page). + +In addition to the systemd user services, there is also a systemd system service _podman.service_. +It runs rootful Podman and is accessed from the Unix socket _/run/podman/podman.sock_. See the systemd unit files + +* _/usr/lib/systemd/system/podman.service_ +* _/usr/lib/systemd/system/podman.socket_ + +The **podman system service** command does not support more than one listening socket for the API service. + +Note: The default systemd unit files (system and user) change the log-level option to *info* from *error*. This change provides additional information on each API call. + +### Run the command directly + +To support running an API service without using a systemd service, the command also takes an +optional endpoint argument for the API in URI form. For example, *unix:///tmp/foobar.sock* or *tcp://localhost:8080*. If no endpoint is provided, defaults is used. The default endpoint for a rootful service is *unix:///run/podman/podman.sock* and rootless is *unix://$XDG_RUNTIME_DIR/podman/podman.sock* (for example *unix:///run/user/1000/podman/podman.sock*) +### Access the Unix socket from inside a container + To access the API service inside a container: - mount the socket as a volume - run the container with `--security-opt label=disable` -The REST API provided by **podman system service** is split into two parts: a compatibility layer offering support for the Docker v1.40 API, and a Podman-native Libpod layer. -Documentation for the latter is available at *https://docs.podman.io/en/latest/_static/api.html*. -Both APIs are versioned, but the server does not reject requests with an unsupported version set. - Please note that the API grants full access to Podman's capabilities, and allows arbitrary code execution as the user running the API. We strongly recommend against making the API socket available via the network. The default configuration (a Unix socket with permissions set to only allow the user running Podman) is the most secure way of running the API. -Note: The default systemd unit files (system and user) change the log-level option to *info* from *error*. This change provides additional information on each API call. - ## OPTIONS #### **--cors** @@ -47,11 +90,51 @@ See **[containers.conf(5)](https://github.com/containers/common/blob/main/docs/c ## EXAMPLES -Run an API listening for 5 seconds using the default socket. +To start the systemd socket for a rootless service, run as the user: + +``` +systemctl --user start podman.socket +``` + +The socket can then be used by for example docker-compose that needs a Docker-compatible API. + +``` +$ ls $XDG_RUNTIME_DIR/podman/podman.sock +/run/user/1000/podman/podman.sock +$ export DOCKER_HOST=unix://$XDG_RUNTIME_DIR/podman/podman.sock +$ docker-compose up +``` + +To configure the systemd socket to be automatically started after reboots, run as the user: + +``` +systemctl --user enable podman.socket +loginctl enable-linger +``` + +To start the systemd socket for the rootful service, run: + +``` +sudo systemctl start podman.socket +``` + +To configure the socket to be automatically started after reboots, run: + +``` +sudo systemctl enable podman.socket +``` + +It is possible to run the API without using systemd socket activation. +In this case the API will not be available on demand because the command will +stay terminated after the inactivity timeout has passed. +Run an API with an inactivity timeout of 5 seconds without using socket activation: + ``` podman system service --time 5 ``` +The default socket was used as no URI argument was provided. + ## SEE ALSO **[podman(1)](podman.1.md)**, **[podman-system-connection(1)](podman-system-connection.1.md)**, **[containers.conf(5)](https://github.com/containers/common/blob/main/docs/containers.conf.5.md)** From a673bb23e1ca2b8061b0b9baa7e35a4c15028ea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Sj=C3=B6lund?= Date: Sun, 9 Jul 2023 10:50:48 +0200 Subject: [PATCH 18/34] [CI:DOCS] uidmap man pages: fix corrupt italics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The markdown-to-manpage sequence interprets _from_uid_ and *from_uid* differently. Use the latter syntax to get the expected result. Fixes: https://github.com/containers/podman/issues/19171 Signed-off-by: Erik Sjölund --- .../markdown/options/uidmap.container.md | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/source/markdown/options/uidmap.container.md b/docs/source/markdown/options/uidmap.container.md index b267d0239d..940ca6e26f 100644 --- a/docs/source/markdown/options/uidmap.container.md +++ b/docs/source/markdown/options/uidmap.container.md @@ -9,7 +9,7 @@ option conflicts with the **--userns** and **--subuidname** options. This option provides a way to map host UIDs to container UIDs. It can be passed several times to map different ranges. -The _from_uid_ value is based upon the user running the command, either rootful or rootless users. +The *from_uid* value is based upon the user running the command, either rootful or rootless users. * rootful user: *container_uid*:*host_uid*:*amount* * rootless user: *container_uid*:*intermediate_uid*:*amount* @@ -23,13 +23,13 @@ If for example _amount_ is **4** the mapping looks like: | host UID | container UID | | ---------- | ---------------- | -| _from_uid_ | _container_uid_ | -| _from_uid_ + 1 | _container_uid_ + 1 | -| _from_uid_ + 2 | _container_uid_ + 2 | -| _from_uid_ + 3 | _container_uid_ + 3 | +| *from_uid* | *container_uid* | +| *from_uid* + 1 | *container_uid* + 1 | +| *from_uid* + 2 | *container_uid* + 2 | +| *from_uid* + 3 | *container_uid* + 3 | When **podman <>** is called by an unprivileged user (i.e. running rootless), -the value _from_uid_ is interpreted as an "intermediate UID". In the rootless +the value *from_uid* is interpreted as an "intermediate UID". In the rootless case, host UIDs are not mapped directly to container UIDs. Instead the mapping happens over two mapping steps: @@ -59,11 +59,11 @@ If for example _amount_ is **5** the second mapping step looks like: | intermediate UID | container UID | | ------------------ | ---------------- | -| _from_uid_ | _container_uid_ | -| _from_uid_ + 1 | _container_uid_ + 1 | -| _from_uid_ + 2 | _container_uid_ + 2 | -| _from_uid_ + 3 | _container_uid_ + 3 | -| _from_uid_ + 4 | _container_uid_ + 4 | +| *from_uid* | *container_uid* | +| *from_uid* + 1 | *container_uid* + 1 | +| *from_uid* + 2 | *container_uid* + 2 | +| *from_uid* + 3 | *container_uid* + 3 | +| *from_uid* + 4 | *container_uid* + 4 | When running as rootless, Podman uses all the ranges configured in the _/etc/subuid_ file. From edc51d9ff2df4ba094d02970b48da28383cd3c06 Mon Sep 17 00:00:00 2001 From: Michael Hrivnak Date: Sun, 9 Jul 2023 15:46:50 -0400 Subject: [PATCH 19/34] Fixes typo in the path where quadlet looks for files This change matches the list above in the same document, in the section `Podman user unit search path`. I also confirmed that this matches [the code](https://github.com/containers/podman/blob/60a5a59/cmd/quadlet/main.go#L119). Signed-off-by: Michael Hrivnak --- docs/source/markdown/podman-systemd.unit.5.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/markdown/podman-systemd.unit.5.md b/docs/source/markdown/podman-systemd.unit.5.md index ee98508a56..4fa2ba08e8 100644 --- a/docs/source/markdown/podman-systemd.unit.5.md +++ b/docs/source/markdown/podman-systemd.unit.5.md @@ -42,7 +42,7 @@ like dependencies or cgroup limits. For rootless containers, when administrators place Quadlet files in the /etc/containers/systemd/users directory, all users' sessions execute the Quadlet when the login session begins. If the administrator places a Quadlet -file in the /etc/containers/systemd/user/${UID}/ directory, then only the +file in the /etc/containers/systemd/users/${UID}/ directory, then only the user with the matching UID execute the Quadlet when the login session gets started. From c81a001011b325b7831fa4c535d551bdaf4946f4 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 10 Jul 2023 11:36:28 +0200 Subject: [PATCH 20/34] make --syslog errors non fatal Podman will always pass down --syslog to conmon since 13c2aca21. However there systems without syslog running, likely in container setups. As reported in this was already a problem before when debug level is used. Then conmon will pass down --syslog back to the podman container cleanup command causing it to fail without doing anything. Given that I think it is better to just ignore the error and log it on debug level, we need to make sure cleanup works consistently. [NO NEW TESTS NEEDED] Fixes #19075 Signed-off-by: Paul Holzinger --- cmd/podman/syslog_common.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/cmd/podman/syslog_common.go b/cmd/podman/syslog_common.go index e035e63652..b9909f8ec0 100644 --- a/cmd/podman/syslog_common.go +++ b/cmd/podman/syslog_common.go @@ -4,9 +4,7 @@ package main import ( - "fmt" "log/syslog" - "os" "github.com/sirupsen/logrus" logrusSyslog "github.com/sirupsen/logrus/hooks/syslog" @@ -19,10 +17,8 @@ func syslogHook() { hook, err := logrusSyslog.NewSyslogHook("", "", syslog.LOG_INFO, "") if err != nil { - fmt.Fprint(os.Stderr, "Failed to initialize syslog hook: "+err.Error()) - os.Exit(1) - } - if err == nil { + logrus.Debug("Failed to initialize syslog hook: " + err.Error()) + } else { logrus.AddHook(hook) } } From e354514dc515fb04f224640bb3be48586c2b4ec7 Mon Sep 17 00:00:00 2001 From: Doug Rabson Date: Sun, 9 Jul 2023 08:42:32 +0100 Subject: [PATCH 21/34] libpod: don't make a broken symlink for /etc/mtab on FreeBSD This file has not been present in BSD systems since 2.9.1 BSD and as far as I remember /proc/mounts has never existed on BSD systems. [NO NEW TESTS NEEDED] Signed-off-by: Doug Rabson --- libpod/container_internal.go | 14 ++------------ libpod/container_internal_freebsd.go | 5 +++++ libpod/container_internal_linux.go | 17 +++++++++++++++++ 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 3188f1b40b..be0560efd9 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -1638,18 +1638,8 @@ func (c *Container) mountStorage() (_ string, deferredErr error) { } defer unix.Close(etcInTheContainerFd) - // If /etc/mtab does not exist in container image, then we need to - // create it, so that mount command within the container will work. - err = unix.Symlinkat("/proc/mounts", etcInTheContainerFd, "mtab") - if err != nil && !os.IsExist(err) { - return "", fmt.Errorf("creating /etc/mtab symlink: %w", err) - } - // If the symlink was created, then also chown it to root in the container - if err == nil && (rootUID != 0 || rootGID != 0) { - err = unix.Fchownat(etcInTheContainerFd, "mtab", rootUID, rootGID, unix.AT_SYMLINK_NOFOLLOW) - if err != nil { - return "", fmt.Errorf("chown /etc/mtab: %w", err) - } + if err := c.makePlatformMtabLink(etcInTheContainerFd, rootUID, rootGID); err != nil { + return "", err } tz := c.Timezone() diff --git a/libpod/container_internal_freebsd.go b/libpod/container_internal_freebsd.go index 5f4538cc0a..6bd872aa45 100644 --- a/libpod/container_internal_freebsd.go +++ b/libpod/container_internal_freebsd.go @@ -336,3 +336,8 @@ func (s *safeMountInfo) Close() { func (c *Container) safeMountSubPath(mountPoint, subpath string) (s *safeMountInfo, err error) { return &safeMountInfo{mountPoint: filepath.Join(mountPoint, subpath)}, nil } + +func (c *Container) makePlatformMtabLink(etcInTheContainerFd, rootUID, rootGID int) error { + // /etc/mtab does not exist on FreeBSD + return nil +} diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 08e4df12fd..2f1082907f 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -784,3 +784,20 @@ func (c *Container) safeMountSubPath(mountPoint, subpath string) (s *safeMountIn mountPoint: npath, }, nil } + +func (c *Container) makePlatformMtabLink(etcInTheContainerFd, rootUID, rootGID int) error { + // If /etc/mtab does not exist in container image, then we need to + // create it, so that mount command within the container will work. + err := unix.Symlinkat("/proc/mounts", etcInTheContainerFd, "mtab") + if err != nil && !os.IsExist(err) { + return fmt.Errorf("creating /etc/mtab symlink: %w", err) + } + // If the symlink was created, then also chown it to root in the container + if err == nil && (rootUID != 0 || rootGID != 0) { + err = unix.Fchownat(etcInTheContainerFd, "mtab", rootUID, rootGID, unix.AT_SYMLINK_NOFOLLOW) + if err != nil { + return fmt.Errorf("chown /etc/mtab: %w", err) + } + } + return nil +} From 28e92b9de2179ac83d1f640cd0efc62a26cb6179 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 10 Jul 2023 10:52:01 +0200 Subject: [PATCH 22/34] manifest inspect: support authentication Previous tests have worked by pure chance since the client and server ran on the same host; the server picked up the credentials created by the client login. Extend the gating tests and add a new integration test which is further capable of exercising the remote code. Note that fixing authentication support requires adding a new `--authfile` CLi flag to `manifest inspect`. This will at least allow for passing an authfile to be bindings. Username and password are not yet supported. Signed-off-by: Valentin Rothberg --- cmd/podman/manifest/inspect.go | 17 ++++++--- docs/source/markdown/options/authfile.md | 2 +- .../markdown/podman-manifest-inspect.1.md.in | 2 + pkg/api/handlers/libpod/manifests.go | 11 +++++- pkg/bindings/manifests/manifests.go | 14 ++++++- pkg/bindings/manifests/types.go | 4 ++ .../manifests/types_inspect_options.go | 15 ++++++++ pkg/domain/entities/manifest.go | 2 + pkg/domain/infra/abi/manifest.go | 4 ++ pkg/domain/infra/tunnel/manifest.go | 2 +- test/e2e/login_logout_test.go | 38 +++++++++++++++++++ test/system/012-manifest.bats | 36 ++++++++++++++++++ test/system/150-login.bats | 29 -------------- 13 files changed, 135 insertions(+), 41 deletions(-) diff --git a/cmd/podman/manifest/inspect.go b/cmd/podman/manifest/inspect.go index 6542e6cafb..0e4875a988 100644 --- a/cmd/podman/manifest/inspect.go +++ b/cmd/podman/manifest/inspect.go @@ -3,6 +3,8 @@ package manifest import ( "fmt" + "github.com/containers/common/pkg/auth" + "github.com/containers/common/pkg/completion" "github.com/containers/image/v5/types" "github.com/containers/podman/v4/cmd/podman/common" "github.com/containers/podman/v4/cmd/podman/registry" @@ -11,8 +13,9 @@ import ( ) var ( - tlsVerifyCLI bool - inspectCmd = &cobra.Command{ + inspectOptions entities.ManifestInspectOptions + tlsVerifyCLI bool + inspectCmd = &cobra.Command{ Use: "inspect [options] IMAGE", Short: "Display the contents of a manifest list or image index", Long: "Display the contents of a manifest list or image index.", @@ -30,6 +33,9 @@ func init() { }) flags := inspectCmd.Flags() + authfileFlagName := "authfile" + flags.StringVar(&inspectOptions.Authfile, authfileFlagName, auth.GetDefaultAuthFile(), "path of the authentication file. Use REGISTRY_AUTH_FILE environment variable to override") + _ = inspectCmd.RegisterFlagCompletionFunc(authfileFlagName, completion.AutocompleteDefault) flags.BoolP("verbose", "v", false, "Added for Docker compatibility") _ = flags.MarkHidden("verbose") flags.BoolVar(&tlsVerifyCLI, "tls-verify", true, "require HTTPS and verify certificates when accessing the registry") @@ -38,14 +44,13 @@ func init() { } func inspect(cmd *cobra.Command, args []string) error { - opts := entities.ManifestInspectOptions{} if cmd.Flags().Changed("tls-verify") { - opts.SkipTLSVerify = types.NewOptionalBool(!tlsVerifyCLI) + inspectOptions.SkipTLSVerify = types.NewOptionalBool(!tlsVerifyCLI) } else if cmd.Flags().Changed("insecure") { insecure, _ := cmd.Flags().GetBool("insecure") - opts.SkipTLSVerify = types.NewOptionalBool(insecure) + inspectOptions.SkipTLSVerify = types.NewOptionalBool(insecure) } - buf, err := registry.ImageEngine().ManifestInspect(registry.Context(), args[0], opts) + buf, err := registry.ImageEngine().ManifestInspect(registry.Context(), args[0], inspectOptions) if err != nil { return err } diff --git a/docs/source/markdown/options/authfile.md b/docs/source/markdown/options/authfile.md index 576b9b5fe7..57416efe91 100644 --- a/docs/source/markdown/options/authfile.md +++ b/docs/source/markdown/options/authfile.md @@ -1,5 +1,5 @@ ####> This option file is used in: -####> podman auto update, build, container runlabel, create, image sign, kube play, login, logout, manifest add, manifest push, pull, push, run, search +####> podman auto update, build, container runlabel, create, image sign, kube play, login, logout, manifest add, manifest inspect, manifest push, pull, push, run, search ####> If file is edited, make sure the changes ####> are applicable to all of those. #### **--authfile**=*path* diff --git a/docs/source/markdown/podman-manifest-inspect.1.md.in b/docs/source/markdown/podman-manifest-inspect.1.md.in index 89993eaa8a..27ce5f4326 100644 --- a/docs/source/markdown/podman-manifest-inspect.1.md.in +++ b/docs/source/markdown/podman-manifest-inspect.1.md.in @@ -15,6 +15,8 @@ A formatted JSON representation of the manifest list or image index. ## OPTIONS +@@option authfile + @@option tls-verify ## EXAMPLES diff --git a/pkg/api/handlers/libpod/manifests.go b/pkg/api/handlers/libpod/manifests.go index bc0f1c628c..b94b937b97 100644 --- a/pkg/api/handlers/libpod/manifests.go +++ b/pkg/api/handlers/libpod/manifests.go @@ -153,12 +153,19 @@ func ManifestInspect(w http.ResponseWriter, r *http.Request) { return } - imageEngine := abi.ImageEngine{Libpod: runtime} - opts := entities.ManifestInspectOptions{} + _, authfile, err := auth.GetCredentials(r) + if err != nil { + utils.Error(w, http.StatusBadRequest, err) + return + } + defer auth.RemoveAuthfile(authfile) + + opts := entities.ManifestInspectOptions{Authfile: authfile} if _, found := r.URL.Query()["tlsVerify"]; found { opts.SkipTLSVerify = types.NewOptionalBool(!query.TLSVerify) } + imageEngine := abi.ImageEngine{Libpod: runtime} rawManifest, err := imageEngine.ManifestInspect(r.Context(), name, opts) if err != nil { utils.Error(w, http.StatusNotFound, err) diff --git a/pkg/bindings/manifests/manifests.go b/pkg/bindings/manifests/manifests.go index 4c40b2dcab..45df54af2c 100644 --- a/pkg/bindings/manifests/manifests.go +++ b/pkg/bindings/manifests/manifests.go @@ -92,7 +92,12 @@ func Inspect(ctx context.Context, name string, options *InspectOptions) (*manife params.Set("tlsVerify", strconv.FormatBool(!options.GetSkipTLSVerify())) } - response, err := conn.DoRequest(ctx, nil, http.MethodGet, "/manifests/%s/json", params, nil, name) + header, err := auth.MakeXRegistryAuthHeader(&imageTypes.SystemContext{AuthFilePath: options.GetAuthfile()}, "", "") + if err != nil { + return nil, err + } + + response, err := conn.DoRequest(ctx, nil, http.MethodGet, "/manifests/%s/json", params, header, name) if err != nil { return nil, err } @@ -125,7 +130,12 @@ func InspectListData(ctx context.Context, name string, options *InspectOptions) params.Set("tlsVerify", strconv.FormatBool(!options.GetSkipTLSVerify())) } - response, err := conn.DoRequest(ctx, nil, http.MethodGet, "/manifests/%s/json", params, nil, name) + header, err := auth.MakeXRegistryAuthHeader(&imageTypes.SystemContext{AuthFilePath: options.GetAuthfile()}, "", "") + if err != nil { + return nil, err + } + + response, err := conn.DoRequest(ctx, nil, http.MethodGet, "/manifests/%s/json", params, header, name) if err != nil { return nil, err } diff --git a/pkg/bindings/manifests/types.go b/pkg/bindings/manifests/types.go index d0485e9319..c9e14b1233 100644 --- a/pkg/bindings/manifests/types.go +++ b/pkg/bindings/manifests/types.go @@ -4,6 +4,10 @@ package manifests // //go:generate go run ../generator/generator.go InspectOptions type InspectOptions struct { + // Authfile - path to an authentication file. + Authfile *string + // SkipTLSVerify - skip https and certificate validation when + // contacting container registries. SkipTLSVerify *bool } diff --git a/pkg/bindings/manifests/types_inspect_options.go b/pkg/bindings/manifests/types_inspect_options.go index fb0c397aec..791dbf3e6a 100644 --- a/pkg/bindings/manifests/types_inspect_options.go +++ b/pkg/bindings/manifests/types_inspect_options.go @@ -17,6 +17,21 @@ func (o *InspectOptions) ToParams() (url.Values, error) { return util.ToParams(o) } +// WithAuthfile set field Authfile to given value +func (o *InspectOptions) WithAuthfile(value string) *InspectOptions { + o.Authfile = &value + return o +} + +// GetAuthfile returns value of field Authfile +func (o *InspectOptions) GetAuthfile() string { + if o.Authfile == nil { + var z string + return z + } + return *o.Authfile +} + // WithSkipTLSVerify set field SkipTLSVerify to given value func (o *InspectOptions) WithSkipTLSVerify(value bool) *InspectOptions { o.SkipTLSVerify = &value diff --git a/pkg/domain/entities/manifest.go b/pkg/domain/entities/manifest.go index 030dc4b6d1..819266980a 100644 --- a/pkg/domain/entities/manifest.go +++ b/pkg/domain/entities/manifest.go @@ -14,6 +14,8 @@ type ManifestCreateOptions struct { // ManifestInspectOptions provides model for inspecting manifest type ManifestInspectOptions struct { + // Path to an authentication file. + Authfile string `json:"-" schema:"-"` // Should TLS registry certificate be verified? SkipTLSVerify types.OptionalBool `json:"-" schema:"-"` } diff --git a/pkg/domain/infra/abi/manifest.go b/pkg/domain/infra/abi/manifest.go index bc60e231ef..224c8488b9 100644 --- a/pkg/domain/infra/abi/manifest.go +++ b/pkg/domain/infra/abi/manifest.go @@ -106,6 +106,10 @@ func (ir *ImageEngine) ManifestInspect(ctx context.Context, name string, opts en func (ir *ImageEngine) remoteManifestInspect(ctx context.Context, name string, opts entities.ManifestInspectOptions) ([]byte, error) { sys := ir.Libpod.SystemContext() + if opts.Authfile != "" { + sys.AuthFilePath = opts.Authfile + } + sys.DockerInsecureSkipTLSVerify = opts.SkipTLSVerify if opts.SkipTLSVerify == types.OptionalBoolTrue { sys.OCIInsecureSkipTLSVerify = true diff --git a/pkg/domain/infra/tunnel/manifest.go b/pkg/domain/infra/tunnel/manifest.go index a94009d6fc..ca708ab0b0 100644 --- a/pkg/domain/infra/tunnel/manifest.go +++ b/pkg/domain/infra/tunnel/manifest.go @@ -34,7 +34,7 @@ func (ir *ImageEngine) ManifestExists(ctx context.Context, name string) (*entiti // ManifestInspect returns contents of manifest list with given name func (ir *ImageEngine) ManifestInspect(ctx context.Context, name string, opts entities.ManifestInspectOptions) ([]byte, error) { - options := new(manifests.InspectOptions) + options := new(manifests.InspectOptions).WithAuthfile(opts.Authfile) if s := opts.SkipTLSVerify; s != types.OptionalBoolUndefined { if s == types.OptionalBoolTrue { options.WithSkipTLSVerify(true) diff --git a/test/e2e/login_logout_test.go b/test/e2e/login_logout_test.go index 69b62e27a6..5574e7b24a 100644 --- a/test/e2e/login_logout_test.go +++ b/test/e2e/login_logout_test.go @@ -190,6 +190,44 @@ var _ = Describe("Podman login and logout", func() { Expect(session).Should(Exit(0)) }) + It("podman manifest with --authfile", func() { + os.Unsetenv("REGISTRY_AUTH_FILE") + + authFile := filepath.Join(podmanTest.TempDir, "auth.json") + session := podmanTest.Podman([]string{"login", "--username", "podmantest", "--password", "test", "--authfile", authFile, server}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + + readAuthInfo(authFile) + + session = podmanTest.Podman([]string{"manifest", "create", testImg}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + + session = podmanTest.Podman([]string{"manifest", "push", testImg}) + session.WaitWithDefaultTimeout() + Expect(session).To(ExitWithError()) + Expect(session.ErrorToString()).To(ContainSubstring(": authentication required")) + + session = podmanTest.Podman([]string{"manifest", "push", "--authfile", authFile, testImg}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + + // Now remove the local manifest to trigger remote inspection + session = podmanTest.Podman([]string{"manifest", "rm", testImg}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + + session = podmanTest.Podman([]string{"manifest", "inspect", testImg}) + session.WaitWithDefaultTimeout() + Expect(session).To(ExitWithError()) + Expect(session.ErrorToString()).To(ContainSubstring(": authentication required")) + + session = podmanTest.Podman([]string{"manifest", "inspect", "--authfile", authFile, testImg}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + }) + It("podman login and logout with --tls-verify", func() { session := podmanTest.Podman([]string{"login", "--username", "podmantest", "--password", "test", "--tls-verify=false", server}) session.WaitWithDefaultTimeout() diff --git a/test/system/012-manifest.bats b/test/system/012-manifest.bats index fcf73721af..df90a135f2 100644 --- a/test/system/012-manifest.bats +++ b/test/system/012-manifest.bats @@ -1,6 +1,8 @@ #!/usr/bin/env bats load helpers +load helpers.network +load helpers.registry # Regression test for #8931 @test "podman images - bare manifest list" { @@ -20,4 +22,38 @@ load helpers run_podman rmi test:1.0 } +@test "podman manifest --tls-verify and --authfile" { + skip_if_remote "running a local registry doesn't work with podman-remote" + start_registry + authfile=${PODMAN_LOGIN_WORKDIR}/auth-$(random_string 10).json + run_podman login --tls-verify=false \ + --username ${PODMAN_LOGIN_USER} \ + --password-stdin \ + --authfile=$authfile \ + localhost:${PODMAN_LOGIN_REGISTRY_PORT} <<<"${PODMAN_LOGIN_PASS}" + is "$output" "Login Succeeded!" "output from podman login" + + manifest1="localhost:${PODMAN_LOGIN_REGISTRY_PORT}/test:1.0" + run_podman manifest create $manifest1 + mid=$output + run_podman manifest push --authfile=$authfile \ + --tls-verify=false $mid \ + $manifest1 + run_podman manifest rm $manifest1 + + # Default is to require TLS; also test explicit opts + for opt in '' '--insecure=false' '--tls-verify=true' "--authfile=$authfile"; do + run_podman 125 manifest inspect $opt $manifest1 + assert "$output" =~ "Error: reading image \"docker://$manifest1\": pinging container registry localhost:${PODMAN_LOGIN_REGISTRY_PORT}:.*x509" \ + "TLE check: fails (as expected) with ${opt:-default}" + done + + run_podman manifest inspect --authfile=$authfile --tls-verify=false $manifest1 + is "$output" ".*\"mediaType\": \"application/vnd.docker.distribution.manifest.list.v2+json\"" "Verify --tls-verify=false --authfile works against an insecure registry" + run_podman manifest inspect --authfile=$authfile --insecure $manifest1 + is "$output" ".*\"mediaType\": \"application/vnd.docker.distribution.manifest.list.v2+json\"" "Verify --insecure --authfile works against an insecure registry" + REGISTRY_AUTH_FILE=$authfile run_podman manifest inspect --tls-verify=false $manifest1 + is "$output" ".*\"mediaType\": \"application/vnd.docker.distribution.manifest.list.v2+json\"" "Verify --tls-verify=false with REGISTRY_AUTH_FILE works against an insecure registry" +} + # vim: filetype=sh diff --git a/test/system/150-login.bats b/test/system/150-login.bats index 2523bc021b..d1d94578d1 100644 --- a/test/system/150-login.bats +++ b/test/system/150-login.bats @@ -326,35 +326,6 @@ function _test_skopeo_credential_sharing() { rm -f $authfile } -@test "podman manifest --tls-verify - basic test" { - run_podman login --tls-verify=false \ - --username ${PODMAN_LOGIN_USER} \ - --password-stdin \ - localhost:${PODMAN_LOGIN_REGISTRY_PORT} <<<"${PODMAN_LOGIN_PASS}" - is "$output" "Login Succeeded!" "output from podman login" - - manifest1="localhost:${PODMAN_LOGIN_REGISTRY_PORT}/test:1.0" - run_podman manifest create $manifest1 - mid=$output - run_podman manifest push --authfile=$authfile \ - --tls-verify=false $mid \ - $manifest1 - run_podman manifest rm $manifest1 - run_podman manifest inspect --insecure $manifest1 - is "$output" ".*\"mediaType\": \"application/vnd.docker.distribution.manifest.list.v2+json\"" "Verify --insecure works against an insecure registry" - run_podman 125 manifest inspect --insecure=false $manifest1 - is "$output" ".*Error: reading image \"docker://$manifest1\": pinging container registry localhost:${PODMAN_LOGIN_REGISTRY_PORT}:" "Verify --insecure=false fails" - run_podman manifest inspect --tls-verify=false $manifest1 - is "$output" ".*\"mediaType\": \"application/vnd.docker.distribution.manifest.list.v2+json\"" "Verify --tls-verify=false works against an insecure registry" - run_podman 125 manifest inspect --tls-verify=true $manifest1 - is "$output" ".*Error: reading image \"docker://$manifest1\": pinging container registry localhost:${PODMAN_LOGIN_REGISTRY_PORT}:" "Verify --tls-verify=true fails" - - # Now log out - run_podman logout localhost:${PODMAN_LOGIN_REGISTRY_PORT} - is "$output" "Removed login credentials for localhost:${PODMAN_LOGIN_REGISTRY_PORT}" \ - "output from podman logout" -} - # END cooperation with skopeo # END actual tests ############################################################################### From a3a62854f5421a5e4de3b4f56934ca62f5098d2a Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 10 Jul 2023 15:30:28 +0200 Subject: [PATCH 23/34] api: fix slow version endpoint This endpoint queried the same package versions twice causing it to be slower than info. Because it already called info we can just reuse the package versions from there. Signed-off-by: Paul Holzinger --- pkg/api/handlers/compat/version.go | 35 ++++++++++-------------------- 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/pkg/api/handlers/compat/version.go b/pkg/api/handlers/compat/version.go index 0d34fbd98e..dd64841ac6 100644 --- a/pkg/api/handlers/compat/version.go +++ b/pkg/api/handlers/compat/version.go @@ -13,7 +13,6 @@ import ( "github.com/containers/podman/v4/pkg/domain/entities" "github.com/containers/podman/v4/pkg/domain/entities/types" "github.com/containers/podman/v4/version" - "github.com/sirupsen/logrus" ) func VersionHandler(w http.ResponseWriter, r *http.Request) { @@ -45,30 +44,20 @@ func VersionHandler(w http.ResponseWriter, r *http.Request) { "MinAPIVersion": version.APIVersion[version.Libpod][version.MinimalAPI].String(), "Os": goRuntime.GOOS, }, + }, { + Name: "Conmon", + Version: info.Host.Conmon.Version, + Details: map[string]string{ + "Package": info.Host.Conmon.Package, + }, + }, { + Name: fmt.Sprintf("OCI Runtime (%s)", info.Host.OCIRuntime.Name), + Version: info.Host.OCIRuntime.Version, + Details: map[string]string{ + "Package": info.Host.OCIRuntime.Package, + }, }} - if conmon, oci, err := runtime.DefaultOCIRuntime().RuntimeInfo(); err != nil { - logrus.Warnf("Failed to retrieve Conmon and OCI Information: %q", err.Error()) - } else { - additional := []types.ComponentVersion{ - { - Name: "Conmon", - Version: conmon.Version, - Details: map[string]string{ - "Package": conmon.Package, - }, - }, - { - Name: fmt.Sprintf("OCI Runtime (%s)", oci.Name), - Version: oci.Version, - Details: map[string]string{ - "Package": oci.Package, - }, - }, - } - components = append(components, additional...) - } - apiVersion := version.APIVersion[version.Compat][version.CurrentAPI] minVersion := version.APIVersion[version.Compat][version.MinimalAPI] From 258135221260202c1652aaac1e10499a78fe73ad Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 10 Jul 2023 15:41:16 +0200 Subject: [PATCH 24/34] test/e2e: wait for socket Do not use podman info/version as they are expensive and clutter the log for no reason. Just checking if we can connect to the socket should be good enough and much faster. Fix the non existing error checking, so that we actually see an useful error when this does not work. Also change the interval, why wait 2s for a retry lets take 100ms steps instead. Fixes #19010 Signed-off-by: Paul Holzinger --- test/e2e/common_test.go | 1 - test/e2e/libpod_suite_remote_test.go | 21 +++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/test/e2e/common_test.go b/test/e2e/common_test.go index 22d0c7aa4b..331e1a8670 100644 --- a/test/e2e/common_test.go +++ b/test/e2e/common_test.go @@ -58,7 +58,6 @@ type PodmanTestIntegration struct { Host HostOS Timings []string TmpDir string - RemoteStartErr error } var LockTmpDir string diff --git a/test/e2e/libpod_suite_remote_test.go b/test/e2e/libpod_suite_remote_test.go index e328830340..ae7d97c059 100644 --- a/test/e2e/libpod_suite_remote_test.go +++ b/test/e2e/libpod_suite_remote_test.go @@ -6,6 +6,7 @@ package integration import ( "errors" "fmt" + "net" "os" "os/exec" "path/filepath" @@ -94,7 +95,8 @@ func (p *PodmanTestIntegration) StartRemoteService() { command.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} p.RemoteCommand = command p.RemoteSession = command.Process - p.RemoteStartErr = p.DelayForService() + err = p.DelayForService() + Expect(err).ToNot(HaveOccurred()) } func (p *PodmanTestIntegration) StopRemoteService() { @@ -145,16 +147,15 @@ func (p *PodmanTestIntegration) RestoreArtifact(image string) error { } func (p *PodmanTestIntegration) DelayForService() error { - var session *PodmanSessionIntegration - for i := 0; i < 5; i++ { - session = p.Podman([]string{"info"}) - session.WaitWithDefaultTimeout() - if session.ExitCode() == 0 { + var err error + var conn net.Conn + for i := 0; i < 100; i++ { + conn, err = net.Dial("unix", strings.TrimPrefix(p.RemoteSocket, "unix:")) + if err == nil { + conn.Close() return nil - } else if i == 4 { - break } - time.Sleep(2 * time.Second) + time.Sleep(100 * time.Millisecond) } - return fmt.Errorf("service not detected, exit code(%d)", session.ExitCode()) + return fmt.Errorf("service socket not detected, timeout after 10 seconds: %w", err) } From d0b0c62853f37685e4f4ca37e537174b5d0fe5c6 Mon Sep 17 00:00:00 2001 From: Peter Jannesen Date: Mon, 10 Jul 2023 22:37:43 +0200 Subject: [PATCH 25/34] Fix: cgroup is not set: internal libpod error after os reboot [NO NEW TESTS NEEDED] Closes #19175 Signed-off-by: Peter Jannesen --- libpod/pod_internal_linux.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libpod/pod_internal_linux.go b/libpod/pod_internal_linux.go index fa85666ca1..ef7fefa41b 100644 --- a/libpod/pod_internal_linux.go +++ b/libpod/pod_internal_linux.go @@ -21,7 +21,7 @@ func (p *Pod) platformRefresh() error { } p.state.CgroupPath = cgroupPath case config.CgroupfsCgroupsManager: - if rootless.IsRootless() && isRootlessCgroupSet(p.config.CgroupParent) { + if !rootless.IsRootless() || isRootlessCgroupSet(p.config.CgroupParent) { p.state.CgroupPath = filepath.Join(p.config.CgroupParent, p.ID()) logrus.Debugf("setting pod cgroup to %s", p.state.CgroupPath) From 2aea98cab1c3e1dfe45397ec0894efb285f30b56 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 11 Jul 2023 15:16:11 +0200 Subject: [PATCH 26/34] libpod: set cid network alias in setupContainer() Since we have sqlite there is no point in duplicating this acroos two db backends. Just set earlier when we validate the networks anyway. Signed-off-by: Paul Holzinger --- libpod/boltdb_state_internal.go | 2 -- libpod/runtime_ctr.go | 2 ++ libpod/sqlite_state_internal.go | 6 ------ 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go index 91164375b5..90420cc687 100644 --- a/libpod/boltdb_state_internal.go +++ b/libpod/boltdb_state_internal.go @@ -607,8 +607,6 @@ func (s *BoltState) addContainer(ctr *Container, pod *Pod) error { if opts.InterfaceName == "" { return fmt.Errorf("network interface name cannot be an empty string: %w", define.ErrInvalidArg) } - // always add the short id as alias for docker compat - opts.Aliases = append(opts.Aliases, ctr.config.ID[:12]) optBytes, err := json.Marshal(opts) if err != nil { return fmt.Errorf("marshalling network options JSON for container %s: %w", ctr.ID(), err) diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index dcd0bd53e5..8c2583fea7 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -282,6 +282,8 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai return nil, errors.New("failed to find free network interface name") } } + // always add the short id as alias for docker compat + opts.Aliases = append(opts.Aliases, ctr.config.ID[:12]) normalizeNetworks[netName] = opts } diff --git a/libpod/sqlite_state_internal.go b/libpod/sqlite_state_internal.go index 7143978a3a..1aa39cbe1a 100644 --- a/libpod/sqlite_state_internal.go +++ b/libpod/sqlite_state_internal.go @@ -385,12 +385,6 @@ func (s *SQLiteState) rewriteContainerConfig(ctr *Container, newCfg *ContainerCo } func (s *SQLiteState) addContainer(ctr *Container) (defErr error) { - for net := range ctr.config.Networks { - opts := ctr.config.Networks[net] - opts.Aliases = append(opts.Aliases, ctr.config.ID[:12]) - ctr.config.Networks[net] = opts - } - configJSON, err := json.Marshal(ctr.config) if err != nil { return fmt.Errorf("marshalling container config json: %w", err) From 5583358f62325902823f0676b1b1525909f9aa79 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Tue, 11 Jul 2023 15:38:24 +0200 Subject: [PATCH 27/34] add hostname to network alias We use the name as alias but using the hostname makes also sense and this is what docker does. We have to keep the short id as well for docker compat. While adding some tests I removed some duplicated tests that were executed twice for nv for no reason. Fixes #17370 Signed-off-by: Paul Holzinger --- libpod/networking_common.go | 13 +++++++-- libpod/runtime_ctr.go | 3 +- test/e2e/run_networking_test.go | 51 ++------------------------------- test/system/500-networking.bats | 6 ++-- 4 files changed, 19 insertions(+), 54 deletions(-) diff --git a/libpod/networking_common.go b/libpod/networking_common.go index 54b08240dd..971b69b807 100644 --- a/libpod/networking_common.go +++ b/libpod/networking_common.go @@ -513,8 +513,7 @@ func (c *Container) NetworkConnect(nameOrID, netName string, netOpts types.PerNe // get network status before we connect networkStatus := c.getNetworkStatus() - // always add the short id as alias for docker compat - netOpts.Aliases = append(netOpts.Aliases, c.config.ID[:12]) + netOpts.Aliases = append(netOpts.Aliases, getExtraNetworkAliases(c)...) if netOpts.InterfaceName == "" { netOpts.InterfaceName = getFreeInterfaceName(networks) @@ -639,6 +638,16 @@ func getFreeInterfaceName(networks map[string]types.PerNetworkOptions) string { return "" } +func getExtraNetworkAliases(c *Container) []string { + // always add the short id as alias for docker compat + alias := []string{c.config.ID[:12]} + // if an explicit hostname was set add it as well + if c.config.Spec.Hostname != "" { + alias = append(alias, c.config.Spec.Hostname) + } + return alias +} + // DisconnectContainerFromNetwork removes a container from its network func (r *Runtime) DisconnectContainerFromNetwork(nameOrID, netName string, force bool) error { ctr, err := r.LookupContainer(nameOrID) diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index 8c2583fea7..c9fab56cb4 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -282,8 +282,7 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (_ *Contai return nil, errors.New("failed to find free network interface name") } } - // always add the short id as alias for docker compat - opts.Aliases = append(opts.Aliases, ctr.config.ID[:12]) + opts.Aliases = append(opts.Aliases, getExtraNetworkAliases(ctr)...) normalizeNetworks[netName] = opts } diff --git a/test/e2e/run_networking_test.go b/test/e2e/run_networking_test.go index e4a0d6ea80..19809203b5 100644 --- a/test/e2e/run_networking_test.go +++ b/test/e2e/run_networking_test.go @@ -1108,7 +1108,8 @@ EXPOSE 2004-2005/tcp`, ALPINE) Expect(session).Should(Exit(0)) pod2 := "testpod2" - session = podmanTest.Podman([]string{"pod", "create", "--network", net, "--name", pod2}) + hostname := "hostn1" + session = podmanTest.Podman([]string{"pod", "create", "--network", net, "--name", pod2, "--hostname", hostname}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) @@ -1128,40 +1129,8 @@ EXPOSE 2004-2005/tcp`, ALPINE) session = podmanTest.Podman([]string{"run", "--name", "con4", "--network", net, ALPINE, "nslookup", pod2 + ".dns.podman"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) - }) - - It("podman run check dnsname plugin with Netavark", func() { - SkipIfCNI(podmanTest) - pod := "testpod" - session := podmanTest.Podman([]string{"pod", "create", "--name", pod}) - session.WaitWithDefaultTimeout() - Expect(session).Should(Exit(0)) - - net := createNetworkName("IntTest") - session = podmanTest.Podman([]string{"network", "create", net}) - session.WaitWithDefaultTimeout() - defer podmanTest.removeNetwork(net) - Expect(session).Should(Exit(0)) - - pod2 := "testpod2" - session = podmanTest.Podman([]string{"pod", "create", "--network", net, "--name", pod2}) - session.WaitWithDefaultTimeout() - Expect(session).Should(Exit(0)) - - session = podmanTest.Podman([]string{"run", "--name", "con1", "--network", net, ALPINE, "nslookup", "con1"}) - session.WaitWithDefaultTimeout() - Expect(session).Should(Exit(0)) - - session = podmanTest.Podman([]string{"run", "--name", "con2", "--pod", pod, "--network", net, ALPINE, "nslookup", "con2"}) - session.WaitWithDefaultTimeout() - Expect(session).Should(Exit(0)) - session = podmanTest.Podman([]string{"run", "--name", "con3", "--pod", pod2, ALPINE, "nslookup", "con1"}) - session.WaitWithDefaultTimeout() - Expect(session).Should(Exit(1)) - Expect(session.ErrorToString()).To(ContainSubstring("can't resolve 'con1'")) - - session = podmanTest.Podman([]string{"run", "--name", "con4", "--network", net, ALPINE, "nslookup", pod2 + ".dns.podman"}) + session = podmanTest.Podman([]string{"run", "--network", net, ALPINE, "nslookup", hostname}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) }) @@ -1179,20 +1148,6 @@ EXPOSE 2004-2005/tcp`, ALPINE) Expect(session.OutputToString()).To(ContainSubstring("search dns.podman")) }) - It("podman run check dnsname adds dns search domain with Netavark", func() { - SkipIfCNI(podmanTest) - net := createNetworkName("dnsname") - session := podmanTest.Podman([]string{"network", "create", net}) - session.WaitWithDefaultTimeout() - defer podmanTest.removeNetwork(net) - Expect(session).Should(Exit(0)) - - session = podmanTest.Podman([]string{"run", "--network", net, ALPINE, "cat", "/etc/resolv.conf"}) - session.WaitWithDefaultTimeout() - Expect(session).Should(Exit(0)) - Expect(session.OutputToString()).To(ContainSubstring("search dns.podman")) - }) - It("Rootless podman run with --net=bridge works and connects to default network", func() { // This is harmless when run as root, so we'll just let it run. ctrName := "testctr" diff --git a/test/system/500-networking.bats b/test/system/500-networking.bats index fb5b722d81..0d64989219 100644 --- a/test/system/500-networking.bats +++ b/test/system/500-networking.bats @@ -471,8 +471,10 @@ load helpers.network run_podman run -d --network $netname $IMAGE top background_cid=$output + local hostname=host-$(random_string 10) # Run a httpd container on first network with exposed port run_podman run -d -p "$HOST_PORT:80" \ + --hostname $hostname \ --network $netname \ -v $INDEX1:/var/www/index.txt:Z \ -w /var/www \ @@ -490,7 +492,7 @@ load helpers.network # check network alias for container short id run_podman inspect $cid --format "{{(index .NetworkSettings.Networks \"$netname\").Aliases}}" - is "$output" "[${cid:0:12}]" "short container id in network aliases" + is "$output" "[${cid:0:12} $hostname]" "short container id and hostname in network aliases" # check /etc/hosts for our entry run_podman exec $cid cat /etc/hosts @@ -550,7 +552,7 @@ load helpers.network # check network2 alias for container short id run_podman inspect $cid --format "{{(index .NetworkSettings.Networks \"$netname2\").Aliases}}" - is "$output" "[${cid:0:12}]" "short container id in network aliases" + is "$output" "[${cid:0:12} $hostname]" "short container id and hostname in network2 aliases" # curl should work run curl --max-time 3 -s $SERVER/index.txt From 9d0470f125482a55a1b4d5d8ef38529cb505b485 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 12 Jul 2023 14:04:03 +0200 Subject: [PATCH 28/34] netavark: macvlan networks keep custom nameservers The change to use the custom dns server in aardvark-dns caused a regression here because macvlan networks never returned the nameservers in netavark and it also does not make sense to do so. Instead check here if we got any network nameservers, if not we then use the ones from the config if set otherwise fallback to host servers. Fixes #19169 Signed-off-by: Paul Holzinger --- libpod/container_internal_common.go | 6 +++--- test/e2e/run_networking_test.go | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index 87794eeba7..13fe44e734 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -2077,12 +2077,12 @@ func (c *Container) addResolvConf() error { // If NetworkBackend is `netavark` do not populate `/etc/resolv.conf` // with custom dns server since after https://github.com/containers/netavark/pull/452 - // netavark will always set required `nameservers` in statsBlock and libpod + // netavark will always set required `nameservers` in StatusBlock and libpod // will correctly populate `networkNameServers`. Also see https://github.com/containers/podman/issues/16172 // Exception: Populate `/etc/resolv.conf` if container is not connected to any network - // ( i.e len(netStatus)==0 ) since in such case netavark is not invoked at all. - if networkBackend != string(types.Netavark) || len(netStatus) == 0 { + // with dns enabled then we do not get any nameservers back. + if networkBackend != string(types.Netavark) || len(networkNameServers) == 0 { nameservers = append(nameservers, c.runtime.config.Containers.DNSServers...) for _, ip := range c.config.DNSServer { nameservers = append(nameservers, ip.String()) diff --git a/test/e2e/run_networking_test.go b/test/e2e/run_networking_test.go index 19809203b5..fc185ca8b2 100644 --- a/test/e2e/run_networking_test.go +++ b/test/e2e/run_networking_test.go @@ -1186,4 +1186,21 @@ EXPOSE 2004-2005/tcp`, ALPINE) Expect(session).Should(Exit(0)) Expect(session.OutputToStringArray()).To(HaveLen(4), "output should only show link local address") }) + + It("podman run with macvlan network", func() { + net := "mv-" + stringid.GenerateRandomID() + session := podmanTest.Podman([]string{"network", "create", "-d", "macvlan", "--subnet", "10.10.0.0/24", net}) + session.WaitWithDefaultTimeout() + defer podmanTest.removeNetwork(net) + Expect(session).Should(Exit(0)) + + // use options and search to make sure we get the same resolv.conf everywhere + run := podmanTest.Podman([]string{"run", "--network", net, "--dns", "127.0.0.128", + "--dns-option", "ndots:1", "--dns-search", ".", ALPINE, "cat", "/etc/resolv.conf"}) + run.WaitWithDefaultTimeout() + Expect(run).Should(Exit(0)) + Expect(string(run.Out.Contents())).To(Equal(`nameserver 127.0.0.128 +options ndots:1 +`)) + }) }) From 7b3d47c3b9998e04b970f73a83fbf427906c8058 Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Mon, 10 Jul 2023 15:59:47 -0400 Subject: [PATCH 29/34] Fix container errors not being sent via pod removal API When I reworked pod removal to provide more detailed errors (including per-container errors, not just a single multierror with all errors squashed), I made it part of the struct returned by the REST API and assumed that would be enough to get errors through to clients. Unfortunately, in case of an overarching error removing the pod (as any error with any container would cause), we don't send the response struct that would include the container errors - we just send a standardized REST error. We could work around this with custom, potentially backwards incompatible error handling for the REST pod delete endpoint, or we could just do what was done before, and package up all the errors in a multierror to send to the other side. Of those options, the multierror seems far simpler. Fixes #19159 Signed-off-by: Matt Heon --- pkg/api/handlers/libpod/pods.go | 14 ++++++++++++++ test/apiv2/40-pods.at | 19 +++++++++++++++++-- test/system/200-pod.bats | 10 ++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/pkg/api/handlers/libpod/pods.go b/pkg/api/handlers/libpod/pods.go index c13af63136..b01bf8f8df 100644 --- a/pkg/api/handlers/libpod/pods.go +++ b/pkg/api/handlers/libpod/pods.go @@ -20,6 +20,7 @@ import ( "github.com/containers/podman/v4/pkg/specgenutil" "github.com/containers/podman/v4/pkg/util" "github.com/gorilla/schema" + "github.com/hashicorp/go-multierror" "github.com/sirupsen/logrus" ) @@ -248,6 +249,19 @@ func PodDelete(w http.ResponseWriter, r *http.Request) { } ctrs, err := runtime.RemovePod(r.Context(), pod, true, query.Force, query.Timeout) if err != nil { + if len(ctrs) > 0 { + // We have container errors to send as well. + // Since we're just writing an error, and we don't want + // special error-handling for just this endpoint: use a + // multierror to package up all container errors. + var allCtrErrors error + for _, ctrErr := range ctrs { + allCtrErrors = multierror.Append(allCtrErrors, ctrErr) + } + + err = fmt.Errorf("%w. %s", err, allCtrErrors.Error()) + } + utils.Error(w, http.StatusInternalServerError, err) return } diff --git a/test/apiv2/40-pods.at b/test/apiv2/40-pods.at index 0e0f1cb18c..71af150772 100644 --- a/test/apiv2/40-pods.at +++ b/test/apiv2/40-pods.at @@ -3,6 +3,9 @@ # test pod-related endpoints # +# Need to make 1 container +podman pull $IMAGE &>/dev/null + t GET "libpod/pods/json (clean slate at start)" 200 '[]' t POST libpod/pods/create name=foo 201 .Id~[0-9a-f]\\{64\\} @@ -28,11 +31,17 @@ t POST "libpod/pods/create (dup pod)" name=foo 409 \ #t POST libpod/pods/create a=b 400 .cause='bad parameter' # FIXME: unimplemented +# Some tests require a single running container. +# Don't bother saving CID, it will be removed as part of the pod. +t POST libpod/containers/create?name=testctr Image=${IMAGE} Pod=foo Entrypoint='["top"]' 201 \ + .Id~[0-9a-f]\\{64\\} +cid=$(jq -r '.Id' <<<"$output") + t POST libpod/pods/foo/start 200 \ .Errs=null \ .Id=$pod_id -t POST libpod/pods/foo/start 304 \ +t POST libpod/pods/foo/start 304 t POST libpod/pods/fakename/start 404 \ .cause="no such pod" \ @@ -52,11 +61,17 @@ t POST "libpod/pods/fakename/unpause" 404\ .cause="no such pod" \ .message="no pod with name or ID fakename found: no such pod" +t DELETE libpod/pods/foo?force=false 500 \ + .cause="removing pod containers" \ + .message~".*cannot remove container .* as it is running.*" t POST libpod/pods/foo/stop 200 \ .Errs=null \ .Id=$pod_id +# Remove container as it is no longer required +t DELETE libpod/containers/$cid 200 .[0].Id=$cid + t POST "libpod/pods/foo/stop (pod is already stopped)" 304 t POST "libpod/pods/fakename/stop" 404\ .cause="no such pod" \ @@ -85,7 +100,7 @@ t POST "libpod/pods/bar/stop?t=invalid" 400 \ .cause="schema: error converting value for \"t\"" \ .message~"failed to parse parameters for" -podman run -d --pod bar busybox sleep 999 +podman run -d --pod bar busybox top t POST libpod/pods/bar/stop?t=1 200 \ .Errs=null \ diff --git a/test/system/200-pod.bats b/test/system/200-pod.bats index 442517b461..f159ed6ee0 100644 --- a/test/system/200-pod.bats +++ b/test/system/200-pod.bats @@ -55,6 +55,16 @@ function teardown() { is "$output" ".*0 \+1 \+0 \+[0-9. ?s]\+/pause" "there is a /pause container" fi + # Cannot remove pod while containers are still running. Error messages + # differ slightly between local and remote; these are the common elements. + run_podman 125 pod rm $podid + assert "${lines[0]}" =~ "Error: not all containers could be removed from pod $podid: removing pod containers.*" \ + "pod rm while busy: error message line 1 of 3" + assert "${lines[1]}" =~ "cannot remove container .* as it is running - running or paused containers cannot be removed without force: container state improper" \ + "pod rm while busy: error message line 2 of 3" + assert "${lines[2]}" =~ "cannot remove container .* as it is running - running or paused containers cannot be removed without force: container state improper" \ + "pod rm while busy: error message line 3 of 3" + # Clean up run_podman --noout pod rm -f -t 0 $podid is "$output" "" "output should be empty" From afe48ba36c22e8373173a8cfaee3f4fb8aabb8de Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Thu, 29 Jun 2023 11:34:49 +0200 Subject: [PATCH 30/34] auto update: fix usage of --authfile The --authfile flag has been ignored. Fix that and add a test to make sure we won't regress another time. Requires a new --tls-verify flag to actually test the code. Also bump c/common since common/pull/1538 is required to correctly check for updates. Note that I had to use the go-mod-edit-replace trick on c/common as c/buildah would otherwise be moved back to 1.30. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2218315 Signed-off-by: Valentin Rothberg --- cmd/podman/auto-update.go | 10 ++- docs/source/markdown/options/tls-verify.md | 2 +- .../markdown/podman-auto-update.1.md.in | 1 + pkg/autoupdate/autoupdate.go | 14 +++- pkg/domain/entities/auto-update.go | 5 ++ test/system/255-auto-update.bats | 64 +++++++++++++++++++ 6 files changed, 92 insertions(+), 4 deletions(-) diff --git a/cmd/podman/auto-update.go b/cmd/podman/auto-update.go index 6a04464225..07a1a0c03e 100644 --- a/cmd/podman/auto-update.go +++ b/cmd/podman/auto-update.go @@ -8,6 +8,7 @@ import ( "github.com/containers/common/pkg/auth" "github.com/containers/common/pkg/completion" "github.com/containers/common/pkg/report" + "github.com/containers/image/v5/types" "github.com/containers/podman/v4/cmd/podman/common" "github.com/containers/podman/v4/cmd/podman/registry" "github.com/containers/podman/v4/pkg/domain/entities" @@ -17,7 +18,8 @@ import ( type cliAutoUpdateOptions struct { entities.AutoUpdateOptions - format string + format string + tlsVerify bool } var ( @@ -56,6 +58,8 @@ func init() { flags.StringVar(&autoUpdateOptions.format, "format", "", "Change the output format to JSON or a Go template") _ = autoUpdateCommand.RegisterFlagCompletionFunc("format", common.AutocompleteFormat(&autoUpdateOutput{})) + + flags.BoolVarP(&autoUpdateOptions.tlsVerify, "tls-verify", "", true, "Require HTTPS and verify certificates when contacting registries") } func autoUpdate(cmd *cobra.Command, args []string) error { @@ -64,6 +68,10 @@ func autoUpdate(cmd *cobra.Command, args []string) error { return fmt.Errorf("`%s` takes no arguments", cmd.CommandPath()) } + if cmd.Flags().Changed("tls-verify") { + autoUpdateOptions.InsecureSkipTLSVerify = types.NewOptionalBool(!autoUpdateOptions.tlsVerify) + } + allReports, failures := registry.ContainerEngine().AutoUpdate(registry.GetContext(), autoUpdateOptions.AutoUpdateOptions) if allReports == nil { return errorhandling.JoinErrors(failures) diff --git a/docs/source/markdown/options/tls-verify.md b/docs/source/markdown/options/tls-verify.md index d694bab26d..13c2a6e634 100644 --- a/docs/source/markdown/options/tls-verify.md +++ b/docs/source/markdown/options/tls-verify.md @@ -1,5 +1,5 @@ ####> This option file is used in: -####> podman build, container runlabel, create, kube play, login, manifest add, manifest create, manifest inspect, manifest push, pull, push, run, search +####> podman auto update, build, container runlabel, create, kube play, login, manifest add, manifest create, manifest inspect, manifest push, pull, push, run, search ####> If file is edited, make sure the changes ####> are applicable to all of those. #### **--tls-verify** diff --git a/docs/source/markdown/podman-auto-update.1.md.in b/docs/source/markdown/podman-auto-update.1.md.in index b44999496b..c615567ff9 100644 --- a/docs/source/markdown/podman-auto-update.1.md.in +++ b/docs/source/markdown/podman-auto-update.1.md.in @@ -79,6 +79,7 @@ Please note that detecting if a systemd unit has failed is best done by the cont For a container to send the READY message via SDNOTIFY it must be created with the `--sdnotify=container` option (see podman-run(1)). The application running inside the container can then execute `systemd-notify --ready` when ready or use the sdnotify bindings of the specific programming language (e.g., sd_notify(3)). +@@option tls-verify ## EXAMPLES Autoupdate with registry policy diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go index 4f2a4a0770..dd8c7b80c6 100644 --- a/pkg/autoupdate/autoupdate.go +++ b/pkg/autoupdate/autoupdate.go @@ -282,7 +282,10 @@ func (t *task) registryUpdateAvailable(ctx context.Context) (bool, error) { if err != nil { return false, err } - options := &libimage.HasDifferentDigestOptions{AuthFilePath: t.authfile} + options := &libimage.HasDifferentDigestOptions{ + AuthFilePath: t.authfile, + InsecureSkipTLSVerify: t.auto.options.InsecureSkipTLSVerify, + } return t.image.HasDifferentDigest(ctx, remoteRef, options) } @@ -296,6 +299,7 @@ func (t *task) registryUpdate(ctx context.Context) error { pullOptions := &libimage.PullOptions{} pullOptions.AuthFilePath = t.authfile pullOptions.Writer = os.Stderr + pullOptions.InsecureSkipTLSVerify = t.auto.options.InsecureSkipTLSVerify if _, err := t.auto.runtime.LibimageRuntime().Pull(ctx, t.rawImageName, config.PullPolicyAlways, pullOptions); err != nil { return err } @@ -416,8 +420,14 @@ func (u *updater) assembleTasks(ctx context.Context) []error { continue } + // Use user-specified auth file (CLI or env variable) unless + // the container was created with the auth-file label. + authfile := u.options.Authfile + if fromContainer, ok := labels[define.AutoUpdateAuthfileLabel]; ok { + authfile = fromContainer + } t := task{ - authfile: labels[define.AutoUpdateAuthfileLabel], + authfile: authfile, auto: u, container: ctr, policy: policy, diff --git a/pkg/domain/entities/auto-update.go b/pkg/domain/entities/auto-update.go index 5ea2cdf150..195f27f84a 100644 --- a/pkg/domain/entities/auto-update.go +++ b/pkg/domain/entities/auto-update.go @@ -1,5 +1,7 @@ package entities +import "github.com/containers/image/v5/types" + // AutoUpdateOptions are the options for running auto-update. type AutoUpdateOptions struct { // Authfile to use when contacting registries. @@ -11,6 +13,9 @@ type AutoUpdateOptions struct { // If restarting the service with the new image failed, restart it // another time with the previous image. Rollback bool + // Allow contacting registries over HTTP, or HTTPS with failed TLS + // verification. Note that this does not affect other TLS connections. + InsecureSkipTLSVerify types.OptionalBool } // AutoUpdateReport contains the results from running auto-update. diff --git a/test/system/255-auto-update.bats b/test/system/255-auto-update.bats index 7fef945299..be5b282c8d 100644 --- a/test/system/255-auto-update.bats +++ b/test/system/255-auto-update.bats @@ -4,6 +4,8 @@ # load helpers +load helpers.network +load helpers.registry load helpers.systemd SNAME_FILE=$BATS_TMPDIR/services @@ -47,6 +49,7 @@ function teardown() { # 4. Generate the service file from the container # 5. Remove the origin container # 6. Start the container from service +# 7. Use this fully-qualified image instead of 2) function generate_service() { local target_img_basename=$1 local autoupdate=$2 @@ -64,6 +67,9 @@ function generate_service() { # IMPORTANT: variable 'cname' is passed (out of scope) up to caller! cname=c_${autoupdate//\'/}_$(random_string) target_img="quay.io/libpod/$target_img_basename:latest" + if [[ -n "$7" ]]; then + target_img="$7" + fi if [[ -z "$noTag" ]]; then run_podman tag $IMAGE $target_img @@ -623,4 +629,62 @@ EOF systemctl daemon-reload } +@test "podman-auto-update --authfile" { + # Test the three supported ways of using authfiles with auto updates + # 1) Passed via --authfile CLI flag + # 2) Passed via the REGISTRY_AUTH_FILE env variable + # 3) Via a label at container creation where 1) and 2) will be ignored + + registry=localhost:${PODMAN_LOGIN_REGISTRY_PORT} + image_on_local_registry=$registry/name:tag + authfile=$PODMAN_TMPDIR/authfile.json + + # First, start the registry and populate the authfile that we can use for the test. + start_registry + run_podman login --authfile=$authfile \ + --tls-verify=false \ + --username ${PODMAN_LOGIN_USER} \ + --password ${PODMAN_LOGIN_PASS} \ + $registry + + run_podman tag $IMAGE $image_on_local_registry + run_podman push --tls-verify=false --creds "${PODMAN_LOGIN_USER}:${PODMAN_LOGIN_PASS}" $image_on_local_registry + + # Generate a systemd service with the "registry" auto-update policy running + # "top" inside the image we just pushed to the local registry. + generate_service "" registry top "" "" "" $image_on_local_registry + ctr=$cname + _wait_service_ready container-$ctr.service + + run_podman 125 auto-update + is "$output" \ + ".*Error: checking image updates for container .*: x509: .*" + + run_podman 125 auto-update --tls-verify=false + is "$output" \ + ".*Error: checking image updates for container .*: authentication required" + + # Test 1) + run_podman auto-update --authfile=$authfile --tls-verify=false --dry-run --format "{{.Unit}},{{.Image}},{{.Updated}},{{.Policy}}" + is "$output" "container-$ctr.service,$image_on_local_registry,false,registry" "auto-update works with authfile" + + # Test 2) + REGISTRY_AUTH_FILE=$authfile run_podman auto-update --tls-verify=false --dry-run --format "{{.Unit}},{{.Image}},{{.Updated}},{{.Policy}}" + is "$output" "container-$ctr.service,$image_on_local_registry,false,registry" "auto-update works with env var" + systemctl stop container-$ctr.service + run_podman rm -f -t0 --ignore $ctr + + # Create a container with the auth-file label + generate_service "" registry top "--label io.containers.autoupdate.authfile=$authfile" "" "" $image_on_local_registry + ctr=$cname + _wait_service_ready container-$ctr.service + + # Test 3) + # Also make sure that the label takes precedence over the CLI flag. + run_podman auto-update --authfile=/dev/null --tls-verify=false --dry-run --format "{{.Unit}},{{.Image}},{{.Updated}},{{.Policy}}" + is "$output" "container-$ctr.service,$image_on_local_registry,false,registry" "auto-update works with authfile container label" + run_podman rm -f -t0 --ignore $ctr + run_podman rmi $image_on_local_registry +} + # vim: filetype=sh From 732a02c57fd93971177aa75fcad3f68e5a0ba96b Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 4 Jul 2023 15:53:12 +0200 Subject: [PATCH 31/34] machine start: qemu: adjust backoffs Make sure that starting a qemu machine uses proper exponential backoffs and that a single variable isn't shared across multiple backoffs. DO NOT BACKPORT: I want to avoid backporting this PR to the upcoming 4.6 release as it increases the flakiness of machine start (see #17403). On my M2 machine, the flake rate seems to have increased with this change and I strongly suspect that additional/redundant sleep after waiting for the machine to be running and listening reduced the flakiness. My hope is to have more predictable behavior and find the sources of the flakes soon. [NO NEW TESTS NEEDED] - still too flaky to add a test to CI. Signed-off-by: Valentin Rothberg --- pkg/machine/qemu/machine.go | 47 ++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index fb792cc425..8806f3f633 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -481,9 +481,11 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error { conn net.Conn err error qemuSocketConn net.Conn - wait = time.Millisecond * 500 ) + defaultBackoff := 500 * time.Millisecond + maxBackoffs := 6 + v.Starting = true if err := v.writeConfig(); err != nil { return fmt.Errorf("writing JSON file: %w", err) @@ -535,13 +537,17 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error { if err := v.QMPMonitor.Address.Delete(); err != nil { return err } - for i := 0; i < 6; i++ { + + backoff := defaultBackoff + for i := 0; i < maxBackoffs; i++ { + if i > 0 { + time.Sleep(backoff) + backoff *= 2 + } qemuSocketConn, err = net.Dial("unix", v.QMPMonitor.Address.GetPath()) if err == nil { break } - time.Sleep(wait) - wait++ } if err != nil { return err @@ -624,7 +630,12 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error { // The socket is not made until the qemu process is running so here // we do a backoff waiting for it. Once we have a conn, we break and // then wait to read it. - for i := 0; i < 6; i++ { + backoff = defaultBackoff + for i := 0; i < maxBackoffs; i++ { + if i > 0 { + time.Sleep(backoff) + backoff *= 2 + } conn, err = net.Dial("unix", filepath.Join(socketPath, "podman", v.Name+"_ready.sock")) if err == nil { break @@ -634,8 +645,6 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error { if err != nil { return err } - time.Sleep(wait) - wait++ } if err != nil { return err @@ -655,18 +664,24 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error { } } if len(v.Mounts) > 0 { - state, err := v.State(true) - if err != nil { - return err - } - listening := v.isListening() - for state != machine.Running || !listening { - time.Sleep(100 * time.Millisecond) - state, err = v.State(true) + connected := false + backoff = 500 * time.Millisecond + for i := 0; i < maxBackoffs; i++ { + if i > 0 { + time.Sleep(backoff) + backoff *= 2 + } + state, err := v.State(true) if err != nil { return err } - listening = v.isListening() + if state == machine.Running && v.isListening() { + connected = true + break + } + } + if !connected { + return fmt.Errorf("machine did not transition into running state") } } for _, mount := range v.Mounts { From 624bb83500bb7a76d5c3bf3c287935bf896fe395 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 12 Jul 2023 15:19:05 +0200 Subject: [PATCH 32/34] machine start: qemu: wait for SSH readiness During the exponential backoff waiting for the machine to be fully up and running, also make sure that SSH is ready. The systemd dependencies of the ready.service include the sshd.service among others but that is not enough. Other CoreOS users reported the same issue on IRC, so I feel fairly confident to use the pragmatic approach of making sure SSH works on the client side. #17403 is quite old and there are other pressing machine issues that need attention. [NO NEW TESTS NEEDED] Fixes: #17403 Signed-off-by: Valentin Rothberg --- pkg/machine/qemu/machine.go | 54 +++++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go index 8806f3f633..a118285f7b 100644 --- a/pkg/machine/qemu/machine.go +++ b/pkg/machine/qemu/machine.go @@ -663,27 +663,47 @@ func (v *MachineVM) Start(name string, opts machine.StartOptions) error { _ = v.writeConfig() } } - if len(v.Mounts) > 0 { - connected := false - backoff = 500 * time.Millisecond - for i := 0; i < maxBackoffs; i++ { - if i > 0 { - time.Sleep(backoff) - backoff *= 2 - } - state, err := v.State(true) - if err != nil { - return err - } - if state == machine.Running && v.isListening() { - connected = true - break + if len(v.Mounts) == 0 { + v.waitAPIAndPrintInfo(forwardState, forwardSock, opts.NoInfo) + return nil + } + + connected := false + backoff = defaultBackoff + var sshError error + for i := 0; i < maxBackoffs; i++ { + if i > 0 { + time.Sleep(backoff) + backoff *= 2 + } + state, err := v.State(true) + if err != nil { + return err + } + if state == machine.Running && v.isListening() { + // Also make sure that SSH is up and running. The + // ready service's dependencies don't fully make sure + // that clients can SSH into the machine immediately + // after boot. + // + // CoreOS users have reported the same observation but + // the underlying source of the issue remains unknown. + if sshError = v.SSH(name, machine.SSHOptions{Args: []string{"true"}}); sshError != nil { + logrus.Debugf("SSH readiness check for machine failed: %v", sshError) + continue } + connected = true + break } - if !connected { - return fmt.Errorf("machine did not transition into running state") + } + if !connected { + msg := "machine did not transition into running state" + if sshError != nil { + return fmt.Errorf("%s: ssh error: %v", msg, sshError) } + return errors.New(msg) } + for _, mount := range v.Mounts { if !opts.Quiet { fmt.Printf("Mounting volume... %s:%s\n", mount.Source, mount.Target) From 8fffcf4d60f898c540e77fa8b971bb1ad249f706 Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Thu, 29 Jun 2023 14:26:46 -0600 Subject: [PATCH 33/34] system tests: refactor registry code The podman-login tests have accumulated much cruft over the years, because that's the only place where we run a local registry, and the process was crufty: we actually start/stopped the registry as the first & last tests of the file. Meaning, you couldn't do 'hack/bats 150:just-one-test' because that would skip the registry start. And just now, a completely unrelated test has had to be shoved into the login file. This PR revamps the whole thing, by adding a new registry helper module that can be used anywhere. And, once the registry is started, it just stays running until the end of tests. (This requires BATS 1.7 or greater). Signed-off-by: Ed Santiago --- hack/podman-registry | 7 +- test/system/150-login.bats | 117 +----------------------------- test/system/helpers.bash | 29 +++++++- test/system/helpers.registry.bash | 116 +++++++++++++++++++++++++++++ test/system/setup_suite.bash | 29 ++++++++ 5 files changed, 181 insertions(+), 117 deletions(-) create mode 100644 test/system/helpers.registry.bash create mode 100644 test/system/setup_suite.bash diff --git a/hack/podman-registry b/hack/podman-registry index ef90993cba..c539a5b6e9 100755 --- a/hack/podman-registry +++ b/hack/podman-registry @@ -242,7 +242,12 @@ function do_stop() { podman rm -f registry # Use straight podman, not our alias function, to avoid 'overlay: EBUSY' - ${PODMAN} unshare rm -rf ${PODMAN_REGISTRY_WORKDIR} + cmd="rm -rf ${PODMAN_REGISTRY_WORKDIR}" + if [[ $(id -u) -eq 0 ]]; then + $cmd + else + ${PODMAN} unshare $cmd + fi } diff --git a/test/system/150-login.bats b/test/system/150-login.bats index d1d94578d1..c6d6501454 100644 --- a/test/system/150-login.bats +++ b/test/system/150-login.bats @@ -5,31 +5,8 @@ load helpers load helpers.network +load helpers.registry -############################################################################### -# BEGIN one-time envariable setup - -# Create a scratch directory; our podman registry will run from here. We -# also use it for other temporary files like authfiles. -if [ -z "${PODMAN_LOGIN_WORKDIR}" ]; then - export PODMAN_LOGIN_WORKDIR=$(mktemp -d --tmpdir=${BATS_TMPDIR:-${TMPDIR:-/tmp}} podman_bats_login.XXXXXX) -fi - -# Randomly-generated username and password -if [ -z "${PODMAN_LOGIN_USER}" ]; then - export PODMAN_LOGIN_USER="user$(random_string 4)" - export PODMAN_LOGIN_PASS=$(random_string 15) -fi - -# Randomly-assigned port in the 5xxx range -if [ -z "${PODMAN_LOGIN_REGISTRY_PORT}" ]; then - export PODMAN_LOGIN_REGISTRY_PORT=$(random_free_port) -fi - -# Override any user-set path to an auth file -unset REGISTRY_AUTH_FILE - -# END one-time envariable setup ############################################################################### # BEGIN filtering - none of these tests will work with podman-remote @@ -37,73 +14,11 @@ function setup() { skip_if_remote "none of these tests work with podman-remote" basic_setup + start_registry } # END filtering - none of these tests will work with podman-remote ############################################################################### -# BEGIN first "test" - start a registry for use by other tests -# -# This isn't really a test: it's a helper that starts a local registry. -# Note that we're careful to use a root/runroot separate from our tests, -# so setup/teardown don't clobber our registry image. -# - -@test "podman login [start registry]" { - AUTHDIR=${PODMAN_LOGIN_WORKDIR}/auth - mkdir -p $AUTHDIR - - # Registry image; copy of docker.io, but on our own registry - local REGISTRY_IMAGE="$PODMAN_TEST_IMAGE_REGISTRY/$PODMAN_TEST_IMAGE_USER/registry:2.8" - - # Pull registry image, but into a separate container storage - mkdir -p ${PODMAN_LOGIN_WORKDIR}/root - mkdir -p ${PODMAN_LOGIN_WORKDIR}/runroot - PODMAN_LOGIN_ARGS="--storage-driver=vfs --root ${PODMAN_LOGIN_WORKDIR}/root --runroot ${PODMAN_LOGIN_WORKDIR}/runroot" - # Give it three tries, to compensate for flakes - run_podman ${PODMAN_LOGIN_ARGS} pull $REGISTRY_IMAGE || - run_podman ${PODMAN_LOGIN_ARGS} pull $REGISTRY_IMAGE || - run_podman ${PODMAN_LOGIN_ARGS} pull $REGISTRY_IMAGE - - # Registry image needs a cert. Self-signed is good enough. - CERT=$AUTHDIR/domain.crt - if [ ! -e $CERT ]; then - openssl req -newkey rsa:4096 -nodes -sha256 \ - -keyout $AUTHDIR/domain.key -x509 -days 2 \ - -out $AUTHDIR/domain.crt \ - -subj "/C=US/ST=Foo/L=Bar/O=Red Hat, Inc./CN=localhost" \ - -addext "subjectAltName=DNS:localhost" - fi - - # Copy a cert to another directory for --cert-dir option tests - mkdir -p ${PODMAN_LOGIN_WORKDIR}/trusted-registry-cert-dir - cp $CERT ${PODMAN_LOGIN_WORKDIR}/trusted-registry-cert-dir - - # Store credentials where container will see them - if [ ! -e $AUTHDIR/htpasswd ]; then - htpasswd -Bbn ${PODMAN_LOGIN_USER} ${PODMAN_LOGIN_PASS} \ - > $AUTHDIR/htpasswd - - # In case $PODMAN_TEST_KEEP_LOGIN_REGISTRY is set, for testing later - echo "${PODMAN_LOGIN_USER}:${PODMAN_LOGIN_PASS}" \ - > $AUTHDIR/htpasswd-plaintext - fi - - # Run the registry container. - run_podman '?' ${PODMAN_LOGIN_ARGS} rm -t 0 -f registry - run_podman ${PODMAN_LOGIN_ARGS} run -d \ - -p ${PODMAN_LOGIN_REGISTRY_PORT}:5000 \ - --name registry \ - -v $AUTHDIR:/auth:Z \ - -e "REGISTRY_AUTH=htpasswd" \ - -e "REGISTRY_AUTH_HTPASSWD_REALM=Registry Realm" \ - -e REGISTRY_AUTH_HTPASSWD_PATH=/auth/htpasswd \ - -e REGISTRY_HTTP_TLS_CERTIFICATE=/auth/domain.crt \ - -e REGISTRY_HTTP_TLS_KEY=/auth/domain.key \ - $REGISTRY_IMAGE -} - -# END first "test" - start a registry for use by other tests -############################################################################### # BEGIN actual tests # BEGIN primary podman login/push/pull tests @@ -329,33 +244,5 @@ function _test_skopeo_credential_sharing() { # END cooperation with skopeo # END actual tests ############################################################################### -# BEGIN teardown (remove the registry container) - -@test "podman login [stop registry, clean up]" { - # For manual debugging; user may request keeping the registry running - if [ -n "${PODMAN_TEST_KEEP_LOGIN_REGISTRY}" ]; then - skip "[leaving registry running by request]" - fi - - run_podman --storage-driver=vfs --root ${PODMAN_LOGIN_WORKDIR}/root \ - --runroot ${PODMAN_LOGIN_WORKDIR}/runroot \ - rm -f registry - run_podman --storage-driver=vfs --root ${PODMAN_LOGIN_WORKDIR}/root \ - --runroot ${PODMAN_LOGIN_WORKDIR}/runroot \ - rmi -a - - # By default, clean up - if [ -z "${PODMAN_TEST_KEEP_LOGIN_WORKDIR}" ]; then - rm -rf ${PODMAN_LOGIN_WORKDIR} - fi - - # Make sure socket is closed - if tcp_port_probe $PODMAN_LOGIN_REGISTRY_PORT; then - die "Socket still seems open" - fi -} - -# END teardown (remove the registry container) -############################################################################### # vim: filetype=sh diff --git a/test/system/helpers.bash b/test/system/helpers.bash index 8cbefa2f3a..3f4af313e4 100644 --- a/test/system/helpers.bash +++ b/test/system/helpers.bash @@ -78,9 +78,36 @@ function _prefetch() { fi fi + # Kludge alert. + # Skopeo has no --storage-driver, --root, or --runroot flags; those + # need to be expressed in the destination string inside [brackets]. + # See containers-transports(5). So if we see those options in + # _PODMAN_TEST_OPTS, transmogrify $want into skopeo form. + skopeo_opts='' + driver="$(expr "$_PODMAN_TEST_OPTS" : ".*--storage-driver \([^ ]\+\)" || true)" + if [[ -n "$driver" ]]; then + skopeo_opts+="$driver@" + fi + + altroot="$(expr "$_PODMAN_TEST_OPTS" : ".*--root \([^ ]\+\)" || true)" + if [[ -n "$altroot" ]] && [[ -d "$altroot" ]]; then + skopeo_opts+="$altroot" + + altrunroot="$(expr "$_PODMAN_TEST_OPTS" : ".*--runroot \([^ ]\+\)" || true)" + if [[ -n "$altrunroot" ]] && [[ -d "$altrunroot" ]]; then + skopeo_opts+="+$altrunroot" + fi + fi + + if [[ -n "$skopeo_opts" ]]; then + want="[$skopeo_opts]$want" + fi + # Cached image is now guaranteed to exist. Be sure to load it # with skopeo, not podman, in order to preserve metadata - skopeo copy --all oci-archive:$cachepath containers-storage:$want + cmd="skopeo copy --all oci-archive:$cachepath containers-storage:$want" + echo "$_LOG_PROMPT $cmd" + $cmd } # END tools for fetching & caching test images diff --git a/test/system/helpers.registry.bash b/test/system/helpers.registry.bash new file mode 100644 index 0000000000..c355355657 --- /dev/null +++ b/test/system/helpers.registry.bash @@ -0,0 +1,116 @@ +# -*- bash -*- +# +# helpers for starting/stopping a local registry. +# +# Used primarily in 150-login.bats +# + +############################################################################### +# BEGIN one-time envariable setup + +# Override any user-set path to an auth file +unset REGISTRY_AUTH_FILE + +# END one-time envariable setup +############################################################################### + +# Start a local registry. Only needed on demand (e.g. by 150-login.bats) +# and then only once: if we start, leave it running until final teardown. +function start_registry() { + if [[ -d "$PODMAN_LOGIN_WORKDIR/auth" ]]; then + # Already started + return + fi + + AUTHDIR=${PODMAN_LOGIN_WORKDIR}/auth + mkdir -p $AUTHDIR + + # Registry image; copy of docker.io, but on our own registry + local REGISTRY_IMAGE="$PODMAN_TEST_IMAGE_REGISTRY/$PODMAN_TEST_IMAGE_USER/registry:2.8" + + # Pull registry image, but into a separate container storage + mkdir ${PODMAN_LOGIN_WORKDIR}/root + mkdir ${PODMAN_LOGIN_WORKDIR}/runroot + PODMAN_LOGIN_ARGS="--storage-driver vfs --root ${PODMAN_LOGIN_WORKDIR}/root --runroot ${PODMAN_LOGIN_WORKDIR}/runroot" + # _prefetch() will retry twice on network error, and will also use + # a pre-cached image if present (helpful on dev workstation, not in CI). + _PODMAN_TEST_OPTS="${PODMAN_LOGIN_ARGS}" _prefetch $REGISTRY_IMAGE + + # Registry image needs a cert. Self-signed is good enough. + CERT=$AUTHDIR/domain.crt + if [ ! -e $CERT ]; then + openssl req -newkey rsa:4096 -nodes -sha256 \ + -keyout $AUTHDIR/domain.key -x509 -days 2 \ + -out $AUTHDIR/domain.crt \ + -subj "/C=US/ST=Foo/L=Bar/O=Red Hat, Inc./CN=localhost" \ + -addext "subjectAltName=DNS:localhost" + fi + + # Copy a cert to another directory for --cert-dir option tests + mkdir -p ${PODMAN_LOGIN_WORKDIR}/trusted-registry-cert-dir + cp $CERT ${PODMAN_LOGIN_WORKDIR}/trusted-registry-cert-dir + + # Store credentials where container will see them + htpasswd -Bbn ${PODMAN_LOGIN_USER} ${PODMAN_LOGIN_PASS} > $AUTHDIR/htpasswd + + # In case $PODMAN_TEST_KEEP_LOGIN_REGISTRY is set, for testing later + echo "${PODMAN_LOGIN_USER}:${PODMAN_LOGIN_PASS}" > $AUTHDIR/htpasswd-plaintext + + # Run the registry container. + run_podman ${PODMAN_LOGIN_ARGS} run -d \ + -p 127.0.0.1:${PODMAN_LOGIN_REGISTRY_PORT}:5000 \ + --name registry \ + -v $AUTHDIR:/auth:Z \ + -e "REGISTRY_AUTH=htpasswd" \ + -e "REGISTRY_AUTH_HTPASSWD_REALM=Registry Realm" \ + -e REGISTRY_AUTH_HTPASSWD_PATH=/auth/htpasswd \ + -e REGISTRY_HTTP_TLS_CERTIFICATE=/auth/domain.crt \ + -e REGISTRY_HTTP_TLS_KEY=/auth/domain.key \ + $REGISTRY_IMAGE + cid="$output" + + # wait_for_port isn't enough: that just checks that podman has mapped the port... + wait_for_port 127.0.0.1 ${PODMAN_LOGIN_REGISTRY_PORT} + # ...so we look in container logs for confirmation that registry is running. + _PODMAN_TEST_OPTS="${PODMAN_LOGIN_ARGS}" wait_for_output "listening on .::.:5000" $cid +} + +function stop_registry() { + if [[ ! -d "$PODMAN_LOGIN_WORKDIR/auth" ]]; then + # No registry running + return + fi + + # For manual debugging; user may request keeping the registry running + if [ -n "${PODMAN_TEST_KEEP_LOGIN_REGISTRY}" ]; then + skip "[leaving registry running by request]" + fi + + run_podman --storage-driver vfs \ + --root ${PODMAN_LOGIN_WORKDIR}/root \ + --runroot ${PODMAN_LOGIN_WORKDIR}/runroot \ + rm -f -t0 registry + run_podman --storage-driver vfs \ + --root ${PODMAN_LOGIN_WORKDIR}/root \ + --runroot ${PODMAN_LOGIN_WORKDIR}/runroot \ + rmi -a -f + + # By default, clean up + if [ -z "${PODMAN_TEST_KEEP_LOGIN_WORKDIR}" ]; then + # FIXME: why is this necessary??? If we don't do this, we can't + # rm -rf the workdir, because ..../overlay is mounted + mount | grep ${PODMAN_LOGIN_WORKDIR} | awk '{print $3}' | xargs --no-run-if-empty umount + + if [[ $(id -u) -eq 0 ]]; then + rm -rf ${PODMAN_LOGIN_WORKDIR} + else + # rootless image data is owned by a subuid + run_podman unshare rm -rf ${PODMAN_LOGIN_WORKDIR} + fi + fi + + # Make sure socket is closed + if tcp_port_probe $PODMAN_LOGIN_REGISTRY_PORT; then + die "Socket still seems open" + fi +} diff --git a/test/system/setup_suite.bash b/test/system/setup_suite.bash new file mode 100644 index 0000000000..ff1bed1daa --- /dev/null +++ b/test/system/setup_suite.bash @@ -0,0 +1,29 @@ +# -*- bash -*- +# +# global setup/teardown for the entire system test suite +# +bats_require_minimum_version 1.8.0 + +load helpers +load helpers.network +load helpers.registry + +# Create common environment just in case we end up needing a registry. +# These environment variables will be available to all tests. +function setup_suite() { + # Can't use $BATS_SUITE_TMPDIR because podman barfs: + # Error: the specified runroot is longer than 50 characters + export PODMAN_LOGIN_WORKDIR=$(mktemp -d --tmpdir=${BATS_TMPDIR:-${TMPDIR:-/tmp}} podman-bats-registry.XXXXXX) + + export PODMAN_LOGIN_USER="user$(random_string 4)" + export PODMAN_LOGIN_PASS="pw$(random_string 15)" + + # FIXME: racy! It could be many minutes between now and when we start it. + # To mitigate, we use a range not used anywhere else in system tests. + export PODMAN_LOGIN_REGISTRY_PORT=$(random_free_port 42000-42999) +} + +# Run at the very end of all tests. Useful for cleanup of non-BATS tmpdirs. +function teardown_suite() { + stop_registry +} From be21bc0826ea42661ddca690f73298968011f49f Mon Sep 17 00:00:00 2001 From: Ashley Cui Date: Wed, 12 Jul 2023 17:27:27 -0400 Subject: [PATCH 34/34] Update release notes Signed-off-by: Ashley Cui --- RELEASE_NOTES.md | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index d977327822..b4ef0e1a9f 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -2,6 +2,7 @@ ## 4.6.0 ### Features +- The `podman manifest inspect` command now supports the `--authfile` option, for authentication purposes. - The `podman wait` command now supports `--condition={healthy,unhealthy}`, allowing waits on successful health checks. - The `podman push` command now supports a new option, ` --compression-level`, which specifies the compression level to use ([#18939](https://github.com/containers/podman/issues/18939)). - The `podman machine start` command, when run with `--log-level=debug`, now creates a console window to display the virtual machine while booting. @@ -51,7 +52,18 @@ - Quadlet now supports the `HostName` field, which sets the container's host name, in `.container` files ([#18486](https://github.com/containers/podman/issues/18486)). ### Bugfixes -- The `podman machine start` command now waits for systemd-user sessions to be up, addressing flaky machine starts ([##17403](https://github.com/containers/podman/issues/#17403)). +- Fixed a bug where the `podman machine start` command would fail with a 255 exit code. It now waits for systemd-user sessions to be up, and for SSH to be ready, addressing the flaky machine starts ([#17403](https://github.com/containers/podman/issues/#17403)). +- Fixed a bug where the `podman auto update` command did not correctly use authentication files when contacting container registries. +- Fixed a bug where the `--dns` option to the `podman run` command was ignored for macvlan networks ([#19169](https://github.com/containers/podman/issues/19169)). +- Fixed a bug in the `podman system service` command where setting LISTEN_FDS when listening on TCP would misbehave. +- Fixed a bug where hostnames were not recognized as a network alias. Containers can now resolve other hostnames, in addition to their names ([#17370](https://github.com/containers/podman/issues/17370)). +- Fixed a bug where the `podman pod run` command would error after a reboot on a non-systemd system ([#19175](https://github.com/containers/podman/issues/19175)). +- Fixed a bug where the `--syslog` option returned a fatal error when no syslog server was found ([#19075](https://github.com/containers/podman/issues/19075)). +- Fixed a bug where the `--mount` option would parse the `readonly` option incorrectly ([#18995](https://github.com/containers/podman/issues/18995)). +- Fixed a bug where hook executables invoked by the `podman run` command set an incorrect working directory. It now sets the correct working directory pointing to the container bundle directory ([#18907](https://github.com/containers/podman/issues/18907)). +- Fixed a bug where the `-device-cgroup-rule` option was silently ignored in rootless mode ([#18698](https://github.com/containers/podman/issues/18698)). +- Listing images is now more resilient towards concurrently running image removals. +- Fixed a bug where the `--force` option to the `podman kube down` command would not remove volumes ([#18797](https://github.com/containers/podman/issues/18797)). - Fixed a bug where setting the `--list-tags` option in the `podman search` command would cause the command to ignore the `--format` option ([#18939](https://github.com/containers/podman/issues/18939)). - Fixed a bug where the `podman machine start` command did not properly translate the proxy IP. - Fixed a bug where the `podman auto-update` command would not restart dependent units (specified via `Requires=`) on auto update ([#18926](https://github.com/containers/podman/issues/18926)). @@ -101,7 +113,7 @@ - Updated Buildah to v1.31.0 - Updated the containers/storage library to v1.48.0 - Updated the containers/image library to v5.26.1 -- Updated the containers/common library to v0.55.1 +- Updated the containers/common library to v0.55.2 ## 4.5.1 ### Security