From 3b838758a34c1c56d20eaaa69246d68484585a2d Mon Sep 17 00:00:00 2001 From: Nicolas Fischer Date: Wed, 11 Dec 2019 15:31:58 +0000 Subject: [PATCH 1/4] support an arbitrary SecurityContext block for the main deployment Signed-off-by: Nicolas Fischer --- deploy/charts/cert-manager/README.md | 5 +- .../cert-manager/templates/deployment.yaml | 14 +- deploy/charts/cert-manager/values.yaml | 16 +- hack/test-securitycontext-deprecation.sh | 240 ++++++++++++++++++ test/fixtures/security-context-deprecation.md | 178 +++++++++++++ 5 files changed, 443 insertions(+), 10 deletions(-) create mode 100755 hack/test-securitycontext-deprecation.sh create mode 100644 test/fixtures/security-context-deprecation.md diff --git a/deploy/charts/cert-manager/README.md b/deploy/charts/cert-manager/README.md index a2b13ac14..526735ab9 100644 --- a/deploy/charts/cert-manager/README.md +++ b/deploy/charts/cert-manager/README.md @@ -90,9 +90,8 @@ The following table lists the configurable parameters of the cert-manager chart | `serviceAccount.name` | Service account to be used. If not set and `serviceAccount.create` is `true`, a name is generated using the fullname template | | | `serviceAccount.annotations` | Annotations to add to the service account | | | `resources` | CPU/memory resource requests/limits | `{}` | -| `securityContext.enabled` | Enable security context | `false` | -| `securityContext.fsGroup` | Group ID for the container | `1001` | -| `securityContext.runAsUser` | User ID for the container | `1001` | +| `securityContext` | Optional security context. The yaml block should adhere to the [SecurityContext spec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.16/#securitycontext-v1-core) | `{}` | +| `securityContext.enabled` | Deprecated (use `securityContext`) - Enable security context | `false` | | `nodeSelector` | Node labels for pod assignment | `{}` | | `affinity` | Node affinity for pod assignment | `{}` | | `tolerations` | Node tolerations for pod assignment | `[]` | diff --git a/deploy/charts/cert-manager/templates/deployment.yaml b/deploy/charts/cert-manager/templates/deployment.yaml index bedc2587a..80e5f61f6 100644 --- a/deploy/charts/cert-manager/templates/deployment.yaml +++ b/deploy/charts/cert-manager/templates/deployment.yaml @@ -53,10 +53,18 @@ spec: {{- if .Values.global.priorityClassName }} priorityClassName: {{ .Values.global.priorityClassName | quote }} {{- end }} - {{- if .Values.securityContext.enabled }} + {{- $enabledDefined := gt (len (keys (pick .Values.securityContext "enabled"))) 0 }} + {{- $legacyEnabledExplicitlydOff := and $enabledDefined (not .Values.securityContext.enabled) }} + {{- if and .Values.securityContext (not $legacyEnabledExplicitlydOff) }} securityContext: - fsGroup: {{ .Values.securityContext.fsGroup }} - runAsUser: {{ .Values.securityContext.runAsUser }} + {{- if .Values.securityContext.enabled -}} + {{/* support legacy securityContext.enabled and its two parameters */}} + fsGroup: {{ default 1001 .Values.securityContext.fsGroup }} + runAsUser: {{ default 1001 .Values.securityContext.runAsUser }} + {{- else -}} + {{/* this is the way forward: support an arbitrary yaml block */}} +{{ toYaml .Values.securityContext | indent 8 }} + {{- end }} {{- end }} containers: - name: {{ .Chart.Name }} diff --git a/deploy/charts/cert-manager/values.yaml b/deploy/charts/cert-manager/values.yaml index b153f031b..870d59b99 100644 --- a/deploy/charts/cert-manager/values.yaml +++ b/deploy/charts/cert-manager/values.yaml @@ -73,10 +73,18 @@ resources: {} # Pod Security Context # ref: https://kubernetes.io/docs/tasks/configure-pod-container/security-context/ -securityContext: - enabled: false - fsGroup: 1001 - runAsUser: 1001 +securityContext: {} +# legacy securityContext parameter format: if enabled is set to true, only fsGroup and runAsUser are supported +#securityContext: +# enabled: false +# fsGroup: 1001 +# runAsUser: 1001 +# to support additional securityContext parameters, omit the `enabled` parameter and simply specify the parameters +# you want to set, e.g. +#securityContext: +# fsGroup: 1000 +# runAsUser: 1000 +# runAsNonRoot: true deploymentAnnotations: {} diff --git a/hack/test-securitycontext-deprecation.sh b/hack/test-securitycontext-deprecation.sh new file mode 100755 index 000000000..5e3461de4 --- /dev/null +++ b/hack/test-securitycontext-deprecation.sh @@ -0,0 +1,240 @@ +#!/usr/bin/env bash +# Copyright 2019 The Jetstack cert-manager contributors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# 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. + +set -o nounset +set -o errexit +set -o pipefail + +SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" +TMPFILES=$TEST_TMPDIR/files + +info() { + echo "info: $1" +} + +error() { + echo "error: $1" +} + +check_pattern_present() { + message=$1 + file=$2 + pattern=$3 + set +o errexit + grep "$pattern" "$file" >& /dev/null + status=$? + set -o errexit + if [[ $status -ne 0 ]]; then + info "generated output: ${file}" + cat "$file" + error "${message} - expected pattern ${pattern} is absent" + exit 1 + fi +} + +check_pattern_absent() { + message=$1 + file=$2 + pattern=$3 + set +o errexit + grep "$pattern" "$file" >& /dev/null + status=$? + set -o errexit + if [[ $status -eq 0 ]]; then + info "generated output: ${file}" + cat "$file" + error "${message} - unexpected pattern ${pattern} is present" + exit 1 + fi +} + +# +# generate_template +# +generate_template() { + values=$1 + generated="$TMPFILES/generated.yaml" + helm template --dry-run --values $values --name-template=jetstack --namespace=cert-manager ${SCRIPT_DIR}/../deploy/charts/cert-manager > $generated + echo $generated +} + +# +# test_use_case_1 +# +test_use_case_1() { + values="$TMPFILES/values.yaml" + cat < $values +--- +EOF + generated="$(generate_template $values)" + check_pattern_absent "use case 1" $generated " securityContext:" + check_pattern_absent "use case 1" $generated " enabled:" + check_pattern_absent "use case 1" $generated " fsGroup:" + check_pattern_absent "use case 1" $generated " runAsUser:" +} + +# +# test_use_case_2 +# +test_use_case_2() { + values="$TMPFILES/values.yaml" + cat < $values +securityContext: + enabled: true +EOF + generated="$(generate_template $values)" + check_pattern_present "use case 2" $generated " securityContext:" + check_pattern_present "use case 2" $generated " fsGroup: 1001" + check_pattern_present "use case 2" $generated " runAsUser: 1001" + check_pattern_absent "use case 2" $generated " enabled:" +} + +# +# test_use_case_3 +# +test_use_case_3() { + values="$TMPFILES/values.yaml" + cat < $values +securityContext: + enabled: true + fsGroup: 1111 + runAsUser: 2222 +EOF + generated="$(generate_template $values)" + check_pattern_present "use case 3" $generated " securityContext:" + check_pattern_present "use case 3" $generated " fsGroup: 1111" + check_pattern_present "use case 3" $generated " runAsUser: 2222" + check_pattern_absent "use case 3" $generated " enabled:" +} + +# +# test_use_case_4 +# +test_use_case_4() { + values="$TMPFILES/values.yaml" + cat < $values +securityContext: {} +EOF + generated="$(generate_template $values)" + check_pattern_absent "use case 4" $generated " securityContext:" + check_pattern_absent "use case 4" $generated " fsGroup:" + check_pattern_absent "use case 4" $generated " runAsUser:" + check_pattern_absent "use case 4" $generated " enabled:" +} + +# +# test_use_case_5 +# +test_use_case_5() { + values="$TMPFILES/values.yaml" + cat < $values +securityContext: + fsGroup: 1111 + runAsUser: 2222 + runAsNonRoot: true +EOF + generated="$(generate_template $values)" + check_pattern_present "use case 5" $generated " securityContext:" + check_pattern_present "use case 5" $generated " fsGroup: 1111" + check_pattern_present "use case 5" $generated " runAsUser: 2222" + check_pattern_present "use case 5" $generated " runAsNonRoot: true" + check_pattern_absent "use case 5" $generated " enabled:" +} + +# +# test_use_case_6 +# +test_use_case_6() { + values="$TMPFILES/values.yaml" + cat < $values +securityContext: + enabled: false + fsGroup: 1111 + runAsUser: 2222 +EOF + generated="$(generate_template $values)" + check_pattern_absent "use case 6" $generated " securityContext:" + check_pattern_absent "use case 6" $generated " enabled:" + check_pattern_absent "use case 6" $generated " fsGroup:" + check_pattern_absent "use case 6" $generated " runAsUser:" +} + +# +# test_use_case_7 +# +test_use_case_7() { + values="$TMPFILES/values.yaml" + cat < $values +securityContext: + enabled: false +EOF + generated="$(generate_template $values)" + check_pattern_absent "use case 7" $generated " securityContext:" + check_pattern_absent "use case 7" $generated " enabled:" + check_pattern_absent "use case 7" $generated " fsGroup:" + check_pattern_absent "use case 7" $generated " runAsUser:" +} + +# +# test_use_case_8 +# +test_use_case_8() { + values="$TMPFILES/values.yaml" + cat < $values +securityContext: + fsGroup: 1111 + runAsUser: 2222 +EOF + generated="$(generate_template $values)" + check_pattern_present "use case 8" $generated " securityContext:" + check_pattern_absent "use case 8" $generated " enabled:" + check_pattern_present "use case 8" $generated " fsGroup: 1111" + check_pattern_present "use case 8" $generated " runAsUser: 2222" +} + +# +# unit_test +# +unit_test() { + values="$TMPFILES/values.yaml" + cat < $values +--- +EOF + generated="$(generate_template $values)" + echo "following should fail" + check_pattern_present "unit test" $generated "foo" + echo "following should succeed" + check_pattern_absent "unit test" $generated "foo" + echo "following should succeed" + check_pattern_present "unit test" $generated "kind" + echo "following should fail" + check_pattern_absent "unit test" $generated "kind" +} + +info "testing securityContext.enabled deprecation in chart parameters" + +mkdir -p "$TMPFILES" + +#unit_test +test_use_case_1 +test_use_case_2 +test_use_case_3 +test_use_case_4 +test_use_case_5 +test_use_case_6 +test_use_case_7 +test_use_case_8 + +info "Tests successful" diff --git a/test/fixtures/security-context-deprecation.md b/test/fixtures/security-context-deprecation.md new file mode 100644 index 000000000..a02adfb3c --- /dev/null +++ b/test/fixtures/security-context-deprecation.md @@ -0,0 +1,178 @@ +# Logic for deprecating securityContext.enabled + +The idea is to be able to define a `securityContext` from an arbitrary yaml block without being restricted to just the attributes currently supported. + +For example, by supporting a block like the following in the `values.yaml` file: + +```yaml +securityContext: + fsGroup: 1111 + runAsUser: 2222 + runAsNonRoot: true +``` + +The logic for supporting the new `securityContext` in the `values.yaml` file should be backwards compatible with the `securityContext.enabled` parameter. + +## Pseudo-code logic + +No security context block should be defined if: + +``` +securityContext is empty +or securityContext.enabled is defined and securityContext.enabled is false +``` + +Otherwise a securityContext block should be defined + +When a securityContext block has to be generated, we should fall back and support the deprecated structure if: + +``` +securityContext.enabled is true +``` + +Otherwise, we copy the `securityContext` block as it is to support the new format. + +## Test cases + +### Test case 1 + +Defaults + +Input: + +No security context specified + +Output: + +Nothing generated + +### Test case 2 + +Legacy parameters with `enabled` only and no additional parameters + +Input: + +```yaml +securityContext: + enabled: true +``` + +Output: + +```yaml +securityContext: + fsGroup: 1001 + runAsUser: 1001 +``` + +### Test case 3 + +Legacy parameters with `enabled`, group and user + +Input: + +```yaml +securityContext: + enabled: true + fsGroup: 1111 + runAsUser: 2222 +``` + +Output: + +```yaml +securityContext: + fsGroup: 1111 + runAsUser: 2222 +``` + +### Test case 4: + +New default format + +Input: + +```yaml +securityContext: {} +``` + +Output: + +```yaml +``` + +### Test case 5: + +New format with arbitrary block + +Input: + +```yaml +securityContext: + fsGroup: 1111 + runAsUser: 2222 + runAsNonRoot: true +``` + +Output: + +```yaml +securityContext: + fsGroup: 1111 + runAsNonRoot: true + runAsUser: 2222 +``` + +### Test case 6: + +Legacy parameters with `enabled` set to false and extra parameters that should be ignored + +Input: + +```yaml +securityContext: + enabled: false + fsGroup: 1111 + runAsUser: 2222 +``` + +Output: + +```yaml +``` + +### Test case 7: + +Legacy parameters with `enabled` set to false + +Input: + +```yaml +securityContext: + enabled: false +``` + +Output: + +```yaml +``` + +### Test case 8: + +Block with only fsGroup and runAsUser + +Input: + +```yaml +securityContext: + fsGroup: 1111 + runAsUser: 2222 +``` + +Output: + +```yaml +securityContext: + fsGroup: 1111 + runAsUser: 2222 +``` From aefa3c9660bf06782d19965a5bfbb9d493886be9 Mon Sep 17 00:00:00 2001 From: Nicolas Fischer Date: Wed, 18 Dec 2019 10:15:12 +0000 Subject: [PATCH 2/4] add securityContext.enabled deprecation doc to the design folder Signed-off-by: Nicolas Fischer --- .../20191218-security-context-deprecation.md | 178 ++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 design/20191218-security-context-deprecation.md diff --git a/design/20191218-security-context-deprecation.md b/design/20191218-security-context-deprecation.md new file mode 100644 index 000000000..a02adfb3c --- /dev/null +++ b/design/20191218-security-context-deprecation.md @@ -0,0 +1,178 @@ +# Logic for deprecating securityContext.enabled + +The idea is to be able to define a `securityContext` from an arbitrary yaml block without being restricted to just the attributes currently supported. + +For example, by supporting a block like the following in the `values.yaml` file: + +```yaml +securityContext: + fsGroup: 1111 + runAsUser: 2222 + runAsNonRoot: true +``` + +The logic for supporting the new `securityContext` in the `values.yaml` file should be backwards compatible with the `securityContext.enabled` parameter. + +## Pseudo-code logic + +No security context block should be defined if: + +``` +securityContext is empty +or securityContext.enabled is defined and securityContext.enabled is false +``` + +Otherwise a securityContext block should be defined + +When a securityContext block has to be generated, we should fall back and support the deprecated structure if: + +``` +securityContext.enabled is true +``` + +Otherwise, we copy the `securityContext` block as it is to support the new format. + +## Test cases + +### Test case 1 + +Defaults + +Input: + +No security context specified + +Output: + +Nothing generated + +### Test case 2 + +Legacy parameters with `enabled` only and no additional parameters + +Input: + +```yaml +securityContext: + enabled: true +``` + +Output: + +```yaml +securityContext: + fsGroup: 1001 + runAsUser: 1001 +``` + +### Test case 3 + +Legacy parameters with `enabled`, group and user + +Input: + +```yaml +securityContext: + enabled: true + fsGroup: 1111 + runAsUser: 2222 +``` + +Output: + +```yaml +securityContext: + fsGroup: 1111 + runAsUser: 2222 +``` + +### Test case 4: + +New default format + +Input: + +```yaml +securityContext: {} +``` + +Output: + +```yaml +``` + +### Test case 5: + +New format with arbitrary block + +Input: + +```yaml +securityContext: + fsGroup: 1111 + runAsUser: 2222 + runAsNonRoot: true +``` + +Output: + +```yaml +securityContext: + fsGroup: 1111 + runAsNonRoot: true + runAsUser: 2222 +``` + +### Test case 6: + +Legacy parameters with `enabled` set to false and extra parameters that should be ignored + +Input: + +```yaml +securityContext: + enabled: false + fsGroup: 1111 + runAsUser: 2222 +``` + +Output: + +```yaml +``` + +### Test case 7: + +Legacy parameters with `enabled` set to false + +Input: + +```yaml +securityContext: + enabled: false +``` + +Output: + +```yaml +``` + +### Test case 8: + +Block with only fsGroup and runAsUser + +Input: + +```yaml +securityContext: + fsGroup: 1111 + runAsUser: 2222 +``` + +Output: + +```yaml +securityContext: + fsGroup: 1111 + runAsUser: 2222 +``` From 1a066a071714746d4a21a86e483c8b4f6be37cc5 Mon Sep 17 00:00:00 2001 From: Nicolas Fischer Date: Tue, 7 Jan 2020 13:02:48 +0000 Subject: [PATCH 3/4] removed redundant security-context-deprecation.md file from test/fixtures Signed-off-by: Nicolas Fischer --- test/fixtures/security-context-deprecation.md | 178 ------------------ 1 file changed, 178 deletions(-) delete mode 100644 test/fixtures/security-context-deprecation.md diff --git a/test/fixtures/security-context-deprecation.md b/test/fixtures/security-context-deprecation.md deleted file mode 100644 index a02adfb3c..000000000 --- a/test/fixtures/security-context-deprecation.md +++ /dev/null @@ -1,178 +0,0 @@ -# Logic for deprecating securityContext.enabled - -The idea is to be able to define a `securityContext` from an arbitrary yaml block without being restricted to just the attributes currently supported. - -For example, by supporting a block like the following in the `values.yaml` file: - -```yaml -securityContext: - fsGroup: 1111 - runAsUser: 2222 - runAsNonRoot: true -``` - -The logic for supporting the new `securityContext` in the `values.yaml` file should be backwards compatible with the `securityContext.enabled` parameter. - -## Pseudo-code logic - -No security context block should be defined if: - -``` -securityContext is empty -or securityContext.enabled is defined and securityContext.enabled is false -``` - -Otherwise a securityContext block should be defined - -When a securityContext block has to be generated, we should fall back and support the deprecated structure if: - -``` -securityContext.enabled is true -``` - -Otherwise, we copy the `securityContext` block as it is to support the new format. - -## Test cases - -### Test case 1 - -Defaults - -Input: - -No security context specified - -Output: - -Nothing generated - -### Test case 2 - -Legacy parameters with `enabled` only and no additional parameters - -Input: - -```yaml -securityContext: - enabled: true -``` - -Output: - -```yaml -securityContext: - fsGroup: 1001 - runAsUser: 1001 -``` - -### Test case 3 - -Legacy parameters with `enabled`, group and user - -Input: - -```yaml -securityContext: - enabled: true - fsGroup: 1111 - runAsUser: 2222 -``` - -Output: - -```yaml -securityContext: - fsGroup: 1111 - runAsUser: 2222 -``` - -### Test case 4: - -New default format - -Input: - -```yaml -securityContext: {} -``` - -Output: - -```yaml -``` - -### Test case 5: - -New format with arbitrary block - -Input: - -```yaml -securityContext: - fsGroup: 1111 - runAsUser: 2222 - runAsNonRoot: true -``` - -Output: - -```yaml -securityContext: - fsGroup: 1111 - runAsNonRoot: true - runAsUser: 2222 -``` - -### Test case 6: - -Legacy parameters with `enabled` set to false and extra parameters that should be ignored - -Input: - -```yaml -securityContext: - enabled: false - fsGroup: 1111 - runAsUser: 2222 -``` - -Output: - -```yaml -``` - -### Test case 7: - -Legacy parameters with `enabled` set to false - -Input: - -```yaml -securityContext: - enabled: false -``` - -Output: - -```yaml -``` - -### Test case 8: - -Block with only fsGroup and runAsUser - -Input: - -```yaml -securityContext: - fsGroup: 1111 - runAsUser: 2222 -``` - -Output: - -```yaml -securityContext: - fsGroup: 1111 - runAsUser: 2222 -``` From 2bb792943a9f14832a722d6936fa30b7f734cc87 Mon Sep 17 00:00:00 2001 From: Nicolas Fischer Date: Fri, 10 Jan 2020 10:09:35 +0000 Subject: [PATCH 4/4] ensure space after comment character Signed-off-by: Nicolas Fischer --- deploy/charts/cert-manager/values.yaml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/deploy/charts/cert-manager/values.yaml b/deploy/charts/cert-manager/values.yaml index 870d59b99..31b40c3ce 100644 --- a/deploy/charts/cert-manager/values.yaml +++ b/deploy/charts/cert-manager/values.yaml @@ -75,16 +75,16 @@ resources: {} # ref: https://kubernetes.io/docs/tasks/configure-pod-container/security-context/ securityContext: {} # legacy securityContext parameter format: if enabled is set to true, only fsGroup and runAsUser are supported -#securityContext: -# enabled: false -# fsGroup: 1001 -# runAsUser: 1001 +# securityContext: +# enabled: false +# fsGroup: 1001 +# runAsUser: 1001 # to support additional securityContext parameters, omit the `enabled` parameter and simply specify the parameters # you want to set, e.g. -#securityContext: -# fsGroup: 1000 -# runAsUser: 1000 -# runAsNonRoot: true +# securityContext: +# fsGroup: 1000 +# runAsUser: 1000 +# runAsNonRoot: true deploymentAnnotations: {}