-
Notifications
You must be signed in to change notification settings - Fork 35
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
RFC: var sources #39
RFC: var sources #39
Conversation
Signed-off-by: Alex Suraci <suraci.alex@gmail.com>
Signed-off-by: Alex Suraci <suraci.alex@gmail.com>
Signed-off-by: Alex Suraci <suraci.alex@gmail.com>
Signed-off-by: Alex Suraci <suraci.alex@gmail.com>
039-var-sources/proposal.md
Outdated
* How should pipeline-level credential managers differ in behavior from | ||
system-level credential managers, if at all? | ||
|
||
* Should we allow multiple var sources to be configured at the system-level? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say no. Because:
- If the second var source is also owned by the concourse ops team, then not need the second at all.
- If the second var source is owned by team/pipeline owners, then they will have give the vault access token to concourse ops team in order to add the vault to concourse cluster, which brings a security concern, token might be leaked by concourse ops team.
With pipeline var source, a pipeline owner only needs to add his own vault's token to the system vault. This way, concourse ops doesn't know anything about users' vaults, which eliminate the security concern. This model will reduce OPs cost of concourse clusters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I'm trying to figure out is if there is clear and consistent behavior that we can figure out in order to support multiple levels of var_sources
, because that will be something we have to figure out once projects (#32) is here.
Having only a single system-level credential manager kind of feels like an artificial limitation if we're going to support multiple at the pipeline-level, so I'd like to have solid reasoning as to why.
I could imagine, for example, a Concourse installation which has a Vault credential manager as well as an EC2 IAM role-based STS credential manager (concourse/concourse#3023) for access to AWS resources. As this depends on the EC2 configuration of the web
node, it makes sense for operators to configure it, not pipeline authors. For the same reasons outlined in this proposal, though, it seems like it would be bad to require operators to choose between Vault and STS as the latter is very specialized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like this turned out to be useful: concourse/concourse#4777 (comment)
I'll update the RFC soon!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops nevermind this was a completely different question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I'm updating the docs for var_sources
, I'm finding it very difficult to consolidate the docs for cluster-wide credential managers, which are configured through env vars and flags, and pipeline-local var_sources
which are configured through YAML.
If it's hard to document, it'll probably be hard to learn and use. I think it would be a lot easier to be able to document var_sources
exclusively and then just say they can be configured cluster-wide or local to a pipeline. This will also force the issue of having to reason about multi-tiered var_sources
, which I'm anticipating to be something we need to address in the future anyway with Projects.
So at this point I'm leaning towards unifying the cluster-wide and pipeline-local credential manager config as a single concept, var_sources
, and defining the scoping semantics such that the local var source names 'shadow' the outer ones. We kinda planned for this already anyway in #4777.
also removed a vaguely worded question and related named var usage to the project-level var sources question Signed-off-by: Alex Suraci <suraci.alex@gmail.com>
Signed-off-by: Alex Suraci <suraci.alex@gmail.com>
now that the Prototypes RFC has a proposal for secure transmission of credentials, we can mention it as a likely future path. also mention the `get_var` step and `across` step RFCs as a motivator for supporting multiple types of var sources (which aren't credential managers). Signed-off-by: Alex Suraci <suraci.alex@gmail.com>
Signed-off-by: Alex Suraci <suraci.alex@gmail.com>
per concourse/concourse#4249 Signed-off-by: Alex Suraci <suraci.alex@gmail.com>
Hey @vito 👋 The docs at https://concourse-ci.org/vars.html#var-sources currently say "var_sources [...] is considered an experimental feature until its associated RFC is resolved" with the RFC link being to this, merged, PR. JOOI is the feature still experimental, should the link point elsewhere, or ... something else? |
@jpluscplusm Still have work in flight on this - we're working on a feature so that var sources can be bring-your-own just like resource types so we don't have to bake them into Concourse core code. Once that lands we plan to phase out the built in ones and there may be changes to how they're configured, so it's still experimental for now. More info in concourse/concourse#5229 - which the docs should probably point to instead at this point. |
the doc is updated to point to the open issue instead of this closed RFC. |
@xtremerui Just FYI, I'm not seeing that update online, and I can't see a commit in the docs repo that would suggest a website rebuild is pending. However I'm not using the project actively any more, so I'll stop commenting now. |
confirming that the docs still points to this PR |
Rendered
supercedes #21