-
Notifications
You must be signed in to change notification settings - Fork 73
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
feat: added support to install manually install plugins #810
Conversation
31af32b
to
95d2149
Compare
Changing this to WIP as we have noticed issues with installing the plugins |
After tinkering with the issue more, I realized that the main issue wasn't the installation from other sources, but the rate-limit of GitHub. - run: |
pulumi plugin rm resource sentry --yes
wget https://github.com/pulumiverse/pulumi-sentry/releases/download/v0.0.8/pulumi-resource-sentry-v0.0.8-linux-amd64.tar.gz
pulumi plugin install resource \
sentry v0.0.8 \
--file pulumi-resource-sentry-v0.0.8-linux-amd64.tar.gz Still if you want to add the option to install plugins from this step, I am happy to contribute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this contribution! 🙏
I have left some notes that I would love for you to look at. I think this change really make sense 👍
@@ -93,6 +93,17 @@ inputs: | |||
description: 'Colorize output. Choices are: always, never, raw, auto' | |||
required: false | |||
default: 'auto' | |||
manual-plugins: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should name this config key plugins
?
I'm not sure we can call them manual plugins. AFAIK, They are plugins that don't have an install script, we need to manually install them.
@@ -51,6 +52,7 @@ | |||
"dedent": "^0.7.0", | |||
"envalid": "^7.3.1", | |||
"got": "^11.8.6", | |||
"js-yaml": "^4.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have yaml
installed; would it make sense to use yaml
?
If you feel js-yaml
is an improvement, feel free to open a separate PR on that 👍
@@ -19,6 +19,7 @@ const main = async () => { | |||
core.debug('Configuration is loaded'); | |||
|
|||
await pulumiCli.downloadCli(config.options.pulumiVersion); | |||
await pulumiCli.installManualPlugins(config.manualPlugins); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
await pulumiCli.installManualPlugins(config.manualPlugins); | |
if (config.manualPlugins) { | |
await pulumiCli.installManualPlugins(config.manualPlugins); | |
} |
Hello folks! |
The build-in functionality seems cleaner to me too. |
We are not using this functionality internally anymore, therefore I will close this PR now. |
This adds the option to manually install Pulumi plugins.
We are currently using this Sentry plugin and repeatedly noticed the following issue:
As the log also states, as automatically installing the plugin failed, we are changing our workflow to manually install this specific plugin. Due to action re-installing the pulumi CLI in our Docker-based GitHub Actions, we want to install the plugins right before running other commands. EDIT: after looking further into the code, I realized only
~/.pulumi/bin
is deleted by this step, and not the full pulumi installation folderTherefore this adds the option to declare a list of plugins to manually install.
As nested array objects are not supported by GitHub Actions, we decided to use a YAML-string and parse it at runtime instead.