feat: implement support for OpenShift wrt service account (#1013)

* feat: implement support for OpenShift wrt service account

Curently both serviceAccount and serviceAccountName must
be set on a Pod/Deployment/StatefulSet.

More details in the documentation:
https://docs.redhat.com/en/documentation/openshift_container_platform/4.17/html/building_applications/deployments#deployments-running-pod-svc-acct_deployment-operations

* refactor: rewrite cluster type detection to avoid boolean flag parameters

* fix(controller): update image for oauth2-proxy

The original bitnami image has been phased out.

* refactor: ClusterType rather than K8sVariant

Co-authored-by: Flora Thiebaut <flora.thiebaut@sdsc.ethz.ch>

* refactor: remove Unknown cluster type

In case of detection failure, it's the error
that is important not the string value.

* chore: make the cluster detection information more generic

---------

Co-authored-by: Flora Thiebaut <flora.thiebaut@sdsc.ethz.ch>

Author: sgaist

api/v1alpha1/amaltheasession_children.go

@@ -87,7 +87,7 @@ func (cr *AmaltheaSession) SessionVolumes() ([]v1.Volume, []v1.VolumeMount) {
 }
 
 // StatefulSet returns a AmaltheaSession StatefulSet object
-func (cr *AmaltheaSession) StatefulSet() (appsv1.StatefulSet, error) {
+func (cr *AmaltheaSession) StatefulSet(clusterType ClusterType) (appsv1.StatefulSet, error) {
 	labels := labelsForAmaltheaSession(cr.Name)
 	replicas := int32(1)
 	if cr.Spec.Hibernated {
@@ -179,6 +179,25 @@ func (cr *AmaltheaSession) StatefulSet() (appsv1.StatefulSet, error) {
 		imagePullSecrets = append(imagePullSecrets, v1.LocalObjectReference{Name: sec.Name})
 	}
 
+	pod := v1.PodSpec{
+		ServiceAccountName:           cr.Spec.ServiceAccountName,
+		EnableServiceLinks:           ptr.To(false),
+		AutomountServiceAccountToken: ptr.To(false),
+		SecurityContext:              &v1.PodSecurityContext{FSGroup: &cr.Spec.Session.RunAsGroup},
+		Containers:                   containers,
+		InitContainers:               initContainers,
+		Volumes:                      volumes,
+		Tolerations:                  cr.Spec.Tolerations,
+		NodeSelector:                 cr.Spec.NodeSelector,
+		Affinity:                     cr.Spec.Affinity,
+		PriorityClassName:            cr.Spec.PriorityClassName,
+		ImagePullSecrets:             imagePullSecrets,
+	}
+
+	if clusterType == OpenShift {
+		pod.DeprecatedServiceAccount = pod.ServiceAccountName
+	}
+
 	sts := appsv1.StatefulSet{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      cr.Name,
@@ -196,20 +215,7 @@ func (cr *AmaltheaSession) StatefulSet() (appsv1.StatefulSet, error) {
 				ObjectMeta: metav1.ObjectMeta{
 					Labels: labels,
 				},
-				Spec: v1.PodSpec{
-					ServiceAccountName:           cr.Spec.ServiceAccountName,
-					EnableServiceLinks:           ptr.To(false),
-					AutomountServiceAccountToken: ptr.To(false),
-					SecurityContext:              &v1.PodSecurityContext{FSGroup: &cr.Spec.Session.RunAsGroup},
-					Containers:                   containers,
-					InitContainers:               initContainers,
-					Volumes:                      volumes,
-					Tolerations:                  cr.Spec.Tolerations,
-					NodeSelector:                 cr.Spec.NodeSelector,
-					Affinity:                     cr.Spec.Affinity,
-					PriorityClassName:            cr.Spec.PriorityClassName,
-					ImagePullSecrets:             imagePullSecrets,
-				},
+				Spec: pod,
 			},
 		},
 	}

api/v1alpha1/detection.go

@@ -0,0 +1,32 @@
+package v1alpha1
+
+import (
+	"k8s.io/client-go/discovery"
+	"k8s.io/client-go/rest"
+)
+
+type ClusterType string
+
+const Kubernetes ClusterType = "kubernetes"
+const OpenShift ClusterType = "openshift"
+
+func DetectClusterType(config *rest.Config) (ClusterType, error) {
+	dcl, err := discovery.NewDiscoveryClientForConfig(config)
+	if err != nil {
+		return "", err
+	}
+
+	apiList, err := dcl.ServerGroups()
+	if err != nil {
+		return "", err
+	}
+
+	apiGroups := apiList.Groups
+	for i := range apiGroups {
+		if apiGroups[i].Name == "project.openshift.io" {
+			return OpenShift, nil
+		}
+	}
+
+	return Kubernetes, nil
+}

api/v1alpha1/detection_test.go

@@ -0,0 +1,51 @@
+package v1alpha1
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/rest"
+)
+
+func TestDetectPlatformBasedOnAvailableAPIGroups(t *testing.T) {
+	for _, tt := range []struct {
+		apiGroupList *metav1.APIGroupList
+		expected     ClusterType
+	}{
+		{
+			&metav1.APIGroupList{},
+			Kubernetes,
+		},
+		{
+			&metav1.APIGroupList{
+				Groups: []metav1.APIGroup{
+					{
+						Name: "project.openshift.io",
+					},
+				},
+			},
+			OpenShift,
+		},
+	} {
+		server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+			output, err := json.Marshal(tt.apiGroupList)
+			assert.NoError(t, err)
+
+			w.Header().Set("Content-Type", "application/json")
+			w.WriteHeader(http.StatusOK)
+			_, err = w.Write(output)
+			assert.NoError(t, err)
+		}))
+		defer server.Close()
+
+		clusterType, err := DetectClusterType(&rest.Config{Host: server.URL})
+		require.NoError(t, err)
+
+		assert.Equal(t, tt.expected, clusterType)
+	}
+}

