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

Ability to set env vars in project admin #3992

Closed
6 tasks done
agjohnson opened this issue Apr 21, 2018 · 8 comments
Closed
6 tasks done

Ability to set env vars in project admin #3992

agjohnson opened this issue Apr 21, 2018 · 8 comments
Assignees
Labels
Accepted Accepted issue on our roadmap Feature New feature
Milestone

Comments

@agjohnson
Copy link
Contributor

agjohnson commented Apr 21, 2018

We should allow users to inject env vars that can be maintained outside of source code. The use case here would be inputting shared secrets like API access tokens which would then be injected into the build processes. Heroku does something similar.

Eventually, we might auto generate an auth token for our api and inject this in the same way, so that Sphinx can automatically have access to our API as the user. This particular feature will likely be an API v3 feature though.

Phase I

  • Add modeling for project environment variables
  • Add site admin UI for models
  • Add API response that includes environment variables so builder can consume this
  • Expose env variables to necessary build commands (all build commands?)

Phase II

  • Add model forms for user facing interface
  • Add CRUD UI to project admin -- copy from established CRUD UI pattern
@agjohnson agjohnson added Improvement Minor improvement to code Needed: design decision A core team decision is required labels Apr 21, 2018
@agjohnson agjohnson added this to the New build features milestone Apr 21, 2018
@stsewd stsewd mentioned this issue Jul 31, 2018
@agjohnson
Copy link
Contributor Author

@humitos what do you think of this feature + pypa/pip#3728 to support private repositories through pip? Maybe there is a way to also address git submodules?

It feels a bit hacky still. A GitHub user looks to be the prescribed path, as much as i hate it.

@agjohnson agjohnson modified the milestones: New build features, 2.9 Nov 10, 2018
@agjohnson agjohnson added Feature New feature and removed Improvement Minor improvement to code labels Nov 10, 2018
@humitos
Copy link
Member

humitos commented Nov 12, 2018

@humitos what do you think of this feature + pypa/pip#3728 to support private repositories through pip?

I like this because it's not something that we are hacking to support but it comes from upstream and also because it seems pretty easy to do, follow and understand what's happening.

Expose env variables to necessary build commands (all build commands?)

Do you see any potential problem by exposing all the ENV vars to all the commands?

I see as cons that we may end up sending secret data to places that should not be sent. On the other hand, as pros I think this will help users to expand other commands that we don't think to be expandable when implementing this: I see this a way of "allowing things from outside". For example, you can run custom code in conf.py without worries about exposing anything if we provide a ENV secret. Many doors (maybe hacky ones) are open there to the users.

But, if we don't want to expose all the ENV to all the commands we could match the ENV to a particular command:

class EnvVariable(models.Model):
   command = models.ChoiceField(choices=['sphinx-build', 'pip', 'git', ...])
   name = models.CharField()
   value = models.CharField()
   project = models.ForeignKey(Project)

with that model we could inject the variables only where they are needed. If it's a ChoiceField we will end up by duplicating the same variable for multiple commands, so it may needs to be a multiple choice field.

Maybe there is a way to also address git submodules?

This case is a little more complex to me. I found that what some CI do is to modify the submodule URL on the fly with a config withtout touching the user configs using insteadOf (more context from SSH management PR)

@humitos
Copy link
Member

humitos commented Nov 12, 2018

Adding this link that contains useful information about how travis handles this: https://docs.travis-ci.com/user/best-practices-security/#Steps-Travis-CI-takes-to-secure-your-data

@agjohnson
Copy link
Contributor Author

But, if we don't want to expose all the ENV to all the commands we could match the ENV to a particular command:

I'd be -1 on this is think, it's too complex for the user.

@agjohnson
Copy link
Contributor Author

We can disregard the SSH use case for now. Users will potentially be able to use this for accessing GitHub, but it's multiple levels of bad UX at this point.

@humitos
Copy link
Member

humitos commented Nov 13, 2018

Add CRUD UI to project admin -- copy from established CRUD UI pattern

Which pattern do we want to follow here?

  1. list and add in the same page, as Domain
  2. list in the first page and a different page for add/edit/delete, as Integration

I think it's clearer the one from Integrations.

@agjohnson
Copy link
Contributor Author

Yeah, integrations is more recent, it's the pattern to follow.

@humitos humitos added Accepted Accepted issue on our roadmap and removed Needed: design decision A core team decision is required labels Dec 4, 2018
@ericholscher
Copy link
Member

This is now done. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Feature New feature
Projects
None yet
Development

No branches or pull requests

3 participants