Skip to content

Commit

Permalink
Cherry pick localhost fix and update CI configuration for release-1.1…
Browse files Browse the repository at this point in the history
…9 branch

Cherry pick @vrothberg's "use local image name for pull policy checks" containers#2908
and update the cirrus and git validations so the test will run on this new(ish)
branch.  From @vrothberg:

Some pull policies require to first look up a local image and compare
that to the remote counter part. When looking up the remote image, we
need to make sure to use the name of the local image, if it exists.

This fixes a bug where a short name resolved to an image with the
"localhost/" prefix. This prefix is only used for local image look ups
via shortnames.ResolveLocally. Hence, when looking up the remote
counter part, we must preserve this prefix.

Fixes: containers#2904

Signed-off-by: TomSweeneyRedHat <[email protected]>
  • Loading branch information
TomSweeneyRedHat committed Jan 14, 2021
1 parent 474febf commit acc1a70
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 11 deletions.
2 changes: 1 addition & 1 deletion .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ env:
#### Global variables used for all tasks
####
# Name of the ultimate destination branch for this CI run, PR or post-merge.
DEST_BRANCH: "master"
DEST_BRANCH: "release-1.19"
GOPATH: "/var/tmp/go"
GOSRC: "${GOPATH}/src/github.com/containers/buildah"
# Overrides default location (/tmp/cirrus) for repo clone
Expand Down
24 changes: 15 additions & 9 deletions new.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,27 +103,27 @@ func newContainerIDMappingOptions(idmapOptions *IDMappingOptions) storage.IDMapp
return options
}

func resolveLocalImage(systemContext *types.SystemContext, store storage.Store, options BuilderOptions) (types.ImageReference, string, *storage.Image, error) {
func resolveLocalImage(systemContext *types.SystemContext, store storage.Store, options BuilderOptions) (types.ImageReference, string, string, *storage.Image, error) {
candidates, _, _, err := util.ResolveName(options.FromImage, options.Registry, systemContext, store)
if err != nil {
return nil, "", nil, errors.Wrapf(err, "error resolving local image %q", options.FromImage)
return nil, "", "", nil, errors.Wrapf(err, "error resolving local image %q", options.FromImage)
}
for _, image := range candidates {
img, err := store.Image(image)
for _, imageName := range candidates {
img, err := store.Image(imageName)
if err != nil {
if errors.Cause(err) == storage.ErrImageUnknown {
continue
}
return nil, "", nil, err
return nil, "", "", nil, err
}
ref, err := is.Transport.ParseStoreReference(store, img.ID)
if err != nil {
return nil, "", nil, errors.Wrapf(err, "error parsing reference to image %q", img.ID)
return nil, "", "", nil, errors.Wrapf(err, "error parsing reference to image %q", img.ID)
}
return ref, ref.Transport().Name(), img, nil
return ref, ref.Transport().Name(), imageName, img, nil
}

return nil, "", nil, nil
return nil, "", "", nil, nil
}

func resolveImage(ctx context.Context, systemContext *types.SystemContext, store storage.Store, options BuilderOptions) (types.ImageReference, string, *storage.Image, error) {
Expand All @@ -145,7 +145,7 @@ func resolveImage(ctx context.Context, systemContext *types.SystemContext, store
}
}

localImageRef, _, localImage, err := resolveLocalImage(systemContext, store, options)
localImageRef, _, localImageName, localImage, err := resolveLocalImage(systemContext, store, options)
if err != nil {
return nil, "", nil, err
}
Expand All @@ -166,6 +166,12 @@ func resolveImage(ctx context.Context, systemContext *types.SystemContext, store
}
}

// If we found a local image, we must use it's name.
// See #2904.
if localImageRef != nil {
fromImage = localImageName
}

resolved, err := shortnames.Resolve(systemContext, fromImage)
if err != nil {
return nil, "", nil, err
Expand Down
15 changes: 15 additions & 0 deletions tests/pull.bats
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,21 @@

load helpers

# Regression test for #2904
@test "local-image resolution" {
run_buildah pull -q busybox
iid=$output
run_buildah tag ${iid} localhost/image

# We want to make sure that "image" will always resolve to "localhost/image"
# (given a local image with that name exists). The trick we're using is to
# force a failed pull and look at the error message which *must* include the
# the resolved image name (localhost/image:latest).
run_buildah 125 pull --policy=always image
[[ "$output" == *"Error initializing source docker://localhost/image:latest"* ]]
run_buildah rmi localhost/image ${iid}
}

@test "pull-flags-order-verification" {
run_buildah 125 pull image1 --tls-verify
check_options_flag_err "--tls-verify"
Expand Down
2 changes: 1 addition & 1 deletion tests/validate/git-validation.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ if [[ -z "$(type -P git-validation)" ]]; then
exit 1
fi

GITVALIDATE_EPOCH="${GITVALIDATE_EPOCH:-1f8bf4dba27d9a157f966dad3a1e0f58091091d8}"
GITVALIDATE_EPOCH="${GITVALIDATE_EPOCH:-474febf0dcb3c76c23a88f3b83834c81f20a7e5c}"

OUTPUT_OPTIONS="-q"
if [[ "$CI" == 'true' ]]; then
Expand Down

0 comments on commit acc1a70

Please sign in to comment.