DoD-Platform-One / bigbang

BigBang the product
https://repo1.dso.mil/big-bang/bigbang
Apache License 2.0
153 stars 67 forks source link

Trying to set istio-injection to disabled doesn't work as expected #14

Closed p1-repo-sync-bot[bot] closed 5 months ago

p1-repo-sync-bot[bot] commented 7 months ago

The initial discovery of this problem resulted from trying to disable istio injection in gitlab-runner, but when digging in we found multiple discrepancies. When trying to set istio injection to disabled, we were confused by the fact that the default was already disabled as visible in the values, even though the running cluster had istio injection enabled:

istio:
  # Toggle istio integration
  enabled: false
  injection: disabled

Even after updating the values in our overrides, this didn't appear to change anything for istio injection in our cluster.

The actual helm code sets it to enabled by default. When trying to override with our own custom values, we similarly noticed that it would stay enabled. Istio settings are confusing because the templates are looking for them within the top level .Values.addons., and the defaults all get set to true because the fallback values are enabled. This adds to the confusion, because all of the settings per app in their respective values.yaml show istio is disabled and injection is disabled. The way these are templated is also inconsistent where some just use dig and others use a more comprehensive ternary.

Both of the following get set to enabled because the default values aren't set in the bigbang values.yaml

Anchore namespace.yaml in Big Bang code:
{{ ternary "enabled" "disabled" (and .Values.istio.enabled (eq (dig "istio" "injection" "enabled" .Values.addons.anchore) "enabled")) }}

and

Gitlab-runner namespace.yaml in Big Bang code:
{{ dig "istio" "injection" "enabled" .Values.addons.gitlabRunner }}

Both fallback to enabled because .Values.addons.\<app>.istio isn't defined for any of the applications. The ternary also evaluates to true (enabled) because the dig falls back to "enabled" since the istio key doesn't exist under .Values.addons.anchore so we get "enabled" == "enabled" making eq true, and istio is enabled by default so the and evaluates to true.

Since the expected templating with the current defaults and values locations seems misleading and can be confusing to setup:

p1-repo-sync-bot[bot] commented 6 months ago

bb8-bot commented:

@kenneth.maguire this issue has been inactive for 60 days and is being labelled as ~marked-for-auto-close. If this issue is still required please take action by removing the ~stale and ~marked-for-auto-close labels and commenting with an update, status, or justification. If this issue is not required please close it or label it as ~delete-me. If no action is taken this issue will be auto closed in 30 days.

p1-repo-sync-bot[bot] commented 5 months ago

Issue 'Trying to set istio-injection to disabled doesn't work as expected' closed from GitLab side

p1-repo-sync-bot[bot] commented 5 months ago

bb8-bot commented:

@kenneth.maguire this issue has been closed due to the label ~delete-me.