Support Authenticated GitHub Requests in Setup

[travisgosselin]

Using this container provides a great deal of portability, but after a certain amount of usage we ran into GitHub rate limiting in setup.py on container start:

Traceback (most recent call last):
  File "/usr/local/startup_scripts/setup.py", line 57, in <module>
    setup()
  File "/usr/local/startup_scripts/setup.py", line 39, in setup
    get_latest_codeql(args)
  File "/usr/local/startup_scripts/setup.py", line 46, in get_latest_codeql
    latest_online_version = codeql.get_latest_codeql_github_version()
  File "/usr/local/startup_scripts/libs/codeql.py", line 80, in get_latest_codeql_github_version
    return get_latest_github_repo_version("github/codeql-cli-binaries")
  File "/usr/local/startup_scripts/libs/github.py", line 8, in get_latest_github_repo_version
    latest_release = get_latest_github_release(releases)
  File "/usr/local/startup_scripts/libs/github.py", line 14, in get_latest_github_release
    for release in releases:
  File "/usr/local/lib/python3.8/dist-packages/github/PaginatedList.py", line 64, in __iter__
    newElements = self._grow()
  File "/usr/local/lib/python3.8/dist-packages/github/PaginatedList.py", line 76, in _grow
    newElements = self._fetchNextPage()
  File "/usr/local/lib/python3.8/dist-packages/github/PaginatedList.py", line 197, in _fetchNextPage
    headers, data = self.__requester.requestJsonAndCheck(
  File "/usr/local/lib/python3.8/dist-packages/github/Requester.py", line 275, in requestJsonAndCheck
    return self.__check(*self.requestJson(verb, url, parameters, headers, input, self.__customConnection(url)))
  File "/usr/local/lib/python3.8/dist-packages/github/Requester.py", line 286, in __check
    raise self.__createException(status, responseHeaders, output)
github.GithubException.RateLimitExceededException: 403 {'message': "API rate limit exceeded for x.x.x.x. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)", 'documentation_url': 'https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting'}
Error 1 executing from command.
Exiting...
Command Output: 
[2022-12-07 15:14:37,788] INFO: Starting setup...
[2022-12-07 15:14:38,234] INFO: Current codeql version: v2.11.3

Our execution commands were all passing the GITHUB_TOKEN environment variable to docker run, as the intent was to also upload the results to a private GitHub repository. However, we have realized that the instantiation of PyGithub client doesn't pass in or make use of the same env variable for any authentication on requests to public repos to validate the CodeQL CLI version:
https://github.com/microsoft/codeql-container/blob/main/container/libs/github.py#L5

Within our organization, the rate-limiting by IP Address is only going to get worse, and there is no reason for us to not authenticate for these requests.

This Pull Request does the following:

• Moves the check for the latest CodeQL CLI version to occur only if CHECK_LATEST_CODEQL_CLI is true (whereas before it always checked and left the value unused).
• Checks for an environment variable called "GITHUB_TOKEN" and passes it into the PyGithub ctor to allow for authenticated requests.

The changes are pretty small, but I'm not sure next steps for testing/validation cross-platform, or if that is included in a build process.

[travisgosselin]

@microsoft-github-policy-service agree [company="SPS Commerce"]

[travisgosselin]

@microsoft-github-policy-service agree company="SPS Commerce"

[travisgosselin]

@jacobmsft just checking in to see if you think there is an opportunity to pull this into main ? This is a pretty big problem for us, and this would be fantastic to get it solved here.

[sargemonkey]

Hi travisgosselin,

Thank you for the PR and the detailed description! I think it definitely makes sense to merge to main. I am stuck on account access (and hence commenting from my alternate login) issues at the moment, hopefully that gets resolved in a couple days and I can merge it.

[travisgosselin]

Thanks @surajjacob

[travisgosselin]

Thank you for the PR and the detailed description! I think it definitely makes sense to merge to main. I am stuck on account access (and hence commenting from my alternate login) issues at the moment, hopefully that gets resolved in a couple days and I can merge it.

Hi @surajjacob / @jacobmsft - any luck in sorting out permissions to be able to merge this?

[travisgosselin]

Thank you for the PR and the detailed description! I think it definitely makes sense to merge to main. I am stuck on account access (and hence commenting from my alternate login) issues at the moment, hopefully that gets resolved in a couple days and I can merge it.

Hi @surajjacob / @jacobmsft - any luck in sorting out permissions to be able to merge this?

Tagging you again @surajjacob / @jacobmsft just in case you see this notification and by chance have permission to merge this in. My organization really badly needs this to continue using this container distribution. Will look at building our own custom version, but would really like to have this issue taken care of in the base here.

[sargemonkey]

Thank you for the PR and the detailed description! I think it definitely makes sense to merge to main. I am stuck on account access (and hence commenting from my alternate login) issues at the moment, hopefully that gets resolved in a couple days and I can merge it.

Hi @surajjacob / @jacobmsft - any luck in sorting out permissions to be able to merge this?

Tagging you again @surajjacob / @jacobmsft just in case you see this notification and by chance have permission to merge this in. My organization really badly needs this to continue using this container distribution. Will look at building our own custom version, but would really like to have this issue taken care of in the base here.

Let me try getting someone else to merge your changes

[travisgosselin]

That would be very much appreciated. Thank you!

[travisgosselin]

@surajjacob / @jacobmsft / @scovetta - thanks for getting this merged in. I pulled down the latest available container in MCR registry and these updates are not in it. Is there a manual process behind the scenes to push updates there?

Struggling a little in general with the visibility and versions in MCR - #45

[sargemonkey]

@surajjacob / @jacobmsft / @scovetta - thanks for getting this merged in. I pulled down the latest available container in MCR registry and these updates are not in it. Is there a manual process behind the scenes to push updates there?

Struggling a little in general with the visibility and versions in MCR - #45

Hey @travisgosselin , the mcr images are updated every Sunday, you should get it in next week's update. Alternatively, you can clone repository and do docker build.
Visibility and versions are something we need to add more in MCR, I need to figure out what to do for it.

[travisgosselin]

Thanks @surajjacob - that helps knowing the schedule for the MCR builds to know when they drop. Is that on a schedule to run even if no changes to the code base? Just wondering if that schedule implies new query pack releases and precompilation of them every Sunday as well on the latest container?

[travisgosselin]

@surajjacob / @jacobmsft / @scovetta - I'm pulling latest on this image from mcr.microsoft.com/cstsectools/codeql-container:latest and still not seeing any updates. Looks like the container is about 3 months old. Are you sure this is running weekly to update the latest with the newest queries / CLI version?

Also not seeing the changes in this PR or others merged in there, so assuming something has gone wrong with the weekly update. Using the weekly update as a base is very helpful not to rebuild everything.