Skip to content

Add a watcher for rotating agentless EC2 nodes#25477

Merged
lxea merged 1 commit intomasterfrom
lxea/adcr-discovery
May 25, 2023
Merged

Add a watcher for rotating agentless EC2 nodes#25477
lxea merged 1 commit intomasterfrom
lxea/adcr-discovery

Conversation

@lxea
Copy link
Copy Markdown
Contributor

@lxea lxea commented May 2, 2023

depends on #24194

Adds an watcher for agentless nodes that uses a node watcher to match appropriate EC2 nodes and re-run teleport openssh join on them.

In the event that a rotation phase is missed, there is a timed watcher that will run on any nodes that havent updated their rotation status.

@lxea lxea marked this pull request as ready for review May 2, 2023 16:03
@lxea lxea requested review from espadolini and r0mant May 2, 2023 16:03
@github-actions github-actions Bot requested a review from kimlisa May 2, 2023 16:04
@lxea lxea force-pushed the lxea/adcr-discovery branch from 903167c to 0a03880 Compare May 10, 2023 12:42
@lxea lxea changed the base branch from master to lxea/adcr-client May 10, 2023 16:05
Comment thread lib/srv/discovery/discovery.go Outdated
Comment thread api/types/installers/agentless-installer.sh.tmpl Outdated
Comment thread api/types/installers/agentless-installer.sh.tmpl Outdated
Comment thread lib/srv/discovery/discovery.go Outdated
Comment thread lib/srv/discovery/discovery.go Outdated
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.

We have a very similar logic in lib/service/connect.go:(*TeleportProcess).syncRotationStateCycle. Is there any way to reuse it or hook into it to avoid duplicating the same watcher? Maybe parameterize it with the types of CAs we want to watch and have it emit some hook/event we can subscribe to here in the discovery service?

@espadolini What do you think, would that make sense?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This watcher is different enough that I don't think it would be worth it to unify them (here we only act on certain phase changes, for instance).

A bigger problem is that until we figure out how to do reloadless CA rotations, this nice watcher loop will just get closed whenever the host CA is rotated (other than theinit and rollback phases, at least) - as it is, it's kinda pointless to watch for host CA rotations of the local CA, since we're just going to be closing the TeleportProcess at some unspecified time in the future, so you don't really have guaranteed time to do anything.

Comment thread lib/srv/server/ec2_watcher.go Outdated
@lxea lxea force-pushed the lxea/adcr-client branch 2 times, most recently from 7288617 to dc44f60 Compare May 11, 2023 10:40
@lxea lxea force-pushed the lxea/adcr-discovery branch 6 times, most recently from 9e21ccc to fecd932 Compare May 12, 2023 15:56
@lxea lxea force-pushed the lxea/adcr-client branch from 7cf4be0 to a43e183 Compare May 12, 2023 16:08
@lxea lxea force-pushed the lxea/adcr-discovery branch 2 times, most recently from 32d6607 to 7fb8986 Compare May 15, 2023 10:22
@lxea lxea force-pushed the lxea/adcr-client branch from a43e183 to 6d94d52 Compare May 15, 2023 11:50
@lxea lxea force-pushed the lxea/adcr-discovery branch 2 times, most recently from 09db2bf to 6a97f82 Compare May 16, 2023 10:30
@lxea lxea force-pushed the lxea/adcr-client branch from 45340c3 to 44d52f8 Compare May 16, 2023 11:13
@lxea lxea force-pushed the lxea/adcr-discovery branch from 2cb65ec to 77e6965 Compare May 16, 2023 11:13
@lxea lxea force-pushed the lxea/adcr-client branch from 44d52f8 to 2e05c3d Compare May 16, 2023 15:28
@lxea lxea force-pushed the lxea/adcr-discovery branch from e75f8d8 to 344132c Compare May 16, 2023 15:30
@lxea lxea force-pushed the lxea/adcr-client branch from f84dd10 to 1fdc16d Compare May 17, 2023 10:38
@lxea lxea force-pushed the lxea/adcr-discovery branch from 344132c to ccbc338 Compare May 17, 2023 10:38
@lxea lxea requested a review from r0mant May 18, 2023 09:28
@lxea lxea force-pushed the lxea/adcr-discovery branch from d960aa5 to 3980692 Compare May 18, 2023 09:31
Comment thread api/types/installers/agentless-installer.sh.tmpl Outdated
Comment thread lib/srv/discovery/discovery.go Outdated
Comment thread lib/srv/discovery/discovery.go Outdated
Comment thread lib/srv/discovery/discovery.go Outdated
Comment thread lib/srv/discovery/discovery.go Outdated
Comment thread lib/srv/server/ec2_watcher.go Outdated
Comment thread lib/srv/server/ec2_watcher.go Outdated
Comment thread lib/srv/server/watcher.go Outdated
Comment thread lib/srv/server/ec2_watcher.go Outdated
Comment thread lib/srv/server/ec2_watcher.go Outdated
@lxea lxea force-pushed the lxea/adcr-discovery branch from 8e01bf7 to 36057f7 Compare May 19, 2023 10:46
@lxea lxea force-pushed the lxea/adcr-client branch from 0fa836c to 8d55488 Compare May 22, 2023 10:06
@lxea lxea force-pushed the lxea/adcr-discovery branch 2 times, most recently from 0e712a3 to 324fa73 Compare May 22, 2023 11:35
@lxea lxea requested a review from r0mant May 22, 2023 12:25
Base automatically changed from lxea/adcr-client to master May 22, 2023 12:39
@lxea lxea force-pushed the lxea/adcr-discovery branch from 324fa73 to 2401241 Compare May 23, 2023 09:17
Copy link
Copy Markdown
Collaborator

@r0mant r0mant left a comment

Choose a reason for hiding this comment

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

Mostly a bunch of naming nitpicks.

Comment thread lib/srv/discovery/discovery.go Outdated
Comment thread lib/srv/discovery/discovery.go Outdated
Comment thread lib/srv/discovery/discovery.go Outdated
Comment thread lib/srv/discovery/discovery.go Outdated
Comment thread lib/srv/server/ec2_watcher.go Outdated
Comment thread lib/srv/server/ec2_watcher.go Outdated
Comment thread lib/srv/server/ec2_watcher.go Outdated
@lxea lxea force-pushed the lxea/adcr-discovery branch from d3ca2e9 to e0f7993 Compare May 24, 2023 08:54
Comment thread lib/srv/discovery/discovery.go Outdated
Comment thread lib/srv/discovery/discovery.go Outdated
Comment thread lib/srv/discovery/discovery.go Outdated
Comment thread lib/srv/server/ec2_watcher.go Outdated
Comment thread lib/srv/server/ec2_watcher.go Outdated
Resolve comments

Remove the agentless watcher and re-use the existing ec2 watcher

Fix getMostRecentRotationForCAs logic

use the clustername from conn.ClientIdentity

get instances once initially and then only from channels

After or Equal last rotation when fetching nodes

resolve comments

Remove the ca rotation watcher and rely on the timer

Resolve comments

resolve comments
@lxea lxea force-pushed the lxea/adcr-discovery branch from e0f7993 to e7074d1 Compare May 25, 2023 09:44
@lxea lxea enabled auto-merge May 25, 2023 09:45
@lxea lxea added this pull request to the merge queue May 25, 2023
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 25, 2023
@lxea lxea added this pull request to the merge queue May 25, 2023
Merged via the queue into master with commit eab8880 May 25, 2023
@lxea lxea deleted the lxea/adcr-discovery branch May 25, 2023 10:29
@public-teleport-github-review-bot
Copy link
Copy Markdown

@lxea See the table below for backport results.

Branch Result
branch/v13 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants