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

introduce more configurable options for service type #32

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

Conversation

carpenike
Copy link

Hello!

I need to host small-step on it's own LoadBalancerIP rather than an ingress controller as the ingress controller uses cert-manager to get a public cert from LE rather than the PKI chain that Small Step provides.

This PR introduces additional service options for Small Step but defaults to the current configuration.

@carpenike
Copy link
Author

@maraino -- would love your review when you get time!

@carpenike
Copy link
Author

Any thoughts?

@zolech
Copy link

zolech commented Aug 19, 2021

Hey, that is awesome MR. I also need LoadBalancer type instead NodePort. Right now I started to modify template locally but It would be great to have it in public chart.

@zolech zolech mentioned this pull request Dec 22, 2021
@larssb
Copy link
Contributor

larssb commented Jan 18, 2022

To someone landing here and the peeps already on the issue. I worked around this issue by using the ytt tool from Carvel.

So when I set service to type LoadBalancer I overlay on the generated Kubernetes Service object and add a loadBalancerIP key/value to that object.

If people are interested in more ... let me know and I can provide a gist. But, of course, the greatest solution would be that the step-certificates Helm charts actually supports providing a loadBalancerIP. So why not get this PR proposal in 💯 - good stuff.

Thanks and have a great day.

@larssb
Copy link
Contributor

larssb commented Jan 18, 2022

Hey, that is awesome MR. I also need LoadBalancer type instead NodePort. Right now I started to modify template locally but It would be great to have it in public chart.

@zolech see my suggestion until this is merged. My solution makes it possible for you to avoid customizing the chart locally.

@zolech
Copy link

zolech commented Jan 18, 2022

Hey, that is awesome MR. I also need LoadBalancer type instead NodePort. Right now I started to modify template locally but It would be great to have it in public chart.

@zolech see my suggestion until this is merged. My solution makes it possible for you to avoid customizing the chart locally.

Thanks but I've already resolved that by passing ssl through with treafik. Works great and there is no need using LoadBalancer type.

@larssb
Copy link
Contributor

larssb commented Jan 18, 2022

Hey, that is awesome MR. I also need LoadBalancer type instead NodePort. Right now I started to modify template locally but It would be great to have it in public chart.

@zolech see my suggestion until this is merged. My solution makes it possible for you to avoid customizing the chart locally.

Thanks but I've already resolved that by passing ssl through with treafik. Works great and there is no need using LoadBalancer type.

Alright. Totally fair! However, based on this issue > #77 < I get the impression that doing that is not the best of ideas.
Hmm.

@zolech
Copy link

zolech commented Jan 18, 2022

Hey, that is awesome MR. I also need LoadBalancer type instead NodePort. Right now I started to modify template locally but It would be great to have it in public chart.

@zolech see my suggestion until this is merged. My solution makes it possible for you to avoid customizing the chart locally.

Thanks but I've already resolved that by passing ssl through with treafik. Works great and there is no need using LoadBalancer type.

Alright. Totally fair! However, based on this issue > #77 < I get the impression that doing that is not the best of ideas. Hmm.

It's not the best idea to use L7 ingress but traefik has CRD that uses L4 load balancing: ingressroutetcps.traefik.containo.us and together with SNI works well in this case.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

5 participants