Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[teleport-update] Fix usage of default $PATH dir #52604

Merged
merged 3 commits into from
Feb 28, 2025
Merged

[teleport-update] Fix usage of default $PATH dir #52604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6f6be8c
Fix usage of default path
sclevine Feb 28, 2025
ff81306
fix other overrides
sclevine Feb 28, 2025
e0ff6a0
fix hang on start
sclevine Feb 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions lib/autoupdate/agent/installer.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading
Loading