cmd/amalthea/main.go

@@ -170,10 +170,20 @@ func main() {
 
 	metricsClient := metricsv.NewForConfigOrDie(config).MetricsV1beta1()
 
+	clusterType, err := amaltheadevv1alpha1.DetectClusterType(config)
+
+	if err != nil {
+		setupLog.Error(err, "failed to do cluster detection")
+		os.Exit(1)
+	}
+
+	setupLog.Info("cluster type detected", "type", clusterType)
+
 	err = (&controller.AmaltheaSessionReconciler{
 		Client:        mgr.GetClient(),
 		Scheme:        mgr.GetScheme(),
 		MetricsClient: metricsClient,
+		ClusterType:   clusterType,
 	}).SetupWithManager(mgr)
 
 	if err != nil {

controller/templates/statefulset.yaml

@@ -152,7 +152,7 @@ spec:
 
         {% if oidc["enabled"] %}
         - name: oauth2-proxy
-          image: "bitnami/oauth2-proxy:7.4.0"
+          image: "harbor.renkulab.io/bitnami-mirror/oauth2-proxy:7.6.0"
           securityContext:
             allowPrivilegeEscalation: false
             runAsNonRoot: true

internal/controller/amaltheasession_controller.go

@@ -43,6 +43,7 @@ type AmaltheaSessionReconciler struct {
 	client.Client
 	Scheme        *runtime.Scheme
 	MetricsClient metricsv1beta1.PodMetricsesGetter
+	ClusterType   amaltheadevv1alpha1.ClusterType
 }
 
 // finalizers
@@ -145,7 +146,7 @@ func (r *AmaltheaSessionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 
 	log.Info("spec", "cr", amaltheasession)
 
-	children, err := NewChildResources(amaltheasession)
+	children, err := NewChildResources(amaltheasession, r.ClusterType)
 	if err != nil {
 		log.Error(
 			err,
@@ -157,6 +158,7 @@ func (r *AmaltheaSessionReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		)
 		return ctrl.Result{}, err
 	}
+
 	updates, err := children.Reconcile(ctx, r.Client, amaltheasession)
 	if err != nil {
 		log.Error(err, "Failed when reconciling children")

internal/controller/children.go

@@ -255,12 +255,12 @@ func (c ChildResource[T]) Reconcile(ctx context.Context, clnt client.Client, cr
 	}
 }
 
-func NewChildResources(cr *amaltheadevv1alpha1.AmaltheaSession) (ChildResources, error) {
+func NewChildResources(cr *amaltheadevv1alpha1.AmaltheaSession, clusterType amaltheadevv1alpha1.ClusterType) (ChildResources, error) {
 	metadata := metav1.ObjectMeta{Name: cr.Name, Namespace: cr.Namespace}
 	secretMetadata := metav1.ObjectMeta{Name: cr.InternalSecretName(), Namespace: cr.Namespace}
 	desiredService := cr.Service()
 	desiredPVC := cr.PVC()
-	desiredStatefulSet, err := cr.StatefulSet()
+	desiredStatefulSet, err := cr.StatefulSet(clusterType)
 	if err != nil {
 		return ChildResources{}, err
 	}