From dfbac70efab64cdff81a98719e2a99353c275782 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 3 Aug 2025 17:50:16 +0200 Subject: [PATCH 1/2] remove some remnants from CLI "experimental" config option Experimental is always enabled (977d3ae046ec6c64be8788a8712251ed547a2bdb), and the `Experimental` field in plugin metadata was deprecated in 977d3ae046ec6c64be8788a8712251ed547a2bdb and removed in commit 6a50c4f70054cf6e60124d911e4ca8754617e21d. Signed-off-by: Sebastiaan van Stijn --- cli-plugins/manager/candidate_test.go | 10 +++++----- e2e/internal/fixtures/fixtures.go | 3 +-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/cli-plugins/manager/candidate_test.go b/cli-plugins/manager/candidate_test.go index e8df42909..b54f6a967 100644 --- a/cli-plugins/manager/candidate_test.go +++ b/cli-plugins/manager/candidate_test.go @@ -36,10 +36,9 @@ func TestValidateCandidate(t *testing.T) { builtinName = metadata.NamePrefix + "builtin" builtinAlias = metadata.NamePrefix + "alias" - badPrefixPath = "/usr/local/libexec/cli-plugins/wobble" - badNamePath = "/usr/local/libexec/cli-plugins/docker-123456" - goodPluginPath = "/usr/local/libexec/cli-plugins/" + goodPluginName - metaExperimental = `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing", "Experimental": true}` + badPrefixPath = "/usr/local/libexec/cli-plugins/wobble" + badNamePath = "/usr/local/libexec/cli-plugins/docker-123456" + goodPluginPath = "/usr/local/libexec/cli-plugins/" + goodPluginName ) fakeroot := &cobra.Command{Use: "docker"} @@ -72,7 +71,8 @@ func TestValidateCandidate(t *testing.T) { {name: "empty vendor", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": ""}`}, invalid: "plugin metadata does not define a vendor"}, // This one should work {name: "valid", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing"}`}}, - {name: "experimental + allowing experimental", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: metaExperimental}}, + // Including the deprecated "experimental" field should not break processing. + {name: "with legacy experimental", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing", "Experimental": true}`}}, } { t.Run(tc.name, func(t *testing.T) { p, err := newPlugin(tc.c, fakeroot.Commands()) diff --git a/e2e/internal/fixtures/fixtures.go b/e2e/internal/fixtures/fixtures.go index f6c49f50c..3c47f8d7f 100644 --- a/e2e/internal/fixtures/fixtures.go +++ b/e2e/internal/fixtures/fixtures.go @@ -44,8 +44,7 @@ func SetupConfigWithNotaryURL(t *testing.T, path, notaryURL string) fs.Dir { "%s": { "auth": "ZWlhaXM6cGFzc3dvcmQK" } - }, - "experimental": "enabled" + } } `, notaryURL)), fs.WithDir("trust", fs.WithDir("private"))) return *dir From 057f3128b6ebd97c2b51b83cafd1ffec565c311d Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 4 Aug 2025 08:50:07 +0200 Subject: [PATCH 2/2] cli-plugins/manager: reformat TestValidateCandidate table Slightly more verbose, but makes it easier to see properties of each test. Signed-off-by: Sebastiaan van Stijn --- cli-plugins/manager/candidate_test.go | 94 ++++++++++++++++++++------- 1 file changed, 72 insertions(+), 22 deletions(-) diff --git a/cli-plugins/manager/candidate_test.go b/cli-plugins/manager/candidate_test.go index b54f6a967..ee7ff788b 100644 --- a/cli-plugins/manager/candidate_test.go +++ b/cli-plugins/manager/candidate_test.go @@ -32,9 +32,8 @@ func (c *fakeCandidate) Metadata() ([]byte, error) { func TestValidateCandidate(t *testing.T) { const ( goodPluginName = metadata.NamePrefix + "goodplugin" - - builtinName = metadata.NamePrefix + "builtin" - builtinAlias = metadata.NamePrefix + "alias" + builtinName = metadata.NamePrefix + "builtin" + builtinAlias = metadata.NamePrefix + "alias" badPrefixPath = "/usr/local/libexec/cli-plugins/wobble" badNamePath = "/usr/local/libexec/cli-plugins/docker-123456" @@ -50,32 +49,83 @@ func TestValidateCandidate(t *testing.T) { }) for _, tc := range []struct { - name string - c *fakeCandidate + name string + plugin *fakeCandidate // Either err or invalid may be non-empty, but not both (both can be empty for a good plugin). err string invalid string }{ - /* Each failing one of the tests */ - {name: "empty path", c: &fakeCandidate{path: ""}, err: "plugin candidate path cannot be empty"}, - {name: "bad prefix", c: &fakeCandidate{path: badPrefixPath}, err: fmt.Sprintf("does not have %q prefix", metadata.NamePrefix)}, - {name: "bad path", c: &fakeCandidate{path: badNamePath}, invalid: "did not match"}, - {name: "builtin command", c: &fakeCandidate{path: builtinName}, invalid: `plugin "builtin" duplicates builtin command`}, - {name: "builtin alias", c: &fakeCandidate{path: builtinAlias}, invalid: `plugin "alias" duplicates an alias of builtin command "builtin"`}, - {name: "fetch failure", c: &fakeCandidate{path: goodPluginPath, exec: false}, invalid: fmt.Sprintf("failed to fetch metadata: faked a failure to exec %q", goodPluginPath)}, - {name: "metadata not json", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `xyzzy`}, invalid: "invalid character"}, - {name: "empty schemaversion", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{}`}, invalid: `plugin SchemaVersion "" is not valid`}, - {name: "invalid schemaversion", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "xyzzy"}`}, invalid: `plugin SchemaVersion "xyzzy" is not valid`}, - {name: "no vendor", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0"}`}, invalid: "plugin metadata does not define a vendor"}, - {name: "empty vendor", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": ""}`}, invalid: "plugin metadata does not define a vendor"}, - // This one should work - {name: "valid", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing"}`}}, - // Including the deprecated "experimental" field should not break processing. - {name: "with legacy experimental", c: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing", "Experimental": true}`}}, + // Invalid cases. + { + name: "empty path", + plugin: &fakeCandidate{path: ""}, + err: "plugin candidate path cannot be empty", + }, + { + name: "bad prefix", + plugin: &fakeCandidate{path: badPrefixPath}, + err: fmt.Sprintf("does not have %q prefix", metadata.NamePrefix), + }, + { + name: "bad path", + plugin: &fakeCandidate{path: badNamePath}, + invalid: "did not match", + }, + { + name: "builtin command", + plugin: &fakeCandidate{path: builtinName}, + invalid: `plugin "builtin" duplicates builtin command`, + }, + { + name: "builtin alias", + plugin: &fakeCandidate{path: builtinAlias}, + invalid: `plugin "alias" duplicates an alias of builtin command "builtin"`, + }, + { + name: "fetch failure", + plugin: &fakeCandidate{path: goodPluginPath, exec: false}, + invalid: fmt.Sprintf("failed to fetch metadata: faked a failure to exec %q", goodPluginPath), + }, + { + name: "metadata not json", + plugin: &fakeCandidate{path: goodPluginPath, exec: true, meta: `xyzzy`}, + invalid: "invalid character", + }, + { + name: "empty schemaversion", + plugin: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{}`}, + invalid: `plugin SchemaVersion "" is not valid`, + }, + { + name: "invalid schemaversion", + plugin: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "xyzzy"}`}, + invalid: `plugin SchemaVersion "xyzzy" is not valid`, + }, + { + name: "no vendor", + plugin: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0"}`}, + invalid: "plugin metadata does not define a vendor", + }, + { + name: "empty vendor", + plugin: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": ""}`}, + invalid: "plugin metadata does not define a vendor", + }, + + // Valid cases. + { + name: "valid", + plugin: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing"}`}, + }, + { + // Including the deprecated "experimental" field should not break processing. + name: "with legacy experimental", + plugin: &fakeCandidate{path: goodPluginPath, exec: true, meta: `{"SchemaVersion": "0.1.0", "Vendor": "e2e-testing", "Experimental": true}`}, + }, } { t.Run(tc.name, func(t *testing.T) { - p, err := newPlugin(tc.c, fakeroot.Commands()) + p, err := newPlugin(tc.plugin, fakeroot.Commands()) switch { case tc.err != "": assert.ErrorContains(t, err, tc.err)