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

hostRules.readOnly #28375

Closed
rarkins opened this issue Apr 12, 2024 · 10 comments · Fixed by #28562
Closed

hostRules.readOnly #28375

rarkins opened this issue Apr 12, 2024 · 10 comments · Fixed by #28562
Assignees
Labels
priority-2-high Bugs impacting wide number of users or very important features status:in-progress Someone is working on implementation type:feature Feature (new functionality)

Comments

@rarkins
Copy link
Collaborator

rarkins commented Apr 12, 2024

Describe the proposed change(s).

Add a new matcher to hostRules so that the request type can be used e.g. GET, PUT, POST, etc.

This way users could give us a token for github.com which is only used for GET and can't interfere with our POST/PUT/PATCH operations.

@rarkins rarkins added type:feature Feature (new functionality) priority-2-high Bugs impacting wide number of users or very important features labels Apr 12, 2024
@viceice
Copy link
Member

viceice commented Apr 18, 2024

What about graphql post?

@rarkins
Copy link
Collaborator Author

rarkins commented Apr 18, 2024

Good point, we probably want to differentiate between read-only graphql and mutating.

@zharinov
Copy link
Collaborator

Yeah, probably we should name the rule field something more high-level, e.g. readOnly

@rarkins rarkins changed the title hostRules.matchRequestType hostRules.readOnly Apr 18, 2024
@rarkins
Copy link
Collaborator Author

rarkins commented Apr 18, 2024

OK

@rarkins rarkins added the status:in-progress Someone is working on implementation label Apr 19, 2024
@rarkins
Copy link
Collaborator Author

rarkins commented Apr 19, 2024

Goal: be able to support a read-only github.com token which works for release and release notes lookups

@zharinov
Copy link
Collaborator

Goal: be able to support a read-only github.com token which works for release and release notes lookups

This is the reason why I refactored the matching logic: if we specify rules like { matchHost: "api.github.com", readOnly: true, token: "..." }, it should work for all hostRule values in the request, and this logic will be a bit trickier than what we have now.

@rarkins
Copy link
Collaborator Author

rarkins commented Apr 19, 2024

So readOnly=true should "win" over readOnly=false if they're both equally specified? (e.g. github.com). We might need to tweak this a bit to get right because Renovate sets hostType=github for example

@zharinov
Copy link
Collaborator

Yes, I can't describe how this should work exactly, I'll just run this on debugger and will make sure it "wins" for reasonable settings

@rarkins
Copy link
Collaborator Author

rarkins commented Apr 21, 2024

I think readOnly should take higher preference than hostType.

e.g. a github.com readOnly token should be used for all read-only requests, regardless of whether the other token has hostType or a longer host match

And if there's two matching readOnly tokens then the longer host wins, or one with hostType wins. etc

@renovate-release
Copy link
Collaborator

🎉 This issue has been resolved in version 37.319.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority-2-high Bugs impacting wide number of users or very important features status:in-progress Someone is working on implementation type:feature Feature (new functionality)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants