Skip to content

Commit

Permalink
Fix flaky test TestProxySSHJumpHost (#40052)
Browse files Browse the repository at this point in the history 
* Use testServer.MakeTestServer to fix issue with parallel runs using the same ports.

* Add deprecation comments for test server helpers outside of tools/teleport/testenv.

* Update tool/teleport/testenv/test_server.go

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

* Disable SSH resumption

Enable LoadAllCAs

Update comments

* Prevent context.Canceled from being wrapped across transport stream

trail doesn't properly handle conversions to and from errors that
are context.Canceled which results in code paths trying to evaluate
errors.Is(err, context.Canceled) failing. The error recevied is
an interceptors.RemoteError, which wraps a trace.TraceErr, which
contains a status.Status with codes.Canceled. To work around this
until trail is updated the server half of the stream was updated
to return an io.EOF if if encouters a canceled error.

---------

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
Co-authored-by: Tim Ross <tim.ross@goteleport.com>
  • Loading branch information
@Joerger @rosstimothy
3 people authored Apr 4, 2024
1 parent 48aa797 commit 5c930ca
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 84 deletions.
5 changes: 4 additions & 1 deletion api/utils/grpc/stream/stream.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
"time"

"github.com/gravitational/trace"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

// MaxChunkSize is the maximum number of bytes to send in a single data message.
Expand Down Expand Up @@ -70,9 +72,10 @@ func (c *ReadWriter) Read(b []byte) (n int, err error) {

if len(c.rBytes) == 0 {
data, err := c.source.Recv()
if errors.Is(err, io.EOF) {
if errors.Is(err, io.EOF) || status.Code(err) == codes.Canceled {
return 0, io.EOF
}

if err != nil {
return 0, trace.ConnectionProblem(trace.Wrap(err), "failed to receive from source: %v", err)
}
Expand Down
33 changes: 32 additions & 1 deletion tool/teleport/testenv/test_server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"time"

"github.com/gravitational/trace"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/gravitational/teleport/api/breaker"
Expand Down Expand Up @@ -276,7 +277,7 @@ type TestServerOptFunc func(o *TestServersOpts)

func WithBootstrap(bootstrap ...types.Resource) TestServerOptFunc {
return func(o *TestServersOpts) {
o.Bootstrap = bootstrap
o.Bootstrap = append(o.Bootstrap, bootstrap...)
}
}

Expand Down Expand Up @@ -330,6 +331,36 @@ func WithAuthPreference(authPref types.AuthPreference) TestServerOptFunc {
})
}

func SetupTrustedCluster(ctx context.Context, t *testing.T, rootServer, leafServer *service.TeleportProcess, additionalRoleMappings ...types.RoleMapping) {
rootProxyAddr, err := rootServer.ProxyWebAddr()
require.NoError(t, err)
rootProxyTunnelAddr, err := rootServer.ProxyTunnelAddr()
require.NoError(t, err)

tc, err := types.NewTrustedCluster("root-cluster", types.TrustedClusterSpecV2{
Enabled: true,
Token: staticToken,
ProxyAddress: rootProxyAddr.String(),
ReverseTunnelAddress: rootProxyTunnelAddr.String(),
RoleMap: append(additionalRoleMappings,
types.RoleMapping{
Remote: "access",
Local: []string{"access"},
},
),
})
require.NoError(t, err)

_, err = leafServer.GetAuthServer().UpsertTrustedCluster(ctx, tc)
require.NoError(t, err)

require.EventuallyWithT(t, func(t *assert.CollectT) {
rt, err := rootServer.GetAuthServer().GetTunnelConnections("leaf")
assert.NoError(t, err)
assert.Len(t, rt, 1)
}, time.Second*10, time.Second)
}

type cliModules struct{}

func (p *cliModules) GenerateAccessRequestPromotions(_ context.Context, _ modules.AccessResourcesGetter, _ types.AccessRequest) (*types.AccessRequestAllowedPromotions, error) {
Expand Down
1 change: 1 addition & 0 deletions tool/tsh/common/app_aws_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ func makeUserWithAWSRole(t *testing.T) (types.User, types.Role) {
return alice, awsRole
}

// deprecated: Use `tools/teleport/testenv.MakeTestServer` instead.
func makeTestApplicationServer(t *testing.T, proxy *service.TeleportProcess, apps ...servicecfg.App) *service.TeleportProcess {
// Proxy uses self-signed certificates in tests.
lib.SetInsecureDevMode(true)
Expand Down
171 changes: 89 additions & 82 deletions tool/tsh/common/proxy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ import (
"github.com/gravitational/teleport/lib/service/servicecfg"
"github.com/gravitational/teleport/lib/teleagent"
"github.com/gravitational/teleport/lib/tlsca"
testserver "github.com/gravitational/teleport/tool/teleport/testenv"
)

// TestSSH verifies "tsh ssh" command.
Expand Down Expand Up @@ -558,101 +559,107 @@ func TestProxySSH(t *testing.T) {

// TestProxySSHJumpHost verifies "tsh proxy ssh -J" functionality.
func TestProxySSHJumpHost(t *testing.T) {
ctx := context.Background()

lib.SetInsecureDevMode(true)
t.Cleanup(func() { lib.SetInsecureDevMode(false) })

createAgent(t)

tests := []struct {
name string
opts []testSuiteOptionFunc
}{
{
name: "TLS routing enabled for root and leaf",
opts: []testSuiteOptionFunc{
withRootConfigFunc(func(cfg *servicecfg.Config) {
cfg.Auth.NetworkingConfig.SetProxyListenerMode(types.ProxyListenerMode_Multiplex)
cfg.Auth.SessionRecordingConfig.SetMode(types.RecordOff)
}),
withLeafConfigFunc(func(cfg *servicecfg.Config) {
cfg.Auth.NetworkingConfig.SetProxyListenerMode(types.ProxyListenerMode_Multiplex)
cfg.Auth.SessionRecordingConfig.SetMode(types.RecordOff)
}),
},
},
{
name: "TLS routing enabled for root and disabled for leaf",
opts: []testSuiteOptionFunc{
withRootConfigFunc(func(cfg *servicecfg.Config) {
cfg.Auth.NetworkingConfig.SetProxyListenerMode(types.ProxyListenerMode_Multiplex)
cfg.Auth.SessionRecordingConfig.SetMode(types.RecordOff)
}),
withLeafConfigFunc(func(cfg *servicecfg.Config) {
cfg.Auth.NetworkingConfig.SetProxyListenerMode(types.ProxyListenerMode_Separate)
cfg.Auth.SessionRecordingConfig.SetMode(types.RecordOff)
}),
},
},
{
name: "TLS routing disabled for root and enabled for leaf",
opts: []testSuiteOptionFunc{
withRootConfigFunc(func(cfg *servicecfg.Config) {
cfg.Auth.NetworkingConfig.SetProxyListenerMode(types.ProxyListenerMode_Separate)
cfg.Auth.SessionRecordingConfig.SetMode(types.RecordOff)
}),
withLeafConfigFunc(func(cfg *servicecfg.Config) {
cfg.Auth.NetworkingConfig.SetProxyListenerMode(types.ProxyListenerMode_Multiplex)
cfg.Auth.SessionRecordingConfig.SetMode(types.RecordOff)
}),
},
},
{
name: "TLS routing disabled for root and leaf",
opts: []testSuiteOptionFunc{
withRootConfigFunc(func(cfg *servicecfg.Config) {
cfg.Auth.NetworkingConfig.SetProxyListenerMode(types.ProxyListenerMode_Separate)
cfg.Auth.SessionRecordingConfig.SetMode(types.RecordOff)
}),
withLeafConfigFunc(func(cfg *servicecfg.Config) {
cfg.Auth.NetworkingConfig.SetProxyListenerMode(types.ProxyListenerMode_Separate)
cfg.Auth.SessionRecordingConfig.SetMode(types.RecordOff)
}),
},
},
listenerModes := []types.ProxyListenerMode{
types.ProxyListenerMode_Multiplex,
types.ProxyListenerMode_Separate,
}

for _, tc := range tests {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
for _, rootListenerMode := range listenerModes {
for _, leafListenerMode := range listenerModes {
rootListenerMode := rootListenerMode
leafListenerMode := leafListenerMode
t.Run(fmt.Sprintf("Proxy listener mode %s for root and %s for leaf", rootListenerMode, leafListenerMode), func(t *testing.T) {
t.Parallel()

s := newTestSuite(t, tc.opts...)
accessUser, err := types.NewUser("access")
require.NoError(t, err)
accessUser.SetRoles([]string{"access"})

runProxySSHJump := func(opts ...CliOption) error {
proxyRequest := fmt.Sprintf("%s:%d",
s.leaf.Config.Proxy.SSHAddr.Host(),
s.leaf.Config.SSH.Addr.Port(defaults.SSHServerListenPort))
return Run(context.Background(), []string{
"--insecure", "proxy", "ssh", "-J", s.leaf.Config.Proxy.WebAddr.Addr, proxyRequest,
}, opts...)
}
user, err := user.Current()
require.NoError(t, err)
accessUser.SetLogins([]string{user.Name})

connector := mockConnector(t)
rootServerOpts := []testserver.TestServerOptFunc{
testserver.WithBootstrap(connector, accessUser),
testserver.WithHostname("node01"),
testserver.WithClusterName(t, "root"),
testserver.WithAuthConfig(
func(cfg *servicecfg.AuthConfig) {
cfg.NetworkingConfig.SetProxyListenerMode(rootListenerMode)
// Disable session recording to prevent writing to disk after the test concludes.
cfg.SessionRecordingConfig.SetMode(types.RecordOff)
// Load all CAs on login so that leaf CA is trusted by clients.
cfg.LoadAllCAs = true
},
),
}
rootServer := testserver.MakeTestServer(t, rootServerOpts...)

leafServerOpts := []testserver.TestServerOptFunc{
testserver.WithBootstrap(accessUser),
testserver.WithHostname("node02"),
testserver.WithClusterName(t, "leaf"),
testserver.WithAuthConfig(
func(cfg *servicecfg.AuthConfig) {
cfg.NetworkingConfig.SetProxyListenerMode(leafListenerMode)
// Disable session recording to prevent writing to disk after the test concludes.
cfg.SessionRecordingConfig.SetMode(types.RecordOff)
},
),
}
leafServer := testserver.MakeTestServer(t, leafServerOpts...)
testserver.SetupTrustedCluster(ctx, t, rootServer, leafServer)

// login to root
tshHome, _ := mustLogin(t, s, s.root.Config.Auth.ClusterName.GetClusterName())
rootProxyAddr, err := rootServer.ProxyWebAddr()
require.NoError(t, err)
leafProxyAddr, err := leafServer.ProxyWebAddr()
require.NoError(t, err)

// Connect to leaf node though proxy jump host. This should automatically
// reissue leaf certs from the root without explicility switching clusters.
err := runProxySSHJump(setHomePath(tshHome))
require.NoError(t, err)
// login to root
tshHome := t.TempDir()
err = Run(ctx, []string{
"--insecure",
"login",
"--proxy", rootProxyAddr.String(),
}, setHomePath(tshHome), setMockSSOLogin(rootServer.GetAuthServer(), accessUser, connector.GetName()))
require.NoError(t, err)

// Terminate root cluster.
err = s.root.Close()
require.NoError(t, err)
// Connect through the leaf proxy jumphost.
err = Run(ctx, []string{
"--debug",
"proxy",
"ssh",
"--no-resume",
"-J", leafProxyAddr.String(),
"node02",
}, setHomePath(tshHome))
require.NoError(t, err)

// Since we've already retrieved valid leaf certs, we should be able to connect without root.
err = runProxySSHJump(setHomePath(tshHome))
require.NoError(t, err)
})
// Terminate root cluster.
err = rootServer.Close()
require.NoError(t, err)

// We should be able to connect without root online.
err = Run(ctx, []string{
"--debug",
"--insecure",
"proxy",
"ssh",
"--no-resume",
"-J", leafProxyAddr.String(),
"node02",
}, setHomePath(tshHome))
require.NoError(t, err)
})
}
}
}

Expand Down
1 change: 1 addition & 0 deletions tool/tsh/common/tsh_helper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ func withValidationFunc(f func(*suite) bool) testSuiteOptionFunc {
}
}

// deprecated: Use `tools/teleport/testenv.MakeTestServer` instead.
func newTestSuite(t *testing.T, opts ...testSuiteOptionFunc) *suite {
var options testSuiteOptions
for _, opt := range opts {
Expand Down
2 changes: 2 additions & 0 deletions tool/tsh/common/tsh_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3389,6 +3389,7 @@ func withSSHLabel(key, value string) testServerOptFunc {
})
}

// deprecated: Use `tools/teleport/testenv.MakeTestServer` instead.
func makeTestSSHNode(t *testing.T, authAddr *utils.NetAddr, opts ...testServerOptFunc) *service.TeleportProcess {
var options testServersOpts
for _, opt := range opts {
Expand Down Expand Up @@ -3418,6 +3419,7 @@ func makeTestSSHNode(t *testing.T, authAddr *utils.NetAddr, opts ...testServerOp
return runTeleport(t, cfg)
}

// deprecated: Use `tools/teleport/testenv.MakeTestServer` instead.
func makeTestServers(t *testing.T, opts ...testServerOptFunc) (auth *service.TeleportProcess, proxy *service.TeleportProcess) {
t.Helper()

Expand Down

0 comments on commit 5c930ca

Please sign in to comment.