Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
refactor(test): add lint rule for messages starting with the component
Browse files Browse the repository at this point in the history
Signed-off-by: Laurentiu Niculae <niculae.laurentiu1@gmail.com>
laurentiuNiculae committed Nov 28, 2023
1 parent 709c904 commit 457c132
Showing 48 changed files with 429 additions and 328 deletions.
9 changes: 1 addition & 8 deletions .github/workflows/golangci-lint.yaml
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -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)
233 changes: 115 additions & 118 deletions errors/errors.go

Large diffs are not rendered by default.

6 changes: 4 additions & 2 deletions pkg/api/authn.go
Original file line number Diff line number Diff line change
@@ -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
}
2 changes: 1 addition & 1 deletion pkg/api/authn_test.go
Original file line number Diff line number Diff line change
@@ -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
8 changes: 4 additions & 4 deletions pkg/api/routes.go
Original file line number Diff line number Diff line change
@@ -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).
2 changes: 1 addition & 1 deletion pkg/cli/client/config_cmd_test.go
Original file line number Diff line number Diff line change
@@ -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() {
12 changes: 6 additions & 6 deletions pkg/cli/client/cve_cmd_test.go
Original file line number Diff line number Diff line change
@@ -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")
})
}

2 changes: 1 addition & 1 deletion pkg/cli/client/image_cmd_internal_test.go
Original file line number Diff line number Diff line change
@@ -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")
})
}

8 changes: 4 additions & 4 deletions pkg/cli/client/image_cmd_test.go
Original file line number Diff line number Diff line change
@@ -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")
})
})

2 changes: 1 addition & 1 deletion pkg/cli/client/repo_test.go
Original file line number Diff line number Diff line change
@@ -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"})
20 changes: 10 additions & 10 deletions pkg/cli/server/root.go
Original file line number Diff line number Diff line change
@@ -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
@@ -798,15 +798,15 @@ func readLDAPCredentials(ldapConfigPath string) (config.LDAPCredentials, error)
viperInstance.SetConfigFile(ldapConfigPath)

if err := viperInstance.ReadInConfig(); err != nil {
log.Error().Err(err).Msg("error while reading configuration")
log.Error().Err(err).Msg("failed to read configuration")

return config.LDAPCredentials{}, err
}

var ldapCredentials config.LDAPCredentials

if err := viperInstance.Unmarshal(&ldapCredentials); err != nil {
log.Error().Err(err).Msg("error while unmarshaling new config")
log.Error().Err(err).Msg("failed to unmarshale new config")

return config.LDAPCredentials{}, err
}
@@ -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
}
8 changes: 4 additions & 4 deletions pkg/extensions/extension_image_trust.go
Original file line number Diff line number Diff line change
@@ -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)
}

2 changes: 1 addition & 1 deletion pkg/extensions/extension_mgmt.go
Original file line number Diff line number Diff line change
@@ -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)
}

2 changes: 1 addition & 1 deletion pkg/extensions/imagetrust/image_trust.go
Original file line number Diff line number Diff line change
@@ -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 {
2 changes: 1 addition & 1 deletion pkg/extensions/imagetrust/image_trust_test.go
Original file line number Diff line number Diff line change
@@ -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() {
8 changes: 4 additions & 4 deletions pkg/extensions/lint/lint.go
Original file line number Diff line number Diff line change
@@ -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
}
4 changes: 2 additions & 2 deletions pkg/extensions/monitoring/minimal_client.go
Original file line number Diff line number Diff line change
@@ -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()
4 changes: 2 additions & 2 deletions pkg/extensions/monitoring/monitoring_test.go
Original file line number Diff line number Diff line change
@@ -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)

Loading

0 comments on commit 457c132

Please sign in to comment.