diff --git a/.changelog/2184.txt b/.changelog/2184.txt new file mode 100644 index 0000000000..bdcb6039fd --- /dev/null +++ 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 index 8c0bbe9bf7..f1f6b3878f 100644 --- 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 index 4ab6570e31..38625c9368 100644 --- 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 index 1fa712759d..048af9fc26 100644 --- 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 index ace8c18d4a..d02b9eacde 100644 --- 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 index d79838770d..09322bd2a7 100644 --- 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 index d373a240f0..487b6f9ac1 100644 --- 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 index 06671382d1..31b3508e80 100644 --- 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 index f0d4dc7988..d83b298d92 100644 --- a/control-plane/api-gateway/common/helm_config.go +++ b/control-plane/api-gateway/common/helm_config.go @@ -11,6 +11,7 @@ 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 @@ -18,7 +19,6 @@ type HelmConfig struct { 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 index 19444831ee..6cb7170fc8 100644 --- 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 index 069643e301..30cc78d464 100644 --- 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 index 35360b7f87..f3d4ad1f95 100644 --- 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 index eb8431075a..705e9bffff 100644 --- 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 index 8891a754e6..1a60e752c8 100644 --- 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 index 47336867aa..d1c5c9883a 100644 --- 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}) } diff --git a/control-plane/api/v1alpha1/api_gateway_types.go b/control-plane/api/v1alpha1/api_gateway_types.go index f9be4bdb47..7f0b958701 100644 --- a/control-plane/api/v1alpha1/api_gateway_types.go +++ b/control-plane/api/v1alpha1/api_gateway_types.go @@ -59,6 +59,9 @@ type GatewayClassConfigSpec struct { // The name of an existing Kubernetes PodSecurityPolicy to bind to the managed ServiceAccount if ACLs are managed. PodSecurityPolicy string `json:"podSecurityPolicy,omitempty"` + + // The name of the OpenShift SecurityContextConstraints resource for this gateway class to use. + OpenshiftSCCName string `json:"openshiftSCCName,omitempty"` } // +k8s:deepcopy-gen=true diff --git a/control-plane/api/v1alpha1/api_gateway_types_test.go b/control-plane/api/v1alpha1/api_gateway_types_test.go index 1f9d8ebef0..6e0690b9b2 100644 --- a/control-plane/api/v1alpha1/api_gateway_types_test.go +++ b/control-plane/api/v1alpha1/api_gateway_types_test.go @@ -21,6 +21,7 @@ func TestGatewayClassConfigDeepCopy(t *testing.T) { NodeSelector: map[string]string{ "test": "test", }, + OpenshiftSCCName: "restricted-v2", } config := &GatewayClassConfig{ ObjectMeta: metav1.ObjectMeta{ diff --git a/control-plane/subcommand/gateway-resources/command.go b/control-plane/subcommand/gateway-resources/command.go index 2da2abccb1..3ad3ff7f53 100644 --- a/control-plane/subcommand/gateway-resources/command.go +++ b/control-plane/subcommand/gateway-resources/command.go @@ -71,6 +71,8 @@ type Command struct { flagTolerations string // this is a multiline yaml string matching the tolerations array flagServiceAnnotations string // this is a multiline yaml string array of annotations to allow + flagOpenshiftSCCName string + k8sClient client.Client once sync.Once @@ -123,6 +125,9 @@ func (c *Command) init() { c.flags.StringVar(&c.flagServiceAnnotations, "service-annotations", "", "The annotations to copy over from a gateway to its service.", ) + c.flags.StringVar(&c.flagOpenshiftSCCName, "openshift-scc-name", "", + "Name of security context constraint to use for gateways on Openshift.", + ) c.k8s = &flags.K8SFlags{} flags.Merge(c.flags, c.k8s.Flags()) @@ -197,6 +202,7 @@ func (c *Command) Run(args []string) int { MaxInstances: nonZeroOrNil(c.flagDeploymentMaxInstances), MinInstances: nonZeroOrNil(c.flagDeploymentMinInstances), }, + OpenshiftSCCName: c.flagOpenshiftSCCName, }, } diff --git a/control-plane/subcommand/gateway-resources/command_test.go b/control-plane/subcommand/gateway-resources/command_test.go index 0c40e67244..f60e376042 100644 --- a/control-plane/subcommand/gateway-resources/command_test.go +++ b/control-plane/subcommand/gateway-resources/command_test.go @@ -163,6 +163,7 @@ bar: 2`, flagServiceAnnotations: ` - foo - bar`, + flagOpenshiftSCCName: "restricted-v2", }, }, } { @@ -245,6 +246,7 @@ func TestRun(t *testing.T) { "-release-name", "test", "-component", "test", "-controller-name", "test", + "-openshift-scc-name", "restricted-v2", }) require.Equal(t, 0, code)