feat: Chart optionally installs CRD / CR Manager configurable for mor…

…e strict clusters (#344) Coupling the CRD creation to a controller requires an unnecessarily privileged cluster role. In some clusters, admins would not be willing to grant the controller service account that role. We require the chart itself to install the CRD, and the service account or admin installing the chart would have the crd:create access while the controller be more tightly scoped. This essentially deprecates the custom resource manager so that it can removed entirely in future 4.x releases.
external-secrets · Jun 2, 2020 · 131e201 · 131e201
1 parent cceb40b
commit 131e201
Show file tree

Hide file tree

Showing 15 changed files with 208 additions and 47 deletions.
diff --git a/.dockerignore b/.dockerignore
@@ -1,6 +1,10 @@
 .env
 architecture.png
-charts
+charts/**/.helmignore
+charts/**/Chart.yaml
+charts/**/README.md
+charts/**/templates/
+charts/**/values.yaml
 coverage
 Dockerfile
 node_modules

diff --git a/.github/workflows/workflow.yml b/.github/workflows/workflow.yml
@@ -17,11 +17,23 @@ jobs:
   test-e2e:
     runs-on: ubuntu-latest
     name: E2E
+    strategy:
+      matrix:
+        disableCustomResourceManager: ['true', 'false']
+        helmVersion: ['V2', 'V3']
     steps:
       - uses: actions/checkout@v2
       - name: Setup node
         uses: actions/setup-node@v1
         with:
           node-version: 12
+      - uses: azure/setup-helm@v1
+        with:
+          version: v2.16.1
+        if: matrix.helmVersion == 'V2'
+      - name: configure helm v2
+        run: |
+          helm init --client-only
+        if: matrix.helmVersion == 'V2'
       - run: npm install
-      - run: npm run test-e2e
+      - run: npm run test-e2e -- ${{ matrix.disableCustomResourceManager }} ${{ matrix.helmVersion }}
diff --git a/bin/daemon.js b/bin/daemon.js
@@ -31,9 +31,7 @@ async function main () {
   logger.info('loading kube specs')
   await kubeClient.loadSpec()
   logger.info('successfully loaded kube specs')
-  logger.info('updating CRD')
-  await customResourceManager.upsertResource({ customResourceManifest })
-  logger.info('successfully updated CRD')
+  await customResourceManager.manageCrd({ customResourceManifest })
 
   const externalSecretEvents = getExternalSecretEvents({
     kubeClient,

diff --git a/charts/kubernetes-external-secrets/README.md b/charts/kubernetes-external-secrets/README.md
@@ -4,11 +4,15 @@
 
 ## TL;DR;
 
+Assumes you are using Helm V3:
+
 ```bash
 $ helm repo add external-secrets https://godaddy.github.io/kubernetes-external-secrets/
-$ helm install external-secrets/kubernetes-external-secrets
+$ helm install kubernetes-external-secrets external-secrets/kubernetes-external-secrets --skip-crds
 ```
 
+See below for [Helm V2 considerations](#helm-v2-considerations) when installing the chart.
+
 ## Prerequisites
 
 * Kubernetes 1.12+
@@ -18,15 +22,39 @@ $ helm install external-secrets/kubernetes-external-secrets
 To install the chart with the release named `my-release`:
 
 ```bash
-$ helm install --name my-release external-secrets/kubernetes-external-secrets
+$ helm install my-release external-secrets/kubernetes-external-secrets --skip-crds
 ```
 
-> **Tip:** A namespace can be specified by the `Helm` option '`--namespace kube-external-secrets`'
+> **Tip:** A namespace can be specified by the `Helm` option '`--namespace kube-external-secrets`', however know this will not [autocreate a namespace](https://helm.sh/docs/faq/#automatically-creating-namespaces) like in Helm V2. To do that, also add the `--create-namespace` flag.
+
+> **Note**: `--skip-crds` is required in order to ensure the custom resource manager is used and will work for backwards compatibility. In future 4.x releases, this will not be required. See below for how to [disable the custom resource manager](#installing-the-crd) via the chart.
 
 To install the chart with [AWS IAM Roles for Service Accounts](https://docs.aws.amazon.com/eks/latest/userguide/iam-roles-for-service-accounts.html):
 
 ```bash
-$ helm install --name my-release --set securityContext.fsGroup=65534 --set serviceAccount.annotations."eks\.amazonaws\.com/role-arn"='arn:aws:iam::111111111111:role/ROLENAME' external-secrets/kubernetes-external-secrets
+$ helm install my-release external-secrets/kubernetes-external-secrets --skip-crds --set securityContext.fsGroup=65534 --set serviceAccount.annotations."eks\.amazonaws\.com/role-arn"='arn:aws:iam::111111111111:role/ROLENAME'
+```
+
+### Installing the CRD
+
+To install the `ExternalSecret` CRD via the chart and disable the custom resource manager, you can omit `--skip-crds` and set `customResourceManagerDisabled`:
+
+```bash
+$ helm install external-secrets/kubernetes-external-secrets --name my-release --set customResourceManagerDisabled=true
+```
+
+### Helm V2 Considerations
+
+For Helm V2, `--skip-crds` is not needed, but `--name` is in order to set the release name:
+
+```bash
+$ helm install external-secrets/kubernetes-external-secrets --name my-release
+```
+
+If you wish to disable the custom resource manager and install the CRD via Helm V2, then `crds.create` must also be set:
+
+```bash
+$ helm install external-secrets/kubernetes-external-secrets --name my-release --set customResourceManagerDisabled=true --set crds.create=true
 ```
 
 ## Uninstalling the Chart
@@ -43,6 +71,8 @@ The following table lists the configurable parameters of the `kubernetes-externa
 
 | Parameter                                 | Description                                                                                                                       | Default                               |
 | ----------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------- |
+| `crds.create`                             | For Helm V2 installations of the chart to install the CRD, for V3 installations use `--skip-crds` appropriately                   | `false`                               |
+| `customResourceManagedDisabled`           | Disables the custom resource manager, requiring the CRD be installed via the chart or other means                                 | `false`                               |
 | `env.AWS_REGION`                          | Set AWS_REGION in Deployment Pod                                                                                                  | `us-west-2`                           |
 | `env.LOG_LEVEL`                           | Set the application log level                                                                                                     | `info`                                |
 | `env.METRICS_PORT`                        | Specify the port for the prometheus metrics server                                                                                | `3001`                                |
@@ -55,9 +85,9 @@ The following table lists the configurable parameters of the `kubernetes-externa
 | `envVarsFromSecret.AZURE_TENANT_ID`       | Set AZURE_TENANT_ID (from a secret) in Deployment Pod                                                                             |                                       |
 | `envVarsFromSecret.AZURE_CLIENT_ID`       | Set AZURE_CLIENT_ID (from a secret) in Deployment Pod                                                                             |                                       |
 | `envVarsFromSecret.AZURE_CLIENT_SECRET`   | Set AZURE_CLIENT_SECRET (from a secret) in Deployment Pod                                                                         |                                       |
-| `envVarsFromSecret.ALICLOUD_ENDPOINT`     | Set ALICLOUD_ENDPOINT for KMS Service in Deployment Pod      |                                                         |
-| `envVarsFromSecret.ALICLOUD_ACCESS_KEY_ID`     | Set ALICLOUD_ACCESS_KEY_ID (from a secret) in Deployment Pod |                                                    |
-| `envVarsFromSecret.ALICLOUD_ACCESS_KEY_SECRET` | Set ALICLOUD_ACCESS_KEY_SECRET (from a secret) in Deployment Pod  |                                               |
+| `envVarsFromSecret.ALICLOUD_ENDPOINT`     | Set ALICLOUD_ENDPOINT for KMS Service in Deployment Pod                                                                           |                                       |
+| `envVarsFromSecret.ALICLOUD_ACCESS_KEY_ID`     | Set ALICLOUD_ACCESS_KEY_ID (from a secret) in Deployment Pod                                                                 |                                       |
+| `envVarsFromSecret.ALICLOUD_ACCESS_KEY_SECRET` | Set ALICLOUD_ACCESS_KEY_SECRET (from a secret) in Deployment Pod                                                             |                                       |
 | `image.repository`                        | kubernetes-external-secrets Image name                                                                                            | `godaddy/kubernetes-external-secrets` |
 | `image.tag`                               | kubernetes-external-secrets Image tag                                                                                             | `3.2.0`                               |
 | `image.pullPolicy`                        | Image pull policy                                                                                                                 | `IfNotPresent`                        |
@@ -83,15 +113,16 @@ The following table lists the configurable parameters of the `kubernetes-externa
 Specify each parameter using the `--set key=value[,key=value]` argument to `helm install`. For example,
 
 ```bash
-helm install external-secrets/kubernetes-external-secrets --name my-releases \
+helm install my-release external-secrets/kubernetes-external-secrets \
+--set customResourceManagerDisabled=true
 --set env.POLLER_INTERVAL_MILLISECONDS='300000' \
 --set podAnnotations."iam\.amazonaws\.com/role"='Name-Of-IAM-Role-With-SecretManager-Access'
 ```
 
 Alternatively, a YAML file that specifies the values for the parameters can be provided while installing the chart. For example,
 
 ```bash
-helm install external-secrets/kubernetes-external-secrets --name my-release -f values.yaml
+helm install my-release external-secrets/kubernetes-external-secrets -f values.yaml
 ```
 
 > **Tip**: You can use the default [values.yaml](https://github.com/godaddy/kubernetes-external-secrets/blob/master/charts/kubernetes-external-secrets/values.yaml)

diff --git a/crd.yaml → ...rnetes-client.io_externalsecrets_crd.yaml b/crd.yaml → ...rnetes-client.io_externalsecrets_crd.yaml
@@ -1,29 +1,35 @@
+---
 apiVersion: apiextensions.k8s.io/v1beta1
 kind: CustomResourceDefinition
 metadata:
   name: externalsecrets.kubernetes-client.io
+  annotations:
+    # for helm v2 backwards compatibility
+    helm.sh/hook: crd-install
+    # used in e2e testing
+    app.kubernetes.io/managed-by: helm
 spec:
   group: kubernetes-client.io
   version: v1
   scope: Namespaced
 
   names:
     shortNames:
-    - es
+      - es
     kind: ExternalSecret
     plural: externalsecrets
     singular: externalsecret
 
   additionalPrinterColumns:
-  - JSONPath: .status.lastSync
-    name: Last Sync
-    type: date
-  - JSONPath: .status.status
-    name: status
-    type: string
-  - JSONPath: .metadata.creationTimestamp
-    name: Age
-    type: date
+    - JSONPath: .status.lastSync
+      name: Last Sync
+      type: date
+    - JSONPath: .status.status
+      name: status
+      type: string
+    - JSONPath: .metadata.creationTimestamp
+      name: Age
+      type: date
 
   validation:
     openAPIV3Schema:
@@ -110,9 +116,8 @@ spec:
                     - alicloudSecretsManager
           anyOf:
             - required:
-              - data
+                - data
             - required:
-              - dataFrom
-
+                - dataFrom
   subresources:
     status: {}
diff --git a/charts/kubernetes-external-secrets/templates/crds.yaml b/charts/kubernetes-external-secrets/templates/crds.yaml
@@ -0,0 +1,8 @@
+{{- if .Values.customResourceManagerDisabled }}
+{{- if .Values.crds.create }}
+{{- range $path, $bytes := .Files.Glob "crds/*.yaml" }}
+{{ $.Files.Get $path }}
+---
+{{- end }}
+{{- end }}
+{{- end }}
diff --git a/charts/kubernetes-external-secrets/templates/deployment.yaml b/charts/kubernetes-external-secrets/templates/deployment.yaml
@@ -41,6 +41,10 @@ spec:
           resources:
             {{- toYaml .Values.resources | nindent 12 }}
           env:
+          {{- if .Values.customResourceManagerDisabled }}
+          - name: DISABLE_CUSTOM_RESOURCE_MANAGER
+            value: "true"
+          {{- end }}
           {{- range $name, $value := .Values.env }}
           {{- if not (empty $value) }}
           - name: {{ $name | quote }}

diff --git a/charts/kubernetes-external-secrets/templates/rbac.yaml b/charts/kubernetes-external-secrets/templates/rbac.yaml
@@ -15,9 +15,6 @@ rules:
   - apiGroups: [""]
     resources: ["namespaces"]
     verbs: ["get", "watch", "list"]
-  - apiGroups: ["apiextensions.k8s.io"]
-    resources: ["customresourcedefinitions"]
-    verbs: ["create"]
   - apiGroups: ["apiextensions.k8s.io"]
     resources: ["customresourcedefinitions"]
     resourceNames: ["externalsecrets.kubernetes-client.io"]
@@ -28,6 +25,11 @@ rules:
   - apiGroups: ["kubernetes-client.io"]
     resources: ["externalsecrets/status"]
     verbs: ["get", "update"]
+ {{- if .Values.customResourceManagerDisabled | not }}
+  - apiGroups: ["apiextensions.k8s.io"]
+    resources: ["customresourcedefinitions"]
+    verbs: ["create"]
+  {{- end }}
 ---
 apiVersion: rbac.authorization.k8s.io/v1beta1
 kind: ClusterRoleBinding

diff --git a/charts/kubernetes-external-secrets/values.yaml b/charts/kubernetes-external-secrets/values.yaml
@@ -2,9 +2,18 @@
 # This is a YAML-formatted file.
 # Declare variables to be passed into your templates.
 
+# Determines whether the Helm chart or kubernetes-external-secrets
+# will handle the ExternalSecret CRD
+customResourceManagerDisabled: false
+
+crds:
+  # only needed for helm v2, leave this disabled for helm v3
+  create: false
+
 # Environment variables to set on deployment pod
 env:
   AWS_REGION: us-west-2
+  AWS_DEFAULT_REGION: us-west-2
   POLLER_INTERVAL_MILLISECONDS: 10000  # Caution, setting this frequency may incur additional charges on some platforms
   LOG_LEVEL: info
   METRICS_PORT: 3001

diff --git a/config/environment.js b/config/environment.js
@@ -28,6 +28,8 @@ const namingPermittedAnnotation = process.env.NAMING_PERMITTED_ANNOTATION || 'ex
 
 const metricsPort = process.env.METRICS_PORT || 3001
 
+const customResourceManagerDisabled = 'DISABLE_CUSTOM_RESOURCE_MANAGER' in process.env
+
 module.exports = {
   vaultEndpoint,
   environment,
@@ -36,5 +38,6 @@ module.exports = {
   rolePermittedAnnotation,
   namingPermittedAnnotation,
   pollingDisabled,
-  logLevel
+  logLevel,
+  customResourceManagerDisabled
 }
diff --git a/config/index.js b/config/index.js
@@ -23,7 +23,8 @@ const AliCloudSecretsManagerBackend = require('../lib/backends/alicloud-secrets-
 
 // Get document, or throw exception on error
 // eslint-disable-next-line security/detect-non-literal-fs-filename
-const customResourceManifest = yaml.safeLoad(fs.readFileSync(path.resolve(__dirname, '../crd.yaml'), 'utf8'))
+const customResourceManifest = yaml.safeLoad(fs.readFileSync(path.resolve(__dirname, '../charts/kubernetes-external-secrets/crds/kubernetes-client.io_externalsecrets_crd.yaml'), 'utf8'))
+customResourceManifest.metadata.annotations['app.kubernetes.io/managed-by'] = 'custom-resource-manager'
 
 const kubeconfig = new kube.KubeConfig()
 kubeconfig.loadFromDefault()
@@ -39,7 +40,8 @@ const logger = pino({
 
 const customResourceManager = new CustomResourceManager({
   kubeClient,
-  logger
+  logger,
+  disabled: envConfig.customResourceManagerDisabled
 })
 
 const secretsManagerBackend = new SecretsManagerBackend({

diff --git a/e2e/run-e2e-suite.sh b/e2e/run-e2e-suite.sh
@@ -13,6 +13,10 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+
+DISABLE_CUSTOM_RESOURCE_MANAGER=${1:-true}
+HELM_VERSION=${2:-V3}
+
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 KIND_LOGGING="--quiet"
 if ! [ -z "$DEBUG" ]; then
@@ -66,7 +70,27 @@ trap cleanup EXIT
 
 kubectl apply -f ${DIR}/localstack.deployment.yaml
 
-helm template e2e $DIR/../charts/kubernetes-external-secrets \
+CHART_DIR="$(dirname "$DIR")/charts/kubernetes-external-secrets"
+HELM_TEMPLATE_ARGS="e2e ${CHART_DIR}"
+HELM_TEMPLATE_EXTRA_ARGS="--include-crds --set customResourceManagerDisabled=true"
+E2E_EXTRA_ARGS="--env=DISABLE_CUSTOM_RESOURCE_MANAGER=true"
+if [[ "$HELM_VERSION" == "V3" ]]; then
+  if [[ "$DISABLE_CUSTOM_RESOURCE_MANAGER" == "false" ]]; then
+    HELM_TEMPLATE_EXTRA_ARGS="--skip-crds"
+    E2E_EXTRA_ARGS=""
+  fi
+else
+  HELM_TEMPLATE_ARGS="${CHART_DIR} --name e2e"
+  if [[ "$DISABLE_CUSTOM_RESOURCE_MANAGER" == "true" ]]; then
+    HELM_TEMPLATE_EXTRA_ARGS="--set crds.create=true --set customResourceManagerDisabled=true"
+  else
+    HELM_TEMPLATE_EXTRA_ARGS=""
+    E2E_EXTRA_ARGS=""
+  fi
+fi
+
+helm template ${HELM_TEMPLATE_ARGS} \
+  ${HELM_TEMPLATE_EXTRA_ARGS} \
   --set image.repository=external-secrets \
   --set image.tag=test \
   --set env.LOG_LEVEL=debug \
@@ -94,7 +118,6 @@ until kubectl get secret | grep -q ^external-secrets-e2e-token; do \
 done
 
 echo -e "${BGREEN}Starting external-secrets e2e tests...${NC}"
-
 kubectl rollout status deploy/localstack
 kubectl rollout status deploy/e2e-kubernetes-external-secrets
 
@@ -109,6 +132,7 @@ kubectl run \
   --env="AWS_DEFAULT_REGION=us-east-1" \
   --env="AWS_REGION=us-east-1" \
   --env="LOCALSTACK_STS_URL=http://sts" \
+  ${E2E_EXTRA_ARGS} \
   --generator=run-pod/v1 \
   --overrides='{ "apiVersion": "v1", "spec":{"serviceAccountName": "external-secrets-e2e"}}' \
   e2e --image=external-secrets-e2e:test
diff --git a/e2e/tests/crd.test.js b/e2e/tests/crd.test.js
@@ -5,22 +5,30 @@ const { expect } = require('chai')
 
 const {
   kubeClient,
-  customResourceManifest
+  customResourceManifest,
+  customResourceManagerDisabled
 } = require('../../config')
 
 const {
   uuid
 } = require('./framework.js')
 
 describe('CRD', () => {
-  it('should register the CRD on startup', async () => {
+  it('ensure CRD is managed correctly', async () => {
     const res = await kubeClient
       .apis['apiextensions.k8s.io']
       .v1beta1
       .customresourcedefinitions(customResourceManifest.metadata.name)
       .get()
+
+    let managedBy = 'custom-resource-manager'
+    if (customResourceManagerDisabled) {
+      managedBy = 'helm'
+    }
+
     expect(res).to.not.equal(undefined)
     expect(res.statusCode).to.equal(200)
+    expect(res.body.metadata.annotations['app.kubernetes.io/managed-by']).to.equal(managedBy)
   })
 
   it('should reject invalid ExternalSecret manifests', async () => {