From 3525e603de0e923f0a386f774c26a416df4be02c Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Wed, 24 Feb 2021 17:56:48 +0000 Subject: [PATCH 1/4] Fix SCC resource to only affect KIC pods Priority of 20 overrides the max priority of the default: AnyPid. This leads to the SCC binding to all pods created in the same namespace rather than just KIC pods. Priority is now changed to the default value so if a new pod is created, it takes the default SCC rather than the SCC of the operator. --- pkg/controller/nginxingresscontroller/scc.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/controller/nginxingresscontroller/scc.go b/pkg/controller/nginxingresscontroller/scc.go index f92cacfc..e7cf0137 100644 --- a/pkg/controller/nginxingresscontroller/scc.go +++ b/pkg/controller/nginxingresscontroller/scc.go @@ -9,7 +9,6 @@ import ( ) func sccForNginxIngressController(name string) *secv1.SecurityContextConstraints { - var priority int32 = 20 var uid int64 = 101 allowPrivilegeEscalation := true @@ -19,7 +18,6 @@ func sccForNginxIngressController(name string) *secv1.SecurityContextConstraints Name: name, }, AllowHostPorts: false, - Priority: &priority, AllowPrivilegedContainer: false, RunAsUser: secv1.RunAsUserStrategyOptions{ Type: "MustRunAs", From 03ff09ae0f26f66175ecdcda1312eb0bba64b276 Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Wed, 24 Feb 2021 18:14:19 +0000 Subject: [PATCH 2/4] Remove priority from unit test --- pkg/controller/nginxingresscontroller/scc_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/controller/nginxingresscontroller/scc_test.go b/pkg/controller/nginxingresscontroller/scc_test.go index 87e279e8..aaffd494 100644 --- a/pkg/controller/nginxingresscontroller/scc_test.go +++ b/pkg/controller/nginxingresscontroller/scc_test.go @@ -11,7 +11,6 @@ import ( ) func TestSccForNginxIngressController(t *testing.T) { - var priority int32 = 20 var uid int64 = 101 name := "my-nginx-ingress" @@ -22,7 +21,6 @@ func TestSccForNginxIngressController(t *testing.T) { Name: name, }, AllowHostPorts: false, - Priority: &priority, AllowPrivilegedContainer: false, RunAsUser: secv1.RunAsUserStrategyOptions{ Type: "MustRunAs", From 4f795817298cb2114b6b9a01de03672f92f1ec6c Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Thu, 25 Feb 2021 11:52:41 +0000 Subject: [PATCH 3/4] Improve test to print diff upon mismatch --- pkg/controller/nginxingresscontroller/scc_test.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkg/controller/nginxingresscontroller/scc_test.go b/pkg/controller/nginxingresscontroller/scc_test.go index aaffd494..de94740c 100644 --- a/pkg/controller/nginxingresscontroller/scc_test.go +++ b/pkg/controller/nginxingresscontroller/scc_test.go @@ -2,9 +2,9 @@ package nginxingresscontroller import ( "fmt" - "reflect" "testing" + "github.com/google/go-cmp/cmp" secv1 "github.com/openshift/api/security/v1" corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -12,7 +12,6 @@ import ( func TestSccForNginxIngressController(t *testing.T) { var uid int64 = 101 - name := "my-nginx-ingress" allowPrivilegeEscalation := true @@ -50,8 +49,8 @@ func TestSccForNginxIngressController(t *testing.T) { } result := sccForNginxIngressController(name) - if !reflect.DeepEqual(result, expected) { - t.Errorf("sccForNginxIngressController(%v) returned %+v but expected %+v", name, result, expected) + if diff := cmp.Diff(expected, result); diff != "" { + t.Errorf("sccForNginxIngressController() mismatch (-want +got):\n%s", diff) } } From cc48ac034eed16fa0147a48eb38c9a609359b759 Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Tue, 2 Mar 2021 17:53:27 +0000 Subject: [PATCH 4/4] Remove SCC group. Fix SCC user reference --- pkg/controller/nginxingresscontroller/scc.go | 3 +-- pkg/controller/nginxingresscontroller/scc_test.go | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/controller/nginxingresscontroller/scc.go b/pkg/controller/nginxingresscontroller/scc.go index e7cf0137..613c179d 100644 --- a/pkg/controller/nginxingresscontroller/scc.go +++ b/pkg/controller/nginxingresscontroller/scc.go @@ -33,7 +33,6 @@ func sccForNginxIngressController(name string) *secv1.SecurityContextConstraints FSGroup: secv1.FSGroupStrategyOptions{ Type: "MustRunAs", }, - Groups: []string{"system:authenticated"}, SupplementalGroups: secv1.SupplementalGroupsStrategyOptions{ Type: "MustRunAs", }, @@ -48,5 +47,5 @@ func sccForNginxIngressController(name string) *secv1.SecurityContextConstraints } func userForSCC(namespace string, name string) string { - return fmt.Sprintf("%v:%v", namespace, name) + return fmt.Sprintf("system:serviceaccount:%v:%v", namespace, name) } diff --git a/pkg/controller/nginxingresscontroller/scc_test.go b/pkg/controller/nginxingresscontroller/scc_test.go index de94740c..fd68491b 100644 --- a/pkg/controller/nginxingresscontroller/scc_test.go +++ b/pkg/controller/nginxingresscontroller/scc_test.go @@ -35,7 +35,6 @@ func TestSccForNginxIngressController(t *testing.T) { FSGroup: secv1.FSGroupStrategyOptions{ Type: "MustRunAs", }, - Groups: []string{"system:authenticated"}, SupplementalGroups: secv1.SupplementalGroupsStrategyOptions{ Type: "MustRunAs", }, @@ -57,7 +56,7 @@ func TestSccForNginxIngressController(t *testing.T) { func TestUserForSCC(t *testing.T) { namespace := "my-nginx-ingress" name := "my-nginx-ingress-controller" - expected := fmt.Sprintf("%v:%v", namespace, name) + expected := fmt.Sprintf("system:serviceaccount:%v:%v", namespace, name) result := userForSCC(namespace, name) if expected != result {