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

Support private Hydra projects #74

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Ma27
Copy link

@Ma27 Ma27 commented Nov 13, 2021

Description

WIP on purpose as it doesn't make sense to merge this without NixOS/hydra#919

Also, before we merge this, it needs some final polishing (see also the unchecked items below).

Checklist
  • Built with make build
  • Formatted with make fmt (goimports doesn't seem to exist when I run nix-shell even though nixpkgs is pinned)
  • Verifed the example configuration still parses with terraform init && terraform validate (you may need to make install from the root of the project) (not yet, but I verified it with a dump of my Hydra settings that I applied against localhost:63333)
  • Ran acceptance tests with HYDRA_HOST=http://0.0.0.0:63333 HYDRA_USERNAME=alice HYDRA_PASSWORD=foobar make testacc (you will need to spin up a local / temporary instance of Hydra)
  • Added or updated relevant documentation (leave unchecked if not applicable)

Ma27 added 3 commits November 13, 2021 22:16
…rojects into account

This is needed for NixOS/hydra#919[1], private projects can only be
accessed as authenticated users (this also applies to jobsets). Without
this flag, only public projects will be used to generate Terraform code.

[1] NixOS/hydra#919
Rather than fetching `hydra-api.yaml` from `master`, I used it from my
PR's branch[1] which should be reasonably up-to-date after I merged in
latest master last week.

Apparently, this also introduced a few more options, but during my local
tests these didn't seem to be an issue. Also, `shell.nix` doesn't
evaluate with `goimports` in the `buildInputs`, so I didn't re-format it
here.

[1] https://github.com/Ma27/hydra/blob/private-projects/hydra-api.yaml
curl -X POST -c "$cookieJar" \
"$serverRoot"/login \
--fail --silent --show-error -o /dev/null \
-d '{"username":"'"$username"'","password":"'"$pw"'"}' \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using jq to construct this JSON document, in case there are "s in the password?

@grahamc
Copy link
Member

grahamc commented Nov 15, 2021

Overall it looks pretty good to me. Before we merge we should make sure @cole-h gets a review in as well.

@grahamc grahamc requested a review from cole-h November 15, 2021 17:16
@Ma27
Copy link
Author

Ma27 commented Nov 15, 2021

@grahamc please keep in mind that the Hydra PR needs to be merged first :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants