From 12879d97181d532d1678a36e1c71104085ca97dd Mon Sep 17 00:00:00 2001 From: Jonathan Crowther Date: Tue, 26 Nov 2024 11:06:59 -0500 Subject: [PATCH] [0.7] Remove references to Restricted Admin (#549) * Remove references to Restricted Admin * Forgot to save one of the changes --- pkg/auth/globalrole.go | 36 +--- pkg/auth/globalrole_test.go | 15 -- .../v3/globalrole/setup_test.go | 32 +--- .../v3/globalrole/validator_test.go | 181 ------------------ .../v3/globalrolebinding/setup_test.go | 31 +-- .../v3/globalrolebinding/validator_test.go | 19 -- 6 files changed, 12 insertions(+), 302 deletions(-) diff --git a/pkg/auth/globalrole.go b/pkg/auth/globalrole.go index ad2b5bf3b..c19ec716c 100644 --- a/pkg/auth/globalrole.go +++ b/pkg/auth/globalrole.go @@ -16,8 +16,6 @@ type GlobalRoleResolver struct { const ownerRT = "cluster-owner" -var adminRoles = []string{"restricted-admin"} - // NewRoleTemplateResolver creates a newly allocated RoleTemplateResolver from the provided caches func NewGlobalRoleResolver(roleTemplateResolver *RoleTemplateResolver, grCache controllerv3.GlobalRoleCache) *GlobalRoleResolver { return &GlobalRoleResolver{ @@ -46,17 +44,7 @@ func (g *GlobalRoleResolver) ClusterRulesFromRole(gr *v3.GlobalRole) ([]rbacv1.P if gr == nil { return nil, nil } - // restricted admin is treated like it is owner of all downstream clusters - // but it doesn't get the same field because this would duplicate legacy logic - for _, name := range adminRoles { - if gr.Name == name { - templateRules, err := g.roleTemplateResolver.RulesFromTemplateName(ownerRT) - if err != nil { - return nil, fmt.Errorf("unable to resolve cluster-owner rules: %w", err) - } - return templateRules, nil - } - } + var rules []rbacv1.PolicyRule for _, inheritedRoleTemplate := range gr.InheritedClusterRoles { templateRules, err := g.roleTemplateResolver.RulesFromTemplateName(inheritedRoleTemplate) @@ -74,18 +62,6 @@ func (g *GlobalRoleResolver) ClusterRulesFromRole(gr *v3.GlobalRole) ([]rbacv1.P // use it to evaluate InheritedFleetWorkspacePermissions.ResourceRules. However, it shouldn't be used in a more generic evaluation // of permissions on the workspace backing namespace. func (g *GlobalRoleResolver) FleetWorkspacePermissionsResourceRulesFromRole(gr *v3.GlobalRole) []rbacv1.PolicyRule { - for _, name := range adminRoles { - if gr.Name == name { - return []rbacv1.PolicyRule{ - { - Verbs: []string{"*"}, - APIGroups: []string{"fleet.cattle.io"}, - Resources: []string{"clusterregistrationtokens", "gitreporestrictions", "clusterregistrations", "clusters", "gitrepos", "bundles", "clustergroups"}, - }, - } - } - } - if gr == nil || gr.InheritedFleetWorkspacePermissions == nil { return nil } @@ -98,16 +74,6 @@ func (g *GlobalRoleResolver) FleetWorkspacePermissionsResourceRulesFromRole(gr * // use it to evaluate InheritedFleetWorkspacePermissions.WorkspaceVerbs. However, it shouldn't be used in a more generic evaluation // of permissions on the workspace object. func (g *GlobalRoleResolver) FleetWorkspacePermissionsWorkspaceVerbsFromRole(gr *v3.GlobalRole) []rbacv1.PolicyRule { - for _, name := range adminRoles { - if gr.Name == name { - return []rbacv1.PolicyRule{{ - Verbs: []string{"*"}, - APIGroups: []string{"management.cattle.io"}, - Resources: []string{"fleetworkspaces"}, - }} - } - } - if gr == nil || gr.InheritedFleetWorkspacePermissions == nil { return nil } diff --git a/pkg/auth/globalrole_test.go b/pkg/auth/globalrole_test.go index e23a08576..fff4d8cd3 100644 --- a/pkg/auth/globalrole_test.go +++ b/pkg/auth/globalrole_test.go @@ -215,21 +215,6 @@ func TestClusterRulesFromRole(t *testing.T) { }, wantRules: append(append(noInheritRules, firstRTRules...), secondRTRules...), }, - { - name: "test restricted admin gr", - globalRole: &v3.GlobalRole{ - ObjectMeta: metav1.ObjectMeta{ - Name: "restricted-admin", - }, - Rules: globalRules, - InheritedClusterRoles: []string{}, - }, - stateSetup: func(state testState) { - state.rtCacheMock.EXPECT().Get("cluster-owner").Return(adminRT, nil) - }, - wantRules: adminRTRules, - }, - { name: "test rt resolver error", globalRole: &v3.GlobalRole{ diff --git a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go index 01406304d..639a7be89 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/setup_test.go @@ -24,9 +24,8 @@ import ( ) const ( - adminUser = "admin-userid" - testUser = "test-user" - restrictedAdminUser = "restricted-admin-userid" + adminUser = "admin-userid" + testUser = "test-user" ) var ( @@ -43,7 +42,7 @@ var ( }, }, } - clusterRoles = []*v1.ClusterRole{adminCR, readPodsCR, baseCR, restrictedAdminCR} + clusterRoles = []*v1.ClusterRole{adminCR, readPodsCR, baseCR} clusterRoleBindings = []*v1.ClusterRoleBinding{ { @@ -52,12 +51,6 @@ var ( }, RoleRef: v1.RoleRef{APIGroup: v1.GroupName, Kind: "ClusterRole", Name: adminCR.Name}, }, - { - Subjects: []v1.Subject{ - {Kind: v1.UserKind, Name: restrictedAdminUser}, - }, - RoleRef: v1.RoleRef{APIGroup: v1.GroupName, Kind: "ClusterRole", Name: restrictedAdminCR.Name}, - }, { Subjects: []v1.Subject{ {Kind: v1.UserKind, Name: testUser}, @@ -131,11 +124,6 @@ var ( WorkspaceVerbs: []string{"GET"}, }, } - restrictedAdminGR = v3.GlobalRole{ - ObjectMeta: metav1.ObjectMeta{ - Name: "restricted-admin", - }, - } baseGRB = v3.GlobalRoleBinding{ ObjectMeta: metav1.ObjectMeta{ Name: "base-grb", @@ -143,13 +131,6 @@ var ( GlobalRoleName: baseGR.Name, UserName: testUser, } - restrictedAdminGRB = v3.GlobalRoleBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "restricted-admin-grb", - }, - GlobalRoleName: restrictedAdminCR.Name, - UserName: restrictedAdminUser, - } ruleReadPods = v1.PolicyRule{ Verbs: []string{"GET", "WATCH"}, @@ -177,12 +158,6 @@ var ( }, Rules: []v1.PolicyRule{ruleAdmin}, } - restrictedAdminCR = &v1.ClusterRole{ - ObjectMeta: metav1.ObjectMeta{ - Name: "restricted-admin", - }, - Rules: []v1.PolicyRule{}, - } readPodsCR = &v1.ClusterRole{ ObjectMeta: metav1.ObjectMeta{Name: "read-pods"}, Rules: []v1.PolicyRule{ruleReadPods}, @@ -315,7 +290,6 @@ func newDefaultState(t *testing.T) testState { grbCacheMock := fake.NewMockNonNamespacedCacheInterface[*v3.GlobalRoleBinding](ctrl) grbs := []*v3.GlobalRoleBinding{&baseGRB} grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(testUser, "")).Return(grbs, nil).AnyTimes() - grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(restrictedAdminUser, "")).Return([]*v3.GlobalRoleBinding{&restrictedAdminGRB}, nil).AnyTimes() grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(adminUser, "")).Return(grbs, nil).AnyTimes() grbCacheMock.EXPECT().AddIndexer(gomock.Any(), gomock.Any()).AnyTimes() grCacheMock.EXPECT().Get(baseGR.Name).Return(&baseGR, nil).AnyTimes() diff --git a/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go b/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go index a9609da99..fe43b330e 100644 --- a/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrole/validator_test.go @@ -870,187 +870,6 @@ func TestAdmit(t *testing.T) { }, allowed: true, }, - { - name: "restricted admin can create GR with InheritedFleetWorkspacePermissions and fleet rules", - args: args{ - username: restrictedAdminUser, - newGR: func() *v3.GlobalRole { - baseGR := newDefaultGR() - baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ - ResourceRules: []v1.PolicyRule{ - { - Verbs: []string{"get", "list", "create", "delete"}, - APIGroups: []string{"fleet.cattle.io"}, - Resources: []string{"bundles", "gitrepos"}, - }, - }, - WorkspaceVerbs: []string{ - "get", - "create", - }, - } - return baseGR - }, - stateSetup: func(state testState) { - state.grCacheMock.EXPECT().Get(restrictedAdminGR.Name).Return(&restrictedAdminGR, nil).AnyTimes() - setSarResponse(false, nil, testUser, newDefaultGR().Name, state.sarMock) - }, - }, - - allowed: true, - }, - { - name: "restricted admin can create GR with InheritedFleetWorkspacePermissions and fleet rules and *", - args: args{ - username: restrictedAdminUser, - newGR: func() *v3.GlobalRole { - baseGR := newDefaultGR() - baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ - ResourceRules: []v1.PolicyRule{ - { - Verbs: []string{"*"}, - APIGroups: []string{"fleet.cattle.io"}, - Resources: []string{"bundles", "gitrepos"}, - }, - }, - WorkspaceVerbs: []string{ - "*", - }, - } - return baseGR - }, - stateSetup: func(state testState) { - state.grCacheMock.EXPECT().Get(restrictedAdminGR.Name).Return(&restrictedAdminGR, nil).AnyTimes() - setSarResponse(false, nil, testUser, newDefaultGR().Name, state.sarMock) - }, - }, - - allowed: true, - }, - { - name: "restricted admin can't create GR with InheritedFleetWorkspacePermissions and pod rules", - args: args{ - username: restrictedAdminUser, - newGR: func() *v3.GlobalRole { - baseGR := newDefaultGR() - baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ - ResourceRules: []v1.PolicyRule{ - { - Verbs: []string{"get", ""}, - APIGroups: []string{""}, - Resources: []string{"pods"}, - }, - }, - WorkspaceVerbs: []string{ - "get", - "create", - }, - } - return baseGR - }, - stateSetup: func(state testState) { - state.grCacheMock.EXPECT().Get(restrictedAdminGR.Name).Return(&restrictedAdminGR, nil).AnyTimes() - setSarResponse(false, nil, testUser, newDefaultGR().Name, state.sarMock) - }, - }, - - allowed: false, - }, - { - name: "restricted admin can update GR with InheritedFleetWorkspacePermissions and fleet rules", - args: args{ - username: restrictedAdminUser, - oldGR: func() *v3.GlobalRole { - return newDefaultGR() - }, - newGR: func() *v3.GlobalRole { - baseGR := newDefaultGR() - baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ - ResourceRules: []v1.PolicyRule{ - { - Verbs: []string{"get", "list", "create", "delete"}, - APIGroups: []string{"fleet.cattle.io"}, - Resources: []string{"bundles", "gitrepos"}, - }, - }, - WorkspaceVerbs: []string{ - "get", - "create", - }, - } - return baseGR - }, - stateSetup: func(state testState) { - state.grCacheMock.EXPECT().Get(restrictedAdminGR.Name).Return(&restrictedAdminGR, nil).AnyTimes() - setSarResponse(false, nil, testUser, newDefaultGR().Name, state.sarMock) - }, - }, - - allowed: true, - }, - { - name: "restricted admin can update GR with InheritedFleetWorkspacePermissions and fleet rules and *", - args: args{ - username: restrictedAdminUser, - oldGR: func() *v3.GlobalRole { - return newDefaultGR() - }, - newGR: func() *v3.GlobalRole { - baseGR := newDefaultGR() - baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ - ResourceRules: []v1.PolicyRule{ - { - Verbs: []string{"*"}, - APIGroups: []string{"fleet.cattle.io"}, - Resources: []string{"bundles", "gitrepos"}, - }, - }, - WorkspaceVerbs: []string{ - "*", - }, - } - return baseGR - }, - stateSetup: func(state testState) { - state.grCacheMock.EXPECT().Get(restrictedAdminGR.Name).Return(&restrictedAdminGR, nil).AnyTimes() - setSarResponse(false, nil, testUser, newDefaultGR().Name, state.sarMock) - }, - }, - - allowed: true, - }, - { - name: "restricted admin can't update GR with InheritedFleetWorkspacePermissions and pod rules", - args: args{ - username: restrictedAdminUser, - oldGR: func() *v3.GlobalRole { - return newDefaultGR() - }, - newGR: func() *v3.GlobalRole { - baseGR := newDefaultGR() - baseGR.InheritedFleetWorkspacePermissions = &v3.FleetWorkspacePermission{ - ResourceRules: []v1.PolicyRule{ - { - Verbs: []string{"get", ""}, - APIGroups: []string{""}, - Resources: []string{"pods"}, - }, - }, - WorkspaceVerbs: []string{ - "get", - "create", - }, - } - return baseGR - }, - stateSetup: func(state testState) { - state.grCacheMock.EXPECT().Get(restrictedAdminGR.Name).Return(&restrictedAdminGR, nil).AnyTimes() - setSarResponse(false, nil, testUser, newDefaultGR().Name, state.sarMock) - }, - }, - - allowed: false, - }, } for _, test := range tests { diff --git a/pkg/resources/management.cattle.io/v3/globalrolebinding/setup_test.go b/pkg/resources/management.cattle.io/v3/globalrolebinding/setup_test.go index 95e44af89..e1a8be77f 100644 --- a/pkg/resources/management.cattle.io/v3/globalrolebinding/setup_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrolebinding/setup_test.go @@ -25,15 +25,14 @@ import ( ) const ( - adminUser = "admin-user" - restrictedAdminUser = "restricted-admin-user" - testUser = "test-user" - noPrivUser = "no-priv-user" - newUser = "newUser-user" - newGroupPrinc = "local://group" - testGroup = "testGroup" - notFoundName = "not-found" - errName = "error-Name" + adminUser = "admin-user" + testUser = "test-user" + noPrivUser = "no-priv-user" + newUser = "newUser-user" + newGroupPrinc = "local://group" + testGroup = "testGroup" + notFoundName = "not-found" + errName = "error-Name" ) type testCase struct { @@ -173,13 +172,6 @@ var ( GlobalRoleName: baseGR.Name, UserName: testUser, } - restrictedAdminGRB = v3.GlobalRoleBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "res-grb", - }, - GlobalRoleName: restrictedAdminGR.Name, - UserName: restrictedAdminUser, - } adminRT = v3.RoleTemplate{ ObjectMeta: metav1.ObjectMeta{ Name: "escalation-rt", @@ -280,11 +272,6 @@ var ( }, }, } - restrictedAdminGR = v3.GlobalRole{ - ObjectMeta: metav1.ObjectMeta{ - Name: "restricted-admin", - }, - } fwResourceRules = []rbacv1.PolicyRule{ { APIGroups: []string{"fleet.cattle.io"}, @@ -395,11 +382,9 @@ func newDefaultState(t *testing.T) testState { grbs := []*v3.GlobalRoleBinding{&baseGRB} grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(testUser, "")).Return(grbs, nil).AnyTimes() grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(adminUser, "")).Return(grbs, nil).AnyTimes() - grbCacheMock.EXPECT().GetByIndex(gomock.Any(), resolvers.GetUserKey(restrictedAdminUser, "")).Return([]*v3.GlobalRoleBinding{&restrictedAdminGRB}, nil).AnyTimes() grbCacheMock.EXPECT().AddIndexer(gomock.Any(), gomock.Any()).AnyTimes() grCacheMock.EXPECT().Get(baseGR.Name).Return(&baseGR, nil).AnyTimes() - grCacheMock.EXPECT().Get(restrictedAdminGR.Name).Return(&restrictedAdminGR, nil).AnyTimes() grCacheMock.EXPECT().Get(adminGR.Name).Return(adminGR, nil).AnyTimes() grCacheMock.EXPECT().Get(notFoundName).Return(nil, newNotFound(notFoundName)).AnyTimes() grCacheMock.EXPECT().Get("").Return(nil, newNotFound("")).AnyTimes() diff --git a/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go b/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go index 94e3a2502..390395c11 100644 --- a/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go +++ b/pkg/resources/management.cattle.io/v3/globalrolebinding/validator_test.go @@ -784,25 +784,6 @@ func TestAdmit(t *testing.T) { }, allowed: false, }, - { - name: "restricted admin can grant GR with InheritedFleetWorkspacePermissions", - args: args{ - username: restrictedAdminUser, - newGRB: func() *v3.GlobalRoleBinding { - return &v3.GlobalRoleBinding{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-grb", - }, - UserName: testUser, - GlobalRoleName: fwGR.Name, - } - }, - stateSetup: func(ts testState) { - ts.grCacheMock.EXPECT().Get(fwGR.Name).Return(&fwGR, nil) - }, - }, - allowed: true, - }, } for _, test := range tests {