-
Notifications
You must be signed in to change notification settings - Fork 7
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
package: fix relative paths and embedded composite actions #22
Conversation
df76c28
to
27fd731
Compare
Fix several small issues that make it possible to migrate from the prior Docker based version to the composite action version * Check out the action repo into a different directory when building nfpm * Execute nfpm from the root workspace directory to support relative paths in configs * Work around runners bugs when executing composite actions inside the scope of other composite actions. Signed-off-by: Ryan Cragun <me@ryan.ec>
dd0c28d
to
f8956bc
Compare
Tested this branch in Vault here: https://github.com/hashicorp/vault/actions/runs/9023783620/job/24796306015?pr=26905 |
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.
LGTM!
action.yml
Outdated
if ! nfpm package -f "$config_file" -p ${1} -t ./out/; then | ||
printf "failed to create package with nfpm for $1 using config $(cat $config_file)\n" | ||
exit 1 | ||
fi |
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.
Nit: I don't think it's helpful to dump the config file a second time, but I do think it's helpful to always dump it. Maybe remove the conditional and let nfpm's non-zero exit directly halt the script and job?
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.
I think that was an artifact of me debugging and I missed taking it out!
Signed-off-by: Ryan Cragun <me@ryan.ec>
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.
Fix several small issues that make it possible to migrate from the prior
Docker based version to the composite action version
paths in configs
scope of other composite actions.