CLI patches Psych for K8s compatible YAML serialization

Fixes issue: #740

lib/krane/cli/krane.rb

@@ -17,6 +17,9 @@ class Krane < Thor
 
       package_name "Krane"
 
+      # Hack: CLI activates patch of Psych to ensure serialized YAML is K8s compatible.
+      PsychK8sPatch.activate
+
       def self.expand_options(task_options)
         task_options.each { |option_name, config| method_option(option_name, config) }
       end

lib/krane/common.rb

@@ -17,6 +17,7 @@
 require 'krane/statsd'
 require 'krane/task_config'
 require 'krane/task_config_validator'
+require 'krane/psych_k8s_patch'
 
 module Krane
   MIN_KUBE_VERSION = '1.11.0'

lib/krane/psych_k8s_patch.rb

@@ -0,0 +1,62 @@
+# frozen_string_literal: true
+
+require 'psych'
+
+# PsychK8sPatch applies patching to Psych.dump/dump_stream with an alternative string serializer that is more compatible
+# with Kubernetes (and other Go-based tooling). Related issue: https://github.com/Shopify/krane/issues/740
+module PsychK8sPatch
+  # Activate will apply the patch after creating backup aliases for the original methods.
+  def self.activate
+    class << Psych
+      raise "Patch was already activated!" if @activated
+      @activated = true
+      alias_method :__orig__dump, :dump
+      alias_method :__orig__dump_stream, :dump_stream
+
+      def dump(o, io = nil, options = {})
+        if io.is_a?(Hash)
+          options = io
+          io = nil
+        end
+
+        visitor = Psych::Visitors::YAMLTree.create(options)
+        visitor << o
+        visitor.tree.each { |n| PsychK8sPatch.massage_node(n) }
+        visitor.tree.yaml(io, options)
+      end
+
+      def dump_stream(*objects)
+        visitor = Psych::Visitors::YAMLTree.create({})
+        objects.each do |o|
+          visitor << o
+        end
+        visitor.tree.each { |n| PsychK8sPatch.massage_node(n) }
+        visitor.tree.yaml
+      end
+    end
+  end
+
+  # Deactivate will restore the original methods from backup aliases.
+  def self.deactivate
+    class << Psych
+      raise "Patch was not activated!" if !@activated
+      @activated = false
+      alias_method :dump, :__orig__dump
+      alias_method :dump_stream, :__orig__dump_stream
+    end
+  end
+
+  private
+
+  # fix_node applies DOUBLE_QUOTED style to string scalars that look like scientific/e-notation numbers.
+  # This is required by YAML 1.2. Failure to do so results in Go-based tools (ie: K8s) to interpret as number!
+  def self.massage_node(n)
+    if n.is_a?(Psych::Nodes::Scalar) &&
+        (n.style == Psych::Nodes::Scalar::PLAIN) &&
+        n.value.is_a?(String) &&
+        n.value =~ /\A[+-]?\d+(?:\.\d+)?[eE][+-]?\d+\z/
+
+      n.style = Psych::Nodes::Scalar::DOUBLE_QUOTED
+    end
+  end
+end

test/fixtures/for_unit_tests/pod_with_e_notation_env.yml

@@ -0,0 +1,34 @@
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: web
+  annotations:
+    shipit.shopify.io/restart: "true"
+  labels:
+    name: web
+    app: hello-cloud
+spec:
+  replicas: 1
+  selector:
+    matchLabels:
+      name: web
+      app: hello-cloud
+  progressDeadlineSeconds: 60
+  template:
+    metadata:
+      labels:
+        name: web
+        app: hello-cloud
+    spec:
+      containers:
+      - name: app
+        image: busybox
+        imagePullPolicy: IfNotPresent
+        command: ["tail", "-f", "/dev/null"]
+        ports:
+        - containerPort: 80
+          name: http
+        env:
+        - name: FOO
+          value: "123e4"

test/unit/krane/psych_k8s_patch_test.rb

@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+require 'test_helper'
+
+class PsychK8sPatchTest < Krane::TestCase
+
+  INPUTS = [
+      'a: "123e4"',
+      'a: "123E4"',
+      'a: "+123e4"',
+      'a: "-123e4"',
+      'a: "123e+4"',
+      'a: "123e-4"',
+      'a: "123.0e-4"'
+  ]
+
+  EXPECTED_DEACTIVATED = [
+      %(---\n- a: 123e4\n), # Psych sees this as non-numeric; deviates from YAML 1.2 spec :(
+      %(---\n- a: 123E4\n), # Psych sees this as non-numeric; deviates from YAML 1.2 spec :(
+      %(---\n- a: "+123e4"\n), # Psych sees this as non-numeric; deviates from YAML 1.2 spec; quoted due to '+' :|
+      %(---\n- a: "-123e4"\n), # Psych sees this as non-numeric; deviates from YAML 1.2 spec; quoted due to '-' :|
+      %(---\n- a: 123e+4\n), # Psych sees this as non-numeric; deviates from YAML 1.2 spec :(
+      %(---\n- a: 123e-4\n), # Psych sees this as non-numeric; deviates from YAML 1.2 spec :(
+      %(---\n- a: '123.0e-4'\n), # Psych sees this as numeric; encapsulated with single quotes :)
+  ]
+
+  EXPECTED_ACTIVATED = [
+      %(---\n- a: "123e4"\n),
+      %(---\n- a: "123E4"\n),
+      %(---\n- a: "+123e4"\n),
+      %(---\n- a: "-123e4"\n),
+      %(---\n- a: "123e+4"\n),
+      %(---\n- a: "123e-4"\n),
+      %(---\n- a: '123.0e-4'\n),
+  ]
+
+  def test_dump
+    run_all_test_cases(->(n) { Psych.dump(n) })
+  end
+
+  def test_dump_stream
+    run_all_test_cases(->(n) { Psych.dump_stream(n) })
+  end
+
+  def test_to_yaml
+    run_all_test_cases(->(n) { n.to_yaml })
+  end
+
+  def run_all_test_cases(serializer)
+    run_test_cases(INPUTS, EXPECTED_DEACTIVATED, serializer)
+    run_test_cases_activated(INPUTS, EXPECTED_ACTIVATED, serializer)
+  end
+
+  def run_test_cases(inputs, expectations, serializer)
+    (0..inputs.length - 1).each { |i|
+      loaded = YAML.load_stream(inputs[i])
+      assert_equal(expectations[i].strip, serializer.call(loaded).strip)
+    }
+  end
+
+  def run_test_cases_activated(inputs, expectations, serializer)
+    PsychK8sPatch.activate
+    begin
+      run_test_cases(inputs, expectations, serializer)
+    ensure
+      PsychK8sPatch.deactivate
+    end
+  end
+end

test/unit/krane/renderer_test.rb

@@ -43,6 +43,103 @@ def test_can_render_template_with_correct_indentation
     assert_equal(expected, actual)
   end
 
+  def test_renders_env_with_e_notation_incorrectly_when_psych_patch_deactivated
+    expected = <<~EOY
+      ---
+      apiVersion: apps/v1
+      kind: Deployment
+      metadata:
+        name: web
+        annotations:
+          shipit.shopify.io/restart: 'true'
+        labels:
+          name: web
+          app: hello-cloud
+      spec:
+        replicas: 1
+        selector:
+          matchLabels:
+            name: web
+            app: hello-cloud
+        progressDeadlineSeconds: 60
+        template:
+          metadata:
+            labels:
+              name: web
+              app: hello-cloud
+          spec:
+            containers:
+            - name: app
+              image: busybox
+              imagePullPolicy: IfNotPresent
+              command:
+              - tail
+              - "-f"
+              - "/dev/null"
+              ports:
+              - containerPort: 80
+                name: http
+              env:
+              - name: FOO
+                value: 123e4
+    EOY
+    actual = YAML.load_stream(render("pod_with_e_notation_env.yml")).map do |t|
+      YAML.dump(t)
+    end.join
+    assert_equal(expected, actual)
+  end
+
+  def test_renders_env_with_e_notation_correctly_when_psych_patch_activated
+    expected = <<~EOY
+      ---
+      apiVersion: apps/v1
+      kind: Deployment
+      metadata:
+        name: web
+        annotations:
+          shipit.shopify.io/restart: 'true'
+        labels:
+          name: web
+          app: hello-cloud
+      spec:
+        replicas: 1
+        selector:
+          matchLabels:
+            name: web
+            app: hello-cloud
+        progressDeadlineSeconds: 60
+        template:
+          metadata:
+            labels:
+              name: web
+              app: hello-cloud
+          spec:
+            containers:
+            - name: app
+              image: busybox
+              imagePullPolicy: IfNotPresent
+              command:
+              - tail
+              - "-f"
+              - "/dev/null"
+              ports:
+              - containerPort: 80
+                name: http
+              env:
+              - name: FOO
+                value: "123e4"
+    EOY
+    PsychK8sPatch.activate
+    begin
+      actual = YAML.load_stream(render("pod_with_e_notation_env.yml")).map do |t|
+        YAML.dump(t)
+      end.join
+    ensure
+      PsychK8sPatch.deactivate
+    end
+    assert_equal(expected, actual)
+  end
+
   def test_invalid_partial_raises
     err = assert_raises(Krane::InvalidTemplateError) do
       render('broken-partial-inclusion.yaml.erb')