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

Add functionality for parsing Action inputs from a workflow file #1392

Merged
merged 8 commits into from
Dec 1, 2022

Conversation

henrymercer
Copy link
Contributor

@henrymercer henrymercer commented Nov 23, 2022

In order to upload SARIF files for code scanning runs where the init step failed, we need to be able to handle the case where the analyze Action did not run. In this case, the post-analyze step will not be run, and therefore we don't know the inputs to analyze. (In particular, we care about category, upload, and checkout_path.)

This PR implements some functionality to make a best effort attempt to work out the value of an input to an Action given the workflow file and information that's available to the init step like matrix variables. I recommend reviewing the first commit separately since it separates out workflow-related code into a separate file workflow.ts.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

@henrymercer henrymercer marked this pull request as ready for review November 24, 2022 15:23
@henrymercer henrymercer requested a review from a team as a code owner November 24, 2022 15:23
src/workflow.ts Outdated Show resolved Hide resolved
src/workflow.ts Outdated Show resolved Hide resolved
This better handles cases where customers have a monorepo and have
separate jobs for different components.
Copy link
Contributor

@aeisenberg aeisenberg left a comment

Choose a reason for hiding this comment

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

OK. This will now throw if we can't determine what the actual input value is. I don't actually see where getting the category is being used. Is that coming in a later PR? We should make sure that the action doesn't fail if it can't find the category (most likely, the action will already have failed for some other reason, so we need to at least make sure that this failure doesn't hide the original failure).

@henrymercer
Copy link
Contributor Author

This functionality is just for #1393 — ideally we'd minimise the number of places where we need to do this hacky parsing of the workflow. We wrap it in a try-catch block there to avoid hiding the original failure as you mention. I think the tryOrThrow names should make it fairly obvious that these functions should generally be surrounded by try-catch blocks — is there anything else we should do to avoid future consumers creating bugs?

@aeisenberg
Copy link
Contributor

I think that all makes sense. I'll review #1393 later today. Maybe just adding a comment to the function suggesting that it should be wrapped in a try-catch.

aeisenberg
aeisenberg previously approved these changes Nov 28, 2022
Copy link
Contributor

@aeisenberg aeisenberg left a comment

Choose a reason for hiding this comment

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

LGTM. Optionally, add a comment to the tryOrThrow... functions to suggest wrapping in try-catch.

@henrymercer henrymercer merged commit 0d9b15c into main Dec 1, 2022
@henrymercer henrymercer deleted the henrymercer/parse-category branch December 1, 2022 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants