Update chart and tests to Helm 3

CHANGELOG.md

@@ -66,21 +66,21 @@ Features:
 
 * Added `extraSecretEnvironmentVars` to allow users to mount secrets as
   environment variables
-* Added `tlsDisable` configurable to change HTTP protocols from HTTP/HTTPS 
+* Added `tlsDisable` configurable to change HTTP protocols from HTTP/HTTPS
   depending on the value
-* Added `serviceNodePort` to configure a NodePort value when setting `serviceType` 
+* Added `serviceNodePort` to configure a NodePort value when setting `serviceType`
   to "NodePort"
 
 Improvements:
 
 * Changed UI port to 8200 for better HTTP protocol support
-* Added `path` to `extraVolumes` to define where the volume should be 
+* Added `path` to `extraVolumes` to define where the volume should be
   mounted.  Defaults to `/vault/userconfig`
 * Upgraded Vault to 1.2.2
 
 Bugs:
 
-* Fixed bug where upgrade would fail because immutable labels were being 
+* Fixed bug where upgrade would fail because immutable labels were being
   changed (Helm Version label)
 * Fixed bug where UI service used wrong selector after updating helm labels
 * Added `VAULT_API_ADDR` env to Vault pod to fixed bug where Vault thinks

Chart.yaml

@@ -1,4 +1,4 @@
-apiVersion: v1
+apiVersion: v2
 name: vault
 version: 0.3.1
 description: Install and configure Vault on Kubernetes.

README.md

@@ -16,7 +16,7 @@ of this README. Please refer to the Kubernetes and Helm documentation.
 
 The versions required are:
 
-  * **Helm 2.10+** - This is the earliest version of Helm tested. It is possible
+  * **Helm 3.0+** - This is the earliest version of Helm tested. It is possible
     it works with earlier versions but this chart is untested for those versions.
   * **Kubernetes 1.9+** - This is the earliest version of Kubernetes tested.
     It is possible that this chart works with earlier versions but it is
@@ -36,3 +36,157 @@ then be installed directly:
 Please see the many options supported in the `values.yaml`
 file. These are also fully documented directly on the
 [Vault website](https://www.vaultproject.io/docs/platform/k8s/helm.html).
+
+## Testing
+
+The Helm chart ships with both unit and acceptance tests.
+
+The unit tests don't require any active Kubernetes cluster and complete
+very quickly. These should be used for fast feedback during development.
+The acceptance tests require a Kubernetes cluster with a configured `kubectl`.
+
+### Prequisites
+* [Bats](https://github.com/bats-core/bats-core)
+  ```bash
+  brew install bats-core
+  ```
+* [yq](https://pypi.org/project/yq/)
+  ```bash
+  brew install python-yq
+  ```
+* [helm](https://helm.sh)
+  ```bash
+  brew install helm
+  ```
+
+### Running The Tests
+
+To run the unit tests:
+
+    bats ./test/unit
+
+To run the acceptance tests:
+
+    bats ./test/acceptance
+
+If the acceptance tests fail, deployed resources in the Kubernetes cluster
+may not be properly cleaned up. We recommend recycling the Kubernetes cluster to
+start from a clean slate.
+
+**Note:** There is a Terraform configuration in the
+[`test/terraform/`](https://github.com/hashicorp/vault-helm/tree/master/test/terraform) directory
+that can be used to quickly bring up a GKE cluster and configure
+`kubectl` and `helm` locally. This can be used to quickly spin up a test
+cluster for acceptance tests. Unit tests _do not_ require a running Kubernetes
+cluster.
+
+### Writing Unit Tests
+
+Changes to the Helm chart should be accompanied by appropriate unit tests.
+
+#### Formatting
+
+- Put tests in the test file in the same order as the variables appear in the `values.yaml`.
+- Start tests for a chart value with a header that says what is being tested, like this:
+    ```
+    #--------------------------------------------------------------------
+    # annotations
+    ```
+
+- Name the test based on what it's testing in the following format (this will be its first line):
+    ```
+    @test "<section being tested>: <short description of the test case>" {
+    ```
+
+    When adding tests to an existing file, the first section will be the same as the other tests in the file.
+
+#### Test Details
+
+[Bats](https://github.com/bats-core/bats-core) provides a way to run commands in a shell and inspect the output in an automated way.
+In all of the tests in this repo, the base command being run is [helm template](https://docs.helm.sh/helm/#helm-template) which turns the templated files into straight yaml output.
+In this way, we're able to test that the various conditionals in the templates render as we would expect.
+
+Each test defines the files that should be rendered using the `-x` flag, then it might adjust chart values by adding `--set` flags as well.
+The output from this `helm template` command is then piped to [yq](https://pypi.org/project/yq/).
+`yq` allows us to pull out just the information we're interested in, either by referencing its position in the yaml file directly or giving information about it (like its length).
+The `-r` flag can be used with `yq` to return a raw string instead of a quoted one which is especially useful when looking for an exact match.
+
+The test passes or fails based on the conditional at the end that is in square brackets, which is a comparison of our expected value and the output of  `helm template` piped to `yq`.
+
+The `| tee /dev/stderr ` pieces direct any terminal output of the `helm template` and `yq` commands to stderr so that it doesn't interfere with `bats`.
+
+#### Test Examples
+
+Here are some examples of common test patterns:
+
+- Check that a value is disabled by default
+
+    ```
+    @test "ui/Service: no type by default" {
+      cd `chart_dir`
+      local actual=$(helm template \
+          -x templates/ui-service.yaml  \
+          . | tee /dev/stderr |
+          yq -r '.spec.type' | tee /dev/stderr)
+      [ "${actual}" = "null" ]
+    }
+    ```
+
+    In this example, nothing is changed from the default templates (no `--set` flags), then we use `yq` to retrieve the value we're checking, `.spec.type`.
+    This output is then compared against our expected value (`null` in this case) in the assertion `[ "${actual}" = "null" ]`.
+
+
+- Check that a template value is rendered to a specific value
+    ```
+    @test "ui/Service: specified type" {
+      cd `chart_dir`
+      local actual=$(helm template \
+          -x templates/ui-service.yaml  \
+          --set 'ui.serviceType=LoadBalancer' \
+          . | tee /dev/stderr |
+          yq -r '.spec.type' | tee /dev/stderr)
+      [ "${actual}" = "LoadBalancer" ]
+    }
+    ```
+
+    This is very similar to the last example, except we've changed a default value with the `--set` flag and correspondingly changed the expected value.
+
+- Check that a template value contains several values
+    ```
+	@test "server/standalone-StatefulSet: custom resources" {
+	  cd `chart_dir`
+	  local actual=$(helm template \
+		  -x templates/server-statefulset.yaml  \
+		  --set 'server.standalone.enabled=true' \
+		  --set 'server.resources.requests.memory=256Mi' \
+		  --set 'server.resources.requests.cpu=250m' \
+		  . | tee /dev/stderr |
+		  yq -r '.spec.template.spec.containers[0].resources.requests.memory' | tee /dev/stderr)
+	  [ "${actual}" = "256Mi" ]
+
+	  local actual=$(helm template \
+		  -x templates/server-statefulset.yaml  \
+		  --set 'server.standalone.enabled=true' \
+		  --set 'server.resources.limits.memory=256Mi' \
+		  --set 'server.resources.limits.cpu=250m' \
+		  . | tee /dev/stderr |
+		  yq -r '.spec.template.spec.containers[0].resources.limits.memory' | tee /dev/stderr)
+	  [ "${actual}" = "256Mi" ]
+    ```
+
+    *Note:* If testing more than two conditions, it would be good to separate the `helm template` part of the command from the `yq` sections to reduce redundant work.
+
+- Check that an entire template file is not rendered
+    ```
+    @test "syncCatalog/Deployment: disabled by default" {
+      cd `chart_dir`
+      run helm template \
+          -x templates/server-statefulset.yaml  \
+          --set 'global.enabled=false' \
+          .
+      [ "$status" -eq 1 ]
+    }
+    ```
+    Here we are checking to ensure that the template isn't rendered by verifying that `helm template` exits with an error code.
+
+    *Note:* This test will not work correctly if you put any comments before the conditional statement.

templates/server-disruptionbudget.yaml

@@ -1,7 +1,7 @@
-# PodDisruptionBudget to prevent degrading the server cluster through
-# voluntary cluster changes.
 {{ template "vault.mode" . }}
 {{- if and (and (eq (.Values.global.enabled | toString) "true") (eq .mode "ha")) (eq (.Values.server.ha.disruptionBudget.enabled | toString) "true") -}}
+# PodDisruptionBudget to prevent degrading the server cluster through
+# voluntary cluster changes.
 apiVersion: policy/v1beta1
 kind: PodDisruptionBudget
 metadata:

templates/server-service.yaml

@@ -1,5 +1,5 @@
-# Service for Vault cluster
 {{- if and (eq (.Values.server.service.enabled | toString) "true" ) (eq (.Values.global.enabled | toString) "true") }}
+# Service for Vault cluster
 apiVersion: v1
 kind: Service
 metadata:

templates/server-statefulset.yaml

@@ -1,6 +1,6 @@
-# StatefulSet to run the actual vault server cluster.
 {{ template "vault.mode" . }}
 {{- if and (ne .mode "") (eq (.Values.global.enabled | toString) "true") }}
+# StatefulSet to run the actual vault server cluster.
 apiVersion: apps/v1
 kind: StatefulSet
 metadata:
@@ -122,7 +122,7 @@ spec:
         {{- end }}
           lifecycle:
             # Vault container doesn't receive SIGTERM from Kubernetes
-            # and after the grace period ends, Kube sends SIGKILL.  This 
+            # and after the grace period ends, Kube sends SIGKILL.  This
             # causes issues with graceful shutdowns such as deregistering itself
             # from Consul (zombie services).
             preStop:

templates/ui-service.yaml

@@ -1,11 +1,11 @@
 {{ template "vault.mode" . }}
 {{- if and (ne .mode "") (eq (.Values.global.enabled | toString) "true") }}
+{{- if eq (.Values.ui.enabled | toString) "true" }}
 # Headless service for Vault server DNS entries. This service should only
 # point to Vault servers. For access to an agent, one should assume that
 # the agent is installed locally on the node and the NODE_IP should be used.
 # If the node can't run a Vault agent, then this service can be used to
 # communicate directly to a server agent.
-{{- if eq (.Values.ui.enabled | toString) "true" }}
 apiVersion: v1
 kind: Service
 metadata:

test/docker/Test.dockerfile

@@ -37,7 +37,7 @@ RUN curl -LO https://storage.googleapis.com/kubernetes-release/release/$(curl -s
     mv ./kubectl /usr/local/bin/kubectl
 
 # helm
-RUN curl https://raw.githubusercontent.com/kubernetes/helm/master/scripts/get | bash
+RUN curl https://raw.githubusercontent.com/kubernetes/helm/master/scripts/get-helm-3 | bash
 
 # bats
 RUN curl -sSL https://github.com/bats-core/bats-core/archive/v${BATS_VERSION}.tar.gz -o /tmp/bats.tgz \

test/terraform/main.tf

@@ -1,7 +1,3 @@
-locals {
-  service_account_path = "${path.module}/service-account.yaml"
-}
-
 provider "google" {
   project = "${var.project}"
   region  = "us-central1"
@@ -91,19 +87,3 @@ resource "null_resource" "kubectl" {
     command    = "kubectl config get-contexts | grep ${google_container_cluster.cluster.name} | xargs -n1 kubectl config delete-context"
   }
 }
-
-resource "null_resource" "helm" {
-  count      = "${var.init_cli ? 1 : 0 }"
-  depends_on = ["null_resource.kubectl"]
-
-  triggers = {
-    cluster = "${google_container_cluster.cluster.id}"
-  }
-
-  provisioner "local-exec" {
-    command = <<EOF
-kubectl apply -f '${local.service_account_path}'
-helm init --service-account helm
-EOF
-  }
-}

test/terraform/variables.tf

@@ -15,7 +15,7 @@ variable "zone" {
 
 variable "init_cli" {
   default     = true
-  description = "Whether to init the CLI tools kubectl, helm, etc. or not."
+  description = "Whether to init kubectl or not."
 }
 
 variable "gcp_service_account" {

test/unit/server-clusterrolebinding.bats

@@ -4,56 +4,54 @@ load _helpers
 
 @test "server/ClusterRoleBinding: enabled by default" {
   cd `chart_dir`
-  local actual=$(helm template \
-      -x templates/server-clusterrolebinding.yaml  \
+  run helm template \
+      --show-only templates/server-clusterrolebinding.yaml  \
       --set 'server.dev.enabled=true' \
-      . | tee /dev/stderr |
-      yq 'length > 0' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
+      .
+  [ "$status" -eq 1 ]
 
-  local actual=$(helm template \
-      -x templates/server-clusterrolebinding.yaml  \
+  run helm template \
+      --show-only templates/server-clusterrolebinding.yaml  \
       --set 'server.ha.enabled=true' \
-      . | tee /dev/stderr |
-      yq 'length > 0' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
+      .
+  [ "$status" -eq 1 ]
 
-  local actual=$(helm template \
-      -x templates/server-clusterrolebinding.yaml  \
-      . | tee /dev/stderr |
-      yq 'length > 0' | tee /dev/stderr)
-  [ "${actual}" = "true" ]
+  run helm template \
+      --show-only templates/server-clusterrolebinding.yaml  \
+      .
+  [ "$status" -eq 1 ]
 }
 
 @test "server/ClusterRoleBinding: disable with global.enabled" {
   cd `chart_dir`
-  local actual=$(helm template \
-      -x templates/server-clusterrolebinding.yaml  \
+  run helm template \
+      --show-only templates/server-clusterrolebinding.yaml  \
       --set 'global.enabled=false' \
-      . | tee /dev/stderr |
-      yq 'length > 0' | tee /dev/stderr)
-  [ "${actual}" = "false" ]
+      .
+  [ "$status" -eq 1 ]
 }
 
 @test "server/ClusterRoleBinding: can disable with server.authDelegator" {
   cd `chart_dir`
   local actual=$(helm template \
-      -x templates/server-clusterrolebinding.yaml  \
+     --show-only templates/server-clusterrolebinding.yaml  \
       --set 'server.authDelegator.enabled=false' \
       . | tee /dev/stderr |
       yq 'length > 0' | tee /dev/stderr)
   [ "${actual}" = "false" ]
 
   local actual=$(helm template \
-      -x templates/server-clusterrolebinding.yaml  \
+      --show-only templates/server-clusterrolebinding.yaml  \
+      --set 'server.authDelegator.enabled=true' \
+      --show-only templates/server-clusterrolebinding.yaml  \
       --set 'server.authDelegator.enabled=false' \
       --set 'server.ha.enabled=true' \
       . | tee /dev/stderr |
       yq 'length > 0' | tee /dev/stderr)
   [ "${actual}" = "false" ]
 
   local actual=$(helm template \
-      -x templates/server-clusterrolebinding.yaml  \
+      --show-only templates/server-clusterrolebinding.yaml  \
       --set 'server.authDelegator.enabled=false' \
       --set 'server.dev.enabled=true' \
       . | tee /dev/stderr |