IP Whitelisting as part of App Model

[helgi]

From @krancour via #197

Is deis config:set still used for managing whitelists? I'm not a big fan of that. I know this was v1.x behavior, but I didn't like it then either. My argument against it is that setting application environment variables is the wrong way to affect platform behavior. i.e. Setting the WHITELIST env var on the app does not affect the application's configuration / behavior at all; rather it affects the router.

I would very much love to see whitelist actually added to workflow's app model, with complimentary API endpoints and CLI subcommands for managing it.

[laurrentt]

Hello !

I'm the guy who did the PR deis/deis#4333.

The initial goal was that the router could access the whitelist configuration for an app using Etcd. Since config:set did just that, using it was the easiest way to get there. The fact that whitelists were also accessible from the application environment was a side effect I could live with but I know it wasn't ideal.

Now that Deis v2 is being built, my take on this is that we could have an endpoint dedicated for configurations that need to be exposed in Etcd and an endpoint for configurations that need to end up as environment variables. This way we could stop exposing sensible things like DB passwords in Etcd and stop exposing configuration details like whitelists inside the app.

I don't have an exact proposal but I think we should clarify on that before we start adding too much stuff inside the App Model.

[krancour]

@laurrentt, we're actually attempting to eliminate etcd wherever we can and hoping to eventually eliminate it entirely.

[sstarcher]

Any chance this feature will make it into 2.0? This was possible in v1, and would block us from upgrading to v2.

[sstarcher]

And I stand correct. The svc is only initially created. You can patch the svc.

kubectl patch svc APP --namespace=APP  -p '{ "metadata": { "annotations": { "router.deis.io/whitelist": "0.0.0.0/0" } } }'

[bacongobbler]

Perhaps for other annotations (whitelist, connectTimeout, tcpTimeout, domains, routable) we should add a top-level command like deis router:annotate whitelist="0.0.0.0/0" that just dumps the data directly to the service's annotations. That way we can handle more application-level annotations gracefully in the future. I know that others have asked for ways to manage the application's routable annotation in the beta so their app can only be used internally (like an internal service API for worker processes, e.g. consul/etcd/elasticsearch).

[krancour]

Just to add so clarification to this ticket, whitelists already are implemented and work perfectly... this ticket is about refactoring how that is done to make it a bit cleaner.

[sstarcher]

As a note for anyone who patches the Deis application RCs. If you migrate between K8S clusters these RCs will be recreated from scratch so they will lose the patched whitelists that are added.

[mboersma]

they will lose the patched whitelists that are added

Perhaps the upgrade procedure needs to take into account saving & restoring annotations like this.

[krancour]

@sgoings asked me to weigh in on a potential UX here.

Responding to @bacongobbler's suggestion, I can see the appeal in something as generic as deis router:annotate, but therein lie a few potential problems.

Compare this first to other cases where the CLI, via the controller, effects changes to a routable service's annotations. A great example is the management of domains and certificates. The fact that the CLI exposes well-defined, interactively documented subcommands for managing these things means that app operators don't need to posses detailed knowledge of how to configure the router using annotations-- that thankless task is left to the controller. Exposing something as generic as router:annotate would obviate that convenience by requiring an app operator to have detailed knowledge of router configuration options. (In all practicality, router's configuration-via-annotations docs are as detailed as they are for the sake of the crowd electing to use router without the rest of Workflow. On principle, deis CLI users shouldn't need to dig that deep into the router config docs.)

A secondary concern:

that just dumps the data directly to the service's annotations

I'm not sure if the word "directly" in the above implies an implementation that bypasses the controller, but bypassing the controller wouldn't be a good idea. In "real world" k8s clusters, I am hoping that cluster admins have locked down permissions on the apiserver more significantly than we tend to (i.e. on our test clusters, we do no such thing). In such a cluster, the deis-controller service account should be granted extensive permissions that are not granted to most users. In other words, ideally, your average deis CLI user lacks the permissions to "dump the data directly to the service's annotations," although the controller has sufficient permissions to do it on their behalf.

The alternative I would recommend a UX that is suggested by the following help:

$ deis whitelist -h

Valid commands for whitelist:

whitelist:add              add an entry to the application's whitelist
whitelist:list                list entries in the application's whitelist
whitelist:remove        remove an entry from the application's whitelist

Use 'deis help [command]' to learn more.

This would require complementary API endpoints to be added to the controller, with the underlying models modified accordingly as well.

[bacongobbler]

@krancour you might want to take a look at #966 and give your opinion there, because that model directly relates to router config. So far maintenance (and apparently routable) settings are being applied there as candidates, but it's essentially a model for simple toggle-able flags which re-deploy the same application version without a Release. deis maintenance and deis routing are both new commands getting released for v2.4.0 which are part of the AppSettings model.

Coming back to the OP, I agree and I think IP Whitelists should be a separate object as @krancour described in his recommendation.