From 04f5e6c953e345af9aa69eb70bd6c0e9ccb557e4 Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Mon, 6 Jan 2020 10:35:30 -0800 Subject: [PATCH] Functions for identifying and fixing yaml 1.1 compatbility issues - Identify if a field value would parse as a non-string yaml 1.1 value - Using OpenAPI Schema to get Resource's field type - Format yaml so value will be parsed as the correct type using a yaml 1.1 parser --- kyaml/yaml/compatibility.go | 85 ++++++++++++++++++ kyaml/yaml/compatibility_test.go | 147 +++++++++++++++++++++++++++++++ 2 files changed, 232 insertions(+) create mode 100644 kyaml/yaml/compatibility.go create mode 100644 kyaml/yaml/compatibility_test.go diff --git a/kyaml/yaml/compatibility.go b/kyaml/yaml/compatibility.go new file mode 100644 index 000000000..ff64f6f06 --- /dev/null +++ b/kyaml/yaml/compatibility.go @@ -0,0 +1,85 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package yaml + +import ( + "reflect" + "strings" + + "github.com/go-openapi/spec" + y1_1 "gopkg.in/yaml.v2" + y1_2 "gopkg.in/yaml.v3" +) + +// typeToTag maps OpenAPI schema types to yaml 1.2 tags +var typeToTag = map[string]string{ + "string": "!!str", + "integer": "!!int", + "boolean": "!!bool", + "number": "!!float", +} + +// FormatNonStringStyle makes sure that values which parse as non-string values in yaml 1.1 +// are correctly formatted given the Schema type. +func FormatNonStringStyle(node *Node, schema spec.Schema) { + if len(schema.Type) != 1 { + return + } + t := schema.Type[0] + + if !IsYaml1_1NonString(node) { + return + } + switch { + case t == "string" && schema.Format != "int-or-string": + if (node.Style&DoubleQuotedStyle == 0) && (node.Style&SingleQuotedStyle == 0) { + // must quote values so they are parsed as strings + node.Style = DoubleQuotedStyle + } + case t == "boolean" || t == "integer" || t == "number": + if (node.Style&DoubleQuotedStyle != 0) || (node.Style&SingleQuotedStyle != 0) { + // must NOT quote the values so they aren't parsed as strings + node.Style = 0 + } + default: + return + } + if tag, found := typeToTag[t]; found { + // make sure the right tag is set + node.Tag = tag + } +} + +// IsYaml1_1NonString returns true if the value parses as a non-string value in yaml 1.1 +// when unquoted. +// +// Note: yaml 1.2 uses different keywords than yaml 1.1. Example: yaml 1.2 interprets +// `field: on` and `field: "on"` as equivalent (both strings). However Yaml 1.1 interprets +// `field: on` as on being a bool and `field: "on"` as on being a string. +// If an input is read with `field: "on"`, and the style is changed from DoubleQuote to 0, +// it will change the type of the field from a string to a bool. For this reason, fields +// which are keywords in yaml 1.1 should never have their style changed, as it would break +// backwards compatibility with yaml 1.1 -- which is what is used by the Kubernetes apiserver. +func IsYaml1_1NonString(node *Node) bool { + if node.Kind != y1_2.ScalarNode { + // not a keyword + return false + } + if strings.Contains(node.Value, "\n") { + // multi-line strings will fail to unmarshal + return false + } + // check if the value will unmarshal into a non-string value using a yaml 1.1 parser + var i1 interface{} + if err := y1_1.Unmarshal([]byte(node.Value), &i1); err != nil { + return false + } + if reflect.TypeOf(i1) != stringType { + return true + } + + return false +} + +var stringType = reflect.TypeOf("string") diff --git a/kyaml/yaml/compatibility_test.go b/kyaml/yaml/compatibility_test.go new file mode 100644 index 000000000..3a85910b6 --- /dev/null +++ b/kyaml/yaml/compatibility_test.go @@ -0,0 +1,147 @@ +// Copyright 2019 The Kubernetes Authors. +// SPDX-License-Identifier: Apache-2.0 + +package yaml_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "sigs.k8s.io/kustomize/kyaml/openapi" + "sigs.k8s.io/kustomize/kyaml/yaml" +) + +func TestIsYaml1_1NonString(t *testing.T) { + type testCase struct { + val string + expected bool + } + + testCases := []testCase{ + {val: "hello world", expected: false}, + {val: "2.0", expected: true}, + {val: "1.0\nhello", expected: false}, // multiline strings should always be false + } + + for k := range valueToTagMap { + testCases = append(testCases, testCase{val: k, expected: true}) + } + + for _, test := range testCases { + assert.Equal(t, test.expected, + yaml.IsYaml1_1NonString(&yaml.Node{Kind: yaml.ScalarNode, Value: test.val}), test.val) + } +} + +func TestFormatNonStringStyle(t *testing.T) { + n := yaml.MustParse(`apiVersion: apps/v1 +kind: Deployment +spec: + template: + spec: + containers: + - name: foo + args: + - bar + - on + image: nginx:1.7.9 + ports: + - name: http + containerPort: "80" +`) + s := openapi.SchemaForResourceType( + yaml.TypeMeta{APIVersion: "apps/v1", Kind: "Deployment"}) + + args, err := n.Pipe(yaml.Lookup( + "spec", "template", "spec", "containers", "[name=foo]", "args")) + if !assert.NoError(t, err) { + t.FailNow() + } + if !assert.NotNil(t, args) { + t.FailNow() + } + on := args.YNode().Content[1] + onS := s. + SchemaForField("spec"). + SchemaForField("template"). + SchemaForField("spec"). + SchemaForField("containers"). + SchemaForElements(). + SchemaForField("args"). + SchemaForElements() + yaml.FormatNonStringStyle(on, *onS.Schema) + + containerPort, err := n.Pipe(yaml.Lookup( + "spec", "template", "spec", "containers", "[name=foo]", "ports", + "[name=http]", "containerPort")) + if !assert.NoError(t, err) { + t.FailNow() + } + if !assert.NotNil(t, containerPort) { + t.FailNow() + } + cpS := s. + SchemaForField("spec"). + SchemaForField("template"). + SchemaForField("spec"). + SchemaForField("containers"). + SchemaForElements(). + SchemaForField("ports"). + SchemaForElements(). + SchemaForField("containerPort") + if !assert.NotNil(t, cpS) { + t.FailNow() + } + yaml.FormatNonStringStyle(containerPort.YNode(), *cpS.Schema) + + actual := n.MustString() + expected := `apiVersion: apps/v1 +kind: Deployment +spec: + template: + spec: + containers: + - name: foo + args: + - bar + - "on" + image: nginx:1.7.9 + ports: + - name: http + containerPort: 80 +` + assert.Equal(t, expected, actual) +} + +// valueToTagMap is a map of values interpreted as non-strings in yaml 1.1 when left +// unquoted. +// To keep compatibility with the yaml parser used by Kubernetes (yaml 1.1) make sure the values +// which are treated as non-string values are kept as non-string values. +// https://github.com/go-yaml/yaml/blob/v2/resolve.go +var valueToTagMap = func() map[string]string { + val := map[string]string{} + + // https://yaml.org/type/null.html + values := []string{"", "~", "null", "Null", "NULL"} + for i := range values { + val[values[i]] = "!!null" + } + + // https://yaml.org/type/bool.html + values = []string{ + "y", "Y", "yes", "Yes", "YES", "true", "True", "TRUE", "on", "On", "ON", "n", "N", "no", + "No", "NO", "false", "False", "FALSE", "off", "Off", "OFF"} + for i := range values { + val[values[i]] = "!!bool" + } + + // https://yaml.org/type/float.html + values = []string{ + ".nan", ".NaN", ".NAN", ".inf", ".Inf", ".INF", + "+.inf", "+.Inf", "+.INF", "-.inf", "-.Inf", "-.INF"} + for i := range values { + val[values[i]] = "!!float" + } + + return val +}()