Skip to content

Conversation

@jackhodgkiss
Copy link
Collaborator

No description provided.

Previously users of the role could not adjust the templates due
`github_kayobe_*_input` using YAML anchors. This has now beeen replaced
with a scalar block string.
Both `KAYOBE_AUTOMATION_SSH_PRIVATE_KEY` and `KAYOBE_VAULT_PASSWORD`
were missing from the workflow definitions and thus leading to broken
workflows.
@jackhodgkiss jackhodgkiss requested a review from a team as a code owner August 23, 2023 10:06
@jackhodgkiss jackhodgkiss marked this pull request as draft August 23, 2023 10:07
@markgoddard
Copy link

This could really use a basic test that at least runs the role.

permissions:
contents: read
packages: read
packages: write

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it need write permission?

technowhizz and others added 6 commits September 5, 2023 16:35
Uses a separate directory for building the docker image and running
the other workflows. This is because the user the is used by the docker
git image is root which results in permission issues when the workflows
try to run. This change means that the directories do not overlap,
avoiding any permission issues.

Co-Authored-By: Will Szumski <will@stackhpc.com>
Pins versions for buildkit and buildx to the current latest working
version to avoid futures issues.

Co-authored-by: Will Szumski <will@stackhpc.com>
Adds a flag to disable build attestations/provenance. This causes issues
on some clouds and so is defaulted to `false`. Whether or not it is
enabled can be chaged with the github_buildx_enable_provenance variable.

Co-authored-by: Will Szumski <will@stackhpc.com>
Co-authored-by: Jack Hodgkiss <identity@jackhodgkiss.uk>
Co-authored-by: Jack Hodgkiss <identity@jackhodgkiss.uk>
Co-authored-by: Jack Hodgkiss <identity@jackhodgkiss.uk>
@jackhodgkiss jackhodgkiss marked this pull request as ready for review September 18, 2023 08:01
@Alex-Welsh
Copy link
Member

Do we need to check the branch protection on this repo? seems like it's a bit open at the moment

@jackhodgkiss
Copy link
Collaborator Author

Do we need to check the branch protection on this repo? seems like it's a bit open at the moment

Resolved.

permissions:
contents: read
packages: read
contents: write

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this for creating PRs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is for pushing any changes back to the repository. Any PRs are opened manually.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manually? What about the stuff in kayobe-automation for opening PRs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will investigate and update shortly. However, from my experience if any changes get staged it will print a link that when you click on fills out all the PR information and then you open the PR.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it should create a PR:

function github_pull_request {
    local branch_name="$(pull_request_branch_name)"

    _git_commit "${branch_name}"

    local body="{
        \"head\": \"${branch_name}\",
        \"base\": \"${KAYOBE_AUTOMATION_PR_TARGET_BRANCH}\",
        \"title\": \"${KAYOBE_AUTOMATION_PR_TITLE}\"
    }"

    log_debug "Pull request body: $body"

    # Support 0Auth token? Current method requires a personal access token, but
    # we might have access to 0Auth token in the gitlab runner? Recommended to
    # to setup a service account otherwise.
    # --header "Authorization: token ${KAYOBE_AUTOMATION_PR_AUTH_TOKEN}" \

    log_info "Submitting merge request to: $KAYOBE_AUTOMATION_PR_URL"

    # https://docs.github.com/en/rest/reference/pulls
    curl -X POST "$KAYOBE_AUTOMATION_PR_URL" \
        --user "${KAYOBE_AUTOMATION_PR_GITHUB_USER}:${KAYOBE_AUTOMATION_PR_AUTH_TOKEN}" \
        --header "Accept: application/vnd.github.v3+json" \
        --header "Content-Type: application/json" \
        --data "${body}" | jq "."

    log_info "Pull request created sucessfully"
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a quick test I have realised what has been occuring. On the rare occassion where a service deploy or similar has changes to cmmit it will attempt to open a PR after pushing the changes to a new branch.

It will not fail if it cannot either due to lack of permissions on the token or repository wide settings preventing Actions from openning a PR.

Without

permissions:
  pull-requests: write

You will silently get the following error.

Resource not accessible by integration

Once applied you must also enable support for Actions to open PRs within the repository settings under Settings -> Actions -> General -> Workflow Permissions. Otherwise you silently get the following error.

{
  "message": "GitHub Actions is not permitted to create or approve pull requests.",
  "documentation_url": "https://docs.github.com/rest/pulls/pulls#create-a-pull-request"
}

Whether set appropriately or not the workflow should be capable of pushing to the repository which in the logs will contain a link to open a PR.

Will update template to include support for this. Thanks.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed write up. GitHub API does claim to provide a success/fail response: https://docs.github.com/en/free-pro-team@latest/rest/pulls/pulls?apiVersion=2022-11-28#create-a-pull-request--status-codes

But it appears we are not failing in kayobe-automation when we get a 403. I've proposed a PR to address this here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just to tie this off, we should only assign write permission when:

'KAYOBE_AUTOMATION_PR_TYPE' in workflow.arguments

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New condition looks good, but need a similar one for content

Copy link

@markgoddard markgoddard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work, looking good!

@jackhodgkiss jackhodgkiss merged commit b4a8545 into main Sep 28, 2023
@jackhodgkiss jackhodgkiss deleted the github_role_refactor branch September 28, 2023 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants