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

[v15] Benchmarks Will Now Be Tested During CI #45867

Merged
merged 2 commits into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions .github/workflows/benchmark-code-nonroot.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
name: Benchmarks (Go)
run-name: Benchmarks (Go) - ${{ github.run_id }} - @${{ github.actor }}

on:
pull_request:

merge_group:

jobs:
changes:
name: Check for relevant changes
runs-on: ubuntu-latest
permissions:
pull-requests: read
outputs:
changed: ${{ steps.changes.outputs.changed }}
steps:
- name: Checkout
if: ${{ github.event_name == 'merge_group' }}
uses: actions/checkout@v4
- uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
id: changes
with:
base: ${{ github.event.pull_request.base.ref || github.event.merge_group.base_ref }}
ref: ${{ github.event.pull_request.head.ref || github.event.merge_group.head_ref }}
filters: |
changed:
- '.github/workflows/benchmark-code-nonroot.yaml'
- '**.go'
- 'go.mod'
- 'go.sum'
- 'build.assets/Makefile'
- 'build.assets/Dockerfile*'
- 'Makefile'
test:
name: Benchmarks (Go)
needs: changes
if: ${{ !startsWith(github.head_ref, 'dependabot/') && needs.changes.outputs.changed == 'true' }}
runs-on: ubuntu-22.04-32core

permissions:
contents: read

container:
image: ghcr.io/gravitational/teleport-buildbox:teleport17
env:
TELEPORT_XAUTH_TEST: yes
WEBASSETS_SKIP_BUILD: 1

steps:
- name: Checkout Teleport
uses: actions/checkout@v4

- name: Prepare workspace
id: prepare
uses: ./.github/actions/prepare-workspace

# Run benchmarks once to make sure they don't break
# Must be run separate since gotestsum is not compatible with benchmark output
- name: Run Benchmarks Once
timeout-minutes: 5
shell: bash # Overriding default shell which is `sh -e`
run: make test-go-bench | sed -u -E "s/^(FAIL\s+github)/::error title=Benchmark Failed::\1/"

- name: Construct Summary
shell: bash
run: |
echo '```' >> $GITHUB_STEP_SUMMARY
go run golang.org/x/perf/cmd/benchstat@latest test-logs/bench.txt \
| sed -E -e 's/^pkg:\s+(github.*)/\n```\n## \1\n\n```/' >> "$GITHUB_STEP_SUMMARY"
71 changes: 71 additions & 0 deletions .github/workflows/benchmark-code-root.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
name: Benchmarks (Root) (Go)
run-name: Benchmarks (Root) (Go) - ${{ github.run_id }} - @${{ github.actor }}

on:
pull_request:

merge_group:

jobs:
changes:
name: Check for relevant changes
runs-on: ubuntu-latest
permissions:
pull-requests: read
outputs:
changed: ${{ steps.changes.outputs.changed }}
steps:
- name: Checkout
if: ${{ github.event_name == 'merge_group' }}
uses: actions/checkout@v4
- uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
id: changes
with:
base: ${{ github.event.pull_request.base.ref || github.event.merge_group.base_ref }}
ref: ${{ github.event.pull_request.head.ref || github.event.merge_group.head_ref }}
filters: |
changed:
- '.github/workflows/benchmark-code-root.yaml'
- '**.go'
- 'go.mod'
- 'go.sum'
- 'build.assets/Makefile'
- 'build.assets/Dockerfile*'
- 'Makefile'
test:
name: Benchmarks (Root)
needs: changes
if: ${{ !startsWith(github.head_ref, 'dependabot/') && needs.changes.outputs.changed == 'true' }}
runs-on: ubuntu-22.04-32core

permissions:
contents: read

container:
image: ghcr.io/gravitational/teleport-buildbox:teleport17
options: --cap-add=SYS_ADMIN --privileged
env:
WEBASSETS_SKIP_BUILD: 1
TELEPORT_XAUTH_TEST: yes

steps:
- name: Checkout Teleport
uses: actions/checkout@v4

- name: Prepare workspace
uses: ./.github/actions/prepare-workspace

# Run benchmarks once to make sure they don't break
# Must be run separate since gotestsum is not compatible with benchmark output
- name: Run Benchmarks Once
timeout-minutes: 5
shell: bash # Overriding default shell which is `sh -e`
run: make test-go-bench-root | sed -u -E "s/^(FAIL\s+github)/::error title=Benchmark Failed::\1/"

- name: Construct Summary
shell: bash
run: |
echo '```' >> $GITHUB_STEP_SUMMARY
go run golang.org/x/perf/cmd/benchstat@latest test-logs/bench.txt \
| sed -E -e 's/^pkg:\s+(github.*)/\n```\n## \1\n\n```/' >> "$GITHUB_STEP_SUMMARY"
1 change: 1 addition & 0 deletions .github/workflows/unit-tests-code.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ jobs:
uses: actions/checkout@v4

- name: Prepare workspace
id: prepare
uses: ./.github/actions/prepare-workspace

- name: Mount debugfs
Expand Down
19 changes: 19 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,25 @@ ifneq ("$(TOUCHID_TAG)", "")
| gotestsum --raw-command -- cat
endif

# Runs benchmarks once to make sure they pass.
# This is intended to run in CI during unit testing to make sure benchmarks don't break.
# To limit noise and improve speed this will only run on packages that have benchmarks.
# Race detection is not enabled because it significantly slows down benchmarks.
# todo: Use gotestsum when it is compatible with benchmark output. Currently will consider all benchmarks failed.
.PHONY: test-go-bench
test-go-bench: PACKAGES = $(shell grep --exclude-dir api --include "*_test.go" -lr testing.B . | xargs dirname | xargs go list | sort -u)
test-go-bench: BENCHMARK_SKIP_PATTERN = "^BenchmarkRoot|^BenchmarkGetMaxNodes$$"
test-go-bench: | $(TEST_LOG_DIR)
go test -run ^$$ -bench . -skip $(BENCHMARK_SKIP_PATTERN) -benchtime 1x $(PACKAGES) \
| tee $(TEST_LOG_DIR)/bench.txt

test-go-bench-root: PACKAGES = $(shell grep --exclude-dir api --include "*_test.go" -lr BenchmarkRoot . | xargs dirname | xargs go list | sort -u)
test-go-bench-root: BENCHMARK_PATTERN = "^BenchmarkRoot"
test-go-bench-root: BENCHMARK_SKIP_PATTERN = "^BenchmarkRootExecCommand$$"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know that the exclusions for BenchmarkGetMaxNodes and BenchmarkRootExecCommand are required anymore

test-go-bench-root: | $(TEST_LOG_DIR)
go test -run ^$$ -bench $(BENCHMARK_PATTERN) -skip $(BENCHMARK_SKIP_PATTERN) -benchtime 1x $(PACKAGES) \
| tee $(TEST_LOG_DIR)/bench.txt

# Runs ci tsh tests
.PHONY: test-go-tsh
test-go-tsh: FLAGS ?= -race -shuffle on
Expand Down
153 changes: 69 additions & 84 deletions lib/multiplexer/multiplexer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -828,42 +828,20 @@ func TestMux(t *testing.T) {
// If listener for IPv6 will fail to be created we'll skip IPv6 portion of test.
listener6, _ := net.Listen("tcp6", "[::1]:0")

startServing := func(muxListener net.Listener, cluster string) (*Mux, *httptest.Server) {
mux, err := New(Config{
Listener: muxListener,
PROXYProtocolMode: PROXYProtocolUnspecified,
CertAuthorityGetter: casGetter,
Clock: clockwork.NewFakeClockAt(time.Now()),
LocalClusterName: cluster,
})
require.NoError(t, err)

muxTLSListener := mux.TLS()

go mux.Serve()

backend := &httptest.Server{
Listener: muxTLSListener,

Config: &http.Server{
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprint(w, r.RemoteAddr)
}),
},
}
backend.StartTLS()

return mux, backend
server := muxServer{
certAuthorityGetter: casGetter,
}

mux4, backend4 := startServing(listener4, clusterName)
mux4, backend4, err := server.startServing(listener4, clusterName)
require.NoError(t, err)
defer mux4.Close()
defer backend4.Close()

var backend6 *httptest.Server
var mux6 *Mux
if listener6 != nil {
mux6, backend6 = startServing(listener6, clusterName)
mux6, backend6, err = server.startServing(listener6, clusterName)
require.NoError(t, err)
defer mux6.Close()
defer backend6.Close()
}
Expand Down Expand Up @@ -1062,7 +1040,8 @@ func TestMux(t *testing.T) {
require.NoError(t, err)

// start multiplexer with wrong cluster name specified
mux, backend := startServing(listener, "different-cluster")
mux, backend, err := server.startServing(listener, "different-cluster")
require.NoError(t, err)
t.Cleanup(func() {
require.NoError(t, mux.Close())
backend.Close()
Expand Down Expand Up @@ -1270,75 +1249,37 @@ func startSSHServer(t *testing.T, listener net.Listener) {

func BenchmarkMux_ProxyV2Signature(b *testing.B) {
const clusterName = "test-teleport"

clock := clockwork.NewFakeClockAt(time.Now())
tlsProxyCert, caGetter, jwtSigner := getTestCertCAsGetterAndSigner(b, clusterName)

ca, err := caGetter(context.Background(), types.CertAuthID{
Type: types.HostCA,
DomainName: clusterName,
}, false)
require.NoError(b, err)

roots := x509.NewCertPool()
ok := roots.AppendCertsFromPEM(ca.GetTrustedTLSKeyPairs()[0].Cert)
require.True(b, ok)

ip := "1.2.3.4"
sAddr := net.TCPAddr{IP: net.ParseIP(ip), Port: 444}
dAddr := net.TCPAddr{IP: net.ParseIP(ip), Port: 555}
listener4, err := net.Listen("tcp", "127.0.0.1:")
require.NoError(b, err)

server := muxServer{
disableTLS: true,
certAuthorityGetter: caGetter,
}
mux4, backend4, err := server.startServing(listener4, clusterName)
require.NoError(b, err)
defer mux4.Close()
defer backend4.Close()

b.Run("simulation of signing and verifying PROXY header", func(b *testing.B) {
for n := 0; n < b.N; n++ {
token, err := jwtSigner.SignPROXYJWT(jwt.PROXYSignParams{
ClusterName: clusterName,
SourceAddress: sAddr.String(),
DestinationAddress: dAddr.String(),
})
require.NoError(b, err)

pl := ProxyLine{
Protocol: TCP4,
Source: sAddr,
Destination: dAddr,
}
err = pl.AddSignature([]byte(token), tlsProxyCert)
conn, err := net.Dial("tcp", listener4.Addr().String())
require.NoError(b, err)
defer conn.Close()

_, err = pl.Bytes()
signedHeader, err := signPROXYHeader(&sAddr, &dAddr, clusterName, tlsProxyCert, jwtSigner)
require.NoError(b, err)

cert, err := tlsca.ParseCertificatePEM(tlsProxyCert)
require.NoError(b, err)
chains, err := cert.Verify(x509.VerifyOptions{Roots: roots})
_, err = conn.Write(signedHeader)
require.NoError(b, err)
require.NotNil(b, chains)

identity, err := tlsca.FromSubject(cert.Subject, cert.NotAfter)
out, err := utils.RoundtripWithConn(conn)
require.NoError(b, err)

foundRole := checkForSystemRole(identity, types.RoleProxy)
require.True(b, foundRole, "Missing 'Proxy' role on the signing certificate")

// Check JWT using proxy cert's public key
jwtVerifier, err := jwt.New(&jwt.Config{
Clock: clock,
PublicKey: cert.PublicKey,
Algorithm: defaults.ApplicationTokenAlgorithm,
ClusterName: clusterName,
})
require.NoError(b, err, "Could not create JWT verifier")

claims, err := jwtVerifier.VerifyPROXY(jwt.PROXYVerifyParams{
ClusterName: clusterName,
SourceAddress: sAddr.String(),
DestinationAddress: dAddr.String(),
RawToken: token,
})
require.NoError(b, err, "Got an error while verifying PROXY JWT")
require.NotNil(b, claims)
require.Equal(b, fmt.Sprintf("%s/%s", sAddr.String(), dAddr.String()), claims.Subject,
"IP addresses in PROXY header don't match JWT")
require.Equal(b, sAddr.String(), out)
}
})
}
Expand Down Expand Up @@ -1437,3 +1378,47 @@ func TestIsDifferentTCPVersion(t *testing.T) {
fmt.Sprintf("Unexpected result for %q, %q", tt.addr1, tt.addr2))
}
}

type muxServer struct {
certAuthorityGetter func(ctx context.Context, id types.CertAuthID, loadKeys bool) (types.CertAuthority, error)
disableTLS bool
}

func (m *muxServer) startServing(muxListener net.Listener, cluster string) (*Mux, *httptest.Server, error) {
mux, err := New(Config{
Listener: muxListener,
PROXYProtocolMode: PROXYProtocolUnspecified,
CertAuthorityGetter: m.certAuthorityGetter,
Clock: clockwork.NewFakeClockAt(time.Now()),
LocalClusterName: cluster,
})
if err != nil {
return mux, &httptest.Server{}, err
}

if m.disableTLS {
muxListener = mux.HTTP()
} else {
muxListener = mux.TLS()
}

go mux.Serve()

backend := &httptest.Server{
Listener: muxListener,

Config: &http.Server{
Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
fmt.Fprint(w, r.RemoteAddr)
}),
},
}

if m.disableTLS {
backend.Start()
} else {
backend.StartTLS()
}

return mux, backend, nil
}
Loading