-
Couldn't load subscription status.
- Fork 238
Add Upgrade Functionality #1683
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
729e7e8
3d8efa0
2193cff
e0568a6
fc77ef4
9a22e47
9129f1d
9d52ebb
7135809
25e6a57
ed07005
8545d43
b8988e5
61e54a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
|
|
||
| % toolbox-upgrade(1) | ||
|
|
||
| ## NAME | ||
| toolbox-upgrade - Upgrade packages in Toolbx containers | ||
|
|
||
| ## SYNOPSIS | ||
| **toolbox upgrade** [*--all* | *-a*] [*--container* | *-c* *CONTAINER*] [*CONTAINER*] | ||
|
|
||
| ## DESCRIPTION | ||
| Upgrades packages inside one or more Toolbx containers. The container must have been created using `toolbox create` and must include a label specifying how to upgrade its packages. | ||
|
|
||
| This command will: | ||
| 1. Read the container's metadata label `com.github.containers.toolbox.package-manager.update` | ||
| 2. Run the specified upgrade command inside the container | ||
| 3. Report any errors that occur during the process | ||
|
|
||
| The `--container` flag is optional when a positional container name is given. | ||
|
|
||
| ## LABEL REQUIREMENT | ||
| Each container **must** have the following OCI label set in its metadata: | ||
|
|
||
| `com.github.containers.toolbox.package-manager.update="COMMAND"` | ||
|
|
||
| This label defines the exact package upgrade command to run inside the container. For example: | ||
|
|
||
| `com.github.containers.toolbox.package-manager.update="sudo dnf --assumeyes update"` | ||
|
|
||
| This label is typically the responsibility of the **image publisher** and should be present at container creation. | ||
|
|
||
| ## OPTIONS | ||
|
|
||
| **--all, -a** | ||
| Upgrade all Toolbx containers. Cannot be used with *--container* or a positional argument. | ||
|
|
||
| **--container, -c** *CONTAINER* | ||
| Upgrade a specific Toolbx container. Optional when a positional container name is provided. | ||
|
|
||
| ## EXAMPLES | ||
|
|
||
| **Upgrade a specific container (positional):** | ||
|
|
||
| $ toolbox upgrade fedora-toolbox-38 | ||
|
|
||
|
|
||
| **Upgrade a specific container (flag):** | ||
|
|
||
| $ toolbox upgrade --container fedora-toolbox-38 | ||
|
|
||
|
|
||
| **Upgrade all containers:** | ||
|
|
||
| $ toolbox upgrade --all | ||
|
|
||
|
|
||
| ## NOTES | ||
|
|
||
| - This command doesn't perform package manager detection itself. | ||
| - It relies entirely on the container image to define the correct update mechanism. | ||
| - The `package-manager.update` label **must be set**; otherwise, the upgrade will fail. | ||
|
|
||
| ## SEE ALSO | ||
| `toolbox(1)`, `toolbox-create(1)`, `toolbox-list(1)` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| FROM docker.io/library/archlinux:base-devel | ||
|
|
||
| LABEL com.github.containers.toolbox="true" \ | ||
| com.github.containers.toolbox.package-manager.update="sudo pacman -Syu --noconfirm" \ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue is that this is not going to work that reliably. or similar could work but there is no guarantees as Arch Linux is not built around the assumptions that unattended upgrades are supported. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, interesting. Thanks for that insight, @Foxboron In that case, my vote is to not add the label to the |
||
| name="arch-toolbox" \ | ||
| version="base-devel" \ | ||
| usage="This image is meant to be used with the toolbox command" \ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| /* | ||
| * Copyright © 2025 Hadi Chokr <[email protected]> | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "os" | ||
|
|
||
| "github.com/containers/toolbox/pkg/podman" | ||
| "github.com/containers/toolbox/pkg/utils" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| var ( | ||
| upgradeAll bool | ||
| upgradeContainer string | ||
| ) | ||
|
|
||
| var upgradeCmd = &cobra.Command{ | ||
| Use: "upgrade [container]", | ||
| Short: "Detect package manager and upgrade packages in toolbx containers", | ||
| Args: cobra.MaximumNArgs(1), | ||
| RunE: runUpgrade, | ||
| ValidArgsFunction: completionContainerNamesFiltered, | ||
| } | ||
|
|
||
| func init() { | ||
| upgradeCmd.Flags().BoolVar(&upgradeAll, "all", false, "Upgrade all Toolbx containers") | ||
| upgradeCmd.Flags().StringVar(&upgradeContainer, "container", "", "Name of the toolbox container to upgrade") | ||
|
|
||
| // Register container flag completion | ||
| if err := upgradeCmd.RegisterFlagCompletionFunc("container", completionContainerNames); err != nil { | ||
| fmt.Fprintf(os.Stderr, "failed to register flag completion function: %v\n", err) | ||
| os.Exit(1) | ||
| } | ||
| upgradeCmd.SetHelpFunc(upgradeHelp) | ||
| rootCmd.AddCommand(upgradeCmd) | ||
| } | ||
|
|
||
| func runUpgrade(cmd *cobra.Command, args []string) error { | ||
| // Use positional argument as container name if --container not set | ||
| if upgradeContainer == "" && len(args) == 1 { | ||
| upgradeContainer = args[0] | ||
| } | ||
|
|
||
| if !upgradeAll && upgradeContainer == "" { | ||
| return errors.New("must specify either --all or a container name") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should show: ... as we do for the other commands. eg., see src/cmd/rm.go |
||
| } | ||
|
|
||
| if upgradeAll && upgradeContainer != "" { | ||
| return errors.New("cannot specify both --all and a container name") | ||
| } | ||
|
|
||
| if upgradeAll { | ||
| containers, err := getContainers() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if len(containers) == 0 { | ||
| return errors.New("no Toolbx containers found") | ||
| } | ||
|
|
||
| for _, container := range containers { | ||
| fmt.Printf("Upgrading container: %s\n", container.Name()) | ||
| if err := execUpgradeInContainer(container.Name()); err != nil { | ||
| fmt.Fprintf(os.Stderr, "Error upgrading container %s: %v\n", container.Name(), err) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| return execUpgradeInContainer(upgradeContainer) | ||
| } | ||
|
|
||
| // execUpgradeInContainer runs detection and upgrade inside the specified container | ||
| func execUpgradeInContainer(container string) error { | ||
|
|
||
| inspectedcontainer, err := podman.InspectContainer(container) | ||
| if err != nil { | ||
| return errors.New("Failed to inspect Metadata.") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error should be: ... as is used elsewhere in the code base. |
||
| } | ||
|
|
||
| labels := inspectedcontainer.Labels() | ||
| pkgline := labels["com.github.containers.toolbox.package-manager.update"] | ||
| if pkgline == "" { | ||
| return errors.New("'com.github.containers.toolbox.package-manager.update' Label not set in Containers Metadata.") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need an error message that a user, who doesn't know about the internal implementation details, can understand and suggests how to move forward. :) Something like: |
||
| } else { | ||
| // Use runCommand to execute the upgrade | ||
| upgradeErr := runCommand(inspectedcontainer.Name(), | ||
| false, | ||
| "", | ||
| "", | ||
| 0, | ||
| []string{"sh", "-c", pkgline}, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking about this over the past few weeks in the context of the NVIDIAScape security issue, and it struck me that someone can trick a user into using a malicious image that contains a harmful command in its It will be wise to prompt the user with the command that will be run before running it. The user can use the This makes me wonder if we should have variants of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Immediately after I submitted the comment, it struck me that a malicious image can already trick Toolbx into doing bad things. Such as, the entry point of Toolbx containers rely on the We could re-implement the parts of So, I suppose, the security argument doesn't really hold? Even then, maybe we should offer a prompt and show what's about to happen just for the sake of politeness? Just like we do before downloading an image. I am still wondering if this means we should have a prompt before the package manager is invoked, or after it's invoked but before the packages are downloaded and changed, or both. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably the best approach would be to tell the user the update command then prompt them and if the user wants via a flag skip it but by default they will be told about it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You mean prompting the user both before and after invoking the package manager? ie., prompting the user that I suspect we could avoid the first prompt and only inform the user that I am beginning to think that having too many prompts is annoying, and it's better than the alternative of potentially rudely surprising the users by not letting them have a say.
How do you want to implement that? The first step for declaring full support for an OS is to run the CI on hosts with that OS, which isn't a trivial task. Do you want to skip the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it makes sense to have a confirmation prompt before running the package manager if the command has some kind of |
||
| false, | ||
| false, | ||
| true) | ||
|
|
||
| return upgradeErr | ||
|
|
||
| } | ||
| } | ||
|
|
||
| func upgradeHelp(cmd *cobra.Command, args []string) { | ||
| if utils.IsInsideContainer() { | ||
| if !utils.IsInsideToolboxContainer() { | ||
| fmt.Fprintf(os.Stderr, "Error: this is not a Toolbx container\n") | ||
| return | ||
| } | ||
|
|
||
| if _, err := utils.ForwardToHost(); err != nil { | ||
| fmt.Fprintf(os.Stderr, "Error: %s\n", err) | ||
| return | ||
| } | ||
|
|
||
| return | ||
| } | ||
|
|
||
| if err := showManual("toolbox-upgrade"); err != nil { | ||
| fmt.Fprintf(os.Stderr, "Error: %s\n", err) | ||
| return | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| # shellcheck shell=bats | ||
| # | ||
| # Copyright © 2025 Hadi Chokr <[email protected]> | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
| # You may not use this file except in compliance with the License. | ||
| # You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, software | ||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| # See the License for the specific language governing permissions and | ||
| # limitations under the License. | ||
| # | ||
| # bats file_tags=commands-options | ||
|
|
||
| load 'libs/bats-support/load' | ||
| load 'libs/bats-assert/load' | ||
| load 'libs/helpers' | ||
|
|
||
| setup() { | ||
| bats_require_minimum_version 1.8.0 | ||
| cleanup_all | ||
| } | ||
|
|
||
| teardown() { | ||
| cleanup_all | ||
| } | ||
|
|
||
| @test "upgrade(Arch): Upgrade Arch container" { | ||
| create_distro_container arch latest arch-toolbox-test | ||
| assert_success | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it really necessary to use assert_success here? It's supposed to be used to check the exit code of the command invoked by the When we only need to check that a command succeeded, there's no need to use |
||
|
|
||
| run "$TOOLBX" upgrade --container arch-toolbox-test | ||
| assert_success | ||
| } | ||
|
|
||
| @test "upgrade(Arch): Upgrade Arch container without --container flag" { | ||
| create_distro_container arch latest arch-toolbox-test | ||
| assert_success | ||
|
|
||
| run "$TOOLBX" upgrade arch-toolbox-test | ||
| assert_success | ||
| } | ||
|
|
||
| @test "upgrade(All): Upgrade all containers" { | ||
| create_distro_container arch latest arch-toolbox-all-test | ||
| create_default_container | ||
| assert_success | ||
|
|
||
| run "$TOOLBX" upgrade --all | ||
| assert_success | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't you want to support specifying multiple containers by name:
$ toolbox upgrade CONTAINER1 CONTAINER2The
rmandrmicommands support that.