Merge pull request #27 from grapl-security/cm/fail-if-no-secrets

Fail if no secrets are provided

Author: christophermaier

.buildkite/pipeline.verify.yml

@@ -2,7 +2,7 @@ env:
   PANTS_CONFIG_FILES: "['pants.toml', 'pants.ci.toml']"
 
 steps:
-  - label: ":jeans: All files are covered by Pants"
+  - label: ":pants: All files are covered by Pants"
     command:
       - ./pants tailor --check
 
@@ -19,10 +19,9 @@ steps:
     command:
       - make lint-plugin
 
-  # TODO: Add some unit tests!
-  # - label: ":lint-roller::buildkite: Test Plugin"
-  #   command:
-  #     - make test-plugin
+  - label: ":lint-roller::buildkite: Test Plugin"
+    command:
+      - make test-plugin
 
   - label: ":buildkite::vault: Run Plugin 1"
     command: echo 'Hello World'

Makefile

@@ -1,38 +1,53 @@
 DOCKER_COMPOSE_CHECK := docker compose run --rm
 
-# Linting
+.PHONY: all
+all: format
+all: lint
+all: test
+all: ## Run all operations
+
+.PHONY: help
+help: ## Print this help
+	@awk 'BEGIN {FS = ":.*##"; printf "Usage: make <target>\n"} \
+		 /^[a-zA-Z0-9_-]+:.*?##/ { printf "  %-46s %s\n", $$1, $$2 } \
+		 /^##@/ { printf "\n%s\n", substr($$0, 5) } ' \
+		 $(MAKEFILE_LIST)
+	@printf '\n'
+
+##@ Linting
 ########################################################################
 
 .PHONY: lint
-lint: lint-plugin lint-shell
+lint: lint-plugin
+lint: lint-shell
+lint: ## Perform lint checks on all files
 
 .PHONY: lint-plugin
-lint-plugin:
+lint-plugin: ## Lint the Buildkite plugin metadata
 	$(DOCKER_COMPOSE_CHECK) plugin-linter
 
 .PHONY: lint-shell
-lint-shell:
+lint-shell: ## Lint the shell scripts
 	./pants lint ::
 
-# Formatting
+##@ Formatting
 ########################################################################
 
 .PHONY: format
 format: format-shell
+format: ## Automatically format all code
 
 .PHONY: format-shell
-format-shell:
+format-shell: ## Format shell scripts
 	./pants fmt ::
 
-# Testing
+##@ Testing
 ########################################################################
 
 .PHONY: test
 test: test-plugin
+test: ## Run all tests
 
 .PHONY: test-plugin
-test-plugin:
+test-plugin: ## Test the Buildkite plugin locally (does *not* run a Buildkite pipeline)
 	$(DOCKER_COMPOSE_CHECK) plugin-tester
-
-.PHONY: all
-all: format lint test

hooks/environment

@@ -2,15 +2,17 @@
 
 set -euo pipefail
 
+# shellcheck source-path=SCRIPTDIR
+source "$(dirname "${BASH_SOURCE[0]}")/../lib/log.sh"
+
 readonly default_image="hashicorp/envconsul"
 readonly default_tag="latest"
 readonly image="${BUILDKITE_PLUGIN_VAULT_ENV_IMAGE:-${default_image}}:${BUILDKITE_PLUGIN_VAULT_ENV_TAG:-${default_tag}}"
 
 # Fail if there is no Vault token; gotta log in first
 ########################################################################
 if [ -z "${VAULT_TOKEN:-}" ]; then
-    echo "--- :skull_and_crossbones: Could not find 'VAULT_TOKEN' in the environment!"
-    exit 1
+    raise_error "Could not find 'VAULT_TOKEN' in the environment!"
 fi
 
 # Resolve Vault address
@@ -20,8 +22,7 @@ if [ -n "${BUILDKITE_PLUGIN_VAULT_ENV_ADDRESS:-}" ]; then
     export VAULT_ADDR
 fi
 if [ -z "${VAULT_ADDR:-}" ]; then
-    echo "--- :skull_and_crossbones: Could not find 'VAULT_ADDR' in the environment, and 'BUILDKITE_PLUGIN_VAULT_ENV_ADDRESS' was not specified!"
-    exit 1
+    raise_error "Could not find 'VAULT_ADDR' in the environment, and 'BUILDKITE_PLUGIN_VAULT_ENV_ADDRESS' was not specified!"
 fi
 
 # Resolve Vault namespace
@@ -31,8 +32,7 @@ if [ -n "${BUILDKITE_PLUGIN_VAULT_ENV_NAMESPACE:-}" ]; then
     export VAULT_NAMESPACE
 fi
 if [ -z "${VAULT_NAMESPACE:-}" ]; then
-    echo "--- :skull_and_crossbones: Could not find 'VAULT_NAMESPACE' in the environment, and 'BUILDKITE_PLUGIN_VAULT_ENV_NAMESPACE' was not specified!"
-    exit 1
+    raise_error "Could not find 'VAULT_NAMESPACE' in the environment, and 'BUILDKITE_PLUGIN_VAULT_ENV_NAMESPACE' was not specified!"
 fi
 
 # Resolve secret prefix
@@ -45,10 +45,9 @@ if [[ -n "${secret_prefix}" && ! "${secret_prefix}" =~ /$ ]]; then
 fi
 readonly secret_prefix
 
+# Resolve secrets
 ########################################################################
 
-readonly container_name="vault-env-plugin-${BUILDKITE_JOB_ID}"
-
 # STOLEN FROM https://github.com/buildkite-plugins/docker-buildkite-plugin/blob/9f90d8ef742d9fa1eb3556720e16f2b842ff1cb2/hooks/command#L25-L47
 #
 # Reads a list from plugin config into a global result array
