Skip to content

Commit

Permalink
Support running with restricted PSA enforcement enabled (part 1) (#2572)
Browse files Browse the repository at this point in the history
Support restricted PSA enforcement in a basic setup. This is enough to get a basic setup with ACLs and TLS working and an acceptance test passing (but does not update every component).

On OpenShift, we have the option to set the security context or not. If the security context is unset, then it is set automatically by OpenShift SCCs. However, we prefer to set the security context to avoid useless warnings on OpenShift and to reduce the config difference between OpenShift and plain Kube. By default, OpenShift namespaces have the audit and warn PSA labels set to restricted, so we receive pod security warnings when deploying Consul to OpenShift even though the pods will be able to run.

Helm chart changes:

* Add a helper to the helm chart to define a "restricted" container security context (when pod security policies are not enabled)
* Update the following container securityContexts to use the "restricted" settings (not exhaustive)

  - gateway-cleanup-job.yaml
  - gateway-resources-job.yaml
  - gossip-encryption-autogenerate-job.yaml
  - server-acl-init-cleanup-job.yaml - only if `.Values.server.containerSecurityContext.server.acl-init` is unset
  - server-acl-init-job.yaml - only if `.Values.server.containerSecurityContext.server.acl-init` is unset
  - server-statefulset.yaml:
     - the locality-init container receives the restricted context
     - the consul container receives the restricted context only if `.Values.server.containerSecurityContext.server` is unset
  - tls-init-cleanup-job.yaml - only if `.Values.server.containerSecurityContext.server.tls-init` is unset
  - tls-init-job.yaml - only if `.Values.server.containerSecurityContext.server.tls-init` is unset
  - webhook-cert-manager-deployment.yaml

Acceptance test changes:

* When `-enable-openshift` and `-enable-cni` are set, configure the CNI
  settings correctly for OpenShift.
* Add the `-enable-restricted-psa-enforcement` test flag. When this is set,
  the tests assume the Consul namespace has restricted PSA enforcement enabled.
  The tests will deploy the CNI (if enabled) into the `kube-system` namespace.
  Compatible test cases will deploy applications outside of the Consul namespace.
* Update the ConnectHelper to configure the NetworkAttachmentDefinition
  required to be compatible with the CNI on OpenShift.
* Add fixtures for static-client and static-server for OpenShift. This
  is necessary because the deployment configs must reference the network
  attachment definition when using the CNI on OpenShift.
* Update tests in the `acceptance/tests/connect` directory to either
  run or skip based on -enable-cni and -enable-openshift
  • Loading branch information
Paul Glass authored and absolutelightning committed Aug 4, 2023
1 parent c61c798 commit be01806
Show file tree
Hide file tree
Showing 31 changed files with 397 additions and 39 deletions.
3 changes: 3 additions & 0 deletions .changelog/2572.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
helm: set container securityContexts to match the `restricted` Pod Security Standards policy to support running Consul in a namespace with restricted PSA enforcement enabled
```
22 changes: 21 additions & 1 deletion acceptance/framework/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"path/filepath"
"strconv"
"strings"
"testing"

"github.com/hashicorp/go-version"
"gopkg.in/yaml.v2"
Expand Down Expand Up @@ -73,7 +74,8 @@ type TestConfig struct {

EnablePodSecurityPolicies bool

EnableCNI bool
EnableCNI bool
EnableRestrictedPSAEnforcement bool

EnableTransparentProxy bool

Expand Down Expand Up @@ -135,10 +137,22 @@ func (t *TestConfig) HelmValuesFromConfig() (map[string]string, error) {

if t.EnableCNI {
setIfNotEmpty(helmValues, "connectInject.cni.enabled", "true")
setIfNotEmpty(helmValues, "connectInject.cni.logLevel", "debug")
// GKE is currently the only cloud provider that uses a different CNI bin dir.
if t.UseGKE {
setIfNotEmpty(helmValues, "connectInject.cni.cniBinDir", "/home/kubernetes/bin")
}
if t.EnableOpenshift {
setIfNotEmpty(helmValues, "connectInject.cni.multus", "true")
setIfNotEmpty(helmValues, "connectInject.cni.cniBinDir", "/var/lib/cni/bin")
setIfNotEmpty(helmValues, "connectInject.cni.cniNetDir", "/etc/kubernetes/cni/net.d")
}

if t.EnableRestrictedPSAEnforcement {
// The CNI requires privilege, so when restricted PSA enforcement is enabled on the Consul
// namespace it must be run in a different privileged namespace.
setIfNotEmpty(helmValues, "connectInject.cni.namespace", "kube-system")
}
}

setIfNotEmpty(helmValues, "connectInject.transparentProxy.defaultEnabled", strconv.FormatBool(t.EnableTransparentProxy))
Expand Down Expand Up @@ -220,6 +234,12 @@ func (t *TestConfig) entImage() (string, error) {
return fmt.Sprintf("hashicorp/consul-enterprise:%s%s-ent", consulImageVersion, preRelease), nil
}

func (c *TestConfig) SkipWhenOpenshiftAndCNI(t *testing.T) {
if c.EnableOpenshift && c.EnableCNI {
t.Skip("skipping because -enable-cni and -enable-openshift are set and this test doesn't deploy apps correctly")
}
}

// setIfNotEmpty sets key to val in map m if value is not empty.
func setIfNotEmpty(m map[string]string, key, val string) {
if val != "" {
Expand Down
1 change: 1 addition & 0 deletions acceptance/framework/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ func TestConfig_HelmValuesFromConfig(t *testing.T) {
},
map[string]string{
"connectInject.cni.enabled": "true",
"connectInject.cni.logLevel": "debug",
"connectInject.transparentProxy.defaultEnabled": "false",
},
},
Expand Down
112 changes: 92 additions & 20 deletions acceptance/framework/connhelper/connect_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ package connhelper
import (
"context"
"strconv"
"strings"
"testing"
"time"

terratestK8s "github.com/gruntwork-io/terratest/modules/k8s"
"github.com/hashicorp/consul-k8s/acceptance/framework/config"
"github.com/hashicorp/consul-k8s/acceptance/framework/consul"
"github.com/hashicorp/consul-k8s/acceptance/framework/environment"
Expand Down Expand Up @@ -44,7 +46,12 @@ type ConnectHelper struct {
// ReleaseName is the name of the Consul cluster.
ReleaseName string

// Ctx is used to deploy Consul
Ctx environment.TestContext
// UseAppNamespace is used top optionally deploy applications into a separate namespace.
// If unset, the namespace associated with Ctx is used.
UseAppNamespace bool

Cfg *config.TestConfig

// consulCluster is the cluster to use for the test.
Expand Down Expand Up @@ -82,6 +89,14 @@ func (c *ConnectHelper) Upgrade(t *testing.T) {
c.consulCluster.Upgrade(t, c.helmValues())
}

func (c *ConnectHelper) KubectlOptsForApp(t *testing.T) *terratestK8s.KubectlOptions {
opts := c.Ctx.KubectlOptions(t)
if !c.UseAppNamespace {
return opts
}
return c.Ctx.KubectlOptionsForNamespace(opts.Namespace + "-apps")
}

// DeployClientAndServer deploys a client and server pod to the Kubernetes
// cluster which will be used to test service mesh connectivity. If the Secure
// flag is true, a pre-check is done to ensure that the ACL tokens for the test
Expand All @@ -108,51 +123,105 @@ func (c *ConnectHelper) DeployClientAndServer(t *testing.T) {

logger.Log(t, "creating static-server and static-client deployments")

k8s.DeployKustomize(t, c.Ctx.KubectlOptions(t), c.Cfg.NoCleanupOnFailure, c.Cfg.NoCleanup, c.Cfg.DebugDirectory, "../fixtures/cases/static-server-inject")
if c.Cfg.EnableTransparentProxy {
k8s.DeployKustomize(t, c.Ctx.KubectlOptions(t), c.Cfg.NoCleanupOnFailure, c.Cfg.NoCleanup, c.Cfg.DebugDirectory, "../fixtures/cases/static-client-tproxy")
c.setupAppNamespace(t)

opts := c.KubectlOptsForApp(t)
if c.Cfg.EnableCNI && c.Cfg.EnableOpenshift {
// On OpenShift with the CNI, we need to create a network attachment definition in the namespace
// where the applications are running, and the app deployment configs need to reference that network
// attachment definition.

// TODO: A base fixture is the wrong place for these files
k8s.KubectlApply(t, opts, "../fixtures/bases/openshift/")
helpers.Cleanup(t, c.Cfg.NoCleanupOnFailure, c.Cfg.NoCleanup, func() {
k8s.KubectlDelete(t, opts, "../fixtures/bases/openshift/")
})

k8s.DeployKustomize(t, opts, c.Cfg.NoCleanupOnFailure, c.Cfg.NoCleanup, c.Cfg.DebugDirectory, "../fixtures/cases/static-server-openshift")
if c.Cfg.EnableTransparentProxy {
k8s.DeployKustomize(t, opts, c.Cfg.NoCleanupOnFailure, c.Cfg.NoCleanup, c.Cfg.DebugDirectory, "../fixtures/cases/static-client-openshift-tproxy")
} else {
k8s.DeployKustomize(t, opts, c.Cfg.NoCleanupOnFailure, c.Cfg.NoCleanup, c.Cfg.DebugDirectory, "../fixtures/cases/static-client-openshift-inject")
}
} else {
k8s.DeployKustomize(t, c.Ctx.KubectlOptions(t), c.Cfg.NoCleanupOnFailure, c.Cfg.NoCleanup, c.Cfg.DebugDirectory, "../fixtures/cases/static-client-inject")
k8s.DeployKustomize(t, c.Ctx.KubectlOptions(t), c.Cfg.NoCleanupOnFailure, c.Cfg.NoCleanup, c.Cfg.DebugDirectory, "../fixtures/cases/static-server-inject")
if c.Cfg.EnableTransparentProxy {
k8s.DeployKustomize(t, c.Ctx.KubectlOptions(t), c.Cfg.NoCleanupOnFailure, c.Cfg.NoCleanup, c.Cfg.DebugDirectory, "../fixtures/cases/static-client-tproxy")
} else {
k8s.DeployKustomize(t, c.Ctx.KubectlOptions(t), c.Cfg.NoCleanupOnFailure, c.Cfg.NoCleanup, c.Cfg.DebugDirectory, "../fixtures/cases/static-client-inject")
}
}

// Check that both static-server and static-client have been injected and
// now have 2 containers.
retry.RunWith(
&retry.Timer{Timeout: 30 * time.Second, Wait: 100 * time.Millisecond}, t,
func(r *retry.R) {
for _, labelSelector := range []string{"app=static-server", "app=static-client"} {
podList, err := c.Ctx.KubernetesClient(t).CoreV1().Pods(c.Ctx.KubectlOptions(t).Namespace).List(context.Background(), metav1.ListOptions{
LabelSelector: labelSelector,
FieldSelector: `status.phase=Running`,
})
podList, err := c.Ctx.KubernetesClient(t).CoreV1().
Pods(opts.Namespace).
List(context.Background(), metav1.ListOptions{
LabelSelector: labelSelector,
FieldSelector: `status.phase=Running`,
})
require.NoError(r, err)
require.Len(r, podList.Items, 1)
require.Len(r, podList.Items[0].Spec.Containers, 2)
}
})
}

// setupAppNamespace creates a namespace where applications are deployed. This
// does nothing if UseAppNamespace is not set. The app namespace is relevant
// when testing with restricted PSA enforcement enabled.
func (c *ConnectHelper) setupAppNamespace(t *testing.T) {
if !c.UseAppNamespace {
return
}
opts := c.KubectlOptsForApp(t)
// If we are deploying apps in another namespace, create the namespace.

_, err := k8s.RunKubectlAndGetOutputE(t, opts, "create", "ns", opts.Namespace)
if err != nil && strings.Contains(err.Error(), "AlreadyExists") {
return
}
require.NoError(t, err)
helpers.Cleanup(t, c.Cfg.NoCleanupOnFailure, c.Cfg.NoCleanup, func() {
k8s.RunKubectl(t, opts, "delete", "ns", opts.Namespace)
})

if c.Cfg.EnableRestrictedPSAEnforcement {
// Allow anything to run in the app namespace.
k8s.RunKubectl(t, opts, "label", "--overwrite", "ns", opts.Namespace,
"pod-security.kubernetes.io/enforce=privileged",
"pod-security.kubernetes.io/enforce-version=v1.24",
)
}

}

// CreateResolverRedirect creates a resolver that redirects to a static-server, a corresponding k8s service,
// and intentions. This helper is primarly used to ensure that the virtual-ips are persisted to consul properly.
func (c *ConnectHelper) CreateResolverRedirect(t *testing.T) {
logger.Log(t, "creating resolver redirect")
options := c.Ctx.KubectlOptions(t)
opts := c.KubectlOptsForApp(t)
c.setupAppNamespace(t)
kustomizeDir := "../fixtures/cases/resolver-redirect-virtualip"
k8s.KubectlApplyK(t, options, kustomizeDir)
k8s.KubectlApplyK(t, opts, kustomizeDir)

helpers.Cleanup(t, c.Cfg.NoCleanupOnFailure, c.Cfg.NoCleanup, func() {
k8s.KubectlDeleteK(t, options, kustomizeDir)
k8s.KubectlDeleteK(t, opts, kustomizeDir)
})
}

// TestConnectionFailureWithoutIntention ensures the connection to the static
// server fails when no intentions are configured.
func (c *ConnectHelper) TestConnectionFailureWithoutIntention(t *testing.T) {
logger.Log(t, "checking that the connection is not successful because there's no intention")
opts := c.KubectlOptsForApp(t)
if c.Cfg.EnableTransparentProxy {
k8s.CheckStaticServerConnectionFailing(t, c.Ctx.KubectlOptions(t), StaticClientName, "http://static-server")
k8s.CheckStaticServerConnectionFailing(t, opts, StaticClientName, "http://static-server")
} else {
k8s.CheckStaticServerConnectionFailing(t, c.Ctx.KubectlOptions(t), StaticClientName, "http://localhost:1234")
k8s.CheckStaticServerConnectionFailing(t, opts, StaticClientName, "http://localhost:1234")
}
}

Expand All @@ -177,11 +246,12 @@ func (c *ConnectHelper) CreateIntention(t *testing.T) {
// static-client pod once the intention is set.
func (c *ConnectHelper) TestConnectionSuccess(t *testing.T) {
logger.Log(t, "checking that connection is successful")
opts := c.KubectlOptsForApp(t)
if c.Cfg.EnableTransparentProxy {
// todo: add an assertion that the traffic is going through the proxy
k8s.CheckStaticServerConnectionSuccessful(t, c.Ctx.KubectlOptions(t), StaticClientName, "http://static-server")
k8s.CheckStaticServerConnectionSuccessful(t, opts, StaticClientName, "http://static-server")
} else {
k8s.CheckStaticServerConnectionSuccessful(t, c.Ctx.KubectlOptions(t), StaticClientName, "http://localhost:1234")
k8s.CheckStaticServerConnectionSuccessful(t, opts, StaticClientName, "http://localhost:1234")
}
}

Expand All @@ -192,8 +262,10 @@ func (c *ConnectHelper) TestConnectionFailureWhenUnhealthy(t *testing.T) {
// Test that kubernetes readiness status is synced to Consul.
// Create a file called "unhealthy" at "/tmp/" so that the readiness probe
// of the static-server pod fails.
opts := c.KubectlOptsForApp(t)

logger.Log(t, "testing k8s -> consul health checks sync by making the static-server unhealthy")
k8s.RunKubectl(t, c.Ctx.KubectlOptions(t), "exec", "deploy/"+StaticServerName, "--", "touch", "/tmp/unhealthy")
k8s.RunKubectl(t, opts, "exec", "deploy/"+StaticServerName, "--", "touch", "/tmp/unhealthy")

// The readiness probe should take a moment to be reflected in Consul,
// CheckStaticServerConnection will retry until Consul marks the service
Expand All @@ -205,20 +277,20 @@ func (c *ConnectHelper) TestConnectionFailureWhenUnhealthy(t *testing.T) {
// other tests.
logger.Log(t, "checking that connection is unsuccessful")
if c.Cfg.EnableTransparentProxy {
k8s.CheckStaticServerConnectionMultipleFailureMessages(t, c.Ctx.KubectlOptions(t), StaticClientName, false, []string{
k8s.CheckStaticServerConnectionMultipleFailureMessages(t, opts, StaticClientName, false, []string{
"curl: (56) Recv failure: Connection reset by peer",
"curl: (52) Empty reply from server",
"curl: (7) Failed to connect to static-server port 80: Connection refused",
}, "", "http://static-server")
} else {
k8s.CheckStaticServerConnectionMultipleFailureMessages(t, c.Ctx.KubectlOptions(t), StaticClientName, false, []string{
k8s.CheckStaticServerConnectionMultipleFailureMessages(t, opts, StaticClientName, false, []string{
"curl: (56) Recv failure: Connection reset by peer",
"curl: (52) Empty reply from server",
}, "", "http://localhost:1234")
}

// Return the static-server to a "healthy state".
k8s.RunKubectl(t, c.Ctx.KubectlOptions(t), "exec", "deploy/"+StaticServerName, "--", "rm", "/tmp/unhealthy")
k8s.RunKubectl(t, opts, "exec", "deploy/"+StaticServerName, "--", "rm", "/tmp/unhealthy")
}

// helmValues uses the Secure and AutoEncrypt fields to set values for the Helm
Expand Down
6 changes: 6 additions & 0 deletions acceptance/framework/consul/helm_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/gruntwork-io/terratest/modules/k8s"
"github.com/hashicorp/consul-k8s/acceptance/framework/config"
"github.com/hashicorp/consul-k8s/acceptance/framework/environment"
"github.com/stretchr/testify/require"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/kubernetes/fake"
Expand Down Expand Up @@ -80,9 +81,14 @@ func (c *ctx) Name() string {
func (c *ctx) KubectlOptions(_ *testing.T) *k8s.KubectlOptions {
return &k8s.KubectlOptions{}
}
func (c *ctx) KubectlOptionsForNamespace(ns string) *k8s.KubectlOptions {
return &k8s.KubectlOptions{}
}
func (c *ctx) KubernetesClient(_ *testing.T) kubernetes.Interface {
return fake.NewSimpleClientset()
}
func (c *ctx) ControllerRuntimeClient(_ *testing.T) client.Client {
return runtimefake.NewClientBuilder().Build()
}

var _ environment.TestContext = (*ctx)(nil)
10 changes: 10 additions & 0 deletions acceptance/framework/environment/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ type TestEnvironment interface {
// for example, information about a specific Kubernetes cluster.
type TestContext interface {
KubectlOptions(t *testing.T) *k8s.KubectlOptions
// TODO: I don't love this.
KubectlOptionsForNamespace(ns string) *k8s.KubectlOptions
KubernetesClient(t *testing.T) kubernetes.Interface
ControllerRuntimeClient(t *testing.T) client.Client
}
Expand Down Expand Up @@ -138,6 +140,14 @@ func (k kubernetesContext) KubectlOptions(t *testing.T) *k8s.KubectlOptions {
return k.options
}

func (k kubernetesContext) KubectlOptionsForNamespace(ns string) *k8s.KubectlOptions {
return &k8s.KubectlOptions{
ContextName: k.kubeContextName,
ConfigPath: k.pathToKubeConfig,
Namespace: ns,
}
}

// KubernetesClientFromOptions takes KubectlOptions and returns Kubernetes API client.
func KubernetesClientFromOptions(t *testing.T, options *k8s.KubectlOptions) kubernetes.Interface {
configPath, err := options.GetConfigPath(t)
Expand Down
14 changes: 12 additions & 2 deletions acceptance/framework/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ type TestFlags struct {

flagEnablePodSecurityPolicies bool

flagEnableCNI bool
flagEnableCNI bool
flagEnableRestrictedPSAEnforcement bool

flagEnableTransparentProxy bool

Expand Down Expand Up @@ -116,6 +117,13 @@ func (t *TestFlags) init() {
flag.BoolVar(&t.flagEnableCNI, "enable-cni", false,
"If true, the test suite will run tests with consul-cni plugin enabled. "+
"In general, this will only run against tests that are mesh related (connect, mesh-gateway, peering, etc")
flag.BoolVar(&t.flagEnableRestrictedPSAEnforcement, "enable-restricted-psa-enforcement", false,
"If true, this indicates that Consul is being run in a namespace with restricted PSA enforcement enabled. "+
"The tests do not configure Consul's namespace with PSA enforcement enabled. This must configured before tests are run. "+
"The CNI and test applications need more privilege than is allowed in a restricted namespace. "+
"When set, the CNI will be deployed into the kube-system namespace, and in supported test cases, applications "+
"are deployed, by default, into a namespace named '<consul-namespace>-apps' instead of being deployed into the "+
"Consul namespace.")

flag.BoolVar(&t.flagEnableTransparentProxy, "enable-transparent-proxy", false,
"If true, the test suite will run tests with transparent proxy enabled. "+
Expand Down Expand Up @@ -176,6 +184,7 @@ func (t *TestFlags) Validate() error {
if t.flagEnableEnterprise && t.flagEnterpriseLicense == "" {
return errors.New("-enable-enterprise provided without setting env var CONSUL_ENT_LICENSE with consul license")
}

return nil
}

Expand All @@ -200,7 +209,8 @@ func (t *TestFlags) TestConfigFromFlags() *config.TestConfig {

EnablePodSecurityPolicies: t.flagEnablePodSecurityPolicies,

EnableCNI: t.flagEnableCNI,
EnableCNI: t.flagEnableCNI,
EnableRestrictedPSAEnforcement: t.flagEnableRestrictedPSAEnforcement,

EnableTransparentProxy: t.flagEnableTransparentProxy,

Expand Down
2 changes: 2 additions & 0 deletions acceptance/tests/connect/connect_external_servers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ func TestConnectInject_ExternalServers(t *testing.T) {
caseName := fmt.Sprintf("secure: %t", secure)
t.Run(caseName, func(t *testing.T) {
cfg := suite.Config()
cfg.SkipWhenOpenshiftAndCNI(t)

ctx := suite.Environment().DefaultContext(t)

serverHelmValues := map[string]string{
Expand Down
2 changes: 2 additions & 0 deletions acceptance/tests/connect/connect_inject_namespaces_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ func TestConnectInjectNamespaces(t *testing.T) {
if !cfg.EnableEnterprise {
t.Skipf("skipping this test because -enable-enterprise is not set")
}
cfg.SkipWhenOpenshiftAndCNI(t)

cases := []struct {
name string
Expand Down Expand Up @@ -246,6 +247,7 @@ func TestConnectInjectNamespaces_CleanupController(t *testing.T) {
if !cfg.EnableEnterprise {
t.Skipf("skipping this test because -enable-enterprise is not set")
}
cfg.SkipWhenOpenshiftAndCNI(t)

consulDestNS := "consul-dest"
cases := []struct {
Expand Down
Loading

0 comments on commit be01806

Please sign in to comment.