[teleport-update] Add --overwrite flag to replace tarball installations (#51579)

* add --overwrite flag

* extra warning
This commit is contained in:
Stephen Levine
2025-01-30 00:35:37 +00:00
committed by GitHub
parent 6a8c0b5330
commit ba65e79c6a
5 changed files with 84 additions and 27 deletions
+17 -12
View File
@@ -413,7 +413,7 @@ func (li *LocalInstaller) List(ctx context.Context) (revs []Revision, err error)
// Link the specified version into the system LinkBinDir and CopyServiceFile.
// The revert function restores the previous linking.
// See Installer interface for additional specs.
func (li *LocalInstaller) Link(ctx context.Context, rev Revision) (revert func(context.Context) bool, err error) {
func (li *LocalInstaller) Link(ctx context.Context, rev Revision, force bool) (revert func(context.Context) bool, err error) {
revert = func(context.Context) bool { return true }
versionDir, err := li.revisionDir(rev)
if err != nil {
@@ -422,6 +422,7 @@ func (li *LocalInstaller) Link(ctx context.Context, rev Revision) (revert func(c
revert, err = li.forceLinks(ctx,
filepath.Join(versionDir, "bin"),
filepath.Join(versionDir, serviceDir, serviceName),
force,
)
if err != nil {
return revert, trace.Wrap(err)
@@ -430,10 +431,11 @@ func (li *LocalInstaller) Link(ctx context.Context, rev Revision) (revert func(c
}
// LinkSystem links the system (package) version into LinkBinDir and CopyServiceFile.
// LinkSystem returns ErrInvalid if LinkBinDir is not DefaultLinkDir.
// The revert function restores the previous linking.
// See Installer interface for additional specs.
func (li *LocalInstaller) LinkSystem(ctx context.Context) (revert func(context.Context) bool, err error) {
revert, err = li.forceLinks(ctx, li.SystemBinDir, li.SystemServiceFile)
revert, err = li.forceLinks(ctx, li.SystemBinDir, li.SystemServiceFile, false)
return revert, trace.Wrap(err)
}
@@ -453,6 +455,7 @@ func (li *LocalInstaller) TryLink(ctx context.Context, revision Revision) error
// TryLinkSystem links the system installation, but only in the case that
// no installation of Teleport is already linked or partially linked.
// TryLinkSystem returns ErrInvalid if LinkBinDir is not DefaultLinkDir.
// See Installer interface for additional specs.
func (li *LocalInstaller) TryLinkSystem(ctx context.Context) error {
return trace.Wrap(li.tryLinks(ctx, li.SystemBinDir, li.SystemServiceFile))
@@ -494,7 +497,7 @@ type smallFile struct {
// forceLinks will revert any overridden links or files if it hits an error.
// If successful, forceLinks may also be reverted after it returns by calling revert.
// The revert function returns true if reverting succeeds.
func (li *LocalInstaller) forceLinks(ctx context.Context, binDir, svcPath string) (revert func(context.Context) bool, err error) {
func (li *LocalInstaller) forceLinks(ctx context.Context, binDir, svcPath string, force bool) (revert func(context.Context) bool, err error) {
// setup revert function
var (
revertLinks []symlink
@@ -562,7 +565,7 @@ func (li *LocalInstaller) forceLinks(ctx context.Context, binDir, svcPath string
if !exec {
continue
}
orig, err := forceLink(oldname, newname)
orig, err := forceLink(oldname, newname, force)
if err != nil && !errors.Is(err, os.ErrExist) {
return revert, trace.Wrap(err, "failed to create symlink for %s", entry.Name())
}
@@ -602,16 +605,18 @@ func (li *LocalInstaller) forceCopyService(dst, src string, n int64) (orig *smal
}
// forceLink attempts to create a symlink, atomically replacing an existing link if already present.
// If a non-symlink file or directory exists in newname already, forceLink errors.
// If a non-symlink file or directory exists in newname already, forceLink errors with ErrFilePresent.
// If the link is already present with the desired oldname, forceLink returns os.ErrExist.
func forceLink(oldname, newname string) (orig string, err error) {
func forceLink(oldname, newname string, force bool) (orig string, err error) {
orig, err = os.Readlink(newname)
if errors.Is(err, os.ErrInvalid) ||
errors.Is(err, syscall.EINVAL) { // workaround missing ErrInvalid wrapper
// important: do not attempt to replace a non-linked install of Teleport
return "", trace.Errorf("refusing to replace file at %s", newname)
}
if err != nil && !errors.Is(err, os.ErrNotExist) {
if force {
return "", trace.Wrap(renameio.Symlink(oldname, newname))
}
// important: do not attempt to replace a non-linked install of Teleport without force
return "", trace.Wrap(ErrFilePresent, "refusing to replace file at %s", newname)
} else if err != nil && !errors.Is(err, os.ErrNotExist) {
return "", trace.Wrap(err)
}
if orig == oldname {
@@ -805,14 +810,14 @@ func (li *LocalInstaller) tryLinks(ctx context.Context, binDir, svcPath string)
}
// needsLink returns true when a symlink from oldname to newname needs to be created, or false if it exists.
// If a non-symlink file or directory exists at newname, needsLink errors.
// If a non-symlink file or directory exists at newname, needsLink errors with ErrFilePresent.
// If a symlink to a different location exists, needsLink errors with ErrLinked.
func needsLink(oldname, newname string) (ok bool, err error) {
orig, err := os.Readlink(newname)
if errors.Is(err, os.ErrInvalid) ||
errors.Is(err, syscall.EINVAL) { // workaround missing ErrInvalid wrapper
// important: do not attempt to replace a non-linked install of Teleport
return false, trace.Errorf("refusing to replace file at %s", newname)
return false, trace.Wrap(ErrFilePresent, "refusing to replace file at %s", newname)
}
if errors.Is(err, os.ErrNotExist) {
return true, nil
+43 -5
View File
@@ -206,6 +206,7 @@ func TestLocalInstaller_Link(t *testing.T) {
installFileMode os.FileMode
existingLinks []string
existingFiles []string
force bool
resultLinks []string
resultServices []string
@@ -344,13 +345,50 @@ func TestLocalInstaller_Link(t *testing.T) {
existingLinks: []string{
"bin/teleport",
"bin/tbot",
"lib/systemd/system/teleport.service",
},
existingFiles: []string{
"lib/systemd/system/teleport.service",
"bin/tsh",
},
errMatch: "refusing",
errMatch: ErrFilePresent.Error(),
},
{
name: "overwriting bin files",
installDirs: []string{
"bin",
"bin/somedir",
"lib",
"lib/systemd",
"lib/systemd/system",
"somedir",
},
installFiles: []string{
"bin/teleport",
"bin/tsh",
"bin/tbot",
servicePath,
"README",
},
installFileMode: os.ModePerm,
existingLinks: []string{
"bin/teleport",
"bin/tbot",
},
existingFiles: []string{
"lib/systemd/system/teleport.service",
"bin/tsh",
},
force: true,
resultLinks: []string{
"bin/teleport",
"bin/tsh",
"bin/tbot",
},
resultServices: []string{
"lib/systemd/system/teleport.service",
},
},
{
name: "no links",
@@ -412,7 +450,7 @@ func TestLocalInstaller_Link(t *testing.T) {
Template: autoupdate.DefaultCDNURITemplate,
}
ctx := context.Background()
revert, err := installer.Link(ctx, NewRevision(version, 0))
revert, err := installer.Link(ctx, NewRevision(version, 0), tt.force)
if tt.errMatch != "" {
require.Error(t, err)
assert.Contains(t, err.Error(), tt.errMatch)
@@ -606,7 +644,7 @@ func TestLocalInstaller_TryLink(t *testing.T) {
"bin/tsh",
},
errMatch: "replace file",
errMatch: ErrFilePresent.Error(),
},
{
name: "no links",
@@ -819,7 +857,7 @@ func TestLocalInstaller_Remove(t *testing.T) {
ctx := context.Background()
if tt.linkedVersion != "" {
_, err = installer.Link(ctx, NewRevision(tt.linkedVersion, 0))
_, err = installer.Link(ctx, NewRevision(tt.linkedVersion, 0), false)
require.NoError(t, err)
}
err = installer.Remove(ctx, NewRevision(tt.removeVersion, 0))
+17 -5
View File
@@ -189,7 +189,7 @@ type Installer interface {
// Link the Teleport agent at the specified revision of Teleport into the linking locations.
// The revert function must restore the previous linking, returning false on any failure.
// Link must be idempotent. Link's revert function must be idempotent.
Link(ctx context.Context, rev Revision) (revert func(context.Context) bool, err error)
Link(ctx context.Context, rev Revision, force bool) (revert func(context.Context) bool, err error)
// LinkSystem links the system installation of Teleport into the linking locations.
// The revert function must restore the previous linking, returning false on any failure.
// LinkSystem must be idempotent. LinkSystem's revert function must be idempotent.
@@ -225,6 +225,8 @@ var (
ErrNotSupported = errors.New("not supported on this platform")
// ErrNoBinaries is returned when no binaries are available to be linked.
ErrNoBinaries = errors.New("no binaries available to link")
// ErrFilePresent is returned when a file is present.
ErrFilePresent = errors.New("file present")
)
const (
@@ -269,6 +271,8 @@ type OverrideConfig struct {
ForceVersion string
// ForceFlags in installed Teleport.
ForceFlags autoupdate.InstallFlags
// AllowOverwrite of installed binaries.
AllowOverwrite bool
}
func deref[T any](ptr *T) T {
@@ -323,7 +327,11 @@ func (u *Updater) Install(ctx context.Context, override OverrideConfig) error {
u.Log.InfoContext(ctx, "Initiating installation.", targetKey, target, activeKey, active)
}
if err := u.update(ctx, cfg, target); err != nil {
if err := u.update(ctx, cfg, target, override.AllowOverwrite); err != nil {
if errors.Is(err, ErrFilePresent) && !override.AllowOverwrite {
u.Log.WarnContext(ctx, "Use --overwrite to force removal of existing binaries installed via script.")
u.Log.WarnContext(ctx, "If a teleport rpm or deb package is installed, upgrade it to the latest version and retry. DO NOT USE --overwrite.")
}
return trace.Wrap(err)
}
if target.Version == skip.Version {
@@ -600,7 +608,7 @@ func (u *Updater) Update(ctx context.Context, now bool) error {
time.Sleep(resp.Jitter)
}
updateErr := u.update(ctx, cfg, target)
updateErr := u.update(ctx, cfg, target, false)
writeErr := writeConfig(u.ConfigPath, cfg)
if writeErr != nil {
writeErr = trace.Wrap(writeErr, "failed to write %s", updateConfigName)
@@ -652,7 +660,7 @@ func (u *Updater) find(ctx context.Context, cfg *UpdateConfig) (FindResp, error)
}, nil
}
func (u *Updater) update(ctx context.Context, cfg *UpdateConfig, target Revision) error {
func (u *Updater) update(ctx context.Context, cfg *UpdateConfig, target Revision, force bool) error {
active := cfg.Status.Active
backup := deref(cfg.Status.Backup)
switch backup {
@@ -685,7 +693,7 @@ func (u *Updater) update(ctx context.Context, cfg *UpdateConfig, target Revision
// Cleanup logic at the end of this function will ensure that they are removed
// eventually.
revert, err := u.Installer.Link(ctx, target)
revert, err := u.Installer.Link(ctx, target, force)
if err != nil {
return trace.Wrap(err, "failed to link")
}
@@ -697,6 +705,10 @@ func (u *Updater) update(ctx context.Context, cfg *UpdateConfig, target Revision
if target.Version != "" {
cfg.Status.Skip = toPtr(target)
}
if force {
u.Log.ErrorContext(ctx, "Unable to revert Teleport symlinks in overwrite mode. Installation likely broken.")
return false
}
if ok := revert(ctx); !ok {
u.Log.ErrorContext(ctx, "Failed to revert Teleport symlinks. Installation likely broken.")
return false
+5 -5
View File
@@ -646,7 +646,7 @@ func TestUpdater_Update(t *testing.T) {
installedBaseURL = baseURL
return tt.installErr
},
FuncLink: func(_ context.Context, rev Revision) (revert func(context.Context) bool, err error) {
FuncLink: func(_ context.Context, rev Revision, force bool) (revert func(context.Context) bool, err error) {
linkedRevision = rev
return func(_ context.Context) bool {
revertFuncCalls++
@@ -1472,7 +1472,7 @@ func TestUpdater_Install(t *testing.T) {
installedBaseURL = baseURL
return tt.installErr
},
FuncLink: func(_ context.Context, rev Revision) (revert func(context.Context) bool, err error) {
FuncLink: func(_ context.Context, rev Revision, force bool) (revert func(context.Context) bool, err error) {
linkedRevision = rev
return func(_ context.Context) bool {
revertFuncCalls++
@@ -1557,7 +1557,7 @@ func blankTestAddr(s []byte) []byte {
type testInstaller struct {
FuncInstall func(ctx context.Context, rev Revision, baseURL string) error
FuncRemove func(ctx context.Context, rev Revision) error
FuncLink func(ctx context.Context, rev Revision) (revert func(context.Context) bool, err error)
FuncLink func(ctx context.Context, rev Revision, force bool) (revert func(context.Context) bool, err error)
FuncLinkSystem func(ctx context.Context) (revert func(context.Context) bool, err error)
FuncTryLink func(ctx context.Context, rev Revision) error
FuncTryLinkSystem func(ctx context.Context) error
@@ -1574,8 +1574,8 @@ func (ti *testInstaller) Remove(ctx context.Context, rev Revision) error {
return ti.FuncRemove(ctx, rev)
}
func (ti *testInstaller) Link(ctx context.Context, rev Revision) (revert func(context.Context) bool, err error) {
return ti.FuncLink(ctx, rev)
func (ti *testInstaller) Link(ctx context.Context, rev Revision, force bool) (revert func(context.Context) bool, err error) {
return ti.FuncLink(ctx, rev, force)
}
func (ti *testInstaller) LinkSystem(ctx context.Context) (revert func(context.Context) bool, err error) {
+2
View File
@@ -115,6 +115,8 @@ func Run(args []string) int {
Short('g').Envar(updateGroupEnvVar).StringVar(&ccfg.Group)
enableCmd.Flag("base-url", "Base URL used to override the Teleport download URL.").
Short('b').Envar(common.BaseURLEnvVar).StringVar(&ccfg.BaseURL)
enableCmd.Flag("overwrite", "Allow existing installed Teleport binaries to be overwritten.").
Short('o').BoolVar(&ccfg.AllowOverwrite)
enableCmd.Flag("force-version", "Force the provided version instead of using the version provided by the Teleport cluster.").
Short('f').Envar(updateVersionEnvVar).Hidden().StringVar(&ccfg.ForceVersion)
enableCmd.Flag("self-setup", "Use the current teleport-update binary to create systemd service config for auto-updates.").