From f84277793f0033e3ab8b17251855a3abad4dc723 Mon Sep 17 00:00:00 2001 From: Matthias Riegler Date: Thu, 14 Jul 2022 15:31:28 +0200 Subject: [PATCH 1/3] doc: add ankorstore to adopters Signed-off-by: Matthias Riegler --- ADOPTERS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/ADOPTERS.md b/ADOPTERS.md index 829576634..e0592772c 100644 --- a/ADOPTERS.md +++ b/ADOPTERS.md @@ -2,3 +2,4 @@ * GitLab * Grafana Labs * Yelp +* Ankorstore From ad7cd87dccd923dc79afb59b85ecb674b132e58c Mon Sep 17 00:00:00 2001 From: Matthias Riegler Date: Thu, 14 Jul 2022 16:48:06 +0200 Subject: [PATCH 2/3] feat: implement --force-yaml-string-quotation fixes #687 Signed-off-by: Matthias Riegler --- cmd/tk/export.go | 2 ++ cmd/tk/flags.go | 10 ++++++++ cmd/tk/workflow.go | 10 +++++--- pkg/kubernetes/client/apply.go | 2 +- pkg/kubernetes/client/diff.go | 2 +- pkg/kubernetes/client/get.go | 2 +- pkg/kubernetes/diff.go | 2 +- pkg/kubernetes/manifest/errors.go | 2 +- pkg/kubernetes/manifest/manifest.go | 36 +++++++++++++++++++++++++---- pkg/kubernetes/subsetdiff.go | 4 ++-- pkg/tanka/export.go | 2 +- pkg/tanka/tanka.go | 5 ++++ 12 files changed, 63 insertions(+), 16 deletions(-) diff --git a/cmd/tk/export.go b/cmd/tk/export.go index 1900bd6ac..325f0a663 100644 --- a/cmd/tk/export.go +++ b/cmd/tk/export.go @@ -43,6 +43,7 @@ func exportCmd() *cli.Command { vars := workflowFlags(cmd.Flags()) getJsonnetOpts := jsonnetFlags(cmd.Flags()) getLabelSelector := labelSelectorFlag(cmd.Flags()) + getYAMLOpts := yamlStyleFlags(cmd.Flags()) recursive := cmd.Flags().BoolP("recursive", "r", false, "Look recursively for Tanka environments") @@ -62,6 +63,7 @@ func exportCmd() *cli.Command { Merge: *merge, Opts: tanka.Opts{ JsonnetOpts: getJsonnetOpts(), + YamlOpts: getYAMLOpts(), Filters: filters, Name: vars.name, }, diff --git a/cmd/tk/flags.go b/cmd/tk/flags.go index d9fa63ee9..4e188cf53 100644 --- a/cmd/tk/flags.go +++ b/cmd/tk/flags.go @@ -24,6 +24,16 @@ func workflowFlags(fs *pflag.FlagSet) *workflowFlagVars { return &v } +func yamlStyleFlags(fs *pflag.FlagSet) func() tanka.YAMLOpts { + forceStringQuotation := fs.Bool("force-yaml-string-quotation", false, "enforce YAML quotation for strings") + + return func() tanka.YAMLOpts { + return tanka.YAMLOpts{ + ForceStringQuotation: *forceStringQuotation, + } + } +} + func labelSelectorFlag(fs *pflag.FlagSet) func() labels.Selector { labelSelector := fs.StringP("selector", "l", "", "Label selector. Uses the same syntax as kubectl does") diff --git a/cmd/tk/workflow.go b/cmd/tk/workflow.go index 125ec02b0..bae466f48 100644 --- a/cmd/tk/workflow.go +++ b/cmd/tk/workflow.go @@ -198,6 +198,7 @@ func showCmd() *cli.Command { vars := workflowFlags(cmd.Flags()) getJsonnetOpts := jsonnetFlags(cmd.Flags()) + getYAMLOpts := yamlStyleFlags(cmd.Flags()) cmd.Run = func(cmd *cli.Command, args []string) error { if !interactive && !*allowRedirect { @@ -212,17 +213,20 @@ Otherwise run tk show --dangerous-allow-redirect to bypass this check.`) return err } - pretty, err := tanka.Show(args[0], tanka.Opts{ + opts := tanka.Opts{ JsonnetOpts: getJsonnetOpts(), + YamlOpts: getYAMLOpts(), Filters: filters, Name: vars.name, - }) + } + + pretty, err := tanka.Show(args[0], opts) if err != nil { return err } - return pageln(pretty.String()) + return pageln(pretty.String(opts.YamlOpts.ForceStringQuotation)) } return cmd } diff --git a/pkg/kubernetes/client/apply.go b/pkg/kubernetes/client/apply.go index 40825efda..3502de8d8 100644 --- a/pkg/kubernetes/client/apply.go +++ b/pkg/kubernetes/client/apply.go @@ -48,7 +48,7 @@ func (k Kubectl) Apply(data manifest.List, opts ApplyOpts) error { cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - cmd.Stdin = strings.NewReader(data.String()) + cmd.Stdin = strings.NewReader(data.String(false)) return cmd.Run() } diff --git a/pkg/kubernetes/client/diff.go b/pkg/kubernetes/client/diff.go index 9b6e23e72..2d99c8096 100644 --- a/pkg/kubernetes/client/diff.go +++ b/pkg/kubernetes/client/diff.go @@ -51,7 +51,7 @@ func (k Kubectl) diff(data manifest.List, serverSide bool) (*string, error) { cmd.Stdout = &raw } cmd.Stderr = &fw - cmd.Stdin = strings.NewReader(data.String()) + cmd.Stdin = strings.NewReader(data.String(false)) err := cmd.Run() if diffErr := parseDiffErr(err, fw.buf, k.Info().ClientVersion); diffErr != nil { return nil, diffErr diff --git a/pkg/kubernetes/client/get.go b/pkg/kubernetes/client/get.go index 8cc12474a..ef64e5be5 100644 --- a/pkg/kubernetes/client/get.go +++ b/pkg/kubernetes/client/get.go @@ -45,7 +45,7 @@ func (k Kubectl) GetByLabels(namespace, kind string, labels map[string]string) ( func (k Kubectl) GetByState(data manifest.List, opts GetByStateOpts) (manifest.List, error) { list, err := k.get("", "", []string{"-f", "-"}, getOpts{ ignoreNotFound: opts.IgnoreNotFound, - stdin: data.String(), + stdin: data.String(false), }) if err != nil { return nil, err diff --git a/pkg/kubernetes/diff.go b/pkg/kubernetes/diff.go index d57759b85..1e1c3ef9c 100644 --- a/pkg/kubernetes/diff.go +++ b/pkg/kubernetes/diff.go @@ -174,7 +174,7 @@ func StaticDiffer(create bool) Differ { return func(state manifest.List) (*string, error) { s := "" for _, m := range state { - is, should := m.String(), "" + is, should := m.String(false), "" if create { is, should = should, is } diff --git a/pkg/kubernetes/manifest/errors.go b/pkg/kubernetes/manifest/errors.go index e1b6dbad3..b44070999 100644 --- a/pkg/kubernetes/manifest/errors.go +++ b/pkg/kubernetes/manifest/errors.go @@ -38,7 +38,7 @@ func (s *SchemaError) Error() string { if s.Manifest != nil { msg += bluef("\nPlease check below object:\n") - msg += SampleString(s.Manifest.String()).Indent(2) + msg += SampleString(s.Manifest.String(false)).Indent(2) } return msg diff --git a/pkg/kubernetes/manifest/manifest.go b/pkg/kubernetes/manifest/manifest.go index ea3a53065..035e062e8 100644 --- a/pkg/kubernetes/manifest/manifest.go +++ b/pkg/kubernetes/manifest/manifest.go @@ -9,9 +9,19 @@ import ( "github.com/Masterminds/sprig/v3" "github.com/pkg/errors" "github.com/stretchr/objx" - yaml "gopkg.in/yaml.v2" + yaml "gopkg.in/yaml.v3" ) +func yamlForceStringQuotationRecursivePatch(n *yaml.Node) { + if n.Tag == "!!str" { + n.Style = yaml.DoubleQuotedStyle + } + for _, cNode := range n.Content { + // for all child nodes + yamlForceStringQuotationRecursivePatch(cNode) + } +} + // Manifest represents a Kubernetes API object. The fields `apiVersion` and // `kind` are required, `metadata.name` should be present as well type Manifest map[string]interface{} @@ -31,8 +41,17 @@ func NewFromObj(raw objx.Map) (Manifest, error) { } // String returns the Manifest in yaml representation -func (m Manifest) String() string { - y, err := yaml.Marshal(m) +func (m Manifest) String(forceQuotedStrings bool) string { + + yamlNode := &yaml.Node{} + if err := yamlNode.Encode(m); err != nil { + panic(errors.Wrap(err, "converting manifest to yaml.Node")) + } + if forceQuotedStrings { + yamlForceStringQuotationRecursivePatch(yamlNode) + } + + y, err := yaml.Marshal(yamlNode) if err != nil { // this should never go wrong in normal operations panic(errors.Wrap(err, "formatting manifest")) @@ -262,12 +281,19 @@ type List []Manifest // String returns the List as a yaml stream. In case of an error, it is // returned as a string instead. -func (m List) String() string { +func (m List) String(forceQuotedStrings bool) string { buf := bytes.Buffer{} enc := yaml.NewEncoder(&buf) for _, d := range m { - if err := enc.Encode(d); err != nil { + yamlNode := &yaml.Node{} + if err := yamlNode.Encode(d); err != nil { + panic(errors.Wrap(err, "converting manifest to yaml.Node")) + } + if forceQuotedStrings { + yamlForceStringQuotationRecursivePatch(yamlNode) + } + if err := enc.Encode(yamlNode); err != nil { // This should never happen in normal operations panic(errors.Wrap(err, "formatting manifests")) } diff --git a/pkg/kubernetes/subsetdiff.go b/pkg/kubernetes/subsetdiff.go index ce87dc45c..1feffbb75 100644 --- a/pkg/kubernetes/subsetdiff.go +++ b/pkg/kubernetes/subsetdiff.go @@ -92,10 +92,10 @@ func subsetDiff(c client.Client, m manifest.Manifest) (*difference, error) { return nil, errors.Wrap(err, "getting state from cluster") } - should := m.String() + should := m.String(false) sub := subset(m, rawIs) - is := manifest.Manifest(sub).String() + is := manifest.Manifest(sub).String(false) if is == "{}\n" { is = "" } diff --git a/pkg/tanka/export.go b/pkg/tanka/export.go index 40febb4b1..feae592e7 100644 --- a/pkg/tanka/export.go +++ b/pkg/tanka/export.go @@ -115,7 +115,7 @@ func ExportEnvironments(envs []*v1alpha1.Environment, to string, opts *ExportEnv } // Write manifest - data := m.String() + data := m.String(opts.Opts.YamlOpts.ForceStringQuotation) if err := writeExportFile(path, []byte(data)); err != nil { return err } diff --git a/pkg/tanka/tanka.go b/pkg/tanka/tanka.go index d7f91a088..aa7a87d28 100644 --- a/pkg/tanka/tanka.go +++ b/pkg/tanka/tanka.go @@ -15,9 +15,14 @@ import ( type JsonnetOpts = jsonnet.Opts +type YAMLOpts struct { + ForceStringQuotation bool +} + // Opts specify general, optional properties that apply to all actions type Opts struct { JsonnetOpts + YamlOpts YAMLOpts // Filters are used to optionally select a subset of the resources Filters process.Matchers From a1a9ecf489c3a05a4db2d71b0b2941a9e6163929 Mon Sep 17 00:00:00 2001 From: Matthias Riegler Date: Thu, 14 Jul 2022 17:03:15 +0200 Subject: [PATCH 3/3] chore: add testcase Signed-off-by: Matthias Riegler --- pkg/kubernetes/manifest/manifest_test.go | 87 +++++++++++++++++++----- 1 file changed, 69 insertions(+), 18 deletions(-) diff --git a/pkg/kubernetes/manifest/manifest_test.go b/pkg/kubernetes/manifest/manifest_test.go index f821f3f53..767045d7b 100644 --- a/pkg/kubernetes/manifest/manifest_test.go +++ b/pkg/kubernetes/manifest/manifest_test.go @@ -9,28 +9,79 @@ import ( "gopkg.in/yaml.v3" ) -// UnmarshalExpect defines the expected Unmarshal result. Types are very -// important here, only nil, float64, bool, string, map[string]interface{} and -// []interface{} may exist. -var UnmarshalExpect = Manifest{ - "apiVersion": string("apps/v1"), - "kind": string("Deployment"), - "metadata": map[string]interface{}{ - "name": string("MyDeployment"), - }, - "spec": map[string]interface{}{ - "replicas": float64(3), - "template": map[string]interface{}{ - "spec": map[string]interface{}{ - "containers": []interface{}{ - map[string]interface{}{ - "name": string("nginx"), - "image": string("nginx:1.14.2"), +var ( + // UnmarshalExpect defines the expected Unmarshal result. Types are very + // important here, only nil, float64, bool, string, map[string]interface{} and + // []interface{} may exist. + UnmarshalExpect = Manifest{ + "apiVersion": string("apps/v1"), + "kind": string("Deployment"), + "metadata": map[string]interface{}{ + "name": string("MyDeployment"), + }, + "spec": map[string]interface{}{ + "replicas": float64(3), + "template": map[string]interface{}{ + "spec": map[string]interface{}{ + "containers": []interface{}{ + map[string]interface{}{ + "name": string("nginx"), + "image": string("nginx:1.14.2"), + }, }, }, }, }, - }, + } + + // Manifest defines the manifest we're comparing the marshal results below to. + SubstituteManifest = Manifest{ + "apiVersion": string("test/v1"), + "kind": string("Test"), + "metadata": map[string]interface{}{ + "name": string("test"), + }, + "spec": map[string]interface{}{ + "integer": float64(3), + "string": string("${TEST_ENV}"), + }, + } + + // SubstituteManifestString defines the expected Marshal result when using String(forceQuotedStrings=true). + SubstituteManifestString = `apiVersion: test/v1 +kind: Test +metadata: + name: test +spec: + integer: 3 + string: ${TEST_ENV} +` + + // SubstituteManifestQuotedString defines the expected Marshal result when using String(forceQuotedStrings=falsetrue). + SubstituteManifestQuotedString = `"apiVersion": "test/v1" +"kind": "Test" +"metadata": + "name": "test" +"spec": + "integer": 3 + "string": "${TEST_ENV}" +` +) + +func TestMarshalString(t *testing.T) { + m := SubstituteManifest.String(false) + + if s := cmp.Diff(SubstituteManifestString, m); s != "" { + t.Error(s) + } +} + +func TestMarshalQuotedString(t *testing.T) { + m := SubstituteManifest.String(true) + + if s := cmp.Diff(SubstituteManifestQuotedString, m); s != "" { + t.Error(s) + } } func TestUnmarshalJSON(t *testing.T) {