Skip to content

Commit bd1cab8

Browse files
Merge pull request #400 from tnierman/revert-393-revert-387-srep-1565
Re-implement: "SREP-1565 - Remove cluster-admins exception from namespaces webhook"
2 parents 60c1ed5 + f4fe447 commit bd1cab8

File tree

2 files changed

+14
-36
lines changed

2 files changed

+14
-36
lines changed

pkg/webhooks/namespace/namespace.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ const (
2727
layeredProductNamespace string = `^redhat-.*`
2828
layeredProductAdminGroupName string = "layered-sre-cluster-admins"
2929
docString string = `Managed OpenShift Customers may not modify namespaces specified in the %v ConfigMaps because customer workloads should be placed in customer-created namespaces. Customers may not create namespaces identified by this regular expression %s because it could interfere with critical DNS resolution. Additionally, customers may not set or change the values of these Namespace labels %s.`
30-
clusterAdminGroup string = "cluster-admins"
3130
)
3231

3332
// exported vars to be used across packages
@@ -352,7 +351,7 @@ func NewWebhook() *NamespaceWebhook {
352351
}
353352

354353
func amIAdmin(request admissionctl.Request) bool {
355-
if slices.Contains(clusterAdminUsers, request.UserInfo.Username) || slices.Contains(request.UserInfo.Groups, clusterAdminGroup) {
354+
if slices.Contains(clusterAdminUsers, request.UserInfo.Username) {
356355
return true
357356
}
358357

pkg/webhooks/namespace/namespace_test.go

Lines changed: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -717,13 +717,13 @@ func TestAdminUser(t *testing.T) {
717717
shouldBeAllowed: true,
718718
},
719719
{
720-
// admin users gonna admin
720+
// cluster-admin users cannot update privilegedNamespaces
721721
testID: "cluster-admin-test",
722722
targetNamespace: privilegedNamespace,
723723
username: "lisa",
724724
userGroups: []string{"cluster-admins", "system:authenticated", "system:authenticated:oauth"},
725725
operation: admissionv1.Update,
726-
shouldBeAllowed: true,
726+
shouldBeAllowed: false,
727727
},
728728
{
729729
// admin users gonna admin
@@ -762,40 +762,31 @@ func TestAdminUser(t *testing.T) {
762762
shouldBeAllowed: true,
763763
},
764764
{
765-
// Admins should be able to create a privileged namespace
766-
testID: "cluster-admin-in-ns-test",
765+
// cluster-admin group members should not be able to create a privileged namespace
766+
testID: "cluster-admin-group-in-ns-test",
767767
targetNamespace: "in",
768768
username: "lisa",
769769
userGroups: []string{"cluster-admins", "system:authenticated", "system:authenticated:oauth"},
770770
operation: admissionv1.Create,
771-
shouldBeAllowed: true,
772-
},
773-
{
774-
// Admins should be able to create a privileged namespace
775-
testID: "cluster-admin-in-ns-test",
776-
targetNamespace: privilegedNamespace,
777-
username: "lisa",
778-
userGroups: []string{"cluster-admins", "system:authenticated", "system:authenticated:oauth"},
779-
operation: admissionv1.Create,
780-
shouldBeAllowed: true,
771+
shouldBeAllowed: false,
781772
},
782773
{
783-
// Admins should be able to update a privileged namespace
774+
// cluster-admin group members should not be able to update a privileged namespace
784775
testID: "cluster-admin-in-ns-test",
785776
targetNamespace: privilegedNamespace,
786777
username: "lisa",
787778
userGroups: []string{"cluster-admins", "system:authenticated", "system:authenticated:oauth"},
788779
operation: admissionv1.Update,
789-
shouldBeAllowed: true,
780+
shouldBeAllowed: false,
790781
},
791782
{
792-
// Admins should be able to delete a privileged namespace
783+
// cluster-admins group members should not be able to delete a privileged namespace
793784
testID: "cluster-admin-in-ns-test",
794785
targetNamespace: privilegedNamespace,
795786
username: "lisa",
796787
userGroups: []string{"cluster-admins", "system:authenticated", "system:authenticated:oauth"},
797788
operation: admissionv1.Delete,
798-
shouldBeAllowed: true,
789+
shouldBeAllowed: false,
799790
},
800791
}
801792
runNamespaceTests(t, tests)
@@ -825,25 +816,13 @@ func TestLabelCreates(t *testing.T) {
825816
shouldBeAllowed: true,
826817
},
827818
{
828-
testID: "cluster-admin-can-create-priv-labelled-ns",
819+
testID: "cluster-admins-group-cannot-create-priv-labelled-ns",
829820
targetNamespace: privilegedNamespace,
830821
username: "no-reply@redhat.com",
831822
userGroups: []string{"cluster-admins", "system:authenticated", "system:authenticated:oauth"},
832823
operation: admissionv1.Create,
833824
labels: map[string]string{"my-label": "hello"},
834-
shouldBeAllowed: true,
835-
},
836-
{
837-
testID: "cluster-admins-can-create-priv-labelled-ns",
838-
targetNamespace: privilegedNamespace,
839-
username: "no-reply@redhat.com",
840-
userGroups: []string{"cluster-admins", "system:authenticated", "system:authenticated:oauth"},
841-
operation: admissionv1.Create,
842-
labels: map[string]string{
843-
"managed.openshift.io/storage-pv-quota-exempt": "true",
844-
"managed.openshift.io/storage-lb-quota-exempt": "true",
845-
},
846-
shouldBeAllowed: true,
825+
shouldBeAllowed: false,
847826
},
848827
{
849828
testID: "admin-test",
@@ -1082,14 +1061,14 @@ func TestLabellingUpdates(t *testing.T) {
10821061
shouldBeAllowed: false,
10831062
},
10841063
{
1085-
testID: "cluster-admin-can-add-removable-label-on-unpriv-ns",
1064+
testID: "cluster-admin-cant-add-removable-label-on-unpriv-ns",
10861065
targetNamespace: "my-customer-ns",
10871066
username: "test@user",
10881067
userGroups: []string{"cluster-admins", "system:authenticated", "system:authenticated:oauth"},
10891068
operation: admissionv1.Update,
10901069
oldObject: createOldObject("my-customer-ns", "user-can-remove-removable-label-from-unpriv-ns", map[string]string{}),
10911070
labels: map[string]string{"openshift.io/cluster-monitoring": "true"},
1092-
shouldBeAllowed: true,
1071+
shouldBeAllowed: false,
10931072
},
10941073
{
10951074
testID: "backplane-cluster-admin-can-add-removable-label-on-unpriv-ns",

0 commit comments

Comments
 (0)