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

[BUG] Action fails in merge queue with v4.3.5 #841

Closed
kylebjordahl opened this issue Oct 21, 2024 · 4 comments · Fixed by #846
Closed

[BUG] Action fails in merge queue with v4.3.5 #841

kylebjordahl opened this issue Oct 21, 2024 · 4 comments · Fixed by #846
Labels
bug Something isn't working

Comments

@kylebjordahl
Copy link

Describe the bug
Action fails in merge queue with unhelpful error (looks like a zod or schema validation?)

Run actions/dependency-review-action@v4
Error: [
  {
    "code": "invalid_type",
    "expected": "object",
    "received": "undefined",
    "path": [],
    "message": "Required"
  }
]

Showed up within minutes of the release of 4.3.5 (which we got due to a floating version pin of v4). Pinning to 4.3.4 seems to resolve the issue

To Reproduce
Unfortunately this is in a private repo, so cannot share a full repro. We use a GitHub merge queue, but have had no issues before the release of 4.3.5

Expected behavior
Action should run.

Action version
Showed up within minutes of the release of 4.3.5 (which we got due to a floating version pin of v4). Pinning to 4.3.4 seems to resolve the issue

Screenshots
Screenshot 2024-10-21 at 4 33 24 PM

@kylebjordahl kylebjordahl added the bug Something isn't working label Oct 21, 2024
@kylebjordahl kylebjordahl changed the title [BUG] [BUG] Action fails in merge queue Oct 21, 2024
@kylebjordahl kylebjordahl changed the title [BUG] Action fails in merge queue [BUG] Action fails in merge queue with v4.3.5 Oct 21, 2024
@Ahmed3lmallah
Copy link
Contributor

HI @kylebjordahl, thank you for reporting this issue!

I'm not able to reproduce the problem with the action in merge queue using default configurations. Can you share the configs that you are using for the action and/or the configuration file, if you're using any.

@kylebjordahl
Copy link
Author

Hi @Ahmed3lmallah - here is our action config yaml (pretty vanilla)

name: 'Dependency review'
on:
  pull_request:
  merge_group:
    types: ['checks_requested']

# If using a dependency submission action in this workflow this permission will need to be set to:
#
# permissions:
#   contents: write
#
# https://docs.github.com/en/enterprise-cloud@latest/code-security/supply-chain-security/understanding-your-software-supply-chain/using-the-dependency-submission-api
permissions:
  contents: read
  # Write permissions for pull-requests are required for using the `comment-summary-in-pr` option, comment out if you aren't using this option
  pull-requests: write

jobs:
  dependency-review:
    name: Dependency Review
    runs-on: ubuntu-latest
    steps:
      - name: 'Checkout repository'
        uses: actions/checkout@v4

      - name: 'Dependency Review'
        # RN-3003: Pinned due to issue with 4.3.5, can probably revert to v4 once a fix is out
        uses: actions/dependency-review-action@v4.3.4
        # Commonly enabled options, see https://github.com/actions/dependency-review-action#configuration-options for all available options.
        with:
          comment-summary-in-pr: always
          fail-on-severity: moderate
          base-ref: ${{ github.event.pull_request.base.ref || github.event.merge_group.base_ref || 'main'}}
          head-ref: ${{ github.event.pull_request.head.ref || github.event.merge_group.head_ref || github.ref}}
          # TODO: review these in mid-sept 2024, see RN-1974 for details
          allow-ghsas: ## REDACTED, THERE ARE TWO
        #   deny-licenses: GPL-1.0-or-later, LGPL-2.0-or-later
        #   retry-on-snapshot-warnings: true

@ebickle
Copy link
Contributor

ebickle commented Oct 27, 2024

Here's my analysis of the issue:

In git-refs.ts, line 17 fails because there's no pull_request field in the event. That passes undefined into the schema parser, which fails to validate the input as a an object.

The code likely needs to do the following:

  • Move if (context.eventName == 'merge_group') to a separate else if block
  • Add a new MergeGroupSchema
  • The merge_group field needs to be pulled out of the context. Right now this is done for pull_request in the function declaration, but this starts to get a bit weird if these fields are not always present. May need some refactoring.
  •  base_ref = base_ref | merge_group.base_sha
     head_ref = head_ref | merge_group.head_sha
    

The short version is that merge_group and pull_request have completely different event structures, so some additional logic was needed beyond allowing the event type. 🙂

@Ahmed3lmallah
Copy link
Contributor

Ahmed3lmallah commented Oct 28, 2024

This bug should be fixed now in the latest release: v4.4.0

Thank you @kylebjordahl for reporting the bug and @ebickle for your analysis of the issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants