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.

Sign up for GitHub

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

Jump to bottom

does not fail on error #130

Closed
omercnet opened this issue Aug 28, 2023 · 2 comments · Fixed by #131
Closed

does not fail on error #130

omercnet opened this issue Aug 28, 2023 · 2 comments · Fixed by #131
Assignees
@iwahbe
Labels
kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Milestone
0.93

Comments

@omercnet
Copy link
Contributor

What happened?

running the action with a malformed config fails the command but the action does not fail

Expected Behavior

action should fail, which would result in a notification to the repo admin that something's broken

Steps to reproduce

see pulumiverse/pulumi-vercel#44 and all the runs in https://github.com/pulumiverse/pulumi-vercel/actions/runs/6001806781/job/16276798111 that were successful although the action failed

Output of pulumi about

n/a

Additional context

No response

Contributing

Vote on this issue by adding a 👍 reaction.
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already).
@omercnet omercnet added kind/bug Some behavior is incorrect or out of spec needs-triage Needs attention from the triage team labels Aug 28, 2023
@iwahbe
Copy link
Member

iwahbe commented Aug 29, 2023

Hi @omercnet. Thank for opening this issue. This is definitely the wrong behavior. It's not the fault of the action though: upgrade-provider returns an exit code of 0 when run with an invalid config file.

I'm going to transfer this issue to the upgrade-provider repo so it can be fixed.

@guineveresaenger guineveresaenger transferred this issue from pulumi/pulumi-upgrade-provider-action Aug 29, 2023
@iwahbe iwahbe removed the needs-triage Needs attention from the triage team label Aug 29, 2023
@iwahbe
Copy link
Member

iwahbe commented Aug 29, 2023

I'm going to pin this until it is fixed, since it is a pretty serious foot gun.

@iwahbe iwahbe pinned this issue Aug 29, 2023
@iwahbe iwahbe self-assigned this Aug 31, 2023
iwahbe added a commit that referenced this issue Aug 31, 2023
@iwahbe
Exit non-zero on error
effce8c 
Fixes #130

We change how errors are displayed on invalid config files. The error message for an
invalid config file now looks like this:

```sh
𝛌 upgrade-provider pulumi/pulumi-aiven
error: While parsing config: yaml: line 4: did not find expected ',' or '}'
```

It used to look like this:

```sh
𝛌 upgrade-provider pulumi/pulumi-aiven
Error: While parsing config: yaml: line 4: did not find expected ',' or '}'
Usage:
  upgrade-provider <provider> [flags]

Flags:
      --allow-missing-docs              If true, don't error on missing docs during tfgen.
                                        This is equivalent to setting PULUMI_MISSING_DOCS_ERROR=${! VALUE}.
      --create-failure-issue            Create an issue in the target repository if the upgrade attempt fails in CI.
      --experimental                    Enable experimental features, such as auto token mapping and auto aliasing
  -h, --help                            help for upgrade-provider

...
```
@iwahbe iwahbe mentioned this issue Aug 31, 2023
Merged
iwahbe added a commit that referenced this issue Aug 31, 2023
@iwahbe
Exit non-zero on error (#131)
b7f53ad 
Fixes #130

We change how errors are displayed on invalid config files. The error message for an
invalid config file now looks like this:

```sh
𝛌 upgrade-provider pulumi/pulumi-aiven
error: While parsing config: yaml: line 4: did not find expected ',' or '}'
```

It used to look like this:

```sh
𝛌 upgrade-provider pulumi/pulumi-aiven
Error: While parsing config: yaml: line 4: did not find expected ',' or '}'
Usage:
  upgrade-provider <provider> [flags]

Flags:
      --allow-missing-docs              If true, don't error on missing docs during tfgen.
                                        This is equivalent to setting PULUMI_MISSING_DOCS_ERROR=${! VALUE}.
      --create-failure-issue            Create an issue in the target repository if the upgrade attempt fails in CI.
      --experimental                    Enable experimental features, such as auto token mapping and auto aliasing
  -h, --help                            help for upgrade-provider

...
```
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Aug 31, 2023
@mikhailshilkov mikhailshilkov added this to the 0.93 milestone Sep 1, 2023
@ringods ringods unpinned this issue Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@iwahbe iwahbe

Labels
kind/bug Some behavior is incorrect or out of spec resolution/fixed This issue was fixed
Projects
None yet
Milestone
0.93
Development

Successfully merging a pull request may close this issue.

Exit non-zero on error pulumi/upgrade-provider
4 participants
@omercnet @mikhailshilkov @iwahbe @pulumi-bot