Skip to content
This repository has been archived by the owner on Aug 28, 2021. It is now read-only.

Add EKS support #102

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add EKS support #102

wants to merge 6 commits into from

Conversation

jensendw
Copy link

Adding functionality which will allow connection to AWS EKS cluster

@gbvanrenswoude
Copy link

Brilliant, we need this

@sbkg0002
Copy link

This would speed up some integration.

@jensendw
Copy link
Author

jensendw commented Feb 3, 2019

Until this is merged I have made my own build available at jensendw/concourse-helm-resource . It's actively being used with multiple EKS clusters.

@ferdinandhuebner
Copy link

Thank you for the time and effort you put into the PR, @jensendw.

I think the support for EKS is useful, but we would like to keep linkyard/concourse-helm-resource cloud-provider agnostic if possible.
While it might seem desirable to add support for the authentication mechanisms of the major cloud providers, this would introduce additional complexity and require appropriate testing of each authentication mechanism.

We would rather try to provide an extension point that you could use to to easily derive a custom concourse-helm-resource tailored to the authentication needs of the cloud provider of your choice.

Please let me know if this is something that might work for you.

@jensendw
Copy link
Author

jensendw commented Feb 4, 2019

@ferdinandhuebner deriving a custom concourse-helm-resource might work, although, the logic surrounding this type of integration would greatly increase the complexity of the product. Do you see other demand for this outside of EKS?

@matthope
Copy link

matthope commented Feb 5, 2019

What if this was updated to use a couple of references to run-parts to make it possible to extend functionality without changing the scripts?

@ferdinandhuebner
Copy link

I think there will be demand to support GKE and AKS if we add support to EKS to this helm resource. We would then end up with a rather large docker image that will be very difficult to properly synchronize with the release schedules of helm, kubectl and the CLI tools for those three cloud providers.

The idea of the extension point is to make it very easy to derive a concourse-helm-resource with another form of authentication.

Without thinking it all the way through, it might look something like this:

  • You use this concourse-helm-resource in the FROM instruction of your Dockerfile
  • You add a shell-script to a well-known location and give it a name that follows a pattern
  • We add two new parameters to the resource, e.g. authenticator and authenticator_parameters
  • When we setup kubectl we call /path/to/authenticators/${authenticator} and pass authenticator_parameters to the authenticator script

The responsibility of the authenticator would be to setup kubeconfig with proper authentication.

@jensendw
Copy link
Author

jensendw commented Feb 6, 2019

@ferdinandhuebner I think the one where you will run into issues is:

* When we setup `kubectl` we call `/path/to/authenticators/${authenticator}` and pass `authenticator_parameters` to the authenticator script

At least as far as I know in the case of using aws-iam-authenticator with kubectl many of the parameters that need to be injected aren't available via kubectl config .... And so, you end up merging the kubectl config file with what has already been created. I think doing it the way you described could potentially lead to some interesting regressions. For example, in this PR take a look at https://github.com/linkyard/concourse-helm-resource/pull/102/files#diff-1623687d4df544193e2c30926ea689bcR39 . I had to unset the user that was previously configured before the kubectl files are combined, if that action does not occur the configuration file will not work correctly. I suppose if the approach you suggested is used then when there is a new release of the linkyard/concoruse-helm-resource container I would change my FROM line, cut a new release and just test my own version and resolve any potential conflicts. Although that sounds a lot like just maintaining a forked project.

@gbvanrenswoude
Copy link

We'd love to follow the progress on this since you guys have a valid discussion here. In the meantime, we changed a forked version of your resource. It is able to leverage the awscli to autogenerate EKS cluster kubeconfig based on cluster name, which works a lot easier from a resource source configuration point of view. Also we added support for sts assumerole. Both things named above would be great functionality to be covered in a later version of the linkyard version.

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

Successfully merging this pull request may close these issues.

None yet

5 participants