Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PluginRequest.Universe doesn't get populated before calling an external plugin #3214

Closed
em-r opened this issue Feb 9, 2023 · 8 comments · Fixed by #3223
Closed

PluginRequest.Universe doesn't get populated before calling an external plugin #3214

em-r opened this issue Feb 9, 2023 · 8 comments · Fixed by #3223
Assignees
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Milestone

Comments

@em-r
Copy link
Contributor

em-r commented Feb 9, 2023

What broke? What's expected?

Before sending a request to an external plugin through STDIN, Kubebuilder doesn't populate PluginRequest.Universe with the content of the files that were modified by plugins earlier in the chain.

For instance, if we try out the example from Phase 2 POC:
kubebuilder create api --plugins=go/v3,myexternalplugin/v2
Files modified by go/v3 won't be available in Universe field in PluginRequest sent to myexternalplugin/v2.

Reproducing this issue

  1. Set up a basic external plugin myexternalplugin/v1 that inspects PluginRequest.Universe. (example)
  2. Initialize a new project using 2 plugins (including the one from 1.): kubebuilder init --plugins=go/v3,myexternalplugin/v1
  3. Notice how the files modified by go/v3 won't be available in PluginRequest.Universe in myexternalplugin.

KubeBuilder (CLI) Version

3.9.0

PROJECT version

No response

Plugin versions

No response

Other versions

Go version: 1.19.4 linux/amd64

Extra Labels

No response

@em-r em-r added the kind/bug Categorizes issue or PR as related to a bug. label Feb 9, 2023
@em-r
Copy link
Contributor Author

em-r commented Feb 9, 2023

@camilamacedo86 @everettraven - I've opened this issue as discussed in the community meeting, thanks!

@em-r
Copy link
Contributor Author

em-r commented Feb 9, 2023

based on helpers.go#L106, Kubebuilder will instantiate an empty before sending PluginRequest to an external plugin, I believe this is one of the reasons why this happens.

@everettraven
Copy link
Contributor

@em-r Thanks for creating this issue! I definitely agree that the line you shared is the problem area.

IMO I think this section should be updated to use the machinery.Filesystem type that is passed through the plugin chain to populate req.Universe. If there are no files in the machinery.Filesystem it may be worth checking if there is a PROJECT file in the current directory to see if the kubebuilder init command has been run in the past and if it has we read the current directory to populate the req.Universe.

@camilamacedo86 @rashmigottipati WDYT?

@camilamacedo86
Copy link
Member

Hi @em-r,

Your help is more than welcome. Please, feel free to raise a PR against to fix this one if you wish.

@camilamacedo86 camilamacedo86 added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Feb 11, 2023
@camilamacedo86 camilamacedo86 added this to the priority milestone Feb 11, 2023
@em-r
Copy link
Contributor Author

em-r commented Feb 11, 2023

/assign

@em-r
Copy link
Contributor Author

em-r commented Feb 16, 2023

Hi @camilamacedo86, I have a question (just out of curiosity): since we injectConfig in all built-in plugins as long as they implement RequiresConfig interface, I was wondering if it would also make sense to have similar behavior for external plugins? my thinking comes from that if a built-in plugin needs config, chances are an external one might need it as well.
Thank you!

@camilamacedo86
Copy link
Member

camilamacedo86 commented Feb 16, 2023

Hi @em-r,

#3214 (comment)
It makes sense for me 👍 The external should also update the config and track on it that the plugin was used and all data inputed. So that it could also be used in features to re-scaffold the projects or in other plugins that might want to do something nice on top or work with them.

@em-r
Copy link
Contributor Author

em-r commented Feb 16, 2023

@camilamacedo86 - this makes sense, I completely agree with you. If there is ever a plan to introduce this, I can help with implementing it (and with POC if needed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
3 participants