Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ao configmap: fixes for handling boolean and numeric values #48

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

galmasi
Copy link

@galmasi galmasi commented Jan 5, 2024

Summary

This is a small fix to allow the helm-generated configmap to accommodate boolean and numeric values -- it forces extra quotes into the configmap.

How to produce the error

The following snippet in values.yaml will cause helm deployment failure. It does not matter whether the word false is quoted, doublequoted, escaped in any way -- as long as it's recognizable as a boolean the failure will happen.

Similar problems happen with any value that is JSON/YAML parseable as an integer.

global:
  configmap:
    configParams:
      KEYLIME_TENANT_REQUIRE_EK_CERT: false

Gist of the fix

Produce extra quotes in the configmap template[s] to prevent parsing of affected values as anything but string.

Affected locations

Two places I noticed that are using .Values.global.configmap.configParams:

  • charts/keylime-init/templates/ca-job.yaml -- global environment variables used to direct CA generation
  • templates/configmap.yaml -- the actual copying of environment variables into the keylime configmap

Why we need this

I cannot start to produce meaningful CI tests without disabling EK cert requirements on AWS machines (since AWS TPMs don't have EK certificates). Expect a second, much larger PR for meaningful CI tests with AWS EC2 VMs as soon as this is merged.

Signed-off-by: George Almasi <gheorghe@us.ibm.com>
@maugustosilva
Copy link
Contributor

Obvious but sorely needed fix. LGTM

@maugustosilva maugustosilva merged commit bac2fda into keylime:main Jan 8, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants