From ca8e9b5193c64cad70febd42b6a6d44cda683645 Mon Sep 17 00:00:00 2001 From: Yingrong Zhao Date: Wed, 25 Jan 2023 11:34:13 -0500 Subject: [PATCH] check for plugins file schema version and type (#2532) * check for plugins file schema version and type Signed-off-by: Yingrong Zhao --- docs/content/blog/install-multiple-plugins.md | 48 +++++++++++++ docs/content/reference/file-formats.md | 19 ++--- pkg/plugins/install.go | 52 ++++++++++---- pkg/plugins/install_test.go | 4 +- pkg/porter/plugins.go | 23 +++--- pkg/porter/plugins_test.go | 63 +++++++++++++++++ pkg/porter/testdata/invalid-plugins.json | 13 ++++ pkg/porter/testdata/plugins.json | 14 ++-- pkg/porter/testdata/plugins.yaml | 11 +-- pkg/schema/plugins.schema.json | 70 ++++++++++--------- tests/smoke/hello_test.go | 7 +- tests/smoke/testdata/plugins.yaml | 4 ++ 12 files changed, 252 insertions(+), 76 deletions(-) create mode 100644 docs/content/blog/install-multiple-plugins.md create mode 100644 pkg/porter/testdata/invalid-plugins.json create mode 100644 tests/smoke/testdata/plugins.yaml diff --git a/docs/content/blog/install-multiple-plugins.md b/docs/content/blog/install-multiple-plugins.md new file mode 100644 index 0000000000..d3cbfdfdd1 --- /dev/null +++ b/docs/content/blog/install-multiple-plugins.md @@ -0,0 +1,48 @@ + +--- +title: "Quickly set up a Porter environment with required plugins" +description: "How to install multiple plugins with Porter" +date: "2023-01-24" +authorname: "Yingrong Zhao" +author: "@vinozzz" +authorlink: "https://github.com/vinozzz" +authorimage: "https://github.com/vinozzz.png" +tags: ["best-practice", "plugins"] +summary: | + Setting up your Porter environment with your required plugins using the new `--file` flag with `porter plugins install` command. +--- + +### Breaking change +The recent porter v1.0.5 release introduced a new flag `--file` on `porter plugins install` command. Its intention is to allow users to install multiple plugins through a plugins definition file with a single porter command. However, it did not work as expected due to bad file format. + +The fix that contains the correct schema has been published with a new v1.0.6 release. If you have an existing plugins file, please update it to work with v1.0.6+. + +### Install multiple plugins with a single command +Now, you can install multiple plugins using a plugin definition yaml file like below: +```yaml +schemaType: Plugins +schemaVersion: 1.0.0 +plugins: + azure: + version: v1.0.1 + kubernetes: + version: v1.0.1 +``` + +After creating the file, you can run the command: +```bash +porter plugins install -f +``` + +The output from the command should look like this: +``` +installed azure plugin v1.0.1 (e361abc) +installed kubernetes plugin v1.0.1 (f01c944) +``` + +Make sure to update your current plugins schema file to the [latest format](/reference/file-formats/#plugins) +Please [let us know][contact] how the change went (good or bad), and we are happy to help if you have questions, or you would like help with your migration. + +[announced]: https://github.com/docker/roadmap/issues/209 +[Install Porter]: /install/ +[contact]: /community/ \ No newline at end of file diff --git a/docs/content/reference/file-formats.md b/docs/content/reference/file-formats.md index 807f4e4a22..5a2dec5543 100644 --- a/docs/content/reference/file-formats.md +++ b/docs/content/reference/file-formats.md @@ -162,21 +162,22 @@ You can use this [json schema][plugins-schema] to validate a plugins config file ```yaml schemaType: Plugins schemaVersion: 1.0.0 -azure: - version: v1.0.0 - feedURL: https://cdn.porter.sh/plugins/atom.xml - url: https://example.com - mirror: https://example.com +plugins: + azure: + version: v1.0.0 + feedURL: https://cdn.porter.sh/plugins/atom.xml + url: https://example.com + mirror: https://example.com ``` | Field | Required | Description | |----------------------|----------|------------------------------------------------------------------------------------------------------------------------------------------------| | schemaType | false | The type of document. This isn't used by Porter but is included when Porter outputs the file, so that editors can determine the resource type. | | schemaVersion | true | The version of the Plugins schema used in this file. | -| .version | false | The version of the plugin. | -| .feedURL | false | The url of an atom feed where the plugin can be downloaded. -| .url | false | The url from where the plugin can be downloaded. | -| .mirror | false | The mirror of official Porter assets. | +| plugins..version | false | The version of the plugin. | +| plugins..feedURL | false | The url of an atom feed where the plugin can be downloaded. +| plugins..url | false | The url from where the plugin can be downloaded. | +| plugins..mirror | false | The mirror of official Porter assets. | [cs-schema]: /schema/v1/credential-set.schema.json [ps-schema]: /schema/v1/parameter-set.schema.json diff --git a/pkg/plugins/install.go b/pkg/plugins/install.go index 158e4325db..f1907cd5ba 100644 --- a/pkg/plugins/install.go +++ b/pkg/plugins/install.go @@ -3,11 +3,17 @@ package plugins import ( "fmt" "sort" + "strings" "get.porter.sh/porter/pkg/pkgmgmt" "get.porter.sh/porter/pkg/portercontext" + "github.com/cnabio/cnab-go/schema" ) +// InstallPluginsSchemaVersion represents the version associated with the schema +// plugins configuration documents. +var InstallPluginsSchemaVersion = schema.Version("1.0.0") + type InstallOptions struct { pkgmgmt.InstallOptions @@ -25,8 +31,9 @@ func (o *InstallOptions) Validate(args []string, cxt *portercontext.Context) err return fmt.Errorf("plugin URL should not be specified when --file is provided") } - if o.Version != "" { - return fmt.Errorf("plugin version should not be specified when --file is provided") + // version should not be set to anything other than the default value + if o.Version != "" && o.Version != "latest" { + return fmt.Errorf("plugin version %s should not be specified when --file is provided", o.Version) } if _, err := cxt.FileSystem.Stat(o.File); err != nil { @@ -39,20 +46,41 @@ func (o *InstallOptions) Validate(args []string, cxt *portercontext.Context) err return o.InstallOptions.Validate(args) } -// InstallFileOption is the go representation of plugin installation file format. -type InstallFileOption map[string]pkgmgmt.InstallOptions +// InstallPluginsSpec represents the user-defined configuration for plugins installation. +type InstallPluginsSpec struct { + SchemaType string `yaml:"schemaType"` + SchemaVersion string `yaml:"schemaVersion"` + Plugins InstallPluginsConfig `yaml:"plugins"` +} + +func (spec InstallPluginsSpec) Validate() error { + if spec.SchemaType != "" && strings.ToLower(spec.SchemaType) != "plugins" { + return fmt.Errorf("invalid schemaType %s, expected Plugins", spec.SchemaType) + } + + if InstallPluginsSchemaVersion != schema.Version(spec.SchemaVersion) { + if spec.SchemaVersion == "" { + spec.SchemaVersion = "(none)" + } + return fmt.Errorf("invalid schemaVersion provided: %s. This version of Porter is compatible with %s.", spec.SchemaVersion, InstallPluginsSchemaVersion) + } + return nil +} + +// InstallPluginsConfig is the go representation of plugin installation file format. +type InstallPluginsConfig map[string]pkgmgmt.InstallOptions -// InstallPluginsConfig is a sorted list of InstallationFileOption in alphabetical order. -type InstallPluginsConfig struct { - data InstallFileOption +// InstallPluginsConfigList is a sorted list of InstallationFileOption in alphabetical order. +type InstallPluginsConfigList struct { + data InstallPluginsConfig keys []string } // NewInstallPluginConfigs returns a new instance of InstallPluginConfigs with plugins sorted in alphabetical order // using their names. -func NewInstallPluginConfigs(opt InstallFileOption) InstallPluginsConfig { +func NewInstallPluginConfigs(opt InstallPluginsConfig) InstallPluginsConfigList { keys := make([]string, 0, len(opt)) - data := make(InstallFileOption, len(opt)) + data := make(InstallPluginsConfig, len(opt)) for k, v := range opt { keys = append(keys, k) @@ -65,14 +93,14 @@ func NewInstallPluginConfigs(opt InstallFileOption) InstallPluginsConfig { return keys[i] < keys[j] }) - return InstallPluginsConfig{ + return InstallPluginsConfigList{ data: data, keys: keys, } } -// Configs returns InstallOptions list in alphabetical order. -func (pc InstallPluginsConfig) Configs() []pkgmgmt.InstallOptions { +// Values returns InstallOptions list in alphabetical order. +func (pc InstallPluginsConfigList) Values() []pkgmgmt.InstallOptions { value := make([]pkgmgmt.InstallOptions, 0, len(pc.keys)) for _, k := range pc.keys { value = append(value, pc.data[k]) diff --git a/pkg/plugins/install_test.go b/pkg/plugins/install_test.go index 40b7393895..8e0bba3420 100644 --- a/pkg/plugins/install_test.go +++ b/pkg/plugins/install_test.go @@ -19,9 +19,9 @@ func TestInstallOptions_Validate(t *testing.T) { } func TestInstallPluginsConfig(t *testing.T) { - input := InstallFileOption{"kubernetes": pkgmgmt.InstallOptions{URL: "test-kubernetes.com"}, "azure": pkgmgmt.InstallOptions{URL: "test-azure.com"}} + input := InstallPluginsConfig{"kubernetes": pkgmgmt.InstallOptions{URL: "test-kubernetes.com"}, "azure": pkgmgmt.InstallOptions{URL: "test-azure.com"}} expected := []pkgmgmt.InstallOptions{{Name: "azure", PackageType: "plugin", URL: "test-azure.com"}, {Name: "kubernetes", PackageType: "plugin", URL: "test-kubernetes.com"}} cfg := NewInstallPluginConfigs(input) - require.Equal(t, expected, cfg.Configs()) + require.Equal(t, expected, cfg.Values()) } diff --git a/pkg/porter/plugins.go b/pkg/porter/plugins.go index 7ff010d77c..c39fc37691 100644 --- a/pkg/porter/plugins.go +++ b/pkg/porter/plugins.go @@ -158,23 +158,23 @@ func (p *Porter) InstallPlugin(ctx context.Context, opts plugins.InstallOptions) ctx, log := tracing.StartSpan(ctx) defer log.EndSpan() - installConfigs, err := p.getPluginInstallConfigs(ctx, opts) + installOpts, err := p.getPluginInstallOptions(ctx, opts) if err != nil { return err } - for _, cfg := range installConfigs { - err := p.Plugins.Install(ctx, cfg) + for _, opt := range installOpts { + err := p.Plugins.Install(ctx, opt) if err != nil { return err } - plugin, err := p.Plugins.GetMetadata(ctx, cfg.Name) + plugin, err := p.Plugins.GetMetadata(ctx, opt.Name) if err != nil { return fmt.Errorf("failed to get plugin metadata: %w", err) } v := plugin.GetVersionInfo() - fmt.Fprintf(p.Out, "installed %s plugin %s (%s)\n", cfg.Name, v.Version, v.Commit) + fmt.Fprintf(p.Out, "installed %s plugin %s (%s)\n", opt.Name, v.Version, v.Commit) } return nil @@ -191,13 +191,13 @@ func (p *Porter) UninstallPlugin(ctx context.Context, opts pkgmgmt.UninstallOpti return nil } -func (p *Porter) getPluginInstallConfigs(ctx context.Context, opts plugins.InstallOptions) ([]pkgmgmt.InstallOptions, error) { +func (p *Porter) getPluginInstallOptions(ctx context.Context, opts plugins.InstallOptions) ([]pkgmgmt.InstallOptions, error) { _, log := tracing.StartSpan(ctx) defer log.EndSpan() var installConfigs []pkgmgmt.InstallOptions if opts.File != "" { - var data plugins.InstallFileOption + var data plugins.InstallPluginsSpec if log.ShouldLog(zapcore.DebugLevel) { // ignoring any error here, printing debug info isn't critical contents, _ := p.FileSystem.ReadFile(opts.File) @@ -207,9 +207,14 @@ func (p *Porter) getPluginInstallConfigs(ctx context.Context, opts plugins.Insta if err := encoding.UnmarshalFile(p.FileSystem, opts.File, &data); err != nil { return nil, fmt.Errorf("unable to parse %s as an installation document: %w", opts.File, err) } - sortedCfgs := plugins.NewInstallPluginConfigs(data) - for _, config := range sortedCfgs.Configs() { + if err := data.Validate(); err != nil { + return nil, err + } + + sortedCfgs := plugins.NewInstallPluginConfigs(data.Plugins) + + for _, config := range sortedCfgs.Values() { // if user specified a feed url or mirror using the flags, it will become // the default value and apply to empty values parsed from the provided file if config.FeedURL == "" { diff --git a/pkg/porter/plugins_test.go b/pkg/porter/plugins_test.go index 0bf124e343..e4d6b64208 100644 --- a/pkg/porter/plugins_test.go +++ b/pkg/porter/plugins_test.go @@ -3,6 +3,9 @@ package porter import ( "context" "fmt" + "os" + "path/filepath" + "strings" "testing" "get.porter.sh/porter/pkg/pkgmgmt" @@ -10,8 +13,11 @@ import ( "get.porter.sh/porter/pkg/plugins" "get.porter.sh/porter/pkg/printer" "get.porter.sh/porter/pkg/test" + "get.porter.sh/porter/pkg/yaml" + "get.porter.sh/porter/tests" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/xeipuuv/gojsonschema" ) func TestPorter_PrintPlugins(t *testing.T) { @@ -310,6 +316,63 @@ func TestPorter_InstallPlugin(t *testing.T) { } } +func TestPorter_InstallPluginsSchema(t *testing.T) { + p := NewTestPorter(t) + schema, err := os.ReadFile(filepath.Join(p.RepoRoot, "pkg/schema/plugins.schema.json")) + require.NoError(t, err, "failed to read plugins.schema.json file") + testcases := []struct { + name string + path string + wantErr string + }{ + { + name: "valid", + path: "testdata/plugins.json", + wantErr: "", + }, + { + name: "invalid", + path: "testdata/invalid-plugins.json", + wantErr: "(root): Additional property invalid-field is not allowed\nplugins.plugin1: Additional property random-field is not allowed\n", + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + // Load manifest as a go dump + testManifest, err := os.ReadFile(tc.path) + require.NoError(t, err, "failed to read %s", tc.path) + + m := make(map[string]interface{}) + err = yaml.Unmarshal(testManifest, &m) + require.NoError(t, err, "failed to unmarshal %s", tc.path) + + // Load the manifest schema returned from `porter schema` + manifestLoader := gojsonschema.NewGoLoader(m) + schemaLoader := gojsonschema.NewBytesLoader(schema) + + // Validate the manifest against the schema + fails, err := gojsonschema.Validate(schemaLoader, manifestLoader) + require.NoError(t, err) + + if tc.wantErr == "" { + assert.Empty(t, fails.Errors(), "expected %s to validate against the plugins schema", tc.path) + // Print any validation errors returned + for _, err := range fails.Errors() { + t.Logf("%s", err) + } + } else { + var allFails strings.Builder + for _, err := range fails.Errors() { + allFails.WriteString(err.String()) + allFails.WriteString("\n") + } + tests.RequireOutputContains(t, tc.wantErr, allFails.String()) + } + }) + } +} + func TestPorter_UninstallPlugin(t *testing.T) { ctx := context.Background() p := NewTestPorter(t) diff --git a/pkg/porter/testdata/invalid-plugins.json b/pkg/porter/testdata/invalid-plugins.json new file mode 100644 index 0000000000..96abe84e1e --- /dev/null +++ b/pkg/porter/testdata/invalid-plugins.json @@ -0,0 +1,13 @@ +{ + "schemaType": "Plugins", + "schemaVersion": "1.0.0", + "invalid-field": "123", + "plugins": { + "plugin1": { + "random-field": 1 + }, + "plugin2": { + "version": "v1.0" + } + } +} \ No newline at end of file diff --git a/pkg/porter/testdata/plugins.json b/pkg/porter/testdata/plugins.json index df8ba43813..b3b6e57822 100644 --- a/pkg/porter/testdata/plugins.json +++ b/pkg/porter/testdata/plugins.json @@ -1,8 +1,12 @@ { - "plugin1": { - "version": "v1.0" - }, - "plugin2": { - "version": "v1.0" + "schemaType": "Plugins", + "schemaVersion": "1.0.0", + "plugins": { + "plugin1": { + "version": "v1.0" + }, + "plugin2": { + "version": "v1.0" + } } } \ No newline at end of file diff --git a/pkg/porter/testdata/plugins.yaml b/pkg/porter/testdata/plugins.yaml index 770d12cd99..a53df058c8 100644 --- a/pkg/porter/testdata/plugins.yaml +++ b/pkg/porter/testdata/plugins.yaml @@ -1,4 +1,7 @@ -plugin1: - version: v1.0 -plugin2: - version: v1.0 \ No newline at end of file +schemaType: Plugins +schemaVersion: 1.0.0 +plugins: + plugin1: + version: v1.0 + plugin2: + version: v1.0 \ No newline at end of file diff --git a/pkg/schema/plugins.schema.json b/pkg/schema/plugins.schema.json index b6d55efeaa..fce721bf48 100644 --- a/pkg/schema/plugins.schema.json +++ b/pkg/schema/plugins.schema.json @@ -1,6 +1,30 @@ { "$id": "https://getporter.org/schema/v1/plugins.schema.json", "$schema": "http://json-schema.org/draft-07/schema#", + "definitions": { + "plugins": { + "type": "object", + "properties": { + "version": { + "description": "The version for the plugin. Defaults to latest when unspecified.", + "type": "string" + }, + "feedURL": { + "description": "The URL of an atom feed where the plugin can be downloaded. Defaults to the official Porter plugin feed.", + "type": "string" + }, + "url": { + "description": "The URL from where the plugin can be downloaded. For example, https://github.com/MChorfa/porter-helm3/releases/download", + "type": "string" + }, + "mirror": { + "description": "Mirror of official Porter assets.", + "type": "string" + } + }, + "additionalProperties": false + } + }, "properties": { "schemaType": { "description": "The resource type of the current document.", @@ -9,43 +33,21 @@ }, "schemaVersion": { "description": "Version of the plugins schema to which this document adheres", - "type": "string", - "default": "1.0.0" + "type": "string" + }, + "plugins": { + "description": "A map of plugins to install, keyed by the plugin name.", + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/plugins" + } } }, + "additionalProperties": false, "required": [ - "schemaVersion" + "schemaVersion", + "plugins" ], "title": "Plugins json schema", - "type": "object", - "additionalProperties": { - "type": "object", - "properties": { - "key": { - "type": "string" - }, - "value": { - "type": "object", - "properties": { - "version": { - "description": "The version for the plugins.", - "type": "string" - }, - "feedURL": { - "description": "The URL of an atom feed where the plugin can be downloaded.", - "type": "string" - }, - "url": { - "description": "The URL from where the plugin can be downloaded", - "type": "string" - }, - "mirror": { - "description": "Mirror of official Porter assets.", - "type": "string" - }, - "additionalProperties": false - } - } - } - } + "type": "object" } \ No newline at end of file diff --git a/tests/smoke/hello_test.go b/tests/smoke/hello_test.go index 69ce3fd777..3284318965 100644 --- a/tests/smoke/hello_test.go +++ b/tests/smoke/hello_test.go @@ -22,10 +22,15 @@ func TestHelloBundle(t *testing.T) { test.PrepareTestBundle() require.NoError(t, shx.Copy("testdata/buncfg.json", test.TestDir)) + require.NoError(t, shx.Copy("testdata/plugins.yaml", test.TestDir)) test.Chdir(test.TestDir) + // Verify plugins installation + _, output := test.RequirePorter("plugins", "install", "-f", "plugins.yaml") + require.Contains(t, output, "installed azure plugin", "expected to see plugin successfully installed") + // Run a stateless action before we install and make sure nothing is persisted - _, output := test.RequirePorter("invoke", testdata.MyBuns, "--action=dry-run", "--reference", testdata.MyBunsRef, "-c=mybuns") + _, output = test.RequirePorter("invoke", testdata.MyBuns, "--action=dry-run", "--reference", testdata.MyBunsRef, "-c=mybuns") t.Log(output) test.RequireInstallationNotFound(test.CurrentNamespace(), testdata.MyBuns) diff --git a/tests/smoke/testdata/plugins.yaml b/tests/smoke/testdata/plugins.yaml new file mode 100644 index 0000000000..d0d7d719e9 --- /dev/null +++ b/tests/smoke/testdata/plugins.yaml @@ -0,0 +1,4 @@ +schemaType: Plugins +schemaVersion: 1.0.0 +plugins: + azure: {} \ No newline at end of file