zendesk / samson

Web interface for deployments, with plugin architecture and kubernetes support
Other
1.45k stars 234 forks source link

Ability to source configuration for env from Consul #2217

Closed epssy closed 6 years ago

epssy commented 6 years ago

It would be really useful to be able to source configuration from Consul in a similar way to Vault.

To populate their ENV applications should be able to call a path like config:// or consul:// that looks up values from a global space like /kv/global/ or an assumed path /my_app_attribute which assumes a full path like /kv/myapp/environment/region/my_app_attribute. When used this should be indistinguishable from the current usage of secret://.

Project maintainers should be able to set and update configuration values for their name spaces. And, unlike secrets, they should be able to see the currently set values for their attributes in Consul as well as the global name space. Global should be read only, and the attributes of other projects should be hidden.

epssy commented 6 years ago

It doesn't look like this feature set can be implemented as a plugin so I've started mocking out resolve_consul() in terminal_executor.rb. This would require a Consul specific duplication of secret_storage.rb and hashicorp_vault_backend.rb and an addition to the secret management view but is otherwise pretty straight forward. The eventual env hash would be passed to the service in either the .env.deploy_group file for instances or via --env for K8 Docker containers.

diff --git a/app/models/terminal_executor.rb b/app/models/terminal_executor.rb
index 0f9288a4..53df2f32 100644
--- a/app/models/terminal_executor.rb
+++ b/app/models/terminal_executor.rb
@@ -14,6 +14,7 @@ require 'pty'
 #
 class TerminalExecutor
   SECRET_PREFIX = "secret://"
+  CONSUL_PREFIX = "consul://"

   attr_reader :pid, :pgid, :output

@@ -52,7 +53,7 @@ class TerminalExecutor
   end

   def verbose_command(c)
-    "echo » #{c.shellescape}\n#{resolve_secrets(c)}"
+    "echo » #{c.shellescape}\n#{resolve_secrets(c)}\n#{resolve_consul(c)}"
   end

   private
@@ -78,6 +79,7 @@ class TerminalExecutor
         verbose_command(c)
       else
         resolve_secrets(c)
+        resolve_consul(c)
       end
     end
     commands.unshift("set -e")
@@ -102,6 +104,24 @@ class TerminalExecutor
     result
   end

+  def resolve_consul(command)
+    return command unless command.include?(CONSUL_PREFIX)
+    deploy_groups = @deploy&.stage&.deploy_groups || {}
+    resolver = Samson::Consul::KeyResolver.new(@project, deploy_groups)
+
+    result = command.gsub(/\b#{CONSUL_PREFIX}(#{ConsulStorage::CONSUL_ID_REGEX})\b/) do
+      key = $1
+      if expanded = resolver.expand('unused', key).first&.last
+        key.replace(expanded)
+        ConsulStorage.read(key, include_value: true).fetch(:value)
+      end
+    end
+
+    resolver.verify!
+
+    result
+  end
+
   # reset pid after a command has finished so we do not kill random pids
   def record_pid(pid)
     @pid = pid

I'll throw up a PR of this working once I've finished writing hashicorp_consul_backend.rb.

In the mean time, thoughts?

grosser commented 6 years ago

Anything can be a plugin by either adding new hooks or patching over existing code.

consul:// needs to be supported in commands and environment variables (see plugins/env/app/models/environment_variable.rb)

So these 2 usages should first be unified into a single method/helper and then we can hook em up like:

... in hooks.rb ...
Samson::Hooks.callback :config_resolver do
 ['secret', Samson::Secrets::KeyResolver]
end

... in the plugin ...
Samson::Hooks.callback :config_resolver do
 ['consul', SamsonConsul::KeyResolver]
end

I'll see if I can get the code ready for this ... might have a few gotchas ...

Making a new backend would not make sense since the secrets UI cannot work with 2 backends ...

jonmoter commented 6 years ago

You want to be able to inject values that are in Consul into deploy scripts that Samson runs? Or you want a mechanism to be able to inject values in consul's k/v store into an application's configuration at runtime?

And are you thinking of apps that run on VMs and are deployed with cap-style commands, or apps that run on Kubernetes?

Note that if you inject stuff from Consul directly into deploy scripts, it's really important that Consul is configured with authentication, some sort of permissions system, etc. That's something that Vault is much more feature-rich on.

epssy commented 6 years ago

How exactly do we hook into the Secrets management page in your suggestion @grosser? Would be good to let engineers to maintain their own project config values in the same way as secrets.

By the way the diplomat gem is excellent for handling any get/set/update operations against Consul. We've got extensive usage in the chef_cookbook_* repos. If you mock out a plugin that can do the above, I'd be able to fill in the rest pretty easily.

grosser commented 6 years ago

making it read things is relatively easy ... making it write stuff requires UI ... so might be best to have a whole new consul UI ...

epssy commented 6 years ago

That's why I started looking at mocking out an extension to the current Secrets view.

It is argued that general configuration values shouldn't be wrapped in security and as a result they are much easier to modify. For example Chef, scripts, and other automation (lambdas etc) can easily update Consul keys without the need for a complicated chain of trust to exist to give those processes a Vault token in the first place. This is a pretty reasonable argument I guess.

epssy commented 6 years ago

@jonmoter I want it to populate ENV with with values it sources from Consul in the same way that it sources from Vault. This should work with both k8 and cap style deploys. Authentication shouldn't be a concern as engineers can populate ENV key values at will for their projects.

screen shot 2017-08-31 at 1 38 48 pm

I do understand that I skimmed over part of the secrets pull process here, just ignore it and assume it'd be passed via the .env file generation process instead. The mock above shows that's easy to do.

grosser commented 6 years ago

we can't really use consul as a secrets backend or we have to modify the UI to support both :(

... the simplest way we could get this done would be to add just the resolver ... then maybe add some restrictions that say "your keys for project bar can only start with /bar/ or /global/" ... assuming that reading consul keys is not a security issue that would be pretty fast to implement and maybe solve some problems ?

... setting consul values is a whole new problem ... need to think about who is authorized to change things ... and track history somehow ...

Resolving secrets: samson would need to connect to every DC to find the correct values from consul ? .. which is pretty weird logic and would need some kind of mapping too "this goes to master so read from DC=xyz" ...

epssy commented 6 years ago

We don't want to use Consul for secrets. We want to use it for configuration values.

It just so happens that the code paths are almost identical in Samson hence the bulk of copy/paste.

I'm just going to go ahead and write what I'm looking for and once I get a PR up it should be clear.

grosser commented 6 years ago

faster to explain it ... I still want to know about prefixes / if everything is readable ... writing a secrets backend won't help ... we cannot have 2 in parallel ... then need to think about who can write / if users can write ...

epssy commented 6 years ago

Users can read anything they want. Preferably it would be nice to limit it to just configuration values from the /global path and from their own projects path /my_project. However, at the end of the day if they want to do something stupid and rely on another projects configuration and that changes on them and kills their app that's their problem.

I'm not trying to make another secrets backend. In my second post I did mentioned duplicating secret_storage.rb and the hashicorp_vault_backend.rb as something like consul_storage.rb and hashicorp_consul_backend.rb because they are so generic they'd almost work as is against Consul. This would be an entirely different class and call path as I pointed out though, something like Samson::Consul.

The only place secrets and configuration would mix at all is that it'd make sense to have the administrative tasks for managing them on the same page. Though this would probably cause the name of that page to change from Secrets to something else.

jonmoter commented 6 years ago

Authentication shouldn't be a concern as engineers can populate ENV key values at will for their projects.

Right, that's not going to fly. We can't have a completely wide-open, unauthenticated key-value store holding configuration for production apps. The same way we don't leave our config files world-writeable for anyone to change at their whim.

grosser commented 6 years ago

I'd opt for a new consul UI since it won't mix well with secrets.

How does samson know which configuration to use ... for example on a deploy of master vs staging vs pod7 ?

For now we could not do the write backend ... it might already be a nice feature to read from consul to env ...

On Thu, Aug 31, 2017 at 7:56 AM, Jon Moter notifications@github.com wrote:

Authentication shouldn't be a concern as engineers can populate ENV key values at will for their projects.

Right, that's not going to fly. We can't have a completely wide-open, unauthenticated key-value store holding configuration for production apps. The same way we don't leave our config files world-writeable for anyone to change at their whim.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/zendesk/samson/issues/2217#issuecomment-326321794, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAsZ5SDuvGqSSZS_yPNcZwUFmtuxP41ks5sdsmegaJpZM4PEJah .

epssy commented 6 years ago

We can't have a completely wide-open, unauthenticated key-value store holding configuration for production apps.

Currently engineers update their production configuration via ENV values populated in Project Settings. This proposal is looking to extend that existing workflow with the option to store and source configuration from Consul KV. It'd be fully managed by Samson and Consul probably would strictly enforce ACLs by Project if we wanted extra security.

Those ACLs would be something like {/project/key/space read, write} and {/global read}.

I'd opt for a new consul UI since it won't mix well with secrets.

Works for me. As long as there is a way for engineers to populate their key values for their project and review what keys are available in the global keypath. The Secrets and Consul UIs should easily link to each other as they tend to be used at the same time (username in one, password in the other).

How does samson know which configuration to use ...

As per my original post that information would be encoded into the Consul key path in the same way it is for Vault secrets. The path is arbitrary and you're more than welcome to set it to what ever make sense.

For now we could not do the write backend ... it might already be a nice feature to read from consul to env

Would totally be happy to just see this part. The original reason I made this request is that I wanted to provide a set of truths that we know about on the infrastructure side that we currently can't easily pass to engineering, and should be able to. This makes up the /global part of the original request.

The rest of the request was from other teams who wanted to be able to dynamically read in new configuration at run time and manage that via Samson.

grosser commented 6 years ago

So let's start discussing reading values from consul.

Should the values be in the pod spec (as ENV vars that are not updated during restart ... might be simple, but then samson has to read from remove DCs to get the values for each deploy group) or dynamically read by the container when starting (like secret puller works ... would require a new sidecar or library in the main container) ?

If it is read-only then we don't need a UI ? ... users would configure the keys they want via kubernetes/*.yml and just deploy that ... somewhat how the ENV setup in the yml files or samson/required_env works ?

Do we need restrictions on who can read which keys ... can all projects read all keys (global / from other projects) ?

Do we need to make it discoverable which projects use what configuration ? (who uses /global/foobar ?) ... then it might make sense to keep all consul lookups in samson UI for example as replacement with consul:// ... then we can add a UI that shows all used values ... but would lead to the same "samson needs to read from remote consul" issue.

epssy commented 6 years ago

Should the values be in the pod spec ... or dynamically read by the container when starting ...?

I don't have an opinion on this. What ever people would find most useful and is easiest to implement.

My guess is that keeping it consistent with how Secrets works will be easiest for users.

If it is read-only then we don't need a UI ?

If we go with just /global then there's no need for a UI. Though it would probably be good to have some way for people to see the currently set values for each key so they can pick the right one for their needs.

Having the paths set in a Kubernetes yaml file that's used to populate ENV at run time works. Though what would the equivalent be for non-Kubernetes projects? I guess we could do the same and have a Cap step that enumerates a list of paths.

Do we need restrictions on who can read which keys ... can all projects read all keys (global / from other projects) ?

Don't mind. Though it seems like a reasonable stance would be that all projects can read all /global keys and then only keys in their project path /my_application if we do that part. Though the only reason for this would be to avoid awkward situations where teams delete or update a key in their project and break other projects.

Do we need to make it discoverable which projects use what configuration ?

That would be a pretty awesome feature for those who need to update keys often. I'd prefer if it were possible but it's certainly not a requirement. There may be other ways to do this in future anyway.

... would lead to the same "samson needs to read from remote consul" issue.

After reviewing how our Consul replication is setup it looks like you can hit any server in production and get /global. All writes need to go to one cluster but we can move that anywhere (like next to Samson). As for a per project namespace we don't currently have anything setup so we can do that how ever fits best with how you want to write it into Samson (per project, per dc+pod+env, globally replicated et al).

grosser commented 6 years ago

The easiest thing I can think of would be adding new annotations that then get resolved by the samson_secret_puller gem. Pro:

... which leads me to the question of: how about we simply make a new gem that reads from consul and writes to the ENV ... we can use that from VMs and kubernetes and don't need any other overhead ...

ConsulPuller.set( ''/global/foobar" => "ZENDESK_FOOBAR" )

... we could still add a consul UI to samson after that (or as a standalone project) to get more visibility, but it would give us what we need ...

On Wed, Oct 4, 2017 at 9:33 PM, Edward Savage notifications@github.com wrote:

Should the values be in the pod spec ... or dynamically read by the container when starting ...?

I don't have an opinion on this. What ever people would find most useful and is easiest to implement.

My guess is that keeping it consistent with how Secrets works will be easiest for users.

If it is read-only then we don't need a UI ?

If we go with just /global then there's no need for a UI. Though it would probably be good to have some way for people to see the currently set values for each key so they can pick the right one for their needs.

Having the paths set in a Kubernetes yaml file that's used to populate ENV at run time works. Though what would the equivalent be for non-Kubernetes projects? I guess we could do the same and have a Cap step that enumerates a list of paths.

Do we need restrictions on who can read which keys ... can all projects read all keys (global / from other projects) ?

Don't mind. Though it seems like a reasonable stance would be that all projects can read all /global keys and then only keys in their project path /my_application if we do that part. Though the only reason for this would be to avoid awkward situations where teams delete or update a key and break other projects.

Do we need to make it discoverable which projects use what configuration ?

That would be a pretty awesome feature for those who need to update keys often. I'd prefer if it were possible but it's certainly not a requirement. There may be other ways to do this in future anyway.

... would lead to the same "samson needs to read from remote consul" issue.

After reviewing how our Consul replication is setup it looks like you can hit any server in production and get /global. All writes need to go to one cluster but we can move that anywhere (like next to Samson). As for a per project namespace we don't currently have anything setup so we can do that how ever fits best with how you want to write it into Samson (per project, per dc+pod+env, globally replicated et al).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/zendesk/samson/issues/2217#issuecomment-334356213, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAsZ8yoSHwdC2nhEw3EK9yblRrF4d7Rks5spFwLgaJpZM4PEJah .

epssy commented 6 years ago

That works for me. We can self management later when people are using it.

grosser commented 6 years ago

You can try this library to read values https://github.com/grosser/consulenv or https://github.com/hashicorp/envconsul

let's re-open this if we need more than that ...