mirror of
https://github.com/89luca89/distrobox.git
synced 2026-08-19 01:14:49 -05:00
fix(list): scope distrobox detection to known label keys
IsDistrobox matched any label key OR value containing "distrobox", so
unrelated containers carrying a path or project tag that happened to
include the substring (e.g. a workdir under a "distrobox" directory)
leaked into `distrobox list` — and into the rm/upgrade/stop pipelines
that filter through the same call.
Label values are never authoritative; only `manager=distrobox` and the
`distrobox.*` key namespace are ever set by us. Check those directly,
keeping 24b31ed8 working for manager-overridden boxes (apx).
Signed-off-by: Luca Di Maio <luca.dimaio1@gmail.com>
This commit is contained in:
@@ -0,0 +1,73 @@
|
||||
package commands_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/89luca89/distrobox/pkg/commands"
|
||||
"github.com/89luca89/distrobox/pkg/config"
|
||||
"github.com/89luca89/distrobox/pkg/containermanager"
|
||||
"github.com/89luca89/distrobox/pkg/internal/testutil"
|
||||
)
|
||||
|
||||
// ListCommand must drop containers that are not distrobox-owned, even when a
|
||||
// foreign tool's label value happens to contain the substring "distrobox"
|
||||
// (the regression that motivated tightening Container.IsDistrobox).
|
||||
func TestListCommand_FiltersNonDistroboxContainers(t *testing.T) {
|
||||
mock := &testutil.MockContainerManager{
|
||||
ListContainersResult: []containermanager.Container{
|
||||
{
|
||||
Name: "real-box",
|
||||
Image: "registry.fedoraproject.org/fedora-toolbox:latest",
|
||||
Status: "Up",
|
||||
Labels: map[string]string{"manager": "distrobox", "distrobox.unshare_groups": "0"},
|
||||
},
|
||||
{
|
||||
Name: "apx-box",
|
||||
Image: "docker.io/library/ubuntu:22.04",
|
||||
Status: "Exited",
|
||||
Labels: map[string]string{"manager": "apx", "distrobox.unshare_groups": "0"},
|
||||
},
|
||||
{
|
||||
Name: "plain-box",
|
||||
Image: "docker.io/library/alpine:latest",
|
||||
Status: "Up",
|
||||
Labels: map[string]string{"foo": "bar"},
|
||||
},
|
||||
{
|
||||
Name: "dir-box",
|
||||
Image: "docker.io/library/alpine:latest",
|
||||
Status: "Up",
|
||||
Labels: map[string]string{
|
||||
"example.dir": "/home/luca/distrobox",
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
cmd := commands.NewListCommand(&config.Values{}, mock)
|
||||
result, err := cmd.Execute(context.Background())
|
||||
require.NoError(t, err)
|
||||
|
||||
names := make([]string, 0, len(result.Containers))
|
||||
for _, c := range result.Containers {
|
||||
names = append(names, c.Name)
|
||||
}
|
||||
assert.Equal(t, []string{"apx-box", "real-box"}, names,
|
||||
"plain-box (no distrobox labels) and dir-box (distrobox only inside a label value) must be filtered out; result must be sorted by name")
|
||||
}
|
||||
|
||||
// Empty container list propagates through the filter without surprise.
|
||||
func TestListCommand_EmptyResult(t *testing.T) {
|
||||
mock := &testutil.MockContainerManager{
|
||||
ListContainersResult: []containermanager.Container{},
|
||||
}
|
||||
|
||||
cmd := commands.NewListCommand(&config.Values{}, mock)
|
||||
result, err := cmd.Execute(context.Background())
|
||||
require.NoError(t, err)
|
||||
assert.Empty(t, result.Containers)
|
||||
}
|
||||
@@ -102,14 +102,24 @@ type RmOptions struct {
|
||||
ContainerHome string
|
||||
}
|
||||
|
||||
// IsDistrobox returns true if any label key or value contains "distrobox".
|
||||
// We can't just check manager=distrobox because users can override it with
|
||||
// --additional-flags --label=manager=foo (apx does this). The
|
||||
// distrobox.unshare_groups label is always set on creation, so the
|
||||
// substring match catches those containers too.
|
||||
// IsDistrobox returns true when the container was created by distrobox.
|
||||
//
|
||||
// Two label shapes count as distrobox-owned, mirroring what the create path
|
||||
// always sets (pkg/containermanager/providers/{docker,podman}.go):
|
||||
// - manager=distrobox (the standard case)
|
||||
// - any label key prefixed with "distrobox." (e.g. distrobox.unshare_groups)
|
||||
//
|
||||
// The key-prefix branch keeps the 24b31ed8 fix working for containers whose
|
||||
// manager label is overridden via --additional-flags --label=manager=apx,
|
||||
// while staying out of label *values* — those are arbitrary tool/user
|
||||
// strings (workdirs, mount paths, project names) and substring-matching
|
||||
// them produces false positives on unrelated containers.
|
||||
func (c Container) IsDistrobox() bool {
|
||||
for key, value := range c.Labels {
|
||||
if strings.Contains(key, "distrobox") || strings.Contains(value, "distrobox") {
|
||||
if c.Labels["manager"] == "distrobox" {
|
||||
return true
|
||||
}
|
||||
for key := range c.Labels {
|
||||
if strings.HasPrefix(key, "distrobox.") {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
@@ -37,3 +37,55 @@ func TestContainer_IsDistrobox_NilLabels(t *testing.T) {
|
||||
c := containermanager.Container{Labels: nil}
|
||||
assert.False(t, c.IsDistrobox())
|
||||
}
|
||||
|
||||
// Regression: a label *value* that happens to contain the substring "distrobox"
|
||||
// (e.g. another tool tagging the container with a workdir under a directory
|
||||
// named "distrobox") must NOT make us claim the container as ours. Only label
|
||||
// keys with the distrobox. prefix, or manager=distrobox, count.
|
||||
func TestContainer_IsDistrobox_LabelValueSubstringIgnored(t *testing.T) {
|
||||
c := containermanager.Container{
|
||||
Labels: map[string]string{
|
||||
"example.dir": "/home/luca/distrobox",
|
||||
},
|
||||
}
|
||||
assert.False(t, c.IsDistrobox())
|
||||
}
|
||||
|
||||
// A foreign manager label combined with a distrobox-suffixed value must not
|
||||
// trigger detection on the value side either. The container has to actually
|
||||
// carry a distrobox.* key (or manager=distrobox) to be ours.
|
||||
func TestContainer_IsDistrobox_ForeignManagerWithDistroboxValue(t *testing.T) {
|
||||
c := containermanager.Container{
|
||||
Labels: map[string]string{
|
||||
"manager": "compose",
|
||||
"com.example.image": "registry.opensuse.org/opensuse/distrobox",
|
||||
},
|
||||
}
|
||||
assert.False(t, c.IsDistrobox())
|
||||
}
|
||||
|
||||
// Keys that merely contain the substring "distrobox" but don't use the
|
||||
// reserved distrobox. namespace (e.g. another project's labels) must not
|
||||
// be treated as ours. Matches the docs at pkg/containermanager/providers
|
||||
// where create always sets distrobox.<something>=… keys.
|
||||
func TestContainer_IsDistrobox_UnrelatedKeyContainingSubstring(t *testing.T) {
|
||||
c := containermanager.Container{
|
||||
Labels: map[string]string{
|
||||
"my-distrobox-thing": "1",
|
||||
},
|
||||
}
|
||||
assert.False(t, c.IsDistrobox())
|
||||
}
|
||||
|
||||
// The manager-label override case (24b31ed8): the user supplied
|
||||
// --additional-flags --label=manager=apx, so manager!=distrobox, but the
|
||||
// distrobox.unshare_groups label is still set by the create path and is
|
||||
// enough to identify the container.
|
||||
func TestContainer_IsDistrobox_DistroboxKeyPrefix(t *testing.T) {
|
||||
c := containermanager.Container{
|
||||
Labels: map[string]string{
|
||||
"distrobox.unshare_groups": "0",
|
||||
},
|
||||
}
|
||||
assert.True(t, c.IsDistrobox())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user