From 52ab99b51592d4a182d95634d31b77d7c388b0e2 Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Wed, 5 Jun 2024 19:56:43 +0000 Subject: [PATCH 1/3] feat(apt): ensure no interactions during aptget with environment variable DEBIAN_FRONTEND=noninteractive is used for disabling all interactions on frontends of commands like apt-get, dpkg, etc. https://bugs.launchpad.net/juju/+bug/2011637 reports that their upgrade was stuck in a grub prompt during an apt-get install. This change preceeds all apt-get commands with the DEBIAN_FRONTEND=noninteractive env variable to ensure no prompts or interactions happen during apt-get. According to the `man 7 debconf`, this env variable "makes the default answers be used for all questions". --- commands/apt.go | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/commands/apt.go b/commands/apt.go index cecb947..d51cd41 100644 --- a/commands/apt.go +++ b/commands/apt.go @@ -6,11 +6,8 @@ package commands import "github.com/juju/packaging/v2/config" +// Constants for apt-based basic commands const ( - // AptConfFilePath is the full file path for the proxy settings that are - // written by cloud-init and the machine environ worker. - AptConfFilePath = "/etc/apt/apt.conf.d/95-juju-proxy-settings" - // the basic command for all dpkg calls: dpkg = "dpkg" @@ -32,6 +29,17 @@ const ( // the basic command for all apt-config calls: aptconfig = "apt-config dump" +) + +// Constants for configurations and environment variables for apt commands +const ( + // AptConfFilePath is the full file path for the proxy settings that are + // written by cloud-init and the machine environ worker. + AptConfFilePath = "/etc/apt/apt.conf.d/95-juju-proxy-settings" + + // Environment variable for disabling interactive prompts in frontends of + // commands like apt-get + frontendNoninteractive = "DEBIAN_FRONTEND=noninteractive" // the basic format for specifying a proxy option for apt: aptProxySettingFormat = "Acquire::%s::Proxy %q;" @@ -42,12 +50,12 @@ const ( // aptCmder is the packageCommander instantiation for apt-based systems. var aptCmder = packageCommander{ - prereq: buildCommand(aptget, "install python-software-properties"), - update: buildCommand(aptget, "update"), - upgrade: buildCommand(aptget, "upgrade"), - install: buildCommand(aptget, "install"), - remove: buildCommand(aptget, "remove"), - purge: buildCommand(aptget, "purge"), + prereq: buildCommand(frontendNoninteractive, aptget, "install python-software-properties"), + update: buildCommand(frontendNoninteractive, aptget, "update"), + upgrade: buildCommand(frontendNoninteractive, aptget, "upgrade"), + install: buildCommand(frontendNoninteractive, aptget, "install"), + remove: buildCommand(frontendNoninteractive, aptget, "remove"), + purge: buildCommand(frontendNoninteractive, aptget, "purge"), search: buildCommand(aptcache, "search --names-only ^%s$"), isInstalled: buildCommand(dpkgquery, "-s %s"), listAvailable: buildCommand(aptcache, "pkgnames"), @@ -55,7 +63,7 @@ var aptCmder = packageCommander{ addRepository: buildCommand(addaptrepo, "%q"), listRepositories: buildCommand(`sed -r -n "s|^deb(-src)? (.*)|\2|p"`, "/etc/apt/sources.list"), removeRepository: buildCommand(addaptrepo, "--remove ppa:%s"), - cleanup: buildCommand(aptget, "autoremove"), + cleanup: buildCommand(frontendNoninteractive, aptget, "autoremove"), getProxy: buildCommand(aptconfig, "Acquire::http::Proxy Acquire::https::Proxy Acquire::ftp::Proxy"), proxySettingsFormat: aptProxySettingFormat, setProxy: buildCommand("echo %s >> ", AptConfFilePath), From d552f473306822196e75fb335eb4f419d602723e Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Wed, 5 Jun 2024 19:57:46 +0000 Subject: [PATCH 2/3] fix(static-analysis): remove deprecated linters, add nolint for test struct --- .github/golangci-lint.config.yaml | 2 -- manager/manager_test.go | 8 ++++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/.github/golangci-lint.config.yaml b/.github/golangci-lint.config.yaml index 6bfdaba..e3ed7ae 100644 --- a/.github/golangci-lint.config.yaml +++ b/.github/golangci-lint.config.yaml @@ -33,5 +33,3 @@ linters: - misspell - unconvert - unused - - deadcode - - varcheck diff --git a/manager/manager_test.go b/manager/manager_test.go index 1a2d1dc..3dd6efa 100644 --- a/manager/manager_test.go +++ b/manager/manager_test.go @@ -112,10 +112,10 @@ type simpleTestCase struct { expectedAptResult interface{} // the expected snap command which will get executed: - expectedSnapCmd string + expectedSnapCmd string //nolint // the expected result of the given snap operation: - expectedSnapResult interface{} + expectedSnapResult interface{} //nolint // the expected yum command which will get executed: expectedYumCmd string @@ -124,10 +124,10 @@ type simpleTestCase struct { expectedYumResult interface{} // the expected yum command which will get executed: - expectedZypperCmd string + expectedZypperCmd string //nolint // the expected result of the given yum operation: - expectedZypperResult interface{} + expectedZypperResult interface{} //nolint // the function to be applied on the package manager. // returns the result of the operation and the error. From 577e9524e1d5f65db68f96024b23790365541b40 Mon Sep 17 00:00:00 2001 From: Caner Derici Date: Wed, 5 Jun 2024 21:11:08 +0000 Subject: [PATCH 3/3] fix(apt): pass environment variables as argument to RunCommandWithRetry --- commands/apt.go | 16 ++++++++-------- manager/apt.go | 4 ++-- manager/manager.go | 18 +++++++++--------- manager/manager_test.go | 4 ++-- manager/run.go | 3 ++- manager/snap.go | 18 +++++++++--------- manager/yum.go | 2 +- manager/zypper.go | 2 +- 8 files changed, 34 insertions(+), 33 deletions(-) diff --git a/commands/apt.go b/commands/apt.go index d51cd41..86f4b71 100644 --- a/commands/apt.go +++ b/commands/apt.go @@ -39,7 +39,7 @@ const ( // Environment variable for disabling interactive prompts in frontends of // commands like apt-get - frontendNoninteractive = "DEBIAN_FRONTEND=noninteractive" + EnvFrontendNoninteractive = "DEBIAN_FRONTEND=noninteractive" // the basic format for specifying a proxy option for apt: aptProxySettingFormat = "Acquire::%s::Proxy %q;" @@ -50,12 +50,12 @@ const ( // aptCmder is the packageCommander instantiation for apt-based systems. var aptCmder = packageCommander{ - prereq: buildCommand(frontendNoninteractive, aptget, "install python-software-properties"), - update: buildCommand(frontendNoninteractive, aptget, "update"), - upgrade: buildCommand(frontendNoninteractive, aptget, "upgrade"), - install: buildCommand(frontendNoninteractive, aptget, "install"), - remove: buildCommand(frontendNoninteractive, aptget, "remove"), - purge: buildCommand(frontendNoninteractive, aptget, "purge"), + prereq: buildCommand(aptget, "install python-software-properties"), + update: buildCommand(aptget, "update"), + upgrade: buildCommand(aptget, "upgrade"), + install: buildCommand(aptget, "install"), + remove: buildCommand(aptget, "remove"), + purge: buildCommand(aptget, "purge"), search: buildCommand(aptcache, "search --names-only ^%s$"), isInstalled: buildCommand(dpkgquery, "-s %s"), listAvailable: buildCommand(aptcache, "pkgnames"), @@ -63,7 +63,7 @@ var aptCmder = packageCommander{ addRepository: buildCommand(addaptrepo, "%q"), listRepositories: buildCommand(`sed -r -n "s|^deb(-src)? (.*)|\2|p"`, "/etc/apt/sources.list"), removeRepository: buildCommand(addaptrepo, "--remove ppa:%s"), - cleanup: buildCommand(frontendNoninteractive, aptget, "autoremove"), + cleanup: buildCommand(aptget, "autoremove"), getProxy: buildCommand(aptconfig, "Acquire::http::Proxy Acquire::https::Proxy Acquire::ftp::Proxy"), proxySettingsFormat: aptProxySettingFormat, setProxy: buildCommand("echo %s >> ", AptConfFilePath), diff --git a/manager/apt.go b/manager/apt.go index 39f2cc7..517d8c8 100644 --- a/manager/apt.go +++ b/manager/apt.go @@ -48,7 +48,7 @@ func NewAptPackageManager() PackageManager { // Search is defined on the PackageManager interface. func (apt *apt) Search(pack string) (bool, error) { - out, _, err := RunCommandWithRetry(apt.cmder.SearchCmd(pack), apt, apt.retryPolicy) + out, _, err := RunCommandWithRetry(apt.cmder.SearchCmd(pack), apt, apt.retryPolicy, nil) if err != nil { return false, err } @@ -63,7 +63,7 @@ func (apt *apt) Search(pack string) (bool, error) { // Install is defined on the PackageManager interface. func (apt *apt) Install(packs ...string) error { - _, _, err := RunCommandWithRetry(apt.cmder.InstallCmd(packs...), apt.installRetryable, apt.retryPolicy) + _, _, err := RunCommandWithRetry(apt.cmder.InstallCmd(packs...), apt.installRetryable, apt.retryPolicy, []string{commands.EnvFrontendNoninteractive}) return err } diff --git a/manager/manager.go b/manager/manager.go index 1b8aae7..40adc2e 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -23,37 +23,37 @@ type basePackageManager struct { // InstallPrerequisite is defined on the PackageManager interface. func (pm *basePackageManager) InstallPrerequisite() error { - _, _, err := RunCommandWithRetry(pm.cmder.InstallPrerequisiteCmd(), pm, pm.retryPolicy) + _, _, err := RunCommandWithRetry(pm.cmder.InstallPrerequisiteCmd(), pm, pm.retryPolicy, nil) return err } // Update is defined on the PackageManager interface. func (pm *basePackageManager) Update() error { - _, _, err := RunCommandWithRetry(pm.cmder.UpdateCmd(), pm, pm.retryPolicy) + _, _, err := RunCommandWithRetry(pm.cmder.UpdateCmd(), pm, pm.retryPolicy, nil) return err } // Upgrade is defined on the PackageManager interface. func (pm *basePackageManager) Upgrade() error { - _, _, err := RunCommandWithRetry(pm.cmder.UpgradeCmd(), pm, pm.retryPolicy) + _, _, err := RunCommandWithRetry(pm.cmder.UpgradeCmd(), pm, pm.retryPolicy, nil) return err } // Install is defined on the PackageManager interface. func (pm *basePackageManager) Install(packs ...string) error { - _, _, err := RunCommandWithRetry(pm.cmder.InstallCmd(packs...), pm, pm.retryPolicy) + _, _, err := RunCommandWithRetry(pm.cmder.InstallCmd(packs...), pm, pm.retryPolicy, nil) return err } // Remove is defined on the PackageManager interface. func (pm *basePackageManager) Remove(packs ...string) error { - _, _, err := RunCommandWithRetry(pm.cmder.RemoveCmd(packs...), pm, pm.retryPolicy) + _, _, err := RunCommandWithRetry(pm.cmder.RemoveCmd(packs...), pm, pm.retryPolicy, nil) return err } // Purge is defined on the PackageManager interface. func (pm *basePackageManager) Purge(packs ...string) error { - _, _, err := RunCommandWithRetry(pm.cmder.PurgeCmd(packs...), pm, pm.retryPolicy) + _, _, err := RunCommandWithRetry(pm.cmder.PurgeCmd(packs...), pm, pm.retryPolicy, nil) return err } @@ -67,19 +67,19 @@ func (pm *basePackageManager) IsInstalled(pack string) bool { // AddRepository is defined on the PackageManager interface. func (pm *basePackageManager) AddRepository(repo string) error { - _, _, err := RunCommandWithRetry(pm.cmder.AddRepositoryCmd(repo), pm, pm.retryPolicy) + _, _, err := RunCommandWithRetry(pm.cmder.AddRepositoryCmd(repo), pm, pm.retryPolicy, nil) return err } // RemoveRepository is defined on the PackageManager interface. func (pm *basePackageManager) RemoveRepository(repo string) error { - _, _, err := RunCommandWithRetry(pm.cmder.RemoveRepositoryCmd(repo), pm, pm.retryPolicy) + _, _, err := RunCommandWithRetry(pm.cmder.RemoveRepositoryCmd(repo), pm, pm.retryPolicy, nil) return err } // Cleanup is defined on the PackageManager interface. func (pm *basePackageManager) Cleanup() error { - _, _, err := RunCommandWithRetry(pm.cmder.CleanupCmd(), pm, pm.retryPolicy) + _, _, err := RunCommandWithRetry(pm.cmder.CleanupCmd(), pm, pm.retryPolicy, nil) return err } diff --git a/manager/manager_test.go b/manager/manager_test.go index 3dd6efa..b5fd66b 100644 --- a/manager/manager_test.go +++ b/manager/manager_test.go @@ -82,8 +82,8 @@ var ( // getMockRunCommandWithRetry returns a function with the same signature as // RunCommandWithRetry which saves the command it receives in the provided // string whilst always returning no output, 0 error code and nil error. -func getMockRunCommandWithRetry(stor *string) func(string, manager.Retryable, manager.RetryPolicy) (string, int, error) { - return func(cmd string, _ manager.Retryable, _ manager.RetryPolicy) (string, int, error) { +func getMockRunCommandWithRetry(stor *string) func(string, manager.Retryable, manager.RetryPolicy, []string) (string, int, error) { + return func(cmd string, _ manager.Retryable, _ manager.RetryPolicy, _ []string) (string, int, error) { *stor = cmd return "", 0, nil } diff --git a/manager/run.go b/manager/run.go index 2123edb..a5bd2eb 100644 --- a/manager/run.go +++ b/manager/run.go @@ -78,7 +78,7 @@ func DefaultRetryPolicy() RetryPolicy { // It returns the output of the command, the exit code, and an error, if one occurs, // logging along the way. // It was aliased for testing purposes. -var RunCommandWithRetry = func(cmd string, retryable Retryable, policy RetryPolicy) (output string, code int, _ error) { +var RunCommandWithRetry = func(cmd string, retryable Retryable, policy RetryPolicy, envVariables []string) (output string, code int, _ error) { // split the command for use with exec args := strings.Fields(cmd) if len(args) <= 1 { @@ -105,6 +105,7 @@ var RunCommandWithRetry = func(cmd string, retryable Retryable, policy RetryPoli // Create the command for each attempt, because we need to // call cmd.CombinedOutput only once. See http://pad.lv/1394524. command := exec.Command(args[0], args[1:]...) + command.Env = append(os.Environ(), envVariables...) var err error out, err = CommandOutput(command) diff --git a/manager/snap.go b/manager/snap.go index aa03b4f..32b145e 100644 --- a/manager/snap.go +++ b/manager/snap.go @@ -65,7 +65,7 @@ func NewSnapPackageManager() *Snap { // Search is defined on the PackageManager interface. func (snap *Snap) Search(pack string) (bool, error) { - out, _, err := RunCommandWithRetry(snap.cmder.SearchCmd(pack), snap, snap.retryPolicy) + out, _, err := RunCommandWithRetry(snap.cmder.SearchCmd(pack), snap, snap.retryPolicy, nil) if strings.Contains(combinedOutput(out, err), "error: no snap found") { return false, nil } else if err != nil { @@ -77,7 +77,7 @@ func (snap *Snap) Search(pack string) (bool, error) { // IsInstalled is defined on the PackageManager interface. func (snap *Snap) IsInstalled(pack string) bool { - out, _, err := RunCommandWithRetry(snap.cmder.IsInstalledCmd(pack), snap, snap.retryPolicy) + out, _, err := RunCommandWithRetry(snap.cmder.IsInstalledCmd(pack), snap, snap.retryPolicy, nil) if strings.Contains(combinedOutput(out, err), "error: no matching snaps installed") || err != nil { return false } @@ -86,7 +86,7 @@ func (snap *Snap) IsInstalled(pack string) bool { // InstalledChannel returns the snap channel for an installed package. func (snap *Snap) InstalledChannel(pack string) string { - out, _, err := RunCommandWithRetry(fmt.Sprintf("snap info %s", pack), snap, snap.retryPolicy) + out, _, err := RunCommandWithRetry(fmt.Sprintf("snap info %s", pack), snap, snap.retryPolicy, nil) combined := combinedOutput(out, err) matches := trackingRE.FindAllStringSubmatch(combined, 1) if len(matches) == 0 { @@ -99,7 +99,7 @@ func (snap *Snap) InstalledChannel(pack string) string { // ChangeChannel updates the tracked channel for an installed snap. func (snap *Snap) ChangeChannel(pack, channel string) error { cmd := fmt.Sprintf("snap refresh --channel %s %s", channel, pack) - out, _, err := RunCommandWithRetry(cmd, snap, snap.retryPolicy) + out, _, err := RunCommandWithRetry(cmd, snap, snap.retryPolicy, nil) if err != nil { return err } else if strings.Contains(combinedOutput(out, err), "not installed") { @@ -111,7 +111,7 @@ func (snap *Snap) ChangeChannel(pack, channel string) error { // Install is defined on the PackageManager interface. func (snap *Snap) Install(packs ...string) error { - out, _, err := RunCommandWithRetry(snap.cmder.InstallCmd(packs...), snap.installRetryable, snap.retryPolicy) + out, _, err := RunCommandWithRetry(snap.cmder.InstallCmd(packs...), snap.installRetryable, snap.retryPolicy, nil) if snapNotFoundRE.MatchString(combinedOutput(out, err)) { return errors.New("unable to locate package") } @@ -122,7 +122,7 @@ func (snap *Snap) Install(packs ...string) error { func (snap *Snap) GetProxySettings() (proxy.Settings, error) { var res proxy.Settings - out, _, err := RunCommandWithRetry(snap.cmder.GetProxyCmd(), snap, snap.retryPolicy) + out, _, err := RunCommandWithRetry(snap.cmder.GetProxyCmd(), snap, snap.retryPolicy, nil) if strings.Contains(combinedOutput(out, err), `no "proxy" configuration option`) { return res, nil } else if err != nil { @@ -170,12 +170,12 @@ func (snap *Snap) ConfigureStoreProxy(assertions, storeID string) error { _ = assertFile.Close() ackCmd := fmt.Sprintf("snap ack %s", assertFile.Name()) - if _, _, err = RunCommandWithRetry(ackCmd, snap, snap.retryPolicy); err != nil { + if _, _, err = RunCommandWithRetry(ackCmd, snap, snap.retryPolicy, nil); err != nil { return errors.Annotate(err, "failed to execute 'snap ack'") } setCmd := fmt.Sprintf("snap set system proxy.store=%s", storeID) - if _, _, err = RunCommandWithRetry(setCmd, snap, snap.retryPolicy); err != nil { + if _, _, err = RunCommandWithRetry(setCmd, snap, snap.retryPolicy, nil); err != nil { return errors.Annotatef(err, "failed to configure snap to use store ID %q", storeID) } @@ -189,7 +189,7 @@ func (snap *Snap) ConfigureStoreProxy(assertions, storeID string) error { // call to SetProxy. func (snap *Snap) DisableStoreProxy() error { setCmd := "snap set system proxy.store=" - if _, _, err := RunCommandWithRetry(setCmd, snap, snap.retryPolicy); err != nil { + if _, _, err := RunCommandWithRetry(setCmd, snap, snap.retryPolicy, nil); err != nil { return errors.Annotate(err, "failed to configure snap to not use a store proxy") } diff --git a/manager/yum.go b/manager/yum.go index e90b222..4d5eeda 100644 --- a/manager/yum.go +++ b/manager/yum.go @@ -37,7 +37,7 @@ func NewYumPackageManager() PackageManager { // Search is defined on the PackageManager interface. func (yum *yum) Search(pack string) (bool, error) { - _, code, err := RunCommandWithRetry(yum.cmder.SearchCmd(pack), yum, yum.retryPolicy) + _, code, err := RunCommandWithRetry(yum.cmder.SearchCmd(pack), yum, yum.retryPolicy, nil) // yum list package returns 1 when it cannot find the package. if code == 1 { diff --git a/manager/zypper.go b/manager/zypper.go index 7f39efe..74e555c 100644 --- a/manager/zypper.go +++ b/manager/zypper.go @@ -36,7 +36,7 @@ func NewZypperPackageManager() PackageManager { // Search is defined on the PackageManager interface. func (zypper *zypper) Search(pack string) (bool, error) { - _, code, err := RunCommandWithRetry(zypper.cmder.SearchCmd(pack), zypper, zypper.retryPolicy) + _, code, err := RunCommandWithRetry(zypper.cmder.SearchCmd(pack), zypper, zypper.retryPolicy, nil) // zypper search returns 104 when it cannot find the package. if code == 104 {