Supporting externally managed Cortex Config

[khaines]

There is a need to support cortex configurations that are defined & managed outside of the helm chart. This quick PR adds support for cortex to reference a pre-existing secret for its config.

addresses #30

[rsteneteg]

Thanks Ken, by using the external config you would loose the checksum ability to have the workloads automatically update when deploying a config only change. I think I found an alternative way to use secrets in the config, by using environment variables in the config ${MYSECRETPASSWORD} and then in the env value for each workload add the environment variable from another "external" secret.

To enable the env variable expansion in the config we will need to add this flag -config.expand-env to the workloads

[khaines]

Using something like external config/secrets implies a certain level of ownership, like having one's own methods for managing updates. So I feel this is an ok trade for losing the hash tracking of the config template.

while -config.expand-env can be useful, using ENV for sensitive values is a serious disclosure risk. Docker does not store them securely and instead makes them very discoverable.

[rsteneteg]

What about making it so that if the external config is used, enables you to set the checksum annotation from a global value.. that way a parent chart can increment the global value on config changes in order to trigger the annotation to change and get the workloads to roll?

[rsteneteg]

I suppose it does not need to be a global value (was thinking if there was a way to have a global value dynamically set based of the parent chart templates), so somthing like

annotations:
{{- if .Values.useExternalConfig }}
        checksum/config: {{ .Values.externalConfigChecksum }}
{{- else }}
        checksum/config: {{ include (print $.Template.BasePath "/secret.yaml") . | sha256sum }}
{{- end }} 

and in the values file set the default value for externalConfigChecksum to "0"

[containerpope]

@rsteneteg would there be any downside to enable env expansion to all components by default and then adding an optional envFrom Secret option to all components? I understand that managing it completely by file based secret is the more secure option, but giving a way in between would also be very helpful for all GitOps type of workflows.

With this change, the config details could be outside a kubernetes Secret and only the values that require protection would be masked by a set of env variables that are provided in a secret of choice. With this adjustments, someone could prepare the secret and then just use it inside the chart without any further adjustments or providing the values multiple times.

This is for example how Grafana does it:

# values.yaml
## The name of a secret in the same kubernetes namespace which contain values to be added to the environment
## This can be useful for auth tokens, etc. Value is templated.
envFromSecret: ""

## Sensible environment variables that will be rendered as new secret object
## This can be useful for auth tokens, etc
envRenderSecret: {}

# deployment
    {{- if .Values.envFromSecret }}
    envFrom:
      - secretRef:
          name: {{ tpl .Values.envFromSecret . }}
    {{- end }}
    {{- if .Values.envRenderSecret }}
    envFrom:
      - secretRef:
          name: {{ template "grafana.fullname" . }}-env
    {{- end }}

This would avoid having to add all secret env variables one by one for each component.

[rsteneteg]

@mkjoerg the downside I can see it that by allowing the use of env var expansions you are enabling users to use a less secure method. Sure there are use cases where you control the whole host and trust all the other apps on that same host, but you are opening a possible security concern for users that may not be aware of the dangers of env vars for secrets.

Another potential problem with your approach of managing the secret outside of the helm chart would be that if you update that secret is that I am not sure if Cortex would automatically detect those changes.

And since you are still suggesting storing the secrets outside, we may just as well store the entire config outside, and use the suggested annotation in my response above to tell the chart that the external config has changed that will trigger a restart of the pods.

I think that another possibility if you still want to use env vars.. is to specify all options that has env vars as args for the command line instead of in the config.

[containerpope]

@rsteneteg thanks for the quick feedback, and sorry for not getting back to you earlier.

I agree, that this solution is less secure, but it is basically a simplification of what you allow by setting and expanding env variables anyway. In the proposed use case, I could instantiate a secret before the helm deployment and then one by one mount the keys of the secret to environment variables. Getting this done by envFrom is only removing the need to hand over each key manually in each deployment. As soon as you provide the secret with the configuration externally, you have the update problem in my opinion.

From my point of view, I would only provide secrets to the resources (Azure CosmosDB and Blob) via this method and prepare them in one secret that I seal and commit to git. I would then create a HelmResource to deploy Cortex and reference the secret where needed (using expand option). Without this I would be forced to provide the values in cleartext into the values.yaml in git. Having the secret standalone is a calculated situation, where I willingly take into account, that there are no automatic updates if I update the secret. But only a rotation in CosmosDB or Blob would make this needed.

The envFrom is only reducing the amount of text I have to put into the values.yaml, getting the same outcome.

[rsteneteg]

@mkjoerg I don't any harm in adding support for the envFrom syntax since it would not enable anything that is not already possible.

[khaines]

@rsteneteg added an externalConfigVersion property to the config. Since we don't really need the checksum here, the only purpose is to change the value when the config is updated. It's a string, so if build pipelines (or a human) just change the value before running a helm update, it will cause updates to all the deployments.

[khaines]

I think the conversation on optimizing what goes in the given file and if we can extract secrets even further is good, but it is very much a continuance of what this PR is doing; so it should be taken up with a different PR. @mkjoerg I believe I understand what you are interested in with your suggestion and I'm going to think about some alternative approaches which do it securely.

[rsteneteg]

LGTM