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

YAML string scalars with scientific/e-notation numeric format not properly quoted #740

Closed
jpfourny opened this issue Aug 20, 2020 · 1 comment

Comments

@jpfourny
Copy link
Contributor

jpfourny commented Aug 20, 2020

Bug report

Due to discrepancies between Go YAML and Ruby YAML (psych), we have run into deployment failures when YAML contains strings that encapsulate numbers with a specific variants of scientific/e-notation format. The discrepancy lies in the fact that Go's YAML parser (mostly) follows YAML 1.2, which accepts a wider range of scientific/e-notation variants as compared to psych. Despite having properly encapsulated (quoted) our string, the final output that Krane pushes to K8s API server is interpreted as a number, rather than a string. This causes K8s to reject the patch.

Example input fragment:

...
env:
  - name: FOO
  - value: "1e3"

Resulting JSON patch applied to Kubernetes:

"env":[{"name":"FOO","value":1000}]

Changing the number format is not a workaround for us, since this has been triggered in the real world with ENV vars that contain hex-formatted API keys that just happen to have the form <digits>'e'<digits>.

Expected behaviour:
Krane preserves encapsulation (quotes) for string scalars that would otherwise be interpreted as numbers.

Actual behaviour:
Krane serializes strings that without encapsulation, causing any YAML 1.2-compliant consumers to read a number.

Version(s) affected: [run krane version]
krane 1.1.4 (AFAIK it has always been broken)

Steps to Reproduce

  1. Using Kubenetes 1.15+ client and server.
  2. Create a deployment spec that includes an ENV var with double-quoted value: "1e3".
  3. Run krane deploy <namespace> <context> -f <deployment_file>
  4. Observe output like the following:
[INFO][2020-08-20 11:12:31 -0400]	------------------------------------------Result: FAILURE-------------------------------------------
[FATAL][2020-08-20 11:12:31 -0400]	Command failed: apply -f /var/folders/1n/6_9v8ch14lj24rhs_qbq835c0000gn/t/d20200820-34356-rce2yd
[FATAL][2020-08-20 11:12:31 -0400]
[FATAL][2020-08-20 11:12:31 -0400]	WARNING: Any resources not mentioned in the error(s) below were likely created/updated. You may wish to roll back this deploy.
[FATAL][2020-08-20 11:12:31 -0400]
[FATAL][2020-08-20 11:12:31 -0400]	Invalid template: Deployment-nginx-deployment20200820-34356-1vih8on.yml
[FATAL][2020-08-20 11:12:31 -0400]	> Error message:
[FATAL][2020-08-20 11:12:31 -0400]	    for: "/var/folders/1n/6_9v8ch14lj24rhs_qbq835c0000gn/T/d20200820-34356-rce2yd/Deployment-nginx-deployment20200820-34356-1vih8on.yml": cannot convert int64 to string
[FATAL][2020-08-20 11:12:31 -0400]	> Template content:
[FATAL][2020-08-20 11:12:31 -0400]	    ---
[FATAL][2020-08-20 11:12:31 -0400]	    apiVersion: apps/v1
[FATAL][2020-08-20 11:12:31 -0400]	    kind: Deployment
[FATAL][2020-08-20 11:12:31 -0400]	    metadata:
[FATAL][2020-08-20 11:12:31 -0400]	      name: nginx-deployment
[FATAL][2020-08-20 11:12:31 -0400]	      labels:
[FATAL][2020-08-20 11:12:31 -0400]	        app: nginx
[FATAL][2020-08-20 11:12:31 -0400]	    spec:
[FATAL][2020-08-20 11:12:31 -0400]	      replicas: 3
[FATAL][2020-08-20 11:12:31 -0400]	      selector:
[FATAL][2020-08-20 11:12:31 -0400]	        matchLabels:
[FATAL][2020-08-20 11:12:31 -0400]	          app: nginx
[FATAL][2020-08-20 11:12:31 -0400]	      template:
[FATAL][2020-08-20 11:12:31 -0400]	        metadata:
[FATAL][2020-08-20 11:12:31 -0400]	          labels:
[FATAL][2020-08-20 11:12:31 -0400]	            app: nginx
[FATAL][2020-08-20 11:12:31 -0400]	        spec:
[FATAL][2020-08-20 11:12:31 -0400]	          containers:
[FATAL][2020-08-20 11:12:31 -0400]	          - name: nginx
[FATAL][2020-08-20 11:12:31 -0400]	            image: nginx:1.14.2
[FATAL][2020-08-20 11:12:31 -0400]	            ports:
[FATAL][2020-08-20 11:12:31 -0400]	            - containerPort: 80
[FATAL][2020-08-20 11:12:31 -0400]	            env:
[FATAL][2020-08-20 11:12:31 -0400]	            - name: FOO
[FATAL][2020-08-20 11:12:31 -0400]	              value: 1e3
[FATAL][2020-08-20 11:12:31 -0400]
[FATAL][2020-08-20 11:12:31 -0400]
[FATAL][2020-08-20 11:12:31 -0400]	Unidentified error(s):
[FATAL][2020-08-20 11:12:31 -0400]	    Error from server: error when applying patch:
[FATAL][2020-08-20 11:12:31 -0400]	    {"metadata":{"annotations":{"kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"apps/v1\",\"kind\":\"Deployment\",\"metadata\":{\"annotations\":{},\"labels\":{\"app\":\"nginx\"},\"name\":\"nginx-deployment\",\"namespace\":\"default\"},\"spec\":{\"replicas\":3,\"selector\":{\"matchLabels\":{\"app\":\"nginx\"}},\"template\":{\"metadata\":{\"labels\":{\"app\":\"nginx\"}},\"spec\":{\"containers\":[{\"env\":[{\"name\":\"FOO\",\"value\":1000}],\"image\":\"nginx:1.14.2\",\"name\":\"nginx\",\"ports\":[{\"containerPort\":80}]}]}}}}\n"}},"spec":{"template":{"spec":{"$setElementOrder/containers":[{"name":"nginx"}],"containers":[{"$setElementOrder/env":[{"name":"FOO"}],"env":[{"name":"FOO","value":1000}],"name":"nginx"}]}}}}

Proposal:
We have faced similar problems with other internal tools at Shopify, and the workaround has been to monkey-patch Psych.dump/dump_stream to massage the YAML. I can prepare a PR for consideration. As this is public-facing OSS, I cannot push such hacks without consensus from primary maintainers.

jpfourny added a commit that referenced this issue Aug 21, 2020
@jpfourny jpfourny changed the title krane deploy ignores quotes in strings that encapsulate scientific/e-notation formatted numbers YAML string scalars with scientific/e-notation numeric format not properly quoted Aug 22, 2020
@jpfourny
Copy link
Contributor Author

Fixed in krane 2.0.0

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

No branches or pull requests

1 participant