From 0ec025a14ce8809b141b3714953c5c01511cf078 Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane Date: Wed, 11 Jun 2025 11:40:28 +0900 Subject: [PATCH 1/4] Allaw users to set nothing config for the plugin in the app.pipecd.yaml Signed-off-by: Yoshiki Fujikane --- pkg/plugin/sdk/deployment_source.go | 14 ++++----- pkg/plugin/sdk/deployment_source_test.go | 39 +++++++++++++----------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/pkg/plugin/sdk/deployment_source.go b/pkg/plugin/sdk/deployment_source.go index 7024a63ec4..b867ffe953 100644 --- a/pkg/plugin/sdk/deployment_source.go +++ b/pkg/plugin/sdk/deployment_source.go @@ -111,15 +111,15 @@ func (c *ApplicationConfig[Spec]) parsePluginConfig(pluginName string) error { c.pluginConfigs = nil }() - if c.pluginConfigs == nil || c.pluginConfigs[pluginName] == nil { - // No config is set for this plugin. - // Set the Spec to the zero value of the spec type to avoid nil pointer dereference. - c.Spec = new(Spec) - return nil + // It is necessary to prepare config with default value when users doesn't set any config. + data := []byte("{}") + + if c.pluginConfigs != nil && c.pluginConfigs[pluginName] != nil { + data = c.pluginConfigs[pluginName] } var spec Spec - if err := json.Unmarshal(c.pluginConfigs[pluginName], &spec); err != nil { + if err := json.Unmarshal(data, &spec); err != nil { return fmt.Errorf("failed to unmarshal application config: plugin spec: %w", err) } @@ -128,7 +128,7 @@ func (c *ApplicationConfig[Spec]) parsePluginConfig(pluginName string) error { } // Validate the spec if it implements the Validate method. - if v, ok := any(spec).(interface{ Validate() error }); ok { + if v, ok := any(&spec).(interface{ Validate() error }); ok { if err := v.Validate(); err != nil { return fmt.Errorf("failed to validate plugin spec: %w", err) } diff --git a/pkg/plugin/sdk/deployment_source_test.go b/pkg/plugin/sdk/deployment_source_test.go index 42e230ad93..fa461d868b 100644 --- a/pkg/plugin/sdk/deployment_source_test.go +++ b/pkg/plugin/sdk/deployment_source_test.go @@ -106,14 +106,13 @@ func TestApplicationConfig_HasStage(t *testing.T) { } type testPluginSpec struct { - Name string `json:"name"` - Value int `json:"value" default:"42"` - Require string `json:"require"` + Name string `json:"name"` + Value int `json:"value" default:"42"` } func (s *testPluginSpec) Validate() error { - if s.Require == "" { - return fmt.Errorf("require must not be empty") + if s.Value < 0 { + return fmt.Errorf("value must not be a negative value") } return nil } @@ -134,8 +133,11 @@ func TestApplicationConfig_ParsePluginConfig(t *testing.T) { config: &ApplicationConfig[testPluginSpec]{ pluginConfigs: nil, }, - wantSpec: &testPluginSpec{}, - wantErr: false, + wantSpec: &testPluginSpec{ + Name: "", + Value: 42, + }, + wantErr: false, }, { name: "empty plugin configs map", @@ -143,21 +145,23 @@ func TestApplicationConfig_ParsePluginConfig(t *testing.T) { config: &ApplicationConfig[testPluginSpec]{ pluginConfigs: make(map[string]json.RawMessage), }, - wantSpec: &testPluginSpec{}, - wantErr: false, + wantSpec: &testPluginSpec{ + Name: "", + Value: 42, + }, + wantErr: false, }, { name: "valid plugin config with defaults", pluginName: "test-plugin", config: &ApplicationConfig[testPluginSpec]{ pluginConfigs: map[string]json.RawMessage{ - "test-plugin": json.RawMessage(`{"name": "test", "require": "yes"}`), + "test-plugin": json.RawMessage(`{"name": "test"}`), }, }, wantSpec: &testPluginSpec{ - Name: "test", - Value: 42, - Require: "yes", + Name: "test", + Value: 42, }, wantErr: false, }, @@ -177,7 +181,7 @@ func TestApplicationConfig_ParsePluginConfig(t *testing.T) { pluginName: "test-plugin", config: &ApplicationConfig[testPluginSpec]{ pluginConfigs: map[string]json.RawMessage{ - "test-plugin": json.RawMessage(`{"name": "test", "value": 10}`), + "test-plugin": json.RawMessage(`{"name": "test", "value": -1}`), }, }, wantSpec: nil, @@ -188,13 +192,12 @@ func TestApplicationConfig_ParsePluginConfig(t *testing.T) { pluginName: "test-plugin", config: &ApplicationConfig[testPluginSpec]{ pluginConfigs: map[string]json.RawMessage{ - "test-plugin": json.RawMessage(`{"name": "test", "value": 100, "require": "yes"}`), + "test-plugin": json.RawMessage(`{"name": "test", "value": 100}`), }, }, wantSpec: &testPluginSpec{ - Name: "test", - Value: 100, - Require: "yes", + Name: "test", + Value: 100, }, wantErr: false, }, From b5c2fa2ee16cc8e496712f3df02fb323d7ec0add Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane Date: Wed, 11 Jun 2025 12:07:01 +0900 Subject: [PATCH 2/4] Remove & Signed-off-by: Yoshiki Fujikane --- pkg/plugin/sdk/deployment_source.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/plugin/sdk/deployment_source.go b/pkg/plugin/sdk/deployment_source.go index b867ffe953..d784c72325 100644 --- a/pkg/plugin/sdk/deployment_source.go +++ b/pkg/plugin/sdk/deployment_source.go @@ -128,7 +128,7 @@ func (c *ApplicationConfig[Spec]) parsePluginConfig(pluginName string) error { } // Validate the spec if it implements the Validate method. - if v, ok := any(&spec).(interface{ Validate() error }); ok { + if v, ok := any(spec).(interface{ Validate() error }); ok { if err := v.Validate(); err != nil { return fmt.Errorf("failed to validate plugin spec: %w", err) } From 01b8d517e7322fee785f6b38d9944008d44b06ea Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane Date: Wed, 11 Jun 2025 12:09:48 +0900 Subject: [PATCH 3/4] Fix comment Signed-off-by: Yoshiki Fujikane --- pkg/plugin/sdk/deployment_source.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/plugin/sdk/deployment_source.go b/pkg/plugin/sdk/deployment_source.go index d784c72325..703fb3979c 100644 --- a/pkg/plugin/sdk/deployment_source.go +++ b/pkg/plugin/sdk/deployment_source.go @@ -111,7 +111,7 @@ func (c *ApplicationConfig[Spec]) parsePluginConfig(pluginName string) error { c.pluginConfigs = nil }() - // It is necessary to prepare config with default value when users doesn't set any config. + // It is necessary to prepare config with default value when users doesn't set any config, or when developers implements custom unmarshalling logic. data := []byte("{}") if c.pluginConfigs != nil && c.pluginConfigs[pluginName] != nil { From 5f2f4535fff4a90d76b6b31d4d844db3353962be Mon Sep 17 00:00:00 2001 From: Yoshiki Fujikane Date: Wed, 11 Jun 2025 12:26:52 +0900 Subject: [PATCH 4/4] Fix comment Signed-off-by: Yoshiki Fujikane --- pkg/plugin/sdk/deployment_source.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/plugin/sdk/deployment_source.go b/pkg/plugin/sdk/deployment_source.go index 703fb3979c..66ebaa78a1 100644 --- a/pkg/plugin/sdk/deployment_source.go +++ b/pkg/plugin/sdk/deployment_source.go @@ -111,7 +111,7 @@ func (c *ApplicationConfig[Spec]) parsePluginConfig(pluginName string) error { c.pluginConfigs = nil }() - // It is necessary to prepare config with default value when users doesn't set any config, or when developers implements custom unmarshalling logic. + // It is necessary to prepare config with default value when users don't set any config, or when plugin developers implement custom unmarshalling logic. data := []byte("{}") if c.pluginConfigs != nil && c.pluginConfigs[pluginName] != nil {