Merge pull request #31146 from helm/plugin-types-apiversion-v1

[HIP-0026] Plugin types and plugin apiVersion v1

internal/plugin/config.go

@@ -17,6 +17,8 @@ package plugin
 
 import (
 	"fmt"
+
+	"go.yaml.in/yaml/v3"
 )
 
 // Config interface defines the methods that all plugin type configurations must implement
@@ -64,3 +66,17 @@ func (c *ConfigGetter) Validate() error {
 	}
 	return nil
 }
+
+func remarshalConfig[T Config](configData map[string]any) (Config, error) {
+	data, err := yaml.Marshal(configData)
+	if err != nil {
+		return nil, err
+	}
+
+	var config T
+	if err := yaml.Unmarshal(data, &config); err != nil {
+		return nil, err
+	}
+
+	return config, nil
+}

internal/plugin/doc.go

@@ -55,7 +55,7 @@ Helm plugins are exposed to uses as the "Plugin" type, the basic interface that
 Internally, plugins must be implemented by a "runtime" that is responsible for creating the plugin instance, and dispatching the plugin's invocation to the plugin's implementation.
 For example:
 - forming environment variables and command line args for subprocess execution
-- converting input to JSON and invoking a function in a future runtime (eg, Wasm)
+- converting input to JSON and invoking a function in a Wasm runtime
 
 Internally, the code structure is:
 Runtime.CreatePlugin()
@@ -78,7 +78,7 @@ Each plugin must have a `plugin.yaml`, that defines the plugin's metadata. The m
 
 For legacy plugins, the type is inferred by which fields are set on the plugin: a downloader plugin is inferred when metadata contains a "downloaders" yaml node, otherwise it is assumed to define a Helm CLI subcommand.
 
-For future plugin api versions, the metadata will include explicit apiVersion and type fields. It will also contain type and runtime specific Config and RuntimeConfig fields.
+For v1 plugins, the metadata includes explicit apiVersion and type fields. It will also contain type-specific Config, and RuntimeConfig fields.
 
 # Runtime and type cardinality
 From a cardinality perspective, this means there a "few" runtimes, and "many" plugins types. It is also expected that the subprocess runtime will not be extended to support extra plugin types, and deprecated in a future version of Helm.

internal/plugin/installer/local_installer_test.go

@@ -34,7 +34,7 @@ func TestLocalInstaller(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	source := "../testdata/plugdir/good/echo-legacy"
+	source := "../testdata/plugdir/good/echo-v1"
 	i, err := NewForSource(source, "")
 	if err != nil {
 		t.Fatalf("unexpected error: %s", err)
@@ -44,14 +44,14 @@ func TestLocalInstaller(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if i.Path() != helmpath.DataPath("plugins", "echo-legacy") {
+	if i.Path() != helmpath.DataPath("plugins", "echo-v1") {
 		t.Fatalf("expected path '$XDG_CONFIG_HOME/helm/plugins/helm-env', got %q", i.Path())
 	}
 	defer os.RemoveAll(filepath.Dir(helmpath.DataPath())) // helmpath.DataPath is like /tmp/helm013130971/helm
 }
 
 func TestLocalInstallerNotAFolder(t *testing.T) {
-	source := "../testdata/plugdir/good/echo-legacy/plugin.yaml"
+	source := "../testdata/plugdir/good/echo-v1/plugin.yaml"
 	i, err := NewForSource(source, "")
 	if err != nil {
 		t.Fatalf("unexpected error: %s", err)

internal/plugin/installer/vcs_installer_test.go

@@ -57,7 +57,7 @@ func TestVCSInstaller(t *testing.T) {
 	}
 
 	source := "https://github.com/adamreese/helm-env"
-	testRepoPath, _ := filepath.Abs("../testdata/plugdir/good/echo-legacy")
+	testRepoPath, _ := filepath.Abs("../testdata/plugdir/good/echo-v1")
 	repo := &testRepo{
 		local: testRepoPath,
 		tags:  []string{"0.1.0", "0.1.1"},

internal/plugin/loader.go

@@ -58,6 +58,29 @@ func loadMetadataLegacy(metadataData []byte) (*Metadata, error) {
 	return m, nil
 }
 
+func loadMetadataV1(metadataData []byte) (*Metadata, error) {
+
+	var mv1 MetadataV1
+	d := yaml.NewDecoder(bytes.NewReader(metadataData))
+	if err := d.Decode(&mv1); err != nil {
+		return nil, err
+	}
+
+	if err := mv1.Validate(); err != nil {
+		return nil, err
+	}
+
+	m, err := fromMetadataV1(mv1)
+	if err != nil {
+		return nil, fmt.Errorf("failed to convert MetadataV1 to Metadata: %w", err)
+	}
+
+	if err := m.Validate(); err != nil {
+		return nil, err
+	}
+	return m, nil
+}
+
 func loadMetadata(metadataData []byte) (*Metadata, error) {
 	apiVersion, err := peekAPIVersion(bytes.NewReader(metadataData))
 	if err != nil {
@@ -67,6 +90,8 @@ func loadMetadata(metadataData []byte) (*Metadata, error) {
 	switch apiVersion {
 	case "": // legacy
 		return loadMetadataLegacy(metadataData)
+	case "v1":
+		return loadMetadataV1(metadataData)
 	}
 
 	return nil, fmt.Errorf("invalid plugin apiVersion: %q", apiVersion)

internal/plugin/loader_test.go

@@ -29,6 +29,13 @@ func TestPeekAPIVersion(t *testing.T) {
 		data     []byte
 		expected string
 	}{
+		"v1": {
+			data: []byte(`---
+apiVersion: v1
+name: "test-plugin"
+`),
+			expected: "v1",
+		},
 		"legacy": { // No apiVersion field
 			data: []byte(`---
 name: "test-plugin"
@@ -97,6 +104,11 @@ func TestLoadDir(t *testing.T) {
 			apiVersion: "legacy",
 			expect:     makeMetadata("legacy"),
 		},
+		"v1": {
+			dirname:    "testdata/plugdir/good/hello-v1",
+			apiVersion: "v1",
+			expect:     makeMetadata("v1"),
+		},
 	}
 
 	for name, tc := range testCases {
@@ -113,6 +125,7 @@ func TestLoadDir(t *testing.T) {
 func TestLoadDirDuplicateEntries(t *testing.T) {
 	testCases := map[string]string{
 		"legacy": "testdata/plugdir/bad/duplicate-entries-legacy",
+		"v1":     "testdata/plugdir/bad/duplicate-entries-v1",
 	}
 	for name, dirname := range testCases {
 		t.Run(name, func(t *testing.T) {
@@ -122,6 +135,34 @@ func TestLoadDirDuplicateEntries(t *testing.T) {
 	}
 }
 
+func TestLoadDirGetter(t *testing.T) {
+	dirname := "testdata/plugdir/good/getter"
+
+	expect := Metadata{
+		Name:       "getter",
+		Version:    "1.2.3",
+		Type:       "getter/v1",
+		APIVersion: "v1",
+		Runtime:    "subprocess",
+		Config: &ConfigGetter{
+			Protocols: []string{"myprotocol", "myprotocols"},
+		},
+		RuntimeConfig: &RuntimeConfigSubprocess{
+			ProtocolCommands: []SubprocessProtocolCommand{
+				{
+					Protocols: []string{"myprotocol", "myprotocols"},
+					Command:   "echo getter",
+				},
+			},
+		},
+	}
+
+	plug, err := LoadDir(dirname)
+	require.NoError(t, err)
+	assert.Equal(t, dirname, plug.Dir())
+	assert.Equal(t, expect, plug.Metadata())
+}
+
 func TestDetectDuplicates(t *testing.T) {
 	plugs := []Plugin{
 		mockSubprocessCLIPlugin(t, "foo"),
@@ -154,10 +195,13 @@ func TestLoadAll(t *testing.T) {
 		plugsMap[p.Metadata().Name] = p
 	}
 
-	assert.Len(t, plugsMap, 3)
+	assert.Len(t, plugsMap, 6)
 	assert.Contains(t, plugsMap, "downloader")
 	assert.Contains(t, plugsMap, "echo-legacy")
+	assert.Contains(t, plugsMap, "echo-v1")
+	assert.Contains(t, plugsMap, "getter")
 	assert.Contains(t, plugsMap, "hello-legacy")
+	assert.Contains(t, plugsMap, "hello-v1")
 }
 
 func TestFindPlugins(t *testing.T) {
@@ -184,7 +228,7 @@ func TestFindPlugins(t *testing.T) {
 		{
 			name:     "normal",
 			plugdirs: "./testdata/plugdir/good",
-			expected: 3,
+			expected: 6,
 		},
 	}
 	for _, c := range cases {

internal/plugin/metadata.go

@@ -20,7 +20,7 @@ import (
 	"fmt"
 )
 
-// Metadata of a plugin, converted from the "on-disk" plugin.yaml
+// Metadata of a plugin, converted from the "on-disk" legacy or v1 plugin.yaml
 // Specifically, Config and RuntimeConfig are converted to their respective types based on the plugin type and runtime
 type Metadata struct {
 	// APIVersion specifies the plugin API version
@@ -153,3 +153,64 @@ func buildLegacyRuntimeConfig(m MetadataLegacy) RuntimeConfig {
 		ProtocolCommands: protocolCommands,
 	}
 }
+
+func fromMetadataV1(mv1 MetadataV1) (*Metadata, error) {
+
+	config, err := convertMetadataConfig(mv1.Type, mv1.Config)
+	if err != nil {
+		return nil, err
+	}
+
+	runtimeConfig, err := convertMetdataRuntimeConfig(mv1.Runtime, mv1.RuntimeConfig)
+	if err != nil {
+		return nil, err
+	}
+
+	return &Metadata{
+		APIVersion:    mv1.APIVersion,
+		Name:          mv1.Name,
+		Type:          mv1.Type,
+		Runtime:       mv1.Runtime,
+		Version:       mv1.Version,
+		SourceURL:     mv1.SourceURL,
+		Config:        config,
+		RuntimeConfig: runtimeConfig,
+	}, nil
+}
+
+func convertMetadataConfig(pluginType string, configRaw map[string]any) (Config, error) {
+	var err error
+	var config Config
+
+	switch pluginType {
+	case "cli/v1":
+		config, err = remarshalConfig[*ConfigCLI](configRaw)
+	case "getter/v1":
+		config, err = remarshalConfig[*ConfigGetter](configRaw)
+	default:
+		return nil, fmt.Errorf("unsupported plugin type: %s", pluginType)
+	}
+
+	if err != nil {
+		return nil, fmt.Errorf("failed to unmarshal config for %s plugin type: %w", pluginType, err)
+	}
+
+	return config, nil
+}
+
+func convertMetdataRuntimeConfig(runtimeType string, runtimeConfigRaw map[string]any) (RuntimeConfig, error) {
+	var runtimeConfig RuntimeConfig
+	var err error
+
+	switch runtimeType {
+	case "subprocess":
+		runtimeConfig, err = remarshalRuntimeConfig[*RuntimeConfigSubprocess](runtimeConfigRaw)
+	default:
+		return nil, fmt.Errorf("unsupported plugin runtime type: %q", runtimeType)
+	}
+
+	if err != nil {
+		return nil, fmt.Errorf("failed to unmarshal runtimeConfig for %s runtime: %w", runtimeType, err)
+	}
+	return runtimeConfig, nil
+}

internal/plugin/metadata_v1.go

@@ -0,0 +1,67 @@
+/*
+Copyright The Helm Authors.
+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 plugin
+
+import (
+	"fmt"
+)
+
+// MetadataV1 is the APIVersion V1 plugin.yaml format
+type MetadataV1 struct {
+	// APIVersion specifies the plugin API version
+	APIVersion string `yaml:"apiVersion"`
+
+	// Name is the name of the plugin
+	Name string `yaml:"name"`
+
+	// Type of plugin (eg, cli/v1, getter/v1)
+	Type string `yaml:"type"`
+
+	// Runtime specifies the runtime type (subprocess, wasm)
+	Runtime string `yaml:"runtime"`
+
+	// Version is a SemVer 2 version of the plugin.
+	Version string `yaml:"version"`
+
+	// SourceURL is the URL where this plugin can be found
+	SourceURL string `yaml:"sourceURL,omitempty"`
+
+	// Config contains the type-specific configuration for this plugin
+	Config map[string]any `yaml:"config"`
+
+	// RuntimeConfig contains the runtime-specific configuration
+	RuntimeConfig map[string]any `yaml:"runtimeConfig"`
+}
+
+func (m *MetadataV1) Validate() error {
+	if !validPluginName.MatchString(m.Name) {
+		return fmt.Errorf("invalid plugin `name`")
+	}
+
+	if m.APIVersion != "v1" {
+		return fmt.Errorf("invalid `apiVersion`: %q", m.APIVersion)
+	}
+
+	if m.Type == "" {
+		return fmt.Errorf("`type` missing")
+	}
+
+	if m.Runtime == "" {
+		return fmt.Errorf("`runtime` missing")
+	}
+
+	return nil
+}

internal/plugin/plugin_test.go

@@ -42,7 +42,7 @@ func mockSubprocessCLIPlugin(t *testing.T, pluginName string) *SubprocessPluginR
 			Name:       pluginName,
 			Version:    "v0.1.2",
 			Type:       "cli/v1",
-			APIVersion: "legacy",
+			APIVersion: "v1",
 			Runtime:    "subprocess",
 			Config: &ConfigCLI{
 				Usage:       "Mock plugin",

internal/plugin/runtime.go

@@ -15,6 +15,8 @@ limitations under the License.
 
 package plugin
 
+import "go.yaml.in/yaml/v3"
+
 // Runtime represents a plugin runtime (subprocess, extism, etc) ie. how a plugin should be executed
 // Runtime is responsible for instantiating plugins that implement the runtime
 // TODO: could call this something more like "PluginRuntimeCreator"?
@@ -31,3 +33,17 @@ type Runtime interface {
 type RuntimeConfig interface {
 	Validate() error
 }
+
+func remarshalRuntimeConfig[T RuntimeConfig](runtimeData map[string]any) (RuntimeConfig, error) {
+	data, err := yaml.Marshal(runtimeData)
+	if err != nil {
+		return nil, err
+	}
+
+	var config T
+	if err := yaml.Unmarshal(data, &config); err != nil {
+		return nil, err
+	}
+
+	return config, nil
+}

internal/plugin/subprocess_commands_test.go

@@ -86,8 +86,6 @@ func TestPrepareCommandExtraArgs(t *testing.T) {
 
 	for name, tc := range testCases {
 		t.Run(name, func(t *testing.T) {
-			//expectedArgs := append(cmdArgs, extraArgs...)
-
 			// extra args are expected when ignoreFlags is unset or false
 			testExtraArgs := extraArgs
 			if tc.ignoreFlags {

internal/plugin/testdata/plugdir/bad/duplicate-entries-v1/plugin.yaml

@@ -0,0 +1,16 @@
+name: "duplicate-entries"
+version: "0.1.0"
+type: cli/v1
+apiVersion: v1
+runtime: subprocess
+config:
+  shortHelp: "test duplicate entries"
+  longHelp: |-
+    description
+  ignoreFlags: true
+runtimeConfig:
+  command: "echo hello"
+  hooks:
+    install: "echo installing..."
+  hooks:
+    install: "echo installing something different"

internal/plugin/testdata/plugdir/good/echo-v1/plugin.yaml

@@ -0,0 +1,15 @@
+---
+name: "echo-v1"
+version: "1.2.3"
+type: cli/v1
+apiVersion: v1
+runtime: subprocess
+config:
+  shortHelp: "echo something"
+  longHelp: |-
+    This is a testing fixture.
+  ignoreFlags: false
+runtimeConfig:
+  command: "echo Hello"
+  hooks:
+    install: "echo Installing"

internal/plugin/testdata/plugdir/good/getter/plugin.yaml

@@ -0,0 +1,16 @@
+---
+name: "getter"
+version: "1.2.3"
+type: getter/v1
+apiVersion: v1
+runtime: subprocess
+config:
+  protocols:
+    - "myprotocol"
+    - "myprotocols"
+runtimeConfig:
+  protocolCommands:
+    - command: "echo getter"
+      protocols:
+        - "myprotocol"
+        - "myprotocols"

internal/plugin/testdata/plugdir/good/hello-v1/hello.ps1

@@ -0,0 +1,3 @@
+#!/usr/bin/env pwsh
+
+Write-Host "Hello, world!"

internal/plugin/testdata/plugdir/good/hello-v1/hello.sh

@@ -0,0 +1,9 @@
+#!/bin/bash
+
+echo "Hello from a Helm plugin"
+
+echo "PARAMS"
+echo $*
+
+$HELM_BIN ls --all
+

internal/plugin/testdata/plugdir/good/hello-v1/plugin.yaml

@@ -0,0 +1,32 @@
+---
+name: "hello-v1"
+version: "0.1.0"
+type: cli/v1
+apiVersion: v1
+runtime: subprocess
+config:
+  usage: hello [params]...
+  shortHelp: "echo hello message"
+  longHelp: |-
+    description
+  ignoreFlags: true
+runtimeConfig:
+  platformCommand:
+    - os: linux
+      arch:
+      command: "sh"
+      args: ["-c", "${HELM_PLUGIN_DIR}/hello.sh"]
+    - os: windows
+      arch:
+      command: "pwsh"
+      args: ["-c", "${HELM_PLUGIN_DIR}/hello.ps1"]
+  platformHooks:
+    install:
+      - os: linux
+        arch: ""
+        command: "sh"
+        args: ["-c", 'echo "installing..."']
+      - os: windows
+        arch: ""
+        command: "pwsh"
+        args: ["-c", 'echo "installing..."']

pkg/cmd/flags.go

@@ -164,7 +164,6 @@ func (o *outputValue) Set(s string) error {
 	return nil
 }
 
-// TODO there is probably a better way to pass cobra settings than as a param
 func bindPostRenderFlag(cmd *cobra.Command, varRef *postrender.PostRenderer) {
 	p := &postRendererOptions{varRef, "", []string{}}
 	cmd.Flags().Var(&postRendererString{p}, postRenderFlag, "the path to an executable to be used for post rendering. If it exists in $PATH, the binary will be used, otherwise it will try to look for the executable at the given path")

pkg/cmd/testdata/helm home with space/helm/plugins/fullenv/plugin.yaml

@@ -1,4 +1,10 @@
 name: fullenv
-usage: "show env vars"
-description: "show all env vars"
-command: "$HELM_PLUGIN_DIR/fullenv.sh"
+type: cli/v1
+apiVersion: v1
+runtime: subprocess
+config:
+  shortHelp: "show env vars"
+  longHelp: "show all env vars"
+  ignoreFlags: false
+runtimeConfig:
+  command: "$HELM_PLUGIN_DIR/fullenv.sh"

pkg/cmd/testdata/helmhome/helm/plugins/args/plugin.yaml

@@ -1,4 +1,10 @@
 name: args
-usage: "echo args"
-description: "This echos args"
-command: "$HELM_PLUGIN_DIR/args.sh"
+type: cli/v1
+apiVersion: v1
+runtime: subprocess
+config:
+  shortHelp: "echo args"
+  longHelp: "This echos args"
+  ignoreFlags: false
+runtimeConfig:
+  command: "$HELM_PLUGIN_DIR/args.sh"

pkg/cmd/testdata/helmhome/helm/plugins/echo/plugin.yaml

@@ -1,4 +1,10 @@
 name: echo
-usage: "echo stuff"
-description: "This echos stuff"
-command: "echo hello"
+type: cli/v1
+apiVersion: v1
+runtime: subprocess
+config:
+  shortHelp: "echo stuff"
+  longHelp: "This echos stuff"
+  ignoreFlags: false
+runtimeConfig:
+  command: "echo hello"

pkg/cmd/testdata/helmhome/helm/plugins/env/plugin.yaml

@@ -1,4 +1,10 @@
 name: env
-usage: "env stuff"
-description: "show the env"
-command: "echo $HELM_PLUGIN_NAME"
+type: cli/v1
+apiVersion: v1
+runtime: subprocess
+config:
+  shortHelp: "env stuff"
+  longHelp: "show the env"
+  ignoreFlags: false
+runtimeConfig:
+  command: "echo $HELM_PLUGIN_NAME"

pkg/cmd/testdata/helmhome/helm/plugins/exitwith/plugin.yaml

@@ -1,4 +1,10 @@
 name: exitwith
-usage: "exitwith code"
-description: "This exits with the specified exit code"
-command: "$HELM_PLUGIN_DIR/exitwith.sh"
+type: cli/v1
+apiVersion: v1
+runtime: subprocess
+config:
+  shortHelp: "exitwith code"
+  longHelp: "This exits with the specified exit code"
+  ignoreFlags: false
+runtimeConfig:
+  command: "$HELM_PLUGIN_DIR/exitwith.sh"

pkg/cmd/testdata/helmhome/helm/plugins/fullenv/plugin.yaml

@@ -1,4 +1,10 @@
 name: fullenv
-usage: "show env vars"
-description: "show all env vars"
-command: "$HELM_PLUGIN_DIR/fullenv.sh"
+type: cli/v1
+apiVersion: v1
+runtime: subprocess
+config:
+  shortHelp: "show env vars"
+  longHelp: "show all env vars"
+  ignoreFlags: false
+runtimeConfig:
+  command: "$HELM_PLUGIN_DIR/fullenv.sh"

pkg/getter/plugingetter_test.go

@@ -106,7 +106,11 @@ func (t *TestPlugin) Dir() string {
 
 func (t *TestPlugin) Metadata() plugin.Metadata {
 	return plugin.Metadata{
-		Name:   "fake-plugin",
+		Name:       "fake-plugin",
+		Type:       "cli/v1",
+		APIVersion: "v1",
+		Runtime:    "subprocess",
+		// TODO: either change Config to plugin.ConfigCLI, or change APIVersion to getter/v1?
 		Config: &plugin.ConfigGetter{},
 		RuntimeConfig: &plugin.RuntimeConfigSubprocess{
 			PlatformCommands: []plugin.PlatformCommand{

pkg/getter/testdata/plugins/testgetter/plugin.yaml

@@ -1,6 +1,13 @@
 name: "testgetter"
 version: "0.1.0"
-downloaders:
-  - command: "echo"
-    protocols:
-      - "test"
+type: getter/v1
+apiVersion: v1
+runtime: subprocess
+config:
+  protocols:
+    - "test"
+runtimeConfig:
+  protocolCommands:
+    - command: "echo"
+      protocols:
+        - "test"

pkg/getter/testdata/plugins/testgetter2/plugin.yaml

@@ -1,6 +1,13 @@
 name: "testgetter2"
 version: "0.1.0"
-downloaders:
-  - command: "echo"
-    protocols:
-      - "test2"
+type: getter/v1
+apiVersion: v1
+runtime: subprocess
+config:
+  protocols:
+    - "test2"
+runtimeConfig:
+  protocolCommands:
+    - command: "echo"
+      protocols:
+        - "test2"