From 7101684ea553d8ee08091e7df96013860e98f29e Mon Sep 17 00:00:00 2001 From: reggie-k Date: Sun, 26 Jan 2025 15:14:33 +0200 Subject: [PATCH] gotestsum locally from dist, containerized from /usr/local/bin Signed-off-by: reggie-k --- cmd/argocd/commands/admin/settings.go | 2 +- cmd/argocd/commands/admin/settings_rbac.go | 38 +------ .../commands/admin/settings_rbac_test.go | 104 +++++------------- docs/operator-manual/argocd-cm.yaml | 6 - server/account/account.go | 15 --- server/account/account_test.go | 29 +---- server/application/application.go | 15 +-- test/e2e/accounts_test.go | 48 +------- test/e2e/app_management_ns_test.go | 66 +---------- test/e2e/app_management_test.go | 62 +---------- util/settings/settings.go | 16 --- util/settings/settings_test.go | 16 --- 12 files changed, 45 insertions(+), 372 deletions(-) diff --git a/cmd/argocd/commands/admin/settings.go b/cmd/argocd/commands/admin/settings.go index 4776b5963339a..aeb41d4fc94b3 100644 --- a/cmd/argocd/commands/admin/settings.go +++ b/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") diff --git a/cmd/argocd/commands/admin/settings_rbac.go b/cmd/argocd/commands/admin/settings_rbac.go index f397edef1b1e6..ed12dd1b01102 100644 --- a/cmd/argocd/commands/admin/settings_rbac.go +++ b/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,7 +118,7 @@ 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", @@ -127,13 +126,13 @@ func NewRBACCommand(cmdCtx commandContext) *cobra.Command { 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) } diff --git a/cmd/argocd/commands/admin/settings_rbac_test.go b/cmd/argocd/commands/admin/settings_rbac_test.go index f0ec3b848ea6a..64b8f7c2726b8 100644 --- a/cmd/argocd/commands/admin/settings_rbac_test.go +++ b/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()) diff --git a/docs/operator-manual/argocd-cm.yaml b/docs/operator-manual/argocd-cm.yaml index 68b4c0c7302b9..9f8cd094350c7 100644 --- a/docs/operator-manual/argocd-cm.yaml +++ b/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" diff --git a/server/account/account.go b/server/account/account.go index 06dceffc4ba3e..117c0569f1ca7 100644 --- a/server/account/account.go +++ b/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 diff --git a/server/account/account_test.go b/server/account/account_test.go index a228e638f7410..50d098dde1b0a 100644 --- a/server/account/account_test.go +++ b/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,13 +318,12 @@ 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()) @@ -332,25 +331,3 @@ func TestCanI_GetLogsDenySwitchOn(t *testing.T) { 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) -} diff --git a/server/application/application.go b/server/application/application.go index c6568187edf1a..73b1183c64cf8 100644 --- a/server/application/application.go +++ b/server/application/application.go @@ -1717,19 +1717,8 @@ func (s *Server) PodLogs(q *application.ApplicationPodLogsQuery, ws application. return err } - // 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, PodLogs will return the logs, - // even if there is no explicit RBAC allow, or if there is an explicit RBAC deny) - serverRBACLogEnforceEnable, err := s.settingsMgr.GetServerRBACLogEnforceEnable() - if err != nil { - return fmt.Errorf("error getting RBAC log enforce enable: %w", err) - } - - if serverRBACLogEnforceEnable { - if err := s.enf.EnforceErr(ws.Context().Value("claims"), rbacpolicy.ResourceLogs, rbacpolicy.ActionGet, a.RBACName(s.ns)); err != nil { - return err - } + if err := s.enf.EnforceErr(ws.Context().Value("claims"), rbacpolicy.ResourceLogs, rbacpolicy.ActionGet, a.RBACName(s.ns)); err != nil { + return err } tree, err := s.getAppResources(ws.Context(), a) diff --git a/test/e2e/accounts_test.go b/test/e2e/accounts_test.go index be9ca7ca5dda4..b2e6dfabd49d3 100644 --- a/test/e2e/accounts_test.go +++ b/test/e2e/accounts_test.go @@ -39,7 +39,7 @@ func TestCreateAndUseAccount(t *testing.T) { }) } -func TestCanIGetLogsAllowNoSwitch(t *testing.T) { +func TestCanIGetLogsAllow(t *testing.T) { ctx := accountFixture.Given(t) ctx. Name("test"). @@ -53,14 +53,13 @@ func TestCanIGetLogsAllowNoSwitch(t *testing.T) { }) } -func TestCanIGetLogsDenySwitchOn(t *testing.T) { +func TestCanIGetLogsDeny(t *testing.T) { ctx := accountFixture.Given(t) ctx. Name("test"). When(). Create(). Login(). - SetParamInSettingConfigMap("server.rbac.log.enforce.enable", "true"). CanIGetLogs(). Then(). AndCLIOutput(func(output string, _ error) { @@ -68,49 +67,6 @@ func TestCanIGetLogsDenySwitchOn(t *testing.T) { }) } -func TestCanIGetLogsAllowSwitchOn(t *testing.T) { - ctx := accountFixture.Given(t) - ctx. - Name("test"). - Project(ProjectName). - When(). - Create(). - Login(). - SetPermissions([]ACL{ - { - Resource: "logs", - Action: "get", - Scope: ProjectName + "/*", - }, - { - Resource: "apps", - Action: "get", - Scope: ProjectName + "/*", - }, - }, "log-viewer"). - SetParamInSettingConfigMap("server.rbac.log.enforce.enable", "true"). - CanIGetLogs(). - Then(). - AndCLIOutput(func(output string, _ error) { - assert.Contains(t, output, "yes") - }) -} - -func TestCanIGetLogsAllowSwitchOff(t *testing.T) { - ctx := accountFixture.Given(t) - ctx. - Name("test"). - When(). - Create(). - Login(). - SetParamInSettingConfigMap("server.rbac.log.enforce.enable", "false"). - CanIGetLogs(). - Then(). - AndCLIOutput(func(output string, _ error) { - assert.Contains(t, output, "yes") - }) -} - func TestCreateAndUseAccountCLI(t *testing.T) { EnsureCleanState(t) diff --git a/test/e2e/app_management_ns_test.go b/test/e2e/app_management_ns_test.go index 07ab6416f1e69..dd429fef45163 100644 --- a/test/e2e/app_management_ns_test.go +++ b/test/e2e/app_management_ns_test.go @@ -43,10 +43,10 @@ import ( ) // This empty test is here only for clarity, to conform to logs rbac tests structure in account. This exact usecase is covered in the TestAppLogs test -func TestNamespacedGetLogsAllowNoSwitch(_ *testing.T) { +func TestNamespacedGetLogsAllow(_ *testing.T) { } -func TestNamespacedGetLogsDenySwitchOn(t *testing.T) { +func TestNamespacedGetLogsDeny(t *testing.T) { fixture.SkipOnEnv(t, "OPENSHIFT") accountFixture.Given(t). @@ -85,7 +85,6 @@ func TestNamespacedGetLogsDenySwitchOn(t *testing.T) { When(). CreateApp(). Sync(). - SetParamInSettingConfigMap("server.rbac.log.enforce.enable", "true"). Then(). Expect(HealthIs(health.HealthStatusHealthy)). And(func(_ *Application) { @@ -94,7 +93,7 @@ func TestNamespacedGetLogsDenySwitchOn(t *testing.T) { }) } -func TestNamespacedGetLogsAllowSwitchOnNS(t *testing.T) { +func TestNamespacedGetLogsAllowNS(t *testing.T) { fixture.SkipOnEnv(t, "OPENSHIFT") accountFixture.Given(t). @@ -138,65 +137,6 @@ func TestNamespacedGetLogsAllowSwitchOnNS(t *testing.T) { When(). CreateApp(). Sync(). - SetParamInSettingConfigMap("server.rbac.log.enforce.enable", "true"). - Then(). - Expect(HealthIs(health.HealthStatusHealthy)). - And(func(_ *Application) { - out, err := fixture.RunCliWithRetry(5, "app", "logs", ctx.AppQualifiedName(), "--kind", "Deployment", "--group", "", "--name", "guestbook-ui") - require.NoError(t, err) - assert.Contains(t, out, "Hi") - }). - And(func(_ *Application) { - out, err := fixture.RunCliWithRetry(5, "app", "logs", ctx.AppQualifiedName(), "--kind", "Pod") - require.NoError(t, err) - assert.Contains(t, out, "Hi") - }). - And(func(_ *Application) { - out, err := fixture.RunCliWithRetry(5, "app", "logs", ctx.AppQualifiedName(), "--kind", "Service") - require.NoError(t, err) - assert.NotContains(t, out, "Hi") - }) -} - -func TestNamespacedGetLogsAllowSwitchOff(t *testing.T) { - fixture.SkipOnEnv(t, "OPENSHIFT") - - accountFixture.Given(t). - Name("test"). - When(). - Create(). - Login(). - SetPermissions([]fixture.ACL{ - { - Resource: "applications", - Action: "create", - Scope: "*", - }, - { - Resource: "applications", - Action: "get", - Scope: "*", - }, - { - Resource: "applications", - Action: "sync", - Scope: "*", - }, - { - Resource: "projects", - Action: "get", - Scope: "*", - }, - }, "app-creator") - ctx := GivenWithSameState(t) - ctx.SetAppNamespace(fixture.AppNamespace()) - ctx. - Path("guestbook-logs"). - SetTrackingMethod("annotation"). - When(). - CreateApp(). - Sync(). - SetParamInSettingConfigMap("server.rbac.log.enforce.enable", "false"). Then(). Expect(HealthIs(health.HealthStatusHealthy)). And(func(_ *Application) { diff --git a/test/e2e/app_management_test.go b/test/e2e/app_management_test.go index c6b753d916fad..ecd0fb8d981e8 100644 --- a/test/e2e/app_management_test.go +++ b/test/e2e/app_management_test.go @@ -57,7 +57,7 @@ const ( func TestGetLogsAllowNoSwitch(_ *testing.T) { } -func TestGetLogsDenySwitchOn(t *testing.T) { +func TestGetLogsDeny(t *testing.T) { fixture.SkipOnEnv(t, "OPENSHIFT") accountFixture.Given(t). @@ -93,7 +93,6 @@ func TestGetLogsDenySwitchOn(t *testing.T) { When(). CreateApp(). Sync(). - SetParamInSettingConfigMap("server.rbac.log.enforce.enable", "true"). Then(). Expect(HealthIs(health.HealthStatusHealthy)). And(func(app *Application) { @@ -102,7 +101,7 @@ func TestGetLogsDenySwitchOn(t *testing.T) { }) } -func TestGetLogsAllowSwitchOn(t *testing.T) { +func TestGetLogsAllow(t *testing.T) { fixture.SkipOnEnv(t, "OPENSHIFT") accountFixture.Given(t). @@ -143,63 +142,6 @@ func TestGetLogsAllowSwitchOn(t *testing.T) { When(). CreateApp(). Sync(). - SetParamInSettingConfigMap("server.rbac.log.enforce.enable", "true"). - Then(). - Expect(HealthIs(health.HealthStatusHealthy)). - And(func(app *Application) { - out, err := fixture.RunCliWithRetry(appLogsRetryCount, "app", "logs", app.Name, "--kind", "Deployment", "--group", "", "--name", "guestbook-ui") - require.NoError(t, err) - assert.Contains(t, out, "Hi") - }). - And(func(app *Application) { - out, err := fixture.RunCliWithRetry(appLogsRetryCount, "app", "logs", app.Name, "--kind", "Pod") - require.NoError(t, err) - assert.Contains(t, out, "Hi") - }). - And(func(app *Application) { - out, err := fixture.RunCliWithRetry(appLogsRetryCount, "app", "logs", app.Name, "--kind", "Service") - require.NoError(t, err) - assert.NotContains(t, out, "Hi") - }) -} - -func TestGetLogsAllowSwitchOff(t *testing.T) { - fixture.SkipOnEnv(t, "OPENSHIFT") - - accountFixture.Given(t). - Name("test"). - When(). - Create(). - Login(). - SetPermissions([]fixture.ACL{ - { - Resource: "applications", - Action: "create", - Scope: "*", - }, - { - Resource: "applications", - Action: "get", - Scope: "*", - }, - { - Resource: "applications", - Action: "sync", - Scope: "*", - }, - { - Resource: "projects", - Action: "get", - Scope: "*", - }, - }, "app-creator") - - GivenWithSameState(t). - Path("guestbook-logs"). - When(). - CreateApp(). - Sync(). - SetParamInSettingConfigMap("server.rbac.log.enforce.enable", "false"). Then(). Expect(HealthIs(health.HealthStatusHealthy)). And(func(app *Application) { diff --git a/util/settings/settings.go b/util/settings/settings.go index c6dca2bbe12eb..f7cf02cecb3e1 100644 --- a/util/settings/settings.go +++ b/util/settings/settings.go @@ -513,8 +513,6 @@ const ( settingsPasswordPatternKey = "passwordPattern" // inClusterEnabledKey is the key to configure whether to allow in-cluster server address inClusterEnabledKey = "cluster.inClusterEnabled" - // settingsServerRBACLogEnforceEnable is the key to configure whether logs RBAC enforcement is enabled - settingsServerRBACLogEnforceEnableKey = "server.rbac.log.enforce.enable" // settingsServerRBACEDisableFineGrainedInheritance is the key to configure find-grained RBAC inheritance settingsServerRBACDisableFineGrainedInheritance = "server.rbac.disableApplicationFineGrainedRBACInheritance" // MaxPodLogsToRender the maximum number of pod logs to render @@ -852,19 +850,6 @@ func (mgr *SettingsManager) GetPasswordPattern() (string, error) { return label, nil } -func (mgr *SettingsManager) GetServerRBACLogEnforceEnable() (bool, error) { - argoCDCM, err := mgr.getConfigMap() - if err != nil { - return false, err - } - - if argoCDCM.Data[settingsServerRBACLogEnforceEnableKey] == "" { - return false, nil - } - - return strconv.ParseBool(argoCDCM.Data[settingsServerRBACLogEnforceEnableKey]) -} - func (mgr *SettingsManager) ApplicationFineGrainedRBACInheritanceDisabled() (bool, error) { argoCDCM, err := mgr.getConfigMap() if err != nil { @@ -1529,7 +1514,6 @@ func updateSettingsFromConfigMap(settings *ArgoCDSettings, argoCDCM *corev1.Conf settings.UiBannerContent = argoCDCM.Data[settingUiBannerContentKey] settings.UiBannerPermanent = argoCDCM.Data[settingUiBannerPermanentKey] == "true" settings.UiBannerPosition = argoCDCM.Data[settingUiBannerPositionKey] - settings.ServerRBACLogEnforceEnable = argoCDCM.Data[settingsServerRBACLogEnforceEnableKey] == "true" settings.BinaryUrls = getDownloadBinaryUrlsFromConfigMap(argoCDCM) if err := validateExternalURL(argoCDCM.Data[settingURLKey]); err != nil { log.Warnf("Failed to validate URL in configmap: %v", err) diff --git a/util/settings/settings_test.go b/util/settings/settings_test.go index b85396e4eb65d..c18b827446cc3 100644 --- a/util/settings/settings_test.go +++ b/util/settings/settings_test.go @@ -273,22 +273,6 @@ func TestGetAppInstanceLabelKey(t *testing.T) { assert.Equal(t, "testLabel", label) } -func TestGetServerRBACLogEnforceEnableKeyDefaultFalse(t *testing.T) { - _, settingsManager := fixtures(nil) - serverRBACLogEnforceEnable, err := settingsManager.GetServerRBACLogEnforceEnable() - require.NoError(t, err) - assert.False(t, serverRBACLogEnforceEnable) -} - -func TestGetServerRBACLogEnforceEnableKey(t *testing.T) { - _, settingsManager := fixtures(map[string]string{ - "server.rbac.log.enforce.enable": "true", - }) - serverRBACLogEnforceEnable, err := settingsManager.GetServerRBACLogEnforceEnable() - require.NoError(t, err) - assert.True(t, serverRBACLogEnforceEnable) -} - func TestApplicationFineGrainedRBACInheritanceDisabledDefault(t *testing.T) { _, settingsManager := fixtures(nil) flag, err := settingsManager.ApplicationFineGrainedRBACInheritanceDisabled()