-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
MINOR: Create GitHub action to update NOTICE file automatically every year #18380
base: trunk
Are you sure you want to change the base?
Conversation
@mumrah would you like to take a look at this since you are familiar with the workflows for our repository. |
contents: write | ||
pull-requests: write |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note for reviewers
context: write
is required to push commit to the new branch created below
pull-requests: write
is required to create a pull request
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @divijvaidya! Some comments inline.
run: | | ||
git config user.name "github-actions" | ||
git config user.email "[email protected]" | ||
git remote set-url origin https://x-access-token:${{ secrets.GITHUB_TOKEN }}@github.com/${{ github.repository_owner }}/${{ github.event.repository.name }}.git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this. Check how we're updating the test-catalog branch here https://github.com/apache/kafka/blob/trunk/.github/workflows/build.yml#L269-L284
Also see the "magic" user.email in that code. I think this is still needed for commit to be correctly associated with the github actions bot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link you provided uses persist-credentials: true
which is not recommended by apache as per https://cwiki.apache.org/confluence/display/BUILDS/GitHub+Actions+Security
if using the 'checkout' action, always enable persist-credentials: false
Since, we use persist-credentials: false
, we need a way to provide credential during git push
for this action. That is why I used this alternative approach of setting origin with credentials.
I will change the user.email and user.name to the "magic" user mentioned at https://github.com/actions/checkout?tab=readme-ov-file#push-a-commit-using-the-built-in-token
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think setting the origin URL like this is essentially the same as persisting the credentials. However, I think either this approach or persist-credentials: true
is fine for this workflow since it's not running any user code (like a PR).
base: "trunk", | ||
body: `This PR is auto-generated to update the NOTICE file on Jan 1st every year.` | ||
}); | ||
core.info(`Pull Request created: #${response.data.number}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a label like needs-attention
maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woudn't the existing logic to add the label needs-attention
after a few days on this PR?
@mumrah ready for review again |
All this code, automation to update year in one file? |
Change
Create a GitHub action that will update the copyright year automatically in the NOTICE file.
Testing
Tested the workflow in my fork which generated the PR divijvaidya#4
Permissions required
This action requires the following permissions:
Reference
Apache Infra github actions policy - https://infra.apache.org/github-actions-policy.html. I have read through this and looks like we should be good here with this script. Please review again in case I missed something.