Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Add dry-run mode to copy skopeo-copy #2178

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+79 −17
Open

Add dry-run mode to copy skopeo-copy #2178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a28ea75
Add dry-run mode to copy skopeo-copy
deriamis Dec 8, 2023
2030d63
Fix containerized tests and builds
deriamis Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 38 additions & 17 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,15 @@ GO ?= go
GOBIN := $(shell $(GO) env GOBIN)
GOOS ?= $(shell go env GOOS)
GOARCH ?= $(shell go env GOARCH)
GOPATH ?= $(shell go env GOPATH)

# N/B: This value is managed by Renovate, manual changes are
# possible, as long as they don't disturb the formatting
# (i.e. DO NOT ADD A 'v' prefix!)
GOLANGCI_LINT_VERSION := 1.55.2

GO_MD2MAN_VERSION := 2.0.3

ifeq ($(GOBIN),)
GOBIN := $(GOPATH)/bin
endif
Expand Down Expand Up @@ -82,7 +85,7 @@ endif
CONTAINER_GOSRC = /src/github.com/containers/skopeo
CONTAINER_RUN ?= $(CONTAINER_CMD) --security-opt label=disable -v $(CURDIR):$(CONTAINER_GOSRC) -w $(CONTAINER_GOSRC) $(SKOPEO_CIDEV_CONTAINER_FQIN)

GIT_COMMIT := $(shell GIT_CEILING_DIRECTORIES=$$(cd ..; pwd) git rev-parse HEAD 2> /dev/null || true)
GIT_COMMIT := $(shell GIT_CEILING_DIRECTORIES=$$(cd ..; pwd) git rev-parse HEAD || true)

EXTRA_LDFLAGS ?=
SKOPEO_LDFLAGS := -ldflags '-X main.gitCommit=${GIT_COMMIT} $(EXTRA_LDFLAGS)'
Expand Down Expand Up @@ -127,7 +130,8 @@ help:

# Do the build and the output (skopeo) should appear in current dir
binary: cmd/skopeo
$(CONTAINER_RUN) make bin/skopeo $(if $(DEBUG),DEBUG=$(DEBUG)) BUILDTAGS='$(BUILDTAGS)'
$(CONTAINER_RUN) bash -c \
"git config --global --add safe.directory $(CONTAINER_GOSRC) && make bin/skopeo $(if $(DEBUG),DEBUG=$(DEBUG)) BUILDTAGS='$(BUILDTAGS)'"
Copy link
Contributor

Choose a reason for hiding this comment

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

I consider a single project overwriting users’ configuration like this completely unexpected to the point of being hostile.

Copy link
Author

Choose a reason for hiding this comment

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

It looks a lot worse than it is. This only happens inside a container that is built from an image the user doesn't control, so it can't overwrite their configuration. It's really only modifying root's git config in the container's ephemeral root volume. Without the change, the git command to grab the current commit hash fails and therefore the go build command also fails.

Incidentally, this seems to be a problem with an ownership mismatch due to user ID mapping issues when the container is started. Either the image should be fixed or user ID mapping should be implemented. This is only a stopgap measure so the tests actually run.


# Build w/o using containers
.PHONY: bin/skopeo
Expand All @@ -145,7 +149,8 @@ endif
docs: $(MANPAGES)

docs-in-container:
${CONTAINER_RUN} $(MAKE) docs $(if $(DEBUG),DEBUG=$(DEBUG))
${CONTAINER_RUN} bash -c \
"git config --global --add safe.directory $(CONTAINER_GOSRC) && $(MAKE) docs $(if $(DEBUG),DEBUG=$(DEBUG))"

.PHONY: completions
completions: bin/skopeo
Expand Down Expand Up @@ -188,6 +193,9 @@ shell:
$(CONTAINER_RUN) bash

tools:
if [ ! -x "$(GOBIN)/go-md2man" ]; then \
go install github.com/cpuguy83/go-md2man/v2@v$(GO_MD2MAN_VERSION) ; \
fi
if [ ! -x "$(GOBIN)/golangci-lint" ]; then \
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(GOBIN) v$(GOLANGCI_LINT_VERSION) ; \
fi
Expand All @@ -199,25 +207,36 @@ test-integration:
# --cap-add=cap_mknod is important to allow skopeo to use containers-storage: directly as it exists in the callers’ environment, without
# creating a nested user namespace (which requires /etc/subuid and /etc/subgid to be set up)
$(CONTAINER_CMD) --security-opt label=disable --cap-add=cap_mknod -v $(CURDIR):$(CONTAINER_GOSRC) -w $(CONTAINER_GOSRC) $(SKOPEO_CIDEV_CONTAINER_FQIN) \
$(MAKE) test-integration-local
bash -c \
"git config --global --add safe.directory $(CONTAINER_GOSRC) && $(MAKE) test-integration-local"


# Intended for CI, assumed to be running in quay.io/libpod/skopeo_cidev container.
test-integration-local: bin/skopeo
hack/warn-destructive-tests.sh
hack/test-integration.sh

# complicated set of options needed to run podman-in-podman
test-system:
DTEMP=$(shell mktemp -d --tmpdir=/var/tmp podman-tmp.XXXXXX); \
$(CONTAINER_CMD) --privileged \
-v $(CURDIR):$(CONTAINER_GOSRC) -w $(CONTAINER_GOSRC) \
-v $$DTEMP:/var/lib/containers:Z -v /run/systemd/journal/socket:/run/systemd/journal/socket \
"$(SKOPEO_CIDEV_CONTAINER_FQIN)" \
$(MAKE) test-system-local; \
rc=$$?; \
$(CONTAINER_RUNTIME) unshare rm -rf $$DTEMP; # This probably doesn't work with Docker, oh well, better than nothing... \
exit $$rc
# complicated set of options needed to run podman-in-podman
if [ "$(CONTAINER_RUNTIME)" = "podman" ]; then \
DTEMP=$(shell mktemp -d --tmpdir=/var/tmp podman-tmp.XXXXXX); \
$(CONTAINER_CMD) --privileged \
-v $(CURDIR):$(CONTAINER_GOSRC) -w $(CONTAINER_GOSRC) \
-v $$DTEMP:/var/lib/containers:Z -v /run/systemd/journal/socket:/run/systemd/journal/socket \
"$(SKOPEO_CIDEV_CONTAINER_FQIN)" \
bash -c \
"git config --global --add safe.directory $(CONTAINER_GOSRC) && $(MAKE) test-system-local"; \
rc=$$?; \
$(CONTAINER_RUNTIME) unshare rm -rf $$DTEMP; \
exit $$rc; \
else \
$(CONTAINER_CMD) --privileged \
-v $(CURDIR):$(CONTAINER_GOSRC) -w $(CONTAINER_GOSRC) \
-v /run/systemd/journal/socket:/run/systemd/journal/socket \
"$(SKOPEO_CIDEV_CONTAINER_FQIN)" \
bash -c \
"git config --global --add safe.directory $(CONTAINER_GOSRC) && $(MAKE) test-system-local"; \
fi;

# Intended for CI, assumed to already be running in quay.io/libpod/skopeo_cidev container.
test-system-local: bin/skopeo
Expand All @@ -226,16 +245,18 @@ test-system-local: bin/skopeo

test-unit:
# Just call (make test unit-local) here instead of worrying about environment differences
$(CONTAINER_RUN) $(MAKE) test-unit-local
$(CONTAINER_RUN) bash -c \
"git config --global --add safe.directory $(CONTAINER_GOSRC) && $(MAKE) test-unit-local"

validate:
$(CONTAINER_RUN) $(MAKE) validate-local
$(CONTAINER_RUN) bash -c \
"git config --global --add safe.directory $(CONTAINER_GOSRC) && $(MAKE) validate-local"

# This target is only intended for development, e.g. executing it from an IDE. Use (make test) for CI or pre-release testing.
test-all-local: validate-local validate-docs test-unit-local

.PHONY: validate-local
validate-local:
validate-local: tools
hack/validate-git-marks.sh
hack/validate-gofmt.sh
GOBIN=$(GOBIN) hack/validate-lint.sh
Expand Down
12 changes: 12 additions & 0 deletions cmd/skopeo/copy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/containers/image/v5/transports/alltransports"
encconfig "github.com/containers/ocicrypt/config"
enchelpers "github.com/containers/ocicrypt/helpers"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
)

Expand All @@ -39,6 +40,7 @@ type copyOptions struct {
format commonFlag.OptionalString // Force conversion of the image to a specified format
quiet bool // Suppress output information when copying images
all bool // Copy all of the images if the source is a list
dryRun bool // Don't actually copy anything, just output what it would have done
multiArch commonFlag.OptionalString // How to handle multi architecture images
preserveDigests bool // Preserve digests during copy
encryptLayer []int // The list of layers to encrypt
Expand Down Expand Up @@ -82,6 +84,7 @@ See skopeo(1) section "IMAGE NAMES" for the expected format
flags.StringSliceVar(&opts.additionalTags, "additional-tag", []string{}, "additional tags (supports docker-archive)")
flags.BoolVarP(&opts.quiet, "quiet", "q", false, "Suppress output information when copying images")
flags.BoolVarP(&opts.all, "all", "a", false, "Copy all images if SOURCE-IMAGE is a list")
flags.BoolVar(&opts.dryRun, "dry-run", false, "Run without actually copying images")
flags.Var(commonFlag.NewOptionalStringValue(&opts.multiArch), "multi-arch", `How to handle multi-architecture images (system, all, or index-only)`)
flags.BoolVar(&opts.preserveDigests, "preserve-digests", false, "Preserve digests of images and lists")
flags.BoolVar(&opts.removeSignatures, "remove-signatures", false, "Do not copy signatures from SOURCE-IMAGE")
Expand Down Expand Up @@ -126,6 +129,10 @@ func (opts *copyOptions) run(args []string, stdout io.Writer) (retErr error) {
opts.deprecatedTLSVerify.warnIfUsed([]string{"--src-tls-verify", "--dest-tls-verify"})
imageNames := args

if opts.dryRun {
logrus.Warn("Running in dry-run mode")
}

if err := reexecIfNecessaryForImages(imageNames...); err != nil {
return err
}
Expand Down Expand Up @@ -282,6 +289,11 @@ func (opts *copyOptions) run(args []string, stdout io.Writer) (retErr error) {

opts.destImage.warnAboutIneffectiveOptions(destRef.Transport())

if opts.dryRun {
logrus.Info(fmt.Sprintf("Would have copied from=%s to=%s", imageNames[0], imageNames[1]))
return nil
}

return retry.IfNecessary(ctx, func() error {
manifestBytes, err := copy.Image(ctx, policyContext, destRef, srcRef, &copy.Options{
RemoveSignatures: opts.removeSignatures,
Expand Down
4 changes: 4 additions & 0 deletions docs/skopeo-copy.1.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ If _source-image_ refers to a list of images, instead of copying just the image
architecture (subject to the use of the global --override-os, --override-arch and --override-variant options), attempt to copy all of
the images in the list, and the list itself.

**--dry-run**

Run the sync without actually copying data to the destination.

**--authfile** _path_

Path of the authentication file. Default is ${XDG_RUNTIME\_DIR}/containers/auth.json, which is set using `skopeo login`.
Expand Down
16 changes: 16 additions & 0 deletions integration/copy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"crypto/x509"
"encoding/json"
"fmt"
"io"
"io/fs"
"log"
"net/http"
Expand Down Expand Up @@ -127,6 +128,13 @@ func (s *copySuite) TestCopyAllWithManifestListRoundTrip() {
assert.Equal(t, "", out)
}

func (s *copySuite) TestCopyDryRun() {
t := s.T()
dir := t.TempDir()
assertSkopeoSucceeds(t, "", "copy", "--dry-run", knownListImage, "dir:"+dir)
assertDirIsEmpty(t, dir)
}

func (s *copySuite) TestCopyAllWithManifestListConverge() {
t := s.T()
oci1 := t.TempDir()
Expand Down Expand Up @@ -662,6 +670,14 @@ func assertSchema1DirImagesAreEqualExceptNames(t *testing.T, dir1, ref1, dir2, r
assert.Equal(t, "", out)
}

func assertDirIsEmpty(t *testing.T, dir string) {
d, err := os.Open(dir)
require.NoError(t, err)
defer d.Close()
_, err = d.Readdirnames(1)
assert.Equal(t, err, io.EOF)
}

// Streaming (skopeo copy)
func (s *copySuite) TestCopyStreaming() {
t := s.T()
Expand Down
9 changes: 9 additions & 0 deletions systemtest/020-copy.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,15 @@ function setup() {
expect_output "amd64"
}

@test "copy: --dry-run" {
local remote_image=docker://quay.io/libpod/busybox:latest
local dir=$TESTDIR/dir

run_skopeo copy --dry-run $remote_image oci:$dir:latest
expect_output --substring "Running in dry-run mode"
expect_output --substring "Would have copied from=${remote_image} to=oci:${dir}:latest"
}

teardown() {
podman rm -f reg

Expand Down