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

TProxy integration test #17103

Merged
merged 5 commits into from
Apr 26, 2023
Merged

TProxy integration test #17103

merged 5 commits into from
Apr 26, 2023

Conversation

pglass
Copy link

@pglass pglass commented Apr 24, 2023

Description

This is a basic integration test for TProxy:

  • Create 2 "client" pods for the static-client / static-server. Separate pods are needed because iptables rules apply for the network namespace
  • Configure the test container with necessary permission for the iptables command:
    • NET_ADMIN container capability. Required to run iptables in a container.
    • sudo, so that the envoy user can run iptables commands
    • A tproxy-startup.sh script that runs the consul connect redirect-traffic command to apply iptables rules

This took some fiddling to setup, so there are some workarounds for the moment:

  • DNS queries don't automatically redirect to Consul DNS (on my machine)
  • Because the Consul client and envoy share a network namespace, the Consul inbound ports and the Consul user id are excluded from traffic redirection.

Testing & Reproduction steps

cd ./test/integration/consul-container
go test -v -timeout=30m ./test/tproxy/ \
    -run 'TestTProxyService' \
    --target-image consul --target-version local \
    --latest-image consul --latest-version latest

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@pglass pglass added pr/no-changelog PR does not need a corresponding .changelog entry pr/no-backport labels Apr 24, 2023
"-exclude-inbound-port", "8302",
"-exclude-inbound-port", "8500",
"-exclude-inbound-port", "8502",
"-exclude-inbound-port", "8600",
Copy link
Author

@pglass pglass Apr 24, 2023

Choose a reason for hiding this comment

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

These are all the Consul client ports that needed to be excluded from traffic redirection rules because Consul and Envoy share a network namespace.

Mode: mgwMode,
},
}},
}
Copy link
Author

@pglass pglass Apr 24, 2023

Choose a reason for hiding this comment

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

I configured this so that tproxy is a toggle (🤷). If tproxy is enabled, we do not need to include an explicit upstream. If tproxy is not enabled, then include the explicit upstream.

I'll defer to others on how we want to structure these.

@pglass pglass force-pushed the pglass/NET-3685/pmtls-integration-test branch from ef46c4f to 4f1373f Compare April 24, 2023 18:12
@pglass pglass marked this pull request as ready for review April 24, 2023 18:12
@pglass pglass requested a review from a team April 24, 2023 18:12
func assertHTTPRequestToVirtualAddress(t *testing.T, clientService libservice.Service) {
timer := &retry.Timer{Timeout: 120 * time.Second, Wait: 500 * time.Millisecond}

retry.RunWith(timer, t, func(r *retry.R) {
Copy link
Author

Choose a reason for hiding this comment

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

Added a retry loop for this check and moved into a function.

t.Logf("making call to upstream\nerr = %v\nout = %s", err, out)
require.NoError(r, err)
require.Regexp(r, `Virtual IP: 240.0.0.\d+`, out)
require.Contains(r, out, "FORTIO_NAME=static-server")
Copy link
Author

Choose a reason for hiding this comment

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

Updated this to validate the response is from the upstream static-server (similar to libassert.AssertFortioName)

@pglass pglass force-pushed the pglass/NET-3685/pmtls-integration-test branch from ef38f71 to 5334082 Compare April 25, 2023 15:53
Paul Glass added 2 commits April 25, 2023 12:07
Previously, when test splitting allocated multiple test directories to a
runner, the workflow ran `go tests "./test/dir1 ./test/dir2"` which
results in a directory not found error. This fixes that.
@pglass pglass merged commit b431b04 into main Apr 26, 2023
@pglass pglass deleted the pglass/NET-3685/pmtls-integration-test branch April 26, 2023 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/no-backport pr/no-changelog PR does not need a corresponding .changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants