Skip to content

[v18] Fix discrepancies between Node and Proxy recording modes.#59599

Closed
Joerger wants to merge 2 commits intobranch/v18from
joerger/v18/proxy-rec-server-id
Closed

[v18] Fix discrepancies between Node and Proxy recording modes.#59599
Joerger wants to merge 2 commits intobranch/v18from
joerger/v18/proxy-rec-server-id

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Sep 25, 2025

Backport #58707, #59610 to branch/v18

* Fix usage of node server ID and proxy host UUID in the forwarding server.

* Make GetInfo and TargetMetadata consistent between the forwarding and regular server.

* Rename TargetMetadata to EventMetadata and only use it for events' ServerMetadata.

* Remove unused method.

* Resolves comments on test.

* Remove unused fields.

* Update lib/srv/ctx.go

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

* Update integration/integration_test.go

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

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>
@Joerger Joerger added the no-changelog Indicates that a PR does not require a changelog entry label Sep 25, 2025
t.Run("PAM", suite.bind(testPAM))
t.Run("PortForwarding", suite.bind(testPortForwarding))
t.Run("ProxyHostKeyCheck", suite.bind(testProxyHostKeyCheck))
t.Run("RecordingModesSessionTrackers", suite.bind(testRecordingModesSessionTrackers))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This test looks to be flaky on master. Should we fix before backporting?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I've been bit by the test flake 2-3 times this afternoon, so I do think we want to fix this before merging, otherwise we could hold up merging any changes to release branches.

@Joerger Joerger requested a review from zmb3 September 29, 2025 17:18
@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Oct 6, 2025

@zmb3 Friendly ping to review, #59600 and #59601 as well

@zmb3
Copy link
Copy Markdown
Collaborator

zmb3 commented Oct 6, 2025

Sorry for the delay, @Joerger. I'm a little hesitant about these backports after the outage we had last week and need to find the time to take a closer look.

What amount of testing have we done on this?

@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Oct 6, 2025

Sorry for the delay, @Joerger. I'm a little hesitant about these backports after the outage we had last week and need to find the time to take a closer look.

What amount of testing have we done on this?

No worries, fair hesitation. I've tested between Proxy/Nodes in different recording modes and some testing between different versions. I'll take some time to formally manually test each combination as well as test it in Teleport Cloud this week.

@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Nov 17, 2025

Replaced by #61246

@Joerger Joerger closed this Nov 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants