Request Github token from Taskcluster's auth service - #825
Conversation
This is the first version that implements the auth service's ability to hand out Github tokens.
|
This still needs:
Though I might land this before steps 2 and 3 just to verify the fallback is working. |
…possible If there's an error (e.g missing scopes or app not installed on the target repo), we fallback to the existing github_oauth_token.
56430bf to
94eff61
Compare
| auth = taskcluster.Auth(options={"rootUrl": context.config["taskcluster_root_url"], "credentials": context.credentials}) | ||
| response = auth.githubRepoToken(self.GITHUB_APP_NAME, owner, payload={"repositories": [repo_name], "permissions": self.GITHUB_PERMISSIONS}) |
There was a problem hiding this comment.
Any reason to not use the async version here? AFAIK, everything else in scriptworker is, except for this now which would block the event loop (and it'd block on that call which ends with a roundtrip to github, which we know is going to be really fast all the time 🤡 )
| auth = aiohttp.BasicAuth(context.config["github_oauth_token"]) | ||
| url_hash = hashlib.sha1(source_url.encode("ascii")).hexdigest() | ||
| tmpl = await load_json_or_yaml_from_url(context, source_url, os.path.join(context.config["work_dir"], "{}_taskcluster.yml".format(url_hash)), auth=auth) | ||
| return tmpl |
There was a problem hiding this comment.
Did you keep this one out of scope on purpose? Or just missed it?
There was a problem hiding this comment.
Missed it as it wasn't using GithubRepository
| github_repo = GitHubRepository(owner=repo_definition["parent"]["owner"]["login"], repo_name=repo_definition["parent"]["name"], token=token) | ||
| github_repo = GitHubRepository(context, repo_definition["parent"]["owner"]["login"], repo_definition["parent"]["name"]) |
There was a problem hiding this comment.
Isn't this going to be a problem? If I open a PR from github.com/Eijebong/foo for github.com/mozilla-releng/foo, the task won't have scopes to get a read token for Eijebong/foo and the tc-auth token request will fail 100% of the time.
I'm not sure how we can do that but we probably want to use a read token minted for the parent repo and use that instead? AFAIK that'd work for public repos but not private ones though (although all fork commits are accessible on the parent directly, maybe that's enough to make this whole branch useless?).
There was a problem hiding this comment.
Thinking more about this, I think the private repo part of this is the same anyway since the token passed a secret wouldn't have access to it either
| """Wrapper around GitHub API. Used to access public data.""" | ||
|
|
||
| def __init__(self, owner, repo_name, token=""): | ||
| GITHUB_APP_NAME = "read" |
There was a problem hiding this comment.
FTR this would've annoyed me when I was using scriptworker as a third party user :p Don't know if we want to care but maybe there should be a way to configure this that doesn't require monkeypatching the class
There was a problem hiding this comment.
Good point, doesn't hurt! It can definitely go in scriptworker config
2 participants
No description provided.