Merge pull request #71 from jvanz/golinter

chore: use more stricter golang linter configuration.

.github/workflows/test.yml

@@ -44,4 +44,4 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@a4f60bb28d35aeee14e6880718e0c85ff1882e64 # v6.0.1
         with:
-          version: v1.55.2
+          version: v1.59.1

Makefile

@@ -5,7 +5,7 @@ BIN_DIR := $(abspath $(ROOT_DIR)/bin)
 SOURCE_FILES := $(shell find . -type f -name '*.go')
 VERSION ?= $(shell git describe | cut -c2-)
 
-GOLANGCI_LINT_VER := v1.55.2
+GOLANGCI_LINT_VER := v1.59.1
 GOLANGCI_LINT_BIN := golangci-lint
 GOLANGCI_LINT := $(BIN_DIR)/$(GOLANGCI_LINT_BIN)
 
@@ -35,6 +35,14 @@ lint: $(GOLANGCI_LINT)
 	go vet ./...
 	$(GOLANGCI_LINT) run
 
+.PHONY: lint-fix
+lint-fix: $(GOLANGCI_LINT)
+	$(GOLANGCI_LINT) run --fix
+
+.PHONY: fmt
+fmt:
+	go fmt ./...
+
 .PHONY: test
 test:
 	go test -v ./...

internal/cel/cel.go

@@ -41,6 +41,7 @@ func NewCompiler() (*Compiler, error) {
 
 		// Kubernetes 1.29 libraries and extensions
 		ext.Sets(),
+		//nolint:mnd // extract the number 2 to a constant will not make the code more readable
 		ext.Strings(ext.StringsVersion(2)),
 		k8sLibrary.URLs(),
 		k8sLibrary.Regex(),

internal/cel/library/crypto.go

@@ -1,4 +1,4 @@
-//nolint:varnamelen
+//nolint:lll // This file has long lines due some examples in the comments
 package library
 
 import (
@@ -213,7 +213,7 @@ type cryptoVerifier struct {
 
 var cryptoResponseType = cel.ObjectType("kw.crypto.Response")
 
-// cryptoResponse is the response object returned by the verify function
+// cryptoResponse is the response object returned by the verify function.
 type cryptoResponse struct {
 	receiverOnlyObjectVal
 	isTrusted bool

internal/cel/library/crypto_test.go

@@ -1,4 +1,3 @@
-//nolint:dupl
 package library
 
 import (

internal/cel/library/kubernetes.go

@@ -1,4 +1,4 @@
-//nolint:varnamelen
+//nolint:lll // This file has long lines due some examples in the comments
 package library
 
 import (
@@ -308,7 +308,7 @@ func (c *k8sClient) list() ref.Val {
 	}
 
 	var response map[string]interface{}
-	if err := json.Unmarshal(responseBytes, &response); err != nil {
+	if err = json.Unmarshal(responseBytes, &response); err != nil {
 		return types.NewErr("cannot unmarshal Kubernetes list resources response: %s", err)
 	}
 
@@ -330,7 +330,7 @@ func (c *k8sClient) get(name string) ref.Val {
 	}
 
 	var response map[string]interface{}
-	if err := json.Unmarshal(responseBytes, &response); err != nil {
+	if err = json.Unmarshal(responseBytes, &response); err != nil {
 		return types.NewErr("cannot unmarshal Kubernetes get resource response: %s", err)
 	}
 

internal/cel/library/kubernetes_test.go

@@ -1,4 +1,3 @@
-//nolint:dupl
 package library
 
 import (

internal/cel/library/library.go

@@ -4,5 +4,5 @@ package library
 
 import "github.com/kubewarden/policy-sdk-go/pkg/capabilities"
 
-// handle to interact with the policy host
+// handle to interact with the policy host.
 var host = capabilities.NewHost()

internal/cel/library/net_test.go

@@ -1,4 +1,3 @@
-//nolint:dupl
 package library
 
 import (

internal/cel/library/oci_test.go

@@ -1,4 +1,3 @@
-//nolint:dupl
 package library
 
 import (

internal/cel/library/receiver_only_val.go

@@ -14,7 +14,8 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-// This code was extracted from: https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/cel/library/authz.go
+// This code was extracted from:
+// https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/apiserver/pkg/cel/library/authz.go
 
 package library
 

internal/cel/library/sigstore.go

@@ -1,4 +1,4 @@
-//nolint:varnamelen
+//nolint:lll // This file has long lines due some examples in the comments
 package library
 
 import (
@@ -9,6 +9,8 @@ import (
 	verify "github.com/kubewarden/policy-sdk-go/pkg/capabilities/oci/verify_v2"
 )
 
+const sigstoreVerifierArgsCount = 3
+
 // Sigstore provides a CEL function library extension for verifying sigstore signatures of an image.
 //
 // image
@@ -142,6 +144,7 @@ func (*sigstoreLib) LibraryName() string {
 	return "kw.sigstore"
 }
 
+//nolint:funlen // Splitting this function would make it harder to read
 func (*sigstoreLib) CompileOptions() []cel.EnvOption {
 	return []cel.EnvOption{
 		cel.Function("kw.sigstore.image",
@@ -285,7 +288,7 @@ func sigstoreImage(arg ref.Val) ref.Val {
 }
 
 func sigstoreVerifierBuilderAnnotation(args ...ref.Val) ref.Val {
-	if len(args) != 3 {
+	if len(args) != sigstoreVerifierArgsCount {
 		return types.NoSuchOverloadErr()
 	}
 
@@ -358,7 +361,7 @@ func sigstorePubKeysVerifierVerify(arg ref.Val) ref.Val {
 }
 
 func sigstoreVerifierBuilderKeyless(args ...ref.Val) ref.Val {
-	if len(args) != 3 {
+	if len(args) != sigstoreVerifierArgsCount {
 		return types.NoSuchOverloadErr()
 	}
 
@@ -388,7 +391,7 @@ func sigstoreVerifierBuilderKeyless(args ...ref.Val) ref.Val {
 }
 
 func sigstoreKeylessVerifierKeyless(args ...ref.Val) ref.Val {
-	if len(args) != 3 {
+	if len(args) != sigstoreVerifierArgsCount {
 		return types.NoSuchOverloadErr()
 	}
 
@@ -422,7 +425,7 @@ func sigstoreKeylessVerifierVerify(arg ref.Val) ref.Val {
 }
 
 func sigstoreVerifierBuilderKeylessPrefix(args ...ref.Val) ref.Val {
-	if len(args) != 3 {
+	if len(args) != sigstoreVerifierArgsCount {
 		return types.NoSuchOverloadErr()
 	}
 
@@ -452,7 +455,7 @@ func sigstoreVerifierBuilderKeylessPrefix(args ...ref.Val) ref.Val {
 }
 
 func sigstoreKeylessPrefixVerifierKeylessPrefix(args ...ref.Val) ref.Val {
-	if len(args) != 3 {
+	if len(args) != sigstoreVerifierArgsCount {
 		return types.NoSuchOverloadErr()
 	}
 
@@ -505,7 +508,7 @@ func sigstoreVerifierBuilderGitHubActionOwner(arg1, arg2 ref.Val) ref.Val {
 }
 
 func sigstoreVerifierBuilderGitHubActionOwnerRepo(args ...ref.Val) ref.Val {
-	if len(args) != 3 {
+	if len(args) != sigstoreVerifierArgsCount {
 		return types.NoSuchOverloadErr()
 	}
 
@@ -622,7 +625,7 @@ func sigstoreResponseDigest(arg ref.Val) ref.Val {
 
 var sigstoreVerifierBuilderType = cel.ObjectType("kw.sigstore.VerifierBuilder")
 
-// sigstoreVerifierBuilder is an intermediate object is used to build a specific verifier
+// sigstoreVerifierBuilder is an intermediate object is used to build a specific verifier.
 type sigstoreVerifierBuilder struct {
 	receiverOnlyObjectVal
 	image       string
@@ -631,7 +634,7 @@ type sigstoreVerifierBuilder struct {
 
 var sigstorePubKeysVerifierType = cel.ObjectType("kw.sigstore.PubKeysVerifier")
 
-// sigstorePubKeysVerifier verifies the signature of an image using a set of public keys
+// sigstorePubKeysVerifier verifies the signature of an image using a set of public keys.
 type sigstorePubKeysVerifier struct {
 	receiverOnlyObjectVal
 	image       string
@@ -654,7 +657,7 @@ func (v *sigstorePubKeysVerifier) verify() ref.Val {
 
 var sigstoreKeylessVerifierType = cel.ObjectType("kw.sigstore.KeylessVerifier")
 
-// sigstoreKeylessVerifier verifies the signature of an image using keyless signing
+// sigstoreKeylessVerifier verifies the signature of an image using keyless signing.
 type sigstoreKeylessVerifier struct {
 	receiverOnlyObjectVal
 	image       string
@@ -677,7 +680,8 @@ func (v *sigstoreKeylessVerifier) verify() ref.Val {
 
 var sigstoreKeylessPrefixVerifierType = cel.ObjectType("kw.sigstore.KeylessPrefixVerifier")
 
-// sigstoreKeylessPrefixVerifier verifies the signature of an image using keyless signing
+// sigstoreKeylessPrefixVerifier verifies the signature of an image using
+// keyless signing.
 type sigstoreKeylessPrefixVerifier struct {
 	receiverOnlyObjectVal
 	image         string
@@ -700,7 +704,8 @@ func (v *sigstoreKeylessPrefixVerifier) verify() ref.Val {
 
 var sigstoreGitHubActionVerifierType = cel.ObjectType("kw.sigstore.GitHubActionVerifier")
 
-// sigstoreGitHubActionVerifier verifies sigstore signatures of an image using keyless signatures made via Github Actions
+// sigstoreGitHubActionVerifier verifies sigstore signatures of an image using
+// keyless signatures made via Github Actions.
 type sigstoreGitHubActionVerifier struct {
 	receiverOnlyObjectVal
 	image       string
@@ -724,7 +729,7 @@ func (v *sigstoreGitHubActionVerifier) verify() ref.Val {
 
 var sigstoreCertificateVerifierType = cel.ObjectType("kw.sigstore.CertificateVerifier")
 
-// sigstoreCertificateVerifier verifies sigstore signatures of an image using a user provided certificate\
+// sigstoreCertificateVerifier verifies sigstore signatures of an image using a user provided certificate.
 type sigstoreCertificateVerifier struct {
 	receiverOnlyObjectVal
 	image              string
@@ -749,7 +754,7 @@ func (v *sigstoreCertificateVerifier) verify() ref.Val {
 
 var sigstoreResponseType = cel.ObjectType("kw.sigstore.Response")
 
-// sigstoreResponse is the response object returned by the verify function
+// sigstoreResponse is the response object returned by the verify function.
 type sigstoreResponse struct {
 	receiverOnlyObjectVal
 	isTrusted bool

internal/cel/library/sigstore_test.go

@@ -1,4 +1,3 @@
-//nolint:dupl
 package library
 
 import (

internal/settings/settings.go

@@ -1,3 +1,4 @@
+//nolint:lll,govet // This file contains long lines and splitting them would not make the code more readable. Also, we shadow the err variable in some places, but it's not a problem.
 package settings
 
 import (
@@ -20,14 +21,15 @@ const (
 	StatusReasonRequestEntityTooLarge = "RequestEntityTooLarge"
 )
 
+//nolint:gochecknoglobals // []string cannot be const
 var supportedValidationPolicyReason = []string{
 	StatusReasonUnauthorized,
 	StatusReasonForbidden,
 	StatusReasonInvalid,
 	StatusReasonRequestEntityTooLarge,
 }
 
-// Settings defines the settings of the policy
+// Settings defines the settings of the policy.
 type Settings struct {
 	Variables   []Variable   `json:"variables"`
 	Validations []Validation `json:"validations"`
@@ -177,8 +179,7 @@ func validateValidations(compiler *cel.Compiler, index int, validation Validatio
 			result = multierror.Append(result, err)
 		}
 	}
-
-	//nolint:gocritic
+	//nolint:gocritic // Rewriting this code as switch would not make it more readable
 	if len(validation.Message) > 0 && len(trimmedMsg) == 0 {
 		err := newInvalidValueError(fmt.Sprintf("validations[%d].message", index), validation.Message, "message must be non-empty if specified")
 		result = multierror.Append(result, err)

internal/validate/validate.go

@@ -1,7 +1,9 @@
+//nolint:lll,govet // This file contains long lines and splitting them would not make the code more readable.Also, we shadow the err variable in some places, but it's not a problem.
 package validate
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log"
 	"strings"
@@ -14,21 +16,28 @@ import (
 	kubewardenProtocol "github.com/kubewarden/policy-sdk-go/protocol"
 )
 
+const (
+	httpBadRequestStatusCode     = 400
+	httpForbiddenStatusCode      = 403
+	httpEntityTooLargeStatusCode = 413
+	httpUnauthorizedStatusCode   = 401
+)
+
 func Validate(payload []byte) ([]byte, error) {
 	validationRequest := kubewardenProtocol.ValidationRequest{}
 
 	err := json.Unmarshal(payload, &validationRequest)
 	if err != nil {
 		return kubewarden.RejectRequest(
 			kubewarden.Message(fmt.Sprintf("Error deserializing validation request: %v", err)),
-			kubewarden.Code(400))
+			kubewarden.Code(httpBadRequestStatusCode))
 	}
 
 	settings, err := settings.NewSettingsFromValidationReq(&validationRequest)
 	if err != nil {
 		return kubewarden.RejectRequest(
 			kubewarden.Message(fmt.Sprintf("Error serializing RawMessage: %v", err)),
-			kubewarden.Code(400))
+			kubewarden.Code(httpBadRequestStatusCode))
 	}
 
 	compiler, err := cel.NewCompiler()
@@ -52,7 +61,7 @@ func Validate(payload []byte) ([]byte, error) {
 
 	objectMeta, ok := object["metadata"].(map[string]interface{})
 	if !ok {
-		return nil, fmt.Errorf("wrong object format: metadata not found in object")
+		return nil, errors.New("wrong object format: metadata not found in object")
 	}
 
 	vars := map[string]interface{}{
@@ -69,7 +78,7 @@ func Validate(payload []byte) ([]byte, error) {
 		},
 	}
 
-	if err := evalVariables(compiler, vars, settings.Variables); err != nil {
+	if err = evalVariables(compiler, vars, settings.Variables); err != nil {
 		return nil, fmt.Errorf("failed to evaluate variables: %w", err)
 	}
 
@@ -83,7 +92,7 @@ func evalVariables(compiler *cel.Compiler, vars map[string]interface{}, variable
 			return err
 		}
 
-		if err := compiler.AddVariable(variable.Name, ast.OutputType()); err != nil {
+		if err = compiler.AddVariable(variable.Name, ast.OutputType()); err != nil {
 			return err
 		}
 		// lazy load variables
@@ -146,7 +155,7 @@ func evalMessageExpression(compiler *cel.Compiler, vars map[string]interface{},
 
 	message, ok := val.Value().(string)
 	if !ok {
-		return "", fmt.Errorf("message expression must evaluate to string")
+		return "", errors.New("message expression must evaluate to string")
 	}
 
 	return message, nil
@@ -156,13 +165,13 @@ func reasonToStatusCode(reason string) kubewarden.Code {
 	var statusCode kubewarden.Code
 	switch reason {
 	case settings.StatusReasonInvalid:
-		statusCode = kubewarden.Code(400)
+		statusCode = kubewarden.Code(httpBadRequestStatusCode)
 	case settings.StatusReasonForbidden:
-		statusCode = 403
+		statusCode = httpForbiddenStatusCode
 	case settings.StatusReasonRequestEntityTooLarge:
-		statusCode = 413
+		statusCode = httpEntityTooLargeStatusCode
 	case settings.StatusReasonUnauthorized:
-		statusCode = 401
+		statusCode = httpUnauthorizedStatusCode
 	default:
 		// This should never happen since we validate the settings when loading the policy
 		log.Fatalf("unknown reason: %v", reason)