Skip to content

Commit

Permalink
fixup! Add traffic permissions integration tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
@erichaberkorn
erichaberkorn committed Oct 6, 2023
1 parent 8610501 commit 6af4fe2
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 89 deletions.
10 changes: 5 additions & 5 deletions .github/workflows/test-integrations.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ jobs:
--packages="./command/agent/consul" \
--junitfile $TEST_RESULTS_DIR/results.xml -- \
-run TestConsul
# NOTE: ENT specific step as we store secrets in Vault.
- name: Authenticate to Vault
if: ${{ endsWith(github.repository, '-enterprise') }}
Expand Down Expand Up @@ -257,7 +257,7 @@ jobs:
- name: Generate Envoy Job Matrix
id: set-matrix
env:
# this is further going to multiplied in envoy-integration tests by the
# this is further going to multiplied in envoy-integration tests by the
# other dimensions in the matrix. Currently TOTAL_RUNNERS would be
# multiplied by 8 based on these values:
# envoy-version: ["1.24.10", "1.25.9", "1.26.4", "1.27.0"]
Expand All @@ -281,7 +281,7 @@ jobs:
| jq --raw-input --argjson runnercount "$NUM_RUNNERS" "$JQ_SLICER" \
| jq --compact-output 'map(join("|"))'
} >> "$GITHUB_OUTPUT"
envoy-integration-test:
runs-on: ${{ fromJSON(needs.setup.outputs.compute-large) }}
needs:
Expand Down Expand Up @@ -384,7 +384,7 @@ jobs:
contents: read
env:
ENVOY_VERSION: "1.25.4"
CONSUL_DATAPLANE_IMAGE: "docker.io/hashicorppreview/consul-dataplane:1.3-dev"
CONSUL_DATAPLANE_IMAGE: "docker.io/hashicorppreview/consul-dataplane:1.3-dev-ubi"
steps:
- uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
# NOTE: This step is specifically needed for ENT. It allows us to access the required private HashiCorp repos.
Expand Down Expand Up @@ -417,7 +417,7 @@ jobs:
if: steps.buildConsulEnvoyImage.outcome == 'failure'
run: docker build -t consul-envoy:target-version --build-arg CONSUL_IMAGE=${{ env.CONSUL_LATEST_IMAGE_NAME }}:local --build-arg ENVOY_VERSION=${{ env.ENVOY_VERSION }} -f ./test/integration/consul-container/assets/Dockerfile-consul-envoy ./test/integration/consul-container/assets
- name: Build consul-dataplane:local image
run: docker build -t consul-dataplane:local --build-arg CONSUL_DATAPLANE_IMAGE=${{ env.CONSUL_DATAPLANE_IMAGE }} -f ./test/integration/consul-container/assets/Dockerfile-consul-dataplane ./test/integration/consul-container/assets
run: docker build -t consul-dataplane:local --build-arg CONSUL_IMAGE=${{ env.CONSUL_LATEST_IMAGE_NAME }}:local --build-arg CONSUL_DATAPLANE_IMAGE=${{ env.CONSUL_DATAPLANE_IMAGE }} -f ./test/integration/consul-container/assets/Dockerfile-consul-dataplane ./test/integration/consul-container/assets
- name: Configure GH workaround for ipv6 loopback
if: ${{ !endsWith(github.repository, '-enterprise') }}
run: |
Expand Down
2 changes: 1 addition & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ test-compat-integ-setup: dev-docker
@docker run --rm -t $(CONSUL_COMPAT_TEST_IMAGE):local consul version
@# 'consul-envoy:target-version' is needed by compatibility integ test
@docker build -t consul-envoy:target-version --build-arg CONSUL_IMAGE=$(CONSUL_COMPAT_TEST_IMAGE):local --build-arg ENVOY_VERSION=${ENVOY_VERSION} -f ./test/integration/consul-container/assets/Dockerfile-consul-envoy ./test/integration/consul-container/assets
@docker build -t consul-dataplane:local --build-arg CONSUL_DATAPLANE_IMAGE=${CONSUL_DATAPLANE_IMAGE} -f ./test/integration/consul-container/assets/Dockerfile-consul-dataplane ./test/integration/consul-container/assets
@docker build -t consul-dataplane:local --build-arg CONSUL_IMAGE=$(CONSUL_COMPAT_TEST_IMAGE):local --build-arg CONSUL_DATAPLANE_IMAGE=${CONSUL_DATAPLANE_IMAGE} -f ./test/integration/consul-container/assets/Dockerfile-consul-dataplane ./test/integration/consul-container/assets

.PHONY: test-compat-integ
test-compat-integ: test-compat-integ-setup ## Test compat integ
Expand Down
14 changes: 13 additions & 1 deletion test/integration/consul-container/assets/Dockerfile-consul-dataplane
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,28 @@
# SPDX-License-Identifier: BUSL-1.1

ARG CONSUL_DATAPLANE_IMAGE
ARG CONSUL_IMAGE

# Docker doesn't support expansion in COPY --copy, so we need to create an intermediate image.
FROM ${CONSUL_IMAGE} as consul

FROM ${CONSUL_DATAPLANE_IMAGE} as consuldataplane

USER root

# On Mac M1s when TProxy is enabled, consul-dataplane that are spawned from this image
# (only used in consul-container integration tests) will terminate with the below error.
# It is related to tproxy-startup.sh calling iptables SDK which then calls the underly
# iptables. We are investigating how this works on M1s with consul-envoy images which
# do not have this problem. For the time being tproxy tests on Mac M1s will fail locally
# but pass in CI.
#
# Error setting up traffic redirection rules: failed to run command: /sbin/iptables -t nat -N CONSUL_PROXY_INBOUND, err: exit status 1, output: iptables: Failed to initialize nft: Protocol not supported
RUN microdnf install -y iptables sudo nc \
&& usermod -a -G wheel consul-dataplane \
&& echo 'consul-dataplane ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers

COPY --from=consul:local /bin/consul /bin/consul
COPY --from=consul /bin/consul /bin/consul

