Fix perpetual diff in claim_ref (#1227)

The claim_ref attribute would perpetually show a diff if the PV was created without a claim_ref specified.
This is because all PVs gain a claimRef once a PVC binds to it. (This creates the bi-directional link between PV and PVC,
and it's done on the Kubernetes API side).

To support this, `claim_ref` is now a Computed attribute, and it will update to show the claimRef once a PVC has bound to the PV, (only now it will do so without creating a diff in terraform).

Also added extra tests to catch this case and moved claim_ref's flatteners/expanders/tests into the correct files.

Author: dak1n1

kubernetes/resource_kubernetes_persistent_volume.go

@@ -118,14 +118,16 @@ func resourceKubernetesPersistentVolume() *schema.Resource {
 							Type:        schema.TypeList,
 							Description: "A reference to the persistent volume claim details for statically managed PVs. More Info: https://kubernetes.io/docs/concepts/storage/persistent-volumes/#binding",
 							Optional:    true,
+							Computed:    true,
 							MaxItems:    1,
 							Elem: &schema.Resource{
 								Schema: map[string]*schema.Schema{
 									"namespace": {
 										Type:        schema.TypeString,
-										Description: "The namespace of the PersistentVolumeClaim",
+										Description: "The namespace of the PersistentVolumeClaim. Uses 'default' namespace if none is specified.",
 										Elem:        schema.TypeString,
-										Required:    true,
+										Optional:    true,
+										Default:     "default",
 									},
 									"name": {
 										Type:        schema.TypeString,

kubernetes/resource_kubernetes_persistent_volume_claim_test.go

@@ -716,11 +716,6 @@ resource "kubernetes_persistent_volume" "test" {
       }
     }
   }
-  lifecycle {
-    ignore_changes = [
-      spec[0].claim_ref,
-    ]
-  }
 }
 resource "kubernetes_persistent_volume_claim" "test" {
   wait_until_bound = true

kubernetes/resource_kubernetes_persistent_volume_test.go

@@ -16,6 +16,29 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
+func TestAccKubernetesPersistentVolume_minimal(t *testing.T) {
+	var conf api.PersistentVolume
+	randString := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
+	name := fmt.Sprintf("tf-acc-test-%s", randString)
+
+	const resourceName = "kubernetes_persistent_volume.test"
+	resource.Test(t, resource.TestCase{
+		PreCheck:          func() { testAccPreCheck(t) },
+		IDRefreshName:     resourceName,
+		ProviderFactories: testAccProviderFactories,
+		CheckDestroy:      testAccCheckKubernetesPersistentVolumeDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccKubernetesPersistentVolumeConfig_hostPath_basic(name),
+				Check: resource.ComposeAggregateTestCheckFunc(
+					testAccCheckKubernetesPersistentVolumeExists(resourceName, &conf),
+					resource.TestCheckResourceAttr(resourceName, "spec.0.claim_ref.#", "0"),
+				),
+			},
+		},
+	})
+}
+
 func TestAccKubernetesPersistentVolume_azure_basic(t *testing.T) {
 	var conf api.PersistentVolume
 	randString := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
@@ -923,12 +946,11 @@ func TestAccKubernetesPersistentVolume_regression(t *testing.T) {
 	})
 }
 
-func TestAccKubernetesPersistentVolume_hostPath_claimRef(t *testing.T) {
-	var conf api.PersistentVolume
+func TestAccKubernetesPersistentVolume_hostpath_claimRef(t *testing.T) {
+	var conf1, conf2 api.PersistentVolume
+	var conf3 api.PersistentVolumeClaim
 	randString := acctest.RandStringFromCharSet(10, acctest.CharSetAlphaNum)
 	name := fmt.Sprintf("tf-acc-test-%s", randString)
-	claimNamespace := "default"
-	claimName := "expected-claim-name"
 
 	const resourceName = "kubernetes_persistent_volume.test"
 	resource.Test(t, resource.TestCase{
@@ -937,30 +959,32 @@ func TestAccKubernetesPersistentVolume_hostPath_claimRef(t *testing.T) {
 		ProviderFactories: testAccProviderFactories,
 		CheckDestroy:      testAccCheckKubernetesPersistentVolumeDestroy,
 		Steps: []resource.TestStep{
-			// create a volume without a claimRef
 			{
-				Config: testAccKubernetesPersistentVolumeConfig_hostPath_basic(name),
+				Config: testAccKubernetesPersistentVolumeConfig_hostPath_claimRef_noNamespace(name),
 				Check: resource.ComposeAggregateTestCheckFunc(
-					testAccCheckKubernetesPersistentVolumeExists(resourceName, &conf),
-					resource.TestCheckResourceAttr(resourceName, "spec.0.claim_ref.#", "0"),
+					testAccCheckKubernetesPersistentVolumeExists(resourceName, &conf1),
+					resource.TestCheckResourceAttr(resourceName, "spec.0.claim_ref.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "spec.0.claim_ref.0.name", name),
+					resource.TestCheckResourceAttr(resourceName, "spec.0.claim_ref.0.namespace", "default"),
 				),
 			},
-			// set the claimRef and assert it's present
 			{
-				Config: testAccKubernetesPersistentVolumeConfig_hostPath_claimRef(name, claimNamespace, claimName),
+				Config: testAccKubernetesPersistentVolumeConfig_hostPath_claimRef_withNamespace(name),
 				Check: resource.ComposeAggregateTestCheckFunc(
-					testAccCheckKubernetesPersistentVolumeExists(resourceName, &conf),
-					resource.TestCheckResourceAttr(resourceName, "spec.0.claim_ref.#", "1"),
-					resource.TestCheckResourceAttr(resourceName, "spec.0.claim_ref.0.namespace", claimNamespace),
-					resource.TestCheckResourceAttr(resourceName, "spec.0.claim_ref.0.name", claimName),
+					testAccCheckKubernetesPersistentVolumeExists(resourceName, &conf2),
+					resource.TestCheckResourceAttr(resourceName, "spec.0.claim_ref.0.namespace", name),
+					resource.TestCheckResourceAttr(resourceName, "spec.0.claim_ref.0.name", name),
+					testAccCheckKubernetesPersistentVolumeForceNew(&conf1, &conf2, false),
 				),
 			},
-			// unset the claimRef and assert it's absent
 			{
-				Config: testAccKubernetesPersistentVolumeConfig_hostPath_basic(name),
+				Config: testAccKubernetesPersistentVolumeConfig_hostPath_claimRef_withPVC(name),
 				Check: resource.ComposeAggregateTestCheckFunc(
-					testAccCheckKubernetesPersistentVolumeExists(resourceName, &conf),
-					resource.TestCheckResourceAttr(resourceName, "spec.0.claim_ref.#", "0"),
+					testAccCheckKubernetesPersistentVolumeExists(resourceName, &conf2),
+					testAccCheckKubernetesPersistentVolumeClaimExists("kubernetes_persistent_volume_claim.test", &conf3),
+					resource.TestCheckResourceAttr(resourceName, "spec.0.claim_ref.0.namespace", name),
+					resource.TestCheckResourceAttr(resourceName, "spec.0.claim_ref.0.name", name),
+					testAccCheckKubernetesPersistentVolumeForceNew(&conf1, &conf2, false),
 				),
 			},
 		},
@@ -1920,11 +1944,71 @@ func testAccKubernetesPersistentVolumeConfig_hostPath_basic(name string) string
 }`, name)
 }
 
-func testAccKubernetesPersistentVolumeConfig_hostPath_claimRef(name string, claimNamespace string, claimName string) string {
+func testAccKubernetesPersistentVolumeConfig_hostPath_claimRef_noNamespace(name string) string {
 	return fmt.Sprintf(`resource "kubernetes_persistent_volume" "test" {
   metadata {
     name = "%s"
   }
+  spec {
+    capacity = {
+      storage = "1Gi"
+    }
+    access_modes = ["ReadWriteMany"]
+    mount_options = ["foo"]
+    claim_ref {
+       name = "%s"
+    }
+
+    persistent_volume_source {
+      host_path {
+        path = "/mnt/local-volume"
+      }
+    }
+  }
+}`, name, name)
+}
+
+func testAccKubernetesPersistentVolumeConfig_hostPath_claimRef_withNamespace(name string) string {
+	return fmt.Sprintf(`resource "kubernetes_namespace" "test" {
+  metadata {
+    name = "%s"
+  }
+}
+resource "kubernetes_persistent_volume" "test" {
+  metadata {
+    name = "%s"
+  }
+  spec {
+    capacity = {
+      storage = "1Gi"
+    }
+    access_modes = ["ReadWriteMany"]
+    mount_options = ["foo"]
+    claim_ref {
+       name = "%s"
+       namespace = kubernetes_namespace.test.metadata.0.name
+    }
+
+    persistent_volume_source {
+      host_path {
+        path = "/mnt/local-volume"
+      }
+    }
+  }
+}`, name, name, name)
+}
+
+func testAccKubernetesPersistentVolumeConfig_hostPath_claimRef_withPVC(name string) string {
+	return fmt.Sprintf(`resource "kubernetes_namespace" "test" {
+  metadata {
+    name = "%s"
+  }
+}
+
+resource "kubernetes_persistent_volume" "test" {
+  metadata {
+    name = "%s"
+  }
   spec {
     capacity = {
       storage = "1Gi"
@@ -1942,7 +2026,27 @@ func testAccKubernetesPersistentVolumeConfig_hostPath_claimRef(name string, clai
       }
     }
   }
-}`, name, claimName, claimNamespace)
+}
+
+resource "kubernetes_persistent_volume_claim" "test" {
+  metadata {
+    name = "%s"
+    namespace = "%s"
+  }
+
+  spec {
+    access_modes       = ["ReadWriteOnce"]
+
+    resources {
+      requests = {
+        storage = "1Gi"
+      }
+    }
+
+    volume_name = kubernetes_persistent_volume.test.metadata.0.name
+  }
+}
+`, name, name, name, name, name, name)
 }
 
 func testAccKubernetesPersistentVolume_regression(provider, name, path, typ string) string {

kubernetes/structure_persistent_volume_spec.go

@@ -403,6 +403,17 @@ func flattenPersistentVolumeSpec(in v1.PersistentVolumeSpec) []interface{} {
 	return []interface{}{att}
 }
 
+func flattenObjectRef(in *v1.ObjectReference) []interface{} {
+	att := make(map[string]interface{})
+	if in.Name != "" {
+		att["name"] = in.Name
+	}
+	if in.Namespace != "" {
+		att["namespace"] = in.Namespace
+	}
+	return []interface{}{att}
+}
+
 func flattenPhotonPersistentDiskVolumeSource(in *v1.PhotonPersistentDiskVolumeSource) []interface{} {
 	att := make(map[string]interface{})
 	att["pd_id"] = in.PdID
@@ -1025,11 +1036,11 @@ func expandPersistentVolumeSpec(l []interface{}) (*v1.PersistentVolumeSpec, erro
 	return obj, nil
 }
 
-func expandClaimRef(v []interface{}) *v1.ObjectReference {
-	if len(v) == 0 || v[0] == nil {
-		return nil
+func expandClaimRef(l []interface{}) *v1.ObjectReference {
+	if len(l) == 0 || l[0] == nil {
+		return &v1.ObjectReference{}
 	}
-	o := v[0].(map[string]interface{})
+	o := l[0].(map[string]interface{})
 	return &v1.ObjectReference{
 		Name:      o["name"].(string),
 		Namespace: o["namespace"].(string),

kubernetes/structure_persistent_volume_spec_test.go

@@ -0,0 +1,40 @@
+package kubernetes
+
+import (
+	"reflect"
+	"testing"
+
+	v1 "k8s.io/api/core/v1"
+)
+
+func TestFlattenObjectRef(t *testing.T) {
+	cases := []struct {
+		Input          *v1.ObjectReference
+		ExpectedOutput []interface{}
+	}{
+		{
+			&v1.ObjectReference{
+				Name:      "demo",
+				Namespace: "default",
+			},
+			[]interface{}{
+				map[string]interface{}{
+					"name":      "demo",
+					"namespace": "default",
+				},
+			},
+		},
+		{
+			&v1.ObjectReference{},
+			[]interface{}{map[string]interface{}{}},
+		},
+	}
+
+	for _, tc := range cases {
+		output := flattenObjectRef(tc.Input)
+		if !reflect.DeepEqual(output, tc.ExpectedOutput) {
+			t.Fatalf("Unexpected output from flattener.\nExpected: %#v\nGiven:    %#v",
+				tc.ExpectedOutput, output)
+		}
+	}
+}

kubernetes/structures_container.go

@@ -1028,14 +1028,3 @@ func expandContainerResourceRequirements(l []interface{}) (*v1.ResourceRequireme
 
 	return obj, nil
 }
-
-func flattenObjectRef(in *v1.ObjectReference) []interface{} {
-	att := make(map[string]interface{})
-	if in.Name != "" {
-		att["name"] = in.Name
-	}
-	if in.Namespace != "" {
-		att["namespace"] = in.Namespace
-	}
-	return []interface{}{att}
-}

kubernetes/structures_container_test.go

@@ -212,35 +212,3 @@ func TestExpandConfigMapKeyRef(t *testing.T) {
 		}
 	}
 }
-
-func TestFlattenObjectRef(t *testing.T) {
-	cases := []struct {
-		Input          *v1.ObjectReference
-		ExpectedOutput []interface{}
-	}{
-		{
-			&v1.ObjectReference{
-				Name:      "demo",
-				Namespace: "default",
-			},
-			[]interface{}{
-				map[string]interface{}{
-					"name":      "demo",
-					"namespace": "default",
-				},
-			},
-		},
-		{
-			&v1.ObjectReference{},
-			[]interface{}{map[string]interface{}{}},
-		},
-	}
-
-	for _, tc := range cases {
-		output := flattenObjectRef(tc.Input)
-		if !reflect.DeepEqual(output, tc.ExpectedOutput) {
-			t.Fatalf("Unexpected output from flattener.\nExpected: %#v\nGiven:    %#v",
-				tc.ExpectedOutput, output)
-		}
-	}
-}