refactor(test): add lint rule for messages starting with the component

Signed-off-by: Laurentiu Niculae <[email protected]>

.github/workflows/golangci-lint.yaml

@@ -47,11 +47,4 @@ jobs:
           make check
       - name: Run log linter
         run: |
-          set +e
-
-          # log messages should never start upper-cased
-          find . -name '*.go' | grep -v '_test.go' | xargs grep -n "Msg(\"[A-Z]" | grep -v -E "Msg\(\"HTTP|OpenID|OAuth|TLS"
-          if [ $? -eq 0 ]; then
-            exit 1
-          fi
-          exit 0
+          make check-logs

Makefile

@@ -310,6 +310,10 @@ $(GOLINTER):
 	curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(TOOLSDIR)/bin $(GOLINTER_VERSION)
 	$(GOLINTER) version
 
+.PHONY: check-logs
+check-logs: 
+	@./scripts/check_logs.sh
+
 .PHONY: check
 check: $(if $(findstring ui,$(BUILD_LABELS)), ui)
 check: ./golangcilint.yaml $(GOLINTER)

pkg/api/authn.go

@@ -790,13 +790,15 @@ func OAuth2Callback(ctlr *Controller, w http.ResponseWriter, r *http.Request, st
 
 	stateOrigin, ok := stateCookie.Values["state"].(string)
 	if !ok {
-		ctlr.Log.Error().Err(zerr.ErrInvalidStateCookie).Msg("openID: unable to get 'state' cookie from request")
+		ctlr.Log.Error().Err(zerr.ErrInvalidStateCookie).Str("component", "openID").
+			Msg(": unable to get 'state' cookie from request")
 
 		return "", zerr.ErrInvalidStateCookie
 	}
 
 	if stateOrigin != state {
-		ctlr.Log.Error().Err(zerr.ErrInvalidStateCookie).Msg("openID: 'state' cookie differs from the actual one")
+		ctlr.Log.Error().Err(zerr.ErrInvalidStateCookie).Str("component", "openID").
+			Msg(": 'state' cookie differs from the actual one")
 
 		return "", zerr.ErrInvalidStateCookie
 	}

pkg/api/authn_test.go

@@ -129,7 +129,7 @@ func TestAPIKeys(t *testing.T) {
 		conf.Extensions.UI.Enable = &defaultVal
 
 		ctlr := api.NewController(conf)
-		ctlr.Log.Info().Int64("seedUser", seedUser).Int64("seedPass", seedPass).Msg("random seed for username & password")
+		ctlr.Log.Info().Int64("seedUser", seedUser).Int64("seedPass", seedPass).Msg("Random seed for username & password")
 		dir := t.TempDir()
 
 		ctlr.Config.Storage.RootDirectory = dir

pkg/api/routes.go

@@ -706,7 +706,7 @@ func (rh *RouteHandler) UpdateManifest(response http.ResponseWriter, request *ht
 			zcommon.WriteJSON(response, http.StatusBadRequest, apiErr.NewErrorList(e))
 		} else {
 			// could be syscall.EMFILE (Err:0x18 too many opened files), etc
-			rh.c.Log.Error().Err(err).Msg("unexpected error: performing cleanup")
+			rh.c.Log.Error().Err(err).Msg("unexpected error, performing cleanup")
 
 			if err = imgStore.DeleteImageManifest(name, reference, false); err != nil {
 				// deletion of image manifest is important, but not critical for image repo consistency
@@ -1428,7 +1428,7 @@ func (rh *RouteHandler) PatchBlobUpload(response http.ResponseWriter, request *h
 			zcommon.WriteJSON(response, http.StatusNotFound, apiErr.NewErrorList(e))
 		} else {
 			// could be io.ErrUnexpectedEOF, syscall.EMFILE (Err:0x18 too many opened files), etc
-			rh.c.Log.Error().Err(err).Msg("unexpected error: removing .uploads/ files")
+			rh.c.Log.Error().Err(err).Msg("unexpected error, removing .uploads/ files")
 
 			if err = imgStore.DeleteBlobUpload(name, sessionID); err != nil {
 				rh.c.Log.Error().Err(err).Str("blobUpload", sessionID).Str("repository", name).
@@ -1550,7 +1550,7 @@ func (rh *RouteHandler) UpdateBlobUpload(response http.ResponseWriter, request *
 				zcommon.WriteJSON(response, http.StatusNotFound, apiErr.NewErrorList(e))
 			} else {
 				// could be io.ErrUnexpectedEOF, syscall.EMFILE (Err:0x18 too many opened files), etc
-				rh.c.Log.Error().Err(err).Msg("unexpected error: removing .uploads/ files")
+				rh.c.Log.Error().Err(err).Msg("unexpected error, removing .uploads/ files")
 
 				if err = imgStore.DeleteBlobUpload(name, sessionID); err != nil {
 					rh.c.Log.Error().Err(err).Str("blobUpload", sessionID).Str("repository", name).
@@ -1585,7 +1585,7 @@ finish:
 			zcommon.WriteJSON(response, http.StatusNotFound, apiErr.NewErrorList(e))
 		} else {
 			// could be io.ErrUnexpectedEOF, syscall.EMFILE (Err:0x18 too many opened files), etc
-			rh.c.Log.Error().Err(err).Msg("unexpected error: removing .uploads/ files")
+			rh.c.Log.Error().Err(err).Msg("unexpected error, removing .uploads/ files")
 
 			if err = imgStore.DeleteBlobUpload(name, sessionID); err != nil {
 				rh.c.Log.Error().Err(err).Str("blobUpload", sessionID).Str("repository", name).

pkg/cli/client/config_cmd_test.go

@@ -221,7 +221,7 @@ func TestConfigCmdMain(t *testing.T) {
 		cmd.SetArgs(args)
 		err := cmd.Execute()
 		So(err, ShouldNotBeNil)
-		So(buff.String(), ShouldContainSubstring, "invalid config")
+		So(buff.String(), ShouldContainSubstring, "invalid server config")
 	})
 
 	Convey("Test remove config bad permissions", t, func() {

pkg/cli/client/cve_cmd_test.go

@@ -369,7 +369,7 @@ func TestServerCVEResponse(t *testing.T) {
 		So(err, ShouldNotBeNil)
 	})
 
-	Convey("Test CVE by image name - GQL - invalid output format", t, func() {
+	Convey("Test CVE by image name - GQL - invalid cli output format", t, func() {
 		args := []string{"list", "zot-cve-test:0.0.1", "-f", "random", "--config", "cvetest"}
 		configPath := makeConfigFile(fmt.Sprintf(`{"configs":[{"_name":"cvetest","url":"%s","showspinner":false}]}`, url))
 		defer os.Remove(configPath)
@@ -380,7 +380,7 @@ func TestServerCVEResponse(t *testing.T) {
 		cveCmd.SetArgs(args)
 		err = cveCmd.Execute()
 		So(err, ShouldNotBeNil)
-		So(buff.String(), ShouldContainSubstring, "invalid output format")
+		So(buff.String(), ShouldContainSubstring, "invalid cli output format")
 	})
 
 	Convey("Test images by CVE ID - GQL - positive", t, func() {
@@ -417,7 +417,7 @@ func TestServerCVEResponse(t *testing.T) {
 		So(str, ShouldNotContainSubstring, "REPOSITORY TAG OS/ARCH DIGEST SIGNED SIZE")
 	})
 
-	Convey("Test images by CVE ID - GQL - invalid output format", t, func() {
+	Convey("Test images by CVE ID - GQL - invalid cli output format", t, func() {
 		args := []string{"affected", "CVE-2019-9923", "-f", "random", "--config", "cvetest"}
 		configPath := makeConfigFile(fmt.Sprintf(`{"configs":[{"_name":"cvetest","url":"%s","showspinner":false}]}`, url))
 		defer os.Remove(configPath)
@@ -428,7 +428,7 @@ func TestServerCVEResponse(t *testing.T) {
 		cveCmd.SetArgs(args)
 		err = cveCmd.Execute()
 		So(err, ShouldNotBeNil)
-		So(buff.String(), ShouldContainSubstring, "invalid output format")
+		So(buff.String(), ShouldContainSubstring, "invalid cli output format")
 	})
 
 	Convey("Test fixed tags by image name and CVE ID - GQL - positive", t, func() {
@@ -532,7 +532,7 @@ func TestServerCVEResponse(t *testing.T) {
 		So(strings.TrimSpace(str), ShouldNotContainSubstring, "REPOSITORY TAG OS/ARCH SIGNED SIZE")
 	})
 
-	Convey("Test CVE by name and CVE ID - GQL - invalid output format", t, func() {
+	Convey("Test CVE by name and CVE ID - GQL - invalid cli output format", t, func() {
 		args := []string{"affected", "CVE-2019-9923", "--repo", "zot-cve-test", "-f", "random", "--config", "cvetest"}
 		configPath := makeConfigFile(fmt.Sprintf(`{"configs":[{"_name":"cvetest","url":"%s","showspinner":false}]}`, url))
 		defer os.Remove(configPath)
@@ -543,7 +543,7 @@ func TestServerCVEResponse(t *testing.T) {
 		cveCmd.SetArgs(args)
 		err = cveCmd.Execute()
 		So(err, ShouldNotBeNil)
-		So(buff.String(), ShouldContainSubstring, "invalid output format")
+		So(buff.String(), ShouldContainSubstring, "invalid cli output format")
 	})
 }
 

pkg/cli/client/image_cmd_internal_test.go

@@ -470,7 +470,7 @@ func TestOutputFormat(t *testing.T) {
 		cmd.SetArgs(args)
 		err := cmd.Execute()
 		So(err, ShouldNotBeNil)
-		So(buff.String(), ShouldContainSubstring, "invalid output format")
+		So(buff.String(), ShouldContainSubstring, "invalid cli output format")
 	})
 }
 

pkg/cli/client/image_cmd_test.go

@@ -500,7 +500,7 @@ func TestOutputFormatGQL(t *testing.T) {
 			cmd.SetArgs(args)
 			err := cmd.Execute()
 			So(err, ShouldNotBeNil)
-			So(buff.String(), ShouldContainSubstring, "invalid output format")
+			So(buff.String(), ShouldContainSubstring, "invalid cli output format")
 		})
 	})
 }
@@ -554,7 +554,7 @@ func TestServerResponseGQL(t *testing.T) {
 				cmd.SetArgs(args)
 				err := cmd.Execute()
 				So(err, ShouldNotBeNil)
-				So(buff.String(), ShouldContainSubstring, "invalid output format")
+				So(buff.String(), ShouldContainSubstring, "invalid cli output format")
 			})
 		})
 
@@ -632,7 +632,7 @@ func TestServerResponseGQL(t *testing.T) {
 				cmd.SetArgs(args)
 				err := cmd.Execute()
 				So(err, ShouldNotBeNil)
-				So(buff.String(), ShouldContainSubstring, "invalid output format")
+				So(buff.String(), ShouldContainSubstring, "invalid cli output format")
 			})
 		})
 
@@ -683,7 +683,7 @@ func TestServerResponseGQL(t *testing.T) {
 				cmd.SetArgs(args)
 				err := cmd.Execute()
 				So(err, ShouldNotBeNil)
-				So(buff.String(), ShouldContainSubstring, "invalid output format")
+				So(buff.String(), ShouldContainSubstring, "invalid cli output format")
 			})
 		})
 

pkg/cli/client/repo_test.go

@@ -95,7 +95,7 @@ func TestSuggestions(t *testing.T) {
 		suggestion := client.ShowSuggestionsIfUnknownCommand(
 			client.NewRepoCommand(client.NewSearchService()), []string{"bad-command"})
 		str := space.ReplaceAllString(suggestion.Error(), " ")
-		So(str, ShouldContainSubstring, "unknown subcommand")
+		So(str, ShouldContainSubstring, "unknown cli subcommand")
 
 		suggestion = client.ShowSuggestionsIfUnknownCommand(
 			client.NewRepoCommand(client.NewSearchService()), []string{"listt"})

pkg/cli/server/root.go

@@ -559,15 +559,15 @@ func applyDefaultValues(config *config.Config, viperInstance *viper.Viper, log z
 
 				if config.Extensions.Search.CVE.Trivy.DBRepository == "" {
 					defaultDBDownloadURL := "ghcr.io/aquasecurity/trivy-db"
-					log.Info().Str("url", defaultDBDownloadURL).
-						Msg("config: using default trivy-db download URL.")
+					log.Info().Str("url", defaultDBDownloadURL).Str("component", "config").
+						Msg("using default trivy-db download URL.")
 					config.Extensions.Search.CVE.Trivy.DBRepository = defaultDBDownloadURL
 				}
 
 				if config.Extensions.Search.CVE.Trivy.JavaDBRepository == "" {
 					defaultJavaDBDownloadURL := "ghcr.io/aquasecurity/trivy-java-db"
-					log.Info().Str("url", defaultJavaDBDownloadURL).
-						Msg("config: using default trivy-java-db download URL.")
+					log.Info().Str("url", defaultJavaDBDownloadURL).Str("component", "config").
+						Msg("using default trivy-java-db download URL.")
 					config.Extensions.Search.CVE.Trivy.JavaDBRepository = defaultJavaDBDownloadURL
 				}
 			}
@@ -643,7 +643,7 @@ func applyDefaultValues(config *config.Config, viperInstance *viper.Viper, log z
 		cachePath := path.Join(cacheDir, storageConstants.BoltdbName+storageConstants.DBExtensionName)
 
 		if _, err := os.Stat(cachePath); err == nil {
-			log.Info().Msg("config: dedupe set to false for s3 driver but used to be true.")
+			log.Info().Str("component", "config").Msg("dedupe set to false for s3 driver but used to be true.")
 			log.Info().Str("cache path", cachePath).Msg("found cache database")
 
 			config.Storage.RemoteCache = false
@@ -664,7 +664,7 @@ func applyDefaultValues(config *config.Config, viperInstance *viper.Viper, log z
 			subpathCachePath := path.Join(subpathCacheDir, storageConstants.BoltdbName+storageConstants.DBExtensionName)
 
 			if _, err := os.Stat(subpathCachePath); err == nil {
-				log.Info().Msg("config: dedupe set to false for s3 driver but used to be true. ")
+				log.Info().Str("component", "config").Msg("dedupe set to false for s3 driver but used to be true. ")
 				log.Info().Str("cache path", subpathCachePath).Msg("found cache database")
 
 				storageConfig.RemoteCache = false
@@ -1008,8 +1008,8 @@ func validateSync(config *config.Config, log zlog.Logger) error {
 
 					if content.StripPrefix && !strings.Contains(content.Prefix, "/*") && content.Destination == "/" {
 						log.Error().Err(zerr.ErrBadConfig).
-							Interface("sync content", content).
-							Msg("sync config: can not use stripPrefix true and destination '/' without using glob patterns in prefix")
+							Interface("sync content", content).Str("component", "sync").
+							Msg("can not use stripPrefix true and destination '/' without using glob patterns in prefix")
 
 						return zerr.ErrBadConfig
 					}

pkg/extensions/extension_image_trust.go

@@ -84,7 +84,7 @@ type ImageTrust struct {
 func (trust *ImageTrust) HandleCosignPublicKeyUpload(response http.ResponseWriter, request *http.Request) {
 	body, err := io.ReadAll(request.Body)
 	if err != nil {
-		trust.Log.Error().Err(err).Msg("image trust: couldn't read cosign key body")
+		trust.Log.Error().Err(err).Str("component", "image-trust").Msg("couldn't read cosign key body")
 		response.WriteHeader(http.StatusInternalServerError)
 
 		return
@@ -95,7 +95,7 @@ func (trust *ImageTrust) HandleCosignPublicKeyUpload(response http.ResponseWrite
 		if errors.Is(err, zerr.ErrInvalidPublicKeyContent) {
 			response.WriteHeader(http.StatusBadRequest)
 		} else {
-			trust.Log.Error().Err(err).Msg("image trust: failed to save cosign key")
+			trust.Log.Error().Err(err).Str("component", "image-trust").Msg("failed to save cosign key")
 			response.WriteHeader(http.StatusInternalServerError)
 		}
 
@@ -127,7 +127,7 @@ func (trust *ImageTrust) HandleNotationCertificateUpload(response http.ResponseW
 
 	body, err := io.ReadAll(request.Body)
 	if err != nil {
-		trust.Log.Error().Err(err).Msg("image trust: couldn't read notation certificate body")
+		trust.Log.Error().Err(err).Str("component", "image-trust").Msg("couldn't read notation certificate body")
 		response.WriteHeader(http.StatusInternalServerError)
 
 		return
@@ -139,7 +139,7 @@ func (trust *ImageTrust) HandleNotationCertificateUpload(response http.ResponseW
 			errors.Is(err, zerr.ErrInvalidCertificateContent) {
 			response.WriteHeader(http.StatusBadRequest)
 		} else {
-			trust.Log.Error().Err(err).Msg("image trust: failed to save notation certificate")
+			trust.Log.Error().Err(err).Str("component", "image-trust").Msg("failed to save notation certificate")
 			response.WriteHeader(http.StatusInternalServerError)
 		}
 

pkg/extensions/extension_mgmt.go

@@ -125,7 +125,7 @@ func (mgmt *Mgmt) HandleGetConfig(w http.ResponseWriter, r *http.Request) {
 
 	buf, err := zcommon.MarshalThroughStruct(sanitizedConfig, &StrippedConfig{})
 	if err != nil {
-		mgmt.Log.Error().Err(err).Msg("mgmt: couldn't marshal config response")
+		mgmt.Log.Error().Err(err).Str("component", "mgmt").Msg("couldn't marshal config response")
 		w.WriteHeader(http.StatusInternalServerError)
 	}
 

pkg/extensions/imagetrust/image_trust.go

@@ -167,7 +167,7 @@ func (imgTrustStore *ImageTrustStore) VerifySignature(
 	}
 
 	if manifestDigest.String() == "" {
-		return "", time.Time{}, false, zerr.ErrBadManifestDigest
+		return "", time.Time{}, false, zerr.ErrBadSignatureManifestDigest
 	}
 
 	switch signatureType {

pkg/extensions/imagetrust/image_trust_test.go

@@ -156,7 +156,7 @@ func TestVerifySignatures(t *testing.T) {
 		imgTrustStore := &imagetrust.ImageTrustStore{}
 		_, _, _, err := imgTrustStore.VerifySignature("", []byte(""), "", "", image.AsImageMeta(), "repo")
 		So(err, ShouldNotBeNil)
-		So(err, ShouldEqual, zerr.ErrBadManifestDigest)
+		So(err, ShouldEqual, zerr.ErrBadSignatureManifestDigest)
 	})
 
 	Convey("wrong signature type", t, func() {

pkg/extensions/lint/lint.go

@@ -43,15 +43,15 @@ func (linter *Linter) CheckMandatoryAnnotations(repo string, manifestDigest godi
 
 	content, err := imgStore.GetBlobContent(repo, manifestDigest)
 	if err != nil {
-		linter.log.Error().Err(err).Msg("linter: unable to get image manifest")
+		linter.log.Error().Err(err).Str("component", "linter").Msg("unable to get image manifest")
 
 		return false, err
 	}
 
 	var manifest ispec.Manifest
 
 	if err := json.Unmarshal(content, &manifest); err != nil {
-		linter.log.Error().Err(err).Msg("linter: couldn't unmarshal manifest JSON")
+		linter.log.Error().Err(err).Str("component", "linter").Msg("couldn't unmarshal manifest JSON")
 
 		return false, err
 	}
@@ -78,15 +78,15 @@ func (linter *Linter) CheckMandatoryAnnotations(repo string, manifestDigest godi
 
 	content, err = imgStore.GetBlobContent(repo, configDigest)
 	if err != nil {
-		linter.log.Error().Err(err).Msg("linter: couldn't get config JSON " +
+		linter.log.Error().Err(err).Str("component", "linter").Msg("couldn't get config JSON " +
 			configDigest.String())
 
 		return false, err
 	}
 
 	var imageConfig ispec.Image
 	if err := json.Unmarshal(content, &imageConfig); err != nil {
-		linter.log.Error().Err(err).Msg("linter: couldn't unmarshal config JSON " + configDigest.String())
+		linter.log.Error().Err(err).Str("component", "linter").Msg("couldn't unmarshal config JSON " + configDigest.String())
 
 		return false, err
 	}

pkg/extensions/monitoring/minimal_client.go

@@ -70,12 +70,12 @@ func (mc *MetricsClient) GetMetrics() (*MetricsInfo, error) {
 func (mc *MetricsClient) makeGETRequest(url string, resultsPtr interface{}) (http.Header, error) {
 	req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, url, nil)
 	if err != nil {
-		return nil, fmt.Errorf("metric scraping: %w", err)
+		return nil, fmt.Errorf("metric scraping failed: %w", err)
 	}
 
 	resp, err := mc.config.HTTPClient.Do(req)
 	if err != nil {
-		return nil, fmt.Errorf("metric scraping error: %w", err)
+		return nil, fmt.Errorf("metric scraping failed: %w", err)
 	}
 
 	defer resp.Body.Close()

pkg/extensions/monitoring/monitoring_test.go

@@ -476,11 +476,11 @@ func TestPopulateStorageMetrics(t *testing.T) {
 
 		// Wait for storage metrics to update
 		found, err := test.ReadLogFileAndSearchString(logPath,
-			"monitoring: computed storage usage for repo alpine", time.Minute)
+			"computed storage usage for repo alpine", time.Minute)
 		So(err, ShouldBeNil)
 		So(found, ShouldBeTrue)
 		found, err = test.ReadLogFileAndSearchString(logPath,
-			"monitoring: computed storage usage for repo busybox", time.Minute)
+			"computed storage usage for repo busybox", time.Minute)
 		So(err, ShouldBeNil)
 		So(found, ShouldBeTrue)