Clone step checkouts default branch on Pull Request event (Bitbucket) #3932
Labels
Comments
|
We found the issue. The Pull Request hook parser is using the destination information, it should use the source: diff --git a/server/forge/bitbucket/convert.go b/server/forge/bitbucket/convert.go
index af573cbff..e64522f94 100644
--- a/server/forge/bitbucket/convert.go
+++ b/server/forge/bitbucket/convert.go
@@ -170,14 +170,14 @@ func convertPullHook(from *internal.PullRequestHook) *model.Pipeline {
return &model.Pipeline{
Event: event,
- Commit: from.PullRequest.Dest.Commit.Hash,
- Ref: fmt.Sprintf("refs/heads/%s", from.PullRequest.Dest.Branch.Name),
+ Commit: from.PullRequest.Source.Commit.Hash,
+ Ref: fmt.Sprintf("refs/heads/%s", from.PullRequest.Source.Branch.Name),
Refspec: fmt.Sprintf("%s:%s",
from.PullRequest.Source.Branch.Name,
from.PullRequest.Dest.Branch.Name,
),
ForgeURL: from.PullRequest.Links.HTML.Href,
- Branch: from.PullRequest.Dest.Branch.Name,
+ Branch: from.PullRequest.Source.Branch.Name,
Message: from.PullRequest.Desc,
Avatar: from.Actor.Links.Avatar.Href,
Author: from.Actor.Login,
Once this is fixed, another issue happens related to how the pipeline pulls the code: + git init -b ${Branch}
Initialized empty Git repository in /woodpecker/src/bitbucket.org/**
+ git config --global --replace-all safe.directory /woodpecker/src/bitbucket.org/***
+ git remote add origin https://bitbucket.org/**
+ git fetch --no-tags --depth=1 --filter=tree:0 origin +${Commit}: # Full SHA required
From https://bitbucket.org/**
* branch ${Commit} -> FETCH_HEAD # Full SHA required
+ git reset --hard -q ${Commit} # Full SHA requiredThe problem is that the Bitbucket Webhooks are not consistent. The pull request update webhook returns a short SHA, not the full one: https://jira.atlassian.com/browse/BCLOUD-21201. We can change the WDYT @qwerty287 ? We can create the PRs. |
|
If that fixes the issues, yes, just open the PRs! :) |
Agree.
Disagree. Everyone can do anything with your pipeline, including secrets. |
No, it should use the PR ones. Currently this is the behavior for all forges and this issue is just about bitbucket, so this is just a bug and the PR one should be used. However, not running the PR configuration has some disadvantages as you can't test your workflows anymore. You should use protected repos to prevent attacks like this (and yes, this feature needs more improvements to be more flexible). |
|
Jenkins, GitHub and GitLab use the Pull Request branch workflows. Imaging you added a required input to your tests and the Pull Request runs the default branch workflow, it will fail. You will have to merge your changes without testing them. You are right:
They mitigate this issues adding access and execution policies to the workflows (e.g. who can run them or who has access to the secrets). |
So, sorry to bother this thread, go ahead and fix the bug :) |
This was referenced Jul 23, 2024
6543
pushed a commit
that referenced
this issue
Jul 23, 2024
) ## Description This is the first fix for: #3932 Change the Pull Request hook parser to return the source commit, branch, and ref instead of the destination. Right now, the workflow pulls the destination configuration and code. It should pull the source configuration and code to verify that the configuration and code work as expected before merging the changes. In case of the close event, the hook parser returns the destination branch, ref and merge commit. Usually, the contributor automatically deletes the source branch after merging the changes to the destination branch. Using the source values will cause the workflow to fail. After the changes, Woodpecker will correctly download the workflow from the source branch (Pull Request commit), but it will fail to clone the repository. This issue is related to the commit format returned by the Bitbucket webhook. This inconsistency has already been reported: https://jira.atlassian.com/browse/BCLOUD-21201. The webhook returns a short SHA. The problem is that the `git fetch` command requires the full SHA. A workaround for this issue is to use the ref to fetch the code: ```yaml clone: git: image: woodpeckerci/plugin-git settings: ref: ${CI_COMMIT_REF} ``` This is not ideal, because the Pull Request head won't always match the workflow commit, but it solves 80% of the event use cases (e.g. trigger a pull request workflow on change). This workaround won't work when re-running a previous workflow pointing to another commit, it will pull the last commit, not the previous one. ## Solutions The solution proposed by the community is to retrieve the full SHA from the Bitbucket API using the short one. This solution has drawbacks: - The Bitbucket API rate limit is 1000 req/h. This solution will reduce the maximum number of workflow runs per hour. - It requires a braking change in the forges interface because the ´Hook(...)´ method does not have an instance of the HTTP Client. We propose to allow the git plugin to fetch the source code from a URL. The Bitbucket returns a link pointing to the commit. This proposal only requires a small change to the git plugin: - Add a new optional parameter (e.g. CommitLink) - Add a clause to the following conditional: https://github.com/woodpecker-ci/plugin-git/blob/7ac9615f409b539486b8841bd5ef01ae16bbc434/plugin.go#L79C1-L88C3 ```go if p.Pipeline.CommitLink != "" {...} ``` Git commands: ```shell $ git fetch --no-tags --depth=1 --filter=tree:0 https://bitbucket.org/workspace/repo/commits/692972aabfec $ git reset --hard -q 692972aabfec # It works with the short SHA ``` Woodpecker will set CommitLink to a blank string for the other forges, but Bitbuckket will use the one returned by the webhook.
6543
pushed a commit
that referenced
this issue
Jul 23, 2024
) ## Description This is the first fix for: #3932 Change the Pull Request hook parser to return the source commit, branch, and ref instead of the destination. Right now, the workflow pulls the destination configuration and code. It should pull the source configuration and code to verify that the configuration and code work as expected before merging the changes. In case of the close event, the hook parser returns the destination branch, ref and merge commit. Usually, the contributor automatically deletes the source branch after merging the changes to the destination branch. Using the source values will cause the workflow to fail. After the changes, Woodpecker will correctly download the workflow from the source branch (Pull Request commit), but it will fail to clone the repository. This issue is related to the commit format returned by the Bitbucket webhook. This inconsistency has already been reported: https://jira.atlassian.com/browse/BCLOUD-21201. The webhook returns a short SHA. The problem is that the `git fetch` command requires the full SHA. A workaround for this issue is to use the ref to fetch the code: ```yaml clone: git: image: woodpeckerci/plugin-git settings: ref: ${CI_COMMIT_REF} ``` This is not ideal, because the Pull Request head won't always match the workflow commit, but it solves 80% of the event use cases (e.g. trigger a pull request workflow on change). This workaround won't work when re-running a previous workflow pointing to another commit, it will pull the last commit, not the previous one. ## Solutions The solution proposed by the community is to retrieve the full SHA from the Bitbucket API using the short one. This solution has drawbacks: - The Bitbucket API rate limit is 1000 req/h. This solution will reduce the maximum number of workflow runs per hour. - It requires a braking change in the forges interface because the ´Hook(...)´ method does not have an instance of the HTTP Client. We propose to allow the git plugin to fetch the source code from a URL. The Bitbucket returns a link pointing to the commit. This proposal only requires a small change to the git plugin: - Add a new optional parameter (e.g. CommitLink) - Add a clause to the following conditional: https://github.com/woodpecker-ci/plugin-git/blob/7ac9615f409b539486b8841bd5ef01ae16bbc434/plugin.go#L79C1-L88C3 ```go if p.Pipeline.CommitLink != "" {...} ``` Git commands: ```shell $ git fetch --no-tags --depth=1 --filter=tree:0 https://bitbucket.org/workspace/repo/commits/692972aabfec $ git reset --hard -q 692972aabfec # It works with the short SHA ``` Woodpecker will set CommitLink to a blank string for the other forges, but Bitbuckket will use the one returned by the webhook.
|
@qwerty287 @6543 Whenever you have a moment, can you review woodpecker-ci/plugin-git#160? This is the last PR to close this issue. Thanks! |
|
woodpecker-ci/plugin-git#161 -> 2.5.2 is available |
|
Pull Request integration for BB is working as expected now. Thanks! |
6543
pushed a commit
to 6543-forks/woodpecker
that referenced
this issue
Sep 5, 2024
…odpecker-ci#3965) ## Description This is the first fix for: woodpecker-ci#3932 Change the Pull Request hook parser to return the source commit, branch, and ref instead of the destination. Right now, the workflow pulls the destination configuration and code. It should pull the source configuration and code to verify that the configuration and code work as expected before merging the changes. In case of the close event, the hook parser returns the destination branch, ref and merge commit. Usually, the contributor automatically deletes the source branch after merging the changes to the destination branch. Using the source values will cause the workflow to fail. After the changes, Woodpecker will correctly download the workflow from the source branch (Pull Request commit), but it will fail to clone the repository. This issue is related to the commit format returned by the Bitbucket webhook. This inconsistency has already been reported: https://jira.atlassian.com/browse/BCLOUD-21201. The webhook returns a short SHA. The problem is that the `git fetch` command requires the full SHA. A workaround for this issue is to use the ref to fetch the code: ```yaml clone: git: image: woodpeckerci/plugin-git settings: ref: ${CI_COMMIT_REF} ``` This is not ideal, because the Pull Request head won't always match the workflow commit, but it solves 80% of the event use cases (e.g. trigger a pull request workflow on change). This workaround won't work when re-running a previous workflow pointing to another commit, it will pull the last commit, not the previous one. ## Solutions The solution proposed by the community is to retrieve the full SHA from the Bitbucket API using the short one. This solution has drawbacks: - The Bitbucket API rate limit is 1000 req/h. This solution will reduce the maximum number of workflow runs per hour. - It requires a braking change in the forges interface because the ´Hook(...)´ method does not have an instance of the HTTP Client. We propose to allow the git plugin to fetch the source code from a URL. The Bitbucket returns a link pointing to the commit. This proposal only requires a small change to the git plugin: - Add a new optional parameter (e.g. CommitLink) - Add a clause to the following conditional: https://github.com/woodpecker-ci/plugin-git/blob/7ac9615f409b539486b8841bd5ef01ae16bbc434/plugin.go#L79C1-L88C3 ```go if p.Pipeline.CommitLink != "" {...} ``` Git commands: ```shell $ git fetch --no-tags --depth=1 --filter=tree:0 https://bitbucket.org/workspace/repo/commits/692972aabfec $ git reset --hard -q 692972aabfec # It works with the short SHA ``` Woodpecker will set CommitLink to a blank string for the other forges, but Bitbuckket will use the one returned by the webhook.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Component
server
Describe the bug
Goal
Run formatters and tests after pushing a new commit to a pull request.
Forge
Bitbucket Cloud
Issue
The clone step checkouts the default branch and uses the default branch workflow. It should checkout the pull request branch and use the pull request workflow.
Workflow
Logs
Additional Information
pullrequest:updated, it was not enabled by default. Probably because it is not selected by default when Woodpecker creates the webhook:woodpecker/server/forge/bitbucket/bitbucket.go
Line 316 in d6e3ebf
Steps to reproduce
pullrequest:updatedExpected behavior
The clone step should checkout the Pull Request workflow and code instead of checking out the default branch.
System Info
Additional context
No response
Validations
nextversion already [https://woodpecker-ci.org/faq#which-version-of-woodpecker-should-i-use]The text was updated successfully, but these errors were encountered: