search

christophebedard / dco-check

Simple DCO check script to be used in any CI
Apache License 2.0
12 stars 6 forks source link

GitLab integration fails with detached pipelines for mere requests #104

Open zyga opened 3 years ago

zyga commented 3 years ago

Our project is using pipelines for merge requests. This feature is documented at https://docs.gitlab.com/ee/ci/merge_request_pipelines/

In the default recommended setup, dco-check 0.0.11 fails as follows:

dco-check
Detected: GitLab
could not get environment variable: 'CI_COMMIT_BRANCH'

Some extra debugging shows more information, interestingly CI_COMMIT_BRANCH is obviously missing and CI_MERGE_REQUEST_EVENT_TYPE=detached indicates that this is a detached merge request.

I think that in this mode, it's sufficient to use CI_COMMIT_REF_NAME instead.

env | grep CI_ | sort
CI_API_V4_URL=https://git.ostc-eu.org/api/v4
CI_BUILDS_DIR=/builds
CI_BUILD_BEFORE_SHA=0000000000000000000000000000000000000000
CI_BUILD_ID=3543
CI_BUILD_NAME=dco
CI_BUILD_REF=d08a62cd72976257694e1598ad1c5bf6520f2034
CI_BUILD_REF_NAME=feature/dco
CI_BUILD_REF_SLUG=feature-dco
CI_BUILD_STAGE=compliance
CI_BUILD_TOKEN=[MASKED]
CI_COMMIT_BEFORE_SHA=0000000000000000000000000000000000000000
CI_COMMIT_DESCRIPTION=
CI_COMMIT_MESSAGE=Add CI job checking DCO
CI_COMMIT_REF_NAME=feature/dco
CI_COMMIT_REF_PROTECTED=false
CI_COMMIT_REF_SLUG=feature-dco
CI_COMMIT_SHA=d08a62cd72976257694e1598ad1c5bf6520f2034
CI_COMMIT_SHORT_SHA=d08a62cd
CI_COMMIT_TIMESTAMP=2021-03-04T15:54:38+01:00
CI_COMMIT_TITLE=Add CI job checking DCO
CI_CONCURRENT_ID=0
CI_CONCURRENT_PROJECT_ID=0
CI_CONFIG_PATH=.ostc-ci/gitlab-ci.yml
CI_DEFAULT_BRANCH=develop
CI_DEPENDENCY_PROXY_GROUP_IMAGE_PREFIX=git.ostc-eu.org:443/OSTC/dependency_proxy/containers
CI_DEPENDENCY_PROXY_PASSWORD=[MASKED]
CI_DEPENDENCY_PROXY_SERVER=git.ostc-eu.org:443
CI_DEPENDENCY_PROXY_USER=gitlab-ci-token
CI_DISPOSABLE_ENVIRONMENT=true
CI_JOB_ID=3543
CI_JOB_IMAGE=registry.ostc-eu.org/ostc/containers/dco-check:latest
CI_JOB_JWT=[MASKED]
CI_JOB_NAME=dco
CI_JOB_STAGE=compliance
CI_JOB_STATUS=running
CI_JOB_TOKEN=[MASKED]
CI_JOB_URL=https://git.ostc-eu.org/OSTC/OHOS/meta-ohos/-/jobs/3543
CI_KUBERNETES_ACTIVE=true
CI_MERGE_REQUEST_DIFF_BASE_SHA=efc6c221c0984c42dc840c54a4554d95c1c68ece
CI_MERGE_REQUEST_DIFF_ID=1381
CI_MERGE_REQUEST_EVENT_TYPE=detached
CI_MERGE_REQUEST_ID=437
CI_MERGE_REQUEST_IID=46
CI_MERGE_REQUEST_PROJECT_ID=92
CI_MERGE_REQUEST_PROJECT_PATH=OSTC/OHOS/meta-ohos
CI_MERGE_REQUEST_PROJECT_URL=https://git.ostc-eu.org/OSTC/OHOS/meta-ohos
CI_MERGE_REQUEST_REF_PATH=refs/merge-requests/46/head
CI_MERGE_REQUEST_SOURCE_BRANCH_NAME=feature/dco
CI_MERGE_REQUEST_SOURCE_BRANCH_SHA=
CI_MERGE_REQUEST_SOURCE_PROJECT_ID=92
CI_MERGE_REQUEST_SOURCE_PROJECT_PATH=OSTC/OHOS/meta-ohos
CI_MERGE_REQUEST_SOURCE_PROJECT_URL=https://git.ostc-eu.org/OSTC/OHOS/meta-ohos
CI_MERGE_REQUEST_TARGET_BRANCH_NAME=develop
CI_MERGE_REQUEST_TARGET_BRANCH_SHA=
CI_MERGE_REQUEST_TITLE=Draft: Add CI job checking DCO
CI_NODE_TOTAL=1
CI_OPEN_MERGE_REQUESTS=OSTC/OHOS/meta-ohos!46
CI_PAGES_DOMAIN=example.com
CI_PAGES_URL=http://ostc.example.com/OHOS/meta-ohos
CI_PIPELINE_ID=1418
CI_PIPELINE_IID=113
CI_PIPELINE_SOURCE=merge_request_event
CI_PIPELINE_URL=https://git.ostc-eu.org/OSTC/OHOS/meta-ohos/-/pipelines/1418
CI_PROJECT_CONFIG_PATH=.ostc-ci/gitlab-ci.yml
CI_PROJECT_DIR=/builds/OSTC/OHOS/meta-ohos
CI_PROJECT_ID=92
CI_PROJECT_NAME=meta-ohos
CI_PROJECT_NAMESPACE=OSTC/OHOS
CI_PROJECT_PATH=OSTC/OHOS/meta-ohos
CI_PROJECT_PATH_SLUG=ostc-ohos-meta-ohos
CI_PROJECT_REPOSITORY_LANGUAGES=bitbake,shell,c++
CI_PROJECT_ROOT_NAMESPACE=OSTC
CI_PROJECT_TITLE=meta-ohos
CI_PROJECT_URL=https://git.ostc-eu.org/OSTC/OHOS/meta-ohos
CI_PROJECT_VISIBILITY=public
CI_REGISTRY=registry.ostc-eu.org
CI_REGISTRY_IMAGE=registry.ostc-eu.org/ostc/ohos/meta-ohos
CI_REGISTRY_PASSWORD=[MASKED]
CI_REGISTRY_USER=gitlab-ci-token
CI_REPOSITORY_URL=https://gitlab-ci-token:[MASKED]@git.ostc-eu.org/OSTC/OHOS/meta-ohos.git
CI_RUNNER_DESCRIPTION=gitlab-runner-gitlab-runner-66758ff899-w9vsm
CI_RUNNER_EXECUTABLE_ARCH=linux/amd64
CI_RUNNER_ID=9
CI_RUNNER_REVISION=8fa89735
CI_RUNNER_SHORT_TOKEN=uKNjDYdc
CI_RUNNER_TAGS=
CI_RUNNER_VERSION=13.6.0
CI_SERVER=yes
CI_SERVER_HOST=git.ostc-eu.org
CI_SERVER_NAME=GitLab
CI_SERVER_PORT=443
CI_SERVER_PROTOCOL=https
CI_SERVER_REVISION=a1a60e9f753
CI_SERVER_TLS_CA_FILE=/builds/OSTC/OHOS/meta-ohos.tmp/CI_SERVER_TLS_CA_FILE
CI_SERVER_URL=https://git.ostc-eu.org
CI_SERVER_VERSION=13.9.0-ee
CI_SERVER_VERSION_MAJOR=13
CI_SERVER_VERSION_MINOR=9
CI_SERVER_VERSION_PATCH=0
zyga commented 3 years ago

This seems to be related to https://gitlab.com/gitlab-org/gitlab/-/issues/288876

zyga commented 3 years ago

So, I think there are two separate changes that need to happen.

First, GitLabRetriever.get_commit_range can switch from CI_COMMIT_BRANCH to CI_COMMIT_REF_NAME which is available at all times: https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/ci/variables/predefined_variables.md

Second, inside the same function, when CI_MERGE_REQUEST_ID is set, we should consider CI_MERGE_REQUEST_EVENT_TYPE to see if it is 'detached'. In that mode CI_MERGE_REQUEST_TARGET_BRANCH_NAME is set but the corresponding CI_MERGE_REQUEST_TARGET_BRANCH_SHA is empty. The same restriction applies to CI_MERGE_REQUEST_SOURCE_BRANCH_SHA. Instead of using the precise values, we can use the symbolic names and let git do the hard work, by calling git rev-parse.

zyga commented 3 years ago

The following GitLab pipeline script is a viable workaround:

    - |
        if [ "${CI_MERGE_REQUEST_EVENT_TYPE:-}" = detached ]; then
            git fetch -a  # so that we can resolve branch names below
            export CI_COMMIT_BRANCH="$CI_COMMIT_REF_NAME";
            export CI_MERGE_REQUEST_SOURCE_BRANCH_SHA="$(git rev-parse "origin/$CI_MERGE_REQUEST_SOURCE_BRANCH_NAME")";
            export CI_MERGE_REQUEST_TARGET_BRANCH_SHA="$(git rev-parse "origin/$CI_MERGE_REQUEST_TARGET_BRANCH_NAME")";
        fi
    - dco-check --default-branch-from-remote --verbose
christophebedard commented 3 years ago

Interesting. Your proposed fix explanation makes sense.

If you don't mind, could you open a PR? I'm not sure if I can test with an actual detached pipeline since it's a premium feature.

zyga commented 3 years ago

@christophebedard yeah, yesterday it took me a while to distill the problem. The one thing that worries me is the apparent need to git fetch origin to resolve the branch names. If that's an acceptable (costly) element then I think we can go ahead with a Python version of the small shell snippet I've posted above.

christophebedard commented 3 years ago

@zyga as long as it's only used/called when needed (for detached pipelines) then I think it's fine.