From c11c69c351ff11ea18938253673c3cf57f2e0367 Mon Sep 17 00:00:00 2001 From: Nicol Date: Tue, 17 Jan 2023 07:52:50 +0200 Subject: [PATCH] refactor: Cleanup/simplify testcases in /pkg/cli (#1103) Signed-off-by: Nicol Draghici --- pkg/cli/client_elevated_test.go | 26 +--- pkg/cli/client_test.go | 71 ++------- pkg/cli/image_cmd_test.go | 259 ++++++-------------------------- pkg/cli/root_test.go | 24 +-- pkg/cli/stress_test.go | 24 +-- 5 files changed, 70 insertions(+), 334 deletions(-) diff --git a/pkg/cli/client_elevated_test.go b/pkg/cli/client_elevated_test.go index 8833d940..dfa208ad 100644 --- a/pkg/cli/client_elevated_test.go +++ b/pkg/cli/client_elevated_test.go @@ -5,7 +5,6 @@ package cli //nolint:testpackage import ( "bytes" - "context" "crypto/tls" "crypto/x509" "fmt" @@ -13,7 +12,6 @@ import ( "os/exec" "path/filepath" "testing" - "time" . "github.com/smartystreets/goconvey/convey" "gopkg.in/resty.v1" @@ -21,6 +19,7 @@ import ( "zotregistry.io/zot/pkg/api" "zotregistry.io/zot/pkg/api/config" "zotregistry.io/zot/pkg/api/constants" + "zotregistry.io/zot/pkg/test" ) func TestElevatedPrivilegesTLSNewControllerPrivilegedCert(t *testing.T) { @@ -84,26 +83,9 @@ func TestElevatedPrivilegesTLSNewControllerPrivilegedCert(t *testing.T) { ctlr := api.NewController(conf) ctlr.Config.Storage.RootDirectory = t.TempDir() - go func() { - // this blocks - if err := ctlr.Run(context.Background()); err != nil { - return - } - }() - - // wait till ready - for { - _, err := resty.R().Get(BaseURL2) - if err == nil { - break - } - time.Sleep(100 * time.Millisecond) - } - - defer func() { - ctx := context.Background() - _ = ctlr.Server.Shutdown(ctx) - }() + cm := test.NewControllerManager(ctlr) + cm.StartAndWait(conf.HTTP.Port) + defer cm.StopServer() Convey("Certs in privileged path", func() { configPath := makeConfigFile( diff --git a/pkg/cli/client_test.go b/pkg/cli/client_test.go index 1f2825c1..54c94ec4 100644 --- a/pkg/cli/client_test.go +++ b/pkg/cli/client_test.go @@ -5,14 +5,12 @@ package cli //nolint:testpackage import ( "bytes" - "context" "crypto/tls" "crypto/x509" "fmt" "os" "path/filepath" "testing" - "time" . "github.com/smartystreets/goconvey/convey" "gopkg.in/resty.v1" @@ -77,26 +75,9 @@ func TestTLSWithAuth(t *testing.T) { ctlr := api.NewController(conf) ctlr.Config.Storage.RootDirectory = t.TempDir() - go func() { - // this blocks - if err := ctlr.Run(context.Background()); err != nil { - return - } - }() - - // wait till ready - for { - _, err := resty.R().Get(BaseSecureURL1) - if err == nil { - break - } - time.Sleep(100 * time.Millisecond) - } - - defer func() { - ctx := context.Background() - _ = ctlr.Server.Shutdown(ctx) - }() + cm := test.NewControllerManager(ctlr) + cm.StartAndWait(conf.HTTP.Port) + defer cm.StopServer() Convey("Test with htpassw auth", func() { configPath := makeConfigFile(`{"configs":[{"_name":"imagetest","showspinner":false}]}`) @@ -174,26 +155,9 @@ func TestTLSWithoutAuth(t *testing.T) { ctlr := api.NewController(conf) ctlr.Config.Storage.RootDirectory = t.TempDir() - go func() { - // this blocks - if err := ctlr.Run(context.Background()); err != nil { - return - } - }() - - // wait till ready - for { - _, err := resty.R().Get(BaseURL1) - if err == nil { - break - } - time.Sleep(100 * time.Millisecond) - } - - defer func() { - ctx := context.Background() - _ = ctlr.Server.Shutdown(ctx) - }() + cm := test.NewControllerManager(ctlr) + cm.StartAndWait(conf.HTTP.Port) + defer cm.StopServer() Convey("Certs in user's home", func() { configPath := makeConfigFile( @@ -239,26 +203,9 @@ func TestTLSBadCerts(t *testing.T) { ctlr := api.NewController(conf) ctlr.Config.Storage.RootDirectory = t.TempDir() - go func() { - // this blocks - if err := ctlr.Run(context.Background()); err != nil { - return - } - }() - - // wait till ready - for { - _, err := resty.R().Get(BaseURL3) - if err == nil { - break - } - time.Sleep(100 * time.Millisecond) - } - - defer func() { - ctx := context.Background() - _ = ctlr.Server.Shutdown(ctx) - }() + cm := test.NewControllerManager(ctlr) + cm.StartAndWait(conf.HTTP.Port) + defer cm.StopServer() Convey("Test with system certs", func() { configPath := makeConfigFile( diff --git a/pkg/cli/image_cmd_test.go b/pkg/cli/image_cmd_test.go index 9793cb52..66538718 100644 --- a/pkg/cli/image_cmd_test.go +++ b/pkg/cli/image_cmd_test.go @@ -313,73 +313,26 @@ func TestSignature(t *testing.T) { } ctlr := api.NewController(conf) ctlr.Config.Storage.RootDirectory = currentDir - go func(controller *api.Controller) { - // this blocks - if err := controller.Run(context.Background()); err != nil { - return - } - }(ctlr) - // wait till ready - for { - _, err := resty.R().Get(url) - if err == nil { - break - } + cm := test.NewControllerManager(ctlr) + cm.StartAndWait(conf.HTTP.Port) + defer cm.StopServer() - time.Sleep(100 * time.Millisecond) - } - defer func(controller *api.Controller) { - ctx := context.Background() - _ = controller.Server.Shutdown(ctx) - }(ctlr) - - // create a blob/layer - resp, _ := resty.R().Post(url + "/v2/repo7/blobs/uploads/") - loc := test.Location(url, resp) - - content := []byte("this is a blob5") - digest := godigest.FromBytes(content) - _, _ = resty.R().SetQueryParam("digest", digest.String()). - SetHeader("Content-Type", "application/octet-stream").SetBody(content).Put(loc) - - // upload image config blob - resp, _ = resty.R().Post(url + "/v2/repo7/blobs/uploads/") - loc = test.Location(url, resp) - cblob, cdigest := test.GetImageConfig() - - _, _ = resty.R(). - SetContentLength(true). - SetHeader("Content-Length", fmt.Sprintf("%d", len(cblob))). - SetHeader("Content-Type", "application/octet-stream"). - SetQueryParam("digest", cdigest.String()). - SetBody(cblob). - Put(loc) - - // create a manifest - manifest := ispec.Manifest{ - Config: ispec.Descriptor{ - MediaType: "application/vnd.oci.image.config.v1+json", - Digest: cdigest, - Size: int64(len(cblob)), - }, - Layers: []ispec.Descriptor{ - { - MediaType: "application/vnd.oci.image.layer.v1.tar", - Digest: digest, - Size: int64(len(content)), - }, - }, - } - manifest.SchemaVersion = 2 - - content, err = json.Marshal(manifest) + cfg, layers, manifest, err := test.GetImageComponents(1) So(err, ShouldBeNil) - _, _ = resty.R().SetHeader("Content-Type", "application/vnd.oci.image.manifest.v1+json"). - SetBody(content).Put(url + "/v2/repo7/manifests/test:1.0") + repoName := "repo7" + err = test.UploadImage( + test.Image{ + Config: cfg, + Layers: layers, + Manifest: manifest, + Tag: "test:1.0", + }, url, repoName) + So(err, ShouldBeNil) - // content = []byte("this is a blob5") - digest = godigest.FromBytes(content) + content, err := json.Marshal(manifest) + So(err, ShouldBeNil) + digest := godigest.FromBytes(content) // generate a keypair if _, err := os.Stat(path.Join(currentDir, "cosign.key")); err != nil { @@ -415,7 +368,7 @@ func TestSignature(t *testing.T) { str := space.ReplaceAllString(buff.String(), " ") actual := strings.TrimSpace(str) So(actual, ShouldContainSubstring, "IMAGE NAME TAG DIGEST SIGNED SIZE") - So(actual, ShouldContainSubstring, "repo7 test:1.0 883fc0c5 true 15B") + So(actual, ShouldContainSubstring, "repo7 test:1.0 6742241d true 1B") t.Log("Test getting all images using rest calls to get catalog and individual manifests") cmd = MockNewImageCommand(new(searchService)) @@ -428,7 +381,7 @@ func TestSignature(t *testing.T) { str = space.ReplaceAllString(buff.String(), " ") actual = strings.TrimSpace(str) So(actual, ShouldContainSubstring, "IMAGE NAME TAG DIGEST SIGNED SIZE") - So(actual, ShouldContainSubstring, "repo7 test:1.0 883fc0c5 true 492B") + So(actual, ShouldContainSubstring, "repo7 test:1.0 6742241d true 447B") err = os.Chdir(currentWorkingDir) So(err, ShouldBeNil) @@ -452,70 +405,27 @@ func TestSignature(t *testing.T) { } ctlr := api.NewController(conf) ctlr.Config.Storage.RootDirectory = currentDir - go func(controller *api.Controller) { - // this blocks - if err := controller.Run(context.Background()); err != nil { - return - } - }(ctlr) - // wait till ready - for { - _, err := resty.R().Get(url) - if err == nil { - break - } + cm := test.NewControllerManager(ctlr) + cm.StartAndWait(conf.HTTP.Port) + defer cm.StopServer() - time.Sleep(100 * time.Millisecond) - } - defer func(controller *api.Controller) { - ctx := context.Background() - _ = controller.Server.Shutdown(ctx) - }(ctlr) - - // create a blob/layer - resp, _ := resty.R().Post(url + "/v2/repo7/blobs/uploads/") - loc := test.Location(url, resp) - - content := []byte("this is a blob5") - digest := godigest.FromBytes(content) - _, _ = resty.R().SetQueryParam("digest", digest.String()). - SetHeader("Content-Type", "application/octet-stream").SetBody(content).Put(loc) - - // upload image config blob - resp, _ = resty.R().Post(url + "/v2/repo7/blobs/uploads/") - loc = test.Location(url, resp) - cblob, cdigest := test.GetImageConfig() - - _, _ = resty.R(). - SetContentLength(true). - SetHeader("Content-Length", fmt.Sprintf("%d", len(cblob))). - SetHeader("Content-Type", "application/octet-stream"). - SetQueryParam("digest", cdigest.String()). - SetBody(cblob). - Put(loc) - - // create a manifest - manifest := ispec.Manifest{ - Config: ispec.Descriptor{ - MediaType: "application/vnd.oci.image.config.v1+json", - Digest: cdigest, - Size: int64(len(cblob)), - }, - Layers: []ispec.Descriptor{ - { - MediaType: "application/vnd.oci.image.layer.v1.tar", - Digest: digest, - Size: int64(len(content)), - }, - }, - } - manifest.SchemaVersion = 2 - - content, err = json.Marshal(manifest) + cfg, layers, manifest, err := test.GetImageComponents(1) So(err, ShouldBeNil) - _, _ = resty.R().SetHeader("Content-Type", "application/vnd.oci.image.manifest.v1+json"). - SetBody(content).Put(url + "/v2/repo7/manifests/0.0.1") + repoName := "repo7" + err = test.UploadImage( + test.Image{ + Config: cfg, + Layers: layers, + Manifest: manifest, + Tag: "0.0.1", + }, url, repoName) + So(err, ShouldBeNil) + + content, err := json.Marshal(manifest) + So(err, ShouldBeNil) + digest := godigest.FromBytes(content) + So(digest, ShouldNotBeNil) err = SignImageUsingNotary("repo7:0.0.1", port) So(err, ShouldBeNil) @@ -535,7 +445,7 @@ func TestSignature(t *testing.T) { str := space.ReplaceAllString(buff.String(), " ") actual := strings.TrimSpace(str) So(actual, ShouldContainSubstring, "IMAGE NAME TAG DIGEST SIGNED SIZE") - So(actual, ShouldContainSubstring, "repo7 0.0.1 883fc0c5 true 15B") + So(actual, ShouldContainSubstring, "repo7 0.0.1 6742241d true 1B") t.Log("Test getting all images using rest calls to get catalog and individual manifests") cmd = MockNewImageCommand(new(searchService)) @@ -548,7 +458,7 @@ func TestSignature(t *testing.T) { str = space.ReplaceAllString(buff.String(), " ") actual = strings.TrimSpace(str) So(actual, ShouldContainSubstring, "IMAGE NAME TAG DIGEST SIGNED SIZE") - So(actual, ShouldContainSubstring, "repo7 0.0.1 883fc0c5 true 492B") + So(actual, ShouldContainSubstring, "repo7 0.0.1 6742241d true 447B") err = os.Chdir(currentWorkingDir) So(err, ShouldBeNil) @@ -568,27 +478,10 @@ func TestDerivedImageList(t *testing.T) { ctlr := api.NewController(conf) ctlr.Config.Storage.RootDirectory = t.TempDir() - go func(controller *api.Controller) { - // this blocks - if err := controller.Run(context.Background()); err != nil { - return - } - }(ctlr) + cm := test.NewControllerManager(ctlr) - // wait till ready - for { - _, err := resty.R().Get(url) - if err == nil { - break - } - - time.Sleep(100 * time.Millisecond) - } - - defer func(controller *api.Controller) { - ctx := context.Background() - _ = controller.Server.Shutdown(ctx) - }(ctlr) + cm.StartAndWait(conf.HTTP.Port) + defer cm.StopServer() err := uploadManifest(url) if err != nil { @@ -666,28 +559,10 @@ func TestBaseImageList(t *testing.T) { } ctlr := api.NewController(conf) ctlr.Config.Storage.RootDirectory = t.TempDir() + cm := test.NewControllerManager(ctlr) - go func(controller *api.Controller) { - // this blocks - if err := controller.Run(context.Background()); err != nil { - return - } - }(ctlr) - - // wait till ready - for { - _, err := resty.R().Get(url) - if err == nil { - break - } - - time.Sleep(100 * time.Millisecond) - } - - defer func(controller *api.Controller) { - ctx := context.Background() - _ = controller.Server.Shutdown(ctx) - }(ctlr) + cm.StartAndWait(conf.HTTP.Port) + defer cm.StopServer() err := uploadManifest(url) if err != nil { @@ -1014,25 +889,9 @@ func TestServerResponseGQL(t *testing.T) { } ctlr := api.NewController(conf) ctlr.Config.Storage.RootDirectory = t.TempDir() - go func(controller *api.Controller) { - // this blocks - if err := controller.Run(context.Background()); err != nil { - return - } - }(ctlr) - // wait till ready - for { - _, err := resty.R().Get(url) - if err == nil { - break - } - - time.Sleep(100 * time.Millisecond) - } - defer func(controller *api.Controller) { - ctx := context.Background() - _ = controller.Server.Shutdown(ctx) - }(ctlr) + cm := test.NewControllerManager(ctlr) + cm.StartAndWait(conf.HTTP.Port) + defer cm.StopServer() err := uploadManifest(url) t.Logf("%s", ctlr.Config.Storage.RootDirectory) @@ -1286,28 +1145,10 @@ func TestServerResponse(t *testing.T) { } ctlr := api.NewController(conf) ctlr.Config.Storage.RootDirectory = t.TempDir() + cm := test.NewControllerManager(ctlr) - go func(controller *api.Controller) { - // this blocks - if err := controller.Run(context.Background()); err != nil { - return - } - }(ctlr) - - // wait till ready - for { - _, err := resty.R().Get(url) - if err == nil { - break - } - - time.Sleep(100 * time.Millisecond) - } - - defer func(controller *api.Controller) { - ctx := context.Background() - _ = controller.Server.Shutdown(ctx) - }(ctlr) + cm.StartAndWait(conf.HTTP.Port) + defer cm.StopServer() err := uploadManifest(url) if err != nil { diff --git a/pkg/cli/root_test.go b/pkg/cli/root_test.go index 6e16b4f9..172acb8d 100644 --- a/pkg/cli/root_test.go +++ b/pkg/cli/root_test.go @@ -1,7 +1,6 @@ package cli_test import ( - "context" "encoding/json" "fmt" "os" @@ -10,7 +9,6 @@ import ( "time" . "github.com/smartystreets/goconvey/convey" - "gopkg.in/resty.v1" "zotregistry.io/zot/pkg/api" "zotregistry.io/zot/pkg/api/config" @@ -1240,21 +1238,8 @@ func TestScrub(t *testing.T) { dir := t.TempDir() controller.Config.Storage.RootDirectory = dir - go func(controller *api.Controller) { - // this blocks - if err := controller.Run(context.Background()); err != nil { - return - } - }(controller) - // wait till ready - for { - _, err := resty.R().Get(fmt.Sprintf("http://127.0.0.1:%s", port)) - if err == nil { - break - } - - time.Sleep(100 * time.Millisecond) - } + ctrlManager := NewControllerManager(controller) + ctrlManager.StartAndWait(port) tmpfile, err := os.CreateTemp("", "zot-test*.json") So(err, ShouldBeNil) @@ -1279,10 +1264,7 @@ func TestScrub(t *testing.T) { os.Args = []string{"cli_test", "scrub", tmpfile.Name()} So(func() { _ = cli.NewServerRootCmd().Execute() }, ShouldPanic) - defer func(controller *api.Controller) { - ctx := context.Background() - _ = controller.Server.Shutdown(ctx) - }(controller) + defer ctrlManager.StopServer() }) Convey("no image store provided", func(c C) { diff --git a/pkg/cli/stress_test.go b/pkg/cli/stress_test.go index 41f65762..77b630db 100644 --- a/pkg/cli/stress_test.go +++ b/pkg/cli/stress_test.go @@ -4,7 +4,6 @@ package cli_test import ( - "context" "fmt" "os" "os/exec" @@ -40,7 +39,6 @@ func TestSressTooManyOpenFiles(t *testing.T) { So(err, ShouldBeNil) port := test.GetFreePort() - baseURL := test.GetBaseURL(port) conf := config.New() conf.HTTP.Port = port conf.Storage.Dedupe = false @@ -75,8 +73,9 @@ func TestSressTooManyOpenFiles(t *testing.T) { t.Log("Storage root dir is: ", dir) ctlr.Config.Storage.RootDirectory = dir - go startServer(ctlr) - test.WaitTillServerReady(baseURL) + ctrlManager := test.NewControllerManager(ctlr) + ctrlManager.StartAndWait(port) + content := fmt.Sprintf(`{ "storage": { "rootDirectory": "%s", @@ -132,7 +131,7 @@ func TestSressTooManyOpenFiles(t *testing.T) { So(err, ShouldBeNil) So(string(data), ShouldContainSubstring, "too many open files") - stopServer(ctlr) + ctrlManager.StopServer() time.Sleep(2 * time.Second) scrubFile, err := os.CreateTemp("", "zot-scrub*.txt") @@ -213,18 +212,3 @@ func setMaxOpenFilesLimit(limit uint64) (uint64, error) { return initialLimit, nil } - -func startServer(c *api.Controller) { - // this blocks - ctx := context.Background() - if err := c.Run(ctx); err != nil { - fmt.Printf("\nerror on starting zot server: %s\n", err) - } -} - -func stopServer(c *api.Controller) { - ctx := context.Background() - if err := c.Server.Shutdown(ctx); err != nil { - fmt.Printf("\nerror on stopping zot server: %s\n", err) - } -}