build: Add revive linter (#7)

.golangci.yml

@@ -18,7 +18,6 @@ linters:
     - gci
     - goconst
     - gocritic
-    - gocyclo
     - godot
     - gofumpt
     - gosec
@@ -29,6 +28,7 @@ linters:
     - misspell
     - nolintlint
     - prealloc
+    - revive
     - staticcheck
     - stylecheck
     - tenv
@@ -71,6 +71,36 @@ linters-settings:
     extra-rules: true
   lll:
     line-length: 120
+  revive:
+    enable-all-rules: true
+    ignore-generated-header: true
+    max-open-files: 2048
+    severity: error
+    rules:
+      - name: add-constant
+        arguments:
+          - maxLitCount: "3"
+            allowStrs: '"","testdata"'
+            allowInts: "0,1,2"
+            allowFloats: "0.0,0.,1.0,1.,2.0,2."
+      - name: argument-limit
+        arguments: [4]
+      - name: cognitive-complexity
+        disabled: true
+      - name: function-length
+        arguments: [30, 0]
+      - name: max-public-structs
+        arguments: [10]
+      - name: file-header
+        disabled: true
+      - name: line-length-limit
+        disabled: true
+      - name: banned-characters
+        disabled: true
+      - name: cyclomatic
+        arguments: [10]
+      - name: function-result-limit
+        arguments: [3]
 
 issues:
   exclude-rules:

pkg/credentialprovider/plugin_test.go

@@ -17,13 +17,14 @@ import (
 
 type fakePlugin struct{}
 
-func (f *fakePlugin) GetCredentials(
-	ctx context.Context,
-	image string,
-	args []string,
+func (fakePlugin) GetCredentials(
+	_ context.Context,
+	_ string,
+	_ []string,
 ) (*v1alpha1.CredentialProviderResponse, error) {
 	return &v1alpha1.CredentialProviderResponse{
-		CacheKeyType:  v1alpha1.RegistryPluginCacheKeyType,
+		CacheKeyType: v1alpha1.RegistryPluginCacheKeyType,
+		//nolint:revive // Dummy value in test file, no need for const.
 		CacheDuration: &metav1.Duration{Duration: 10 * time.Minute},
 		Auth: map[string]v1alpha1.AuthConfig{
 			"*.registry.io": {
@@ -45,27 +46,27 @@ func Test_runPlugin(t *testing.T) {
 	}{
 		{
 			name: "successful test case",
-			//nolint:lll // just a long string
+			//nolint:lll // Just a long string.
 			req: `{"kind":"CredentialProviderRequest","apiVersion":"credentialprovider.kubelet.k8s.io/v1alpha1","image":"test.registry.io/foobar"}`,
-			//nolint:lll // just a long string
+			//nolint:lll // Just a long string.
 			expectedOut: `{"kind":"CredentialProviderResponse","apiVersion":"credentialprovider.kubelet.k8s.io/v1alpha1","cacheKeyType":"Registry","cacheDuration":"10m0s","auth":{"*.registry.io":{"username":"user","password":"password"}}}
 `,
 		},
 		{
 			name: "invalid kind",
-			//nolint:lll // just a long string
+			//nolint:lll // Just a long string.
 			req:       `{"kind":"CredentialProviderFoo","apiVersion":"credentialprovider.kubelet.k8s.io/v1alpha1","image":"test.registry.io/foobar"}`,
 			expectErr: ErrUnsupportedRequestKind,
 		},
 		{
 			name: "invalid apiVersion",
-			//nolint:lll // just a long string
+			//nolint:lll // Just a long string.
 			req:       `{"kind":"CredentialProviderRequest","apiVersion":"foo.k8s.io/v1alpha1","image":"test.registry.io/foobar"}`,
 			expectErr: ErrUnsupportedAPIVersion,
 		},
 		{
 			name: "empty image",
-			//nolint:lll // just a long string
+			//nolint:lll // Just a long string.
 			req:       `{"kind":"CredentialProviderRequest","apiVersion":"credentialprovider.kubelet.k8s.io/v1alpha1","image":""}`,
 			expectErr: ErrEmptyImageInRequest,
 		},
@@ -76,7 +77,7 @@ func Test_runPlugin(t *testing.T) {
 
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			p := NewCredentialProvider(&fakePlugin{})
+			p := NewCredentialProvider(fakePlugin{})
 
 			out := &bytes.Buffer{}
 			require.ErrorIs(

pkg/credentialprovider/static_credentials.go

@@ -23,9 +23,9 @@ func NewStaticProvider(credentialsFile string) (CredentialProvider, error) {
 
 // GetCredentials will ignore the image and args arguments and simply read a credentials file and return its content.
 func (s staticProvider) GetCredentials(
-	ctx context.Context,
-	image string,
-	args []string,
+	_ context.Context,
+	_ string,
+	_ []string,
 ) (response *v1alpha1.CredentialProviderResponse, err error) {
 	credentials, err := os.ReadFile(s.credentialsFile)
 	if err != nil {

pkg/credentialprovider/static_credentials_test.go

@@ -70,6 +70,7 @@ func TestGetCredentials(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 			credentialsFile := path.Join(t.TempDir(), "image-credentials.json")
+			//nolint:revive // Dummy value in test file, no need for const.
 			err := os.WriteFile(credentialsFile, []byte(tt.credentialsString), 0o600)
 			require.NoError(t, err, "error writing temporary credentials file")
 

pkg/install/install.go

@@ -15,6 +15,14 @@ import (
 	"go.etcd.io/etcd/client/pkg/v3/fileutil"
 )
 
+//nolint:gosec // None of these are security credentials.
+const (
+	CredentialProviderSourceDirEnvVar      = "CREDENTIAL_PROVIDER_SOURCE_DIR"
+	CredentialProviderTargetDirEnvVar      = "CREDENTIAL_PROVIDER_TARGET_DIR"
+	SkipCredentialProviderBinariesEnvVar   = "SKIP_CREDENTIAL_PROVIDER_BINARIES"
+	UpdateCredentialProviderBinariesEnvVar = "UPDATE_CREDENTIAL_PROVIDER_BINARIES"
+)
+
 type config struct {
 	// SkipCredentialProviderBinaries is a comma-separated list of binaries. Each binary in the list.
 	// will be skipped when installing to the host.
@@ -25,11 +33,11 @@ type config struct {
 	UpdateCredentialProviderBinaries bool `envconfig:"UPDATE_CREDENTIAL_PROVIDER_BINARIES" default:"true"`
 
 	// CredentialProviderSourceDir controls where to read the binaries from to copy to the host.
-	//nolint:lll // This is just a long default dir.
+	//nolint:lll // Long struct tag value.
 	CredentialProviderSourceDir string `envconfig:"CREDENTIAL_PROVIDER_SOURCE_DIR" default:"/opt/image-credential-provider/bin/"`
 
 	// CredentialProviderTargetDir controls where to place the binaries on the host.
-	//nolint:lll // This is just a long default dir.
+	//nolint:lll // Long struct tag value.
 	CredentialProviderTargetDir string `envconfig:"CREDENTIAL_PROVIDER_TARGET_DIR" default:"/host/etc/kubernetes/image-credential-provider/"`
 }
 
@@ -105,7 +113,7 @@ func Install(logger logrus.FieldLogger) error {
 }
 
 // copyFileAndPermissions copies file permission.
-func copyFileAndPermissions(src, dst string, logger logrus.FieldLogger) (err error) {
+func copyFileAndPermissions(src, dst string, logger logrus.FieldLogger) error {
 	// If the source and destination are the same, we can simply return.
 	skip, err := destinationUpToDate(src, dst, logger)
 	if err != nil {
@@ -129,12 +137,14 @@ func copyFileAndPermissions(src, dst string, logger logrus.FieldLogger) (err err
 		return fmt.Errorf("failed to rename file: %s", err)
 	}
 
-	return
+	return nil
 }
 
 // destinationUptoDate compares the given files and returns
 // whether or not the destination file needs to be updated with the
 // contents of the source file.
+//
+//nolint:revive // Easy to read func that just does a lot, ignore cyclomatic complexity.
 func destinationUpToDate(src, dst string, logger logrus.FieldLogger) (bool, error) {
 	// Stat the src file.
 	f1info, err := os.Stat(src)

pkg/install/install_test.go

@@ -60,8 +60,8 @@ func assertFileHashesDifferent(t *testing.T, a, b string) {
 func TestSuccessfulCopy(t *testing.T) {
 	tmpDir := t.TempDir()
 
-	t.Setenv("CREDENTIAL_PROVIDER_SOURCE_DIR", "testdata")
-	t.Setenv("CREDENTIAL_PROVIDER_TARGET_DIR", tmpDir)
+	t.Setenv(install.CredentialProviderSourceDirEnvVar, "testdata")
+	t.Setenv(install.CredentialProviderTargetDirEnvVar, tmpDir)
 
 	require.NoError(t, install.Install(logrus.New()))
 
@@ -84,9 +84,11 @@ func TestSuccessfulCopy(t *testing.T) {
 func TestSuccessfulCopySkipFile(t *testing.T) {
 	tmpDir := t.TempDir()
 
-	t.Setenv("CREDENTIAL_PROVIDER_SOURCE_DIR", "testdata")
-	t.Setenv("CREDENTIAL_PROVIDER_TARGET_DIR", tmpDir)
-	t.Setenv("SKIP_CREDENTIAL_PROVIDER_BINARIES", "dummybinary2")
+	const dummyBinary2Name = "dummybinary2"
+
+	t.Setenv(install.CredentialProviderSourceDirEnvVar, "testdata")
+	t.Setenv(install.CredentialProviderTargetDirEnvVar, tmpDir)
+	t.Setenv(install.SkipCredentialProviderBinariesEnvVar, dummyBinary2Name)
 
 	require.NoError(t, install.Install(logrus.New()))
 
@@ -96,7 +98,7 @@ func TestSuccessfulCopySkipFile(t *testing.T) {
 	for _, f := range testFiles {
 		destFile := filepath.Join(tmpDir, f.Name())
 
-		if f.Name() == "dummybinary2" {
+		if f.Name() == dummyBinary2Name {
 			assert.NoFileExists(t, destFile)
 			continue
 		}
@@ -115,12 +117,13 @@ func TestSuccessfulCopySkipFile(t *testing.T) {
 func TestSuccessfulCopyDoNotUpdate(t *testing.T) {
 	tmpDir := t.TempDir()
 
-	t.Setenv("CREDENTIAL_PROVIDER_SOURCE_DIR", "testdata")
-	t.Setenv("CREDENTIAL_PROVIDER_TARGET_DIR", tmpDir)
-	t.Setenv("UPDATE_CREDENTIAL_PROVIDER_BINARIES", "false")
+	t.Setenv(install.CredentialProviderSourceDirEnvVar, "testdata")
+	t.Setenv(install.CredentialProviderTargetDirEnvVar, tmpDir)
+	t.Setenv(install.UpdateCredentialProviderBinariesEnvVar, "false")
 
 	assert.NoError(
 		t,
+		//nolint:revive // Dummy value in test file, no need for const.
 		os.WriteFile(filepath.Join(tmpDir, "dummybinary2"), []byte("differentcontent"), 0o600),
 	)