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] Check compatibility doesn't run on the code in the pull request #9371

Closed
peternied opened this issue Aug 15, 2023 · 2 comments · Fixed by #9374
Closed

[BUG] Check compatibility doesn't run on the code in the pull request #9371

peternied opened this issue Aug 15, 2023 · 2 comments · Fixed by #9374
Labels
bug Something isn't working CI CI related

Comments

@peternied
Copy link
Member

peternied commented Aug 15, 2023

Describe the bug
When the Check Compatibility workflow is run, it uses pull_request_target [1], this allows the check to have write access to the repository for updating comments; however, when built it isn't using the code from the pull request itself, so the check is always running against what is in main or 2.x at the time the pull request is updated.

To Reproduce

  1. Go to a recent pull request, like Mute remote store + segRep flaky tests that frequently block checks. #9366
  2. Look for the Compatibility status comment
  3. See the compatible with change XXXXX part of the message
  4. Check which commit that change is associated with
  5. Notice the change is from the main branch

Expected behavior
The commit should be from the pull request.

Additional context

@peternied peternied added bug Something isn't working untriaged labels Aug 15, 2023
@peternied
Copy link
Member Author

@gaiksaya Might be able to make small changes to the structure of the job to perform a workflow dispatch against the pull request branch that generates the output, then the workflow grabs the output and makes it into a comment. This will prevent someone from adding a task into the build process that can operate with write access to OpenSearch's GitHub repository.

Alternatively, drop the comment and use the check result to communicate.

@gaiksaya
Copy link
Member

My bad! I believe I exclusively used pull_request_target to run code from the PR. Looks like I misunderstood.
Will create a PR to change the this to pull_request. However, regarding comment, the reason we went with this approach is to not block the PR (due to failing checks caused by false negatives)

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

Successfully merging a pull request may close this issue.

3 participants