coverage: add failure injection framework

Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>

.github/workflows/ci-cd.yml

@@ -44,7 +44,7 @@ jobs:
           go get github.com/wadey/gocovmerge
 
       - name: Run build and test
-        timeout-minutes: 30
+        timeout-minutes: 60
         run: |
           cd $GITHUB_WORKSPACE && make && make ARCH=arm64 binary-arch-minimal && make ARCH=arm64 binary-arch
         env:

Makefile

@@ -68,6 +68,9 @@ test: check-skopeo $(NOTATION)
 	$(shell sudo chmod a=rwx /etc/containers/certs.d/127.0.0.1:8089/*.key)
 	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/api/... ./pkg/test/...
+	go test -tags dev,minimal,containers_image_openpgp -v -trimpath -race -cover -coverpkg ./... -coverprofile=coverage-dev-minimal.txt -covermode=atomic ./pkg/api/... ./pkg/test/...
 
 .PHONY: run-bench
 run-bench: binary bench
@@ -92,8 +95,8 @@ $(NOTATION):
 
 .PHONY: covhtml
 covhtml:
-	tail -n +2 coverage-minimal.txt > tmp.txt && mv tmp.txt coverage-minimal.txt
-	cat coverage-extended.txt coverage-minimal.txt > coverage.txt
+	go install github.com/wadey/gocovmerge@latest
+	gocovmerge coverage-minimal.txt coverage-extended.txt coverage-dev-minimal.txt coverage-dev-extended.txt > coverage.txt
 	go tool cover -html=coverage.txt -o coverage.html
 
 $(GOLINTER):
@@ -105,6 +108,8 @@ $(GOLINTER):
 check: ./golangcilint.yaml $(GOLINTER)
 	$(GOLINTER) --config ./golangcilint.yaml run --enable-all --out-format=colored-line-number --build-tags minimal,containers_image_openpgp ./...
 	$(GOLINTER) --config ./golangcilint.yaml run --enable-all --out-format=colored-line-number --build-tags extended,containers_image_openpgp ./...
+	$(GOLINTER) --config ./golangcilint.yaml run --enable-all --out-format=colored-line-number --build-tags dev,minimal,containers_image_openpgp ./...
+	$(GOLINTER) --config ./golangcilint.yaml run --enable-all --out-format=colored-line-number --build-tags dev,extended,containers_image_openpgp ./...
 
 swagger/docs.go: 
 	swag -v || go install github.com/swaggo/swag/cmd/swag

errors/errors.go

@@ -47,4 +47,5 @@ var (
 	ErrSyncMissingCatalog      = errors.New("sync: couldn't fetch upstream registry's catalog")
 	ErrMethodNotSupported      = errors.New("storage: method not supported")
 	ErrInvalidMetric           = errors.New("metrics: invalid metric func")
+	ErrInjected                = errors.New("test: injected failure")
 )

pkg/api/controller_test.go

@@ -43,6 +43,7 @@ import (
 	"zotregistry.io/zot/pkg/api"
 	"zotregistry.io/zot/pkg/api/config"
 	"zotregistry.io/zot/pkg/storage"
+	"zotregistry.io/zot/pkg/test"
 	. "zotregistry.io/zot/test"
 )
 
@@ -2083,6 +2084,29 @@ func TestAuthorizationWithBasicAuth(t *testing.T) {
 		So(resp, ShouldNotBeNil)
 		So(resp.StatusCode(), ShouldEqual, http.StatusOK)
 
+		resp, err = resty.R().SetBasicAuth(username, passphrase).
+			Get(baseURL + "/v2/" + AuthorizationNamespace + "/tags/list")
+		So(err, ShouldBeNil)
+		So(resp, ShouldNotBeNil)
+		So(resp.StatusCode(), ShouldEqual, http.StatusOK)
+
+		Convey("Hard to reach cases", func() {
+			injected := test.InjectFailure(0)
+
+			// get tags with read access should get 200
+			conf.AccessControl.Repositories[AuthorizationNamespace].Policies[0].Actions =
+				append(conf.AccessControl.Repositories[AuthorizationNamespace].Policies[0].Actions, "read")
+			resp, err = resty.R().SetBasicAuth(username, passphrase).
+				Get(baseURL + "/v2/" + AuthorizationNamespace + "/tags/list")
+			So(err, ShouldBeNil)
+			So(resp, ShouldNotBeNil)
+			if injected {
+				So(resp.StatusCode(), ShouldEqual, http.StatusNotFound)
+			} else {
+				So(resp.StatusCode(), ShouldEqual, http.StatusOK)
+			}
+		})
+
 		// head blob should get 200 now
 		resp, err = resty.R().SetBasicAuth(username, passphrase).
 			Head(baseURL + "/v2/" + AuthorizationNamespace + "/blobs/" + digest)
@@ -2838,6 +2862,24 @@ func TestParallelRequests(t *testing.T) {
 				assert.Equal(t, err, nil, "Error should be nil")
 				assert.Equal(t, headResponse.StatusCode(), http.StatusNotFound, "response status code should return 404")
 
+				Convey("Hard to reach cases", t, func() {
+					_ = test.InjectFailure(0)
+
+					headResponse, err := client.R().SetBasicAuth(username, passphrase).
+						Head(baseURL + "/v2/" + testcase.destImageName + "/manifests/test:1.0")
+					assert.Equal(t, err, nil, "Error should be nil")
+					assert.Equal(t, headResponse.StatusCode(), http.StatusNotFound, "response status code should return 404")
+				})
+
+				Convey("Hard to reach cases", t, func() {
+					_ = test.InjectFailure(1)
+
+					headResponse, err := client.R().SetBasicAuth(username, passphrase).
+						Head(baseURL + "/v2/" + testcase.destImageName + "/manifests/test:1.0")
+					assert.Equal(t, err, nil, "Error should be nil")
+					assert.Equal(t, headResponse.StatusCode(), http.StatusNotFound, "response status code should return 404")
+				})
+
 				getResponse, err := client.R().SetBasicAuth(username, passphrase).
 					Get(baseURL + "/v2/" + testcase.destImageName + "/manifests/" + manifest)
 				assert.Equal(t, err, nil, "Error should be nil")

pkg/api/routes.go

@@ -32,6 +32,7 @@ import (
 	ext "zotregistry.io/zot/pkg/extensions"
 	"zotregistry.io/zot/pkg/log"
 	"zotregistry.io/zot/pkg/storage"
+	"zotregistry.io/zot/pkg/test"
 
 	// as required by swaggo.
 	_ "zotregistry.io/zot/swagger"
@@ -165,7 +166,7 @@ func (rh *RouteHandler) ListTags(response http.ResponseWriter, request *http.Req
 
 	name, ok := vars["name"]
 
-	if !ok || name == "" {
+	if !test.Ok(ok) || name == "" {
 		response.WriteHeader(http.StatusNotFound)
 
 		return
@@ -287,7 +288,7 @@ func (rh *RouteHandler) CheckManifest(response http.ResponseWriter, request *htt
 	vars := mux.Vars(request)
 	name, ok := vars["name"]
 
-	if !ok || name == "" {
+	if !test.Ok(ok) || name == "" {
 		response.WriteHeader(http.StatusNotFound)
 
 		return
@@ -296,7 +297,7 @@ func (rh *RouteHandler) CheckManifest(response http.ResponseWriter, request *htt
 	imgStore := rh.getImageStore(name)
 
 	reference, ok := vars["reference"]
-	if !ok || reference == "" {
+	if !test.Ok(ok) || reference == "" {
 		WriteJSON(response,
 			http.StatusNotFound,
 			NewErrorList(NewError(MANIFEST_INVALID, map[string]string{"reference": reference})))

pkg/compliance/v1_0_0/check.go

@@ -670,10 +670,26 @@ func CheckWorkflows(t *testing.T, config *compliance.Config) {
 			So(err, ShouldBeNil)
 			So(resp.StatusCode(), ShouldEqual, http.StatusOK)
 
+			resp, err = resty.R().Get(baseURL + "/v2/page0/tags/list?n= ")
+			So(err, ShouldBeNil)
+			So(resp.StatusCode(), ShouldEqual, http.StatusBadRequest)
+
+			resp, err = resty.R().Get(baseURL + "/v2/page0/tags/list?n=a")
+			So(err, ShouldBeNil)
+			So(resp.StatusCode(), ShouldEqual, http.StatusBadRequest)
+
 			resp, err = resty.R().Get(baseURL + "/v2/page0/tags/list?n=0")
 			So(err, ShouldBeNil)
 			So(resp.StatusCode(), ShouldEqual, http.StatusOK)
 
+			resp, err = resty.R().Get(baseURL + "/v2/page0/tags/list?n=0&last=100")
+			So(err, ShouldBeNil)
+			So(resp.StatusCode(), ShouldEqual, http.StatusNotFound)
+
+			resp, err = resty.R().Get(baseURL + "/v2/page0/tags/list?n=0&last=test:0.0")
+			So(err, ShouldBeNil)
+			So(resp.StatusCode(), ShouldEqual, http.StatusOK)
+
 			resp, err = resty.R().Get(baseURL + "/v2/page0/tags/list?n=3")
 			So(err, ShouldBeNil)
 			So(resp.StatusCode(), ShouldEqual, http.StatusOK)

pkg/test/dev.go

@@ -0,0 +1,81 @@
+//go:build dev
+// +build dev
+
+// This file should be linked only in **development** mode.
+
+package test
+
+import (
+	"sync"
+
+	zerr "zotregistry.io/zot/errors"
+)
+
+func Ok(ok bool) bool {
+	if !ok {
+		return ok
+	}
+
+	if injectedFailure() {
+		return false
+	}
+
+	return true
+}
+
+func Error(err error) error {
+	if err != nil {
+		return err
+	}
+
+	if injectedFailure() {
+		return zerr.ErrInjected
+	}
+
+	return nil
+}
+
+/**
+ *
+ * Failure injection infrastructure to cover hard-to-reach code paths.
+ *
+ **/
+
+type inject struct {
+	skip    int
+	enabled bool
+}
+
+//nolint:gochecknoglobals // only used by test code
+var (
+	injlock sync.Mutex
+	injst   = inject{}
+)
+
+func InjectFailure(skip int) bool {
+	injlock.Lock()
+	injst = inject{enabled: true, skip: skip}
+	injlock.Unlock()
+
+	return true
+}
+
+func injectedFailure() bool {
+	injlock.Lock()
+	defer injlock.Unlock()
+
+	if !injst.enabled {
+		return false
+	}
+
+	if injst.skip == 0 {
+		// disable the injection point
+		injst.enabled = false
+
+		return true
+	}
+
+	injst.skip--
+
+	return false
+}

pkg/test/inject_test.go

@@ -0,0 +1,120 @@
+//go:build dev
+// +build dev
+
+// This file should be linked only in **development** mode.
+
+package test_test
+
+import (
+	"errors"
+	"testing"
+
+	. "github.com/smartystreets/goconvey/convey"
+	"zotregistry.io/zot/pkg/test"
+)
+
+var (
+	errKey1    = errors.New("key1 not found")
+	errKey2    = errors.New("key2 not found")
+	errNotZero = errors.New("not zero")
+	errCall1   = errors.New("call1 error")
+	errCall2   = errors.New("call2 error")
+)
+
+func foo() error {
+	fmap := map[string]string{"key1": "val1", "key2": "val2"}
+
+	_, ok := fmap["key1"] // should never fail
+	if !test.Ok(ok) {
+		return errKey1
+	}
+
+	_, ok = fmap["key2"] // should never fail
+	if !test.Ok(ok) {
+		return errKey2
+	}
+
+	return nil
+}
+
+func errgen(i int) error {
+	if i != 0 {
+		return errNotZero
+	}
+
+	return nil
+}
+
+func bar() error {
+	err := errgen(0) // should never fail
+	if test.Error(err) != nil {
+		return errCall1
+	}
+
+	err = errgen(0) // should never fail
+	if test.Error(err) != nil {
+		return errCall2
+	}
+
+	return nil
+}
+
+func alwaysErr() error {
+	return errNotZero
+}
+
+func alwaysNotOk() bool {
+	return false
+}
+
+func TestInject(t *testing.T) {
+	Convey("Injected failure", t, func(c C) {
+		// should be success without injection
+		err := foo()
+		So(err, ShouldBeNil)
+
+		Convey("Check Ok", func() {
+			Convey("Without skipping", func() {
+				test.InjectFailure(0)   // inject a failure
+				err := foo()            // should be a failure
+				So(err, ShouldNotBeNil) // should be a failure
+				So(errors.Is(err, errKey1), ShouldBeTrue)
+			})
+
+			Convey("With skipping", func() {
+				test.InjectFailure(1) // inject a failure but skip first one
+				err := foo()          // should be a failure
+				So(errors.Is(err, errKey1), ShouldBeFalse)
+				So(errors.Is(err, errKey2), ShouldBeTrue)
+			})
+		})
+
+		// should be success without injection
+		err = bar()
+		So(err, ShouldBeNil)
+
+		Convey("Check Err", func() {
+			Convey("Without skipping", func() {
+				test.InjectFailure(0)   // inject a failure
+				err := bar()            // should be a failure
+				So(err, ShouldNotBeNil) // should be a failure
+				So(errors.Is(err, errCall1), ShouldBeTrue)
+			})
+
+			Convey("With skipping", func() {
+				test.InjectFailure(1) // inject a failure but skip first one
+				err := bar()          // should be a failure
+				So(errors.Is(err, errCall1), ShouldBeFalse)
+				So(errors.Is(err, errCall2), ShouldBeTrue)
+			})
+		})
+	})
+
+	Convey("Without injected failure", t, func(c C) {
+		err := alwaysErr()
+		So(test.Error(err), ShouldNotBeNil)
+
+		ok := alwaysNotOk()
+		So(test.Ok(ok), ShouldBeFalse)
+	})
+}

pkg/test/prod.go

@@ -0,0 +1,20 @@
+//go:build !dev
+// +build !dev
+
+package test
+
+func Error(err error) error {
+	return err
+}
+
+func Ok(ok bool) bool {
+	return ok
+}
+
+/**
+ *
+ * Failure injection infrastructure to cover hard-to-reach code paths (nop in production).
+ *
+ **/
+
+func InjectFailure(skip int) bool { return false }