Skip to content

Pass plugin name from Piped to plugin#5931

Merged
t-kikuc merged 1 commit into
masterfrom
plugin/pass-name
Jun 9, 2025
Merged

Pass plugin name from Piped to plugin#5931
t-kikuc merged 1 commit into
masterfrom
plugin/pass-name

Conversation

@t-kikuc
Copy link
Copy Markdown
Member

@t-kikuc t-kikuc commented Jun 9, 2025

What this PR does:

  • Deleted the name arg from sdk.NewPlugin()
  • Instead, pass the name from the piped plugin config

After merging this PR, we need to update the plugins.

Why we need it:

  • The uniqueness scope of name is confusing for plugin developers
  • Currently, users need to get plugin names from each plugin code since they must be matched
    • Now, deploymentSource is looked up by the name defined in the plugin.
      // newDeploymentSource converts the common.DeploymentSource to the internal representation.
      func newDeploymentSource[Spec any](pluginName string, source *common.DeploymentSource) (DeploymentSource[Spec], error) {
      cfg, err := config.DecodeYAML[*ApplicationConfig[Spec]](source.GetApplicationConfig())
      if err != nil {
      return DeploymentSource[Spec]{}, fmt.Errorf("failed to decode application config: %w", err)
      }
      if err := cfg.Spec.parsePluginConfig(pluginName); err != nil {
      return DeploymentSource[Spec]{}, fmt.Errorf("failed to parse plugin config: %w", err)
      }

Which issue(s) this PR fixes:

Part of #5530

Does this PR introduce a user-facing change?:

This is a breaking change of the SDK. Not a user-facing change.

  • How are users affected by this change:
  • Is this breaking change:
  • How to migrate (if breaking change):

Signed-off-by: t-kikuc <tkikuchi07f@gmail.com>
@t-kikuc t-kikuc requested a review from a team as a code owner June 9, 2025 09:58
Comment thread pkg/plugin/sdk/plugin.go
// Run runs the plugin.
func (p *Plugin[Config, DeployTargetConfig, ApplicationConfigSpec]) Run() error {
app := cli.NewApp(
fmt.Sprintf("pipecd-plugin-%s", p.name),
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is not important. And p.name is empty when showing this message.

Comment thread pkg/plugin/sdk/plugin.go
func (p *Plugin[Config, DeployTargetConfig, ApplicationConfigSpec]) command() *cobra.Command {
cmd := &cobra.Command{
Use: "start",
Short: fmt.Sprintf("Start running a %s plugin.", p.name),
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is not important. And p.name is empty when showing this message.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 9, 2025

Codecov Report

Attention: Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.

Project coverage is 28.46%. Comparing base (e1fcf93) to head (8dbe938).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
pkg/plugin/sdk/plugin.go 0.00% 6 Missing ⚠️
pkg/app/pipedv1/cmd/piped/piped.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5931      +/-   ##
==========================================
- Coverage   28.46%   28.46%   -0.01%     
==========================================
  Files         520      520              
  Lines       56416    56419       +3     
==========================================
  Hits        16061    16061              
- Misses      39089    39092       +3     
  Partials     1266     1266              
Flag Coverage Δ
. 23.74% <0.00%> (-0.01%) ⬇️
.-pkg-app-pipedv1-plugin-example 0.00% <ø> (ø)
.-pkg-app-pipedv1-plugin-kubernetes 66.42% <ø> (ø)
.-pkg-app-pipedv1-plugin-kubernetes_multicluster 67.51% <ø> (ø)
.-pkg-app-pipedv1-plugin-wait 35.51% <ø> (ø)
.-pkg-plugin-sdk 49.14% <0.00%> (-0.06%) ⬇️
.-tool-actions-gh-release 19.23% <ø> (ø)
.-tool-actions-plan-preview 25.30% <ø> (ø)
.-tool-codegen-protoc-gen-auth 0.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@ffjlabo ffjlabo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Copy Markdown
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch, thank you 👍

@t-kikuc t-kikuc merged commit 2e1d97a into master Jun 9, 2025
39 of 40 checks passed
@t-kikuc t-kikuc deleted the plugin/pass-name branch June 9, 2025 12:36
@github-actions github-actions Bot mentioned this pull request Jul 14, 2025
@github-actions github-actions Bot mentioned this pull request Aug 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants