fix: Parsing dynamic values for agent results in error (#564)

The logic required a constant value before, which disallowed dynamic value injection into the agent. This isn't an accurate limitation, so inverting the logic resolves it.
coder · Mar 25, 2022 · 21fdb80 · 21fdb80
1 parent a06821c
commit 21fdb80
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 37 deletions.
diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go
@@ -285,25 +285,21 @@ func parseTerraformPlan(ctx context.Context, terraform *tfexec.Terraform, planfi
 		}
 		if envRaw, has := resource.Expressions["env"]; has {
 			env, ok := envRaw.ConstantValue.(map[string]string)
-			if !ok {
-				return nil, xerrors.Errorf("unexpected type %T for env map", envRaw.ConstantValue)
+			if ok {
+				agent.Env = env
 			}
-			agent.Env = env
 		}
 		if startupScriptRaw, has := resource.Expressions["startup_script"]; has {
 			startupScript, ok := startupScriptRaw.ConstantValue.(string)
-			if !ok {
-				return nil, xerrors.Errorf("unexpected type %T for startup script", startupScriptRaw.ConstantValue)
+			if ok {
+				agent.StartupScript = startupScript
 			}
-			agent.StartupScript = startupScript
 		}
-		if instanceIDRaw, has := resource.Expressions["instance_id"]; has {
-			instanceID, ok := instanceIDRaw.ConstantValue.(string)
-			if !ok {
-				return nil, xerrors.Errorf("unexpected type %T for instance_id", instanceIDRaw.ConstantValue)
-			}
+		if _, has := resource.Expressions["instance_id"]; has {
+			// This is a dynamic value. If it's expressed, we know
+			// it's at least an instance ID, which is better than nothing.
 			agent.Auth = &proto.Agent_InstanceId{
-				InstanceId: instanceID,
+				InstanceId: "",
 			}
 		}
 
@@ -429,15 +425,6 @@ func parseTerraformApply(ctx context.Context, terraform *tfexec.Terraform, state
 				}
 			}
 
-			if agent != nil && agent.GetInstanceId() != "" {
-				// Make sure the instance has an instance ID!
-				_, exists := resource.AttributeValues["instance_id"]
-				if !exists {
-					// This was a mistake!
-					agent = nil
-				}
-			}
-
 			resources = append(resources, &proto.Resource{
 				Name:  resource.Name,
 				Type:  resource.Type,

diff --git a/provisioner/terraform/provision_test.go b/provisioner/terraform/provision_test.go
@@ -166,9 +166,7 @@ provider "coder" {
 		Request: &proto.Provision_Request{
 			Type: &proto.Provision_Request_Start{
 				Start: &proto.Provision_Start{
-					Metadata: &proto.Provision_Metadata{
-						CoderUrl: "https://example.com",
-					},
+					Metadata: &proto.Provision_Metadata{},
 				},
 			},
 		},
@@ -195,16 +193,14 @@ provider "coder" {
 				depends_on = [
 					null_resource.A
 				]
-				instance_id = "an-instance"
+				instance_id = "example"
 			}
 			resource "null_resource" "A" {}`,
 		},
 		Request: &proto.Provision_Request{
 			Type: &proto.Provision_Request_Start{
 				Start: &proto.Provision_Start{
-					Metadata: &proto.Provision_Metadata{
-						CoderUrl: "https://example.com",
-					},
+					Metadata: &proto.Provision_Metadata{},
 				},
 			},
 		},
@@ -214,6 +210,11 @@ provider "coder" {
 					Resources: []*proto.Resource{{
 						Name: "A",
 						Type: "null_resource",
+						Agent: &proto.Agent{
+							Auth: &proto.Agent_InstanceId{
+								InstanceId: "example",
+							},
+						},
 					}},
 				},
 			},
@@ -232,10 +233,8 @@ provider "coder" {
 		Request: &proto.Provision_Request{
 			Type: &proto.Provision_Request_Start{
 				Start: &proto.Provision_Start{
-					DryRun: true,
-					Metadata: &proto.Provision_Metadata{
-						CoderUrl: "https://example.com",
-					},
+					DryRun:   true,
+					Metadata: &proto.Provision_Metadata{},
 				},
 			},
 		},
@@ -267,10 +266,43 @@ provider "coder" {
 		Request: &proto.Provision_Request{
 			Type: &proto.Provision_Request_Start{
 				Start: &proto.Provision_Start{
-					DryRun: true,
-					Metadata: &proto.Provision_Metadata{
-						CoderUrl: "https://example.com",
-					},
+					DryRun:   true,
+					Metadata: &proto.Provision_Metadata{},
+				},
+			},
+		},
+		Response: &proto.Provision_Response{
+			Type: &proto.Provision_Response_Complete{
+				Complete: &proto.Provision_Complete{
+					Resources: []*proto.Resource{{
+						Name: "A",
+						Type: "null_resource",
+						Agent: &proto.Agent{
+							Auth: &proto.Agent_InstanceId{
+								InstanceId: "",
+							},
+						},
+					}},
+				},
+			},
+		},
+	}, {
+		Name: "dryrun-agent-associated-with-resource-instance-id",
+		Files: map[string]string{
+			"main.tf": provider + `
+			resource "coder_agent" "A" {
+				count = length(null_resource.A)
+				instance_id = length(null_resource.A)
+			}
+			resource "null_resource" "A" {
+				count = 1
+			}`,
+		},
+		Request: &proto.Provision_Request{
+			Type: &proto.Provision_Request_Start{
+				Start: &proto.Provision_Start{
+					DryRun:   true,
+					Metadata: &proto.Provision_Metadata{},
 				},
 			},
 		},
@@ -282,7 +314,7 @@ provider "coder" {
 						Type: "null_resource",
 						Agent: &proto.Agent{
 							Auth: &proto.Agent_InstanceId{
-								InstanceId: "an-instance",
+								InstanceId: "",
 							},
 						},
 					}},