Merge pull request #9084 from rifelpet/gce-tf-metadata

Dont use terraform's file() for singleline strings in GCE metadata
kubernetes · May 8, 2020 · 8768178 · 8768178
2 parents 890ddcf + ef6abbc
commit 8768178
Show file tree

Hide file tree

Showing 18 changed files with 29 additions and 41 deletions.
diff --git a/cmd/kops/integration_test.go b/cmd/kops/integration_test.go
@@ -589,20 +589,16 @@ func (i *integrationTest) runTestTerraformGCE(t *testing.T) {
 	h.SetupMockGCE()
 
 	expectedFilenames := []string{
-		"google_compute_instance_template_nodes-" + gce.SafeClusterName(i.clusterName) + "_metadata_cluster-name",
 		"google_compute_instance_template_nodes-" + gce.SafeClusterName(i.clusterName) + "_metadata_startup-script",
 		"google_compute_instance_template_nodes-" + gce.SafeClusterName(i.clusterName) + "_metadata_ssh-keys",
-		"google_compute_instance_template_nodes-" + gce.SafeClusterName(i.clusterName) + "_metadata_kops-k8s-io-instance-group-name",
 	}
 
 	for j := 0; j < i.zones; j++ {
 		zone := "us-test1-" + string([]byte{byte('a') + byte(j)})
 		prefix := "google_compute_instance_template_master-" + zone + "-" + gce.SafeClusterName(i.clusterName) + "_metadata_"
 
-		expectedFilenames = append(expectedFilenames, prefix+"cluster-name")
 		expectedFilenames = append(expectedFilenames, prefix+"startup-script")
 		expectedFilenames = append(expectedFilenames, prefix+"ssh-keys")
-		expectedFilenames = append(expectedFilenames, prefix+"kops-k8s-io-instance-group-name")
 	}
 
 	i.runTest(t, h, expectedFilenames, "", "", nil)

diff --git a/...ogle_compute_instance_template_master-us-test1-a-ha-gce-example-com_metadata_cluster-name b/...ogle_compute_instance_template_master-us-test1-a-ha-gce-example-com_metadata_cluster-name
diff --git a/...ce_template_master-us-test1-a-ha-gce-example-com_metadata_kops-k8s-io-instance-group-name b/...ce_template_master-us-test1-a-ha-gce-example-com_metadata_kops-k8s-io-instance-group-name
diff --git a/...ogle_compute_instance_template_master-us-test1-b-ha-gce-example-com_metadata_cluster-name b/...ogle_compute_instance_template_master-us-test1-b-ha-gce-example-com_metadata_cluster-name
diff --git a/...ce_template_master-us-test1-b-ha-gce-example-com_metadata_kops-k8s-io-instance-group-name b/...ce_template_master-us-test1-b-ha-gce-example-com_metadata_kops-k8s-io-instance-group-name
diff --git a/...ogle_compute_instance_template_master-us-test1-c-ha-gce-example-com_metadata_cluster-name b/...ogle_compute_instance_template_master-us-test1-c-ha-gce-example-com_metadata_cluster-name
diff --git a/...ce_template_master-us-test1-c-ha-gce-example-com_metadata_kops-k8s-io-instance-group-name b/...ce_template_master-us-test1-c-ha-gce-example-com_metadata_kops-k8s-io-instance-group-name
diff --git a/..._gce/data/google_compute_instance_template_nodes-ha-gce-example-com_metadata_cluster-name b/..._gce/data/google_compute_instance_template_nodes-ha-gce-example-com_metadata_cluster-name
diff --git a/...mpute_instance_template_nodes-ha-gce-example-com_metadata_kops-k8s-io-instance-group-name b/...mpute_instance_template_nodes-ha-gce-example-com_metadata_kops-k8s-io-instance-group-name
diff --git a/tests/integration/update_cluster/ha_gce/kubernetes.tf b/tests/integration/update_cluster/ha_gce/kubernetes.tf
@@ -348,8 +348,8 @@ resource "google_compute_instance_template" "master-us-test1-a-ha-gce-example-co
   }
   machine_type = "n1-standard-1"
   metadata = {
-    "cluster-name"                    = file("${path.module}/data/google_compute_instance_template_master-us-test1-a-ha-gce-example-com_metadata_cluster-name")
-    "kops-k8s-io-instance-group-name" = file("${path.module}/data/google_compute_instance_template_master-us-test1-a-ha-gce-example-com_metadata_kops-k8s-io-instance-group-name")
+    "cluster-name"                    = "ha-gce.example.com"
+    "kops-k8s-io-instance-group-name" = "master-us-test1-a"
     "ssh-keys"                        = file("${path.module}/data/google_compute_instance_template_master-us-test1-a-ha-gce-example-com_metadata_ssh-keys")
     "startup-script"                  = file("${path.module}/data/google_compute_instance_template_master-us-test1-a-ha-gce-example-com_metadata_startup-script")
   }
