Add teleport-update binary scaffolding and disable command #46418

sclevine · 2024-09-10T01:24:03Z

This PR contains scaffolding and config file management for the teleport-update binary described by #40190.

The teleport-update binary will be used to enable, disable, and trigger automatic Teleport agent updates. The new auto-updates system manages a local installation of the cluster-specified version of Teleport stored in /var/lib/teleport/versions.

This is the first in a series of PRs extracted and tested from this proof-of-concept: #46357

The disable command is not useful without enable, but it is implemented to drive out configuration management.

RFD: #47126
Goal (internal): https://github.com/gravitational/cloud/issues/10289

Example:

root@71aeb39eabec:/# cat /var/lib/teleport/versions/updates.yaml
version: v1
kind: updates
spec:
    proxy: example.com
    group: ""
    url_template: ""
    enabled: true
    active_version: 16.2.0
root@71aeb39eabec:/# ./teleport/teleport-update disable
root@71aeb39eabec:/# ./teleport/teleport-update disable
2024-09-10T01:19:16Z INFO [UPDATER]   Automatic updates already disabled teleport-update/main.go:272
root@71aeb39eabec:/# cat /var/lib/teleport/versions/updates.yaml
version: v1
kind: updates
spec:
    proxy: example.com
    group: ""
    url_template: ""
    enabled: false
    active_version: 16.2.0
root@71aeb39eabec:/# rm /var/lib/teleport/versions/updates.yaml
root@71aeb39eabec:/# ./teleport/teleport-update disable
2024-09-10T01:20:18Z INFO [UPDATER]   Automatic updates already disabled teleport-update/main.go:272

github-actions · 2024-09-10T01:24:41Z

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

espadolini · 2024-09-10T09:40:57Z

AFAIK there were some concerns about new binaries that had a name that had teleport as a prefix in our build process. cc @fheinecke

tool/teleport-update/main.go

espadolini · 2024-09-10T15:08:46Z

tool/teleport-update/main.go

+		Proxy: func(req *http.Request) (*url.URL, error) {
+			return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
+		},


Do we gain much from using x/net/http/httpproxy over just http.ProxyFromEnvironment?

I copied the webclient for consistency:

teleport/api/client/webclient/webclient.go

Lines 92 to 112 in f2b0d11

func newWebClient(cfg *Config) (*http.Client, error) {

if err := cfg.CheckAndSetDefaults(); err != nil {

return nil, trace.Wrap(err)

}

rt := utils.NewHTTPRoundTripper(&http.Transport{

TLSClientConfig: &tls.Config{

InsecureSkipVerify: cfg.Insecure,

RootCAs: cfg.Pool,

},

Proxy: func(req *http.Request) (*url.URL, error) {

return httpproxy.FromEnvironment().ProxyFunc()(req.URL)

},

IdleConnTimeout: defaults.DefaultIOTimeout,

}, nil)

return &http.Client{

Transport: tracehttp.NewTransport(rt),

Timeout: cfg.Timeout,

}, nil

}

Would you prefer I use http.ProxyFromEnvironment instead?

We should definitely update the webclient too, that's quite inefficient - the standard library uses a sync.Once to store the returned ProxyFunc() from the vendored copy of x/net/http/httpproxy.

FWIW I would just take the lib/defaults.Transport() and add the TLSClientConfig to it unless there's a reason to avoid that.

The webclient uses tracehttp -- should I avoid that?

Switched to the defaults transport to merge -- let me know if I should bring tracehttp back in the future. Not sure If we need the telemetry provided by that package.

tool/teleport-update/main.go

espadolini · 2024-09-10T15:25:01Z

tool/teleport-update/main.go

+
+	app := libutils.InitCLIParser("teleport-updater", appHelp).Interspersed(false)
+	app.Flag("debug", "Verbose logging to stdout.").Short('d').BoolVar(&ccfg.Debug)
+	app.Flag("data-dir", "Directory to store teleport versions. Access to this directory should be limited.").StringVar(&ccfg.DataDir)


I'd prefer the config file to be an option (containing the path to a data dir) rather than the data dir being an option and the config file being in a fixed position in the data dir, but that's sort of a matter of style.

From an end-user perspective, this should always be the equivalent of /var/lib/teleport. Users shouldn't have to worry about multiple data directories. The directory is shared with Teleport, because there is (limited) interaction between the tools (e.g., DB backups, host UUID)

Additionally, the data directory contains more than just the config file (e.g., teleport versions, DB backups, DB metadata files).

fheinecke · 2024-09-10T18:01:06Z

Thanks for catching this @espadolini. It's probably not a huge issue here if we throughly test all releases and processes (dev workflows, releases, etc.) first. I was apprehensive about this with fdpass because we were on a very fast timetable and it added unnecessary risk.

sclevine · 2024-10-08T02:17:01Z

@hugoShaka mind giving this another review?

tool/teleport-update/main.go

sclevine · 2024-10-09T22:05:55Z

Looking for an additional review 🙂

tool/teleport-update/main.go

espadolini · 2024-10-10T08:33:59Z

tool/teleport-update/main.go

+		Proxy: func(req *http.Request) (*url.URL, error) {
+			return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
+		},


We should definitely update the webclient too, that's quite inefficient - the standard library uses a sync.Once to store the returned ProxyFunc() from the vendored copy of x/net/http/httpproxy.

FWIW I would just take the lib/defaults.Transport() and add the TLSClientConfig to it unless there's a reason to avoid that.

* Add main.go * wip * group flag * wip * wip * mvp * wip * separate files * cleanup * jitter * scaffold only * remove teleport changes * remove teleport changes - group * test * test lock * remove edition * feedback * clarify default data dir * cleanup * move version to status * consistent naming for update.yaml * improve lock test * explain lint * use shared locking logic * fix test * Move disable logic to lib * feedback * switch to default transport

sclevine requested review from hugoShaka and vapopov September 10, 2024 01:24

github-actions bot added the size/md label Sep 10, 2024

github-actions bot requested review from capnspacehook and espadolini September 10, 2024 01:24

sclevine added the no-changelog Indicates that a PR does not require a changelog entry label Sep 10, 2024

espadolini reviewed Sep 10, 2024

View reviewed changes

sclevine requested a review from espadolini September 10, 2024 20:29

sclevine force-pushed the sclevine/teleport-update-scaffold branch from 3e7b2e2 to 02476f1 Compare October 8, 2024 02:24

hugoShaka approved these changes Oct 9, 2024

View reviewed changes

tool/teleport-update/main.go Show resolved Hide resolved

espadolini approved these changes Oct 10, 2024

View reviewed changes

public-teleport-github-review-bot bot removed request for vapopov and capnspacehook October 10, 2024 08:34

sclevine added 12 commits October 10, 2024 14:15

Add main.go

661b3ad

wip

b3fafc4

group flag

dea4689

wip

0d09faf

wip

510e09d

mvp

3d7fdba

wip

f305b78

separate files

e877b01

cleanup

abc5aed

jitter

afa0474

scaffold only

344eff8

remove teleport changes

7ce4d83

sclevine added 15 commits October 10, 2024 14:15

test

13fd14a

test lock

85e4a19

remove edition

f451e06

feedback

0202eed

clarify default data dir

c68657e

cleanup

f050b06

move version to status

83b09f8

consistent naming for update.yaml

2338152

improve lock test

ffaf8a0

explain lint

08e5761

use shared locking logic

d7fcf01

fix test

7b05e1e

Move disable logic to lib

0ef48e6

feedback

f36917b

switch to default transport

9cfde07

sclevine force-pushed the sclevine/teleport-update-scaffold branch from 1c1e7d5 to 9cfde07 Compare October 10, 2024 18:35

sclevine added this pull request to the merge queue Oct 10, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 10, 2024

sclevine added this pull request to the merge queue Oct 10, 2024

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 10, 2024

sclevine added this pull request to the merge queue Oct 10, 2024

Merged via the queue into master with commit aae732a Oct 10, 2024
41 checks passed

sclevine deleted the sclevine/teleport-update-scaffold branch October 10, 2024 19:44

sclevine mentioned this pull request Oct 15, 2024

[teleport-update] Add enable command #47565

Merged

This was referenced Oct 24, 2024

[teleport-update] Add linking into /usr/local #47879

Merged

[teleport-update] Add support for reloading the agent & reverting symlinks on failed reload #47929

Merged

sclevine mentioned this pull request Oct 31, 2024

[teleport-update] Add update subcommand #48244

Merged

sclevine mentioned this pull request Nov 8, 2024

[teleport-update] Add link subcommand #48712

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add teleport-update binary scaffolding and disable command #46418

Add teleport-update binary scaffolding and disable command #46418

sclevine commented Sep 10, 2024 •

edited

Loading

github-actions bot commented Sep 10, 2024

espadolini commented Sep 10, 2024

espadolini Sep 10, 2024

sclevine Sep 10, 2024 •

edited

Loading

espadolini Oct 10, 2024

sclevine Oct 10, 2024

sclevine Oct 10, 2024

espadolini Sep 10, 2024

sclevine Sep 10, 2024

fheinecke commented Sep 10, 2024

sclevine commented Oct 8, 2024

sclevine commented Oct 9, 2024

espadolini Oct 10, 2024

	func newWebClient(cfg Config) (http.Client, error) {
	if err := cfg.CheckAndSetDefaults(); err != nil {
	return nil, trace.Wrap(err)
	}

	rt := utils.NewHTTPRoundTripper(&http.Transport{
	TLSClientConfig: &tls.Config{
	InsecureSkipVerify: cfg.Insecure,
	RootCAs: cfg.Pool,
	},
	Proxy: func(req http.Request) (url.URL, error) {
	return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
	},
	IdleConnTimeout: defaults.DefaultIOTimeout,
	}, nil)

	return &http.Client{
	Transport: tracehttp.NewTransport(rt),
	Timeout: cfg.Timeout,
	}, nil
	}

Add teleport-update binary scaffolding and disable command #46418

Add teleport-update binary scaffolding and disable command #46418

Conversation

sclevine commented Sep 10, 2024 • edited Loading

github-actions bot commented Sep 10, 2024

espadolini commented Sep 10, 2024

espadolini Sep 10, 2024

Choose a reason for hiding this comment

sclevine Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

espadolini Oct 10, 2024

Choose a reason for hiding this comment

sclevine Oct 10, 2024

Choose a reason for hiding this comment

sclevine Oct 10, 2024

Choose a reason for hiding this comment

espadolini Sep 10, 2024

Choose a reason for hiding this comment

sclevine Sep 10, 2024

Choose a reason for hiding this comment

fheinecke commented Sep 10, 2024

sclevine commented Oct 8, 2024

sclevine commented Oct 9, 2024

espadolini Oct 10, 2024

Choose a reason for hiding this comment

sclevine commented Sep 10, 2024 •

edited

Loading

sclevine Sep 10, 2024 •

edited

Loading