From b2e150e7d705169452d9f4733d2448f93db33abc Mon Sep 17 00:00:00 2001 From: Nic Klaassen Date: Fri, 20 Feb 2026 18:50:09 -0800 Subject: [PATCH] test: stabilize TestIntegrations/SessionRecordingModes by isolating per-subtest user/role state (#64033) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test: stabilize TestIntegrations/SessionRecordingModes by isolating per-subtest user/role state Fixes #48043 `TestIntegrations/SessionRecordingModes` was flaky because the subtests reused the same Teleport user and role while switching recording mode between `strict` and `best_effort`. Under cache propagation delay/staleness, one subtest could observe the previous subtest’s role state, causing incorrect behavior (for example, `BestEffortMode` failing with `ssh: could not start shell` as if strict mode was still active). For local reproduction, I introduced a temporary debug cache fault to drop role update events after initial cache population. With `StrictMode` run before `BestEffortMode`, that reliably reproduced the failure by forcing stale role reads in later subtests. The fix removes shared mutable identity state from the test: - create a unique Teleport user per mode subtest - create a unique role per mode subtest This makes each subtest evaluate recording mode against isolated auth state, instead of depending on cache timing/order. Validation: - with the temporary cache fault enabled, the old test shape reproduced the failure while the isolated user/role version passed - after removing the temporary fault, `go test ./integration -run 'TestIntegrations/SessionRecordingModes' -count=50` passes with the final code * stop loading test keys into local SSH agent --- integration/helpers/fixture.go | 1 - integration/helpers/instance.go | 1 + integration/integration_test.go | 27 +++++++++++++++++---------- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/integration/helpers/fixture.go b/integration/helpers/fixture.go index cdfd878c713..374b08bba82 100644 --- a/integration/helpers/fixture.go +++ b/integration/helpers/fixture.go @@ -54,7 +54,6 @@ func NewFixture(t *testing.T) *Fixture { fixture.Priv, fixture.Pub, err = testauthority.GenerateKeyPair() require.NoError(t, err) - // Find AllocatePortsNum free listening ports to use. fixture.Me, err = user.Current() require.NoError(t, err) diff --git a/integration/helpers/instance.go b/integration/helpers/instance.go index 4483d2b9140..ffa4b2ed1b8 100644 --- a/integration/helpers/instance.go +++ b/integration/helpers/instance.go @@ -1525,6 +1525,7 @@ func (i *TeleInstance) NewUnauthenticatedClient(cfg ClientConfig) (tc *client.Te Stdout: cfg.Stdout, NonInteractive: true, DisableSSHResumption: cfg.DisableSSHResumption, + AddKeysToAgent: client.AddKeysToAgentNo, } // JumpHost turns on jump host mode diff --git a/integration/integration_test.go b/integration/integration_test.go index ffe2fcec5d0..cfa80456d0b 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -889,15 +889,16 @@ func testSessionRecordingModes(t *testing.T, suite *integrationTestSuite) { // startSession starts an interactive session, users must terminate the // session by typing "exit" in the terminal. - startSession := func(username string) (*Terminal, chan error) { + startSession := func(login, teleportUser string) (*Terminal, chan error) { term := NewTerminal(250) errCh := make(chan error) go func() { cl, err := teleport.NewClient(helpers.ClientConfig{ - Login: username, - Cluster: helpers.Site, - Host: Host, + Login: login, + TeleportUser: teleportUser, + Cluster: helpers.Site, + Host: Host, }) if err != nil { errCh <- trace.Wrap(err) @@ -946,10 +947,15 @@ func testSessionRecordingModes(t *testing.T, suite *integrationTestSuite) { } { t.Run(name, func(t *testing.T) { // Setup user and session recording mode. - username := suite.Me.Username - role, err := types.NewRole("devs", types.RoleSpecV6{ + login := suite.Me.Username + // Use unique Teleport user and role names per subtest to avoid + // cross-test cache/state reuse when role mappings are updated. + id := uuid.NewString()[:8] + teleportUser := fmt.Sprintf("%s-%s", strings.ToLower(name), id) + roleName := fmt.Sprintf("devs-%s", id) + role, err := types.NewRole(roleName, types.RoleSpecV6{ Allow: types.RoleConditions{ - Logins: []string{username}, + Logins: []string{login}, NodeLabels: types.Labels{types.Wildcard: []string{types.Wildcard}}, }, Options: types.RoleOptions{ @@ -959,7 +965,7 @@ func testSessionRecordingModes(t *testing.T, suite *integrationTestSuite) { }, }) require.NoError(t, err) - require.NoError(t, helpers.SetupUser(teleport.Process, username, []types.Role{role})) + require.NoError(t, helpers.SetupUser(teleport.Process, teleportUser, []types.Role{role})) t.Run("BeforeStartFailure", func(t *testing.T) { // Enable disk failure. @@ -967,7 +973,7 @@ func testSessionRecordingModes(t *testing.T, suite *integrationTestSuite) { defer disableDiskFailure() // Start session. - term, errCh := startSession(username) + term, errCh := startSession(login, teleportUser) if test.expectSessionFailure { waitSessionTermination(t, errCh, require.Error) return @@ -990,7 +996,7 @@ func testSessionRecordingModes(t *testing.T, suite *integrationTestSuite) { t.Run("MidSessionFailure", func(t *testing.T) { // Start session. - term, errCh := startSession(username) + term, errCh := startSession(login, teleportUser) // Guarantee the session started properly. select { @@ -1016,6 +1022,7 @@ func testSessionRecordingModes(t *testing.T, suite *integrationTestSuite) { term.Type("exit\n\r") waitSessionTermination(t, errCh, require.NoError) }) + }) } }