Pomerium: refactor configuration handling

[travisgroth]

As initially discussed in #89 - I'd like to propose refactoring configuration handling from pomerium/envconfig to spf13/viper I see this as a precursor to #89, since reloading of policy naturally leads into how run time configuration is brought into the proxy and authz services.

• Brings in support for a variety of additional config storage formats
• Has vetted handling of live configuration notifications to support #89
• Should reduce code surface area in pomerium

User facing spec

Changes

• Non-breaking: Add support for a new configuration file located at /etc/pomerium/config.[yaml|toml|json]
  
  
  
  • This file can contain _all_ configuration options, including inline policy
  
  
• Non-breaking: Configuration file location can be overridden with an env_var
• Breaking: Deprecate POLICY_FILE and require policy to be defined as either a base64 encoded env var or as part of config file policy key

Config file example

authenticate_service_url: https://authenticate.corp.example.com
authorize_service_url: https://authorize.corp.example.com
shared_secret: 9wiTZq4qvmS/plYQyvzGKWPlH/UBy0DMYMA2x/zngrM= 
cookie_secret: uPGHo1ujND/k3B9V6yr52Gweq3RRYfFho98jxDG5Br8=
idp_provider: "google"
idp_client_id: "REPLACE-ME.googleusercontent.com"
idp_client_secret: "REPLACEME"
policy:
    - from: httpbin.corp.beyondperimeter.com
    to: http://httpbin
    allowed_domains:
        - pomerium.io
    cors_allow_preflight: true
    timeout: 30s
    - from: external-httpbin.corp.beyondperimeter.com
    to: httpbin.org
    allowed_domains:
        - gmail.com
    - from: weirdlyssl.corp.beyondperimeter.com
    to: http://neverssl.com
    allowed_users:
        - [email protected]
    allowed_groups:
        - admins
        - developers
    - from: hello.corp.beyondperimeter.com
    to: http://hello:8080
    allowed_groups:
        - admins

Internal Spec

The plan is to continue using a struct unmarshalled from the configuration to represent options.

At a high level, there are two approaches to the refactor:

1. Extend the top level Options struct to support all options and pass this along to each service.

   • Basic validation and loading can be done in a single location
   • Individual services can still validate the fields they're interested in
   • Makes the largest reduction in maintained/duplicate Service level code
   • Options shared across services don't need to be added to multiple service scope structs

2. Retain current pattern of each scope (global level + 1 per service) initializing their own scoped Options structs

   • The change is smaller and retains more of existing call patterns
   • There is explicit struct level documentation of which fields are required by which service outside of validation

I prefer the first approach but, as it is a bit of a change from the current structure, I'm quite open to input or questions. @desimone please weigh in with any preference or guidance here and I can start working on the PR.

[desimone]

Thank you @travisgroth for taking the lead on this (#89) and creating this spec.

I think this is a great improvement; I know having a single source of configuration (both policy and environmental) would simplify deployment for many users.

RE: User facing spec

Non-breaking: Configuration file location can be overridden with an env_var

LGTM. Perhaps we can do something similar to what we do with certificates today in how we fall back to a default file location if non is provided via a flag or env-var?

Breaking: Deprecate POLICY_FILE

LGTM. I'd like to get versioning (#109) and upgrade (#114) documentation finished in line with this to make migration as seamless as possible for current users. Hopefully, by the end of the week.

require policy to be defined as either a base64 encoded env var or as part of config file policy key

Would it be possible to support mixing environmental variables and configuration files? I can imagine a scenario (it's discussed a bit in the BeyondCorp papers as well) in which one team (e.g. SREs) manages the operation/deployment of pomerium while another (e.g. security) manages access policy (policy.yml).

Config file example

LGTM.

Internal Spec

...

Extend the top level Options struct to support all options and pass this along to each service.

… vs...

Retain current pattern of each scope (global level + 1 per service) initializing their own scoped Options structs

I prefer the first approach

I agree. We've got several overlapping Options structs with duplicated fields that would be easier to reason about and work with if they were in a single place. As you said, we can still do service specific option validation. As I write this, this makes so much sense I wonder why we haven't done it sooner 😀.

Miscellaneous thoughts /questions

• It looks like Viper also supports distributed K/V stores which may be useful in the future
• Could this change break cross compatibility (it doesn't look like fsnotify would require cgo but just want to make sure)?

Let me know if there's anything I can do to help.

[travisgroth]

RE: User facing spec

Non-breaking: Configuration file location can be overridden with an env_var

LGTM. Perhaps we can do something similar to what we do with certificates today in how we fall back to a default file location if non is provided via a flag or env-var?

Yep defaulting behavior can be retained. https://github.com/spf13/viper#establishing-defaults

LGTM. I'd like to get versioning (#109) and upgrade (#114) documentation finished in line with this to make migration as seamless as possible for current users. Hopefully, by the end of the week.

👍 I will look for that as I wrap up the work. I'm hoping this will be done this week but it depends on how much free time I wind up with.

Would it be possible to support mixing environmental variables and configuration files? I can imagine a scenario (it's discussed a bit in the BeyondCorp papers as well) in which one team (e.g. SREs) manages the operation/deployment of pomerium while another (e.g. security) manages access policy (policy.yml).

Yes; for "free". Normally, an env_var option will override configuration file options transparently with viper. With our policy it gets a little weird because we __must__ figure out if it came from env_var to decode it. I did a mini test of how to achieve this and I think I've got it. Worst case is we have to brute force attempt a decode.

I agree. We've got several overlapping Options structs with duplicated fields that would be easier to reason about and work with if they were in a single place. As you said, we can still do service specific option validation. As I write this, this makes so much sense I wonder why we haven't done it sooner 😀.

👍 happy to help

Miscellaneous thoughts /questions

• It looks like Viper also supports distributed K/V stores which may be useful in the future

Yes I figured this would be a nice possibility later. I'm very used to treating kubernetes as my distributed config but I know consul/etcd are still in use a lot out there. I think making k/v store support work requires a little more wiring but it should be a small lift effort after the refactor. I want to keep the scope of this issue to just retaining existing functionality.

• Could this change break cross compatibility (it doesn't look like fsnotify would require cgo but just want to make sure)?

Not that I'm aware of. Should just be system calls.

Let me know if there's anything I can do to help.

Will do. Thanks. I guess one question - this is going to be a fair number of LoC, so a series of diffs might be easier to review than the entire refactor. If you have any input on how you want that structured, I'm happy to work with it. In any case, I'll open a draft PR so you can see the progress.

[desimone]

Perfect.

this is going to be a fair number of LoC, so a series of diffs might be easier to review than the entire refactor. If you have any input on how you want that structured, I'm happy to work with it. In any case, I'll open a draft PR so you can see the progress.

Use whatever process works best for you. Feel free to ping me on github or request a review whenever you'd feel it would be helpful. If you'd prefer more synchronous conversation drop me a line [email protected] and we can find an appropriate medium .

[travisgroth]

To track the todo list:

• [x] Introduce central options handling (#118)
• [x] Switch to viper for all config handling (#130)
• [x] Address docker interface (#139)
• [x] Address helm interface (https://github.com/pomerium/pomerium-helm/pull/2)
• [x] Upgrade/deprecation documentation (#137)
• [x] Update examples (#147)

[desimone]

@travisgroth I'm going through the examples (updating and checking) for the v0.0.5 release if you are cool with me taking that piece of work / if you haven't started already yet.

[travisgroth]

Absolutely. I haven't had a chance to start that and would appreciate the help.

[travisgroth]

@desimone do you want to close this out? You're covering the last bit as part of other work (#147)

[desimone]

edit: yes 👍

Thank you @travisgroth !