COPY tproxy-startup.sh /bin/tproxy-startup.sh
RUN chmod +x /bin/tproxy-startup.sh && chown root:root /bin/tproxy-startup.sh
Expand Down
47 changes: 24 additions & 23 deletions test/integration/consul-container/libs/cluster/dataplane.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,37 +92,16 @@ func NewConsulDataplane(ctx context.Context, proxyID string, serverAddresses str
copy(exposedPorts, appPortStrs)
exposedPorts = append(exposedPorts, adminPortStr)

command := []string{
"consul-dataplane",
"-addresses", serverAddresses,
fmt.Sprintf("-grpc-port=%d", grpcPort),
fmt.Sprintf("-proxy-id=%s", proxyID),
"-proxy-namespace=default",
"-proxy-partition=default",
"-log-level=info",
"-log-json=false",
"-envoy-concurrency=2",
"-tls-disabled",
fmt.Sprintf("-envoy-admin-bind-port=%d", internalAdminPort),
}

if bootstrapToken != "" {
command = append(command,
"-credential-type=static",
fmt.Sprintf("-static-token=%s", bootstrapToken))
}

command = append(command, containerArgs...)

req := testcontainers.ContainerRequest{
Image: "consul-dataplane:local",
WaitingFor: wait.ForLog("").WithStartupTimeout(60 * time.Second),
AutoRemove: false,
Name: containerName,
Cmd: command,
Env: map[string]string{},
}

var command []string

if tproxy {
req.Entrypoint = []string{"sh", "/bin/tproxy-startup.sh"}
req.Env["REDIRECT_TRAFFIC_ARGS"] = strings.Join(
Expand All @@ -142,8 +121,30 @@ func NewConsulDataplane(ctx context.Context, proxyID string, serverAddresses str
" ",
)
req.CapAdd = append(req.CapAdd, "NET_ADMIN")
command = append(command, "consul-dataplane")
}

command = append(command,
"-addresses", serverAddresses,
fmt.Sprintf("-grpc-port=%d", grpcPort),
fmt.Sprintf("-proxy-id=%s", proxyID),
"-proxy-namespace=default",
"-proxy-partition=default",
"-log-level=info",
"-log-json=false",
"-envoy-concurrency=2",
"-tls-disabled",
fmt.Sprintf("-envoy-admin-bind-port=%d", internalAdminPort),
)

if bootstrapToken != "" {
command = append(command,
"-credential-type=static",
fmt.Sprintf("-static-token=%s", bootstrapToken))
}

req.Cmd = append(command, containerArgs...)

info, err := LaunchContainerOnNode(ctx, node, req, exposedPorts)
if err != nil {
return nil, err
Expand Down
103 changes: 44 additions & 59 deletions ...iner/test/traffic_permissions/tcp_test.go → ...ainer/test/trafficpermissions/tcp_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
// Copyright (c) HashiCorp, Inc.
// SPDX-License-Identifier: BUSL-1.1

package tproxy
package trafficpermissions

import (
"context"
"fmt"
"strings"
"testing"
"time"

"github.com/hashicorp/consul/sdk/testutil/retry"

Expand All @@ -32,8 +31,6 @@ const (
staticServerIdentity = "static-server-identity"
)

var requestRetryTimer = &retry.Timer{Timeout: 20 * time.Second, Wait: 500 * time.Millisecond}

type trafficPermissionsCase struct {
tp1 *pbauth.TrafficPermissions
tp2 *pbauth.TrafficPermissions
Expand All @@ -43,14 +40,19 @@ type trafficPermissionsCase struct {
client2EchoSuccess bool
}

// We are using tproxy to test traffic permissions now because explicitly specifying destinations
// doesn't work when multiple downstreams specify the same destination yet. In the future, we will need
// to update this to use explicit destinations once we infer tproxy destinations from traffic permissions.
//
// This also explicitly uses virtual IPs and virtual ports because Consul DNS doesn't support v2 resources yet.
// We should update this to use Consul DNS when it is working.
func runTrafficPermissionsTests(t *testing.T, aclsEnabled bool, cases map[string]trafficPermissionsCase) {
t.Parallel()

cluster, resourceClient := createCluster(t, aclsEnabled)

serverService, serverDataplane := createServerResources(t, resourceClient, cluster, cluster.Agents[1])
client1Dataplane := createClientResources(t, resourceClient, cluster, serverService, cluster.Agents[2], 1)
client2Dataplane := createClientResources(t, resourceClient, cluster, serverService, cluster.Agents[3], 2)
serverDataplane := createServerResources(t, resourceClient, cluster, cluster.Agents[1])
client1Dataplane := createClientResources(t, resourceClient, cluster, cluster.Agents[2], 1)
client2Dataplane := createClientResources(t, resourceClient, cluster, cluster.Agents[3], 2)

assertDataplaneContainerState(t, client1Dataplane, "running")
assertDataplaneContainerState(t, client2Dataplane, "running")
Expand All @@ -63,7 +65,7 @@ func runTrafficPermissionsTests(t *testing.T, aclsEnabled bool, cases map[string

// We must establish a new TCP connection each time because TCP traffic permissions are
// enforced at the connection level.
retry.RunWith(requestRetryTimer, t, func(r *retry.R) {
retry.Run(t, func(r *retry.R) {
assertPassing(r, httpRequestToVirtualAddress, client1Dataplane, tc.client1TCPSuccess)
assertPassing(r, echoToVirtualAddress, client1Dataplane, tc.client1EchoSuccess)
assertPassing(r, httpRequestToVirtualAddress, client2Dataplane, tc.client2TCPSuccess)
Expand Down Expand Up @@ -295,6 +297,30 @@ func TestTrafficPermission_TCP_DefaultAllow(t *testing.T) {
client2TCPSuccess: true,
client2EchoSuccess: true,
},
"empty allow denies everything": {
tp1: &pbauth.TrafficPermissions{
Destination: &pbauth.Destination{
IdentityName: staticServerIdentity,
},
Action: pbauth.Action_ACTION_ALLOW,
},
client1TCPSuccess: false,
client1EchoSuccess: false,
client2TCPSuccess: false,
client2EchoSuccess: false,
},
"empty deny denies everything": {
tp1: &pbauth.TrafficPermissions{
Destination: &pbauth.Destination{
IdentityName: staticServerIdentity,
},
Action: pbauth.Action_ACTION_DENY,
},
client1TCPSuccess: false,
client1EchoSuccess: false,
client2TCPSuccess: false,
client2EchoSuccess: false,
},
"allow everything": {
tp1: &pbauth.TrafficPermissions{
Destination: &pbauth.Destination{
Expand All @@ -318,8 +344,7 @@ func TestTrafficPermission_TCP_DefaultAllow(t *testing.T) {
client2TCPSuccess: true,
client2EchoSuccess: true,
},
// TODO I don't like this behavior.
"allow one protocol doesnt impact the other protocol": {
"allow one protocol denies the other protocol": {
tp1: &pbauth.TrafficPermissions{
Destination: &pbauth.Destination{
IdentityName: staticServerIdentity,
Expand All @@ -343,9 +368,9 @@ func TestTrafficPermission_TCP_DefaultAllow(t *testing.T) {
},
},
client1TCPSuccess: true,
client1EchoSuccess: true,
client1EchoSuccess: false,
client2TCPSuccess: true,
client2EchoSuccess: true,
client2EchoSuccess: false,
},
"allow something unrelated": {
tp1: &pbauth.TrafficPermissions{
Expand Down Expand Up @@ -426,8 +451,8 @@ func storeStaticServerTrafficPermissions(t *testing.T, resourceClient *rtest.Cli
}
}

func createServerResources(t *testing.T, resourceClient *rtest.Client, cluster *libcluster.Cluster, node libcluster.Agent) (*pbresource.Resource, *libcluster.ConsulDataplaneContainer) {
serverService := rtest.ResourceID(&pbresource.ID{
func createServerResources(t *testing.T, resourceClient *rtest.Client, cluster *libcluster.Cluster, node libcluster.Agent) *libcluster.ConsulDataplaneContainer {
rtest.ResourceID(&pbresource.ID{
Name: "static-server-service",
Type: pbcatalog.ServiceType,
}).
Expand Down Expand Up @@ -483,10 +508,10 @@ func createServerResources(t *testing.T, resourceClient *rtest.Client, cluster *
serverDataplane, err := createServiceAndDataplane(t, node, cluster, "static-server-workload", "static-server", 8080, 8079, []int{})
require.NoError(t, err)

return serverService, serverDataplane
return serverDataplane
}

func createClientResources(t *testing.T, resourceClient *rtest.Client, cluster *libcluster.Cluster, staticServerRef *pbresource.Resource, node libcluster.Agent, idx int) *libcluster.ConsulDataplaneContainer {
func createClientResources(t *testing.T, resourceClient *rtest.Client, cluster *libcluster.Cluster, node libcluster.Agent, idx int) *libcluster.ConsulDataplaneContainer {
prefix := fmt.Sprintf("static-client-%d", idx)
rtest.ResourceID(&pbresource.ID{
Name: prefix + "-service",
Expand Down Expand Up @@ -580,30 +605,10 @@ func assertDataplaneContainerState(t *testing.T, dataplane *libcluster.ConsulDat
func httpRequestToVirtualAddress(dp *libcluster.ConsulDataplaneContainer) (string, error) {
addr := fmt.Sprintf("%s:%d", staticServerVIP, tcpPort)

// Test that we can make a request to the virtual ip to reach the upstream.
//
// NOTE(pglass): This uses a workaround for DNS because I had trouble modifying
// /etc/resolv.conf. There is a --dns option to docker run, but it
// didn't seem to be exposed via testcontainers. I'm not sure if it would
// do what I want. In any case, Docker sets up /etc/resolv.conf for certain
// functionality so it seems better to leave DNS alone.
//
// But, that means DNS queries aren't redirected to Consul out of the box.
// As a workaround, we `dig @localhost:53` which is iptables-redirected to
// localhost:8600 where the Consul client responds with the virtual ip.
//
// In tproxy tests, Envoy is not configured with a unique listener for each
// upstream. This means the usual approach for non-tproxy tests doesn't
// work - where we send the request to a host address mapped in to Envoy's
// upstream listener. Instead, we exec into the container and run curl.
//
// We must make this request with a non-envoy user. The envoy and consul
// users are excluded from traffic redirection rules, so instead we
// make the request as root.
out, err := dp.Exec(
context.Background(),
[]string{"sudo", "sh", "-c", fmt.Sprintf(`
set -e
set -e
curl -s "%s/debug?env=dump"
`, addr),
},
Expand All @@ -622,26 +627,6 @@ set -e
}

func echoToVirtualAddress(dp *libcluster.ConsulDataplaneContainer) (string, error) {
// Test that we can make a request to the virtual ip to reach the upstream.
//
// NOTE(pglass): This uses a workaround for DNS because I had trouble modifying
// /etc/resolv.conf. There is a --dns option to docker run, but it
// didn't seem to be exposed via testcontainers. I'm not sure if it would
// do what I want. In any case, Docker sets up /etc/resolv.conf for certain
// functionality so it seems better to leave DNS alone.
//
// But, that means DNS queries aren't redirected to Consul out of the box.
// As a workaround, we `dig @localhost:53` which is iptables-redirected to
// localhost:8600 where the Consul client responds with the virtual ip.
//
// In tproxy tests, Envoy is not configured with a unique listener for each
// upstream. This means the usual approach for non-tproxy tests doesn't
// work - where we send the request to a host address mapped in to Envoy's
// upstream listener. Instead, we exec into the container and run curl.
//
// We must make this request with a non-envoy user. The envoy and consul
// users are excluded from traffic redirection rules, so instead we
// make the request as root.
out, err := dp.Exec(
context.Background(),
[]string{"sudo", "sh", "-c", fmt.Sprintf(`
Expand All @@ -662,7 +647,7 @@ func echoToVirtualAddress(dp *libcluster.ConsulDataplaneContainer) (string, erro
return out, err
}

func assertPassing(t require.TestingT, fn func(*libcluster.ConsulDataplaneContainer) (string, error), dp *libcluster.ConsulDataplaneContainer, success bool) {
func assertPassing(t *retry.R, fn func(*libcluster.ConsulDataplaneContainer) (string, error), dp *libcluster.ConsulDataplaneContainer, success bool) {
_, err := fn(dp)
if success {
require.NoError(t, err)
Expand Down

0 comments on commit 6af4fe2

Please sign in to comment.