diff --git a/controllers/ingressclass.go b/controllers/ingressclass.go index 4e55da5d..24075823 100644 --- a/controllers/ingressclass.go +++ b/controllers/ingressclass.go @@ -7,9 +7,13 @@ import ( ) func ingressClassForNginxIngressController(instance *k8sv1alpha1.NginxIngressController) *networking.IngressClass { + ingressClassName := "nginx" + if instance.Spec.IngressClass != "" { + ingressClassName = instance.Spec.IngressClass + } ic := &networking.IngressClass{ ObjectMeta: metav1.ObjectMeta{ - Name: instance.Spec.IngressClass, + Name: ingressClassName, }, Spec: networking.IngressClassSpec{ Controller: "nginx.org/ingress-controller", diff --git a/controllers/ingressclass_test.go b/controllers/ingressclass_test.go index 8b018dc9..91c46479 100644 --- a/controllers/ingressclass_test.go +++ b/controllers/ingressclass_test.go @@ -29,3 +29,22 @@ func TestIngressClassForNginxIngressController(t *testing.T) { t.Errorf("ingressClassForNginxIngressController() mismatch (-want +got):\n%s", diff) } } + +func TestIngressClassForNginxIngressControllerDefault(t *testing.T) { + instance := &k8sv1alpha1.NginxIngressController{ + Spec: k8sv1alpha1.NginxIngressControllerSpec{}, + } + expected := &networking.IngressClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nginx", + }, + Spec: networking.IngressClassSpec{ + Controller: "nginx.org/ingress-controller", + }, + } + + result := ingressClassForNginxIngressController(instance) + if diff := cmp.Diff(expected, result); diff != "" { + t.Errorf("ingressClassForNginxIngressController() mismatch (-want +got):\n%s", diff) + } +} diff --git a/controllers/nginxingresscontroller_controller.go b/controllers/nginxingresscontroller_controller.go index 99a8080c..e9f2d7f3 100644 --- a/controllers/nginxingresscontroller_controller.go +++ b/controllers/nginxingresscontroller_controller.go @@ -21,6 +21,8 @@ import ( "fmt" "strings" + "github.com/nginxinc/nginx-ingress-operator/controllers/scc" + appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -39,7 +41,6 @@ import ( const ( clusterRoleName = "nginx-ingress-role" - sccName = "nginx-ingress-scc" finalizer = "nginxingresscontroller.k8s.nginx.org/finalizer" ) @@ -287,33 +288,12 @@ func (r *NginxIngressControllerReconciler) finalizeNginxIngressController(log lo } if r.SccAPIExists { - scc := sccForNginxIngressController(sccName) - - err := r.Get(context.TODO(), types.NamespacedName{Name: sccName, Namespace: v1.NamespaceAll}, scc) - if err != nil { - return err - } - - var users []string - for _, u := range scc.Users { - if u != userForSCC(instance.Namespace, instance.Name) { - users = append(users, u) - } - } - - scc.Users = users - - err = r.Update(context.TODO(), scc) + err := scc.RemoveServiceAccount(r.Client, instance.Namespace, instance.Name) if err != nil { - return err + return fmt.Errorf("failed to remove service account user from SCC: %w", err) } } - ic := ingressClassForNginxIngressController(instance) - if err := r.Delete(context.TODO(), ic); client.IgnoreNotFound(err) != nil { - return err - } - log.Info("Successfully finalized NginxIngressController") return nil } diff --git a/controllers/prerequisites.go b/controllers/prerequisites.go index 1db12021..d9dfaf13 100644 --- a/controllers/prerequisites.go +++ b/controllers/prerequisites.go @@ -4,6 +4,8 @@ import ( "context" "fmt" + "github.com/nginxinc/nginx-ingress-operator/controllers/scc" + "github.com/go-logr/logr" k8sv1alpha1 "github.com/nginxinc/nginx-ingress-operator/api/v1alpha1" v1 "k8s.io/api/core/v1" @@ -56,12 +58,7 @@ func (r *NginxIngressControllerReconciler) checkPrerequisites(log logr.Logger, i // IngressClass is available from k8s 1.18+ minVersion, _ := version.ParseGeneric("v1.18.0") if RunningK8sVersion.AtLeast(minVersion) { - if instance.Spec.IngressClass == "" { - instance.Spec.IngressClass = "nginx" - log.Info("Warning! IngressClass not set, using default", "IngressClass.Name", instance.Spec.IngressClass) - } ic := ingressClassForNginxIngressController(instance) - existed, err = r.createIfNotExists(ic) if err != nil { return err @@ -96,30 +93,9 @@ func (r *NginxIngressControllerReconciler) checkPrerequisites(log logr.Logger, i } if r.SccAPIExists { - // Assign this new User to the SCC (if is not present already) - scc := sccForNginxIngressController(sccName) - - err = r.Get(context.TODO(), types.NamespacedName{Name: sccName, Namespace: v1.NamespaceAll}, scc) + err := scc.AddServiceAccount(r.Client, sa.Namespace, sa.Name) if err != nil { - return err - } - - user := userForSCC(sa.Namespace, sa.Name) - found := false - for _, u := range scc.Users { - if u == user { - found = true - break - } - } - - if !found { - scc.Users = append(scc.Users, user) - - err = r.Update(context.TODO(), scc) - if err != nil { - return err - } + return fmt.Errorf("failed to add service account user to scc: %w", err) } } @@ -175,16 +151,9 @@ func (r *NginxIngressControllerReconciler) createCommonResources(log logr.Logger if r.SccAPIExists { log.Info("OpenShift detected as platform.") - scc := sccForNginxIngressController(sccName) - - err = r.Get(context.TODO(), types.NamespacedName{Name: sccName, Namespace: v1.NamespaceAll}, scc) - if err != nil && errors.IsNotFound(err) { - log.Info("no previous SecurityContextConstraints found, creating a new one.") - err = r.Create(context.TODO(), scc) - } - + err := scc.Create(r.Client, log) if err != nil { - return fmt.Errorf("error creating SecurityContextConstraints: %w", err) + return fmt.Errorf("failed to create SecurityContextConstraints: %w", err) } } diff --git a/controllers/scc.go b/controllers/scc.go deleted file mode 100644 index 08a5d1bf..00000000 --- a/controllers/scc.go +++ /dev/null @@ -1,52 +0,0 @@ -package controllers - -import ( - "fmt" - - secv1 "github.com/openshift/api/security/v1" - corev1 "k8s.io/api/core/v1" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -func sccForNginxIngressController(name string) *secv1.SecurityContextConstraints { - var uid int64 = 101 - - allowPrivilegeEscalation := true - - scc := &secv1.SecurityContextConstraints{ - ObjectMeta: v1.ObjectMeta{ - Name: name, - }, - AllowHostPorts: false, - AllowPrivilegedContainer: false, - RunAsUser: secv1.RunAsUserStrategyOptions{ - Type: "MustRunAs", - UID: &uid, - }, - Users: nil, - AllowHostDirVolumePlugin: false, - AllowHostIPC: false, - SELinuxContext: secv1.SELinuxContextStrategyOptions{ - Type: "MustRunAs", - }, - ReadOnlyRootFilesystem: false, - FSGroup: secv1.FSGroupStrategyOptions{ - Type: "MustRunAs", - }, - SupplementalGroups: secv1.SupplementalGroupsStrategyOptions{ - Type: "MustRunAs", - }, - Volumes: []secv1.FSType{"secret"}, - AllowHostPID: false, - AllowHostNetwork: false, - AllowPrivilegeEscalation: &allowPrivilegeEscalation, - RequiredDropCapabilities: []corev1.Capability{"ALL"}, - DefaultAddCapabilities: []corev1.Capability{"NET_BIND_SERVICE"}, - AllowedCapabilities: nil, - } - return scc -} - -func userForSCC(namespace string, name string) string { - return fmt.Sprintf("system:serviceaccount:%v:%v", namespace, name) -} diff --git a/controllers/scc/scc.go b/controllers/scc/scc.go new file mode 100644 index 00000000..f5f7106f --- /dev/null +++ b/controllers/scc/scc.go @@ -0,0 +1,125 @@ +package scc + +import ( + "context" + "fmt" + + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + secv1 "github.com/openshift/api/security/v1" + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +const defaultName = "nginx-ingress-scc" + +func sccConfigTemplate() *secv1.SecurityContextConstraints { + var uid int64 = 101 + allowPrivilegeEscalation := true + + return &secv1.SecurityContextConstraints{ + ObjectMeta: v1.ObjectMeta{ + Name: defaultName, + }, + AllowHostPorts: false, + AllowPrivilegedContainer: false, + RunAsUser: secv1.RunAsUserStrategyOptions{ + Type: "MustRunAs", + UID: &uid, + }, + Users: nil, + AllowHostDirVolumePlugin: false, + AllowHostIPC: false, + SELinuxContext: secv1.SELinuxContextStrategyOptions{ + Type: "MustRunAs", + }, + ReadOnlyRootFilesystem: false, + FSGroup: secv1.FSGroupStrategyOptions{ + Type: "MustRunAs", + }, + SupplementalGroups: secv1.SupplementalGroupsStrategyOptions{ + Type: "MustRunAs", + }, + Volumes: []secv1.FSType{"secret"}, + AllowHostPID: false, + AllowHostNetwork: false, + AllowPrivilegeEscalation: &allowPrivilegeEscalation, + RequiredDropCapabilities: []corev1.Capability{"ALL"}, + DefaultAddCapabilities: []corev1.Capability{"NET_BIND_SERVICE"}, + AllowedCapabilities: nil, + } +} + +func serviceAccountName(namespace string, name string) string { + return fmt.Sprintf("system:serviceaccount:%v:%v", namespace, name) +} + +func Create(client client.Client, log logr.Logger) error { + scc := sccConfigTemplate() + err := client.Get(context.TODO(), types.NamespacedName{Name: defaultName, Namespace: v1.NamespaceAll}, scc) + if err != nil { + if errors.IsNotFound(err) { + log.Info("no previous SecurityContextConstraints found, creating a new one.") + err = client.Create(context.TODO(), scc) + if err != nil { + return fmt.Errorf("error creating SecurityContextConstraints: %w", err) + } + } + return fmt.Errorf("error getting scc: %w", err) + } + + return nil +} + +func AddServiceAccount(client client.Client, namespace string, name string) error { + scc := sccConfigTemplate() + err := client.Get(context.TODO(), types.NamespacedName{Name: defaultName, Namespace: v1.NamespaceAll}, scc) + if err != nil { + return fmt.Errorf("failed to get scc: %w", err) + } + + saName := serviceAccountName(namespace, name) + for _, u := range scc.Users { + if u == saName { + // scc already has the service account name + return nil + } + } + + scc.Users = append(scc.Users, saName) + err = client.Update(context.TODO(), scc) + if err != nil { + return fmt.Errorf("failed to update scc: %w", err) + } + + return nil +} + +func RemoveServiceAccount(client client.Client, namespace string, name string) error { + scc := sccConfigTemplate() + err := client.Get(context.TODO(), types.NamespacedName{Name: defaultName, Namespace: v1.NamespaceAll}, scc) + if err != nil { + return fmt.Errorf("failed to get scc: %w", err) + } + + scc.Users = removeStringValue(scc.Users, serviceAccountName(namespace, name)) + + err = client.Update(context.TODO(), scc) + if err != nil { + return fmt.Errorf("failed to update scc: %w", err) + } + return nil +} + +func removeStringValue(values []string, value string) []string { + var filtered []string + for _, v := range values { + if v != value { + filtered = append(filtered, v) + } + } + return filtered +} diff --git a/controllers/scc_test.go b/controllers/scc/scc_test.go similarity index 80% rename from controllers/scc_test.go rename to controllers/scc/scc_test.go index fc74dbaa..8b912b4a 100644 --- a/controllers/scc_test.go +++ b/controllers/scc/scc_test.go @@ -1,4 +1,4 @@ -package controllers +package scc import ( "fmt" @@ -12,12 +12,11 @@ import ( func TestSccForNginxIngressController(t *testing.T) { var uid int64 = 101 - name := "my-nginx-ingress" allowPrivilegeEscalation := true expected := &secv1.SecurityContextConstraints{ ObjectMeta: v1.ObjectMeta{ - Name: name, + Name: "nginx-ingress-scc", }, AllowHostPorts: false, AllowPrivilegedContainer: false, @@ -47,19 +46,19 @@ func TestSccForNginxIngressController(t *testing.T) { AllowedCapabilities: nil, } - result := sccForNginxIngressController(name) + result := sccConfigTemplate() if diff := cmp.Diff(expected, result); diff != "" { - t.Errorf("sccForNginxIngressController() mismatch (-want +got):\n%s", diff) + t.Errorf("sccConfigTemplate() mismatch (-want +got):\n%s", diff) } } -func TestUserForSCC(t *testing.T) { +func TestServiceAccountName(t *testing.T) { namespace := "my-nginx-ingress" name := "my-nginx-ingress-controller" expected := fmt.Sprintf("system:serviceaccount:%v:%v", namespace, name) - result := userForSCC(namespace, name) + result := serviceAccountName(namespace, name) if expected != result { - t.Errorf("userForSCC(%v, %v) returned %v but expected %v", namespace, name, result, expected) + t.Errorf("serviceAccountName(%v, %v) returned %v but expected %v", namespace, name, result, expected) } }