NET-3908: allow configuration of SecurityContextConstraints when runn…

…ing on OpenShift (#2184) Co-authored-by: Melisa Griffin <[email protected]>
hashicorp · Aug 8, 2023 · cf0ce26 · cf0ce26
1 parent f2f8bf7
commit cf0ce26
Show file tree

Hide file tree

Showing 19 changed files with 180 additions and 29 deletions.
diff --git a/.changelog/2184.txt b/.changelog/2184.txt
@@ -0,0 +1,3 @@
+```release-note:feature
+api-gateway: support deploying to OpenShift 4.11
+```
diff --git a/charts/consul/templates/connect-inject-clusterrole.yaml b/charts/consul/templates/connect-inject-clusterrole.yaml
@@ -186,4 +186,14 @@ rules:
     - "get"
     - "list"
     - "watch"
+{{- if .Values.global.openshift.enabled }}
+- apiGroups:
+    - security.openshift.io
+  resources:
+    - securitycontextconstraints
+  resourceNames:
+    - {{ .Values.connectInject.apiGateway.managedGatewayClass.openshiftSCCName }}
+  verbs:
+    - use
+  {{- end }}
 {{- end }}
diff --git a/charts/consul/templates/crd-gatewayclassconfigs.yaml b/charts/consul/templates/crd-gatewayclassconfigs.yaml
@@ -138,6 +138,10 @@ spec:
                       type: string
                   type: object
                 type: array
+              openshiftSCCName:
+                description: The name of an existing SecurityContextConstraints
+                  resource to bind to the managed role when running on OpenShift.
+                type: string
             type: object
         type: object
     served: true

diff --git a/charts/consul/templates/gateway-resources-job.yaml b/charts/consul/templates/gateway-resources-job.yaml
@@ -99,6 +99,9 @@ spec:
             - {{- toYaml .Values.connectInject.apiGateway.managedGatewayClass.copyAnnotations.service.annotations | nindent 14 -}}
             {{- end }}
             - -service-type={{ .Values.connectInject.apiGateway.managedGatewayClass.serviceType }}
+            {{- if .Values.global.openshift.enabled  }}
+            - -openshift-scc-name={{ .Values.connectInject.apiGateway.managedGatewayClass.openshiftSCCName }}
+            {{- end }}
             {{- end}}
           resources:
             requests:

diff --git a/charts/consul/test/unit/connect-inject-clusterrole.bats b/charts/consul/test/unit/connect-inject-clusterrole.bats
@@ -217,3 +217,27 @@ load _helpers
   local actual=$(echo $object | yq -r '.verbs | index("watch")' | tee /dev/stderr)
   [ "${actual}" != null ]
 }
+
+#--------------------------------------------------------------------
+# openshift
+
+@test "connectInject/ClusterRole: adds permission to securitycontextconstraints for Openshift with global.openshift.enabled=true with default apiGateway Openshift SCC Name" {
+  cd `chart_dir`
+  local object=$(helm template \
+      -s templates/connect-inject-clusterrole.yaml  \
+      --set 'global.openshift.enabled=true' \
+      . | tee /dev/stderr |
+      yq '.rules[13].resourceNames | index("restricted-v2")' | tee /dev/stderr)
+  [ "${object}" == 0 ]
+}
+
+@test "connectInject/ClusterRole: adds permission to securitycontextconstraints for Openshift with global.openshift.enabled=true and sets apiGateway Openshift SCC Name" {
+  cd `chart_dir`
+  local object=$(helm template \
+      -s templates/connect-inject-clusterrole.yaml  \
+      --set 'global.openshift.enabled=true' \
+      --set 'connectInject.apiGateway.managedGatewayClass.openshiftSCCName=fakescc' \
+      . | tee /dev/stderr |
+       yq '.rules[13].resourceNames | index("fakescc")' | tee /dev/stderr)
+   [ "${object}" == 0 ]
+}
diff --git a/charts/consul/test/unit/gateway-resources-job.bats b/charts/consul/test/unit/gateway-resources-job.bats
@@ -92,6 +92,7 @@ target=templates/gateway-resources-job.yaml
       --set 'connectInject.apiGateway.managedGatewayClass.tolerations=- key: bar' \
       --set 'connectInject.apiGateway.managedGatewayClass.copyAnnotations.service.annotations=- bingo' \
       --set 'connectInject.apiGateway.managedGatewayClass.serviceType=Foo' \
+      --set 'connectInject.apiGateway.managedGatewayClass.openshiftSCCName=hello' \
       . | tee /dev/stderr |
       yq '.spec.template.spec.containers[0].args' | tee /dev/stderr)
 
@@ -121,6 +122,22 @@ target=templates/gateway-resources-job.yaml
 
   local actual=$(echo "$spec" | jq '.[16]')
   [ "${actual}" = "\"- bingo\"" ]
+
+  local actual=$(echo "$spec" | jq '.[17]')
+  [ "${actual}" = "\"-service-type=Foo\"" ]
+}
+
+@test "apiGateway/GatewayClassConfig: custom configuration openshift enabled" {
+  cd `chart_dir`
+  local spec=$(helm template \
+      -s $target  \
+      --set 'global.openshift.enabled=true' \
+      --set 'connectInject.apiGateway.managedGatewayClass.openshiftSCCName=hello' \
+      . | tee /dev/stderr |
+      yq '.spec.template.spec.containers[0].args' | tee /dev/stderr)
+
+  local actual=$(echo "$spec" | jq '.[13]')
+  [ "${actual}" = "\"-openshift-scc-name=hello\"" ]
 }
 
 

diff --git a/charts/consul/values.yaml b/charts/consul/values.yaml
@@ -2201,6 +2201,11 @@ connectInject:
         maxInstances: 1
         minInstances: 1
 
+      # The name of the OpenShift SecurityContextConstraints resource to use for Gateways.
+      # Only applicable if `global.openshift.enabled` is true.
+      # @type: string
+      openshiftSCCName: "restricted-v2"
+
     # Configuration for the ServiceAccount created for the api-gateway component
     serviceAccount:
       # This value defines additional annotations for the client service account. This should be formatted as a multi-line

diff --git a/cli/helm/values.go b/cli/helm/values.go
@@ -576,11 +576,12 @@ type CopyAnnotations struct {
 }
 
 type ManagedGatewayClass struct {
-	Enabled         bool            `yaml:"enabled"`
-	NodeSelector    interface{}     `yaml:"nodeSelector"`
-	ServiceType     string          `yaml:"serviceType"`
-	UseHostPorts    bool            `yaml:"useHostPorts"`
-	CopyAnnotations CopyAnnotations `yaml:"copyAnnotations"`
+	Enabled          bool            `yaml:"enabled"`
+	NodeSelector     interface{}     `yaml:"nodeSelector"`
+	ServiceType      string          `yaml:"serviceType"`
+	UseHostPorts     bool            `yaml:"useHostPorts"`
+	CopyAnnotations  CopyAnnotations `yaml:"copyAnnotations"`
+	OpenshiftSCCName string          `yaml:"openshiftSCCName"`
 }
 
 type Service struct {

diff --git a/control-plane/api-gateway/common/helm_config.go b/control-plane/api-gateway/common/helm_config.go
@@ -11,14 +11,14 @@ import (
 const componentAuthMethod = "k8s-component-auth-method"
 
 // HelmConfig is the configuration of gateways that comes in from the user's Helm values.
+// This is a combination of the apiGateway stanza and other settings that impact api-gateways.
 type HelmConfig struct {
 	// ImageDataplane is the Consul Dataplane image to use in gateway deployments.
 	ImageDataplane             string
 	ImageConsulK8S             string
 	ConsulDestinationNamespace string
 	NamespaceMirroringPrefix   string
 	EnableNamespaces           bool
-	EnableOpenShift            bool
 	EnableNamespaceMirroring   bool
 	AuthMethod                 string
 	// LogLevel is the logging level of the deployed Consul Dataplanes.
@@ -30,6 +30,10 @@ type HelmConfig struct {
 	ConsulTLSServerName string
 	ConsulCACert        string
 	ConsulConfig        ConsulConfig
+
+	// EnableOpenShift indicates whether we're deploying into an OpenShift environment
+	// and should create SecurityContextConstraints.
+	EnableOpenShift bool
 }
 
 type ConsulConfig struct {

diff --git a/control-plane/api-gateway/gatekeeper/gatekeeper.go b/control-plane/api-gateway/gatekeeper/gatekeeper.go
@@ -96,7 +96,7 @@ func (g *Gatekeeper) namespacedName(gateway gwv1beta1.Gateway) types.NamespacedN
 }
 
 func (g *Gatekeeper) serviceAccountName(gateway gwv1beta1.Gateway, config common.HelmConfig) string {
-	if config.AuthMethod == "" {
+	if config.AuthMethod == "" && !config.EnableOpenShift {
 		return ""
 	}
 	return gateway.Name

diff --git a/control-plane/api-gateway/gatekeeper/gatekeeper_test.go b/control-plane/api-gateway/gatekeeper/gatekeeper_test.go
@@ -193,7 +193,7 @@ func TestUpsert(t *testing.T) {
 					configureDeployment(name, namespace, labels, 3, nil, nil, "", "1"),
 				},
 				roles: []*rbac.Role{
-					configureRole(name, namespace, labels, "1"),
+					configureRole(name, namespace, labels, "1", false),
 				},
 				roleBindings: []*rbac.RoleBinding{
 					configureRoleBinding(name, namespace, labels, "1"),
@@ -319,7 +319,7 @@ func TestUpsert(t *testing.T) {
 					configureDeployment(name, namespace, labels, 3, nil, nil, "", "1"),
 				},
 				roles: []*rbac.Role{
-					configureRole(name, namespace, labels, "1"),
+					configureRole(name, namespace, labels, "1", false),
 				},
 				roleBindings: []*rbac.RoleBinding{
 					configureRoleBinding(name, namespace, labels, "1"),
@@ -342,7 +342,7 @@ func TestUpsert(t *testing.T) {
 					configureDeployment(name, namespace, labels, 3, nil, nil, "", "2"),
 				},
 				roles: []*rbac.Role{
-					configureRole(name, namespace, labels, "1"),
+					configureRole(name, namespace, labels, "1", false),
 				},
 				roleBindings: []*rbac.RoleBinding{
 					configureRoleBinding(name, namespace, labels, "1"),
@@ -400,7 +400,7 @@ func TestUpsert(t *testing.T) {
 					configureDeployment(name, namespace, labels, 3, nil, nil, "", "1"),
 				},
 				roles: []*rbac.Role{
-					configureRole(name, namespace, labels, "1"),
+					configureRole(name, namespace, labels, "1", false),
 				},
 				roleBindings: []*rbac.RoleBinding{
 					configureRoleBinding(name, namespace, labels, "1"),
@@ -428,7 +428,7 @@ func TestUpsert(t *testing.T) {
 					configureDeployment(name, namespace, labels, 3, nil, nil, "", "2"),
 				},
 				roles: []*rbac.Role{
-					configureRole(name, namespace, labels, "1"),
+					configureRole(name, namespace, labels, "1", false),
 				},
 				roleBindings: []*rbac.RoleBinding{
 					configureRoleBinding(name, namespace, labels, "1"),
@@ -603,6 +603,50 @@ func TestUpsert(t *testing.T) {
 				serviceAccounts: []*corev1.ServiceAccount{},
 			},
 		},
+		"create a new gateway with openshift enabled": {
+			gateway: gwv1beta1.Gateway{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      name,
+					Namespace: namespace,
+				},
+				Spec: gwv1beta1.GatewaySpec{
+					Listeners: listeners,
+				},
+			},
+			gatewayClassConfig: v1alpha1.GatewayClassConfig{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "consul-gatewayclassconfig",
+				},
+				Spec: v1alpha1.GatewayClassConfigSpec{
+					DeploymentSpec: v1alpha1.DeploymentSpec{
+						DefaultInstances: common.PointerTo(int32(3)),
+						MaxInstances:     common.PointerTo(int32(3)),
+						MinInstances:     common.PointerTo(int32(1)),
+					},
+					CopyAnnotations:  v1alpha1.CopyAnnotationsSpec{},
+					OpenshiftSCCName: "test-api-gateway",
+				},
+			},
+			helmConfig: common.HelmConfig{
+				EnableOpenShift: true,
+			},
+			initialResources: resources{},
+			finalResources: resources{
+				deployments: []*appsv1.Deployment{
+					configureDeployment(name, namespace, labels, 3, nil, nil, "", "1"),
+				},
+				roles: []*rbac.Role{
+					configureRole(name, namespace, labels, "1", true),
+				},
+				roleBindings: []*rbac.RoleBinding{
+					configureRoleBinding(name, namespace, labels, "1"),
+				},
+				services: []*corev1.Service{},
+				serviceAccounts: []*corev1.ServiceAccount{
+					configureServiceAccount(name, namespace, labels, "1"),
+				},
+			},
+		},
 	}
 
 	for name, tc := range cases {
@@ -754,7 +798,7 @@ func TestDelete(t *testing.T) {
 					configureDeployment(name, namespace, labels, 3, nil, nil, "", "1"),
 				},
 				roles: []*rbac.Role{
-					configureRole(name, namespace, labels, "1"),
+					configureRole(name, namespace, labels, "1", false),
 				},
 				roleBindings: []*rbac.RoleBinding{
 					configureRoleBinding(name, namespace, labels, "1"),
@@ -1057,7 +1101,19 @@ func configureDeployment(name, namespace string, labels map[string]string, repli
 	}
 }
 
-func configureRole(name, namespace string, labels map[string]string, resourceVersion string) *rbac.Role {
+func configureRole(name, namespace string, labels map[string]string, resourceVersion string, openshiftEnabled bool) *rbac.Role {
+	rules := []rbac.PolicyRule{}
+
+	if openshiftEnabled {
+		rules = []rbac.PolicyRule{
+			{
+				APIGroups:     []string{"security.openshift.io"},
+				Resources:     []string{"securitycontextconstraints"},
+				ResourceNames: []string{name + "-api-gateway"},
+				Verbs:         []string{"use"},
+			},
+		}
+	}
 	return &rbac.Role{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "rbac.authorization.k8s.io/v1",
@@ -1078,7 +1134,7 @@ func configureRole(name, namespace string, labels map[string]string, resourceVer
 				},
 			},
 		},
-		Rules: []rbac.PolicyRule{},
+		Rules: rules,
 	}
 }
 

diff --git a/control-plane/api-gateway/gatekeeper/init.go b/control-plane/api-gateway/gatekeeper/init.go
@@ -168,14 +168,17 @@ func initContainer(config common.HelmConfig, name, namespace string) (corev1.Con
 			})
 	}
 
-	container.SecurityContext = &corev1.SecurityContext{
-		RunAsUser:    pointer.Int64(initContainersUserAndGroupID),
-		RunAsGroup:   pointer.Int64(initContainersUserAndGroupID),
-		RunAsNonRoot: pointer.Bool(true),
-		Privileged:   pointer.Bool(false),
-		Capabilities: &corev1.Capabilities{
-			Drop: []corev1.Capability{"ALL"},
-		},
+	// Openshift Assigns the security context for us, do not enable if it is enabled.
+	if !config.EnableOpenShift {
+		container.SecurityContext = &corev1.SecurityContext{
+			RunAsUser:    pointer.Int64(initContainersUserAndGroupID),
+			RunAsGroup:   pointer.Int64(initContainersUserAndGroupID),
+			RunAsNonRoot: pointer.Bool(true),
+			Privileged:   pointer.Bool(false),
+			Capabilities: &corev1.Capabilities{
+				Drop: []corev1.Capability{"ALL"},
+			},
+		}
 	}
 
 	return container, nil

diff --git a/control-plane/api-gateway/gatekeeper/role.go b/control-plane/api-gateway/gatekeeper/role.go
@@ -19,7 +19,7 @@ import (
 )
 
 func (g *Gatekeeper) upsertRole(ctx context.Context, gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConfig, config common.HelmConfig) error {
-	if config.AuthMethod == "" {
+	if config.AuthMethod == "" && !config.EnableOpenShift {
 		return g.deleteRole(ctx, types.NamespacedName{Namespace: gateway.Namespace, Name: gateway.Name})
 	}
 
@@ -40,7 +40,7 @@ func (g *Gatekeeper) upsertRole(ctx context.Context, gateway gwv1beta1.Gateway,
 		return errors.New("role not owned by controller")
 	}
 
-	role = g.role(gateway, gcc)
+	role = g.role(gateway, gcc, config)
 	if err := ctrl.SetControllerReference(&gateway, role, g.Client.Scheme()); err != nil {
 		return err
 	}
@@ -62,7 +62,7 @@ func (g *Gatekeeper) deleteRole(ctx context.Context, gwName types.NamespacedName
 	return nil
 }
 
-func (g *Gatekeeper) role(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConfig) *rbac.Role {
+func (g *Gatekeeper) role(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConfig, config common.HelmConfig) *rbac.Role {
 	role := &rbac.Role{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      gateway.Name,
@@ -81,5 +81,14 @@ func (g *Gatekeeper) role(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassCo
 		})
 	}
 
+	if config.EnableOpenShift {
+		role.Rules = append(role.Rules, rbac.PolicyRule{
+			APIGroups:     []string{"security.openshift.io"},
+			Resources:     []string{"securitycontextconstraints"},
+			ResourceNames: []string{gcc.Spec.OpenshiftSCCName},
+			Verbs:         []string{"use"},
+		})
+	}
+
 	return role
 }
diff --git a/control-plane/api-gateway/gatekeeper/rolebinding.go b/control-plane/api-gateway/gatekeeper/rolebinding.go
@@ -19,7 +19,7 @@ import (
 )
 
 func (g *Gatekeeper) upsertRoleBinding(ctx context.Context, gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConfig, config common.HelmConfig) error {
-	if config.AuthMethod == "" {
+	if config.AuthMethod == "" && !config.EnableOpenShift {
 		return g.deleteRole(ctx, types.NamespacedName{Namespace: gateway.Namespace, Name: gateway.Name})
 	}
 
@@ -66,7 +66,7 @@ func (g *Gatekeeper) deleteRoleBinding(ctx context.Context, gwName types.Namespa
 func (g *Gatekeeper) roleBinding(gateway gwv1beta1.Gateway, gcc v1alpha1.GatewayClassConfig, config common.HelmConfig) *rbac.RoleBinding {
 	// Create resources for reference. This avoids bugs if naming patterns change.
 	serviceAccount := g.serviceAccount(gateway)
-	role := g.role(gateway, gcc)
+	role := g.role(gateway, gcc, config)
 
 	return &rbac.RoleBinding{
 		ObjectMeta: metav1.ObjectMeta{

diff --git a/control-plane/api-gateway/gatekeeper/serviceaccount.go b/control-plane/api-gateway/gatekeeper/serviceaccount.go
@@ -18,7 +18,7 @@ import (
 )
 
 func (g *Gatekeeper) upsertServiceAccount(ctx context.Context, gateway gwv1beta1.Gateway, config common.HelmConfig) error {
-	if config.AuthMethod == "" {
+	if config.AuthMethod == "" && !config.EnableOpenShift {
 		return g.deleteServiceAccount(ctx, types.NamespacedName{Namespace: gateway.Namespace, Name: gateway.Name})
 	}