Feature/ddb backend #77

BonzTM · 2018-07-05T14:40:44Z

Issue: #76

Adding functionality for DynamoDB HA storage backend in the run-vault script. Cannot be used in conjunction with S3 storage backend.
Documented usage in README.
Added a small if statement to check if supervisor is already installed and to skip (for airgapped installations that have no outbound access).
Fixed typo in print_usage function causing it to bomb out.
Added actual Dynamo TF infrastructure. Basic DDB table (No Autoscale, Global Tables etc). No logic added to main module to disable Consul if DDB enabled at this time. Only at the nested module level.
Quick example of code in the examples/ folder using the S3 example as a template. Not really any different.

BonzTM · 2018-07-05T14:42:08Z

modules/run-vault/run-vault

@@ -35,7 +35,10 @@ function print_usage {
  echo -e "  --skip-vault-config\tIf this flag is set, don't generate a Vault configuration file. Optional. Default is false."
  echo -e "  --enable-s3-backend\tIf this flag is set, an S3 backend will be enabled in addition to the HA Consul backend. Default is false."
  echo -e "  --s3-bucket\tSpecifies the S3 bucket to use to store Vault data. Only used if '--enable-s3-backend' is set."
-  echo -e "  --s3-bucket-region\tSpecifies the AWS region where `--s3-bucket` lives. Only used if `--enable-s3-backend` is set."


backticks were previously used causing this function to behave unexpectedly. Replaced with single quotes per all other items in the list.

Whoops! Good fix, thanks!

BonzTM · 2018-07-05T14:42:29Z

modules/run-vault/run-vault

  local readonly config_path="$config_dir/$VAULT_CONFIG_FILE"

  local instance_ip_address
  instance_ip_address=$(get_instance_ip_address)

  log_info "Creating default Vault config file in $config_path"
  local readonly listener_config=$(cat <<EOF
+ui = true


Enabled vault UI by default

Thx! We should probably make this configurable though with a --disable-ui flag for those that want to leave it off.

BonzTM · 2018-07-05T17:32:54Z

modules/vault-cluster/main.tf

@@ -194,15 +194,46 @@ resource "aws_s3_bucket" "vault_storage" {
  }
 }

+resource "aws_dynamodb_table" "vault_dynamo" {


Default dynamo table with small read/write capacity. Anything beyond (autoscaled, global tables) would need to be added or used in client code.

…rawing

brikis98

This is a fantastic PR, thank you! We were just talking about adding DynamoDB as a backend, so this is great timing.

I've made some detailed code comments below. One high level comment: we should add an automated test for this and make sure all existing tests pass. See the test folder for details.

brikis98 · 2018-07-06T09:58:00Z

examples/vault-ddb-backend/README.md

+This example creates a Vault cluster spread across the subnets in the default VPC of the AWS account. For an example of a Vault cluster
+that is publicly accessible, see [vault-cluster-public](https://github.com/hashicorp/terraform-aws-vault/tree/master/examples/vault-cluster-public).
+
+![Vault architecture]()


Missing image?

I'm not 100% certain where to get a documentation template for Cloudcraft that includes the Hashicorp custom images. If you could point me in the right direction that would be great.

@brikis98 Do you have a template used for previous diagrams?

Yes, I do. I created the diagrams with CloudCraft. Let me know if these shared links work for you:

https://cloudcraft.co/view/9cf13b28-3c9b-47a6-b8f5-79b4f4be97e6?key=H-BXpuoP1I7hyjLm4LcO-Q
https://cloudcraft.co/view/8607afd0-2993-4dd9-87b4-1ada2790b3d6?key=Xlcx8IEgf9A_jBiCBf0sFA

brikis98 · 2018-07-06T09:59:23Z

examples/vault-ddb-backend/README.md

+To deploy a Vault Cluster:
+
+1. `git clone` this repo to your computer.
+1. Optional: build a Vault and Consul AMI. See the [vault-consul-ami


Optional: build a Vault AMI. See the [vault-consul-ami example](https://github.com/hashicorp/terraform-aws-vault/tree/master/examples/vault-consul-ami) documentation for instructions on how to build an AMI that has both Vault and Consul installed (note that for this example, you'll only need Vault, but having both won't hurt anything).

brikis98 · 2018-07-06T09:59:38Z

examples/vault-ddb-backend/main.tf

+# ---------------------------------------------------------------------------------------------------------------------
+# DEPLOY A VAULT SERVER CLUSTER AND A CONSUL SERVER CLUSTER IN AWS
+# This is an example of how to use the vault-cluster module to deploy a Vault cluster in AWS. This cluster uses Consul,
+# running in a separate cluster, as its storage backend.


Update comment

brikis98 · 2018-07-06T09:59:52Z

examples/vault-ddb-backend/main.tf

+  user_data = "${data.template_file.user_data_vault_cluster.rendered}"
+
+  enable_dynamo_backend = true
+  dynamo_table_name     = "${var.dynamo_table_name}"


Nice API 👍

brikis98 · 2018-07-06T10:00:15Z

examples/vault-ddb-backend/main.tf

+    aws_region               = "${data.aws_region.current.name}"
+    s3_bucket_name           = "${var.s3_bucket_name}"
+    consul_cluster_tag_key   = "${var.consul_cluster_tag_key}"
+    consul_cluster_tag_value = "${var.consul_cluster_name}"


I assume these Consul and S3 params are no longer necessary, either as input variables, or to pass to User Data?

Ahh yes. Removing these and adding the Dynamo table variable.

brikis98 · 2018-07-06T10:06:21Z

modules/run-vault/run-vault

-  local readonly consul_storage=$(cat <<EOF
+
+  if [[ "$enable_dynamo" == "true" ]]; then
+    consul_storage=""


Please declare local consul_storage and local vault_storage up above this if/else block. That way, both default to empty strings, and only one gets set to a proper value.

Good call. Adjusting.

brikis98 · 2018-07-06T10:08:42Z

modules/run-vault/run-vault

+    local readonly enable_s3_backend=""
+    local readonly s3_bucket=""
+    local readonly s3_bucket_region=""
+  fi


I find changing the list of input params a bit confusing. I'd recommend making this a single list:

local readonly enable_s3_backend="$8" local readonly s3_bucket="$9" local readonly s3_bucket_region="${10}" local readonly enable_dynamo="${11}" local readonly dynamo_region="${12}" local readonly dynamo_table="${13}"

The run method should pass all these params, setting them to true, false, empty strings, or proper values, depending on what the user passed in.

I agree, however, by default the vault config generation just adds the S3 params at the end (no harm if they are empty). I can add extra vault config generation logic (at the bottom) if you'd rather see this as a list.

Changed it back to a list and updated logic at the bottom (vault generation script) to match

brikis98 · 2018-07-06T10:09:21Z

modules/vault-cluster/main.tf

+  }
+
+  tags {
+    Description = "Used for HA storage with Vault."


I believe we already take in a list of custom tags; we should use it here.

Added Name tag. Didn't see any other list of tags anywhere.

brikis98 · 2018-07-06T10:10:28Z

modules/vault-cluster/outputs.tf

+
+output "dynamo_table_arn" {
+  value = "${aws_dynamodb_table.vault_dynamo.arn}"


This won't work, as aws_dynamodb_table might not be created. Instead, use:

value = "${element(concat(aws_dynamodb_table.vault_dynamo.*.arn, list("")), 0)}"

brikis98 · 2018-07-06T10:10:58Z

modules/vault-cluster/variables.tf

+
+variable "dynamo_write_capacity" {
+  description = "Sets the DynamoDB write capacity for storage backend"
+  default     = "5"


Should these be numbers (i.e., not wrapped in quotes)?

They can be but don't have to be. Changed them for consistency. Typically as tf only has 3 types of variables, I wrap all 'string' variables in double quotes for consistency. But I see this isn't done in this module.

…mo table

…ic at bottom to check for s3, dynamo, or else

…sistency

BonzTM · 2018-07-06T12:27:18Z

I'm merely a novice when it comes to software development. I'll do my best to look over the tests and see what I can come up with in the meantime.

brikis98 · 2018-07-06T12:42:17Z

I'm merely a novice when it comes to software development. I'll do my best to look over the tests and see what I can come up with in the meantime.

Well, learning automated testing is a very valuable skill, and we're happy to help :)

There's lots of examples already in the repo, so you should be able to follow their structure. The first step, of course, is to test everything manually and make sure it works! Once it works manually, automate those steps as shown in the existing tests in the test folder. If you get stuck, don't hesitate to ask. Thank you!

BMonsalvatge · 2018-07-20T23:22:09Z

modules/run-vault/run-vault

+  if [[ "$enable_dynamo" == "true" ]]; then
+    consul_storage=""
+    vault_storage=$(cat <<EOF
+storage "dynamodb" {


In order to have both dynamodb and s3 backend we would need to be able to make that block similar to how its handled for consul. Currently what I have adjusted on my end, and is working so far with manual testing:

local consul_storage_type="storage" local dynamodb_storage_type="storage" local s3_config="" local consul_storage="" local vault_storage="" if [[ "$enable_s3_backend" == "true" ]]; then s3_config=$(cat <<EOF storage "s3" { bucket = "$s3_bucket" region = "$s3_bucket_region" }\n EOF ) consul_storage_type="ha_storage" dynamodb_storage_type="ha_storage" fi if [[ "$enable_dynamo_backend" == "true" ]]; then vault_storage=$(cat <<EOF $dynamodb_storage_type "dynamodb" { ha_enabled = "true" region = "$dynamo_region" table = "$dynamo_table" } # HA settings cluster_addr = "https://$instance_ip_address:$cluster_port" api_addr = "$api_addr" EOF )

jp · 2018-09-27T19:29:13Z

Hello, I'm checking here and it seems to be awesome and very useful, that would be sad to have this getting stale ;)
Is there anything I could do to help? I'm definitely going to try this branch and provide feedback.

BonzTM · 2018-10-01T13:45:45Z

Hello, I'm checking here and it seems to be awesome and very useful, that would be sad to have this getting stale ;)
Is there anything I could do to help? I'm definitely going to try this branch and provide feedback.

@jp I ended up going back a different direction (Consul) due to certain encryption specifics. This module with DynamoDB was in use and functioning, I just never wrote the testing pieces for it to be merged. I haven't really had the time to do so.

You're more than welcome to update the testing functionality and/or update the functionality to resolve the conflicts into the master branch if you'd like.

idris · 2018-10-30T16:01:59Z

@brikis98 can someone at Hashicorp take a look at this to take it over the finish line?

@BonzTM

* Backticks causing unexpected behavior * Removes backticks in favor of single quote to match other items in the list * Credit to @BonzTM in hashicorp#77 for initially finding/fixing; isolated to get fix merged

luizm · 2019-05-30T17:15:20Z

It's a great PR, someone is looking at this? 😄

felipejfc · 2019-08-02T15:08:40Z

This PR needed rebase to resolve conflicts with master, I've made them and it's working great, do you guys want to go ahead with this? If so I can send an updated PR.

Best

hashicorp-cla · 2020-06-08T02:32:38Z

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

0 out of 2 committers have signed the CLA.

A120670
Joshua Wells

Joshua Wells seems not to be a GitHub user.
You need a GitHub account to be able to sign the CLA. If you already have a GitHub account, please add the email address used for this commit to your account.

_{Have you signed the CLA already but the status is still pending? Recheck it.}

pgr-josh-wells added 5 commits July 5, 2018 09:59

added checking for supervisor existence before installing

d6f5711

added dynamo flags and logic to use either s3 or dynamo

6544406

correct useradd with --system flag

a8e6b72

updated dynamo checking for config generation

6cd9bfa

add dynamo documentation

fdc8fac

BonzTM commented Jul 5, 2018

View reviewed changes

pgr-josh-wells added 4 commits July 5, 2018 13:21

tf fmt only

6d308d2

adding standard dynamo table and policy to iam role

966df0a

adding dynamo arn output

36a74d7

adding vault dynamo variables (read/write and name)

0761984

BonzTM commented Jul 5, 2018

View reviewed changes

added tf example for vault with DDB backend. lacking infrastructure d…

36e1b94

…rawing

brikis98 suggested changes Jul 6, 2018

View reviewed changes

Joshua Wells added 14 commits July 6, 2018 07:32

updated main comment to specify ddb

8d539cd

updated README to specify using a vault ami only

14c4e94

updated userdata 'data' block to remove consul variables and add dyna…

6a55724

…mo table

set consul_storage and vault_storage to and removed redundant logic

93fb7b6

removed logic changing dynamo/s3 values and made just list. Added log…

f3020a8

…ic at bottom to check for s3, dynamo, or else

added Name tag to DDB

2df7148

updated dyanmo output as concat element

ffabbb8

updated ddb read/write string variables without quotes

10b5cb1

update userdata comment

011d71a

updated enable-dynamo and enable_dynamo to include 'backend' with con…

0b74d0a

…sistency

updated enable dynamo flag in userdata

7449675

updated readme to include enable-dynamo-backend references

45f7658

update generate vault config line to include all as list

c5f0603

remove checking supervisor pip

481c675

BMonsalvatge reviewed Jul 20, 2018

View reviewed changes

pgr-josh-wells added 2 commits July 31, 2018 14:23

updated to allow s3 with ddb backend

773d47a

tf fmt

c8d9fab

briandbecker mentioned this pull request Dec 10, 2018

Replace backtick in run-vault with single quote #120

Merged

stephencoe mentioned this pull request Feb 4, 2020

Adding support for DynamoDB backend #181

Merged

BonzTM closed this Feb 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/ddb backend #77

Feature/ddb backend #77

BonzTM commented Jul 5, 2018 •

edited

BonzTM Jul 5, 2018

brikis98 Jul 6, 2018

BonzTM Jul 5, 2018

brikis98 Jul 6, 2018

BonzTM Jul 5, 2018

brikis98 left a comment

brikis98 Jul 6, 2018

BonzTM Jul 6, 2018

BonzTM Jul 31, 2018

brikis98 Jul 31, 2018

brikis98 Jul 6, 2018

brikis98 Jul 6, 2018

brikis98 Jul 6, 2018

brikis98 Jul 6, 2018

BonzTM Jul 6, 2018

brikis98 Jul 6, 2018

BonzTM Jul 6, 2018

brikis98 Jul 6, 2018

BonzTM Jul 6, 2018

BonzTM Jul 6, 2018

brikis98 Jul 6, 2018

BonzTM Jul 6, 2018

brikis98 Jul 6, 2018

brikis98 Jul 6, 2018

BonzTM Jul 6, 2018 •

edited

BonzTM commented Jul 6, 2018

brikis98 commented Jul 6, 2018

BMonsalvatge Jul 20, 2018 •

edited

jp commented Sep 27, 2018

BonzTM commented Oct 1, 2018

idris commented Oct 30, 2018

luizm commented May 30, 2019

felipejfc commented Aug 2, 2019

hashicorp-cla commented Jun 8, 2020 •

edited


		output "dynamo_table_arn" {
		value = "${aws_dynamodb_table.vault_dynamo.arn}"

Feature/ddb backend #77

Feature/ddb backend #77

Conversation

BonzTM commented Jul 5, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brikis98 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BonzTM Jul 6, 2018 • edited

Choose a reason for hiding this comment

BonzTM commented Jul 6, 2018

brikis98 commented Jul 6, 2018

BMonsalvatge Jul 20, 2018 • edited

Choose a reason for hiding this comment

jp commented Sep 27, 2018

BonzTM commented Oct 1, 2018

idris commented Oct 30, 2018

luizm commented May 30, 2019

felipejfc commented Aug 2, 2019

hashicorp-cla commented Jun 8, 2020 • edited

BonzTM commented Jul 5, 2018 •

edited

BonzTM Jul 6, 2018 •

edited

BMonsalvatge Jul 20, 2018 •

edited

hashicorp-cla commented Jun 8, 2020 •

edited