teamhephy / controller

Hephy Workflow Controller (API)
https://teamhephy.com
MIT License
14 stars 26 forks source link

feat(controller): Add POD_IP environment variable with pod's IP address #105

Closed n0n0x closed 5 years ago

n0n0x commented 5 years ago

Expose each pod's IP address as an environment variable POD_IP. This feature is really useful to us that we use python + django a lot.

We are using prometheus-operator and we are adding business metrics to our applications and fetching them with prometheus.

The main issue is that django requires you to set ALLOWED_HOSTS variable with a set of domains that Django can serve.

Prometheus uses the internal pod IP to fetch the metrics, and pod's IPs are dynamic. With this modification we can do something like this on python:

ALLOWED_HOSTS = env.list('DJANGO_ALLOWED_HOSTS', default=['some-domain.com'])

if env('POD_IP'):
    ALLOWED_HOSTS.append('POD_IP')

this way prometheus would be able to fetch the metrics and not get a 400 BAD REQUEST as a response.

Cryptophobia commented 5 years ago

Hi @n0n0x , the way I understand this PR is that the environment variable is to be injected into each pod and then you can use it in django/python applications to whitelist prometheus or am I misunderstanding how this works?

Can you also please take a look at https://docs.teamhephy.com/contributing/submitting-a-pull-request/#commit-style and fix the commit messages for our tool?

n0n0x commented 5 years ago

Hi @n0n0x , the way I understand this PR is that the environment variable is to be injected into each pod and then you can use it in django/python applications to whitelist prometheus or am I misunderstanding how this works?

Yes! This would be our case, but maybe someone can find this useful in another way :)

n0n0x commented 5 years ago

Looks good, can you fix the commit message? https://docs.teamhephy.com/contributing/submitting-a-pull-request/#commit-style

Done! Let me know if it looks good now!

Cryptophobia commented 5 years ago

@n0n0x , I like this hack, it serves a good purpose. But I can think of at least one scenario when you wouldn't want to do this for security purposes. For example, if you are hosting a 3rd-party application, you wouldn't want to expose the internal pod IP address to the application inside the k8s cluster.

The easiest fix is to allow the above POD_IP env var to be injected only if DEIS_EXPOSE_POD_IP env variable is set or something similar.

Cryptophobia commented 5 years ago

@n0n0x Slight problem this way is that the variable needs to be unset, if it is set to false, the IP is still exposed:

kubectl exec -it testing-hephy-two-web-f64cd8898-4ml7d -n testing-hephy-two -- env | grep POD_IP 
WORKFLOW_RELEASE_SUMMARY=admin changed DEIS_EXPOSE_POD_IP
DEIS_EXPOSE_POD_IP=false
POD_IP=10.244.0.23

I think we can allow this as it is a good feature and feature-flag protected this way. :+1:

Cryptophobia commented 5 years ago

Hi @n0n0x , I've added the above small fix to https://github.com/teamhephy/controller/pull/109 and reworded some of the commits. Tested and now it is time to merge! :+1: