Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(actions): removed token from remaining actions #805

Merged
merged 1 commit into from
Apr 26, 2022

Conversation

dreamorosi
Copy link
Contributor

Description of your changes

As of recently, the on-merge-to-main workflow has been failing in the very first step (actions/checkout@v3) (example). I have tried to run it again multiple times and then was able to track down the issue to a recent change in the action’s repo (PR).

Based on the content of that PR and related discussion, the GH action introduced a new default value around the git config safe.directory setting (default true) that seems to be what’s breaking our workflow, as well as others (one and two).

I have tried this in a fork, and setting the config to * (aka no check for safe.directory) fixes the issue (link).

Just for context, the docs for that config say this:

safe.directory
These config entries specify Git-tracked directories that are considered safe even if they are owned by someone other than the current user. By default, Git will refuse to even parse a Git config of a repository owned by someone else, let alone run its hooks, and this config setting allows users to specify exceptions, e.g. for intentionally shared repositories (see the --shared option in git-init[1]).
This is a multi-valued setting, i.e. you can add more than one directory via git config --add. To reset the list of safe directories (e.g. to override any such directories specified in the system config), add a safe.directory entry with an empty value.

Even though the risk of disabling the git check is acceptable, while making the changes for this PR I have noticed that other workflows like pr-lint-and-test (who also use that same GH Action) have kept running and not erroring (example).

The main, and only, difference between the two workflow and how they use actions/checkout@v3 is the one (the failing one) used to set a specific GitHub secret. This secret was there for historical reasons as the initial version of the workflow was set up when the repo was still private - hence the need for a token.

Before setting the safe.directory config to allow all I would like, with this PR, to try to remove the token first. If my understanding of the issue is correct then this workflow should start working again (@ijemmy you were right on Slack as for where the actual issue was) since with no token, the ownership/user will be the default one like in the other workflow that are still working.

How to verify this change

I have already tried this config on a forked repo: https://github.com/dreamorosi/aws-lambda-powertools-typescript/runs/6160173289?check_suite_focus=true

But the only way to see it (AFAIK) is to merge and see the result. If this doesn't work then I'll roll-back & set the wildcard.

Related issues, RFCs

N/A

PR status

Is this ready for review?: YES
Is it a breaking change?: NO

Checklist

Breaking change checklist

N/A


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@dreamorosi dreamorosi added automation This item relates to automation internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.) labels Apr 25, 2022
@dreamorosi dreamorosi added this to the production-ready-release milestone Apr 25, 2022
@dreamorosi dreamorosi self-assigned this Apr 25, 2022
@github-actions github-actions bot added the bug Something isn't working label Apr 25, 2022
@dreamorosi dreamorosi merged commit 4fd9ecb into main Apr 26, 2022
@dreamorosi dreamorosi deleted the fix/aamorosi/fix_gh_actions branch April 26, 2022 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation This item relates to automation bug Something isn't working internal PRs that introduce changes in governance, tech debt and chores (linting setup, baseline, etc.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants