diff --git a/Makefile b/Makefile index 6c116987..454e44af 100644 --- a/Makefile +++ b/Makefile @@ -49,8 +49,8 @@ test: check-skopeo $(NOTATION) go test -tags extended,containers_image_openpgp -v -trimpath -race -timeout 15m -cover -coverpkg ./... -coverprofile=coverage-extended.txt -covermode=atomic ./... go test -tags minimal,containers_image_openpgp -v -trimpath -race -cover -coverpkg ./... -coverprofile=coverage-minimal.txt -covermode=atomic ./... # development-mode unit tests possibly using failure injection - go test -tags dev,extended,containers_image_openpgp -v -trimpath -race -timeout 15m -cover -coverpkg ./... -coverprofile=coverage-dev-extended.txt -covermode=atomic ./pkg/test/... ./pkg/storage/... - go test -tags dev,minimal,containers_image_openpgp -v -trimpath -race -cover -coverpkg ./... -coverprofile=coverage-dev-minimal.txt -covermode=atomic ./pkg/test/... ./pkg/storage/... + go test -tags dev,extended,containers_image_openpgp -v -trimpath -race -timeout 15m -cover -coverpkg ./... -coverprofile=coverage-dev-extended.txt -covermode=atomic ./pkg/test/... ./pkg/storage/... ./pkg/extensions/sync/... -run ^TestInject + go test -tags dev,minimal,containers_image_openpgp -v -trimpath -race -cover -coverpkg ./... -coverprofile=coverage-dev-minimal.txt -covermode=atomic ./pkg/test/... ./pkg/storage/... ./pkg/extensions/sync/... -run ^TestInject .PHONY: run-bench run-bench: binary bench diff --git a/go.sum b/go.sum index 204be4c3..fe103787 100644 --- a/go.sum +++ b/go.sum @@ -1862,6 +1862,8 @@ github.com/onsi/gomega v1.15.0/go.mod h1:cIuvLEne0aoVhAgh/O6ac0Op8WWw9H6eYCriF+t github.com/onsi/gomega v1.16.0 h1:6gjqkI8iiRHMvdccRJM8rVKjCWk6ZIm6FTm3ddIe4/c= github.com/onsi/gomega v1.16.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY= github.com/op/go-logging v0.0.0-20160315200505-970db520ece7/go.mod h1:HzydrMdWErDVzsI23lYNej1Htcns9BCg93Dk0bBINWk= +github.com/open-policy-agent/opa v0.32.0/go.mod h1:5sJdtc+1/U8zy/j30njpQl6u9rM4MzTOhG9EW1uOmsY= +github.com/open-policy-agent/opa v0.34.1/go.mod h1:buysXn+6zB/b+6JgLkP4WgKZ9+UgUtFAgtemYGrL9Ik= github.com/open-policy-agent/opa v0.37.0 h1:OUXB+RAcxQpmXeNW2BN1wYzQQvVCPF1T9zv+QXGr9Wg= github.com/open-policy-agent/opa v0.37.0/go.mod h1:xX3NUCZuXK8f0CNhFQvhm4495mZLptf94pIkWRLaFqo= github.com/opencontainers/distribution-spec/specs-go v0.0.0-20211026153258-b3f631f25f1a h1:cZbrj1PTlynrvx4pIVbrI3rvncbxtMG5U4EYM6ScQ/A= diff --git a/pkg/extensions/sync/on_demand.go b/pkg/extensions/sync/on_demand.go index a4e424ac..d37b5c23 100644 --- a/pkg/extensions/sync/on_demand.go +++ b/pkg/extensions/sync/on_demand.go @@ -194,16 +194,7 @@ func syncOneImage(imageChannel chan error, cfg Config, storeController storage.S return } - localCachePath, err := getLocalCachePath(imageStore, repo) - if err != nil { - log.Error().Err(err).Str("dir", localCachePath).Msg("couldn't create temporary dir") - - imageChannel <- err - - return - } - - localImageRef, err := getLocalImageRef(localCachePath, repo, tag) + localImageRef, localCachePath, err := getLocalImageRef(imageStore, repo, tag) if err != nil { log.Error().Err(err).Msgf("couldn't obtain a valid image reference for reference %s/%s:%s", localCachePath, repo, tag) diff --git a/pkg/extensions/sync/sync.go b/pkg/extensions/sync/sync.go index ab093f7e..30c6bc51 100644 --- a/pkg/extensions/sync/sync.go +++ b/pkg/extensions/sync/sync.go @@ -22,6 +22,7 @@ import ( "zotregistry.io/zot/errors" "zotregistry.io/zot/pkg/log" "zotregistry.io/zot/pkg/storage" + "zotregistry.io/zot/pkg/test" ) const ( @@ -102,7 +103,8 @@ func getUpstreamCatalog(client *resty.Client, upstreamURL string, log log.Logger // It returns a string slice of tags and any error encountered. func getImageTags(ctx context.Context, sysCtx *types.SystemContext, repoRef reference.Named) ([]string, error) { dockerRef, err := docker.NewReference(reference.TagNameOnly(repoRef)) - if err != nil { + // hard to reach test case, injected error, see pkg/test/dev.go + if err = test.Error(err); err != nil { return nil, err // Should never happen for a reference with tag and no digest } @@ -364,16 +366,7 @@ func syncRegistry(regCfg RegistryConfig, upstreamURL string, storeController sto continue } - localCachePath, err := getLocalCachePath(imageStore, repo) - if err != nil { - log.Error().Err(err).Str("dir", localCachePath).Msg("couldn't create temporary dir") - - return err - } - - defer os.RemoveAll(localCachePath) - - localImageRef, err := getLocalImageRef(localCachePath, repo, tag) + localImageRef, localCachePath, err := getLocalImageRef(imageStore, repo, tag) if err != nil { log.Error().Err(err).Msgf("couldn't obtain a valid image reference for reference %s/%s:%s", localCachePath, repo, tag) @@ -381,6 +374,8 @@ func syncRegistry(regCfg RegistryConfig, upstreamURL string, storeController sto return err } + defer os.RemoveAll(localCachePath) + log.Info().Msgf("copying image %s to %s", upstreamImageRef.DockerReference(), localCachePath) if err = retry.RetryIfNecessary(context.Background(), func() error { @@ -431,7 +426,7 @@ func getLocalContexts(log log.Logger) (*types.SystemContext, *signature.PolicyCo policy = &signature.Policy{Default: []signature.PolicyRequirement{signature.NewPRInsecureAcceptAnything()}} policyContext, err := signature.NewPolicyContext(policy) - if err != nil { + if err := test.Error(err); err != nil { log.Error().Err(err).Msg("couldn't create policy context") return &types.SystemContext{}, &signature.PolicyContext{}, err diff --git a/pkg/extensions/sync/sync_internal_test.go b/pkg/extensions/sync/sync_internal_test.go index cc94932b..1c7c5421 100644 --- a/pkg/extensions/sync/sync_internal_test.go +++ b/pkg/extensions/sync/sync_internal_test.go @@ -34,6 +34,52 @@ const ( host = "127.0.0.1:45117" ) +func TestInjectSyncUtils(t *testing.T) { + Convey("Inject errors in utils functions", t, func() { + repositoryReference := fmt.Sprintf("%s/%s", host, testImage) + ref, err := parseRepositoryReference(repositoryReference) + So(err, ShouldBeNil) + So(ref.Name(), ShouldEqual, repositoryReference) + + taggedRef, err := reference.WithTag(ref, "tag") + So(err, ShouldBeNil) + + injected := test.InjectFailure(0) + if injected { + _, err = getImageTags(context.Background(), &types.SystemContext{}, taggedRef) + So(err, ShouldNotBeNil) + } + + injected = test.InjectFailure(0) + _, _, err = getLocalContexts(log.NewLogger("debug", "")) + if injected { + So(err, ShouldNotBeNil) + } else { + So(err, ShouldBeNil) + } + + storageDir, err := ioutil.TempDir("", "oci-dest-repo-test") + if err != nil { + panic(err) + } + + defer os.RemoveAll(storageDir) + + log := log.Logger{Logger: zerolog.New(os.Stdout)} + metrics := monitoring.NewMetricsServer(false, log) + + imageStore := storage.NewImageStore(storageDir, false, storage.DefaultGCDelay, false, false, log, metrics) + + injected = test.InjectFailure(0) + _, _, err = getLocalImageRef(imageStore, testImage, testImageTag) + if injected { + So(err, ShouldNotBeNil) + } else { + So(err, ShouldBeNil) + } + }) +} + func TestSyncInternal(t *testing.T) { Convey("Verify parseRepositoryReference func", t, func() { repositoryReference := fmt.Sprintf("%s/%s", host, testImage) @@ -82,8 +128,6 @@ func TestSyncInternal(t *testing.T) { dockerRef, err := docker.NewReference(taggedRef) So(err, ShouldBeNil) - // tag := getTagFromRef(dockerRef, log.NewLogger("", "")) - So(getTagFromRef(dockerRef, log.NewLogger("debug", "")), ShouldNotBeNil) var tlsVerify bool @@ -110,6 +154,32 @@ func TestSyncInternal(t *testing.T) { So(err, ShouldNotBeNil) }) + Convey("Verify getLocalImageRef()", t, func() { + storageDir, err := ioutil.TempDir("", "oci-dest-repo-test") + if err != nil { + panic(err) + } + + defer os.RemoveAll(storageDir) + + log := log.Logger{Logger: zerolog.New(os.Stdout)} + metrics := monitoring.NewMetricsServer(false, log) + + imageStore := storage.NewImageStore(storageDir, false, storage.DefaultGCDelay, false, false, log, metrics) + + err = os.Chmod(imageStore.RootDir(), 0o000) + So(err, ShouldBeNil) + + _, _, err = getLocalImageRef(imageStore, testImage, testImageTag) + So(err, ShouldNotBeNil) + + err = os.Chmod(imageStore.RootDir(), 0o755) + So(err, ShouldBeNil) + + _, _, err = getLocalImageRef(imageStore, "zot][]321", "tag_tag][]") + So(err, ShouldNotBeNil) + }) + Convey("Test getUpstreamCatalog() with missing certs", t, func() { var tlsVerify bool updateDuration := time.Microsecond @@ -191,10 +261,6 @@ func TestSyncInternal(t *testing.T) { httpClient, err = getHTTPClient(&syncRegistryConfig, baseSecureURL, Credentials{}, log.NewLogger("debug", "")) So(err, ShouldNotBeNil) So(httpClient, ShouldBeNil) - - httpClient, err = getHTTPClient(&syncRegistryConfig, "invalidUrl]", Credentials{}, log.NewLogger("debug", "")) - So(err, ShouldNotBeNil) - So(httpClient, ShouldBeNil) }) Convey("Test imagesToCopyFromUpstream()", t, func() { diff --git a/pkg/extensions/sync/sync_test.go b/pkg/extensions/sync/sync_test.go index 8e260f54..bb2e742b 100644 --- a/pkg/extensions/sync/sync_test.go +++ b/pkg/extensions/sync/sync_test.go @@ -626,14 +626,13 @@ func TestOnDemandPermsDenied(t *testing.T) { } }() - for { - err = os.Chmod(path.Join(destDir, testImage, sync.SyncBlobUploadDir), 0o000) - if err != nil { - continue - } + syncSubDir := path.Join(destDir, testImage, sync.SyncBlobUploadDir) - break - } + err = os.MkdirAll(syncSubDir, 0o755) + So(err, ShouldBeNil) + + err = os.Chmod(syncSubDir, 0o000) + So(err, ShouldBeNil) // wait till ready for { @@ -649,108 +648,13 @@ func TestOnDemandPermsDenied(t *testing.T) { So(err, ShouldBeNil) So(resp.StatusCode(), ShouldEqual, 404) - err = os.Chmod(path.Join(destDir, testImage, sync.SyncBlobUploadDir), 0o755) + err = os.Chmod(syncSubDir, 0o755) if err != nil { panic(err) } }) } -func TestPeriodicallyPermsDenied(t *testing.T) { - Convey("Verify periodically sync feature without perm on sync cache", t, func() { - updateDuration, _ := time.ParseDuration("30m") - - sctlr, srcBaseURL, srcDir, _, _ := startUpstreamServer(false, false) - defer os.RemoveAll(srcDir) - - defer func() { - sctlr.Shutdown() - }() - - regex := ".*" - semver := true - var tlsVerify bool - - syncRegistryConfig := sync.RegistryConfig{ - Content: []sync.Content{ - { - Prefix: testImage, - Tags: &sync.Tags{ - Regex: ®ex, - Semver: &semver, - }, - }, - }, - URLs: []string{srcBaseURL}, - PollInterval: updateDuration, - TLSVerify: &tlsVerify, - CertDir: "", - OnDemand: true, - } - - defaultVal := true - syncConfig := &sync.Config{ - Enable: &defaultVal, - Registries: []sync.RegistryConfig{syncRegistryConfig}, - } - - destPort := test.GetFreePort() - destConfig := config.New() - destBaseURL := test.GetBaseURL(destPort) - - destConfig.HTTP.Port = destPort - - destDir, err := ioutil.TempDir("", "oci-dest-repo-test") - if err != nil { - panic(err) - } - - defer os.RemoveAll(destDir) - - destConfig.Storage.RootDirectory = destDir - - destConfig.Extensions = &extconf.ExtensionConfig{} - destConfig.Extensions.Search = nil - destConfig.Extensions.Sync = syncConfig - - dctlr := api.NewController(destConfig) - - go func() { - // this blocks - if err := dctlr.Run(); err != nil { - return - } - }() - - for { - err = os.Chmod(path.Join(destDir, testImage, sync.SyncBlobUploadDir), 0o000) - if err != nil { - continue - } - - break - } - - // wait till ready - for { - _, err := resty.R().Get(destBaseURL) - if err == nil { - break - } - - time.Sleep(100 * time.Millisecond) - } - - defer func() { - dctlr.Shutdown() - err := os.Chmod(path.Join(destDir, testImage, sync.SyncBlobUploadDir), 0o755) - if err != nil { - panic(err) - } - }() - }) -} - func TestBadTLS(t *testing.T) { Convey("Verify sync TLS feature", t, func() { updateDuration, _ := time.ParseDuration("30m") @@ -2895,7 +2799,10 @@ func TestError(t *testing.T) { // give permission denied on pushSyncedLocalImage() localRepoPath := path.Join(destDir, testImage, "blobs") - err := os.MkdirAll(localRepoPath, 0o000) + err := os.MkdirAll(localRepoPath, 0o755) + So(err, ShouldBeNil) + + err = os.Chmod(localRepoPath, 0o000) So(err, ShouldBeNil) resp, err := client.R().Get(destBaseURL + "/v2/" + testImage + "/manifests/" + testImageTag) diff --git a/pkg/extensions/sync/utils.go b/pkg/extensions/sync/utils.go index 58833fd7..d4ffc803 100644 --- a/pkg/extensions/sync/utils.go +++ b/pkg/extensions/sync/utils.go @@ -28,6 +28,7 @@ import ( "zotregistry.io/zot/pkg/extensions/monitoring" "zotregistry.io/zot/pkg/log" "zotregistry.io/zot/pkg/storage" + "zotregistry.io/zot/pkg/test" ) type ReferenceList struct { @@ -527,22 +528,6 @@ func StripRegistryTransport(url string) string { return strings.Replace(strings.Replace(url, "http://", "", 1), "https://", "", 1) } -// get a .sync subdir used for temporary store one synced image. -func getLocalCachePath(imageStore storage.ImageStore, repo string) (string, error) { - uuid, err := guuid.NewV4() - if err != nil { - return "", err - } - - localCachePath := path.Join(imageStore.RootDir(), repo, SyncBlobUploadDir, uuid.String()) - - if err = os.MkdirAll(path.Join(localCachePath, repo), storage.DefaultDirPerms); err != nil { - return "", err - } - - return localCachePath, nil -} - // get an ImageReference given the registry, repo and tag. func getImageRef(registryDomain, repo, tag string) (types.ImageReference, error) { repoRef, err := parseRepositoryReference(fmt.Sprintf("%s/%s", registryDomain, repo)) @@ -564,16 +549,28 @@ func getImageRef(registryDomain, repo, tag string) (types.ImageReference, error) } // get a local ImageReference used to temporary store one synced image. -func getLocalImageRef(localCachePath, repo, tag string) (types.ImageReference, error) { +func getLocalImageRef(imageStore storage.ImageStore, repo, tag string) (types.ImageReference, string, error) { + uuid, err := guuid.NewV4() + // hard to reach test case, injected error, see pkg/test/dev.go + if err := test.Error(err); err != nil { + return nil, "", err + } + + localCachePath := path.Join(imageStore.RootDir(), repo, SyncBlobUploadDir, uuid.String()) + + if err = os.MkdirAll(path.Join(localCachePath, repo), storage.DefaultDirPerms); err != nil { + return nil, "", err + } + localRepo := path.Join(localCachePath, repo) localTaggedRepo := fmt.Sprintf("%s:%s", localRepo, tag) localImageRef, err := layout.ParseReference(localTaggedRepo) if err != nil { - return nil, err + return nil, "", err } - return localImageRef, nil + return localImageRef, localCachePath, nil } // canSkipImage returns whether or not the image can be skipped from syncing. diff --git a/pkg/storage/storage_fs_test.go b/pkg/storage/storage_fs_test.go index 85941338..1c083fd6 100644 --- a/pkg/storage/storage_fs_test.go +++ b/pkg/storage/storage_fs_test.go @@ -793,7 +793,7 @@ func TestHardLink(t *testing.T) { }) } -func TestWriteFile(t *testing.T) { +func TestInjectWriteFile(t *testing.T) { Convey("writeFile with commit", t, func() { dir, err := ioutil.TempDir("", "oci-repo-test") So(err, ShouldBeNil)