@@ -61,8 +60,7 @@ plugin_read_list_into_result() {
         local parameter="${prefix}_${i}"
 
         if [[ -n "${!prefix:-}" ]]; then
-            echo ":rotating_light: Plugin received a string for $prefix, expected an array" >&2
-            exit 1
+            raise_error "Plugin received a string for $prefix, expected an array"
         fi
 
         while [[ -n "${!parameter:-}" ]]; do
@@ -75,24 +73,32 @@ plugin_read_list_into_result() {
     [[ ${#result[@]} -gt 0 ]] || return 1
 }
 
-envconsul_env() {
-    # This populates a `result` array for later use
-    plugin_read_list_into_result BUILDKITE_PLUGIN_VAULT_ENV_SECRETS
+secrets=()
+if plugin_read_list_into_result BUILDKITE_PLUGIN_VAULT_ENV_SECRETS; then
+    secrets=("${result[@]}")
+else
+    raise_error "At least one secret must be specified!"
+fi
 
-    secrets=()
-    for secret in "${result[@]}"; do
+########################################################################
+
+readonly container_name="vault-env-plugin-${BUILDKITE_JOB_ID}"
+
+envconsul_env() {
+    secret_args=()
+    for secret in "${secrets[@]}"; do
         # secret_prefix is guaranteed to end with a / if it is non-empty
-        secrets+=("-secret=${secret_prefix}${secret}")
+        secret_args+=("-secret=${secret_prefix}${secret}")
     done
 
     # Explicitly *not* using `--rm` so we can output the container
     # logs in case of a failure.
-    docker run \
+    log_and_run docker run \
         --env VAULT_TOKEN \
         --name="${container_name}" \
         -- \
         "${image}" \
-        "${secrets[@]}" \
+        "${secret_args[@]}" \
         -once \
         -upcase \
         -pristine \
@@ -105,23 +111,23 @@ envconsul_env() {
 }
 
 cleanup() {
-    docker container rm --force "${container_name}" > /dev/null 2>&1
+    log_and_run docker container rm --force "${container_name}" > /dev/null 2>&1
 }
 
 trap cleanup EXIT INT QUIT
 
-echo "--- :vault: Pulling secrets from Vault"
-echo "Using Docker image: ${image}"
-echo "VAULT_ADDR=${VAULT_ADDR}"
-echo "VAULT_NAMESPACE=${VAULT_NAMESPACE}"
+log "--- :vault: Pulling secrets from Vault"
+log "Using Docker image: ${image}"
+log "VAULT_ADDR=${VAULT_ADDR}"
+log "VAULT_NAMESPACE=${VAULT_NAMESPACE}"
 
 if vault_env=$(envconsul_env); then
     set -o allexport
     eval "${vault_env}"
     set +o allexport
 else
     retval=$?
-    echo "--- :skull_and_crossbones: Failed to retrieve secrets from Vault"
-    docker container logs "${container_name}"
+    log "--- :skull_and_crossbones: Failed to retrieve secrets from Vault"
+    log_and_run docker container logs "${container_name}"
     exit ${retval}
 fi

lib/BUILD

@@ -0,0 +1 @@
+shell_sources()

lib/log.sh

@@ -0,0 +1,55 @@
+#!/usr/bin/env bash
+
+# Various logging functions and helpers to make logging nice.
+
+# ANSI Formatting Codes
+########################################################################
+# Because who wants to remember all those fiddly details?
+
+# CSI = "Control Sequence Introducer"
+CSI="\e["
+END=m
+
+NORMAL=0
+BOLD=1
+
+WHITE=37
+
+RESET="${CSI}${NORMAL}${END}"
+
+function _bold_color() {
+    color="${1}"
+    shift
+    echo "${CSI}${BOLD};${color}${END}${*}${RESET}"
+}
+
+function bright_white() {
+    _bold_color "${WHITE}" "${@}"
+}
+
+# Logging
+########################################################################
+# NOTE: All logs get sent to standard error
+
+function log() {
+    echo -e "${@}" >&2
+}
+
+# Handy for logging the exact command to be run, and then running it
+function log_and_run() {
+    log ❯❯ "$(bright_white "$(printf "%q " "${@}")")"
+    "$@"
+}
+
+raise_error() {
+    log "--- :rotating_light:" "${@}"
+    # Yes, these numbers are correct :/
+    if [ -z "${BASH_SOURCE[2]:-}" ]; then
+        # If we're calling raise_error from a script directly, we'll
+        # have a shorter call stack.
+        log "Failed in ${FUNCNAME[1]}() at [${BASH_SOURCE[1]}:${BASH_LINENO[0]}]"
+    else
+        log "Failed in ${FUNCNAME[1]}() at [${BASH_SOURCE[1]}:${BASH_LINENO[0]}], called from [${BASH_SOURCE[2]}:${BASH_LINENO[1]}]"
+    fi
+    exit 1
+}

tests/environment.bats

@@ -0,0 +1,50 @@
+#!/usr/bin/env bats
+
+load "$BATS_PLUGIN_PATH/load.bash"
+
+# Uncomment to enable stub debugging
+# export DOCKER_STUB_DEBUG=/dev/tty
+
+setup() {
+    export DEFAULT_IMAGE=hashicorp/envconsul
+    export DEFAULT_TAG=latest
+
+    export VAULT_TOKEN=testingtoken
+    export VAULT_ADDR=default.vault.mycompany.com:8200
+    export VAULT_NAMESPACE=default_namespace
+
+    export BUILDKITE_JOB_ID=2112
+}
+
+teardown() {
+    unset VAULT_TOKEN
+    unset VAULT_ADDR
+    unset VAULT_NAMESPACE
+
+    unset BUILDKITE_PLUGIN_VAULT_ENV_SECRETS_0
+}
+
+@test "The happy path works" {
+    export BUILDKITE_PLUGIN_VAULT_ENV_SECRETS_0="foo"
+
+    docker_vault_cmd="run --env VAULT_TOKEN --name=vault-env-plugin-${BUILDKITE_JOB_ID} -- ${DEFAULT_IMAGE}:${DEFAULT_TAG}"
+
+    stub docker \
+         "${docker_vault_cmd} -secret=foo -once -upcase -pristine -no-prefix=true -vault-addr=${VAULT_ADDR} -vault-namespace=${VAULT_NAMESPACE} -vault-renew-token=false -vault-retry-attempts=1 env : echo 'foo=something_very_secret'" \
+         "container rm --force vault-env-plugin-2112 : echo 'removing'"
+
+    run "${PWD}/hooks/environment"
+    assert_success
+
+    unstub docker
+}
+
+@test "Fails if no secrets are specified" {
+
+    unset BUILDKITE_PLUGIN_VAULT_ENV_SECRETS_0
+
+    run "${PWD}/hooks/environment"
+    assert_failure
+
+    assert_output --partial "At least one secret must be specified"
+}