chore: extend require-error rule from testifylint (argoproj#18658)

Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>

.golangci.yaml

@@ -7,7 +7,7 @@ issues:
       text: "require-error:"
       linters:
         - testifylint
-    - path: "util/(argo|cache|cert|clusterauth|config|db|dex|git|gpg|grpc|helm|http|io|kube|kustomize|lua|notification|oidc|rbac|security|session|settings|tls|webhook)/"
+    - path: "util/(argo|cache|cert|clusterauth|config|db|dex|git|gpg|grpc|helm|http|io|kube|kustomize|lua|notification)/"
       text: "require-error:"
       linters:
         - testifylint

util/oidc/oidc_test.go

@@ -33,10 +33,10 @@ func TestInferGrantType(t *testing.T) {
 	for _, path := range []string{"dex", "okta", "auth0", "onelogin"} {
 		t.Run(path, func(t *testing.T) {
 			rawConfig, err := os.ReadFile("testdata/" + path + ".json")
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			var config OIDCConfiguration
 			err = json.Unmarshal(rawConfig, &config)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			grantType := InferGrantType(&config)
 			assert.Equal(t, GrantTypeAuthorizationCode, grantType)
 
@@ -74,10 +74,10 @@ func TestIDTokenClaims(t *testing.T) {
 	assert.Len(t, opts, 1)
 
 	authCodeURL, err := url.Parse(oauth2Config.AuthCodeURL("TEST", opts...))
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	values, err := url.ParseQuery(authCodeURL.RawQuery)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	assert.Equal(t, "{\"id_token\":{\"groups\":{\"essential\":true}}}", values.Get("claims"))
 }
@@ -421,7 +421,7 @@ func TestGenerateAppState(t *testing.T) {
 		}
 
 		returnURL, err := app.verifyAppState(req, httptest.NewRecorder(), state)
-		assert.NoError(t, err)
+		require.NoError(t, err)
 		assert.Equal(t, expectedReturnURL, returnURL)
 	})
 
@@ -432,7 +432,7 @@ func TestGenerateAppState(t *testing.T) {
 		}
 
 		_, err := app.verifyAppState(req, httptest.NewRecorder(), "wrong state")
-		assert.Error(t, err)
+		require.Error(t, err)
 	})
 }
 
@@ -465,7 +465,7 @@ func TestGenerateAppState_XSS(t *testing.T) {
 		}
 
 		returnURL, err := app.verifyAppState(req, httptest.NewRecorder(), state)
-		assert.ErrorIs(t, err, InvalidRedirectURLError)
+		require.ErrorIs(t, err, InvalidRedirectURLError)
 		assert.Empty(t, returnURL)
 	})
 
@@ -481,7 +481,7 @@ func TestGenerateAppState_XSS(t *testing.T) {
 		}
 
 		returnURL, err := app.verifyAppState(req, httptest.NewRecorder(), state)
-		assert.NoError(t, err, InvalidRedirectURLError)
+		require.NoError(t, err, InvalidRedirectURLError)
 		assert.Equal(t, expectedReturnURL, returnURL)
 	})
 }
@@ -502,7 +502,7 @@ func TestGenerateAppState_NoReturnURL(t *testing.T) {
 
 	req.AddCookie(&http.Cookie{Name: common.StateCookieName, Value: hex.EncodeToString(encrypted)})
 	returnURL, err := app.verifyAppState(req, httptest.NewRecorder(), "123")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, "/argo-cd", returnURL)
 }
 
@@ -728,15 +728,15 @@ func TestGetUserInfo(t *testing.T) {
 			require.NoError(t, err)
 			cdSettings := &settings.ArgoCDSettings{ServerSignature: signature}
 			encryptionKey, err := cdSettings.GetServerEncryptionKey()
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			a, _ := NewClientApp(cdSettings, "", nil, "/argo-cd", tt.cache)
 
 			for _, item := range tt.cacheItems {
 				var newValue []byte
 				newValue = []byte(item.value)
 				if item.encrypt {
 					newValue, err = crypto.Encrypt([]byte(item.value), encryptionKey)
-					assert.NoError(t, err)
+					require.NoError(t, err)
 				}
 				err := a.clientCache.Set(&cache.Item{
 					Key:    item.key,
@@ -749,9 +749,9 @@ func TestGetUserInfo(t *testing.T) {
 			assert.Equal(t, tt.expectedOutput, got)
 			assert.Equal(t, tt.expectUnauthenticated, unauthenticated)
 			if tt.expectError {
-				assert.Error(t, err)
+				require.Error(t, err)
 			} else {
-				assert.NoError(t, err)
+				require.NoError(t, err)
 			}
 			for _, item := range tt.expectedCacheItems {
 				var tmpValue []byte

util/rbac/rbac_norace_test.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	apiv1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/kubernetes/fake"
 )
@@ -48,7 +49,7 @@ func TestPolicyInformer(t *testing.T) {
 	// update the configmap and update policy
 	delete(cm.Data, ConfigMapPolicyCSVKey)
 	err := enf.syncUpdate(cm, noOpUpdate)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.False(t, enf.Enforce("admin", "applications", "delete", "foo/bar"))
 }
 

util/rbac/rbac_test.go

@@ -108,8 +108,7 @@ func TestPolicyCSV(t *testing.T) {
 func TestBuiltinPolicyEnforcer(t *testing.T) {
 	kubeclientset := fake.NewSimpleClientset()
 	enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil)
-	err := enf.syncUpdate(fakeConfigMap(), noOpUpdate)
-	assert.NoError(t, err)
+	require.NoError(t, enf.syncUpdate(fakeConfigMap(), noOpUpdate))
 
 	// Without setting builtin policy, this should fail
 	assert.False(t, enf.Enforce("admin", "applications", "get", "foo/bar"))
@@ -181,8 +180,7 @@ g, alice, role:foo-readonly
 func TestDefaultRole(t *testing.T) {
 	kubeclientset := fake.NewSimpleClientset()
 	enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil)
-	err := enf.syncUpdate(fakeConfigMap(), noOpUpdate)
-	assert.NoError(t, err)
+	require.NoError(t, enf.syncUpdate(fakeConfigMap(), noOpUpdate))
 	_ = enf.SetBuiltinPolicy(assets.BuiltinPolicyCSV)
 
 	assert.False(t, enf.Enforce("bob", "applications", "get", "foo/bar"))
@@ -195,8 +193,7 @@ func TestDefaultRole(t *testing.T) {
 func TestURLAsObjectName(t *testing.T) {
 	kubeclientset := fake.NewSimpleClientset()
 	enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil)
-	err := enf.syncUpdate(fakeConfigMap(), noOpUpdate)
-	assert.NoError(t, err)
+	require.NoError(t, enf.syncUpdate(fakeConfigMap(), noOpUpdate))
 	policy := `
 p, alice, repositories, *, foo/*, allow
 p, bob, repositories, *, foo/https://github.com/argoproj/argo-cd.git, allow
@@ -294,8 +291,7 @@ func TestClaimsEnforcerFunc(t *testing.T) {
 func TestDefaultRoleWithRuntimePolicy(t *testing.T) {
 	kubeclientset := fake.NewSimpleClientset()
 	enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil)
-	err := enf.syncUpdate(fakeConfigMap(), noOpUpdate)
-	assert.NoError(t, err)
+	require.NoError(t, enf.syncUpdate(fakeConfigMap(), noOpUpdate))
 	runtimePolicy := assets.BuiltinPolicyCSV
 	assert.False(t, enf.EnforceRuntimePolicy("", runtimePolicy, "bob", "applications", "get", "foo/bar"))
 	enf.SetDefaultRole("role:readonly")
@@ -307,8 +303,7 @@ func TestDefaultRoleWithRuntimePolicy(t *testing.T) {
 func TestClaimsEnforcerFuncWithRuntimePolicy(t *testing.T) {
 	kubeclientset := fake.NewSimpleClientset()
 	enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil)
-	err := enf.syncUpdate(fakeConfigMap(), noOpUpdate)
-	assert.NoError(t, err)
+	require.NoError(t, enf.syncUpdate(fakeConfigMap(), noOpUpdate))
 	runtimePolicy := assets.BuiltinPolicyCSV
 	claims := jwt.RegisteredClaims{
 		Subject: "foo",
@@ -325,8 +320,7 @@ func TestInvalidRuntimePolicy(t *testing.T) {
 	cm := fakeConfigMap()
 	kubeclientset := fake.NewSimpleClientset(cm)
 	enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil)
-	err := enf.syncUpdate(fakeConfigMap(), noOpUpdate)
-	assert.NoError(t, err)
+	require.NoError(t, enf.syncUpdate(fakeConfigMap(), noOpUpdate))
 	_ = enf.SetBuiltinPolicy(assets.BuiltinPolicyCSV)
 	assert.True(t, enf.EnforceRuntimePolicy("", "", "admin", "applications", "update", "foo/bar"))
 	assert.False(t, enf.EnforceRuntimePolicy("", "", "role:readonly", "applications", "update", "foo/bar"))
@@ -344,14 +338,14 @@ func TestValidatePolicy(t *testing.T) {
 		` p, role:admin, projects, delete, *, allow `,
 	}
 	for _, good := range goodPolicies {
-		assert.NoError(t, ValidatePolicy(good))
+		require.NoError(t, ValidatePolicy(good))
 	}
 	badPolicies := []string{
 		"this, is, not, a, good, policy",
 		"this\ttoo",
 	}
 	for _, bad := range badPolicies {
-		assert.Error(t, ValidatePolicy(bad))
+		require.Error(t, ValidatePolicy(bad))
 	}
 }
 
@@ -360,48 +354,47 @@ func TestEnforceErrorMessage(t *testing.T) {
 	kubeclientset := fake.NewSimpleClientset()
 	enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil)
 	err := enf.syncUpdate(fakeConfigMap(), noOpUpdate)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 
 	err = enf.EnforceErr("admin", "applications", "get", "foo/bar")
-	assert.Error(t, err)
+	require.Error(t, err)
 	assert.Equal(t, "rpc error: code = PermissionDenied desc = permission denied: applications, get, foo/bar", err.Error())
 
 	err = enf.EnforceErr()
-	assert.Error(t, err)
+	require.Error(t, err)
 	assert.Equal(t, "rpc error: code = PermissionDenied desc = permission denied", err.Error())
 
 	// nolint:staticcheck
 	ctx := context.WithValue(context.Background(), "claims", &jwt.RegisteredClaims{Subject: "proj:default:admin"})
 	err = enf.EnforceErr(ctx.Value("claims"), "project")
-	assert.Error(t, err)
+	require.Error(t, err)
 	assert.Equal(t, "rpc error: code = PermissionDenied desc = permission denied: project, sub: proj:default:admin", err.Error())
 
 	iat := time.Unix(int64(1593035962), 0).Format(time.RFC3339)
 	exp := fmt.Sprintf("rpc error: code = PermissionDenied desc = permission denied: project, sub: proj:default:admin, iat: %s", iat)
 	// nolint:staticcheck
 	ctx = context.WithValue(context.Background(), "claims", &jwt.RegisteredClaims{Subject: "proj:default:admin", IssuedAt: jwt.NewNumericDate(time.Unix(int64(1593035962), 0))})
 	err = enf.EnforceErr(ctx.Value("claims"), "project")
-	assert.Error(t, err)
+	require.Error(t, err)
 	assert.Equal(t, exp, err.Error())
 
 	// nolint:staticcheck
 	ctx = context.WithValue(context.Background(), "claims", &jwt.RegisteredClaims{ExpiresAt: jwt.NewNumericDate(time.Now())})
 	err = enf.EnforceErr(ctx.Value("claims"), "project")
-	assert.Error(t, err)
+	require.Error(t, err)
 	assert.Equal(t, "rpc error: code = PermissionDenied desc = permission denied: project", err.Error())
 
 	// nolint:staticcheck
 	ctx = context.WithValue(context.Background(), "claims", &jwt.RegisteredClaims{Subject: "proj:default:admin", IssuedAt: nil})
 	err = enf.EnforceErr(ctx.Value("claims"), "project")
-	assert.Error(t, err)
+	require.Error(t, err)
 	assert.Equal(t, "rpc error: code = PermissionDenied desc = permission denied: project, sub: proj:default:admin", err.Error())
 }
 
 func TestDefaultGlobMatchMode(t *testing.T) {
 	kubeclientset := fake.NewSimpleClientset()
 	enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil)
-	err := enf.syncUpdate(fakeConfigMap(), noOpUpdate)
-	assert.NoError(t, err)
+	require.NoError(t, enf.syncUpdate(fakeConfigMap(), noOpUpdate))
 	policy := `
 p, alice, clusters, get, "https://github.com/*/*.git", allow
 `
@@ -416,8 +409,7 @@ func TestGlobMatchMode(t *testing.T) {
 	cm.Data[ConfigMapMatchModeKey] = GlobMatchMode
 	kubeclientset := fake.NewSimpleClientset()
 	enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil)
-	err := enf.syncUpdate(cm, noOpUpdate)
-	assert.NoError(t, err)
+	require.NoError(t, enf.syncUpdate(cm, noOpUpdate))
 	policy := `
 p, alice, clusters, get, "https://github.com/*/*.git", allow
 `
@@ -432,8 +424,7 @@ func TestRegexMatchMode(t *testing.T) {
 	cm.Data[ConfigMapMatchModeKey] = RegexMatchMode
 	kubeclientset := fake.NewSimpleClientset()
 	enf := NewEnforcer(kubeclientset, fakeNamespace, fakeConfigMapName, nil)
-	err := enf.syncUpdate(cm, noOpUpdate)
-	assert.NoError(t, err)
+	require.NoError(t, enf.syncUpdate(cm, noOpUpdate))
 	policy := `
 p, alice, clusters, get, "https://github.com/argo[a-z]{4}/argo-[a-z]+.git", allow
 `
@@ -461,55 +452,46 @@ func TestLoadPolicyLine(t *testing.T) {
 	t.Run("Valid permission line", func(t *testing.T) {
 		policy := `p, role:Myrole, applications, *, myproj/*, allow`
 		model := newBuiltInModel()
-		err := loadPolicyLine(policy, model)
-		require.NoError(t, err)
+		require.NoError(t, loadPolicyLine(policy, model))
 	})
 	t.Run("Valid grant line", func(t *testing.T) {
 		policy := `g, your-github-org:your-team, role:org-admin`
 		model := newBuiltInModel()
-		err := loadPolicyLine(policy, model)
-		require.NoError(t, err)
+		require.NoError(t, loadPolicyLine(policy, model))
 	})
 	t.Run("Empty policy line", func(t *testing.T) {
 		policy := ""
 		model := newBuiltInModel()
-		err := loadPolicyLine(policy, model)
-		require.NoError(t, err)
+		require.NoError(t, loadPolicyLine(policy, model))
 	})
 	t.Run("Comment policy line", func(t *testing.T) {
 		policy := "# Some comment"
 		model := newBuiltInModel()
-		err := loadPolicyLine(policy, model)
-		require.NoError(t, err)
+		require.NoError(t, loadPolicyLine(policy, model))
 	})
 	t.Run("Invalid policy line: single token", func(t *testing.T) {
 		policy := "p"
 		model := newBuiltInModel()
-		err := loadPolicyLine(policy, model)
-		require.Error(t, err)
+		require.Error(t, loadPolicyLine(policy, model))
 	})
 	t.Run("Invalid policy line: plain text", func(t *testing.T) {
 		policy := "Some comment"
 		model := newBuiltInModel()
-		err := loadPolicyLine(policy, model)
-		require.Error(t, err)
+		require.Error(t, loadPolicyLine(policy, model))
 	})
 	t.Run("Invalid policy line", func(t *testing.T) {
 		policy := "agh, foo, bar"
 		model := newBuiltInModel()
-		err := loadPolicyLine(policy, model)
-		require.Error(t, err)
+		require.Error(t, loadPolicyLine(policy, model))
 	})
 	t.Run("Invalid policy line missing comma", func(t *testing.T) {
 		policy := "p, role:Myrole, applications, *, myproj/* allow"
 		model := newBuiltInModel()
-		err := loadPolicyLine(policy, model)
-		require.Error(t, err)
+		require.Error(t, loadPolicyLine(policy, model))
 	})
 	t.Run("Invalid policy line missing policy type", func(t *testing.T) {
 		policy := ", role:Myrole, applications, *, myproj/*, allow"
 		model := newBuiltInModel()
-		err := loadPolicyLine(policy, model)
-		require.Error(t, err)
+		require.Error(t, loadPolicyLine(policy, model))
 	})
 }

util/security/path_traversal_test.go

@@ -4,23 +4,24 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestEnforceToCurrentRoot(t *testing.T) {
 	cleanDir, err := EnforceToCurrentRoot("/home/argo/helmapp/", "/home/argo/helmapp/values.yaml")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, "/home/argo/helmapp/values.yaml", cleanDir)
 
 	// File is outside current working directory
 	_, err = EnforceToCurrentRoot("/home/argo/helmapp/", "/home/values.yaml")
-	assert.Error(t, err)
+	require.Error(t, err)
 
 	// File is outside current working directory
 	_, err = EnforceToCurrentRoot("/home/argo/helmapp/", "/home/argo/helmapp/../differentapp/values.yaml")
-	assert.Error(t, err)
+	require.Error(t, err)
 
 	// Goes back and forth, but still legal
 	cleanDir, err = EnforceToCurrentRoot("/home/argo/helmapp/", "/home/argo/helmapp/../../argo/helmapp/values.yaml")
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, "/home/argo/helmapp/values.yaml", cleanDir)
 }

util/session/sessionmanager_norace_test.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/argoproj/argo-cd/v2/util/settings"
 )
@@ -30,9 +31,7 @@ func TestRandomPasswordVerificationDelay(t *testing.T) {
 	for i := 0; i < 10; i++ {
 		sleptFor = 0
 		start := time.Now()
-		if !assert.NoError(t, mgr.VerifyUsernamePassword("admin", "password")) {
-			return
-		}
+		require.NoError(t, mgr.VerifyUsernamePassword("admin", "password"))
 		totalDuration := time.Since(start) + sleptFor
 		assert.GreaterOrEqual(t, totalDuration.Nanoseconds(), verificationDelayNoiseMin.Nanoseconds())
 		assert.LessOrEqual(t, totalDuration.Nanoseconds(), verificationDelayNoiseMax.Nanoseconds())