@@ -388,8 +388,8 @@ resource "google_compute_instance_template" "master-us-test1-b-ha-gce-example-co
   }
   machine_type = "n1-standard-1"
   metadata = {
-    "cluster-name"                    = file("${path.module}/data/google_compute_instance_template_master-us-test1-b-ha-gce-example-com_metadata_cluster-name")
-    "kops-k8s-io-instance-group-name" = file("${path.module}/data/google_compute_instance_template_master-us-test1-b-ha-gce-example-com_metadata_kops-k8s-io-instance-group-name")
+    "cluster-name"                    = "ha-gce.example.com"
+    "kops-k8s-io-instance-group-name" = "master-us-test1-b"
     "ssh-keys"                        = file("${path.module}/data/google_compute_instance_template_master-us-test1-b-ha-gce-example-com_metadata_ssh-keys")
     "startup-script"                  = file("${path.module}/data/google_compute_instance_template_master-us-test1-b-ha-gce-example-com_metadata_startup-script")
   }
@@ -428,8 +428,8 @@ resource "google_compute_instance_template" "master-us-test1-c-ha-gce-example-co
   }
   machine_type = "n1-standard-1"
   metadata = {
-    "cluster-name"                    = file("${path.module}/data/google_compute_instance_template_master-us-test1-c-ha-gce-example-com_metadata_cluster-name")
-    "kops-k8s-io-instance-group-name" = file("${path.module}/data/google_compute_instance_template_master-us-test1-c-ha-gce-example-com_metadata_kops-k8s-io-instance-group-name")
+    "cluster-name"                    = "ha-gce.example.com"
+    "kops-k8s-io-instance-group-name" = "master-us-test1-c"
     "ssh-keys"                        = file("${path.module}/data/google_compute_instance_template_master-us-test1-c-ha-gce-example-com_metadata_ssh-keys")
     "startup-script"                  = file("${path.module}/data/google_compute_instance_template_master-us-test1-c-ha-gce-example-com_metadata_startup-script")
   }
@@ -468,8 +468,8 @@ resource "google_compute_instance_template" "nodes-ha-gce-example-com" {
   }
   machine_type = "n1-standard-2"
   metadata = {
-    "cluster-name"                    = file("${path.module}/data/google_compute_instance_template_nodes-ha-gce-example-com_metadata_cluster-name")
-    "kops-k8s-io-instance-group-name" = file("${path.module}/data/google_compute_instance_template_nodes-ha-gce-example-com_metadata_kops-k8s-io-instance-group-name")
+    "cluster-name"                    = "ha-gce.example.com"
+    "kops-k8s-io-instance-group-name" = "nodes"
     "ssh-keys"                        = file("${path.module}/data/google_compute_instance_template_nodes-ha-gce-example-com_metadata_ssh-keys")
     "startup-script"                  = file("${path.module}/data/google_compute_instance_template_nodes-ha-gce-example-com_metadata_startup-script")
   }

diff --git a/...compute_instance_template_master-us-test1-a-minimal-gce-example-com_metadata_cluster-name b/...compute_instance_template_master-us-test1-a-minimal-gce-example-com_metadata_cluster-name
diff --git a/...mplate_master-us-test1-a-minimal-gce-example-com_metadata_kops-k8s-io-instance-group-name b/...mplate_master-us-test1-a-minimal-gce-example-com_metadata_kops-k8s-io-instance-group-name
diff --git a/...data/google_compute_instance_template_nodes-minimal-gce-example-com_metadata_cluster-name b/...data/google_compute_instance_template_nodes-minimal-gce-example-com_metadata_cluster-name
diff --git a/..._instance_template_nodes-minimal-gce-example-com_metadata_kops-k8s-io-instance-group-name b/..._instance_template_nodes-minimal-gce-example-com_metadata_kops-k8s-io-instance-group-name
diff --git a/tests/integration/update_cluster/minimal_gce/kubernetes.tf b/tests/integration/update_cluster/minimal_gce/kubernetes.tf
@@ -260,8 +260,8 @@ resource "google_compute_instance_template" "master-us-test1-a-minimal-gce-examp
   }
   machine_type = "n1-standard-1"
   metadata = {
-    "cluster-name"                    = file("${path.module}/data/google_compute_instance_template_master-us-test1-a-minimal-gce-example-com_metadata_cluster-name")
-    "kops-k8s-io-instance-group-name" = file("${path.module}/data/google_compute_instance_template_master-us-test1-a-minimal-gce-example-com_metadata_kops-k8s-io-instance-group-name")
+    "cluster-name"                    = "minimal-gce.example.com"
+    "kops-k8s-io-instance-group-name" = "master-us-test1-a"
     "ssh-keys"                        = file("${path.module}/data/google_compute_instance_template_master-us-test1-a-minimal-gce-example-com_metadata_ssh-keys")
     "startup-script"                  = file("${path.module}/data/google_compute_instance_template_master-us-test1-a-minimal-gce-example-com_metadata_startup-script")
   }
