woodpecker-ci / woodpecker

Woodpecker is a simple, yet powerful CI/CD engine with great extensibility.
https://woodpecker-ci.org
Apache License 2.0
4.3k stars 371 forks source link

Clone step checkouts default branch on Pull Request event (Bitbucket) #3932

Closed j04n-f closed 3 months ago

j04n-f commented 4 months ago

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

when:
  event: pull_request

clone:
  git:
    image: woodpeckerci/plugin-git
    settings:
      partial: false

steps:
  echo:
    image: ubuntu
    commands:
      - echo "Hello World"
    when:
      event: pull_request

Logs

+ git init -b master 0s
+ Initialized empty Git repository in /woodpecker/src/**
+ git config --global --replace-all safe.directory /woodpecker/src/**
+ git remote add origin https://bitbucket.org/**
+ git fetch --no-tags --depth=1 --filter=tree:0 origin +a6b5f1dfe5ac:
fatal: couldn't find remote ref a6b5f1dfe5ac
retry in 5s
+ git fetch --no-tags --depth=1 --filter=tree:0 origin +a6b5f1dfe5ac:5s
fatal: couldn't find remote ref a6b5f1dfe5ac
retry in 5s
exit status 128

Additional Information

Steps to reproduce

  1. Create a Bitbucket Cloud repository
  2. Update the Bitbucket webhook: pullrequest:updated
  3. Push a workflow to the main branch
    
    when:
    event: push

clone: git: image: woodpeckerci/plugin-git settings: partial: false

steps: echo: image: ubuntu commands:

clone: git: image: woodpeckerci/plugin-git settings: partial: false

steps: echo: image: ubuntu commands:

Expected behavior

The clone step should checkout the Pull Request workflow and code instead of checking out the default branch.

System Info

Version 2.6

Additional context

No response

Validations

j04n-f commented 4 months ago

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 required

The 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 plugin-git to ensure it always uses the full SHA: git rev-parse 3cdd5d -> 3cdd5d19178a54d2e51b5098d43b57571241d0ab

WDYT @qwerty287 ? We can create the PRs.

qwerty287 commented 4 months ago

If that fixes the issues, yes, just open the PRs! :)

zc-devs commented 4 months ago

The clone step should checkout the pull request branch

Agree.

and (Woodpecker should) use the pull request workflow

Disagree. Everyone can do anything with your pipeline, including secrets.

qwerty287 commented 4 months ago

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).

j04n-f commented 4 months ago

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:

Disagree. Everyone can do anything with your pipeline, including secrets.

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).

zc-devs commented 4 months ago
  1. I've already thumbed up qwerty (thanks for clarification)
  2. Indeed, it is implemented this way and there is a warning
  3. I stay with my opinion, but it is not related to this issue

So, sorry to bother this thread, go ahead and fix the bug :)

j04n-f commented 4 months ago

@qwerty287 @6543 Whenever you have a moment, can you review https://github.com/woodpecker-ci/plugin-git/pull/160? This is the last PR to close this issue. Thanks!

6543 commented 3 months ago

https://github.com/woodpecker-ci/plugin-git/pull/161 -> 2.5.2 is available

j04n-f commented 3 months ago

Pull Request integration for BB is working as expected now. Thanks!