From c2f7f396723d43e0b1650719ec8036b88f6b8571 Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Fri, 11 Oct 2024 20:25:49 -0400 Subject: [PATCH 01/18] clean up download logic --- lib/autoupdate/agent/installer.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/lib/autoupdate/agent/installer.go b/lib/autoupdate/agent/installer.go index e31813866eacf..4cba9e69b115d 100644 --- a/lib/autoupdate/agent/installer.go +++ b/lib/autoupdate/agent/installer.go @@ -161,6 +161,29 @@ func (li *LocalInstaller) Install(ctx context.Context, version, template string, return nil } +func extract(dstDir string, src io.Reader, max uint64) error { + if err := os.MkdirAll(dstDir, 0755); err != nil { + return trace.Wrap(err) + } + free, err := utils.FreeDiskWithReserve(dstDir, reservedFreeDisk) + if err != nil { + return trace.Errorf("failed to calculate free disk in %q: %w", dstDir, err) + } + // Bail if there's not enough free disk space at the target + if d := free - max; d < 0 { + return trace.Errorf("%q needs %d additional bytes of disk space for decompression", dstDir, -d) + } + zr, err := gzip.NewReader(src) + if err != nil { + return trace.Errorf("requires gzip-compressed body: %v", err) + } + err = utils.Extract(zr, dstDir) + if err != nil { + return trace.Wrap(err) + } + return nil +} + // makeURL to download the Teleport tgz. func makeURL(uriTmpl, version string, flags InstallFlags) (string, error) { tmpl, err := template.New("uri").Parse(uriTmpl) From ceeb3964f72340c312e1717cb4efda551770adf0 Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Fri, 11 Oct 2024 22:07:13 -0400 Subject: [PATCH 02/18] Finish installer tests --- lib/autoupdate/agent/installer.go | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/lib/autoupdate/agent/installer.go b/lib/autoupdate/agent/installer.go index 4cba9e69b115d..9d839cb51476a 100644 --- a/lib/autoupdate/agent/installer.go +++ b/lib/autoupdate/agent/installer.go @@ -161,29 +161,6 @@ func (li *LocalInstaller) Install(ctx context.Context, version, template string, return nil } -func extract(dstDir string, src io.Reader, max uint64) error { - if err := os.MkdirAll(dstDir, 0755); err != nil { - return trace.Wrap(err) - } - free, err := utils.FreeDiskWithReserve(dstDir, reservedFreeDisk) - if err != nil { - return trace.Errorf("failed to calculate free disk in %q: %w", dstDir, err) - } - // Bail if there's not enough free disk space at the target - if d := free - max; d < 0 { - return trace.Errorf("%q needs %d additional bytes of disk space for decompression", dstDir, -d) - } - zr, err := gzip.NewReader(src) - if err != nil { - return trace.Errorf("requires gzip-compressed body: %v", err) - } - err = utils.Extract(zr, dstDir) - if err != nil { - return trace.Wrap(err) - } - return nil -} - // makeURL to download the Teleport tgz. func makeURL(uriTmpl, version string, flags InstallFlags) (string, error) { tmpl, err := template.New("uri").Parse(uriTmpl) @@ -231,6 +208,7 @@ func readChecksum(path string) ([]byte, error) { func (li *LocalInstaller) getChecksum(ctx context.Context, url string) ([]byte, error) { ctx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return nil, trace.Wrap(err) @@ -302,6 +280,7 @@ func (li *LocalInstaller) extract(ctx context.Context, dstDir string, src io.Rea return trace.Wrap(err) } free, err := utils.FreeDiskWithReserve(dstDir, li.ReservedFreeInstallDisk) + if err != nil { return trace.Errorf("failed to calculate free disk in %q: %w", dstDir, err) } From 81e0722f280b46acfa63c1df760e1f6660d6c76b Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Wed, 16 Oct 2024 17:15:51 -0400 Subject: [PATCH 03/18] fips and ent support --- lib/autoupdate/agent/feature_ent.go | 26 ++++++++++++++++++++++++++ lib/autoupdate/agent/feature_fips.go | 26 ++++++++++++++++++++++++++ lib/autoupdate/agent/updater.go | 11 +++++++++++ 3 files changed, 63 insertions(+) create mode 100644 lib/autoupdate/agent/feature_ent.go create mode 100644 lib/autoupdate/agent/feature_fips.go diff --git a/lib/autoupdate/agent/feature_ent.go b/lib/autoupdate/agent/feature_ent.go new file mode 100644 index 0000000000000..7677823e1f068 --- /dev/null +++ b/lib/autoupdate/agent/feature_ent.go @@ -0,0 +1,26 @@ +//go:build webassets_ent +// +build webassets_ent + +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package agent + +func init() { + featureFlag |= flagEnt +} diff --git a/lib/autoupdate/agent/feature_fips.go b/lib/autoupdate/agent/feature_fips.go new file mode 100644 index 0000000000000..680a3f218c27f --- /dev/null +++ b/lib/autoupdate/agent/feature_fips.go @@ -0,0 +1,26 @@ +//go:build fips +// +build fips + +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package agent + +func init() { + featureFlag |= flagFIPS +} diff --git a/lib/autoupdate/agent/updater.go b/lib/autoupdate/agent/updater.go index 59df5f0b3ba85..c70cd34c12f2c 100644 --- a/lib/autoupdate/agent/updater.go +++ b/lib/autoupdate/agent/updater.go @@ -40,6 +40,17 @@ import ( libutils "github.com/gravitational/teleport/lib/utils" ) +var ( + featureFlag int +) + +const ( + // flagEnt represents enterprise version. + flagEnt = 1 << iota + // flagFIPS represents enterprise version with fips feature enabled. + flagFIPS +) + const ( // cdnURITemplate is the default template for the Teleport tgz download. cdnURITemplate = "https://cdn.teleport.dev/teleport{{if .Enterprise}}-ent{{end}}-v{{.Version}}-{{.OS}}-{{.Arch}}{{if .FIPS}}-fips{{end}}-bin.tar.gz" From 4853c04b07841b413d1ccd0307b8e28b2e54e17d Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Fri, 18 Oct 2024 13:43:21 -0400 Subject: [PATCH 04/18] feedback --- lib/autoupdate/agent/installer.go | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/autoupdate/agent/installer.go b/lib/autoupdate/agent/installer.go index 9d839cb51476a..ebdce6e7b9bfa 100644 --- a/lib/autoupdate/agent/installer.go +++ b/lib/autoupdate/agent/installer.go @@ -280,7 +280,6 @@ func (li *LocalInstaller) extract(ctx context.Context, dstDir string, src io.Rea return trace.Wrap(err) } free, err := utils.FreeDiskWithReserve(dstDir, li.ReservedFreeInstallDisk) - if err != nil { return trace.Errorf("failed to calculate free disk in %q: %w", dstDir, err) } From eadf952f1319895fbcbd10ef86d6ddcd4aabb77e Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Fri, 18 Oct 2024 14:52:09 -0400 Subject: [PATCH 05/18] move enterprise/fips to webapi --- lib/autoupdate/agent/feature_ent.go | 26 -------------------------- lib/autoupdate/agent/feature_fips.go | 26 -------------------------- lib/autoupdate/agent/updater.go | 11 ----------- 3 files changed, 63 deletions(-) delete mode 100644 lib/autoupdate/agent/feature_ent.go delete mode 100644 lib/autoupdate/agent/feature_fips.go diff --git a/lib/autoupdate/agent/feature_ent.go b/lib/autoupdate/agent/feature_ent.go deleted file mode 100644 index 7677823e1f068..0000000000000 --- a/lib/autoupdate/agent/feature_ent.go +++ /dev/null @@ -1,26 +0,0 @@ -//go:build webassets_ent -// +build webassets_ent - -/* - * Teleport - * Copyright (C) 2024 Gravitational, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -package agent - -func init() { - featureFlag |= flagEnt -} diff --git a/lib/autoupdate/agent/feature_fips.go b/lib/autoupdate/agent/feature_fips.go deleted file mode 100644 index 680a3f218c27f..0000000000000 --- a/lib/autoupdate/agent/feature_fips.go +++ /dev/null @@ -1,26 +0,0 @@ -//go:build fips -// +build fips - -/* - * Teleport - * Copyright (C) 2024 Gravitational, Inc. - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - */ - -package agent - -func init() { - featureFlag |= flagFIPS -} diff --git a/lib/autoupdate/agent/updater.go b/lib/autoupdate/agent/updater.go index c70cd34c12f2c..59df5f0b3ba85 100644 --- a/lib/autoupdate/agent/updater.go +++ b/lib/autoupdate/agent/updater.go @@ -40,17 +40,6 @@ import ( libutils "github.com/gravitational/teleport/lib/utils" ) -var ( - featureFlag int -) - -const ( - // flagEnt represents enterprise version. - flagEnt = 1 << iota - // flagFIPS represents enterprise version with fips feature enabled. - flagFIPS -) - const ( // cdnURITemplate is the default template for the Teleport tgz download. cdnURITemplate = "https://cdn.teleport.dev/teleport{{if .Enterprise}}-ent{{end}}-v{{.Version}}-{{.OS}}-{{.Arch}}{{if .FIPS}}-fips{{end}}-bin.tar.gz" From 1303bafecac6327c6f13b4e5ef5dc5bc3d65eea8 Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Tue, 15 Oct 2024 14:12:40 -0400 Subject: [PATCH 06/18] wip --- lib/autoupdate/agent/installer.go | 28 ++++++++++++++++-- lib/autoupdate/agent/updater.go | 2 ++ lib/utils/unpack.go | 48 +++++++++++++++++++++++++++++-- 3 files changed, 74 insertions(+), 4 deletions(-) diff --git a/lib/autoupdate/agent/installer.go b/lib/autoupdate/agent/installer.go index ebdce6e7b9bfa..ddfbe92b69cbc 100644 --- a/lib/autoupdate/agent/installer.go +++ b/lib/autoupdate/agent/installer.go @@ -49,6 +49,7 @@ const ( type LocalInstaller struct { // InstallDir contains each installation, named by version. InstallDir string + // LinkDir contains symlinks to the // HTTP is an HTTP client for downloading Teleport. HTTP *http.Client // Log contains a logger. @@ -293,8 +294,14 @@ func (li *LocalInstaller) extract(ctx context.Context, dstDir string, src io.Rea } li.Log.InfoContext(ctx, "Extracting Teleport tarball.", "path", dstDir, "size", max) - // TODO(sclevine): add variadic arg to Extract to extract teleport/ subdir into bin/. - err = utils.Extract(zr, dstDir) + err = utils.Extract(zr, dstDir, []utils.ExtractPath{ + {Src: "teleport/examples/systemd/teleport.service", Dst: "etc/systemd/teleport.service"}, + {Src: "teleport/examples", Skip: true}, + {Src: "teleport/README.md", Dst: "share/README.md"}, + {Src: "teleport/CHANGELOG.md", Dst: "share/CHANGELOG.md"}, + {Src: "teleport/VERSION", Dst: "share/VERSION"}, + {Src: "teleport", Dst: "bin"}, + }...) if err != nil { return trace.Wrap(err) } @@ -316,3 +323,20 @@ func uncompressedSize(f io.Reader) (int64, error) { } return n, nil } + +func (ai *LocalInstaller) Link(ctx context.Context, version string, linkDir string) error { + binDir := filepath.Join(ai.InstallDir, version, "bin") + + entries, err := os.ReadDir(binDir) + if err != nil { + return trace.Wrap(err) + } + for _, entry := range entries { + if entry.IsDir() { + continue + } + os.Symlink(filepath.Join(binDir, entry.Name()), filepath.Join(linkDir, entry.Name())) + + } + return nil +} diff --git a/lib/autoupdate/agent/updater.go b/lib/autoupdate/agent/updater.go index 59df5f0b3ba85..27eca08692a3e 100644 --- a/lib/autoupdate/agent/updater.go +++ b/lib/autoupdate/agent/updater.go @@ -161,6 +161,8 @@ type Installer interface { // Install the Teleport agent at version from the download template. // This function must be idempotent. Install(ctx context.Context, version, template string, flags InstallFlags) error + // Link the Teleport agent at version to the system location. + Link(ctx context.Context, version string) error // Remove the Teleport agent at version. // This function must be idempotent. Remove(ctx context.Context, version string) error diff --git a/lib/utils/unpack.go b/lib/utils/unpack.go index a32cff6ef76b9..427bd8c61a4b1 100644 --- a/lib/utils/unpack.go +++ b/lib/utils/unpack.go @@ -23,6 +23,7 @@ import ( "errors" "io" "os" + "path" "path/filepath" "strings" @@ -36,7 +37,9 @@ import ( // resulting files and directories are created using the current user context. // Extract will only unarchive files into dir, and will fail if the tarball // tries to write files outside of dir. -func Extract(r io.Reader, dir string) error { +// If paths are specified, only the specified paths are extracted. +// The destination specified in the first matching path is used. +func Extract(r io.Reader, dir string, paths ...ExtractPath) error { tarball := tar.NewReader(r) for { @@ -46,7 +49,9 @@ func Extract(r io.Reader, dir string) error { } else if err != nil { return trace.Wrap(err) } - + if ok := filterHeader(header, paths); !ok { + continue + } err = sanitizeTarPath(header, dir) if err != nil { return trace.Wrap(err) @@ -59,6 +64,45 @@ func Extract(r io.Reader, dir string) error { return nil } +// ExtractPath specifies a path to be extracted. +type ExtractPath struct { + // Src path prefix and Dst path within the archive to extract files to. + // Src path directories are not included in the extraction dir. + // Trialing slashes are always ignores. + Src, Dst string + // Skip the path entirely. + Skip bool +} + +func filterHeader(hdr *tar.Header, paths []ExtractPath) (ok bool) { + name := path.Clean(hdr.Name) + for _, p := range paths { + src := path.Clean(p.Src) + switch hdr.Typeflag { + case tar.TypeDir: + if src != "/" { + src += "/" + } + if !strings.HasPrefix(name, src) { + continue + } + dst := path.Join(p.Dst, strings.TrimPrefix(name, src)) + if dst != "/" { + dst += "/" + } + hdr.Name = dst + return !p.Skip + default: + if src != name { + continue + } + hdr.Name = path.Clean(p.Dst) + return !p.Skip + } + } + return len(paths) == 0 +} + // extractFile extracts a single file or directory from tarball into dir. // Uses header to determine the type of item to create // Based on https://github.com/mholt/archiver From f1f2572cc7e94f3de088b5a4fd339dad4d7446ce Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Wed, 16 Oct 2024 16:41:29 -0400 Subject: [PATCH 07/18] wip2 --- lib/autoupdate/agent/installer.go | 15 +++--- lib/autoupdate/agent/installer_test.go | 69 ++++++++++++++++++++++++++ lib/autoupdate/agent/updater.go | 17 ++++++- lib/autoupdate/agent/updater_test.go | 12 ++++- 4 files changed, 104 insertions(+), 9 deletions(-) diff --git a/lib/autoupdate/agent/installer.go b/lib/autoupdate/agent/installer.go index ddfbe92b69cbc..6646bbd19c289 100644 --- a/lib/autoupdate/agent/installer.go +++ b/lib/autoupdate/agent/installer.go @@ -34,6 +34,7 @@ import ( "text/template" "time" + "github.com/google/renameio/v2" "github.com/gravitational/trace" "github.com/gravitational/teleport/lib/utils" @@ -49,7 +50,8 @@ const ( type LocalInstaller struct { // InstallDir contains each installation, named by version. InstallDir string - // LinkDir contains symlinks to the + // LinkDir contains symlinks to the most recently linked installation. + LinkDir string // HTTP is an HTTP client for downloading Teleport. HTTP *http.Client // Log contains a logger. @@ -155,7 +157,7 @@ func (li *LocalInstaller) Install(ctx context.Context, version, template string, return trace.Errorf("failed to extract teleport: %w", err) } // Write the checksum last. This marks the version directory as valid. - err = os.WriteFile(sumPath, []byte(hex.EncodeToString(newSum)), 0755) + err = renameio.WriteFile(sumPath, []byte(hex.EncodeToString(newSum)), 0755) if err != nil { return trace.Errorf("failed to write checksum: %w", err) } @@ -324,9 +326,8 @@ func uncompressedSize(f io.Reader) (int64, error) { return n, nil } -func (ai *LocalInstaller) Link(ctx context.Context, version string, linkDir string) error { +func (ai *LocalInstaller) Link(ctx context.Context, version string) error { binDir := filepath.Join(ai.InstallDir, version, "bin") - entries, err := os.ReadDir(binDir) if err != nil { return trace.Wrap(err) @@ -335,8 +336,10 @@ func (ai *LocalInstaller) Link(ctx context.Context, version string, linkDir stri if entry.IsDir() { continue } - os.Symlink(filepath.Join(binDir, entry.Name()), filepath.Join(linkDir, entry.Name())) - + err := renameio.Symlink(filepath.Join(binDir, entry.Name()), filepath.Join(ai.LinkDir, entry.Name())) + if err != nil { + return trace.Wrap(err) + } } return nil } diff --git a/lib/autoupdate/agent/installer_test.go b/lib/autoupdate/agent/installer_test.go index be778f7bcf16a..095c67e80349a 100644 --- a/lib/autoupdate/agent/installer_test.go +++ b/lib/autoupdate/agent/installer_test.go @@ -187,3 +187,72 @@ func testTGZ(t *testing.T, version string) (tgz *bytes.Buffer, shasum string) { } return &buf, hex.EncodeToString(sha.Sum(nil)) } + +func TestTeleportInstaller_Link(t *testing.T) { + t.Parallel() + const version = "new-version" + + tests := []struct { + name string + files []string + dirs []string + + links []string + errMatch string + }{ + { + name: "present", + files: []string{"teleport", "tsh", "tbot"}, + dirs: []string{"somedir"}, + + links: []string{"teleport", "tsh", "tbot"}, + }, + { + name: "not present", + files: []string{"teleport"}, + + errMatch: "no such", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + versionsDir := t.TempDir() + versionDir := filepath.Join(versionsDir, version) + err := os.MkdirAll(versionDir, 0o755) + require.NoError(t, err) + + for _, n := range tt.files { + err := os.WriteFile(filepath.Join(versionDir, n), []byte("test"), os.ModePerm) + require.NoError(t, err) + } + for _, d := range []string{"somedir"} { + err := os.Mkdir(filepath.Join(versionDir, d), os.ModePerm) + require.NoError(t, err) + } + + linkDir := t.TempDir() + + installer := &LocalInstaller{ + InstallDir: versionsDir, + LinkDir: linkDir, + Log: slog.Default(), + } + ctx := context.Background() + err = installer.Link(ctx, version) + if tt.errMatch != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errMatch) + return + } + require.NoError(t, err) + + for _, link := range tt.links { + v, err := os.ReadFile(filepath.Join(linkDir, link)) + require.NoError(t, err) + require.Equal(t, "test", string(v)) + } + }) + } +} diff --git a/lib/autoupdate/agent/updater.go b/lib/autoupdate/agent/updater.go index 27eca08692a3e..1148c1fc1f972 100644 --- a/lib/autoupdate/agent/updater.go +++ b/lib/autoupdate/agent/updater.go @@ -112,6 +112,12 @@ func NewLocalUpdater(cfg LocalUpdaterConfig) (*Updater, error) { if cfg.Log == nil { cfg.Log = slog.Default() } + if cfg.LinkDir == "" { + cfg.LinkDir = "/usr/local/bin" + } + if cfg.VersionsDir == "" { + cfg.VersionsDir = "/var/lib/teleport/versions" + } return &Updater{ Log: cfg.Log, Pool: certPool, @@ -119,6 +125,7 @@ func NewLocalUpdater(cfg LocalUpdaterConfig) (*Updater, error) { ConfigPath: filepath.Join(cfg.VersionsDir, updateConfigName), Installer: &LocalInstaller{ InstallDir: cfg.VersionsDir, + LinkDir: cfg.LinkDir, HTTP: client, Log: cfg.Log, @@ -140,6 +147,8 @@ type LocalUpdaterConfig struct { DownloadTimeout time.Duration // VersionsDir for installing Teleport (usually /var/lib/teleport/versions). VersionsDir string + // LinkDir for installing Teleport (usually /usr/local/bin). + LinkDir string } // Updater implements the agent-local logic for Teleport agent auto-updates. @@ -161,7 +170,7 @@ type Installer interface { // Install the Teleport agent at version from the download template. // This function must be idempotent. Install(ctx context.Context, version, template string, flags InstallFlags) error - // Link the Teleport agent at version to the system location. + // Link the Teleport agent at version into the system location. Link(ctx context.Context, version string) error // Remove the Teleport agent at version. // This function must be idempotent. @@ -251,7 +260,11 @@ func (u *Updater) Enable(ctx context.Context, override OverrideConfig) error { } err = u.Installer.Install(ctx, desiredVersion, template, 0) // TODO(sclevine): add web API for flags if err != nil { - return trace.Wrap(err) + return trace.Errorf("failed to install: %w", err) + } + err = u.Installer.Link(ctx, desiredVersion) + if err != nil { + return trace.Errorf("failed to link: %w", err) } if cfg.Status.ActiveVersion != desiredVersion { u.Log.InfoContext(ctx, "Target version successfully installed.", "version", desiredVersion) diff --git a/lib/autoupdate/agent/updater_test.go b/lib/autoupdate/agent/updater_test.go index 6568fbaede9e2..85600b8b30aa3 100644 --- a/lib/autoupdate/agent/updater_test.go +++ b/lib/autoupdate/agent/updater_test.go @@ -263,13 +263,17 @@ func TestUpdater_Enable(t *testing.T) { }) require.NoError(t, err) - var installedVersion, installedTemplate string + var installedVersion, installedTemplate, linkedVersion string updater.Installer = &testInstaller{ FuncInstall: func(_ context.Context, version, template string, _ InstallFlags) error { installedVersion = version installedTemplate = template return tt.installErr }, + FuncLink: func(_ context.Context, version string) error { + linkedVersion = version + return nil + }, } ctx := context.Background() @@ -282,6 +286,7 @@ func TestUpdater_Enable(t *testing.T) { require.NoError(t, err) require.Equal(t, tt.installedVersion, installedVersion) require.Equal(t, tt.installedTemplate, installedTemplate) + require.Equal(t, tt.installedVersion, linkedVersion) data, err := os.ReadFile(cfgPath) require.NoError(t, err) @@ -304,6 +309,7 @@ func blankTestAddr(s []byte) []byte { type testInstaller struct { FuncInstall func(ctx context.Context, version, template string, flags InstallFlags) error FuncRemove func(ctx context.Context, version string) error + FuncLink func(ctx context.Context, version string) error } func (ti *testInstaller) Install(ctx context.Context, version, template string, flags InstallFlags) error { @@ -313,3 +319,7 @@ func (ti *testInstaller) Install(ctx context.Context, version, template string, func (ti *testInstaller) Remove(ctx context.Context, version string) error { return ti.FuncRemove(ctx, version) } + +func (ti *testInstaller) Link(ctx context.Context, version string) error { + return ti.FuncLink(ctx, version) +} From c8179fdcb822b98a1d9ffde29a1ce4d350afc2e4 Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Thu, 17 Oct 2024 14:35:23 -0400 Subject: [PATCH 08/18] add cleanup --- lib/autoupdate/agent/installer.go | 16 +++++++ .../already_disabled.golden | 1 + .../TestUpdater_Disable/enabled.golden | 1 + .../TestUpdater_Enable/already_enabled.golden | 1 + .../backup_version_kept_otherwise.golden | 10 ++++ .../backup_version_removed_on_install.golden | 10 ++++ .../config_does_not_exist.golden | 1 + .../config_from_file.golden | 1 + .../config_from_user.golden | 1 + .../version_already_installed.golden | 1 + lib/autoupdate/agent/updater.go | 27 +++++++++++ lib/autoupdate/agent/updater_test.go | 47 ++++++++++++++++++- 12 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 lib/autoupdate/agent/testdata/TestUpdater_Enable/backup_version_kept_otherwise.golden create mode 100644 lib/autoupdate/agent/testdata/TestUpdater_Enable/backup_version_removed_on_install.golden diff --git a/lib/autoupdate/agent/installer.go b/lib/autoupdate/agent/installer.go index 6646bbd19c289..6c136d1880e82 100644 --- a/lib/autoupdate/agent/installer.go +++ b/lib/autoupdate/agent/installer.go @@ -326,6 +326,22 @@ func uncompressedSize(f io.Reader) (int64, error) { return n, nil } +// List installed versions of Teleport. +func (ai *LocalInstaller) List(ctx context.Context) (versions []string, err error) { + entries, err := os.ReadDir(ai.InstallDir) + if err != nil { + return nil, trace.Wrap(err) + } + for _, entry := range entries { + if !entry.IsDir() { + continue + } + versions = append(versions, entry.Name()) + } + return versions, nil +} + +// Link the specified version into the system LinkDir. func (ai *LocalInstaller) Link(ctx context.Context, version string) error { binDir := filepath.Join(ai.InstallDir, version, "bin") entries, err := os.ReadDir(binDir) diff --git a/lib/autoupdate/agent/testdata/TestUpdater_Disable/already_disabled.golden b/lib/autoupdate/agent/testdata/TestUpdater_Disable/already_disabled.golden index e8773a6d88b7f..2ddb840b01794 100644 --- a/lib/autoupdate/agent/testdata/TestUpdater_Disable/already_disabled.golden +++ b/lib/autoupdate/agent/testdata/TestUpdater_Disable/already_disabled.golden @@ -7,3 +7,4 @@ spec: enabled: false status: active_version: "" + backup_version: "" diff --git a/lib/autoupdate/agent/testdata/TestUpdater_Disable/enabled.golden b/lib/autoupdate/agent/testdata/TestUpdater_Disable/enabled.golden index e8773a6d88b7f..2ddb840b01794 100644 --- a/lib/autoupdate/agent/testdata/TestUpdater_Disable/enabled.golden +++ b/lib/autoupdate/agent/testdata/TestUpdater_Disable/enabled.golden @@ -7,3 +7,4 @@ spec: enabled: false status: active_version: "" + backup_version: "" diff --git a/lib/autoupdate/agent/testdata/TestUpdater_Enable/already_enabled.golden b/lib/autoupdate/agent/testdata/TestUpdater_Enable/already_enabled.golden index e03f369eb1017..9dd03b7888da4 100644 --- a/lib/autoupdate/agent/testdata/TestUpdater_Enable/already_enabled.golden +++ b/lib/autoupdate/agent/testdata/TestUpdater_Enable/already_enabled.golden @@ -7,3 +7,4 @@ spec: enabled: true status: active_version: 16.3.0 + backup_version: old-version diff --git a/lib/autoupdate/agent/testdata/TestUpdater_Enable/backup_version_kept_otherwise.golden b/lib/autoupdate/agent/testdata/TestUpdater_Enable/backup_version_kept_otherwise.golden new file mode 100644 index 0000000000000..77829fb91706a --- /dev/null +++ b/lib/autoupdate/agent/testdata/TestUpdater_Enable/backup_version_kept_otherwise.golden @@ -0,0 +1,10 @@ +version: v1 +kind: update_config +spec: + proxy: localhost + group: "" + url_template: "" + enabled: true +status: + active_version: 16.3.0 + backup_version: backup-version diff --git a/lib/autoupdate/agent/testdata/TestUpdater_Enable/backup_version_removed_on_install.golden b/lib/autoupdate/agent/testdata/TestUpdater_Enable/backup_version_removed_on_install.golden new file mode 100644 index 0000000000000..9dd03b7888da4 --- /dev/null +++ b/lib/autoupdate/agent/testdata/TestUpdater_Enable/backup_version_removed_on_install.golden @@ -0,0 +1,10 @@ +version: v1 +kind: update_config +spec: + proxy: localhost + group: "" + url_template: "" + enabled: true +status: + active_version: 16.3.0 + backup_version: old-version diff --git a/lib/autoupdate/agent/testdata/TestUpdater_Enable/config_does_not_exist.golden b/lib/autoupdate/agent/testdata/TestUpdater_Enable/config_does_not_exist.golden index e03f369eb1017..d9e09a2c95d71 100644 --- a/lib/autoupdate/agent/testdata/TestUpdater_Enable/config_does_not_exist.golden +++ b/lib/autoupdate/agent/testdata/TestUpdater_Enable/config_does_not_exist.golden @@ -7,3 +7,4 @@ spec: enabled: true status: active_version: 16.3.0 + backup_version: "" diff --git a/lib/autoupdate/agent/testdata/TestUpdater_Enable/config_from_file.golden b/lib/autoupdate/agent/testdata/TestUpdater_Enable/config_from_file.golden index b172d858bc55a..61e41f76ca234 100644 --- a/lib/autoupdate/agent/testdata/TestUpdater_Enable/config_from_file.golden +++ b/lib/autoupdate/agent/testdata/TestUpdater_Enable/config_from_file.golden @@ -7,3 +7,4 @@ spec: enabled: true status: active_version: 16.3.0 + backup_version: old-version diff --git a/lib/autoupdate/agent/testdata/TestUpdater_Enable/config_from_user.golden b/lib/autoupdate/agent/testdata/TestUpdater_Enable/config_from_user.golden index bb9ce8b9d8fa8..c1f0fd166b497 100644 --- a/lib/autoupdate/agent/testdata/TestUpdater_Enable/config_from_user.golden +++ b/lib/autoupdate/agent/testdata/TestUpdater_Enable/config_from_user.golden @@ -7,3 +7,4 @@ spec: enabled: true status: active_version: new-version + backup_version: old-version diff --git a/lib/autoupdate/agent/testdata/TestUpdater_Enable/version_already_installed.golden b/lib/autoupdate/agent/testdata/TestUpdater_Enable/version_already_installed.golden index e03f369eb1017..d9e09a2c95d71 100644 --- a/lib/autoupdate/agent/testdata/TestUpdater_Enable/version_already_installed.golden +++ b/lib/autoupdate/agent/testdata/TestUpdater_Enable/version_already_installed.golden @@ -7,3 +7,4 @@ spec: enabled: true status: active_version: 16.3.0 + backup_version: "" diff --git a/lib/autoupdate/agent/updater.go b/lib/autoupdate/agent/updater.go index 1148c1fc1f972..27228fc81126a 100644 --- a/lib/autoupdate/agent/updater.go +++ b/lib/autoupdate/agent/updater.go @@ -86,6 +86,8 @@ type UpdateSpec struct { type UpdateStatus struct { // ActiveVersion is the currently active Teleport version. ActiveVersion string `yaml:"active_version"` + // BackupVersion is the last working version of Teleport. + BackupVersion string `yaml:"backup_version"` } // NewLocalUpdater returns a new Updater that auto-updates local @@ -171,7 +173,10 @@ type Installer interface { // This function must be idempotent. Install(ctx context.Context, version, template string, flags InstallFlags) error // Link the Teleport agent at version into the system location. + // This function must be idempotent. Link(ctx context.Context, version string) error + // List the installed versions of Teleport. + List(ctx context.Context) (versions []string, err error) // Remove the Teleport agent at version. // This function must be idempotent. Remove(ctx context.Context, version string) error @@ -253,6 +258,19 @@ func (u *Updater) Enable(ctx context.Context, override OverrideConfig) error { if desiredVersion == "" { return trace.Errorf("agent version not available from Teleport cluster") } + switch cfg.Status.BackupVersion { + case "", desiredVersion, cfg.Status.ActiveVersion: + default: + if desiredVersion == cfg.Status.ActiveVersion { + // Keep backup version if we are only verifying active version + break + } + err := u.Installer.Remove(ctx, cfg.Status.BackupVersion) + if err != nil { + // this could happen if it was already removed due to a failed installation + u.Log.WarnContext(ctx, "Failed to remove backup version of Teleport before new install.", "error", err) + } + } // If the active version and target don't match, kick off upgrade. template := cfg.Spec.URLTemplate if template == "" { @@ -271,8 +289,17 @@ func (u *Updater) Enable(ctx context.Context, override OverrideConfig) error { } else { u.Log.InfoContext(ctx, "Target version successfully validated.", "version", desiredVersion) } + cfg.Status.BackupVersion = cfg.Status.ActiveVersion cfg.Status.ActiveVersion = desiredVersion + versions, err := u.Installer.List(ctx) + if err != nil { + return trace.Errorf("failed to list installed versions: %w", err) + } + if n := len(versions); n > 2 { + u.Log.WarnContext(ctx, "More than 2 versions of Teleport installed. Version directory may need cleanup to save space.", "count", n) + } + // Always write the configuration file if enable succeeds. if err := u.writeConfig(u.ConfigPath, cfg); err != nil { return trace.Errorf("failed to write %s: %w", updateConfigName, err) diff --git a/lib/autoupdate/agent/updater_test.go b/lib/autoupdate/agent/updater_test.go index 85600b8b30aa3..c285e72ece47a 100644 --- a/lib/autoupdate/agent/updater_test.go +++ b/lib/autoupdate/agent/updater_test.go @@ -130,6 +130,7 @@ func TestUpdater_Enable(t *testing.T) { userCfg OverrideConfig installErr error + removedVersion string installedVersion string installedTemplate string errMatch string @@ -221,6 +222,32 @@ func TestUpdater_Enable(t *testing.T) { installedVersion: "16.3.0", installedTemplate: cdnURITemplate, }, + { + name: "backup version removed on install", + cfg: &UpdateConfig{ + Version: updateConfigVersion, + Kind: updateConfigKind, + Status: UpdateStatus{ + ActiveVersion: "old-version", + BackupVersion: "backup-version", + }, + }, + installedVersion: "16.3.0", + installedTemplate: cdnURITemplate, + removedVersion: "backup-version", + }, + { + name: "backup version kept otherwise", + cfg: &UpdateConfig{ + Version: updateConfigVersion, + Kind: updateConfigKind, + Status: UpdateStatus{ + ActiveVersion: "16.3.0", + BackupVersion: "backup-version", + }, + }, + removedVersion: "", + }, { name: "config does not exist", installedVersion: "16.3.0", @@ -263,7 +290,12 @@ func TestUpdater_Enable(t *testing.T) { }) require.NoError(t, err) - var installedVersion, installedTemplate, linkedVersion string + var ( + installedVersion string + installedTemplate string + linkedVersion string + removedVersion string + ) updater.Installer = &testInstaller{ FuncInstall: func(_ context.Context, version, template string, _ InstallFlags) error { installedVersion = version @@ -274,6 +306,13 @@ func TestUpdater_Enable(t *testing.T) { linkedVersion = version return nil }, + FuncList: func(_ context.Context) (versions []string, err error) { + return []string{"old"}, nil + }, + FuncRemove: func(_ context.Context, version string) error { + removedVersion = version + return nil + }, } ctx := context.Background() @@ -287,6 +326,7 @@ func TestUpdater_Enable(t *testing.T) { require.Equal(t, tt.installedVersion, installedVersion) require.Equal(t, tt.installedTemplate, installedTemplate) require.Equal(t, tt.installedVersion, linkedVersion) + require.Equal(t, tt.removedVersion, removedVersion) data, err := os.ReadFile(cfgPath) require.NoError(t, err) @@ -310,6 +350,7 @@ type testInstaller struct { FuncInstall func(ctx context.Context, version, template string, flags InstallFlags) error FuncRemove func(ctx context.Context, version string) error FuncLink func(ctx context.Context, version string) error + FuncList func(ctx context.Context) (versions []string, err error) } func (ti *testInstaller) Install(ctx context.Context, version, template string, flags InstallFlags) error { @@ -323,3 +364,7 @@ func (ti *testInstaller) Remove(ctx context.Context, version string) error { func (ti *testInstaller) Link(ctx context.Context, version string) error { return ti.FuncLink(ctx, version) } + +func (ti *testInstaller) List(ctx context.Context) (versions []string, err error) { + return ti.FuncList(ctx) +} From c44f65b312e6aa2ea2313f1f5b6091a14006cbdb Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Fri, 18 Oct 2024 12:16:30 -0400 Subject: [PATCH 09/18] fix extract --- lib/autoupdate/agent/installer_test.go | 21 ++++++++++++--------- lib/utils/unpack.go | 18 ++++++++++++++++-- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/lib/autoupdate/agent/installer_test.go b/lib/autoupdate/agent/installer_test.go index 095c67e80349a..f1e17d8e3a844 100644 --- a/lib/autoupdate/agent/installer_test.go +++ b/lib/autoupdate/agent/installer_test.go @@ -136,13 +136,15 @@ func TestTeleportInstaller_Install(t *testing.T) { require.Equal(t, expectedPath, dlPath) require.Equal(t, expectedPath+"."+checksumType, shaPath) - teleportVersion, err := os.ReadFile(filepath.Join(dir, version, "teleport")) - require.NoError(t, err) - require.Equal(t, version, string(teleportVersion)) - - tshVersion, err := os.ReadFile(filepath.Join(dir, version, "tsh")) - require.NoError(t, err) - require.Equal(t, version, string(tshVersion)) + for _, p := range []string{ + filepath.Join(dir, version, "etc", "systemd", "teleport.service"), + filepath.Join(dir, version, "bin", "teleport"), + filepath.Join(dir, version, "bin", "tsh"), + } { + v, err := os.ReadFile(p) + require.NoError(t, err) + require.Equal(t, version, string(v)) + } sum, err := os.ReadFile(filepath.Join(dir, version, checksumType)) require.NoError(t, err) @@ -163,8 +165,9 @@ func testTGZ(t *testing.T, version string) (tgz *bytes.Buffer, shasum string) { var files = []struct { Name, Body string }{ - {"teleport", version}, - {"tsh", version}, + {"teleport/examples/systemd/teleport.service", version}, + {"teleport/teleport", version}, + {"teleport/tsh", version}, } for _, file := range files { hdr := &tar.Header{ diff --git a/lib/utils/unpack.go b/lib/utils/unpack.go index 427bd8c61a4b1..1e832191c1e91 100644 --- a/lib/utils/unpack.go +++ b/lib/utils/unpack.go @@ -80,6 +80,9 @@ func filterHeader(hdr *tar.Header, paths []ExtractPath) (ok bool) { src := path.Clean(p.Src) switch hdr.Typeflag { case tar.TypeDir: + // If name is a directory, + // assume src is a directory prefix, or the directory itself, + // and replace that prefix with dst if src != "/" { src += "/" } @@ -93,11 +96,22 @@ func filterHeader(hdr *tar.Header, paths []ExtractPath) (ok bool) { hdr.Name = dst return !p.Skip default: - if src != name { + // If name is a file, + // if src is an exact match to the file name, assume src is a file and write directly to dst, + // otherwise, assume src is a directory prefix, and replace that prefix with dst + if src == name { + hdr.Name = path.Clean(p.Dst) + return !p.Skip + } + if src != "/" { + src += "/" + } + if !strings.HasPrefix(name, src) { continue } - hdr.Name = path.Clean(p.Dst) + hdr.Name = path.Join(p.Dst, strings.TrimPrefix(name, src)) return !p.Skip + } } return len(paths) == 0 From 375942db389b9556958465ee7beb0010acc3f6aa Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Mon, 21 Oct 2024 14:23:22 -0400 Subject: [PATCH 10/18] wip --- lib/autoupdate/agent/installer.go | 34 +++++++++++- lib/autoupdate/agent/installer_test.go | 77 ++++++++++++++++++++++++-- 2 files changed, 106 insertions(+), 5 deletions(-) diff --git a/lib/autoupdate/agent/installer.go b/lib/autoupdate/agent/installer.go index 6c136d1880e82..62490cf580fbb 100644 --- a/lib/autoupdate/agent/installer.go +++ b/lib/autoupdate/agent/installer.go @@ -62,15 +62,25 @@ type LocalInstaller struct { ReservedFreeInstallDisk uint64 } +var ErrLinked = errors.New("linked version cannot be removed") + // Remove a Teleport version directory from InstallDir. // This function is idempotent. func (li *LocalInstaller) Remove(ctx context.Context, version string) error { versionDir := filepath.Join(li.InstallDir, version) sumPath := filepath.Join(versionDir, checksumType) + linked, err := li.linkedVersion(version) + if err != nil { + return trace.Errorf("failed to determine if linked: %w", err) + } + if linked { + return ErrLinked + } + // invalidate checksum first, to protect against partially-removed // directory with valid checksum. - err := os.Remove(sumPath) + err = os.Remove(sumPath) if err != nil && !errors.Is(err, os.ErrNotExist) { return trace.Wrap(err) } @@ -359,3 +369,25 @@ func (ai *LocalInstaller) Link(ctx context.Context, version string) error { } return nil } + +// linkedVersion returns true if any +func (ai *LocalInstaller) linkedVersion(version string) (bool, error) { + binDir := filepath.Join(ai.InstallDir, version, "bin") + entries, err := os.ReadDir(binDir) + if err != nil { + return false, trace.Wrap(err) + } + for _, entry := range entries { + if entry.IsDir() { + continue + } + v, err := os.Readlink(filepath.Join(ai.LinkDir, entry.Name())) + if err != nil { + continue + } + if v == filepath.Join(binDir, entry.Name()) { + return true, nil + } + } + return false, nil +} diff --git a/lib/autoupdate/agent/installer_test.go b/lib/autoupdate/agent/installer_test.go index f1e17d8e3a844..8c6c7a0adffd5 100644 --- a/lib/autoupdate/agent/installer_test.go +++ b/lib/autoupdate/agent/installer_test.go @@ -40,7 +40,7 @@ import ( "github.com/stretchr/testify/require" ) -func TestTeleportInstaller_Install(t *testing.T) { +func TestLocalInstaller_Install(t *testing.T) { t.Parallel() const version = "new-version" @@ -191,7 +191,7 @@ func testTGZ(t *testing.T, version string) (tgz *bytes.Buffer, shasum string) { return &buf, hex.EncodeToString(sha.Sum(nil)) } -func TestTeleportInstaller_Link(t *testing.T) { +func TestLocalInstaller_Link(t *testing.T) { t.Parallel() const version = "new-version" @@ -227,7 +227,7 @@ func TestTeleportInstaller_Link(t *testing.T) { require.NoError(t, err) for _, n := range tt.files { - err := os.WriteFile(filepath.Join(versionDir, n), []byte("test"), os.ModePerm) + err := os.WriteFile(filepath.Join(versionDir, n), []byte(n), os.ModePerm) require.NoError(t, err) } for _, d := range []string{"somedir"} { @@ -254,7 +254,76 @@ func TestTeleportInstaller_Link(t *testing.T) { for _, link := range tt.links { v, err := os.ReadFile(filepath.Join(linkDir, link)) require.NoError(t, err) - require.Equal(t, "test", string(v)) + require.Equal(t, filepath.Base("link"), string(v)) + } + }) + } +} + +func TestLocalInstaller_Remove(t *testing.T) { + t.Parallel() + const version = "new-version" + + tests := []struct { + name string + files []string + dirs []string + + links []string + errMatch string + }{ + { + name: "present", + files: []string{"teleport", "tsh", "tbot"}, + dirs: []string{"somedir"}, + + links: []string{"teleport", "tsh", "tbot"}, + }, + { + name: "not present", + files: []string{"teleport"}, + + errMatch: "no such", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + versionsDir := t.TempDir() + versionDir := filepath.Join(versionsDir, version) + err := os.MkdirAll(versionDir, 0o755) + require.NoError(t, err) + + for _, n := range tt.files { + err := os.WriteFile(filepath.Join(versionDir, n), []byte(n), os.ModePerm) + require.NoError(t, err) + } + for _, d := range []string{"somedir"} { + err := os.Mkdir(filepath.Join(versionDir, d), os.ModePerm) + require.NoError(t, err) + } + + linkDir := t.TempDir() + + installer := &LocalInstaller{ + InstallDir: versionsDir, + LinkDir: linkDir, + Log: slog.Default(), + } + ctx := context.Background() + err = installer.Link(ctx, version) + if tt.errMatch != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errMatch) + return + } + require.NoError(t, err) + + for _, link := range tt.links { + v, err := os.ReadFile(filepath.Join(linkDir, link)) + require.NoError(t, err) + require.Equal(t, filepath.Base("link"), string(v)) } }) } From d3ab11f771291527aa086cee4ad5dbeccaedc374 Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Wed, 23 Oct 2024 17:29:05 -0400 Subject: [PATCH 11/18] fix tests --- lib/autoupdate/agent/installer.go | 12 ++- lib/autoupdate/agent/installer_test.go | 102 +++++++++++------- ...backup_version_kept_for_validation.golden} | 0 lib/autoupdate/agent/updater.go | 4 +- lib/autoupdate/agent/updater_test.go | 6 +- 5 files changed, 80 insertions(+), 44 deletions(-) rename lib/autoupdate/agent/testdata/TestUpdater_Enable/{backup_version_kept_otherwise.golden => backup_version_kept_for_validation.golden} (100%) diff --git a/lib/autoupdate/agent/installer.go b/lib/autoupdate/agent/installer.go index 62490cf580fbb..78a34be785788 100644 --- a/lib/autoupdate/agent/installer.go +++ b/lib/autoupdate/agent/installer.go @@ -71,7 +71,7 @@ func (li *LocalInstaller) Remove(ctx context.Context, version string) error { sumPath := filepath.Join(versionDir, checksumType) linked, err := li.linkedVersion(version) - if err != nil { + if err != nil && !errors.Is(err, os.ErrNotExist) { return trace.Errorf("failed to determine if linked: %w", err) } if linked { @@ -356,8 +356,9 @@ func (ai *LocalInstaller) Link(ctx context.Context, version string) error { binDir := filepath.Join(ai.InstallDir, version, "bin") entries, err := os.ReadDir(binDir) if err != nil { - return trace.Wrap(err) + return trace.Errorf("failed to find Teleport binary directory: %w", err) } + var linked int for _, entry := range entries { if entry.IsDir() { continue @@ -366,11 +367,16 @@ func (ai *LocalInstaller) Link(ctx context.Context, version string) error { if err != nil { return trace.Wrap(err) } + linked++ + } + if linked == 0 { + return trace.Errorf("no binaries available to link") } return nil } -// linkedVersion returns true if any +// linkedVersion returns true if any binaries for the version are linked. +// Returns os.ErrNotExist error if the version does not exist. func (ai *LocalInstaller) linkedVersion(version string) (bool, error) { binDir := filepath.Join(ai.InstallDir, version, "bin") entries, err := os.ReadDir(binDir) diff --git a/lib/autoupdate/agent/installer_test.go b/lib/autoupdate/agent/installer_test.go index 8c6c7a0adffd5..ebdf16c735366 100644 --- a/lib/autoupdate/agent/installer_test.go +++ b/lib/autoupdate/agent/installer_test.go @@ -197,24 +197,31 @@ func TestLocalInstaller_Link(t *testing.T) { tests := []struct { name string - files []string dirs []string + files []string links []string errMatch string }{ { name: "present", - files: []string{"teleport", "tsh", "tbot"}, - dirs: []string{"somedir"}, + dirs: []string{"bin", "bin/somedir", "somedir"}, + files: []string{"bin/teleport", "bin/tsh", "bin/tbot", "README"}, links: []string{"teleport", "tsh", "tbot"}, }, { - name: "not present", - files: []string{"teleport"}, + name: "no links", + files: []string{"README"}, + dirs: []string{"bin"}, + + errMatch: "no binaries", + }, + { + name: "no bin directory", + files: []string{"README"}, - errMatch: "no such", + errMatch: "binary directory", }, } @@ -226,12 +233,12 @@ func TestLocalInstaller_Link(t *testing.T) { err := os.MkdirAll(versionDir, 0o755) require.NoError(t, err) - for _, n := range tt.files { - err := os.WriteFile(filepath.Join(versionDir, n), []byte(n), os.ModePerm) + for _, d := range tt.dirs { + err := os.Mkdir(filepath.Join(versionDir, d), os.ModePerm) require.NoError(t, err) } - for _, d := range []string{"somedir"} { - err := os.Mkdir(filepath.Join(versionDir, d), os.ModePerm) + for _, n := range tt.files { + err := os.WriteFile(filepath.Join(versionDir, n), []byte(filepath.Base(n)), os.ModePerm) require.NoError(t, err) } @@ -254,7 +261,7 @@ func TestLocalInstaller_Link(t *testing.T) { for _, link := range tt.links { v, err := os.ReadFile(filepath.Join(linkDir, link)) require.NoError(t, err) - require.Equal(t, filepath.Base("link"), string(v)) + require.Equal(t, link, string(v)) } }) } @@ -262,28 +269,48 @@ func TestLocalInstaller_Link(t *testing.T) { func TestLocalInstaller_Remove(t *testing.T) { t.Parallel() - const version = "new-version" + const version = "existing-version" tests := []struct { - name string - files []string - dirs []string + name string + dirs []string + files []string + createVersion string + linkedVersion string + removeVersion string - links []string errMatch string }{ { - name: "present", - files: []string{"teleport", "tsh", "tbot"}, - dirs: []string{"somedir"}, - - links: []string{"teleport", "tsh", "tbot"}, + name: "present", + dirs: []string{"bin", "bin/somedir", "somedir"}, + files: []string{checksumType, "bin/teleport", "bin/tsh", "bin/tbot", "README"}, + createVersion: version, + removeVersion: version, }, { - name: "not present", - files: []string{"teleport"}, - - errMatch: "no such", + name: "present missing checksum", + dirs: []string{"bin", "bin/somedir", "somedir"}, + files: []string{"bin/teleport", "bin/tsh", "bin/tbot", "README"}, + createVersion: version, + removeVersion: version, + }, + { + name: "not present", + dirs: []string{"bin", "bin/somedir", "somedir"}, + files: []string{checksumType, "bin/teleport", "bin/tsh", "bin/tbot", "README"}, + createVersion: version, + removeVersion: "missing-version", + }, + { + name: "version linked", + dirs: []string{"bin", "bin/somedir", "somedir"}, + files: []string{checksumType, "bin/teleport", "bin/tsh", "bin/tbot", "README"}, + createVersion: version, + linkedVersion: version, + removeVersion: version, + + errMatch: ErrLinked.Error(), }, } @@ -291,16 +318,16 @@ func TestLocalInstaller_Remove(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { versionsDir := t.TempDir() - versionDir := filepath.Join(versionsDir, version) + versionDir := filepath.Join(versionsDir, tt.createVersion) err := os.MkdirAll(versionDir, 0o755) require.NoError(t, err) - for _, n := range tt.files { - err := os.WriteFile(filepath.Join(versionDir, n), []byte(n), os.ModePerm) + for _, d := range tt.dirs { + err := os.Mkdir(filepath.Join(versionDir, d), os.ModePerm) require.NoError(t, err) } - for _, d := range []string{"somedir"} { - err := os.Mkdir(filepath.Join(versionDir, d), os.ModePerm) + for _, n := range tt.files { + err := os.WriteFile(filepath.Join(versionDir, n), []byte(filepath.Base(n)), os.ModePerm) require.NoError(t, err) } @@ -312,19 +339,20 @@ func TestLocalInstaller_Remove(t *testing.T) { Log: slog.Default(), } ctx := context.Background() - err = installer.Link(ctx, version) + + if tt.linkedVersion != "" { + err = installer.Link(ctx, tt.linkedVersion) + require.NoError(t, err) + } + err = installer.Remove(ctx, tt.removeVersion) if tt.errMatch != "" { require.Error(t, err) assert.Contains(t, err.Error(), tt.errMatch) return } require.NoError(t, err) - - for _, link := range tt.links { - v, err := os.ReadFile(filepath.Join(linkDir, link)) - require.NoError(t, err) - require.Equal(t, filepath.Base("link"), string(v)) - } + _, err = os.Stat(filepath.Join(versionDir, "bin", tt.removeVersion)) + require.ErrorIs(t, err, os.ErrNotExist) }) } } diff --git a/lib/autoupdate/agent/testdata/TestUpdater_Enable/backup_version_kept_otherwise.golden b/lib/autoupdate/agent/testdata/TestUpdater_Enable/backup_version_kept_for_validation.golden similarity index 100% rename from lib/autoupdate/agent/testdata/TestUpdater_Enable/backup_version_kept_otherwise.golden rename to lib/autoupdate/agent/testdata/TestUpdater_Enable/backup_version_kept_for_validation.golden diff --git a/lib/autoupdate/agent/updater.go b/lib/autoupdate/agent/updater.go index 27228fc81126a..302943fd13dd0 100644 --- a/lib/autoupdate/agent/updater.go +++ b/lib/autoupdate/agent/updater.go @@ -285,12 +285,12 @@ func (u *Updater) Enable(ctx context.Context, override OverrideConfig) error { return trace.Errorf("failed to link: %w", err) } if cfg.Status.ActiveVersion != desiredVersion { + cfg.Status.BackupVersion = cfg.Status.ActiveVersion + cfg.Status.ActiveVersion = desiredVersion u.Log.InfoContext(ctx, "Target version successfully installed.", "version", desiredVersion) } else { u.Log.InfoContext(ctx, "Target version successfully validated.", "version", desiredVersion) } - cfg.Status.BackupVersion = cfg.Status.ActiveVersion - cfg.Status.ActiveVersion = desiredVersion versions, err := u.Installer.List(ctx) if err != nil { diff --git a/lib/autoupdate/agent/updater_test.go b/lib/autoupdate/agent/updater_test.go index c285e72ece47a..d6d0128316c20 100644 --- a/lib/autoupdate/agent/updater_test.go +++ b/lib/autoupdate/agent/updater_test.go @@ -237,7 +237,7 @@ func TestUpdater_Enable(t *testing.T) { removedVersion: "backup-version", }, { - name: "backup version kept otherwise", + name: "backup version kept for validation", cfg: &UpdateConfig{ Version: updateConfigVersion, Kind: updateConfigKind, @@ -246,7 +246,9 @@ func TestUpdater_Enable(t *testing.T) { BackupVersion: "backup-version", }, }, - removedVersion: "", + installedVersion: "16.3.0", + installedTemplate: cdnURITemplate, + removedVersion: "", }, { name: "config does not exist", From 8f32a9b8d738426c09225dfcf0ba71a0fe5736c6 Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Wed, 23 Oct 2024 17:55:39 -0400 Subject: [PATCH 12/18] remove safety --- lib/autoupdate/agent/installer.go | 10 +++++-- lib/autoupdate/agent/installer_test.go | 36 ++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/lib/autoupdate/agent/installer.go b/lib/autoupdate/agent/installer.go index 78a34be785788..1d7d02a341464 100644 --- a/lib/autoupdate/agent/installer.go +++ b/lib/autoupdate/agent/installer.go @@ -62,13 +62,19 @@ type LocalInstaller struct { ReservedFreeInstallDisk uint64 } +// ErrLinked is returned when a linked version cannot be removed. var ErrLinked = errors.New("linked version cannot be removed") // Remove a Teleport version directory from InstallDir. // This function is idempotent. func (li *LocalInstaller) Remove(ctx context.Context, version string) error { + // os.RemoveAll is dangerous. + // We must validate the version to ensure that we remove only a single path + // element under the InstallDir, and not, e.g., InstallDir itself. versionDir := filepath.Join(li.InstallDir, version) - sumPath := filepath.Join(versionDir, checksumType) + if filepath.Dir(versionDir) != filepath.Clean(li.InstallDir) { + return trace.Errorf("refusing to directory outside of version directory") + } linked, err := li.linkedVersion(version) if err != nil && !errors.Is(err, os.ErrNotExist) { @@ -80,7 +86,7 @@ func (li *LocalInstaller) Remove(ctx context.Context, version string) error { // invalidate checksum first, to protect against partially-removed // directory with valid checksum. - err = os.Remove(sumPath) + err = os.Remove(filepath.Join(versionDir, checksumType)) if err != nil && !errors.Is(err, os.ErrNotExist) { return trace.Wrap(err) } diff --git a/lib/autoupdate/agent/installer_test.go b/lib/autoupdate/agent/installer_test.go index ebdf16c735366..96a3feff138e8 100644 --- a/lib/autoupdate/agent/installer_test.go +++ b/lib/autoupdate/agent/installer_test.go @@ -312,6 +312,42 @@ func TestLocalInstaller_Remove(t *testing.T) { errMatch: ErrLinked.Error(), }, + { + name: "version empty", + dirs: []string{"bin", "bin/somedir", "somedir"}, + files: []string{checksumType, "bin/teleport", "bin/tsh", "bin/tbot", "README"}, + createVersion: version, + removeVersion: "", + + errMatch: "outside", + }, + { + name: "version has path", + dirs: []string{"bin", "bin/somedir", "somedir"}, + files: []string{checksumType, "bin/teleport", "bin/tsh", "bin/tbot", "README"}, + createVersion: version, + removeVersion: "one/two", + + errMatch: "outside", + }, + { + name: "version is ..", + dirs: []string{"bin", "bin/somedir", "somedir"}, + files: []string{checksumType, "bin/teleport", "bin/tsh", "bin/tbot", "README"}, + createVersion: version, + removeVersion: "..", + + errMatch: "outside", + }, + { + name: "version is .", + dirs: []string{"bin", "bin/somedir", "somedir"}, + files: []string{checksumType, "bin/teleport", "bin/tsh", "bin/tbot", "README"}, + createVersion: version, + removeVersion: ".", + + errMatch: "outside", + }, } for _, tt := range tests { From c676f6715a261007ce0a30f1803477a7ed5af293 Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Wed, 23 Oct 2024 18:05:42 -0400 Subject: [PATCH 13/18] cleanup --- lib/autoupdate/agent/installer.go | 47 +++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/lib/autoupdate/agent/installer.go b/lib/autoupdate/agent/installer.go index 1d7d02a341464..d62415a6d1fdf 100644 --- a/lib/autoupdate/agent/installer.go +++ b/lib/autoupdate/agent/installer.go @@ -71,17 +71,17 @@ func (li *LocalInstaller) Remove(ctx context.Context, version string) error { // os.RemoveAll is dangerous. // We must validate the version to ensure that we remove only a single path // element under the InstallDir, and not, e.g., InstallDir itself. - versionDir := filepath.Join(li.InstallDir, version) - if filepath.Dir(versionDir) != filepath.Clean(li.InstallDir) { - return trace.Errorf("refusing to directory outside of version directory") + versionDir, err := li.versionDir(version) + if err != nil { + return trace.Wrap(err) } - linked, err := li.linkedVersion(version) + linked, err := li.isLinked(filepath.Join(versionDir, "bin")) if err != nil && !errors.Is(err, os.ErrNotExist) { return trace.Errorf("failed to determine if linked: %w", err) } if linked { - return ErrLinked + return trace.Wrap(ErrLinked) } // invalidate checksum first, to protect against partially-removed @@ -99,7 +99,10 @@ func (li *LocalInstaller) Remove(ctx context.Context, version string) error { // Install a Teleport version directory in InstallDir. // This function is idempotent. func (li *LocalInstaller) Install(ctx context.Context, version, template string, flags InstallFlags) error { - versionDir := filepath.Join(li.InstallDir, version) + versionDir, err := li.versionDir(version) + if err != nil { + return trace.Wrap(err) + } sumPath := filepath.Join(versionDir, checksumType) // generate download URI from template @@ -343,8 +346,8 @@ func uncompressedSize(f io.Reader) (int64, error) { } // List installed versions of Teleport. -func (ai *LocalInstaller) List(ctx context.Context) (versions []string, err error) { - entries, err := os.ReadDir(ai.InstallDir) +func (li *LocalInstaller) List(ctx context.Context) (versions []string, err error) { + entries, err := os.ReadDir(li.InstallDir) if err != nil { return nil, trace.Wrap(err) } @@ -358,8 +361,12 @@ func (ai *LocalInstaller) List(ctx context.Context) (versions []string, err erro } // Link the specified version into the system LinkDir. -func (ai *LocalInstaller) Link(ctx context.Context, version string) error { - binDir := filepath.Join(ai.InstallDir, version, "bin") +func (li *LocalInstaller) Link(ctx context.Context, version string) error { + versionDir, err := li.versionDir(version) + if err != nil { + return trace.Wrap(err) + } + binDir := filepath.Join(versionDir, "bin") entries, err := os.ReadDir(binDir) if err != nil { return trace.Errorf("failed to find Teleport binary directory: %w", err) @@ -369,7 +376,7 @@ func (ai *LocalInstaller) Link(ctx context.Context, version string) error { if entry.IsDir() { continue } - err := renameio.Symlink(filepath.Join(binDir, entry.Name()), filepath.Join(ai.LinkDir, entry.Name())) + err := renameio.Symlink(filepath.Join(binDir, entry.Name()), filepath.Join(li.LinkDir, entry.Name())) if err != nil { return trace.Wrap(err) } @@ -381,10 +388,20 @@ func (ai *LocalInstaller) Link(ctx context.Context, version string) error { return nil } -// linkedVersion returns true if any binaries for the version are linked. +// versionDir returns the storage directory for a Teleport version. +// versionDir will fail if the version cannot be used to construct the directory name. +// For example, it ensures that ".." cannot be provided to return a system directory. +func (li *LocalInstaller) versionDir(version string) (string, error) { + versionDir := filepath.Join(li.InstallDir, version) + if filepath.Dir(versionDir) != filepath.Clean(li.InstallDir) { + return "", trace.Errorf("refusing to directory outside of version directory") + } + return versionDir, nil +} + +// isLinked returns true if any binaries in binDir are linked. // Returns os.ErrNotExist error if the version does not exist. -func (ai *LocalInstaller) linkedVersion(version string) (bool, error) { - binDir := filepath.Join(ai.InstallDir, version, "bin") +func (li *LocalInstaller) isLinked(binDir string) (bool, error) { entries, err := os.ReadDir(binDir) if err != nil { return false, trace.Wrap(err) @@ -393,7 +410,7 @@ func (ai *LocalInstaller) linkedVersion(version string) (bool, error) { if entry.IsDir() { continue } - v, err := os.Readlink(filepath.Join(ai.LinkDir, entry.Name())) + v, err := os.Readlink(filepath.Join(li.LinkDir, entry.Name())) if err != nil { continue } From 923cfcd3446666f23f6bc3be2bcd8ed393a6deac Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Wed, 23 Oct 2024 18:20:38 -0400 Subject: [PATCH 14/18] cleanup extract --- lib/autoupdate/agent/updater.go | 2 +- lib/utils/unpack.go | 32 ++++++++++++++++++-------------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/lib/autoupdate/agent/updater.go b/lib/autoupdate/agent/updater.go index 302943fd13dd0..7e66e461ddc7b 100644 --- a/lib/autoupdate/agent/updater.go +++ b/lib/autoupdate/agent/updater.go @@ -118,7 +118,7 @@ func NewLocalUpdater(cfg LocalUpdaterConfig) (*Updater, error) { cfg.LinkDir = "/usr/local/bin" } if cfg.VersionsDir == "" { - cfg.VersionsDir = "/var/lib/teleport/versions" + cfg.VersionsDir = filepath.Join(libdefaults.DataDir, "versions") } return &Updater{ Log: cfg.Log, diff --git a/lib/utils/unpack.go b/lib/utils/unpack.go index 1e832191c1e91..78b111daf8992 100644 --- a/lib/utils/unpack.go +++ b/lib/utils/unpack.go @@ -37,8 +37,9 @@ import ( // resulting files and directories are created using the current user context. // Extract will only unarchive files into dir, and will fail if the tarball // tries to write files outside of dir. -// If paths are specified, only the specified paths are extracted. -// The destination specified in the first matching path is used. +// +// If any paths are specified, only the specified paths are extracted. +// The destination specified in the first matching path is selected. func Extract(r io.Reader, dir string, paths ...ExtractPath) error { tarball := tar.NewReader(r) @@ -66,45 +67,48 @@ func Extract(r io.Reader, dir string, paths ...ExtractPath) error { // ExtractPath specifies a path to be extracted. type ExtractPath struct { - // Src path prefix and Dst path within the archive to extract files to. - // Src path directories are not included in the extraction dir. - // Trialing slashes are always ignores. + // Src path and Dst path within the archive to extract files to. + // Directories in the Src path are not included in the extraction dir. + // For example, given foo/bar/file.txt with Src=foo/bar Dst=baz, baz/file.txt results. + // Trailing slashes are always ignored. Src, Dst string - // Skip the path entirely. + // Skip extracting the Src path and ignore Dst. Skip bool } -func filterHeader(hdr *tar.Header, paths []ExtractPath) (ok bool) { +// filterHeader modifies the tar header by filtering it through the ExtractPaths. +// filterHeader returns false if the tar header should be skipped. +func filterHeader(hdr *tar.Header, paths []ExtractPath) (include bool) { name := path.Clean(hdr.Name) for _, p := range paths { src := path.Clean(p.Src) switch hdr.Typeflag { case tar.TypeDir: - // If name is a directory, + // If name is a directory, then // assume src is a directory prefix, or the directory itself, - // and replace that prefix with dst + // and replace that prefix with dst. if src != "/" { - src += "/" + src += "/" // ensure HasPrefix does not match partial names } if !strings.HasPrefix(name, src) { continue } dst := path.Join(p.Dst, strings.TrimPrefix(name, src)) if dst != "/" { - dst += "/" + dst += "/" // tar directory headers end in / } hdr.Name = dst return !p.Skip default: - // If name is a file, + // If name is a file, then // if src is an exact match to the file name, assume src is a file and write directly to dst, - // otherwise, assume src is a directory prefix, and replace that prefix with dst + // otherwise, assume src is a directory prefix, and replace that prefix with dst. if src == name { hdr.Name = path.Clean(p.Dst) return !p.Skip } if src != "/" { - src += "/" + src += "/" // ensure HasPrefix does not match partial names } if !strings.HasPrefix(name, src) { continue From 5024b5ae788b23377e0bb24c421f4a64ddeb652c Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Wed, 23 Oct 2024 20:17:18 -0400 Subject: [PATCH 15/18] cleanup --- lib/autoupdate/agent/installer.go | 25 +++++++++++++++---------- lib/autoupdate/agent/installer_test.go | 22 ++++++++++++++++++++++ 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/lib/autoupdate/agent/installer.go b/lib/autoupdate/agent/installer.go index d62415a6d1fdf..b029dfd73f3dc 100644 --- a/lib/autoupdate/agent/installer.go +++ b/lib/autoupdate/agent/installer.go @@ -45,6 +45,17 @@ const ( checksumHexLen = sha256.Size * 2 // bytes to hex ) +var ( + tgzExtractPaths = []utils.ExtractPath{ + {Src: "teleport/examples/systemd/teleport.service", Dst: "etc/systemd/teleport.service"}, + {Src: "teleport/examples", Skip: true}, + {Src: "teleport/README.md", Dst: "share/README.md"}, + {Src: "teleport/CHANGELOG.md", Dst: "share/CHANGELOG.md"}, + {Src: "teleport/VERSION", Dst: "share/VERSION"}, + {Src: "teleport", Dst: "bin"}, + } +) + // LocalInstaller manages the creation and removal of installations // of Teleport. type LocalInstaller struct { @@ -68,9 +79,10 @@ var ErrLinked = errors.New("linked version cannot be removed") // Remove a Teleport version directory from InstallDir. // This function is idempotent. func (li *LocalInstaller) Remove(ctx context.Context, version string) error { - // os.RemoveAll is dangerous. + // os.RemoveAll is dangerous because it can remove an entire directory tree. // We must validate the version to ensure that we remove only a single path - // element under the InstallDir, and not, e.g., InstallDir itself. + // element under the InstallDir, and not InstallDir or its parents. + // versionDir performs these validations. versionDir, err := li.versionDir(version) if err != nil { return trace.Wrap(err) @@ -315,14 +327,7 @@ func (li *LocalInstaller) extract(ctx context.Context, dstDir string, src io.Rea } li.Log.InfoContext(ctx, "Extracting Teleport tarball.", "path", dstDir, "size", max) - err = utils.Extract(zr, dstDir, []utils.ExtractPath{ - {Src: "teleport/examples/systemd/teleport.service", Dst: "etc/systemd/teleport.service"}, - {Src: "teleport/examples", Skip: true}, - {Src: "teleport/README.md", Dst: "share/README.md"}, - {Src: "teleport/CHANGELOG.md", Dst: "share/CHANGELOG.md"}, - {Src: "teleport/VERSION", Dst: "share/VERSION"}, - {Src: "teleport", Dst: "bin"}, - }...) + err = utils.Extract(zr, dstDir, tgzExtractPaths...) if err != nil { return trace.Wrap(err) } diff --git a/lib/autoupdate/agent/installer_test.go b/lib/autoupdate/agent/installer_test.go index 96a3feff138e8..ec9dacaabbc1c 100644 --- a/lib/autoupdate/agent/installer_test.go +++ b/lib/autoupdate/agent/installer_test.go @@ -392,3 +392,25 @@ func TestLocalInstaller_Remove(t *testing.T) { }) } } + +func TestLocalInstaller_List(t *testing.T) { + installDir := t.TempDir() + versions := []string{"v1", "v2"} + + for _, d := range versions { + err := os.Mkdir(filepath.Join(installDir, d), os.ModePerm) + require.NoError(t, err) + } + for _, n := range []string{"file1", "file2"} { + err := os.WriteFile(filepath.Join(installDir, n), []byte(filepath.Base(n)), os.ModePerm) + require.NoError(t, err) + } + installer := &LocalInstaller{ + InstallDir: installDir, + Log: slog.Default(), + } + ctx := context.Background() + versions, err := installer.List(ctx) + require.NoError(t, err) + require.Equal(t, []string{"v1", "v2"}, versions) +} From 360ad0698e09d7cd7f7259a044a3e2c4db403fca Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Wed, 23 Oct 2024 21:41:13 -0400 Subject: [PATCH 16/18] cleanup --- lib/autoupdate/agent/installer.go | 49 ++++++++++++++++++++------ lib/autoupdate/agent/installer_test.go | 44 ++++++++++++++++------- lib/autoupdate/agent/updater.go | 13 +++---- tool/teleport-update/main.go | 20 ++++------- 4 files changed, 83 insertions(+), 43 deletions(-) diff --git a/lib/autoupdate/agent/installer.go b/lib/autoupdate/agent/installer.go index b029dfd73f3dc..ee9d7c81c844a 100644 --- a/lib/autoupdate/agent/installer.go +++ b/lib/autoupdate/agent/installer.go @@ -46,6 +46,9 @@ const ( ) var ( + // tgzExtractPaths describes how to extract the Teleport tgz. + // See utils.Extract for more details on how this list is parsed. + // Paths must use tarball-style / separators (not filepath). tgzExtractPaths = []utils.ExtractPath{ {Src: "teleport/examples/systemd/teleport.service", Dst: "etc/systemd/teleport.service"}, {Src: "teleport/examples", Skip: true}, @@ -54,6 +57,9 @@ var ( {Src: "teleport/VERSION", Dst: "share/VERSION"}, {Src: "teleport", Dst: "bin"}, } + + // servicePath contains the path to the Teleport SystemD service within the version directory. + servicePath = filepath.Join("etc", "systemd", "teleport.service") ) // LocalInstaller manages the creation and removal of installations @@ -61,8 +67,10 @@ var ( type LocalInstaller struct { // InstallDir contains each installation, named by version. InstallDir string - // LinkDir contains symlinks to the most recently linked installation. - LinkDir string + // LinkBinDir contains symlinks to the linked installation's binaries. + LinkBinDir string + // LinkServiceDir contains a symlink to the linked installation's systemd service. + LinkServiceDir string // HTTP is an HTTP client for downloading Teleport. HTTP *http.Client // Log contains a logger. @@ -88,7 +96,7 @@ func (li *LocalInstaller) Remove(ctx context.Context, version string) error { return trace.Wrap(err) } - linked, err := li.isLinked(filepath.Join(versionDir, "bin")) + linked, err := li.isLinked(versionDir) if err != nil && !errors.Is(err, os.ErrNotExist) { return trace.Errorf("failed to determine if linked: %w", err) } @@ -365,7 +373,7 @@ func (li *LocalInstaller) List(ctx context.Context) (versions []string, err erro return versions, nil } -// Link the specified version into the system LinkDir. +// Link the specified version into the system LinkBinDir. func (li *LocalInstaller) Link(ctx context.Context, version string) error { versionDir, err := li.versionDir(version) if err != nil { @@ -376,12 +384,16 @@ func (li *LocalInstaller) Link(ctx context.Context, version string) error { if err != nil { return trace.Errorf("failed to find Teleport binary directory: %w", err) } + err = os.MkdirAll(li.LinkBinDir, 0755) + if err != nil { + return trace.Wrap(err) + } var linked int for _, entry := range entries { if entry.IsDir() { continue } - err := renameio.Symlink(filepath.Join(binDir, entry.Name()), filepath.Join(li.LinkDir, entry.Name())) + err := renameio.Symlink(filepath.Join(binDir, entry.Name()), filepath.Join(li.LinkBinDir, entry.Name())) if err != nil { return trace.Wrap(err) } @@ -390,6 +402,15 @@ func (li *LocalInstaller) Link(ctx context.Context, version string) error { if linked == 0 { return trace.Errorf("no binaries available to link") } + service := filepath.Join(versionDir, servicePath) + err = os.MkdirAll(li.LinkServiceDir, 0755) + if err != nil { + return trace.Wrap(err) + } + err = renameio.Symlink(service, filepath.Join(li.LinkServiceDir, filepath.Base(servicePath))) + if err != nil { + return trace.Wrap(err) + } return nil } @@ -404,9 +425,10 @@ func (li *LocalInstaller) versionDir(version string) (string, error) { return versionDir, nil } -// isLinked returns true if any binaries in binDir are linked. -// Returns os.ErrNotExist error if the version does not exist. -func (li *LocalInstaller) isLinked(binDir string) (bool, error) { +// isLinked returns true if any binaries or services in versionDir are linked. +// Returns os.ErrNotExist error if the versionDir does not exist. +func (li *LocalInstaller) isLinked(versionDir string) (bool, error) { + binDir := filepath.Join(versionDir, "bin") entries, err := os.ReadDir(binDir) if err != nil { return false, trace.Wrap(err) @@ -415,13 +437,18 @@ func (li *LocalInstaller) isLinked(binDir string) (bool, error) { if entry.IsDir() { continue } - v, err := os.Readlink(filepath.Join(li.LinkDir, entry.Name())) + v, err := os.Readlink(filepath.Join(li.LinkBinDir, entry.Name())) if err != nil { continue } - if v == filepath.Join(binDir, entry.Name()) { + if filepath.Clean(v) == filepath.Join(binDir, entry.Name()) { return true, nil } } - return false, nil + v, err := os.Readlink(filepath.Join(li.LinkServiceDir, filepath.Base(servicePath))) + if err != nil { + return false, nil + } + return filepath.Clean(v) == + filepath.Join(versionDir, servicePath), nil } diff --git a/lib/autoupdate/agent/installer_test.go b/lib/autoupdate/agent/installer_test.go index ec9dacaabbc1c..2602704208855 100644 --- a/lib/autoupdate/agent/installer_test.go +++ b/lib/autoupdate/agent/installer_test.go @@ -204,11 +204,29 @@ func TestLocalInstaller_Link(t *testing.T) { errMatch string }{ { - name: "present", - dirs: []string{"bin", "bin/somedir", "somedir"}, - files: []string{"bin/teleport", "bin/tsh", "bin/tbot", "README"}, - - links: []string{"teleport", "tsh", "tbot"}, + name: "present", + dirs: []string{ + "bin", + "bin/somedir", + "etc", + "etc/systemd", + "etc/systemd/somedir", + "somedir", + }, + files: []string{ + "bin/teleport", + "bin/tsh", + "bin/tbot", + servicePath, + "README", + }, + + links: []string{ + "bin/teleport", + "bin/tsh", + "bin/tbot", + "lib/systemd/system/teleport.service", + }, }, { name: "no links", @@ -245,9 +263,10 @@ func TestLocalInstaller_Link(t *testing.T) { linkDir := t.TempDir() installer := &LocalInstaller{ - InstallDir: versionsDir, - LinkDir: linkDir, - Log: slog.Default(), + InstallDir: versionsDir, + LinkBinDir: filepath.Join(linkDir, "bin"), + LinkServiceDir: filepath.Join(linkDir, "lib/systemd/system"), + Log: slog.Default(), } ctx := context.Background() err = installer.Link(ctx, version) @@ -261,7 +280,7 @@ func TestLocalInstaller_Link(t *testing.T) { for _, link := range tt.links { v, err := os.ReadFile(filepath.Join(linkDir, link)) require.NoError(t, err) - require.Equal(t, link, string(v)) + require.Equal(t, filepath.Base(link), string(v)) } }) } @@ -370,9 +389,10 @@ func TestLocalInstaller_Remove(t *testing.T) { linkDir := t.TempDir() installer := &LocalInstaller{ - InstallDir: versionsDir, - LinkDir: linkDir, - Log: slog.Default(), + InstallDir: versionsDir, + LinkBinDir: linkDir, + LinkServiceDir: linkDir, + Log: slog.Default(), } ctx := context.Background() diff --git a/lib/autoupdate/agent/updater.go b/lib/autoupdate/agent/updater.go index 7e66e461ddc7b..51c875aa10d45 100644 --- a/lib/autoupdate/agent/updater.go +++ b/lib/autoupdate/agent/updater.go @@ -115,7 +115,7 @@ func NewLocalUpdater(cfg LocalUpdaterConfig) (*Updater, error) { cfg.Log = slog.Default() } if cfg.LinkDir == "" { - cfg.LinkDir = "/usr/local/bin" + cfg.LinkDir = "/usr/local" } if cfg.VersionsDir == "" { cfg.VersionsDir = filepath.Join(libdefaults.DataDir, "versions") @@ -126,10 +126,11 @@ func NewLocalUpdater(cfg LocalUpdaterConfig) (*Updater, error) { InsecureSkipVerify: cfg.InsecureSkipVerify, ConfigPath: filepath.Join(cfg.VersionsDir, updateConfigName), Installer: &LocalInstaller{ - InstallDir: cfg.VersionsDir, - LinkDir: cfg.LinkDir, - HTTP: client, - Log: cfg.Log, + InstallDir: cfg.VersionsDir, + LinkBinDir: filepath.Join(cfg.LinkDir, "bin"), + LinkServiceDir: filepath.Join(cfg.LinkDir, "lib", "systemd", "system"), + HTTP: client, + Log: cfg.Log, ReservedFreeTmpDisk: reservedFreeDisk, ReservedFreeInstallDisk: reservedFreeDisk, @@ -149,7 +150,7 @@ type LocalUpdaterConfig struct { DownloadTimeout time.Duration // VersionsDir for installing Teleport (usually /var/lib/teleport/versions). VersionsDir string - // LinkDir for installing Teleport (usually /usr/local/bin). + // LinkDir for installing Teleport (usually /usr/local). LinkDir string } diff --git a/tool/teleport-update/main.go b/tool/teleport-update/main.go index 11aee2aae3906..300da6736471a 100644 --- a/tool/teleport-update/main.go +++ b/tool/teleport-update/main.go @@ -81,16 +81,8 @@ type cliConfig struct { LogFormat string // DataDir for Teleport (usually /var/lib/teleport) DataDir string -} - -func (c *cliConfig) CheckAndSetDefaults() error { - if c.DataDir == "" { - c.DataDir = libdefaults.DataDir - } - if c.LogFormat == "" { - c.LogFormat = libutils.LogFormatText - } - return nil + // LinkDir for linking binaries and systemd services + LinkDir string } func Run(args []string) error { @@ -105,6 +97,8 @@ func Run(args []string) error { Default(libdefaults.DataDir).StringVar(&ccfg.DataDir) app.Flag("log-format", "Controls the format of output logs. Can be `json` or `text`. Defaults to `text`."). Default(libutils.LogFormatText).EnumVar(&ccfg.LogFormat, libutils.LogFormatJSON, libutils.LogFormatText) + app.Flag("link-dir", "Directory to create system symlinks to binaries and services."). + Default(filepath.Join("usr", "local")).Hidden().StringVar(&ccfg.LinkDir) app.HelpFlag.Short('h') @@ -138,10 +132,6 @@ func Run(args []string) error { return trace.Errorf("failed to set up logger") } - if err := ccfg.CheckAndSetDefaults(); err != nil { - return trace.Wrap(err) - } - switch command { case enableCmd.FullCommand(): err = cmdEnable(ctx, &ccfg) @@ -194,6 +184,7 @@ func cmdDisable(ctx context.Context, ccfg *cliConfig) error { }() updater, err := autoupdate.NewLocalUpdater(autoupdate.LocalUpdaterConfig{ VersionsDir: versionsDir, + LinkDir: ccfg.LinkDir, Log: plog, }) if err != nil { @@ -225,6 +216,7 @@ func cmdEnable(ctx context.Context, ccfg *cliConfig) error { updater, err := autoupdate.NewLocalUpdater(autoupdate.LocalUpdaterConfig{ VersionsDir: versionsDir, + LinkDir: ccfg.LinkDir, Log: plog, }) if err != nil { From 772881e8b12cf8cc6a1a74f3aa4a175f0e0dbe0e Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Wed, 23 Oct 2024 22:07:33 -0400 Subject: [PATCH 17/18] fix bugs --- lib/autoupdate/agent/installer.go | 28 +++++++++++++++++++--------- lib/autoupdate/agent/updater.go | 3 +++ 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/lib/autoupdate/agent/installer.go b/lib/autoupdate/agent/installer.go index ee9d7c81c844a..978fa64c67688 100644 --- a/lib/autoupdate/agent/installer.go +++ b/lib/autoupdate/agent/installer.go @@ -52,6 +52,7 @@ var ( tgzExtractPaths = []utils.ExtractPath{ {Src: "teleport/examples/systemd/teleport.service", Dst: "etc/systemd/teleport.service"}, {Src: "teleport/examples", Skip: true}, + {Src: "teleport/install", Skip: true}, {Src: "teleport/README.md", Dst: "share/README.md"}, {Src: "teleport/CHANGELOG.md", Dst: "share/CHANGELOG.md"}, {Src: "teleport/VERSION", Dst: "share/VERSION"}, @@ -380,14 +381,21 @@ func (li *LocalInstaller) Link(ctx context.Context, version string) error { return trace.Wrap(err) } binDir := filepath.Join(versionDir, "bin") - entries, err := os.ReadDir(binDir) + // ensure target directories exist before trying to create links + err = os.MkdirAll(li.LinkBinDir, 0755) if err != nil { - return trace.Errorf("failed to find Teleport binary directory: %w", err) + return trace.Wrap(err) } - err = os.MkdirAll(li.LinkBinDir, 0755) + err = os.MkdirAll(li.LinkServiceDir, 0755) if err != nil { return trace.Wrap(err) } + + // create binary links + entries, err := os.ReadDir(binDir) + if err != nil { + return trace.Errorf("failed to find Teleport binary directory: %w", err) + } var linked int for _, entry := range entries { if entry.IsDir() { @@ -402,11 +410,9 @@ func (li *LocalInstaller) Link(ctx context.Context, version string) error { if linked == 0 { return trace.Errorf("no binaries available to link") } + + // create systemd service link service := filepath.Join(versionDir, servicePath) - err = os.MkdirAll(li.LinkServiceDir, 0755) - if err != nil { - return trace.Wrap(err) - } err = renameio.Symlink(service, filepath.Join(li.LinkServiceDir, filepath.Base(servicePath))) if err != nil { return trace.Wrap(err) @@ -418,8 +424,12 @@ func (li *LocalInstaller) Link(ctx context.Context, version string) error { // versionDir will fail if the version cannot be used to construct the directory name. // For example, it ensures that ".." cannot be provided to return a system directory. func (li *LocalInstaller) versionDir(version string) (string, error) { - versionDir := filepath.Join(li.InstallDir, version) - if filepath.Dir(versionDir) != filepath.Clean(li.InstallDir) { + installDir, err := filepath.Abs(li.InstallDir) + if err != nil { + return "", trace.Wrap(err) + } + versionDir := filepath.Join(installDir, version) + if filepath.Dir(versionDir) != filepath.Clean(installDir) { return "", trace.Errorf("refusing to directory outside of version directory") } return versionDir, nil diff --git a/lib/autoupdate/agent/updater.go b/lib/autoupdate/agent/updater.go index 51c875aa10d45..ade0704607cb9 100644 --- a/lib/autoupdate/agent/updater.go +++ b/lib/autoupdate/agent/updater.go @@ -292,6 +292,9 @@ func (u *Updater) Enable(ctx context.Context, override OverrideConfig) error { } else { u.Log.InfoContext(ctx, "Target version successfully validated.", "version", desiredVersion) } + if v := cfg.Status.BackupVersion; v != "" { + u.Log.InfoContext(ctx, "Backup version set.", "version", v) + } versions, err := u.Installer.List(ctx) if err != nil { From 28bb522afea76c1206dc5138f3ebcf95555c7b93 Mon Sep 17 00:00:00 2001 From: Stephen Levine Date: Wed, 23 Oct 2024 22:14:17 -0400 Subject: [PATCH 18/18] cleanup --- lib/autoupdate/agent/installer.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/autoupdate/agent/installer.go b/lib/autoupdate/agent/installer.go index 978fa64c67688..4da41e8e55509 100644 --- a/lib/autoupdate/agent/installer.go +++ b/lib/autoupdate/agent/installer.go @@ -380,7 +380,7 @@ func (li *LocalInstaller) Link(ctx context.Context, version string) error { if err != nil { return trace.Wrap(err) } - binDir := filepath.Join(versionDir, "bin") + // ensure target directories exist before trying to create links err = os.MkdirAll(li.LinkBinDir, 0755) if err != nil { @@ -392,6 +392,7 @@ func (li *LocalInstaller) Link(ctx context.Context, version string) error { } // create binary links + binDir := filepath.Join(versionDir, "bin") entries, err := os.ReadDir(binDir) if err != nil { return trace.Errorf("failed to find Teleport binary directory: %w", err)