Backport of NET-5947 Add NET_BIND_SERVICE capability in security cont…

…ext for api-gateway pod on OpenShift into release/1.2.x (#3076) Co-authored-by: Nathan Coleman <[email protected]>
hashicorp · Oct 16, 2023 · f8a2341 · f8a2341
1 parent 24e10f1
commit f8a2341
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 33 deletions.
diff --git a/.changelog/3070.txt b/.changelog/3070.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+api-gateway: fix issue where missing `NET_BIND_SERVICE` capability prevented api-gateway `Pod` from starting up when deployed to OpenShift
+```
diff --git a/control-plane/api-gateway/gatekeeper/dataplane.go b/control-plane/api-gateway/gatekeeper/dataplane.go
@@ -8,17 +8,17 @@ import (
 	"strconv"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/utils/pointer"
 
 	"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
 	"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
 	"github.com/hashicorp/consul-k8s/control-plane/connect-inject/constants"
 	"github.com/hashicorp/consul-k8s/control-plane/namespaces"
-	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
 const (
-	allCapabilities              = "all"
+	allCapabilities              = "ALL"
 	netBindCapability            = "NET_BIND_SERVICE"
 	consulDataplaneDNSBindHost   = "127.0.0.1"
 	consulDataplaneDNSBindPort   = 8600
@@ -105,21 +105,18 @@ func consulDataplaneContainer(config common.HelmConfig, gcc v1alpha1.GatewayClas
 		container.Resources = *gcc.Spec.DeploymentSpec.Resources
 	}
 
-	// If not running in an OpenShift environment,
-	// skip setting the security context and let OpenShift set it for us.
+	// If running in vanilla K8s, run as root to allow binding to privileged ports;
+	// otherwise, allow the user to be assigned by OpenShift.
+	container.SecurityContext = &corev1.SecurityContext{
+		ReadOnlyRootFilesystem: pointer.Bool(true),
+		// Drop any Linux capabilities you'd get as root other than NET_BIND_SERVICE.
+		Capabilities: &corev1.Capabilities{
+			Add:  []corev1.Capability{netBindCapability},
+			Drop: []corev1.Capability{allCapabilities},
+		},
+	}
 	if !config.EnableOpenShift {
-		container.SecurityContext = &corev1.SecurityContext{
-			ReadOnlyRootFilesystem: pointer.Bool(true),
-			// We have to run as root if we want to bind to any
-			// sort of privileged ports. The drop "all" is intended
-			// to drop any Linux capabilities you'd get as root
-			// other than NET_BIND_SERVICE.
-			RunAsUser: pointer.Int64(0),
-			Capabilities: &corev1.Capabilities{
-				Add:  []corev1.Capability{netBindCapability},
-				Drop: []corev1.Capability{allCapabilities},
-			},
-		}
+		container.SecurityContext.RunAsUser = pointer.Int64(0)
 	}
 
 	return container, nil

diff --git a/control-plane/api-gateway/gatekeeper/gatekeeper_test.go b/control-plane/api-gateway/gatekeeper/gatekeeper_test.go
@@ -9,8 +9,7 @@ import (
 	"testing"
 
 	logrtest "github.com/go-logr/logr/testr"
-	common "github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
-	"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -23,11 +22,15 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	gwv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
+
+	"github.com/hashicorp/consul-k8s/control-plane/api-gateway/common"
+	"github.com/hashicorp/consul-k8s/control-plane/api/v1alpha1"
 )
 
 var (
 	createdAtLabelKey   = "gateway.consul.hashicorp.com/created"
 	createdAtLabelValue = "101010"
+	dataplaneImage      = "hashicorp/consul-dataplane"
 	name                = "test"
 	namespace           = "default"
 	labels              = map[string]string{
@@ -102,7 +105,9 @@ func TestUpsert(t *testing.T) {
 					ServiceType:     (*corev1.ServiceType)(common.PointerTo("NodePort")),
 				},
 			},
-			helmConfig:       common.HelmConfig{},
+			helmConfig: common.HelmConfig{
+				ImageDataplane: dataplaneImage,
+			},
 			initialResources: resources{},
 			finalResources: resources{
 				deployments: []*appsv1.Deployment{
@@ -149,7 +154,9 @@ func TestUpsert(t *testing.T) {
 					MapPrivilegedContainerPorts: 2000,
 				},
 			},
-			helmConfig:       common.HelmConfig{},
+			helmConfig: common.HelmConfig{
+				ImageDataplane: dataplaneImage,
+			},
 			initialResources: resources{},
 			finalResources: resources{
 				deployments: []*appsv1.Deployment{
@@ -199,7 +206,9 @@ func TestUpsert(t *testing.T) {
 					ServiceType:     (*corev1.ServiceType)(common.PointerTo("NodePort")),
 				},
 			},
-			helmConfig:       common.HelmConfig{},
+			helmConfig: common.HelmConfig{
+				ImageDataplane: dataplaneImage,
+			},
 			initialResources: resources{},
 			finalResources: resources{
 				deployments: []*appsv1.Deployment{
@@ -250,7 +259,8 @@ func TestUpsert(t *testing.T) {
 				},
 			},
 			helmConfig: common.HelmConfig{
-				AuthMethod: "method",
+				AuthMethod:     "method",
+				ImageDataplane: dataplaneImage,
 			},
 			initialResources: resources{},
 			finalResources: resources{
@@ -308,7 +318,9 @@ func TestUpsert(t *testing.T) {
 					ServiceType:     (*corev1.ServiceType)(common.PointerTo("NodePort")),
 				},
 			},
-			helmConfig:       common.HelmConfig{},
+			helmConfig: common.HelmConfig{
+				ImageDataplane: dataplaneImage,
+			},
 			initialResources: resources{},
 			finalResources: resources{
 				deployments: []*appsv1.Deployment{
@@ -343,7 +355,9 @@ func TestUpsert(t *testing.T) {
 					ServiceType:     (*corev1.ServiceType)(common.PointerTo("NodePort")),
 				},
 			},
-			helmConfig:       common.HelmConfig{},
+			helmConfig: common.HelmConfig{
+				ImageDataplane: dataplaneImage,
+			},
 			initialResources: resources{},
 			finalResources: resources{
 				deployments: []*appsv1.Deployment{
@@ -379,7 +393,8 @@ func TestUpsert(t *testing.T) {
 				},
 			},
 			helmConfig: common.HelmConfig{
-				AuthMethod: "method",
+				AuthMethod:     "method",
+				ImageDataplane: dataplaneImage,
 			},
 			initialResources: resources{
 				deployments: []*appsv1.Deployment{
@@ -462,7 +477,8 @@ func TestUpsert(t *testing.T) {
 				},
 			},
 			helmConfig: common.HelmConfig{
-				AuthMethod: "method",
+				AuthMethod:     "method",
+				ImageDataplane: dataplaneImage,
 			},
 			initialResources: resources{
 				deployments: []*appsv1.Deployment{
@@ -541,7 +557,9 @@ func TestUpsert(t *testing.T) {
 					ServiceType:     (*corev1.ServiceType)(common.PointerTo("NodePort")),
 				},
 			},
-			helmConfig: common.HelmConfig{},
+			helmConfig: common.HelmConfig{
+				ImageDataplane: dataplaneImage,
+			},
 			initialResources: resources{
 				deployments: []*appsv1.Deployment{
 					configureDeployment(name, namespace, labels, 5, nil, nil, "", "1"),
@@ -580,7 +598,9 @@ func TestUpsert(t *testing.T) {
 					ServiceType:     (*corev1.ServiceType)(common.PointerTo("NodePort")),
 				},
 			},
-			helmConfig: common.HelmConfig{},
+			helmConfig: common.HelmConfig{
+				ImageDataplane: dataplaneImage,
+			},
 			initialResources: resources{
 				deployments: []*appsv1.Deployment{
 					configureDeployment(name, namespace, labels, 8, nil, nil, "", "1"),
@@ -619,7 +639,9 @@ func TestUpsert(t *testing.T) {
 					ServiceType:     (*corev1.ServiceType)(common.PointerTo("NodePort")),
 				},
 			},
-			helmConfig: common.HelmConfig{},
+			helmConfig: common.HelmConfig{
+				ImageDataplane: dataplaneImage,
+			},
 			initialResources: resources{
 				deployments: []*appsv1.Deployment{
 					configureDeployment(name, namespace, labels, 1, nil, nil, "", "1"),
@@ -658,7 +680,9 @@ func TestUpsert(t *testing.T) {
 					ServiceType:     (*corev1.ServiceType)(common.PointerTo("NodePort")),
 				},
 			},
-			helmConfig: common.HelmConfig{},
+			helmConfig: common.HelmConfig{
+				ImageDataplane: dataplaneImage,
+			},
 			initialResources: resources{
 				deployments: []*appsv1.Deployment{
 					configureDeployment(name, namespace, labels, 10, nil, nil, "", "1"),
@@ -699,6 +723,7 @@ func TestUpsert(t *testing.T) {
 			},
 			helmConfig: common.HelmConfig{
 				EnableOpenShift: true,
+				ImageDataplane:  "hashicorp/consul-dataplane",
 			},
 			initialResources: resources{},
 			finalResources: resources{
@@ -770,7 +795,9 @@ func TestDelete(t *testing.T) {
 					ServiceType:     (*corev1.ServiceType)(common.PointerTo("NodePort")),
 				},
 			},
-			helmConfig: common.HelmConfig{},
+			helmConfig: common.HelmConfig{
+				ImageDataplane: dataplaneImage,
+			},
 			initialResources: resources{
 				deployments: []*appsv1.Deployment{
 					configureDeployment(name, namespace, labels, 3, nil, nil, "", "1"),
@@ -807,7 +834,9 @@ func TestDelete(t *testing.T) {
 					ServiceType:     (*corev1.ServiceType)(common.PointerTo("NodePort")),
 				},
 			},
-			helmConfig: common.HelmConfig{},
+			helmConfig: common.HelmConfig{
+				ImageDataplane: dataplaneImage,
+			},
 			initialResources: resources{
 				deployments: []*appsv1.Deployment{
 					configureDeployment(name, namespace, labels, 3, nil, nil, "", "1"),
@@ -861,7 +890,8 @@ func TestDelete(t *testing.T) {
 				},
 			},
 			helmConfig: common.HelmConfig{
-				AuthMethod: "method",
+				AuthMethod:     "method",
+				ImageDataplane: dataplaneImage,
 			},
 			initialResources: resources{
 				deployments: []*appsv1.Deployment{
@@ -977,6 +1007,22 @@ func validateResourcesExist(t *testing.T, client client.Client, resources resour
 			require.NotNil(t, actual.Spec.Replicas)
 			require.EqualValues(t, *expected.Spec.Replicas, *actual.Spec.Replicas)
 		}
+
+		// Ensure there is a consul-dataplane container dropping ALL capabilities, adding
+		// back the NET_BIND_SERVICE capability, and establishing a read-only root filesystem
+		hasDataplaneContainer := false
+		for _, container := range actual.Spec.Template.Spec.Containers {
+			if container.Image == dataplaneImage {
+				hasDataplaneContainer = true
+				require.NotNil(t, container.SecurityContext)
+				require.NotNil(t, container.SecurityContext.Capabilities)
+				require.NotNil(t, container.SecurityContext.ReadOnlyRootFilesystem)
+				assert.True(t, *container.SecurityContext.ReadOnlyRootFilesystem)
+				assert.Equal(t, []corev1.Capability{netBindCapability}, container.SecurityContext.Capabilities.Add)
+				assert.Equal(t, []corev1.Capability{allCapabilities}, container.SecurityContext.Capabilities.Drop)
+			}
+		}
+		assert.True(t, hasDataplaneContainer)
 	}
 
 	for _, expected := range resources.roles {