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

Create a method to use secrets in QHub config #13

Closed
aktech opened this issue Jul 16, 2020 · 11 comments · Fixed by #720
Closed

Create a method to use secrets in QHub config #13

aktech opened this issue Jul 16, 2020 · 11 comments · Fixed by #720
Assignees
Labels
area: security 🔐 needs: discussion 💬 Needs discussion with the rest of the team

Comments

@aktech
Copy link
Member

aktech commented Jul 16, 2020

After discussion with @costrouc we realise we don't have a way to use secrets in the Qhub ops, one example is secrets are expected to be in the config.yaml (which is not ideal):

https://github.com/Quansight/qhub-ops/blob/e5dd52df558c2c4eb805594b6ae574f12e0986a1/tests/assets/config_aws.yaml#L11

The solution we came upon was to make qhub read from environment variables and refer those environment variables in the cookiecutter's generation of the project.

Some thing like:

QHUB_GITHUB_CLIENT_SECRET
QHUB_GITHUB_CLIENT_ID
...
@costrouc
Copy link
Member

To add why this seems like a reasonable path forward is that terraform does the exact same thing https://www.terraform.io/docs/configuration/variables.html#environment-variables

@costrouc costrouc transferred this issue from Quansight/qhub-ops Aug 18, 2020
@tylerpotts tylerpotts self-assigned this Sep 17, 2020
@costrouc
Copy link
Member

costrouc commented Sep 22, 2020

The override should be done similarly to Terraform. E.g. suppose we have a config file

a:
  b_e:
    - c: 1
      d: [1, 2, 3]

We would override via QHUB__a__b_e__0__c=2. I realize this adds several requirements. Since we already have underscores in our keys and terminals typically only allow [A-Za-z0-9_] we have to parse the attribute path and handle the underscores properly (we could get arround this by requiring __ two underscores between all keys. Also we should handle list accessing by checking if the key is an integer and treat it as a list. Additionally we should attempt to cast the value as an integer since there are several integers in the config file if that fails just supply the value as a string.

We would apply all environment variables that start with QHUB__

@tylerpotts
Copy link
Contributor

tylerpotts commented Sep 24, 2020

In the course of working on this, we realized that this will require a more nuanced approach. Editing the input config doesn't actually hide the secrets, as they will get written out in plain text in the infrastructure files

@costrouc
Copy link
Member

Closing this issue since we don't think that adding this feature is a good idea due to the secrets being in the repo regardless

@tylerpotts tylerpotts reopened this Mar 9, 2021
@tylerpotts tylerpotts added the needs: discussion 💬 Needs discussion with the rest of the team label Mar 9, 2021
@tylerpotts
Copy link
Contributor

@aktech @costrouc Let's continue this discussion

@danlester
Copy link
Contributor

I think a possible solution for Auth0 at least could be to use the Auth0 terraform provider: https://registry.terraform.io/providers/alexkappa/auth0/latest

The management API key/secret can be in GitHub secrets / env vars going into qhub deploy, then the provider should be able to sync with Auth0 to get details for a specific named client.

The final step would be to take the client key/secret and pass it to jupyterhub config, probably through env vars.

This way, the client key/secret isn't needed in the qhub-config.yaml at all. It would only be in the remote state.

I haven't found a similar provider for GitHub but haven't looked hard yet...

@github-actions
Copy link

This issue has been automatically marked as stale because there was no recent activity in 60 days. Remove the stale label or add a comment, otherwise, this issue will automatically be closed in 7 days if no further activity occurs.

@github-actions github-actions bot added the status: stale 🥖 Not up to date with the default branch - needs update label Jun 22, 2021
@github-actions
Copy link

This issue was closed because it has been stalled for 7 days with no activity.

@leej3 leej3 reopened this Jun 29, 2021
@leej3
Copy link
Contributor

leej3 commented Jun 29, 2021

I will try to work through this in the next week or two.

@github-actions github-actions bot removed the status: stale 🥖 Not up to date with the default branch - needs update label Jun 30, 2021
@costrouc
Copy link
Member

Reopening since this feature was removed in #1003.

@viniciusdc
Copy link
Contributor

This needs to be addressed, this issue was resolved with #720. As per @costrouc
comment, this was removed from 0.4.0, so I will link this as a new issue for re-enabling this feature.

Repository owner moved this from Needs Triage 🔍 to Done 💪🏾 in QHub Project Mangement 🚀 May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: security 🔐 needs: discussion 💬 Needs discussion with the rest of the team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants