configure-aws-credentials icon indicating copy to clipboard operation
configure-aws-credentials copied to clipboard

Branch name validation too strict

Open simonmumenthaler opened this issue 4 years ago • 12 comments
trafficstars

We name our branches with a specific pattern that starts with a # followed by digits and other chars like #2-bit-ref-r.

But when executing the configure-aws-credentials action it fails since the name does not match the required regex pattern.

1 validation error detected: Value 'refs/heads/#2-bit-ref-r' at 'tags.7.member.value' failed to satisfy constraint: Member must satisfy regular expression pattern: [\p{L}\p{Z}\p{N}_.:/=+\-@]*

Since for github it's a valid branch name it should also be accepted by this action.

simonmumenthaler avatar Aug 06 '21 08:08 simonmumenthaler

I have the same thing today for one of our branches

dejwsz avatar Aug 10 '21 10:08 dejwsz

In our case we had such error: Error: 1 validation error detected: Value 'refs/heads/CFA-94/Image_uploading_prototype_(company_logo)_' at 'tags.7.member.value' failed to satisfy constraint: Member must satisfy regular expression pattern: [\p{L}\p{Z}\p{N}_.:/=+\-@]*

dejwsz avatar Aug 10 '21 10:08 dejwsz

After we changed the branch name it went through without any problems.

dejwsz avatar Aug 10 '21 12:08 dejwsz

@simonmumenthaler Thank you for reporting this issue with us. I tried reproducing this issue on my end and couldn't see the error that you are getting with the same branch name as yours i.e. #2-bit-ref-r Could you please provide me some more information regarding this issue such as some screen shots, steps to reproduce, environment etc.

paragbhingre avatar Aug 12 '21 16:08 paragbhingre

@paragbhingre Thank you for investigating this problem.

I receive the error in different repos. The action works when running on master but fails on our #\d+-[a-z\d-]+ branches.

The config I use for the action:

name: Delete Orphan Stacks
on:
  schedule:
    - cron: '15 2 * * 1'
  workflow_dispatch: 
jobs:
  delete-stack:
    runs-on: ubuntu-latest
    steps:
      - name: Configure AWS Credentials for eu-central-1
        uses: aws-actions/configure-aws-credentials@v1
        with:
          aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }}
          aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
          role-to-assume: 'arn:aws:iam::************:role/ScAccess'
          role-duration-seconds: 3600
          aws-region: eu-central-1
      - name: Delete Stacks in eu-central-1
        uses: shiftcode/[email protected]
        with:
          githubToken: ${{ secrets.GH_TOKEN_REPO_READ }}
          stackNamePrefix: 'pai-website'
          ignoreStacks: '["xx1"]'

Output when running on #001-dev: image

simonmumenthaler avatar Aug 13 '21 06:08 simonmumenthaler

I hit this, I tried overriding GITHUB_REF to a sanitised value but found out that GitHub throws away updates to the GITHUB_* env vars:

https://docs.github.com/en/[email protected]/actions/reference/environment-variables#naming-conventions-for-environment-variables

When you set a custom environment variable, you cannot use any of the default environment variable names listed above with the prefix GITHUB_. If you attempt to override the value of one of these default environment variables, the assignment is ignored.

Rather than disabling session tagging altogether, I made #259 to apply the same sanitation to the branch name that the workflow name gets.

dgholz avatar Sep 14 '21 12:09 dgholz

@dgholz Thank you for your contribution, I was just wondering if changing the name of the branch have any side effects on the workflow? As in if we modify the name of the branch then does it affect/break the workflow at any point as real name of the branch is different than the one we are providing after modifying it?

paragbhingre avatar Sep 15 '21 00:09 paragbhingre

I think changing the branch name is safe. The modified branch name is only used on the session tag, and that's just a 'nice to know' bit of information when looking at them in the console/API.

dgholz avatar Sep 16 '21 09:09 dgholz

I mentioned this in the PR but I'm having trouble reproducing this issue when running from branches containing the # character. Maybe there are more steps to reproduce this than just running this action on a branch with certain characters?

peterwoodworth avatar Oct 05 '22 00:10 peterwoodworth

You're having trouble reproducing because you're using the OIDC token instead of explicit secret access key/access key ID, and the code specifically throws away the session tags when the OIDC token is used https://github.com/aws-actions/configure-aws-credentials/blob/58206600641582a608a0c8b1871ae620318648ca/index.js#L92

dgholz avatar Oct 18 '22 15:10 dgholz

Thanks for getting back @dgholz,

I did use access keys! You can see from my comment in the PR that I was testing using access keys because of this OIDC functionality. I can try to test again later this week, is there anything else you think i could have misconfigured based on my workflow file?

peterwoodworth avatar Oct 19 '22 06:10 peterwoodworth

I see you passed the access keys, but because of

    permissions:
      id-token: write

they are ignored, and the OIDC token is used instead. Take that out and you will reproduce it.

I know it seem impossible from reading the code, since the GitHub OIDC token should only be used if the access key is not set. But that's the only difference I can see between your workflow and my reproduction https://github.com/dgholz/aws-credentials-repro/actions/runs/3282098461/jobs/5405027659#step:2:12

dgholz avatar Oct 19 '22 12:10 dgholz

Thanks for the suggestion, I tried it out and still couldn't get it to fail - Specifying permissions doesn't necessarily mean the action will try to use OIDC. If an access key is provided, that signals to the action to use access keys as authentication https://github.com/aws-actions/configure-aws-credentials/blob/a12c5029930fcd33589131d496622beb4e256718/index.js#L295

However, I finally figured out what I was doing wrong. I was trying to use a pull request from the branch to test it, and it turns out that a PR is considered its own branch, at least for this. TIL. I checked CloudTrail and saw this for the session tag:

            {
                "key": "Branch",
                "value": "refs/pull/45/merge"
            }

So, my bad for hesitating this long due to my own error 😅. Are you willing to reopen the PR for this or make a new one @dgholz? I'm not able to reopen it 🙂

peterwoodworth avatar Oct 19 '22 20:10 peterwoodworth

Are you willing to reopen the PR for this or make a new one @dgholz? I'm not able to reopen it

Most certainly. I think I did something dumb and force-pushed the branch (after rebasing), which stopped it from being re-openable. I've reset the branch, so I think you can re-open it now.

dgholz avatar Oct 20 '22 13:10 dgholz

I have just experienced this when trying to call TagLogGroup with cf:version=bugfix/#46-add-region-runner-tags-to-tms-address-book-api. It looks like this validation is enforced within AWS and the documentation at https://docs.aws.amazon.com/tag-editor/latest/userguide/tagging.html#tag-conventions mentions that hash symbol (#) is not supported in tags, which causes problems when the tag is based on the branch name

Woodz avatar May 05 '23 08:05 Woodz

I have also experienced this issue with a git branch name of AM-22&AM-32|alert_tooltip. While the & and | characters are not crucial to our workflow, it would be nice to make sure that the action is compatible with valid git branch names.

jonathanberube-zylo avatar Aug 03 '23 18:08 jonathanberube-zylo

This should be fixed in v3, thanks for the patience everyone

peterwoodworth avatar Aug 24 '23 22:08 peterwoodworth

** Note ** Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

github-actions[bot] avatar Aug 24 '23 22:08 github-actions[bot]