@@ -300,8 +300,8 @@ resource "google_compute_instance_template" "nodes-minimal-gce-example-com" {
   }
   machine_type = "n1-standard-2"
   metadata = {
-    "cluster-name"                    = file("${path.module}/data/google_compute_instance_template_nodes-minimal-gce-example-com_metadata_cluster-name")
-    "kops-k8s-io-instance-group-name" = file("${path.module}/data/google_compute_instance_template_nodes-minimal-gce-example-com_metadata_kops-k8s-io-instance-group-name")
+    "cluster-name"                    = "minimal-gce.example.com"
+    "kops-k8s-io-instance-group-name" = "nodes"
     "ssh-keys"                        = file("${path.module}/data/google_compute_instance_template_nodes-minimal-gce-example-com_metadata_ssh-keys")
     "startup-script"                  = file("${path.module}/data/google_compute_instance_template_nodes-minimal-gce-example-com_metadata_startup-script")
   }

diff --git a/upup/pkg/fi/cloudup/gcetasks/instancetemplate.go b/upup/pkg/fi/cloudup/gcetasks/instancetemplate.go
@@ -479,9 +479,7 @@ func addNetworks(network *Network, subnet *Subnet, networkInterfaces []*compute.
 		for _, gac := range g.AccessConfigs {
 			tac := &terraformAccessConfig{}
 			natIP := gac.NatIP
-			if strings.HasPrefix(natIP, "${") {
-				tac.NatIP = terraform.LiteralExpression(natIP)
-			} else if natIP != "" {
+			if natIP != "" {
 				tac.NatIP = terraform.LiteralFromStringValue(natIP)
 			}
 
@@ -499,13 +497,17 @@ func addMetadata(target *terraform.TerraformTarget, name string, metadata *compu
 	}
 	m := make(map[string]*terraform.Literal)
 	for _, g := range metadata.Items {
-		v := fi.NewStringResource(fi.StringValue(g.Value))
-		tfResource, err := target.AddFile("google_compute_instance_template", name, "metadata_"+g.Key, v)
-		if err != nil {
-			return nil, err
+		val := fi.StringValue(g.Value)
+		if strings.Contains(val, "\n") {
+			v := fi.NewStringResource(val)
+			tfResource, err := target.AddFile("google_compute_instance_template", name, "metadata_"+g.Key, v)
+			if err != nil {
+				return nil, err
+			}
+			m[g.Key] = tfResource
+		} else {
+			m[g.Key] = terraform.LiteralFromStringValue(val)
 		}
-
-		m[g.Key] = tfResource
 	}
 	return m, nil
 }

diff --git a/upup/pkg/fi/cloudup/terraform/hcl2.go b/upup/pkg/fi/cloudup/terraform/hcl2.go
@@ -194,6 +194,12 @@ func writeMap(body *hclwrite.Body, key string, values map[string]cty.Value) {
 			// If we ever need to support a map of strings to resource property references that can be added here
 			if literal.FilePath != "" {
 				tokens = append(tokens, &hclwrite.Token{Type: hclsyntax.TokenIdent, Bytes: []byte(fmt.Sprintf("file(%q)", literal.FilePath))})
+			} else if literal.Value != "" {
+				tokens = append(tokens, []*hclwrite.Token{
+					{Type: hclsyntax.TokenOQuote, Bytes: []byte{'"'}, SpacesBefore: 1},
+					{Type: hclsyntax.TokenQuotedLit, Bytes: []byte(literal.Value)},
+					{Type: hclsyntax.TokenOQuote, Bytes: []byte{'"'}, SpacesBefore: 1},
+				}...)
 			}
 		} else {
 			tokens = append(tokens, []*hclwrite.Token{

diff --git a/upup/pkg/fi/cloudup/terraform/literal.go b/upup/pkg/fi/cloudup/terraform/literal.go
@@ -47,10 +47,6 @@ func (l *Literal) MarshalJSON() ([]byte, error) {
 	return json.Marshal(&l.Value)
 }
 
-func LiteralExpression(s string) *Literal {
-	return &Literal{Value: s}
-}
-
 func LiteralFileExpression(modulePath string) *Literal {
 	return &Literal{
 		Value:    fmt.Sprintf("${file(%q)}", modulePath),