Support interactive correctly (#778)

* Support interactive correctly

* Add explicit list of resources which need interaction

* Remove Martin

internal/tfimport/importer/google_billing_budget.go

@@ -29,6 +29,11 @@ type BillingBudget struct{}
 
 // ImportID returns the ID of the resource to use in importing.
 func (i *BillingBudget) ImportID(rc terraform.ResourceChange, pcv ConfigMap, interactive bool) (string, error) {
+	// Can't import if not interactive, since budget names are system-generated and will never be in the plan.
+	if !interactive {
+		return "", &InsufficientInfoErr{[]string{"budget"}, "Budget name is system-generated and will never be in the plan"}
+	}
+
 	billingAccountI, err := fromConfigValues("billing_account", rc.Change.After, pcv)
 	if err != nil {
 		return "", err
@@ -42,18 +47,20 @@ func (i *BillingBudget) ImportID(rc terraform.ResourceChange, pcv ConfigMap, int
 		return "", err
 	}
 
-	// Present the choices, if any.
-	prompt := fmt.Sprintf("No existing budgets found in %v", billingAccount)
-	if len(budgets) > 0 {
-		// Format the budgets more nicely.
-		budgetsLines := []string{"Name|Display Name"}
-		for _, budget := range budgets {
-			budgetsLines = append(budgetsLines, fmt.Sprintf("%v|%v", budget.Name, budget.DisplayName))
-		}
+	if len(budgets) <= 0 {
+		// There are no budgets, just return that it doesn't exist so it can be created.
+		return "", &DoesNotExistErr{rc.Address}
+	}
 
-		prompt = fmt.Sprintf("Found the following budgets in billing account %v (copy the full value from the \"Name\" column):\n%s", billingAccount, columnize.SimpleFormat(budgetsLines))
+	// Present the choices, if any.
+	// Format the budgets more nicely.
+	budgetsLines := []string{"Name|Display Name"}
+	for _, budget := range budgets {
+		budgetsLines = append(budgetsLines, fmt.Sprintf("%v|%v", budget.Name, budget.DisplayName))
 	}
 
+	prompt := fmt.Sprintf("Found the following budgets in billing account %v (copy the full value from the \"Name\" column):\n%s", billingAccount, columnize.SimpleFormat(budgetsLines))
+
 	// Get the value from the user
 	budget, err := fromUser(os.Stdin, "budget", prompt)
 	if err != nil {

internal/tfimport/importer/google_resource_manager_lien.go

@@ -30,6 +30,11 @@ type ResourceManagerLien struct{}
 
 // ImportID returns the ID of the resource to use in importing.
 func (i *ResourceManagerLien) ImportID(rc terraform.ResourceChange, pcv ConfigMap, interactive bool) (string, error) {
+	// Can't import if not interactive, since lien names are system-generated and will never be in the plan.
+	if !interactive {
+		return "", &InsufficientInfoErr{[]string{"name"}, "Lien name is system-generated and will never be in the plan"}
+	}
+
 	parentI, err := fromConfigValues("parent", rc.Change.After, pcv)
 	if err != nil {
 		return "", err
@@ -47,20 +52,22 @@ func (i *ResourceManagerLien) ImportID(rc terraform.ResourceChange, pcv ConfigMa
 		return "", err
 	}
 
-	// Present the choices, if any
-	prompt := fmt.Sprintf("No existing liens found in %v", parent)
-	if len(liens) > 0 {
-		// Format the liens more nicely.
-		liensLines := []string{"Name|Origin|Reason|Restrictions"}
-		for _, lien := range liens {
-			// The name is always "liens/<id for import>".
-			lienID := strings.Split(lien.Name, "/")[1]
-			liensLines = append(liensLines, fmt.Sprintf("%v|%v|%v|%v", lienID, lien.Origin, lien.Reason, strings.Join(lien.Restrictions, ", ")))
-		}
+	if len(liens) <= 0 {
+		// There are no liens, just return that it doesn't exist so it can be created.
+		return "", &DoesNotExistErr{rc.Address}
+	}
 
-		prompt = fmt.Sprintf("Found the following liens in %v:\n%s", parentFull, columnize.SimpleFormat(liensLines))
+	// Present the choices, if any
+	// Format the liens more nicely.
+	liensLines := []string{"Name|Origin|Reason|Restrictions"}
+	for _, lien := range liens {
+		// The name is always "liens/<id for import>".
+		lienID := strings.Split(lien.Name, "/")[1]
+		liensLines = append(liensLines, fmt.Sprintf("%v|%v|%v|%v", lienID, lien.Origin, lien.Reason, strings.Join(lien.Restrictions, ", ")))
 	}
 
+	prompt := fmt.Sprintf("Found the following liens in %v:\n%s", parentFull, columnize.SimpleFormat(liensLines))
+
 	// Get the value from the user
 	name, err := fromUser(os.Stdin, "name", prompt)
 	if err != nil {

internal/tfimport/importer/resources.go

@@ -50,6 +50,16 @@ func (e *SkipErr) Error() string {
 	return "Skipped resource"
 }
 
+// DoesNotExistErr indicates that a resources specifically determined that it doesn't exist.
+// Example: google_resource_manager_lien requires a name to import, but if it finds no liens, it should return this error.
+type DoesNotExistErr struct {
+	Resource string
+}
+
+func (e *DoesNotExistErr) Error() string {
+	return fmt.Sprintf("Resource %v does not exist", e.Resource)
+}
+
 // fromConfigValues returns the first matching config value for key, from the given config value maps cvs.
 func fromConfigValues(key string, cvs ...ConfigMap) (interface{}, error) {
 	for _, cv := range cvs {

internal/tfimport/tfimport.go

@@ -446,6 +446,12 @@ var Unimportable = map[string]bool{
 	"tls_private_key":                true,
 }
 
+// RequiresInteractive lists resources which require interactivity and can't be fully automatically imported.
+var RequiresInteractive = map[string]bool{
+	"google_billing_budget":        true,
+	"google_resource_manager_lien": true,
+}
+
 // Resource represents a resource and an importer that can import it.
 type Resource struct {
 	Change         terraform.ResourceChange
@@ -466,12 +472,16 @@ func (ir Resource) ImportID(interactive bool) (string, error) {
 
 // Importable returns an importable Resource which contains an Importer, and whether it successfully created that resource.
 // pcv represents provider config values, which will be used if the resource does not have values defined.
-func Importable(rc terraform.ResourceChange, pcv importer.ConfigMap) (*Resource, bool) {
+func Importable(rc terraform.ResourceChange, pcv importer.ConfigMap, interactive bool) (*Resource, bool) {
 	ri, ok := Importers[rc.Kind]
 	if !ok {
 		return nil, false
 	}
 
+	if _, reqInteractive := RequiresInteractive[rc.Kind]; reqInteractive && !interactive {
+		return nil, false
+	}
+
 	return &Resource{
 		Change:         rc,
 		ProviderConfig: pcv,
@@ -485,7 +495,7 @@ func Import(rn runner.Runner, ir *Resource, inputDir string, terraformPath strin
 	// Try to get the ImportID()
 	importID, err := ir.ImportID(interactive)
 	if err != nil {
-		return output, err
+		return "", err
 	}
 
 	// Run the import.
@@ -592,7 +602,7 @@ func planAndImport(rn, importRn runner.Runner, runArgs *RunArgs) (retry bool, er
 		}
 
 		// Try to convert to an importable resource.
-		ir, ok := Importable(cc, pcv)
+		ir, ok := Importable(cc, pcv, runArgs.Interactive)
 		if !ok {
 			notImportableMsg := fmt.Sprintf("Resource %q of type %q not importable\n", cc.Address, cc.Kind)
 			log.Println(notImportableMsg)

internal/tfimport/tfimport_test.go

@@ -74,7 +74,7 @@ func TestImportable(t *testing.T) {
 		},
 	}
 	for _, tc := range tests {
-		got, ok := Importable(tc.rc, tc.pcv)
+		got, ok := Importable(tc.rc, tc.pcv, false)
 
 		// If we want nil, we should get nil.
 		// If we don't want nil, then the address and importer should match.