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

Add test to check all repos in Homu's cfg.toml are valid #581

Closed
aneeshusa opened this issue Jan 22, 2017 · 10 comments
Closed

Add test to check all repos in Homu's cfg.toml are valid #581

aneeshusa opened this issue Jan 22, 2017 · 10 comments

Comments

@aneeshusa
Copy link
Member

@aneeshusa aneeshusa commented Jan 22, 2017

This will prevent typos in repo names as well as issues like #580.

Files:

  • Make a new test file tests/sls/homu/valid_repos.py

The test should read the deployed cfg.toml file for Homu (should be at /home/servo/homu/cfg.toml) using the toml Python library. Each subkey under the repo key of the configuration should be queried for existence on GitHub as a valid repo.

Bonus points if you query the GitHub API for the webhooks of each repo to check that an appropriate webhook is set up (see https://github.com/servo/servo/wiki/Adding-a-repo-to-Homu), although I think this might require an new access token with permissions we don't necessarily want to provide.

@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented Jan 22, 2017

This can be tested locally using the servo-master1 VM in Vagrant; look at other tests under tests/sls/homu for similar examples that read the deployed configuration.

@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented Jan 22, 2017

It looks like we'd need the read:repo_hook, which only allows read and ping abilities for webhooks on public and private repos, per the GitHub v3 API docs. I think this is a reasonably small scope to provide to Travis to integrate into CI, since Travis already knows our webhook secret (that's a different secret).

@aneeshusa aneeshusa added the E-easy label Jan 22, 2017
@rwaweber
Copy link
Contributor

@rwaweber rwaweber commented Feb 14, 2017

Hey @aneeshusa, I think I may give this a shot tonight.

Also, mentioned it in #servo, but since I had a tough time grabbing the dependencies (at least on El Capitan) for the vagrant environment, I setup this cask.

@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented Feb 15, 2017

Great @rwaweber, ask if you have any questions.

BTW, did you try installing Vagrant 1.8.1 from the Hashicorp releases or via an older Cask URL (at least, I think you can do brew cask install <some_url>...)?

rwaweber added a commit to rwaweber/saltfs that referenced this issue Feb 18, 2017
Enumerates the inconsistencies between repositories configured in homu
vs those configured in the servo github organization.
@rwaweber
Copy link
Contributor

@rwaweber rwaweber commented Feb 18, 2017

I've got a WIP here.

A couple things:

  1. Just making sure that I'm not breaking anything, since I'm only developing with the single servo-master1 machine, it shouldn't be able to retrieve the builders. This error was what I was getting and led me to that conclusion.
  2. I'm open to a different means than importing requests to query github for repos to avoid adding another dependency.

Reminder for myself to look into the pagination part of the github API. In the current state, a curl https://api.github.com/orgs/servo/repos will retrieve a subset of repositories, not all of them. Currently 123 repos are configured for the servo org from what I can tell.

@rwaweber
Copy link
Contributor

@rwaweber rwaweber commented Feb 24, 2017

Pagination is configured, though I ended up rate-limiting myself a couple times during testing. As a result, I implemented a check for a token variable to allow for authenticated requests. I could imagine that being rate-limited by the github api would be less than ideal 😅

@aneeshusa not sure what the best means of retrieving that could be, maybe yanking something from pillar? Is there a token thats available?

I still need to spend some time thinking about the webhook bit as well.

@jdm
Copy link
Member

@jdm jdm commented Mar 15, 2017

@aneeshusa Ping to reply to the previous question.

@rwaweber
Copy link
Contributor

@rwaweber rwaweber commented Mar 18, 2017

Spent a bit of time investigating the webhook check, and the earlier assumptions are correct that it would be necessary to issue an auth token with elevated access.

Also, poking around some of the configurations, I noticed that there's a line in the cfg.toml at /home/servo/homu/ that seems to be referring to a "TEST_HOMU_GH_ACCESS_TOKEN". Is there a way that I would be able to refer to that value in these tests like I attempted to do here?

@rwaweber
Copy link
Contributor

@rwaweber rwaweber commented May 3, 2017

We determined that a Get request to each repo url was sufficient and ultimately didn't warrant an access token.
I think that this could probably be closed now, right?

@aneeshusa
Copy link
Member Author

@aneeshusa aneeshusa commented May 3, 2017

Yep, this can be closed for now. We can open another issue for the bonus points task at another time if that becomes necessary.

@aneeshusa aneeshusa closed this May 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.