gotestsum locally from dist, containerized from /usr/local/bin

Signed-off-by: reggie-k <[email protected]>

cmd/argocd/commands/admin/settings.go

@@ -162,7 +162,7 @@ func NewSettingsCommand() *cobra.Command {
 
 	command.AddCommand(NewValidateSettingsCommand(&opts))
 	command.AddCommand(NewResourceOverridesCommand(&opts))
-	command.AddCommand(NewRBACCommand(&opts))
+	command.AddCommand(NewRBACCommand())
 
 	opts.clientConfig = cli.AddKubectlFlagsToCmd(command)
 	command.PersistentFlags().StringVar(&opts.argocdCMPath, "argocd-cm-path", "", "Path to local argocd-cm.yaml file")

cmd/argocd/commands/admin/settings_rbac.go

@@ -18,7 +18,6 @@ import (
 	"github.com/argoproj/argo-cd/v3/server/rbacpolicy"
 	"github.com/argoproj/argo-cd/v3/util/assets"
 	"github.com/argoproj/argo-cd/v3/util/cli"
-	"github.com/argoproj/argo-cd/v3/util/errors"
 	"github.com/argoproj/argo-cd/v3/util/rbac"
 )
 
@@ -119,21 +118,21 @@ var extensionActions = actionTraitMap{
 }
 
 // NewRBACCommand is the command for 'rbac'
-func NewRBACCommand(cmdCtx commandContext) *cobra.Command {
+func NewRBACCommand() *cobra.Command {
 	command := &cobra.Command{
 		Use:   "rbac",
 		Short: "Validate and test RBAC configuration",
 		Run: func(c *cobra.Command, args []string) {
 			c.HelpFunc()(c, args)
 		},
 	}
-	command.AddCommand(NewRBACCanCommand(cmdCtx))
+	command.AddCommand(NewRBACCanCommand())
 	command.AddCommand(NewRBACValidateCommand())
 	return command
 }
 
 // NewRBACCanCommand is the command for 'rbac can'
-func NewRBACCanCommand(cmdCtx commandContext) *cobra.Command {
+func NewRBACCanCommand() *cobra.Command {
 	var (
 		policyFile   string
 		defaultRole  string
@@ -219,29 +218,7 @@ argocd admin settings rbac can someuser create application 'default/app' --defau
 				defaultRole = newDefaultRole
 			}
 
-			// Logs RBAC will be enforced only if an internal var serverRBACLogEnforceEnable
-			// (representing server.rbac.log.enforce.enable env var in argocd-cm)
-			// is defined and has a "true" value
-			// Otherwise, no RBAC enforcement for logs will take place (meaning, 'can' request on a logs resource will result in "yes",
-			// even if there is no explicit RBAC allow, or if there is an explicit RBAC deny)
-			var isLogRbacEnforced func() bool
-			if nsOverride && policyFile == "" {
-				if resolveRBACResourceName(resource) == rbacpolicy.ResourceLogs {
-					isLogRbacEnforced = func() bool {
-						if opts, ok := cmdCtx.(*settingsOpts); ok {
-							opts.loadClusterSettings = true
-							opts.clientConfig = clientConfig
-							settingsMgr, err := opts.createSettingsManager(ctx)
-							errors.CheckError(err)
-							logEnforceEnable, err := settingsMgr.GetServerRBACLogEnforceEnable()
-							errors.CheckError(err)
-							return logEnforceEnable
-						}
-						return false
-					}
-				}
-			}
-			res := checkPolicy(subject, action, resource, subResource, builtinPolicy, userPolicy, defaultRole, matchMode, strict, isLogRbacEnforced)
+			res := checkPolicy(subject, action, resource, subResource, builtinPolicy, userPolicy, defaultRole, matchMode, strict)
 
 			if res {
 				if !quiet {
@@ -408,7 +385,7 @@ func getPolicyConfigMap(ctx context.Context, client kubernetes.Interface, namesp
 
 // checkPolicy checks whether given subject is allowed to execute specified
 // action against specified resource
-func checkPolicy(subject, action, resource, subResource, builtinPolicy, userPolicy, defaultRole, matchMode string, strict bool, isLogRbacEnforced func() bool) bool {
+func checkPolicy(subject, action, resource, subResource, builtinPolicy, userPolicy, defaultRole, matchMode string, strict bool) bool {
 	enf := rbac.NewEnforcer(nil, "argocd", "argocd-rbac-cm", nil)
 	enf.SetDefaultRole(defaultRole)
 	enf.SetMatchMode(matchMode)
@@ -450,11 +427,6 @@ func checkPolicy(subject, action, resource, subResource, builtinPolicy, userPoli
 			subResource = "*/*"
 		}
 	}
-	if realResource == rbacpolicy.ResourceLogs {
-		if isLogRbacEnforced != nil && !isLogRbacEnforced() {
-			return true
-		}
-	}
 	return enf.Enforce(subject, realResource, action, subResource)
 }
 

cmd/argocd/commands/admin/settings_rbac_test.go

@@ -131,15 +131,7 @@ func Test_PolicyFromYAML(t *testing.T) {
 	require.Equal(t, "role:unknown", dRole)
 	require.Empty(t, matchMode)
 	require.True(t, checkPolicy("my-org:team-qa", "update", "project", "foo",
-		"", uPol, dRole, matchMode, true, nil))
-}
-
-func trueLogRbacEnforce() bool {
-	return true
-}
-
-func falseLogRbacEnforce() bool {
-	return false
+		"", uPol, dRole, matchMode, true))
 }
 
 func Test_PolicyFromK8s(t *testing.T) {
@@ -163,105 +155,63 @@ func Test_PolicyFromK8s(t *testing.T) {
 	require.Equal(t, "", matchMode)
 
 	t.Run("get applications", func(t *testing.T) {
-		ok := checkPolicy("role:user", "get", "applications", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, nil)
+		ok := checkPolicy("role:user", "get", "applications", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true)
 		require.True(t, ok)
 	})
 	t.Run("get clusters", func(t *testing.T) {
-		ok := checkPolicy("role:user", "get", "clusters", "*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, nil)
+		ok := checkPolicy("role:user", "get", "clusters", "*", assets.BuiltinPolicyCSV, uPol, dRole, "", true)
 		require.True(t, ok)
 	})
 	t.Run("get certificates", func(t *testing.T) {
-		ok := checkPolicy("role:user", "get", "certificates", "*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, nil)
+		ok := checkPolicy("role:user", "get", "certificates", "*", assets.BuiltinPolicyCSV, uPol, dRole, "", true)
 		require.False(t, ok)
 	})
 	t.Run("get certificates by default role", func(t *testing.T) {
-		ok := checkPolicy("role:user", "get", "certificates", "*", assets.BuiltinPolicyCSV, uPol, "role:readonly", "glob", true, nil)
+		ok := checkPolicy("role:user", "get", "certificates", "*", assets.BuiltinPolicyCSV, uPol, "role:readonly", "glob", true)
 		require.True(t, ok)
 	})
 	t.Run("get certificates by default role without builtin policy", func(t *testing.T) {
-		ok := checkPolicy("role:user", "get", "certificates", "*", "", uPol, "role:readonly", "glob", true, nil)
+		ok := checkPolicy("role:user", "get", "certificates", "*", "", uPol, "role:readonly", "glob", true)
 		require.False(t, ok)
 	})
 	t.Run("use regex match mode instead of glob", func(t *testing.T) {
-		ok := checkPolicy("role:user", "get", "certificates", ".*", assets.BuiltinPolicyCSV, uPol, "role:readonly", "regex", true, nil)
+		ok := checkPolicy("role:user", "get", "certificates", ".*", assets.BuiltinPolicyCSV, uPol, "role:readonly", "regex", true)
 		require.False(t, ok)
 	})
 	t.Run("get logs", func(t *testing.T) {
-		ok := checkPolicy("role:test", "get", "logs", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, nil)
+		ok := checkPolicy("role:test", "get", "logs", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true)
 		require.True(t, ok)
 	})
-	// no function is provided to check if logs rbac is enforced or not, so the policy permissions are queried to determine if no-such-user can get logs
 	t.Run("no-such-user get logs", func(t *testing.T) {
-		ok := checkPolicy("no-such-user", "get", "logs", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, nil)
-		require.False(t, ok)
-	})
-	// logs rbac policy is enforced, and no-such-user is not granted logs permission in user policy, so the result should be false (cannot get logs)
-	t.Run("no-such-user get logs rbac enforced", func(t *testing.T) {
-		ok := checkPolicy("no-such-user", "get", "logs", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, trueLogRbacEnforce)
+		ok := checkPolicy("no-such-user", "get", "logs", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true)
 		require.False(t, ok)
 	})
-	// no-such-user is not granted logs permission in user policy, but logs rbac policy is not enforced, so logs permission is open to all
-	t.Run("no-such-user get logs rbac not enforced", func(t *testing.T) {
-		ok := checkPolicy("no-such-user", "get", "logs", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, falseLogRbacEnforce)
-		require.True(t, ok)
-	})
-	// no function is provided to check if logs rbac is enforced or not, so the policy permissions are queried to determine if log-deny-user can get logs
 	t.Run("log-deny-user get logs", func(t *testing.T) {
-		ok := checkPolicy("log-deny-user", "get", "logs", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, nil)
-		require.False(t, ok)
-	})
-	// logs rbac policy is enforced, and log-deny-user is denied logs permission in user policy, so the result should be false (cannot get logs)
-	t.Run("log-deny-user get logs rbac enforced", func(t *testing.T) {
-		ok := checkPolicy("log-deny-user", "get", "logs", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, trueLogRbacEnforce)
+		ok := checkPolicy("log-deny-user", "get", "logs", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true)
 		require.False(t, ok)
 	})
-	// log-deny-user is denied logs permission in user policy, but logs rbac policy is not enforced, so logs permission is open to all
-	t.Run("log-deny-user get logs rbac not enforced", func(t *testing.T) {
-		ok := checkPolicy("log-deny-user", "get", "logs", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, falseLogRbacEnforce)
-		require.True(t, ok)
-	})
-	// no function is provided to check if logs rbac is enforced or not, so the policy permissions are queried to determine if log-allow-user can get logs
 	t.Run("log-allow-user get logs", func(t *testing.T) {
-		ok := checkPolicy("log-allow-user", "get", "logs", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, nil)
-		require.True(t, ok)
-	})
-	// logs rbac policy is enforced, and log-allow-user is granted logs permission in user policy, so the result should be true (can get logs)
-	t.Run("log-allow-user get logs rbac enforced", func(t *testing.T) {
-		ok := checkPolicy("log-allow-user", "get", "logs", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, trueLogRbacEnforce)
-		require.True(t, ok)
-	})
-	// log-allow-user is granted logs permission in user policy, and logs rbac policy is not enforced, so logs permission is open to all
-	t.Run("log-allow-user get logs rbac not enforced", func(t *testing.T) {
-		ok := checkPolicy("log-allow-user", "get", "logs", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, falseLogRbacEnforce)
+		ok := checkPolicy("log-allow-user", "get", "logs", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true)
 		require.True(t, ok)
 	})
 	t.Run("get logs", func(t *testing.T) {
-		ok := checkPolicy("role:test", "get", "logs", "*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, nil)
+		ok := checkPolicy("role:test", "get", "logs", "*", assets.BuiltinPolicyCSV, uPol, dRole, "", true)
 		require.True(t, ok)
 	})
 	t.Run("get logs", func(t *testing.T) {
-		ok := checkPolicy("role:test", "get", "logs", "", assets.BuiltinPolicyCSV, uPol, dRole, "", true, nil)
+		ok := checkPolicy("role:test", "get", "logs", "", assets.BuiltinPolicyCSV, uPol, dRole, "", true)
 		require.True(t, ok)
 	})
 	t.Run("create exec", func(t *testing.T) {
-		ok := checkPolicy("role:test", "create", "exec", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, nil)
+		ok := checkPolicy("role:test", "create", "exec", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true)
 		require.True(t, ok)
 	})
 	t.Run("create applicationsets", func(t *testing.T) {
-		ok := checkPolicy("role:user", "create", "applicationsets", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, nil)
-		require.True(t, ok)
-	})
-	// trueLogRbacEnforce or falseLogRbacEnforce should not affect non-logs resources
-	t.Run("create applicationsets with trueLogRbacEnforce", func(t *testing.T) {
-		ok := checkPolicy("role:user", "create", "applicationsets", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, trueLogRbacEnforce)
-		require.True(t, ok)
-	})
-	t.Run("create applicationsets with falseLogRbacEnforce", func(t *testing.T) {
-		ok := checkPolicy("role:user", "create", "applicationsets", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, trueLogRbacEnforce)
+		ok := checkPolicy("role:user", "create", "applicationsets", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true)
 		require.True(t, ok)
 	})
 	t.Run("delete applicationsets", func(t *testing.T) {
-		ok := checkPolicy("role:user", "delete", "applicationsets", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true, nil)
+		ok := checkPolicy("role:user", "delete", "applicationsets", "*/*", assets.BuiltinPolicyCSV, uPol, dRole, "", true)
 		require.True(t, ok)
 	})
 }
@@ -301,49 +251,49 @@ p, role:readonly, certificates, get, .*, allow
 p, role:, certificates, get, .*, allow`
 
 	t.Run("get applications", func(t *testing.T) {
-		ok := checkPolicy("role:user", "get", "applications", ".*/.*", builtInPolicy, uPol, dRole, "regex", true, nil)
+		ok := checkPolicy("role:user", "get", "applications", ".*/.*", builtInPolicy, uPol, dRole, "regex", true)
 		require.True(t, ok)
 	})
 	t.Run("get clusters", func(t *testing.T) {
-		ok := checkPolicy("role:user", "get", "clusters", ".*", builtInPolicy, uPol, dRole, "regex", true, nil)
+		ok := checkPolicy("role:user", "get", "clusters", ".*", builtInPolicy, uPol, dRole, "regex", true)
 		require.True(t, ok)
 	})
 	t.Run("get certificates", func(t *testing.T) {
-		ok := checkPolicy("role:user", "get", "certificates", ".*", builtInPolicy, uPol, dRole, "regex", true, nil)
+		ok := checkPolicy("role:user", "get", "certificates", ".*", builtInPolicy, uPol, dRole, "regex", true)
 		require.False(t, ok)
 	})
 	t.Run("get certificates by default role", func(t *testing.T) {
-		ok := checkPolicy("role:user", "get", "certificates", ".*", builtInPolicy, uPol, "role:readonly", "regex", true, nil)
+		ok := checkPolicy("role:user", "get", "certificates", ".*", builtInPolicy, uPol, "role:readonly", "regex", true)
 		require.True(t, ok)
 	})
 	t.Run("get certificates by default role without builtin policy", func(t *testing.T) {
-		ok := checkPolicy("role:user", "get", "certificates", ".*", "", uPol, "role:readonly", "regex", true, nil)
+		ok := checkPolicy("role:user", "get", "certificates", ".*", "", uPol, "role:readonly", "regex", true)
 		require.False(t, ok)
 	})
 	t.Run("use glob match mode instead of regex", func(t *testing.T) {
-		ok := checkPolicy("role:user", "get", "certificates", ".+", builtInPolicy, uPol, dRole, "glob", true, nil)
+		ok := checkPolicy("role:user", "get", "certificates", ".+", builtInPolicy, uPol, dRole, "glob", true)
 		require.False(t, ok)
 	})
 	t.Run("get logs via glob match mode", func(t *testing.T) {
-		ok := checkPolicy("role:user", "get", "logs", ".*/.*", builtInPolicy, uPol, dRole, "glob", true, nil)
+		ok := checkPolicy("role:user", "get", "logs", ".*/.*", builtInPolicy, uPol, dRole, "glob", true)
 		require.True(t, ok)
 	})
 	t.Run("create exec", func(t *testing.T) {
-		ok := checkPolicy("role:user", "create", "exec", ".*/.*", builtInPolicy, uPol, dRole, "regex", true, nil)
+		ok := checkPolicy("role:user", "create", "exec", ".*/.*", builtInPolicy, uPol, dRole, "regex", true)
 		require.True(t, ok)
 	})
 	t.Run("create applicationsets", func(t *testing.T) {
-		ok := checkPolicy("role:user", "create", "applicationsets", ".*/.*", builtInPolicy, uPol, dRole, "regex", true, nil)
+		ok := checkPolicy("role:user", "create", "applicationsets", ".*/.*", builtInPolicy, uPol, dRole, "regex", true)
 		require.True(t, ok)
 	})
 	t.Run("delete applicationsets", func(t *testing.T) {
-		ok := checkPolicy("role:user", "delete", "applicationsets", ".*/.*", builtInPolicy, uPol, dRole, "regex", true, nil)
+		ok := checkPolicy("role:user", "delete", "applicationsets", ".*/.*", builtInPolicy, uPol, dRole, "regex", true)
 		require.True(t, ok)
 	})
 }
 
 func TestNewRBACCanCommand(t *testing.T) {
-	command := NewRBACCanCommand(&settingsOpts{})
+	command := NewRBACCanCommand()
 
 	require.NotNil(t, command)
 	assert.Equal(t, "can", command.Name())

docs/operator-manual/argocd-cm.yaml

@@ -345,12 +345,6 @@ data:
   # This is to prevent the UI from becoming unresponsive when rendering a large number of logs. Default is 10.
   server.maxPodLogsToRender: "10"
 
-  # Application pod logs RBAC enforcement enables control over who can and who can't view application pod logs.
-  # When you enable the switch, pod logs will be visible only to admin role by default. Other roles/users will not be able to view them via cli and UI.
-  # When you enable the switch, viewing pod logs for other roles/users will require explicit RBAC allow policies (allow get on logs subresource).
-  # When you disable the switch (either add it to the configmap with a "false" value or do not add it to the configmap), no actual RBAC enforcement will take place.
-  server.rbac.log.enforce.enable: "false"
-
   # exec.enabled indicates whether the UI exec feature is enabled. It is disabled by default.
   exec.enabled: "false"
 

server/account/account.go

@@ -125,21 +125,6 @@ func (s *Server) CanI(ctx context.Context, r *account.CanIRequest) (*account.Can
 		return nil, status.Errorf(codes.InvalidArgument, "%v does not contain %s", rbacpolicy.Resources, r.Resource)
 	}
 
-	// Logs RBAC will be enforced only if an internal var serverRBACLogEnforceEnable (representing server.rbac.log.enforce.enable env var)
-	// is defined and has a "true" value
-	// Otherwise, no RBAC enforcement for logs will take place (meaning, can-i request on a logs resource will result in "yes",
-	// even if there is no explicit RBAC allow, or if there is an explicit RBAC deny)
-	if r.Resource == "logs" {
-		serverRBACLogEnforceEnable, err := s.settingsMgr.GetServerRBACLogEnforceEnable()
-		if err != nil {
-			return nil, fmt.Errorf("failed to get server RBAC log enforcement setting: %w", err)
-		}
-
-		if !serverRBACLogEnforceEnable {
-			return &account.CanIResponse{Value: "yes"}, nil
-		}
-	}
-
 	ok := s.enf.Enforce(ctx.Value("claims"), r.Resource, r.Action, r.Subresource)
 	if ok {
 		return &account.CanIResponse{Value: "yes"}, nil

server/account/account_test.go

@@ -308,7 +308,7 @@ func TestDeleteToken_SuccessfullyRemoved(t *testing.T) {
 	assert.Empty(t, acc.Tokens)
 }
 
-func TestCanI_GetLogsAllowNoSwitch(t *testing.T) {
+func TestCanI_GetLogsAllow(t *testing.T) {
 	accountServer, _ := newTestAccountServer(context.Background(), func(_ *corev1.ConfigMap, _ *corev1.Secret) {
 	})
 
@@ -318,39 +318,16 @@ func TestCanI_GetLogsAllowNoSwitch(t *testing.T) {
 	assert.EqualValues(t, "yes", resp.Value)
 }
 
-func TestCanI_GetLogsDenySwitchOn(t *testing.T) {
+func TestCanI_GetLogsDeny(t *testing.T) {
 	enforcer := func(_ jwt.Claims, _ ...any) bool {
 		return false
 	}
 
-	accountServer, _ := newTestAccountServerExt(context.Background(), enforcer, func(cm *corev1.ConfigMap, _ *corev1.Secret) {
-		cm.Data["server.rbac.log.enforce.enable"] = "true"
+	accountServer, _ := newTestAccountServerExt(context.Background(), enforcer, func(_ *corev1.ConfigMap, _ *corev1.Secret) {
 	})
 
 	ctx := projTokenContext(context.Background())
 	resp, err := accountServer.CanI(ctx, &account.CanIRequest{Resource: "logs", Action: "get", Subresource: "*/*"})
 	require.NoError(t, err)
 	assert.EqualValues(t, "no", resp.Value)
 }
-
-func TestCanI_GetLogsAllowSwitchOn(t *testing.T) {
-	accountServer, _ := newTestAccountServer(context.Background(), func(cm *corev1.ConfigMap, _ *corev1.Secret) {
-		cm.Data["server.rbac.log.enforce.enable"] = "true"
-	})
-
-	ctx := projTokenContext(context.Background())
-	resp, err := accountServer.CanI(ctx, &account.CanIRequest{Resource: "logs", Action: "get", Subresource: ""})
-	require.NoError(t, err)
-	assert.EqualValues(t, "yes", resp.Value)
-}
-
-func TestCanI_GetLogsAllowSwitchOff(t *testing.T) {
-	accountServer, _ := newTestAccountServer(context.Background(), func(cm *corev1.ConfigMap, _ *corev1.Secret) {
-		cm.Data["server.rbac.log.enforce.enable"] = "false"
-	})
-
-	ctx := projTokenContext(context.Background())
-	resp, err := accountServer.CanI(ctx, &account.CanIRequest{Resource: "logs", Action: "get", Subresource: ""})
-	require.NoError(t, err)
-	assert.EqualValues(t, "yes", resp.Value)
-}