-
Notifications
You must be signed in to change notification settings - Fork 55
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
Have cache invalidation not run on pull requests #688
Conversation
9f9346a
to
2642b80
Compare
Apparently the cache action command got merged into the gh CLI and the old plugin got deprecated yesterday! So updated to new command and fixed the passing of the token. |
2642b80
to
423786a
Compare
.github/workflows/ci.yml
Outdated
permissions: | ||
actions: 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.
Permission write on a PR workflow sounds wrong to me.
Or is "actions: write" special?
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.
It is suppose to be if I am understanding their docs correctly. The cache delete command is getting 403, so I think it needs more perms, so I am trying to add the minimum amount of extra perms. Not sure which permission I am missing.
https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#permissions
https://docs.github.com/en/rest/authentication/permissions-required-for-github-apps?apiVersion=2022-11-28#repository-permissions-for-actions
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.
Ok, I see that the token didn't get the write permission (https://github.com/pulp/pulp-oci-images/actions/runs/11463241947/job/31896626984#step:1:17), probably because this change is coming from a forked repository and default sec op is to not allow granting of write permissions. https://docs.github.com/en/actions/security-for-github-actions/security-guides/automatic-token-authentication#modifying-the-permissions-for-the-github_token. Maybe I should open this PR as a branch in this repository?
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.
But this would still fail with any subsequent PR from a fork. I think we should never give write perms to a pr.
Can we just resort to not using the version from cache when we are in a PR workflow?
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.
Saw this comment after I merged the other one, so I updated this PR to add a check to prevent trying to invalidate the cache when ran in a pull request. This way PRs can take advantage of the cache if present, but will not affect the cache or need special perms.
423786a
to
b60f314
Compare
b60f314
to
3fda76b
Compare
No description provided.