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

Export endpoint config #42810

Merged
merged 7 commits into from
Jul 18, 2024
Merged

Export endpoint config #42810

merged 7 commits into from
Jul 18, 2024

Conversation

bernardjkim
Copy link
Contributor

Closes #41983

This PR registers an endpoint export function with the teleport process. On startup the proxy version endpoint will be exported to the /etc/teleport-upgrade.d/endpoint config. If an endpoint value is already configured, the value will not be overwritten.

This will allow the teleport updater to be compatible with the previous versions of the teleport config.

changelog: Fixes automatic updates with previous versions of the teleport.yaml config.

@bernardjkim bernardjkim marked this pull request as ready for review July 2, 2024 19:39
@github-actions github-actions bot requested review from greedy52 and strideynet July 2, 2024 19:39
lib/service/service.go Show resolved Hide resolved
@@ -1129,6 +1130,20 @@ func NewTeleport(cfg *servicecfg.Config) (*TeleportProcess, error) {
process.logger.WarnContext(process.ExitContext(), "Use of external upgraders on control-plane instances is not recommended.")
}

if upgraderKind == "unit" {
process.RegisterFunc("autoupdates.endpoint.export", func() error {
if err := endpoint.Export(process.ExitContext(), resolverAddr); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are joining via auth I think this will do weird things with the updater as it will set the endpoint override to auth.example.com:3025 which is not a valid version server.

This is not a possible case for Cloud, but this can and will happen in self-hosted setups. I'm not sure we can catch all cases where we connect through a proxy, but known cases are:

  • when cfg.ProxyServer is set
  • (ugly) when port is 443 or 3080

We can also make this logic cloud-specific by looking for the teleport.sh suffix in the resolver address.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made changes in 57cf444 so that this export function will only be run for Cloud instances.

Additionally, the export function will attempt to verify the endpoint first before writing the config. So if the resolver address results in an invalid version server, the endpoint will not be written.

@bernardjkim bernardjkim requested a review from hugoShaka July 10, 2024 23:32
Copy link
Contributor

@fheinecke fheinecke left a comment

Choose a reason for hiding this comment

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

This directly couples the updater with Teleport, and it'll now be coupled in both directions. Are we 100% sure that we want this?

Doing so will also remove any chance of the updater being versioned outside of Teleport.

@bernardjkim
Copy link
Contributor Author

This directly couples the updater with Teleport, and it'll now be coupled in both directions.

The updater is already dependent on Teleport to export an update schedule, so I think it has already been coupled in both directions.

We can handle this in the teleport updater logic, but correctly parsing the different teleport.yaml config versions in bash doesn't sound like the simplest task. I can give it a shot if you're really against adding any updater logic into teleport.

@bernardjkim bernardjkim requested a review from fheinecke July 16, 2024 23:06
@bernardjkim
Copy link
Contributor Author

Hey @fheinecke, actually I think it would be preferred to implement this logic inside Teleport because it would also help address the issue with the global version endpoint https://github.com/gravitational/cloud/issues/8361. There are still a number of users that have not upgraded their teleport updater from the deprecated version, and they still query the target agent version from the global version endpoint.

This change would allow agents to override that default without requiring the updater package itself to be updated.

@bernardjkim bernardjkim added this pull request to the merge queue Jul 18, 2024
Merged via the queue into master with commit 2eb7284 Jul 18, 2024
38 checks passed
@bernardjkim bernardjkim deleted the bernard/export-endpoint-config branch July 18, 2024 01:51
@public-teleport-github-review-bot

@bernardjkim See the table below for backport results.

Branch Result
branch/v14 Create PR
branch/v15 Create PR
branch/v16 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.

The updater cannot identify auto updates version endpoint if proxy_server is not specified
3 participants