Merge pull request #31220 from gjenkins8/gjenkins/plugin-integration/plugin_config

refactor: utilize `pluginTypesIndex` for config unmarshalling

internal/plugin/config.go

@@ -16,72 +16,39 @@ limitations under the License.
 package plugin
 
 import (
+	"bytes"
 	"fmt"
+	"reflect"
 
 	"go.yaml.in/yaml/v3"
 )
 
-// Config interface defines the methods that all plugin type configurations must implement
+// Config represents an plugin type specific configuration
+// It is expected to type assert (cast) the a Config to its expected underlying type (schema.ConfigCLIV1, schema.ConfigGetterV1, etc).
 type Config interface {
 	Validate() error
 }
 
-// ConfigCLI represents the configuration for CLI plugins
-type ConfigCLI struct {
-	// Usage is the single-line usage text shown in help
-	// For recommended syntax, see [spf13/cobra.command.Command] Use field comment:
-	// https://pkg.go.dev/github.com/spf13/cobra#Command
-	Usage string `yaml:"usage"`
-	// ShortHelp is the short description shown in the 'helm help' output
-	ShortHelp string `yaml:"shortHelp"`
-	// LongHelp is the long message shown in the 'helm help <this-command>' output
-	LongHelp string `yaml:"longHelp"`
-	// IgnoreFlags ignores any flags passed in from Helm
-	IgnoreFlags bool `yaml:"ignoreFlags"`
-}
-
-// ConfigGetter represents the configuration for download plugins
-type ConfigGetter struct {
-	// Protocols are the list of URL schemes supported by this downloader
-	Protocols []string `yaml:"protocols"`
-}
-
-// ConfigPostrenderer represents the configuration for postrenderer plugins
-// there are no runtime-independent configurations for postrenderer/v1 plugin type
-type ConfigPostrenderer struct{}
-
-func (c *ConfigCLI) Validate() error {
-	// Config validation for CLI plugins
-	return nil
-}
+func unmarshaConfig(pluginType string, configData map[string]any) (Config, error) {
 
-func (c *ConfigGetter) Validate() error {
-	if len(c.Protocols) == 0 {
-		return fmt.Errorf("getter has no protocols")
-	}
-	for i, protocol := range c.Protocols {
-		if protocol == "" {
-			return fmt.Errorf("getter has empty protocol at index %d", i)
-		}
+	pluginTypeMeta, ok := pluginTypesIndex[pluginType]
+	if !ok {
+		return nil, fmt.Errorf("unknown plugin type %q", pluginType)
 	}
-	return nil
-}
 
-func (c *ConfigPostrenderer) Validate() error {
-	// Config validation for postrenderer plugins
-	return nil
-}
+	// TODO: Avoid (yaml) serialization/deserialization for type conversion here
 
-func remarshalConfig[T Config](configData map[string]any) (Config, error) {
 	data, err := yaml.Marshal(configData)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to marshel config data (plugin type %s): %w", pluginType, err)
 	}
 
-	var config T
-	if err := yaml.Unmarshal(data, &config); err != nil {
+	config := reflect.New(pluginTypeMeta.configType)
+	d := yaml.NewDecoder(bytes.NewReader(data))
+	d.KnownFields(true)
+	if err := d.Decode(config.Interface()); err != nil {
 		return nil, err
 	}
 
-	return config, nil
+	return config.Interface().(Config), nil
 }

internal/plugin/config_test.go

@@ -0,0 +1,56 @@
+/*
+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 (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"helm.sh/helm/v4/internal/plugin/schema"
+)
+
+func TestUnmarshaConfig(t *testing.T) {
+	// Test unmarshalling a CLI plugin config
+	{
+		config, err := unmarshaConfig("cli/v1", map[string]any{
+			"usage":       "usage string",
+			"shortHelp":   "short help string",
+			"longHelp":    "long help string",
+			"ignoreFlags": true,
+		})
+		require.NoError(t, err)
+
+		require.IsType(t, &schema.ConfigCLIV1{}, config)
+		assert.Equal(t, schema.ConfigCLIV1{
+			Usage:       "usage string",
+			ShortHelp:   "short help string",
+			LongHelp:    "long help string",
+			IgnoreFlags: true,
+		}, *(config.(*schema.ConfigCLIV1)))
+	}
+
+	// Test unmarshalling invalid config data
+	{
+		config, err := unmarshaConfig("cli/v1", map[string]any{
+			"invalid field": "foo",
+		})
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "field not found")
+		assert.Nil(t, config)
+	}
+}

internal/plugin/loader_test.go

@@ -22,6 +22,8 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"helm.sh/helm/v4/internal/plugin/schema"
 )
 
 func TestPeekAPIVersion(t *testing.T) {
@@ -73,7 +75,7 @@ func TestLoadDir(t *testing.T) {
 			Version:    "0.1.0",
 			Type:       "cli/v1",
 			Runtime:    "subprocess",
-			Config: &ConfigCLI{
+			Config: &schema.ConfigCLIV1{
 				Usage:       usage,
 				ShortHelp:   "echo hello message",
 				LongHelp:    "description",
@@ -145,7 +147,7 @@ func TestLoadDirGetter(t *testing.T) {
 		Type:       "getter/v1",
 		APIVersion: "v1",
 		Runtime:    "subprocess",
-		Config: &ConfigGetter{
+		Config: &schema.ConfigGetterV1{
 			Protocols: []string{"myprotocol", "myprotocols"},
 		},
 		RuntimeConfig: &RuntimeConfigSubprocess{
@@ -173,7 +175,7 @@ func TestPostRenderer(t *testing.T) {
 		Type:       "postrenderer/v1",
 		APIVersion: "v1",
 		Runtime:    "subprocess",
-		Config:     &ConfigPostrenderer{},
+		Config:     &schema.ConfigPostRendererV1{},
 		RuntimeConfig: &RuntimeConfigSubprocess{
 			PlatformCommand: []PlatformCommand{
 				{

internal/plugin/metadata.go

@@ -123,11 +123,11 @@ func buildLegacyConfig(m MetadataLegacy, pluginType string) Config {
 		for _, d := range m.Downloaders {
 			protocols = append(protocols, d.Protocols...)
 		}
-		return &ConfigGetter{
+		return &schema.ConfigGetterV1{
 			Protocols: protocols,
 		}
 	case "cli/v1":
-		return &ConfigCLI{
+		return &schema.ConfigCLIV1{
 			Usage:       "",            // Legacy plugins don't have Usage field for command syntax
 			ShortHelp:   m.Usage,       // Map legacy usage to shortHelp
 			LongHelp:    m.Description, // Map legacy description to longHelp
@@ -175,7 +175,7 @@ func buildLegacyRuntimeConfig(m MetadataLegacy) RuntimeConfig {
 
 func fromMetadataV1(mv1 MetadataV1) (*Metadata, error) {
 
-	config, err := convertMetadataConfig(mv1.Type, mv1.Config)
+	config, err := unmarshaConfig(mv1.Type, mv1.Config)
 	if err != nil {
 		return nil, err
 	}
@@ -197,30 +197,6 @@ func fromMetadataV1(mv1 MetadataV1) (*Metadata, error) {
 	}, nil
 }
 
-func convertMetadataConfig(pluginType string, configRaw map[string]any) (Config, error) {
-	var err error
-	var config Config
-
-	switch pluginType {
-	case "test/v1":
-		config, err = remarshalConfig[*schema.ConfigTestV1](configRaw)
-	case "cli/v1":
-		config, err = remarshalConfig[*ConfigCLI](configRaw)
-	case "getter/v1":
-		config, err = remarshalConfig[*ConfigGetter](configRaw)
-	case "postrenderer/v1":
-		config, err = remarshalConfig[*ConfigPostrenderer](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

internal/plugin/plugin_test.go

@@ -17,6 +17,8 @@ package plugin
 
 import (
 	"testing"
+
+	"helm.sh/helm/v4/internal/plugin/schema"
 )
 
 func mockSubprocessCLIPlugin(t *testing.T, pluginName string) *SubprocessPluginRuntime {
@@ -46,7 +48,7 @@ func mockSubprocessCLIPlugin(t *testing.T, pluginName string) *SubprocessPluginR
 			Type:       "cli/v1",
 			APIVersion: "v1",
 			Runtime:    "subprocess",
-			Config: &ConfigCLI{
+			Config: &schema.ConfigCLIV1{
 				Usage:       "Mock plugin",
 				ShortHelp:   "Mock plugin",
 				LongHelp:    "Mock plugin for testing",

internal/plugin/plugin_type_registry.go

@@ -81,13 +81,19 @@ var pluginTypes = []pluginTypeMeta{
 		pluginType: "cli/v1",
 		inputType:  reflect.TypeOf(schema.InputMessageCLIV1{}),
 		outputType: reflect.TypeOf(schema.OutputMessageCLIV1{}),
-		configType: reflect.TypeOf(ConfigCLI{}),
+		configType: reflect.TypeOf(schema.ConfigCLIV1{}),
 	},
 	{
 		pluginType: "getter/v1",
 		inputType:  reflect.TypeOf(schema.InputMessageGetterV1{}),
 		outputType: reflect.TypeOf(schema.OutputMessageGetterV1{}),
-		configType: reflect.TypeOf(ConfigGetter{}),
+		configType: reflect.TypeOf(schema.ConfigGetterV1{}),
+	},
+	{
+		pluginType: "postrenderer/v1",
+		inputType:  reflect.TypeOf(schema.InputMessagePostRendererV1{}),
+		outputType: reflect.TypeOf(schema.OutputMessagePostRendererV1{}),
+		configType: reflect.TypeOf(schema.ConfigPostRendererV1{}),
 	},
 }
 

internal/plugin/plugin_type_registry_test.go

@@ -34,5 +34,5 @@ func TestMakeOutputMessage(t *testing.T) {
 func TestMakeConfig(t *testing.T) {
 	ptm := pluginTypesIndex["getter/v1"]
 	config := reflect.New(ptm.configType).Interface().(Config)
-	assert.IsType(t, &ConfigGetter{}, config)
+	assert.IsType(t, &schema.ConfigGetterV1{}, config)
 }

internal/plugin/runtime_subprocess_test.go

@@ -45,7 +45,7 @@ func mockSubprocessCLIPluginErrorExit(t *testing.T, pluginName string, exitCode
 		Type:       "cli/v1",
 		APIVersion: "v1",
 		Runtime:    "subprocess",
-		Config: &ConfigCLI{
+		Config: &schema.ConfigCLIV1{
 			Usage:       "Mock plugin",
 			ShortHelp:   "Mock plugin",
 			LongHelp:    "Mock plugin for testing",

internal/plugin/schema/cli.go

@@ -27,3 +27,22 @@ type InputMessageCLIV1 struct {
 type OutputMessageCLIV1 struct {
 	Data *bytes.Buffer `json:"data"`
 }
+
+// ConfigCLIV1 represents the configuration for CLI plugins
+type ConfigCLIV1 struct {
+	// Usage is the single-line usage text shown in help
+	// For recommended syntax, see [spf13/cobra.command.Command] Use field comment:
+	// https://pkg.go.dev/github.com/spf13/cobra#Command
+	Usage string `yaml:"usage"`
+	// ShortHelp is the short description shown in the 'helm help' output
+	ShortHelp string `yaml:"shortHelp"`
+	// LongHelp is the long message shown in the 'helm help <this-command>' output
+	LongHelp string `yaml:"longHelp"`
+	// IgnoreFlags ignores any flags passed in from Helm
+	IgnoreFlags bool `yaml:"ignoreFlags"`
+}
+
+func (c *ConfigCLIV1) Validate() error {
+	// Config validation for CLI plugins
+	return nil
+}

internal/plugin/schema/doc.go

@@ -0,0 +1,18 @@
+/*
+ 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 schema

internal/plugin/schema/getter.go

@@ -14,10 +14,11 @@
 package schema
 
 import (
+	"fmt"
 	"time"
 )
 
-// TODO: can we generate these plugin input/outputs?
+// TODO: can we generate these plugin input/output messages?
 
 type GetterOptionsV1 struct {
 	URL                   string
@@ -45,3 +46,21 @@ type InputMessageGetterV1 struct {
 type OutputMessageGetterV1 struct {
 	Data []byte `json:"data"`
 }
+
+// ConfigGetterV1 represents the configuration for download plugins
+type ConfigGetterV1 struct {
+	// Protocols are the list of URL schemes supported by this downloader
+	Protocols []string `yaml:"protocols"`
+}
+
+func (c *ConfigGetterV1) Validate() error {
+	if len(c.Protocols) == 0 {
+		return fmt.Errorf("getter has no protocols")
+	}
+	for i, protocol := range c.Protocols {
+		if protocol == "" {
+			return fmt.Errorf("getter has empty protocol at index %d", i)
+		}
+	}
+	return nil
+}

internal/plugin/schema/postrenderer.go

@@ -30,3 +30,9 @@ type InputMessagePostRendererV1 struct {
 type OutputMessagePostRendererV1 struct {
 	Manifests *bytes.Buffer `json:"manifests"`
 }
+
+type ConfigPostRendererV1 struct{}
+
+func (c *ConfigPostRendererV1) Validate() error {
+	return nil
+}

pkg/cmd/load_plugins.go

@@ -71,7 +71,7 @@ func loadCLIPlugins(baseCmd *cobra.Command, out io.Writer) {
 	for _, plug := range found {
 		var use, short, long string
 		var ignoreFlags bool
-		if cliConfig, ok := plug.Metadata().Config.(*plugin.ConfigCLI); ok {
+		if cliConfig, ok := plug.Metadata().Config.(*schema.ConfigCLIV1); ok {
 			use = cliConfig.Usage
 			short = cliConfig.ShortHelp
 			long = cliConfig.LongHelp
@@ -340,7 +340,7 @@ func pluginDynamicComp(plug plugin.Plugin, cmd *cobra.Command, args []string, to
 	}
 
 	var ignoreFlags bool
-	if cliConfig, ok := subprocessPlug.Metadata().Config.(*plugin.ConfigCLI); ok {
+	if cliConfig, ok := subprocessPlug.Metadata().Config.(*schema.ConfigCLIV1); ok {
 		ignoreFlags = cliConfig.IgnoreFlags
 	}
 

pkg/cmd/plugin_list.go

@@ -26,6 +26,7 @@ import (
 	"github.com/spf13/cobra"
 
 	"helm.sh/helm/v4/internal/plugin"
+	"helm.sh/helm/v4/internal/plugin/schema"
 )
 
 func newPluginListCmd(out io.Writer) *cobra.Command {
@@ -106,7 +107,7 @@ func compListPlugins(_ string, ignoredPluginNames []string) []string {
 		for _, p := range filteredPlugins {
 			m := p.Metadata()
 			var shortHelp string
-			if config, ok := m.Config.(*plugin.ConfigCLI); ok {
+			if config, ok := m.Config.(*schema.ConfigCLIV1); ok {
 				shortHelp = config.ShortHelp
 			}
 			pNames = append(pNames, fmt.Sprintf("%s\t%s", p.Metadata().Name, shortHelp))

pkg/cmd/testdata/helmhome/helm/plugins/postrenderer-v1/plugin.yaml

@@ -4,10 +4,6 @@ name: "postrenderer-v1"
 version: "1.2.3"
 type: postrenderer/v1
 runtime: subprocess
-config:
-  shortHelp: "echo test"
-  longHelp: "This echos test"
-  ignoreFlags: false
 runtimeConfig:
   platformCommand:
   - command: "${HELM_PLUGIN_DIR}/sed-test.sh"

pkg/getter/plugingetter.go

@@ -49,7 +49,7 @@ func collectGetterPlugins(settings *cli.EnvSettings) (Providers, error) {
 	}
 	results := make([]Provider, 0, len(plgs))
 	for _, plg := range plgs {
-		if c, ok := plg.Metadata().Config.(*plugin.ConfigGetter); ok {
+		if c, ok := plg.Metadata().Config.(*schema.ConfigGetterV1); ok {
 			results = append(results, Provider{
 				Schemes: c.Protocols,
 				New:     pluginConstructorBuilder(plg),

pkg/getter/plugingetter_test.go

@@ -110,7 +110,7 @@ func (t *testPlugin) Metadata() plugin.Metadata {
 		Type:       "cli/v1",
 		APIVersion: "v1",
 		Runtime:    "subprocess",
-		Config:     &plugin.ConfigCLI{},
+		Config:     &schema.ConfigCLIV1{},
 		RuntimeConfig: &plugin.RuntimeConfigSubprocess{
 			PlatformCommand: []plugin.PlatformCommand{
 				{