cloudfoundry-community / vault-boshrelease

Apache License 2.0
28 stars 35 forks source link

Add operators and make consul more secure by default #41

Closed z4ce closed 6 years ago

z4ce commented 6 years ago

Added some operators for different tasks, most notably switching the backend to s3. Also changed the default for consul deployments to use an encryption key.

drnic commented 6 years ago

@z4ce general bosh/operator file organization question/observation - could azs.yml + network.yml + vm_type.yml be merged into a single operator file for overriding cloud-config names? (that's what they all have in common - you have different cloud-config features than the base manifest)

drnic commented 6 years ago

@z4ce bonus warning - @jhunt may ask you to split out the cloud-config-esque changes into a separate PR to keep the topics isolated.

z4ce commented 6 years ago

@drnic I'm inclined to believe they should remain separate. It seems entirely reasonable to think you'd only want to use one in isolation. Personally, I'd really like to see bosh allow text variables with defaults in the manifests, then the operators wouldn't be necessary at all.. a syntax like

variables:
- name: vm-type
  type: text
  default: large
z4ce commented 6 years ago

Also, I'm fine with breaking up the PR if so desired. Just let me know how you want it. The change set seems sufficiently small and the changes benign enough I didn't think issuing three separate PRs would make sense.

drnic commented 6 years ago

@z4ce I've pondered this too. I asked in #bosh https://cloudfoundry.slack.com/archives/C02HPPYQ2/p1510607831000626

drnic commented 6 years ago

@z4ce yes please, create a second PR for the cloud-config specific operator files (or since most of this conversation is on this topic, perhaps create a new PR for the original "make consul more secure" commit)

drnic commented 6 years ago

@z4ce @jhunt question, should we add encrypt: ((consul-encrypt-key)) to the base manifest?

z4ce commented 6 years ago

I can think of no reason to not include it by default, and it will be much more secure.. the way it is deployed by default right now anyone with network access could join the consul cluster.. which isn't optimal

drnic commented 6 years ago

Will it "just work" if someone enables encryption on an existing unencrypted deployment? Or will bad things happen?

If bad things, how should we treat upgraders? Perhaps we keep it as a separate operator file initially, add warnings to release notes that we will default to encryption in future. Later, we put encryption in the base manifest and include a "manifests/operators/disable-encryption.yml" for upgraders who cannot enable encryption.

jhunt commented 6 years ago

Is the concern of random clients joining the consul cluster that they will see Vaultified credentials?

(Incidentally, more and more, this is why I don't even bother spinning "highly-available" vault -- file-backed storage works just fine for me)

z4ce commented 6 years ago

@jhunt yes they could see the paths within the vault store, delete values, etc. I'm closing this PR and opening new ones for each item...