From c2d73ca48914b01936b87e1ce6d37d777719089e Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Thu, 3 Aug 2017 17:02:55 +0200 Subject: [PATCH] Escape '/' in JSON Patch path correctly --- kubernetes/patch_operations.go | 16 +++++- kubernetes/patch_operations_test.go | 42 ++++++++++++++ .../resource_kubernetes_namespace_test.go | 57 +++++++++++++++++++ 3 files changed, 112 insertions(+), 3 deletions(-) diff --git a/kubernetes/patch_operations.go b/kubernetes/patch_operations.go index e794a1324e..ea552105fe 100644 --- a/kubernetes/patch_operations.go +++ b/kubernetes/patch_operations.go @@ -21,7 +21,9 @@ func diffStringMap(pathPrefix string, oldV, newV map[string]interface{}) PatchOp if _, ok := newV[k]; ok { continue } - ops = append(ops, &RemoveOperation{Path: pathPrefix + "/" + k}) + ops = append(ops, &RemoveOperation{ + Path: pathPrefix + "/" + escapeJsonPointer(k), + }) } for k, v := range newV { @@ -33,14 +35,14 @@ func diffStringMap(pathPrefix string, oldV, newV map[string]interface{}) PatchOp } ops = append(ops, &ReplaceOperation{ - Path: pathPrefix + "/" + k, + Path: pathPrefix + "/" + escapeJsonPointer(k), Value: newValue, }) continue } ops = append(ops, &AddOperation{ - Path: pathPrefix + "/" + k, + Path: pathPrefix + "/" + escapeJsonPointer(k), Value: newValue, }) } @@ -48,6 +50,14 @@ func diffStringMap(pathPrefix string, oldV, newV map[string]interface{}) PatchOp return ops } +// escapeJsonPointer escapes string per RFC 6901 +// so it can be used as path in JSON patch operations +func escapeJsonPointer(path string) string { + path = strings.Replace(path, "~", "~0", -1) + path = strings.Replace(path, "/", "~1", -1) + return path +} + type PatchOperations []PatchOperation func (po PatchOperations) MarshalJSON() ([]byte, error) { diff --git a/kubernetes/patch_operations_test.go b/kubernetes/patch_operations_test.go index c60a5e628c..523579ac01 100644 --- a/kubernetes/patch_operations_test.go +++ b/kubernetes/patch_operations_test.go @@ -112,6 +112,30 @@ func TestDiffStringMap(t *testing.T) { }, }, }, + { + Path: "/parent/", + Old: map[string]interface{}{ + "two~with-tilde": "220", + "three/with/three/slashes": "330", + }, + New: map[string]interface{}{ + "one/with-slash": "111", + "three/with/three/slashes": "333", + }, + ExpectedOps: []PatchOperation{ + &AddOperation{ + Path: "/parent/one~1with-slash", + Value: "111", + }, + &RemoveOperation{ + Path: "/parent/two~0with-tilde", + }, + &ReplaceOperation{ + Path: "/parent/three~1with~1three~1slashes", + Value: "333", + }, + }, + }, } for i, tc := range testCases { @@ -122,5 +146,23 @@ func TestDiffStringMap(t *testing.T) { } }) } +} +func TestEscapeJsonPointer(t *testing.T) { + testCases := []struct { + Input string + ExpectedOutput string + }{ + {"simple", "simple"}, + {"special-chars,but no escaping", "special-chars,but no escaping"}, + {"escape-this/forward-slash", "escape-this~1forward-slash"}, + {"escape-this~tilde", "escape-this~0tilde"}, + } + for _, tc := range testCases { + output := escapeJsonPointer(tc.Input) + if output != tc.ExpectedOutput { + t.Fatalf("Expected %q as after escaping %q, given: %q", + tc.ExpectedOutput, tc.Input, output) + } + } } diff --git a/kubernetes/resource_kubernetes_namespace_test.go b/kubernetes/resource_kubernetes_namespace_test.go index 8092b48643..e4ed78d451 100644 --- a/kubernetes/resource_kubernetes_namespace_test.go +++ b/kubernetes/resource_kubernetes_namespace_test.go @@ -134,6 +134,45 @@ func TestAccKubernetesNamespace_generatedName(t *testing.T) { }) } +func TestAccKubernetesNamespace_withSpecialCharacters(t *testing.T) { + var conf api.Namespace + nsName := fmt.Sprintf("tf-acc-test-%s", acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + IDRefreshName: "kubernetes_namespace.test", + Providers: testAccProviders, + CheckDestroy: testAccCheckKubernetesNamespaceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccKubernetesNamespaceConfig_specialCharacters(nsName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckKubernetesNamespaceExists("kubernetes_namespace.test", &conf), + resource.TestCheckResourceAttr("kubernetes_namespace.test", "metadata.0.annotations.%", "2"), + testAccCheckMetaAnnotations(&conf.ObjectMeta, map[string]string{ + "myhost.co.uk/any-path": "one", + "Different": "1234", + }), + resource.TestCheckResourceAttr("kubernetes_namespace.test", "metadata.0.annotations.myhost.co.uk/any-path", "one"), + resource.TestCheckResourceAttr("kubernetes_namespace.test", "metadata.0.annotations.Different", "1234"), + resource.TestCheckResourceAttr("kubernetes_namespace.test", "metadata.0.labels.%", "2"), + testAccCheckMetaLabels(&conf.ObjectMeta, map[string]string{ + "myhost.co.uk/any-path": "one", + "TestLabelThree": "three", + }), + resource.TestCheckResourceAttr("kubernetes_namespace.test", "metadata.0.labels.myhost.co.uk/any-path", "one"), + resource.TestCheckResourceAttr("kubernetes_namespace.test", "metadata.0.labels.TestLabelThree", "three"), + resource.TestCheckResourceAttr("kubernetes_namespace.test", "metadata.0.name", nsName), + resource.TestCheckResourceAttrSet("kubernetes_namespace.test", "metadata.0.generation"), + resource.TestCheckResourceAttrSet("kubernetes_namespace.test", "metadata.0.resource_version"), + resource.TestCheckResourceAttrSet("kubernetes_namespace.test", "metadata.0.self_link"), + resource.TestCheckResourceAttrSet("kubernetes_namespace.test", "metadata.0.uid"), + ), + }, + }, + }) +} + func TestAccKubernetesNamespace_importGeneratedName(t *testing.T) { resourceName := "kubernetes_namespace.test" prefix := "tf-acc-test-gen-import-" @@ -271,3 +310,21 @@ resource "kubernetes_namespace" "test" { } }`, prefix) } + +func testAccKubernetesNamespaceConfig_specialCharacters(nsName string) string { + return fmt.Sprintf(` +resource "kubernetes_namespace" "test" { + metadata { + annotations { + "myhost.co.uk/any-path" = "one" + "Different" = "1234" + } + labels { + "myhost.co.uk/any-path" = "one" + "TestLabelThree" = "three" + } + + name = "%s" + } +}`, nsName) +}