Merge pull request #731 from humio/mike/cleanup_scc_logic

openshift: Remove built-in management of SCC

charts/humio-operator/templates/operator-deployment.yaml

@@ -37,15 +37,15 @@ spec:
 {{- with .Values.operator.nodeSelector }}
       nodeSelector:
         {{- toYaml . | nindent 8 }}
-{{- end }}  
+{{- end }}
 {{- with .Values.operator.affinity }}
       affinity:
         {{- toYaml . | nindent 8 }}
-{{- end }}        
+{{- end }}
 {{- with .Values.operator.tolerations }}
       tolerations:
         {{- toYaml . | nindent 8 }}
-{{- end }}  
+{{- end }}
       serviceAccountName: {{ .Release.Name }}
       containers:
       - name: humio-operator
@@ -64,10 +64,6 @@ spec:
           value: "humio-operator"
         - name: USE_CERTMANAGER
           value: {{ .Values.certmanager | quote }}
-{{- if .Values.openshift }}
-        - name: OPENSHIFT_SCC_NAME
-          value: '{{ default "default" .Release.Namespace }}-{{ .Release.Name }}'
-{{- end }}
         livenessProbe:
           httpGet:
             path: /metrics

charts/humio-operator/templates/operator-rbac.yaml

@@ -317,25 +317,6 @@ rules:
   - get
   - list
   - watch
-{{- if .Values.openshift }}
-- apiGroups:
-  - security.openshift.io
-  resourceNames:
-  - '{{ default "default" .Release.Namespace }}-{{ .Release.Name }}'
-  resources:
-  - securitycontextconstraints
-  verbs:
-  - use
-  - update
-- apiGroups:
-  - security.openshift.io
-  resources:
-  - securitycontextconstraints
-  verbs:
-  - get
-  - list
-  - watch
-{{- end }}
 
 ---
 
@@ -354,47 +335,4 @@ roleRef:
   name: '{{ default "default" .Release.Namespace }}-{{ .Release.Name }}'
   apiGroup: rbac.authorization.k8s.io
 
-{{- if .Values.openshift }}
----
-apiVersion: security.openshift.io/v1
-kind: SecurityContextConstraints
-metadata:
-  name: '{{ default "default" .Release.Namespace }}-{{ .Release.Name }}'
-  labels:
-    {{- $commonLabels | nindent 4 }}
-allowPrivilegedContainer: true
-allowHostDirVolumePlugin: true
-allowHostIPC: false
-allowHostNetwork: false
-allowHostPID: false
-allowHostPorts: false
-priority: 0
-allowedCapabilities:
-- SYS_NICE
-readOnlyRootFilesystem: true
-requiredDropCapabilities:
-- KILL
-- MKNOD
-- SETUID
-- SETGID
-defaultAddCapabilities: []
-runAsUser:
-  type: RunAsAny
-seLinuxContext:
-  type: MustRunAs
-fsGroup:
-  type: RunAsAny
-supplementalGroups:
-  type: RunAsAny
-volumes:
-- configMap
-- downwardAPI
-- emptyDir
-- hostPath
-- persistentVolumeClaim
-- projected
-- secret
-users: []
-{{- end }}
-
 {{- end }}

charts/humio-operator/values.yaml

@@ -39,5 +39,4 @@ operator:
             values:
             - linux
 
-openshift: false
 certmanager: true

controllers/humiocluster_controller.go

@@ -28,7 +28,6 @@ import (
 	humioapi "github.com/humio/cli/api"
 	"github.com/humio/humio-operator/pkg/helpers"
 	"github.com/humio/humio-operator/pkg/kubernetes"
-	"github.com/humio/humio-operator/pkg/openshift"
 	corev1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
@@ -340,7 +339,6 @@ func (r *HumioClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request
 	}
 
 	for _, fun := range []ctxHumioClusterFunc{
-		r.cleanupUsersInSecurityContextConstraints,
 		r.cleanupUnusedTLSCertificates,
 		r.cleanupUnusedTLSSecrets,
 		r.cleanupUnusedCAIssuer,
@@ -807,12 +805,6 @@ func (r *HumioClusterReconciler) ensureHumioPodPermissions(ctx context.Context,
 		return r.logErrorAndReturn(err, "unable to ensure humio service account exists")
 	}
 
-	// In cases with OpenShift, we must ensure our ServiceAccount has access to the SecurityContextConstraint
-	if helpers.IsOpenShift() {
-		if err := r.ensureSecurityContextConstraintsContainsServiceAccount(ctx, hnp.GetNamespace(), hnp.GetInitServiceAccountName()); err != nil {
-			return r.logErrorAndReturn(err, "could not ensure SecurityContextConstraints contains ServiceAccount")
-		}
-	}
 	return nil
 }
 
@@ -862,13 +854,6 @@ func (r *HumioClusterReconciler) ensureInitContainerPermissions(ctx context.Cont
 		return r.logErrorAndReturn(err, "unable to ensure init cluster role binding exists")
 	}
 
-	// In cases with OpenShift, we must ensure our ServiceAccount has access to the SecurityContextConstraint
-	if helpers.IsOpenShift() {
-		if err := r.ensureSecurityContextConstraintsContainsServiceAccount(ctx, hnp.GetNamespace(), hnp.GetInitServiceAccountName()); err != nil {
-			return r.logErrorAndReturn(err, "could not ensure SecurityContextConstraints contains ServiceAccount")
-		}
-	}
-
 	return nil
 }
 
@@ -905,77 +890,6 @@ func (r *HumioClusterReconciler) ensureAuthContainerPermissions(ctx context.Cont
 		return r.logErrorAndReturn(err, "unable to ensure auth role binding exists")
 	}
 
-	// In cases with OpenShift, we must ensure our ServiceAccount has access to the SecurityContextConstraint
-	if helpers.IsOpenShift() {
-		if err := r.ensureSecurityContextConstraintsContainsServiceAccount(ctx, hnp.GetNamespace(), hnp.GetAuthServiceAccountName()); err != nil {
-			return r.logErrorAndReturn(err, "could not ensure SecurityContextConstraints contains ServiceAccount")
-		}
-	}
-
-	return nil
-}
-
-func (r *HumioClusterReconciler) ensureSecurityContextConstraintsContainsServiceAccount(ctx context.Context, namespace, serviceAccountName string) error {
-	// TODO: Write unit/e2e test for this
-
-	if !helpers.IsOpenShift() {
-		return fmt.Errorf("updating SecurityContextConstraints are only suppoted when running on OpenShift")
-	}
-
-	// Get current SCC
-	scc, err := openshift.GetSecurityContextConstraints(ctx, r)
-	if err != nil {
-		return r.logErrorAndReturn(err, "unable to get details about SecurityContextConstraints")
-	}
-
-	// Give ServiceAccount access to SecurityContextConstraints if not already present
-	usersEntry := fmt.Sprintf("system:serviceaccount:%s:%s", namespace, serviceAccountName)
-	if !helpers.ContainsElement(scc.Users, usersEntry) {
-		scc.Users = append(scc.Users, usersEntry)
-		err = r.Update(ctx, scc)
-		if err != nil {
-			return r.logErrorAndReturn(err, fmt.Sprintf("could not update SecurityContextConstraints %s to add ServiceAccount %s", scc.Name, serviceAccountName))
-		}
-	}
-	return nil
-}
-
-// Ensure the users in the SCC are cleaned up.
-// This cleanup is only called as part of reconciling HumioCluster objects,
-// this means that you can end up with the SCC listing the service accounts
-// used for the last cluster to be deleted, in the case that all HumioCluster's are removed.
-// TODO: Determine if we should move this to a finalizer to fix the situation described above.
-func (r *HumioClusterReconciler) cleanupUsersInSecurityContextConstraints(ctx context.Context, _ *humiov1alpha1.HumioCluster) error {
-	if !helpers.IsOpenShift() {
-		return nil
-	}
-
-	scc, err := openshift.GetSecurityContextConstraints(ctx, r)
-	if err != nil {
-		return r.logErrorAndReturn(err, "unable to get details about SecurityContextConstraints")
-	}
-
-	for _, userEntry := range scc.Users {
-		sccUserData := strings.Split(userEntry, ":")
-		sccUserNamespace := sccUserData[2]
-		sccUserName := sccUserData[3]
-
-		_, err := kubernetes.GetServiceAccount(ctx, r, sccUserName, sccUserNamespace)
-		if err == nil {
-			// We found an existing service account
-			continue
-		}
-		if k8serrors.IsNotFound(err) {
-			// Remove the entry from the list if the servicea doesn't exist
-			scc.Users = helpers.RemoveElement(scc.Users, fmt.Sprintf("system:serviceaccount:%s:%s", sccUserNamespace, sccUserName))
-			if err = r.Update(ctx, scc); err != nil {
-				return r.logErrorAndReturn(err, "unable to update SecurityContextConstraints")
-			}
-		} else {
-			return r.logErrorAndReturn(err, "unable to get existing service account")
-		}
-	}
-
 	return nil
 }
 

controllers/suite/clusters/suite_test.go

@@ -39,7 +39,6 @@ import (
 	"github.com/go-logr/logr"
 	"github.com/go-logr/zapr"
 	humioapi "github.com/humio/cli/api"
-	openshiftsecurityv1 "github.com/openshift/api/security/v1"
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -49,7 +48,6 @@ import (
 
 	"github.com/humio/humio-operator/pkg/helpers"
 	"github.com/humio/humio-operator/pkg/humio"
-	"github.com/humio/humio-operator/pkg/openshift"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -142,11 +140,6 @@ var _ = BeforeSuite(func() {
 	}, 30*time.Second, 5*time.Second).Should(Succeed())
 	Expect(cfg).NotTo(BeNil())
 
-	if helpers.IsOpenShift() {
-		err = openshiftsecurityv1.AddToScheme(scheme.Scheme)
-		Expect(err).NotTo(HaveOccurred())
-	}
-
 	if helpers.UseCertManager() {
 		err = cmapi.AddToScheme(scheme.Scheme)
 		Expect(err).NotTo(HaveOccurred())
@@ -260,81 +253,6 @@ var _ = BeforeSuite(func() {
 	Expect(err).ToNot(HaveOccurred())
 
 	suite.CreateDockerRegredSecret(context.TODO(), testNamespace, k8sClient)
-
-	if helpers.IsOpenShift() {
-		var err error
-		ctx := context.Background()
-		Eventually(func() bool {
-			_, err = openshift.GetSecurityContextConstraints(ctx, k8sClient)
-			if k8serrors.IsNotFound(err) {
-				// Object has not been created yet
-				return true
-			}
-			if err != nil {
-				// Some other error happened. Typically:
-				//   <*cache.ErrCacheNotStarted | 0x31fc738>: {}
-				//         the cache is not started, can not read objects occurred
-				return false
-			}
-			// At this point we know the object already exists.
-			return true
-		}, testTimeout, suite.TestInterval).Should(BeTrue())
-		if k8serrors.IsNotFound(err) {
-			By("Simulating helm chart installation of the SecurityContextConstraints object")
-			sccName := os.Getenv("OPENSHIFT_SCC_NAME")
-			priority := int32(0)
-			scc := openshiftsecurityv1.SecurityContextConstraints{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      sccName,
-					Namespace: testProcessNamespace,
-				},
-				Priority:                 &priority,
-				AllowPrivilegedContainer: true,
-				DefaultAddCapabilities:   []corev1.Capability{},
-				RequiredDropCapabilities: []corev1.Capability{
-					"KILL",
-					"MKNOD",
-					"SETUID",
-					"SETGID",
-				},
-				AllowedCapabilities: []corev1.Capability{
-					"SYS_NICE",
-				},
-				AllowHostDirVolumePlugin: true,
-				Volumes: []openshiftsecurityv1.FSType{
-					openshiftsecurityv1.FSTypeConfigMap,
-					openshiftsecurityv1.FSTypeDownwardAPI,
-					openshiftsecurityv1.FSTypeEmptyDir,
-					openshiftsecurityv1.FSTypeHostPath,
-					openshiftsecurityv1.FSTypePersistentVolumeClaim,
-					openshiftsecurityv1.FSProjected,
-					openshiftsecurityv1.FSTypeSecret,
-				},
-				AllowedFlexVolumes: nil,
-				AllowHostNetwork:   false,
-				AllowHostPorts:     false,
-				AllowHostPID:       false,
-				AllowHostIPC:       false,
-				SELinuxContext: openshiftsecurityv1.SELinuxContextStrategyOptions{
-					Type: openshiftsecurityv1.SELinuxStrategyMustRunAs,
-				},
-				RunAsUser: openshiftsecurityv1.RunAsUserStrategyOptions{
-					Type: openshiftsecurityv1.RunAsUserStrategyRunAsAny,
-				},
-				SupplementalGroups: openshiftsecurityv1.SupplementalGroupsStrategyOptions{
-					Type: openshiftsecurityv1.SupplementalGroupsStrategyRunAsAny,
-				},
-				FSGroup: openshiftsecurityv1.FSGroupStrategyOptions{
-					Type: openshiftsecurityv1.FSGroupStrategyRunAsAny,
-				},
-				ReadOnlyRootFilesystem: false,
-				Users:                  []string{},
-				Groups:                 nil,
-				SeccompProfiles:        nil,
-			}
-			Expect(k8sClient.Create(ctx, &scc)).To(Succeed())
-		}
-	}
 })
 
 var _ = AfterSuite(func() {