Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support running with restricted PSA enforcement enabled (part 1) #2572

Merged
merged 14 commits into from
Jul 24, 2023
Merged
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")
curtbushko marked this conversation as resolved.
Show resolved Hide resolved
}
}

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/")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of using the AppNamespace flag, you could set the namespace here using opts when CNI + OpenShift is set.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really clear what you mean. This is in ConnectHelper so how does it know the namespace to use?

Right now, it's using a namespace sourced from the test flags via the context (bc that's what I set up). Do you mean I should instead have each test pass a namespace to ConnectHelper if needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried something else:

  • I added ConnectHelper.UseAppNamespace = true|false option
    • If true then it creates, configures, and deploys into a namespace named <consul-namespace>-apps.

This way there is a shared/common way to easily create an app namespace that tests can use if needed and there are no more -app-namespace flags.

I also added a -restricted-psa-enforcement-enabled flag and tests key off of that to determine whether to deploy apps into a separate namespace. (I don't know if I really like this, so I'm open to alternatives)

Copy link
Contributor Author

@pglass pglass Jul 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Also, I accidentally did all of this while rebasing, so the latest commit that looks like a merge from main actually contains the changes - sorry!)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the -restricted-psa-enforcement-enabled flag. I think this is a good + global way to do things as I could see us wanting to test this on all the clouds in the future.

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