Skip to content
New issue

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

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

Already on GitHub? # to your account

Add permissions to website deployment job #4285

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

jackkoenig
Copy link
Contributor

I think chipsalliance changed something about default repo settings and now GITHUB_TOKEN only defaults to read permissions. We need to be more explicit with permissions to ensure various automation can continue working. I noticed the website deployment started failing but it is likely that other automation jobs will need similar permissions (publishing release artifacts, release notes, etc.).

Contributor Checklist

  • Did you add Scaladoc to every public function/method?
  • Did you add at least one test demonstrating the PR?
  • Did you delete any extraneous printlns/debugging code?
  • Did you specify the type of improvement?
  • Did you add appropriate documentation in docs/src?
  • Did you request a desired merge strategy?
  • Did you add text to be included in the Release Notes for this change?

Type of Improvement

  • Internal or build-related (includes code refactoring/cleanup)

Desired Merge Strategy

  • Squash

Release Notes

Reviewer Checklist (only modified by reviewer)

  • Did you add the appropriate labels? (Select the most appropriate one based on the "Type of Improvement")
  • Did you mark the proper milestone (Bug fix: 3.6.x, 5.x, or 6.x depending on impact, API modification or big change: 7.0)?
  • Did you review?
  • Did you check whether all relevant Contributor checkboxes have been checked?
  • Did you do one of the following when ready to merge:
    • Squash: You/ the contributor Enable auto-merge (squash), clean up the commit message, and label with Please Merge.
    • Merge: Ensure that contributor has cleaned up their commit history, then merge with Create a merge commit.

@jackkoenig jackkoenig added the Internal Internal change, does not affect users, will be included in release notes label Jul 18, 2024
@jackkoenig jackkoenig added this to the 3.6.x milestone Jul 18, 2024
@jackkoenig jackkoenig merged commit a074f7d into main Jul 19, 2024
18 checks passed
@jackkoenig jackkoenig deleted the jackkoenig/fix-website-deploy branch July 19, 2024 03:32
@mergify mergify bot added the Backported This PR has been backported label Jul 19, 2024
mergify bot pushed a commit that referenced this pull request Jul 19, 2024
(cherry picked from commit a074f7d)

# Conflicts:
#	.github/workflows/ci.yml
mergify bot pushed a commit that referenced this pull request Jul 19, 2024
(cherry picked from commit a074f7d)

# Conflicts:
#	.github/workflows/ci.yml
mergify bot pushed a commit that referenced this pull request Jul 19, 2024
chiselbot pushed a commit that referenced this pull request Jul 19, 2024
(cherry picked from commit a074f7d)

Co-authored-by: Jack Koenig <koenig@sifive.com>
@bensternthal
Copy link

@jackkoenig So, I upgraded our Github organization yesterday (which would have happened automatically in a few weeks).

I did not change any permissions, but what I did change was to remove some of the folks who were admins over the entire CHIPS enterprise account. I did not expect this to cause issues for individual projects.

To me I think this means I probably need to audit our organization and how it is setup. You should not need to be an admin over the entire org to administer a project.

Let me know if you need to be added back as as admin or if the above has solved the problem.

@jackkoenig
Copy link
Contributor Author

@bensternthal Thanks for the info!

I did some poking around and I don't think you did anything wrong or need to audit anything. I think this is just a setting that is stricter in Github Enterprise, see the screenshot below.

Screenshot 2024-07-24 at 1 17 27 PM

We used to have this set to Read and write permissions which, honestly, is not a great idea to have as your default setting. I think having to enable greater permissions per job that needs them is a good thing so I don't think any action is needed on your end (and I'm not sure if it's even possible to change this setting, I certainly wouldn't suggest doing so if it is!). Thank you for following up!

@bensternthal
Copy link

@jackkoenig Thanks for tagging me in on this. I ran into a similar issue on the chipsalliance website github action and your response saved me quite a bit of time hunting around for the fix!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Backported This PR has been backported Internal Internal change, does not affect users, will be included in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants