Merge pull request #40 from terraform-providers/b-escape-json-ptr

Escape '/' in JSON Patch path correctly
hashicorp · Aug 4, 2017 · 98bee7c · 98bee7c
2 parents 6b4d17d + c2d73ca
commit 98bee7c
Show file tree

Hide file tree

Showing 3 changed files with 112 additions and 3 deletions.
diff --git 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,21 +35,29 @@ 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,
 		})
 	}
 
 	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
@@ -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
@@ -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)
+}