Skip to content

Use the Actions artifacts client to avoid needing the actions: read permission#288

Merged
JamesMGreene merged 19 commits into
mainfrom
use-artifacts-client
Dec 22, 2023
Merged

Use the Actions artifacts client to avoid needing the actions: read permission#288
JamesMGreene merged 19 commits into
mainfrom
use-artifacts-client

Conversation

@JamesMGreene

@JamesMGreene JamesMGreene commented Dec 20, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

This was referenced Dec 20, 2023
Closed
Closed
Closed
@ntkme

ntkme commented Dec 20, 2023 •
edited
Loading

Copy link
Copy Markdown

@JamesMGreene My own test shows that actions: read does not need to be specified at all, and it was not the issue (things are failing the same way with or without it). The real issue is that createPagesDeployment is calling POST /repos/{owner}/{repo}/pages/deployments with artifact_id, but that API only takes artifact_url, which I'm trying to fix in #287. Using actions artifacts client does not really change that behavior so it likely won't fixes the issue.

@kphrx

kphrx commented Dec 20, 2023

Copy link
Copy Markdown

artifact_id is a new api parameter used from #251. if reverting, maybe unnecessary v4.

@JamesMGreene JamesMGreene mentioned this pull request Dec 20, 2023
Closed
TheMrMilchmann added a commit to TheMrMilchmann/committing-crimes.com that referenced this pull request Dec 21, 2023
@TheMrMilchmann
build: add actions: read permission for deployment
41ab713 
See actions/deploy-pages#285
See actions/deploy-pages#288
@JamesMGreene JamesMGreene marked this pull request as ready for review December 22, 2023 17:41
@JamesMGreene JamesMGreene requested a review from a team as a code owner December 22, 2023 17:41
@JamesMGreene

JamesMGreene commented Dec 22, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Failures from a workflow run using actions/deploy-pages@v4 without also granting actions: read permission:

permissions failure

After switching to use this PR branch for `actions/deploy-pages`, the same workflow succeeded without any errors. 🎉 🚀

yoannchaudet
yoannchaudet reviewed Dec 22, 2023
View reviewed changes
Comment thread src/internal/api-client.js
yoannchaudet
yoannchaudet reviewed Dec 22, 2023
View reviewed changes
Comment thread src/internal/api-client.js Outdated
yoannchaudet
yoannchaudet reviewed Dec 22, 2023
View reviewed changes
Comment thread src/internal/api-client.js Outdated
yoannchaudet
yoannchaudet previously approved these changes Dec 22, 2023
View reviewed changes

@yoannchaudet yoannchaudet left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking comments from me!

@JamesMGreene JamesMGreene mentioned this pull request Dec 22, 2023
Merged
23 tasks
@JamesMGreene JamesMGreene merged commit d8af841 into main Dec 22, 2023
@JamesMGreene JamesMGreene deleted the use-artifacts-client branch December 22, 2023 19:31
@Javagedes Javagedes mentioned this pull request Jan 22, 2024
Closed
@threeal threeal mentioned this pull request Mar 21, 2024
Closed
@coderabbitai coderabbitai Bot mentioned this pull request Jun 17, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yoannchaudet yoannchaudet yoannchaudet approved these changes

Assignees

@JamesMGreene JamesMGreene

@TooManyBees TooManyBees

Labels

None yet

Projects

None yet

Milestone

No milestone

5 participants

@JamesMGreene @ntkme @kphrx @yoannchaudet @TooManyBees