kustomize icon indicating copy to clipboard operation
kustomize copied to clipboard

configMapGenerator: double quote handling incorrect when generating a config map from properties file

Open kingofdisasterr opened this issue 3 years ago • 4 comments

Description

The configMapGenerator handles double quotes wrong when a config map is generated from a properties file. When double quotes or single quotes are provided in the properties file, they should be kept in the output. If necessary, kustomize should add single quotes to the output, i.e. when the property value begins with a space character. Reserved yaml words should be surrounded with double quotes -> works as expected. Numbers with leading 0 should be treated as string and therefore surrounded with double quotes. Numbers without leading 0 should be kept as number in the output.

Files that can reproduce the issue

base/kustomization.yaml

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
configMapGenerator:
  - name: app-config-map
    envs:
      - application.env

base/application.env

app.base.name=appName

overlay/kustomization.yaml

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization
resources:
  - ../base
configMapGenerator:
  - name: app-config-map
    envs:
      - application-overlay.env
    behavior: merge

overlay/application-overlay.env

spring.datasource.h2db.url1="jdbc:h2:tcp://appl-svc.appl-e1.svc.cluster.local:9050/h2db;MODE=Oracle;DATABASE_TO_LOWER=TRUE;INIT=CREATE SCHEMA IF NOT EXISTS APP_SCHEMA\\;SET SCHEMA APP_SCHEMA"
spring.datasource.h2db.url2='jdbc:h2:tcp://appl-svc.appl-e1.svc.cluster.local:9050/h2db;MODE=Oracle;DATABASE_TO_LOWER=TRUE;INIT=CREATE SCHEMA IF NOT EXISTS APP_SCHEMA\\;SET SCHEMA APP_SCHEMA'
spring.datasource.h2db.url3=jdbc:h2:tcp://appl-svc.appl-e1.svc.cluster.local:9050/h2db;MODE=Oracle;DATABASE_TO_LOWER=TRUE;INIT=CREATE SCHEMA IF NOT EXISTS APP_SCHEMA\\;SET SCHEMA APP_SCHEMA
spring.datasource.h2db.url4= jdbc:h2:tcp://appl-svc.appl-e1.svc.cluster.local:9050/h2db;MODE=Oracle;DATABASE_TO_LOWER=TRUE;INIT=CREATE SCHEMA IF NOT EXISTS APP_SCHEMA\\;SET SCHEMA APP_SCHEMA
test.007=007
test.7=7

Expected output

apiVersion: v1
data:
  app.base.name: appName
  spring.datasource.h2db.url1: "jdbc:h2:tcp://appl-svc.appl-e1.svc.cluster.local:9050/h2db;MODE=Oracle;DATABASE_TO_LOWER=TRUE;INIT=CREATE
    SCHEMA IF NOT EXISTS APP_SCHEMA\\;SET SCHEMA APP_SCHEMA"
  spring.datasource.h2db.url2: 'jdbc:h2:tcp://appl-svc.appl-e1.svc.cluster.local:9050/h2db;MODE=Oracle;DATABASE_TO_LOWER=TRUE;INIT=CREATE
    SCHEMA IF NOT EXISTS APP_SCHEMA\\;SET SCHEMA APP_SCHEMA'
  spring.datasource.h2db.url3: jdbc:h2:tcp://appl-svc.appl-e1.svc.cluster.local:9050/h2db;MODE=Oracle;DATABASE_TO_LOWER=TRUE;INIT=CREATE
    SCHEMA IF NOT EXISTS APP_SCHEMA\\;SET SCHEMA APP_SCHEMA
  spring.datasource.h2db.url4: ' jdbc:h2:tcp://appl-svc.appl-e1.svc.cluster.local:9050/h2db;MODE=Oracle;DATABASE_TO_LOWER=TRUE;INIT=CREATE
    SCHEMA IF NOT EXISTS APP_SCHEMA\\;SET SCHEMA APP_SCHEMA'
  test.7: 7
  test.007: "007"
kind: ConfigMap
metadata:
  name: app-config-map-c6k5f8b75g

Actual output

apiVersion: v1
data:
  app.base.name: appName
  spring.datasource.h2db.url1: '"jdbc:h2:tcp://appl-svc.appl-e1.svc.cluster.local:9050/h2db;MODE=Oracle;DATABASE_TO_LOWER=TRUE;INIT=CREATE
    SCHEMA IF NOT EXISTS APP_SCHEMA\\;SET SCHEMA APP_SCHEMA"'
  spring.datasource.h2db.url2: '''jdbc:h2:tcp://appl-svc.appl-e1.svc.cluster.local:9050/h2db;MODE=Oracle;DATABASE_TO_LOWER=TRUE;INIT=CREATE
    SCHEMA IF NOT EXISTS APP_SCHEMA\\;SET SCHEMA APP_SCHEMA'''
  spring.datasource.h2db.url3: jdbc:h2:tcp://appl-svc.appl-e1.svc.cluster.local:9050/h2db;MODE=Oracle;DATABASE_TO_LOWER=TRUE;INIT=CREATE
    SCHEMA IF NOT EXISTS APP_SCHEMA\\;SET SCHEMA APP_SCHEMA
  spring.datasource.h2db.url4: ' jdbc:h2:tcp://appl-svc.appl-e1.svc.cluster.local:9050/h2db;MODE=Oracle;DATABASE_TO_LOWER=TRUE;INIT=CREATE
    SCHEMA IF NOT EXISTS APP_SCHEMA\\;SET SCHEMA APP_SCHEMA'
  test.7: "7"
  test.007: "007"
kind: ConfigMap
metadata:
  name: app-config-map-c6k5f8b75g

Kustomize version $ kustomize version {Version:kustomize/v4.4.1 GitCommit:b2d65ddc98e09187a8e38adc27c30bab078c1dbf BuildDate:2021-11-11T23:36:27Z GoOs:windows GoArch:amd64}

Platform Windows

Additional context

The problem described leads to problems in a spring boot application. When starting the application, the database connection fails because of missing double quotes in the config map. This leads spring to cut the connection string after the first semicolon.

kingofdisasterr avatar Apr 06 '22 15:04 kingofdisasterr

@kingofdisasterr: This issue is currently awaiting triage.

SIG CLI takes a lead on issue triage for this repo, but any Kubernetes member can accept issues by applying the triage/accepted label.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot avatar Apr 06 '22 15:04 k8s-ci-robot

When double quotes or single quotes are provided in the properties file, they should be kept in the output.

What do you propose a user do if they want the surrounding double quotes to be included as part of the string? What if they want '"string"'?

If necessary, kustomize should add single quotes to the output, i.e. when the property value begins with a space character.

Why not double?

Numbers with leading 0 should be treated as string and therefore surrounded with double quotes. Numbers without leading 0 should be kept as number in the output.

ConfigMap data are strings. So I would be inclined to think that all numbers in string fields should be quoted.

IMO I would think that kustomize should be double quoting all string fields with special characters unless otherwise necessary. I.e. if you do

spring.datasource.h2db.url1=jdbc:h2:tcp://appl-svc.appl-e1.svc.cluster.local:9050/h2db;MODE=Oracle;DATABASE_TO_LOWER=TRUE;INIT=CREATE SCHEMA IF NOT EXISTS APP_SCHEMA\\;SET SCHEMA APP_SCHEMA

you should get

spring.datasource.h2db.url1: "jdbc:h2:tcp://appl-svc.appl-e1.svc.cluster.local:9050/h2db;MODE=Oracle;DATABASE_TO_LOWER=TRUE;INIT=CREATE SCHEMA IF NOT EXISTS APP_SCHEMA\\;SET SCHEMA APP_SCHEMA"

Do you see any potential problems with that?

In any case, I don't know if this is something we can fix in kustomize. My guess is that this change would be something in the go-yaml library.

natasha41575 avatar Apr 13 '22 16:04 natasha41575

I actually think the current implementation is what most people would expect.

.env file are not properties file: I'm not sure there is a defined standard for any of them but .env file should not contain any simple/double quote in the 1st place unless they are expected in the output because .env files only can contain string values. In key=value, value is always taken as is, as a string.

On the other side, configmap to be generated by Kustomize is expected to be a YAML object where values are YAML strings. In YAML a string is either wrapped by ' or " (or nothing) depending on the content to be escaped.

Agree with @natasha41575 that all of this is "standard YAML" (even though I admit it's not really a nice experience) and not something specific to Kustomize.

@kingofdisasterr how do you use the configmap in your container? What do you expect in the container in the end?

gaeljw avatar Apr 19 '22 20:04 gaeljw

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Jul 18 '22 21:07 k8s-triage-robot

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot avatar Aug 17 '22 22:08 k8s-triage-robot

I agree with @natasha41575 and @gaeljw that Kustomize seems to be doing the right thing here, given that in the source we're talking about a properties file (where all quotes are literals) and in the configmap we're talking about YAML (where a first set of quotes only identifies the string type, and only properly escaped internal quotes are literals). To summarize:

  • If the properties file contains quotes, they should be interpreted as literals and retained. To retain them, Kustomize must wrap them in additional quotes so that the YAML parser will not remove them as redundantly indicating the string type. Therefore url1 is wrapped in '" ... "' and url2 is wrapped in ''' ... '''.
  • If required to ensure the content is preserved and valid YAML (for example because it starts with a space that would otherwise be treated as non-content in YAML, or because it ends in a colon that would be treated as a key identifier in YAML), Kustomize should add quotes. Therefore url4 gets quoted.
  • If required to distinguish types (for example because it looks like an integer or a boolean), Kustomize should add quotes to force the string type. Sometimes this isn't strictly necessary from a YAML 1.2 perspective, but we need to do it anyway because our output is parsed by yaml 1.1 implementations. Therefore test.007 and test.7 both get quotes.

Going back to the original reason for the issue:

The problem described leads to problems in a spring boot application. When starting the application, the database connection fails because of missing double quotes in the config map. This leads spring to cut the connection string after the first semicolon.

I don't understand what problematic value the application is seeing in this scenario, given that the original post shows it is already possible to have the ConfigMap value include literal double quotes, include literal single quotes, or just be the unquoted value, as needed. The only value that can't be produced is one pair of quotes around the value, because Kustomize sees these as redundant string indicators (correctly AFAICT). I'm going to close this issue, but feel free to reopen with more information about the problem.

KnVerey avatar Aug 22 '22 22:08 KnVerey