Skip to content

Commit

Permalink
[teleport-update] Fix usage of default $PATH dir (#52604)
Browse files Browse the repository at this point in the history 
* Fix usage of default path

* fix other overrides
  • Loading branch information
@sclevine
sclevine authored Feb 28, 2025
1 parent 569cb3c commit 1751fa7
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 56 deletions.
12 changes: 6 additions & 6 deletions lib/autoupdate/agent/installer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ type LocalInstaller struct {
// ReservedFreeInstallDisk is the amount of disk that must remain free in the install directory.
ReservedFreeInstallDisk uint64
// TransformService transforms the systemd service during copying.
TransformService func([]byte) []byte
TransformService func(cfg []byte, pathDir string) []byte
// ValidateBinary returns true if a file is a linkable binary, or
// false if a file should not be linked.
ValidateBinary func(ctx context.Context, path string) (bool, error)
Expand Down Expand Up @@ -585,7 +585,7 @@ func (li *LocalInstaller) forceLinks(ctx context.Context, srcBinDir, srcSvcFile,

// create systemd service file

orig, err := li.forceCopyService(li.TargetServiceFile, srcSvcFile, maxServiceFileSize)
orig, err := li.forceCopyService(li.TargetServiceFile, srcSvcFile, maxServiceFileSize, dstBinDir)
if err != nil && !errors.Is(err, os.ErrExist) {
return revert, trace.Wrap(err, "failed to copy service")
}
Expand All @@ -598,12 +598,12 @@ func (li *LocalInstaller) forceLinks(ctx context.Context, srcBinDir, srcSvcFile,
// forceCopyService uses forceCopy to copy a systemd service file from src to dst.
// The contents of both src and dst must be smaller than n.
// See forceCopy for more details.
func (li *LocalInstaller) forceCopyService(dst, src string, n int64) (orig *smallFile, err error) {
func (li *LocalInstaller) forceCopyService(dst, src string, n int64, dstBinDir string) (orig *smallFile, err error) {
srcData, err := readFileAtMost(src, n)
if err != nil {
return nil, trace.Wrap(err)
}
return forceCopy(dst, li.TransformService(srcData), n)
return forceCopy(dst, li.TransformService(srcData, dstBinDir), n)
}

// forceLink attempts to create a symlink, atomically replacing an existing link if already present.
Expand Down Expand Up @@ -735,7 +735,7 @@ func (li *LocalInstaller) removeLinks(ctx context.Context, srcBinDir, srcSvcFile
if err != nil {
return trace.Wrap(err)
}
if !bytes.Equal(li.TransformService(srcBytes), dstBytes) {
if !bytes.Equal(li.TransformService(srcBytes, dstBinDir), dstBytes) {
li.Log.WarnContext(ctx, "Removed teleport binary link, but skipping removal of custom teleport.service: the service file does not match the reference file for this version. The file might have been manually edited.")
return nil
}
Expand Down Expand Up @@ -807,7 +807,7 @@ func (li *LocalInstaller) tryLinks(ctx context.Context, srcBinDir, srcSvcFile, d
}

// if any binaries are linked from srcBinDir, always link the service from svcDir
_, err = li.forceCopyService(li.TargetServiceFile, srcSvcFile, maxServiceFileSize)
_, err = li.forceCopyService(li.TargetServiceFile, srcSvcFile, maxServiceFileSize, dstBinDir)
if err != nil && !errors.Is(err, os.ErrExist) {
return trace.Wrap(err, "failed to copy service")
}
Expand Down
35 changes: 18 additions & 17 deletions lib/autoupdate/agent/installer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"context"
"crypto/sha256"
"encoding/hex"
"fmt"
"io"
"log/slog"
"net/http"
Expand Down Expand Up @@ -458,8 +459,8 @@ func TestLocalInstaller_Link(t *testing.T) {
InstallDir: versionsDir,
TargetServiceFile: filepath.Join(linkDir, serviceDir, serviceName),
Log: slog.Default(),
TransformService: func(b []byte) []byte {
return []byte("[transform]" + string(b))
TransformService: func(b []byte, pathDir string) []byte {
return []byte(fmt.Sprintf("[service=%s][path=%s]", string(b), pathDir))
},
ValidateBinary: validator.IsExecutable,
Template: autoupdate.DefaultCDNURITemplate,
Expand Down Expand Up @@ -498,7 +499,7 @@ func TestLocalInstaller_Link(t *testing.T) {
for _, svc := range tt.resultServices {
v, err := os.ReadFile(filepath.Join(linkDir, svc))
require.NoError(t, err)
require.Equal(t, "[transform]"+filepath.Base(svc), string(v))
require.Equal(t, fmt.Sprintf("[service=%s][path=%s]", filepath.Base(svc), filepath.Join(linkDir, "bin")), string(v))
}

// verify manual revert
Expand Down Expand Up @@ -713,8 +714,8 @@ func TestLocalInstaller_TryLink(t *testing.T) {
InstallDir: versionsDir,
TargetServiceFile: filepath.Join(linkDir, serviceDir, serviceName),
Log: slog.Default(),
TransformService: func(b []byte) []byte {
return []byte("[transform]" + string(b))
TransformService: func(b []byte, pathDir string) []byte {
return []byte(fmt.Sprintf("[service=%s][path=%s]", string(b), pathDir))
},
ValidateBinary: validator.IsExecutable,
}
Expand Down Expand Up @@ -748,7 +749,7 @@ func TestLocalInstaller_TryLink(t *testing.T) {
for _, svc := range tt.resultServices {
v, err := os.ReadFile(filepath.Join(linkDir, svc))
require.NoError(t, err)
require.Equal(t, "[transform]"+filepath.Base(svc), string(v))
require.Equal(t, fmt.Sprintf("[service=%s][path=%s]", filepath.Base(svc), filepath.Join(linkDir, "bin")), string(v))
}

})
Expand Down Expand Up @@ -850,8 +851,8 @@ func TestLocalInstaller_Remove(t *testing.T) {
InstallDir: versionsDir,
TargetServiceFile: filepath.Join(linkDir, serviceDir, serviceName),
Log: slog.Default(),
TransformService: func(b []byte) []byte {
return []byte("[transform]" + string(b))
TransformService: func(b []byte, pathDir string) []byte {
return []byte(fmt.Sprintf("[service=%s][path=%s]", string(b), pathDir))
},
ValidateBinary: validator.IsExecutable,
}
Expand Down Expand Up @@ -920,8 +921,8 @@ func TestLocalInstaller_IsLinked(t *testing.T) {
InstallDir: versionsDir,
TargetServiceFile: filepath.Join(linkDir, serviceDir, serviceName),
Log: slog.Default(),
TransformService: func(b []byte) []byte {
return []byte("[transform]" + string(b))
TransformService: func(b []byte, pathDir string) []byte {
return []byte(fmt.Sprintf("[service=%s][path=%s]", string(b), pathDir))
},
ValidateBinary: validator.IsExecutable,
}
Expand Down Expand Up @@ -980,7 +981,7 @@ func TestLocalInstaller_Unlink(t *testing.T) {
{oldname: "bin/teleport", newname: "bin/teleport"},
{oldname: "bin/tsh", newname: "bin/tsh"},
},
svcCopy: []byte("[transform]orig"),
svcCopy: []byte("[service=orig][path=bin]"),
},
{
name: "different services",
Expand Down Expand Up @@ -1020,7 +1021,7 @@ func TestLocalInstaller_Unlink(t *testing.T) {
links: []symlink{
{oldname: "bin/tsh", newname: "bin/tsh"},
},
svcCopy: []byte("[transform]orig"),
svcCopy: []byte("[service=orig][path=bin]"),
remaining: []string{servicePath},
},
{
Expand All @@ -1030,7 +1031,7 @@ func TestLocalInstaller_Unlink(t *testing.T) {
links: []symlink{
{oldname: "bin/teleport", newname: "bin/teleport"},
},
svcCopy: []byte("[transform]orig"),
svcCopy: []byte("[service=orig][path=bin]"),
},
{
name: "wrong teleport link",
Expand All @@ -1040,7 +1041,7 @@ func TestLocalInstaller_Unlink(t *testing.T) {
{oldname: "other", newname: "bin/teleport"},
{oldname: "bin/tsh", newname: "bin/tsh"},
},
svcCopy: []byte("[transform]orig"),
svcCopy: []byte("[service=orig][path=bin]"),
remaining: []string{servicePath, "bin/teleport"},
},
{
Expand All @@ -1051,7 +1052,7 @@ func TestLocalInstaller_Unlink(t *testing.T) {
{oldname: "bin/teleport", newname: "bin/teleport"},
{oldname: "wrong", newname: "bin/tsh"},
},
svcCopy: []byte("[transform]orig"),
svcCopy: []byte("[service=orig][path=bin]"),
remaining: []string{"bin/tsh"},
},
}
Expand Down Expand Up @@ -1106,8 +1107,8 @@ func TestLocalInstaller_Unlink(t *testing.T) {
InstallDir: versionsDir,
TargetServiceFile: filepath.Join(linkDir, serviceDir, serviceName),
Log: slog.Default(),
TransformService: func(b []byte) []byte {
return []byte("[transform]" + string(b))
TransformService: func(b []byte, pathDir string) []byte {
return []byte(fmt.Sprintf("[service=%s][path=%s]", string(b), filepath.Base(pathDir)))
},
}
ctx := context.Background()
Expand Down
25 changes: 17 additions & 8 deletions lib/autoupdate/agent/setup.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ var alphanum = regexp.MustCompile("^[a-zA-Z0-9-]*$")
// Namespaces must be alphanumeric + `-`.
// defaultPathDir overrides the destination directory for namespace setup (i.e., /usr/local)
func NewNamespace(ctx context.Context, log *slog.Logger, name, installDir string) (ns *Namespace, err error) {
defer ns.overrideFromConfig(ctx)
defer func() { ns.overrideFromConfig(ctx) }()

if name == defaultNamespace ||
name == systemNamespace {
Expand Down Expand Up @@ -386,14 +386,17 @@ func writeSystemTemplate(path, t string, values any) error {
return trace.Wrap(f.CloseAtomicallyReplace())
}

// replaceTeleportService replaces the default paths in the Teleport service config with namespaced paths.
func (ns *Namespace) replaceTeleportService(cfg []byte) []byte {
// ReplaceTeleportService replaces the default paths in the Teleport service config with namespaced paths.
func (ns *Namespace) ReplaceTeleportService(cfg []byte, pathDir string) []byte {
if pathDir == "" {
pathDir = ns.defaultPathDir
}
for _, rep := range []struct {
old, new string
}{
{
old: "/usr/local/bin/",
new: ns.defaultPathDir + "/",
new: pathDir + "/",
},
{
old: "/etc/teleport.yaml",
Expand All @@ -409,13 +412,19 @@ func (ns *Namespace) replaceTeleportService(cfg []byte) []byte {
return cfg
}

func (ns *Namespace) LogWarning(ctx context.Context) {
func (ns *Namespace) LogWarnings(ctx context.Context, pathDir string) {
if ns.name == "" {
return
}
if pathDir == "" {
pathDir = ns.defaultPathDir
}
ns.log.WarnContext(ctx, "Custom install suffix specified. Teleport data_dir must be configured in the config file.",
"data_dir", ns.dataDir,
"path", ns.defaultPathDir,
"config", ns.configFile,
"path_dir", pathDir,
"config_file", ns.configFile,
"service", filepath.Base(ns.serviceFile),
"pid", ns.pidFile,
"pid_file", ns.pidFile,
)
}

Expand Down
40 changes: 22 additions & 18 deletions lib/autoupdate/agent/updater.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ func NewLocalUpdater(cfg LocalUpdaterConfig, ns *Namespace) (*Updater, error) {
Log: cfg.Log,
Pool: certPool,
InsecureSkipVerify: cfg.InsecureSkipVerify,
UpdateConfigPath: filepath.Join(ns.Dir(), updateConfigName),
TeleportConfigPath: ns.configFile,
UpdateConfigFile: filepath.Join(ns.Dir(), updateConfigName),
TeleportConfigFile: ns.configFile,
DefaultProxyAddr: ns.defaultProxyAddr,
DefaultPathDir: ns.defaultPathDir,
Installer: &LocalInstaller{
Expand All @@ -113,7 +113,7 @@ func NewLocalUpdater(cfg LocalUpdaterConfig, ns *Namespace) (*Updater, error) {
Log: cfg.Log,
ReservedFreeTmpDisk: reservedFreeDisk,
ReservedFreeInstallDisk: reservedFreeDisk,
TransformService: ns.replaceTeleportService,
TransformService: ns.ReplaceTeleportService,
ValidateBinary: validator.IsBinary,
Template: autoupdate.DefaultCDNURITemplate,
},
Expand Down Expand Up @@ -149,6 +149,7 @@ func NewLocalUpdater(cfg LocalUpdaterConfig, ns *Namespace) (*Updater, error) {
},
SetupNamespace: ns.Setup,
TeardownNamespace: ns.Teardown,
LogConfigWarnings: ns.LogWarnings,
}, nil
}

Expand Down Expand Up @@ -180,10 +181,10 @@ type Updater struct {
Pool *x509.CertPool
// InsecureSkipVerify skips TLS verification.
InsecureSkipVerify bool
// UpdateConfigPath contains the path to the agent auto-updates configuration.
UpdateConfigPath string
// TeleportConfig contains the path to Teleport's configuration.
TeleportConfigPath string
// UpdateConfigFile contains the path to the agent auto-updates configuration.
UpdateConfigFile string
// TeleportConfigFile contains the path to Teleport's configuration.
TeleportConfigFile string
// DefaultProxyAddr contains Teleport's proxy address. This may differ from the updater's.
DefaultProxyAddr string
// DefaultPathDir contains the default path that Teleport binaries should be installed into.
Expand All @@ -199,6 +200,8 @@ type Updater struct {
SetupNamespace func(ctx context.Context, path string) error
// TeardownNamespace removes all traces of the updater service in the current Namespace, including Teleport.
TeardownNamespace func(ctx context.Context) error
// LogConfigWarnings logs warnings related to the configuration Namespace.
LogConfigWarnings func(ctx context.Context, pathDir string)
}

// Installer provides an API for installing Teleport agents.
Expand Down Expand Up @@ -315,7 +318,7 @@ func toPtr[T any](t T) *T {
// This function is idempotent.
func (u *Updater) Install(ctx context.Context, override OverrideConfig) error {
// Read configuration from update.yaml and override any new values passed as flags.
cfg, err := readConfig(u.UpdateConfigPath)
cfg, err := readConfig(u.UpdateConfigFile)
if err != nil {
return trace.Wrap(err, "failed to read %s", updateConfigName)
}
Expand Down Expand Up @@ -373,10 +376,11 @@ func (u *Updater) Install(ctx context.Context, override OverrideConfig) error {
// Only write the configuration file if the initial update succeeds.
// Note: skip_version is never set on failed enable, only failed update.

if err := writeConfig(u.UpdateConfigPath, cfg); err != nil {
if err := writeConfig(u.UpdateConfigFile, cfg); err != nil {
return trace.Wrap(err, "failed to write %s", updateConfigName)
}
u.Log.InfoContext(ctx, "Configuration updated.")
u.LogConfigWarnings(ctx, cfg.Spec.Path)
return trace.Wrap(u.notices(ctx))
}

Expand All @@ -403,7 +407,7 @@ func sameProxies(a, b string) bool {
// Before attempting this, Remove attempts to gracefully recover the system-packaged version of Teleport (if present).
// This function is idempotent.
func (u *Updater) Remove(ctx context.Context, force bool) error {
cfg, err := readConfig(u.UpdateConfigPath)
cfg, err := readConfig(u.UpdateConfigFile)
if err != nil {
return trace.Wrap(err, "failed to read %s", updateConfigName)
}
Expand Down Expand Up @@ -544,7 +548,7 @@ func isActiveOrEnabled(ctx context.Context, s Process) (bool, error) {
func (u *Updater) Status(ctx context.Context) (Status, error) {
var out Status
// Read configuration from update.yaml.
cfg, err := readConfig(u.UpdateConfigPath)
cfg, err := readConfig(u.UpdateConfigFile)
if err != nil {
return out, trace.Wrap(err, "failed to read %s", updateConfigName)
}
Expand All @@ -566,7 +570,7 @@ func (u *Updater) Status(ctx context.Context) (Status, error) {
// Disable disables agent auto-updates.
// This function is idempotent.
func (u *Updater) Disable(ctx context.Context) error {
cfg, err := readConfig(u.UpdateConfigPath)
cfg, err := readConfig(u.UpdateConfigFile)
if err != nil {
return trace.Wrap(err, "failed to read %s", updateConfigName)
}
Expand All @@ -575,7 +579,7 @@ func (u *Updater) Disable(ctx context.Context) error {
return nil
}
cfg.Spec.Enabled = false
if err := writeConfig(u.UpdateConfigPath, cfg); err != nil {
if err := writeConfig(u.UpdateConfigFile, cfg); err != nil {
return trace.Wrap(err, "failed to write %s", updateConfigName)
}
return nil
Expand All @@ -584,7 +588,7 @@ func (u *Updater) Disable(ctx context.Context) error {
// Unpin allows the current version to be changed by Update.
// This function is idempotent.
func (u *Updater) Unpin(ctx context.Context) error {
cfg, err := readConfig(u.UpdateConfigPath)
cfg, err := readConfig(u.UpdateConfigFile)
if err != nil {
return trace.Wrap(err, "failed to read %s", updateConfigName)
}
Expand All @@ -593,7 +597,7 @@ func (u *Updater) Unpin(ctx context.Context) error {
return nil
}
cfg.Spec.Pinned = false
if err := writeConfig(u.UpdateConfigPath, cfg); err != nil {
if err := writeConfig(u.UpdateConfigFile, cfg); err != nil {
return trace.Wrap(err, "failed to write %s", updateConfigName)
}
return nil
Expand All @@ -606,7 +610,7 @@ func (u *Updater) Unpin(ctx context.Context) error {
// This function is idempotent.
func (u *Updater) Update(ctx context.Context, now bool) error {
// Read configuration from update.yaml and override any new values passed as flags.
cfg, err := readConfig(u.UpdateConfigPath)
cfg, err := readConfig(u.UpdateConfigFile)
if err != nil {
return trace.Wrap(err, "failed to read %s", updateConfigName)
}
Expand Down Expand Up @@ -686,7 +690,7 @@ func (u *Updater) Update(ctx context.Context, now bool) error {
}

updateErr := u.update(ctx, cfg, target, false, resp.AGPL)
writeErr := writeConfig(u.UpdateConfigPath, cfg)
writeErr := writeConfig(u.UpdateConfigFile, cfg)
if writeErr != nil {
writeErr = trace.Wrap(writeErr, "failed to write %s", updateConfigName)
} else {
Expand Down Expand Up @@ -977,7 +981,7 @@ func (u *Updater) cleanup(ctx context.Context, cfg *UpdateConfig, keep []Revisio
// LinkPackage returns an error only if an unknown version of Teleport is present (e.g., manually copied files).
// This function is idempotent.
func (u *Updater) LinkPackage(ctx context.Context) error {
cfg, err := readConfig(u.UpdateConfigPath)
cfg, err := readConfig(u.UpdateConfigFile)
if err != nil {
return trace.Wrap(err, "failed to read %s", updateConfigName)
}
Expand Down
7 changes: 0 additions & 7 deletions tool/teleport-update/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,13 +312,6 @@ func cmdUnpin(ctx context.Context, ccfg *cliConfig) error {

// cmdInstall installs Teleport and sets configuration.
func cmdInstall(ctx context.Context, ccfg *cliConfig) error {
if ccfg.InstallSuffix != "" {
ns, err := autoupdate.NewNamespace(ctx, plog, ccfg.InstallSuffix, ccfg.InstallDir)
if err != nil {
return trace.Wrap(err)
}
ns.LogWarning(ctx)
}
updater, lockFile, err := initConfig(ctx, ccfg)
if err != nil {
return trace.Wrap(err, "failed to initialize updater")
Expand Down

0 comments on commit 1751fa7

Please sign in to comment.