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

WIP: Provide runtime generated credentials through credentials file #77

Closed
wants to merge 1 commit into from

Conversation

tenjaa
Copy link

@tenjaa tenjaa commented Apr 8, 2019

A first draft how a solution for #76 could look like.
I dont expect this to be merged directly. It should be a basis for discussion how to solve the problem given in #76.

@x6j8x
Copy link

x6j8x commented Apr 9, 2019

Instead of passing a single file one could simply detect if the value of a parameter points to a file in the working directory and replace the name with its contents.

Add documentation for credentials file

Signed-off-by: Torben Neufeldt <[email protected]>
@vito
Copy link
Contributor

vito commented May 15, 2019

Thanks for taking a crack at this, and sorry for the long wait!

What's here makes sense, except I think the problem extends to check too, which can't depend on a task or anything generating files for it.

Could these credentials instead come from a credential manager (assuming something like concourse/rfcs#21 exists)? What does a task that fetches these credentials typically look like?

@tenjaa
Copy link
Author

tenjaa commented Jun 4, 2019

@vito thanks for the reply!

As far as we could see the check doesn't do anything at all. It is just printing an empty array without using the provided credentials at all.
So what should be the issue there?

Using a credentials manager would not really solve our problem. Yes it might be possible but also concourse isn't the only CI/CD tool used I am not sure we can expect a deep integration there.

The task fetching the credentials is usually just some curl + jq. But that is a custom service therefore it won't make sense to integrate it directly into the resource.

@vito
Copy link
Contributor

vito commented Jun 5, 2019

As far as we could see the check doesn't do anything at all. It is just printing an empty array without using the provided credentials at all.

Ah, lame, I forgot this resource is only partially implemented. Technically it should also be able to detect whether the app is pushed and such.

I'm still not quite sure about merging this. We're going to be extracting all of our base resource types from Concourse soon (concourse/rfcs#30), so at that point there won't be much difference between running our "official" version and running your own fork, since we don't change this resource type very often. It's entirely community-maintained and we've been trying to let CloudFoundry own it instead of us. Based on your workflow it sounds like you could potentially have your own variant that does the credential fetching for you, so you don't even need the separate task. 🤔

The main reason I'd rather not merge it is it's kind of leveraging the fact that this resource isn't properly implemented. A proper implementation should be able to support check, get, and put. IMO it's important that our core resource types be 'exemplary' as people will use them as a reference point when designing their own resources. If your fork configured the location to fetch the credentials from, it could fetch them during check too (in theory).

Does that make sense? 😅

@tenjaa
Copy link
Author

tenjaa commented Jun 21, 2019

Thanks for the explanations and yes it does make sense.
We will look into other solutions then.

@tenjaa tenjaa closed this Jun 21, 2019
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.

3 participants