Added - Improve Resource discovery error messaging

Author: sagarp337

internal/commonexport/commonexport_functions.go

@@ -399,14 +399,18 @@ func (ctx *ResourceDiscoveryContext) PrintSummary() {
 	utils.Logln(utils.Green(fmt.Sprintf("Total time taken by entire export: %v", ctx.TimeTakenForEntireExport)))
 }
 
-func (ctx *ResourceDiscoveryContext) PrintErrors() {
+func (ctx *ResourceDiscoveryContext) PrintErrors() ([]string, []string) {
 	utils.Logln(utils.Yellow("\n\n[WARN] Resource discovery finished with errors listed below:\n"))
+	var notDiscoveredParentResources []string
+	var notDiscoveredChildResources []string
 	for _, resourceDiscoveryError := range ctx.ErrorList.Errors {
 		if resourceDiscoveryError.ResourceType == "" || ctx.TargetSpecificResources {
 			utils.Logln(utils.Yellow(resourceDiscoveryError.Error.Error()))
 
 		} else if resourceDiscoveryError.ParentResource == "export" {
 			utils.Logln(utils.Yellow(fmt.Sprintf("Error discovering `%s` resources: %s", resourceDiscoveryError.ResourceType, resourceDiscoveryError.Error.Error())))
+			partiallyResourcesDiscovered := "Error discovering " + resourceDiscoveryError.ResourceType + " resources: " + resourceDiscoveryError.Error.Error()
+			notDiscoveredParentResources = append(notDiscoveredParentResources, partiallyResourcesDiscovered)
 
 		} else {
 			utils.Logln(utils.Yellow(fmt.Sprintf("Error discovering `%s` resources for %s: %s", resourceDiscoveryError.ResourceType, resourceDiscoveryError.ParentResource, resourceDiscoveryError.Error.Error())))
@@ -417,9 +421,13 @@ func (ctx *ResourceDiscoveryContext) PrintErrors() {
 			getNotFoundChildren(resourceDiscoveryError.ResourceType, resourceDiscoveryError.ResourceGraph, &notFoundChildren)
 			if len(notFoundChildren) > 0 {
 				utils.Logln(utils.Yellow(fmt.Sprintf("\tFollowing child resources were also not discovered due to parent error: %v", strings.Join(notFoundChildren, ", "))))
+				notFoundChildResources := "\tFollowing child resources were also not discovered due to parent error: " + strings.Join(notFoundChildren, ", ")
+				notDiscoveredChildResources = append(notDiscoveredChildResources, notFoundChildResources)
 			}
 		}
 	}
+
+	return notDiscoveredParentResources, notDiscoveredChildResources
 }
 
 func (h *TerraformResourceHints) DiscoversWithSingularDatasource() bool {

internal/commonexport/commonexport_types.go

@@ -138,6 +138,52 @@ type ResourceDiscoveryError struct {
 	Error          error
 	ResourceGraph  *TerraformResourceGraph
 }
+type ResourceDiscoveryCustomError struct {
+	TypeOfError ErrorTypeEnum
+	Message     error
+	Suggestion  string
+}
+
+const (
+	// RD Error Message
+	WriteTmpStateError                ErrorTypeEnum = "WriteTmpStateError"
+	IdsNotFoundError                  ErrorTypeEnum = "IdsNotFoundError"
+	PartiallyResourcesDiscoveredError ErrorTypeEnum = "PartiallyResourcesDiscoveredError"
+
+	WriteTmpStateErrorMessage              = "[ERROR] error writing temp state for resources found:"
+	IdsNotFoundErrorMessage                = "[ERROR] one or more expected resource ids were not found"
+	PartiallyResourcesDiscoveredSuggestion = `This error could happen if:
+				1. GET/LIST api failed to get the resource.
+				2. If GET request requires more attributes than the OCID of a resource. If the resource supports composite id, make sure correct composite Id is getting set in resource data using getIdFn resource hint.
+				3. If resource is dependent on another resource(parent), the field for the parent resource should be a required field in API.
+				4. If a resource is tenancy specific i.e. it can only be created in the tenancy then the resource should be present under tenancyResourcesGraphs.`
+	WriteTmpStateErrorSuggestion = "Unsupported Terraform version! Please install supported terraform v.0.12 - https://releases.hashicorp.com/terraform/0.12.31/"
+	IdsNotFoundSuggestion        = "SetData() method should not contains parameters are not supported by schema\""
+)
+
+func (tfE ResourceDiscoveryCustomError) Error() error {
+	switch tfE.TypeOfError {
+	case WriteTmpStateError:
+		return fmt.Errorf(
+			"Error Message: %s \n"+
+				"Suggestion: %s\n",
+			tfE.Message.Error(), tfE.Suggestion)
+	case IdsNotFoundError:
+		return fmt.Errorf(
+			"Error Message: %s \n"+
+				"Suggestion: %s\n",
+			tfE.Message.Error(), tfE.Suggestion)
+	case PartiallyResourcesDiscoveredError:
+		return fmt.Errorf(
+			"Error Message: %s \n"+
+				"Suggestion: %s\n",
+			tfE.Message.Error(), tfE.Suggestion)
+	default:
+		return fmt.Errorf(tfE.Message.Error())
+	}
+}
+
+type ErrorTypeEnum string
 
 var TfHclVersionvar TfHclVersion
 var GetHclStringFromGenericMap = func(builder *strings.Builder, ociRes *OCIResource, interpolationMap map[string]string) error {

internal/resourcediscovery/export_compartment.go

@@ -314,12 +314,28 @@ func RunExportCommand(args *tf_export.ExportCommandArgs) (err error, status Stat
 	}
 	if len(ctx.ErrorList.Errors) > 0 {
 		// If the errors were from discovery of resources return partial success status
-		ctx.PrintErrors()
-		return nil, StatusPartialSuccess
+		error, status := getListOfNotDiscoveredResources(ctx)
+
+		return error, status
 	}
 	return nil, StatusSuccess
 }
 
+func getListOfNotDiscoveredResources(ctx *tf_export.ResourceDiscoveryContext) (error, Status) {
+	notDiscoveredParentResources, notDiscoveredChildResources := ctx.PrintErrors()
+	var notDiscoveredResources []string
+	var notDiscoveredError tf_export.ResourceDiscoveryCustomError
+
+	notDiscoveredResources = append(notDiscoveredParentResources, notDiscoveredChildResources...) // Not discovered resources eg.Parent resources + Child Resources
+
+	notDiscoveredError = tf_export.ResourceDiscoveryCustomError{
+		TypeOfError: tf_export.PartiallyResourcesDiscoveredError,
+		Message:     errors.New(strings.Join(notDiscoveredResources, ",")),
+	}
+
+	return notDiscoveredError.Error(), StatusPartialSuccess
+}
+
 func getExportConfig(d *schema.ResourceData) (interface{}, error) {
 	clients := &tf_client.OracleClients{
 		SdkClientMap:  make(map[string]interface{}, len(tf_client.OracleClientRegistrationsVar.RegisteredClients)),
@@ -600,10 +616,15 @@ func generateStateParallel(ctx *tf_export.ResourceDiscoveryContext, steps []reso
 
 			// Write temp state file for each service, this step will import resources into a separate state file for each service in parallel
 			// If writing temp configuration thrown error, won't attempt to write temp state
-			if err == nil {
-				if err = step.writeTmpState(); err != nil {
-					errorChannel <- fmt.Errorf("[ERROR] error while writing temp state for resources found in step %d: %s", i, err.Error())
+			// Write temp state file for each service, this step will import resources into a separate state file for each service in parallel
+			if err := step.writeTmpState(); err != nil {
+				var tfError tf_export.ResourceDiscoveryCustomError
+				tfError = tf_export.ResourceDiscoveryCustomError{
+					TypeOfError: tf_export.WriteTmpStateError,
+					Message:     errors.New(tf_export.WriteTmpStateErrorMessage),
+					Suggestion:  tf_export.WriteTmpStateErrorSuggestion,
 				}
+				errorChannel <- tfError.Error()
 			}
 
 			utils.Debugf("writing temp config and state: Completed step %d", i)

internal/resourcediscovery/export_compartment_test.go

@@ -1046,7 +1046,7 @@ func TestUnitRunExportCommand_exitStatusForPartialSuccess(t *testing.T) {
 		return getTestClients(), nil
 	}
 	exportConfigProvider = acctest.MockConfigurationProvider{}
-	if err, status := RunExportCommand(args); err != nil {
+	if err, status := RunExportCommand(args); err != nil && status == StatusFail {
 		t.Logf("(TF version %s) export command failed due to err: %v", tf_export.TfHclVersionvar.ToString(), err)
 		t.Fail()
 	} else if status != StatusPartialSuccess {
@@ -1066,6 +1066,50 @@ func TestUnitRunExportCommand_exitStatusForPartialSuccess(t *testing.T) {
 	os.RemoveAll(outputDir)
 }
 
+// Test exit status in case of partial success
+// issue-routing-tag: terraform/default
+func TestUnitRunExportCommand_errorSuggestionForPartialSuccess(t *testing.T) {
+	type testFormat struct {
+		name        string
+		errorLength int
+		ctx         *tf_export.ResourceDiscoveryContext
+	}
+	errors := []*tf_export.ResourceDiscoveryError{}
+	parentResource := "oci_test_parent:ocid1.parent.abcdefghiklmnop.0"
+
+	resourceDiscoveryContext := &tf_export.ResourceDiscoveryContext{
+		ResourceHintsLookup: map[string]*tf_export.TerraformResourceHints{"oci_test_parent": exportParentDefinition},
+		ExpectedResourceIds: map[string]bool{"oci_test_parent:ocid1.parent.abcdefghiklmnop.0": false},
+		ErrorList: tf_export.ErrorList{
+			Errors: append(errors, &tf_export.ResourceDiscoveryError{
+				ResourceType:   "load_balancer",
+				ParentResource: "export",
+				Error:          fmt.Errorf("Error Message: [ERROR] Error while discovering below resources:\n" + parentResource),
+				ResourceGraph:  nil,
+			}),
+		},
+		TargetSpecificResources: false,
+	}
+	tests := []testFormat{
+		{
+			name:        "Test error message for partially not discovered resources",
+			ctx:         resourceDiscoveryContext,
+			errorLength: 1,
+		},
+	}
+	for _, test := range tests {
+		t.Logf("Running %s", test.name)
+		if error, _ := getListOfNotDiscoveredResources(test.ctx); error != nil {
+			expectedErrorMessage := error.Error()
+			fmt.Println("\n", expectedErrorMessage)
+			if !strings.Contains(expectedErrorMessage, parentResource) {
+				t.Errorf("Output error - %s which is not equal to expected error - %s", expectedErrorMessage, test.ctx.ErrorList.Errors[0].Error.Error())
+			}
+
+		}
+	}
+}
+
 // Test that resources can be found using a resource dependency graph
 // issue-routing-tag: terraform/default
 func TestUnitFindResources_basic(t *testing.T) {

internal/resourcediscovery/export_resource_helpers_test.go

@@ -505,11 +505,23 @@ func TestUnitpostValidate(t *testing.T) {
 			name:        "Test one missing resource",
 			errorLength: 1,
 		},
+		{
+			name:        "Missing resource id check",
+			errorLength: 2,
+		},
 	}
 	for _, test := range tests {
 		t.Logf("Running %s", test.name)
 		ctx.PostValidate()
+		// Validate encapsulated error message: one or more expected resource ids were not found
+		if len(ctx.ErrorList.Errors) == 2 {
+			for _, resourceDiscoveryError := range ctx.ErrorList.Errors {
+				// element is the element from someSlice for where we are
+				fmt.Println("Error", resourceDiscoveryError.Error)
+			}
+		}
 		if len(ctx.ErrorList.Errors) != test.errorLength {
+
 			t.Errorf("Output error length - %d which is not equal to expected error - %d", len(ctx.ErrorList.Errors), test.errorLength)
 		}
 	}

internal/resourcediscovery/export_test_helpers.go

@@ -133,8 +133,14 @@ func testExportCompartment(compartmentId *string, exportCommandArgs *tf_export.E
 		}
 		// For generated tests, RD will only return this error if one of the `ids` was not found
 		// (which in case of tests is the id for the resource RD is looking for)
-		if status == StatusPartialSuccess {
-			return fmt.Errorf("[ERROR] expected resource was not found")
+		if errExport != nil && status == StatusPartialSuccess {
+			var idsNotFoundError tf_export.ResourceDiscoveryCustomError
+			idsNotFoundError = tf_export.ResourceDiscoveryCustomError{
+				TypeOfError: tf_export.PartiallyResourcesDiscoveredError,
+				Message:     errExport,
+				Suggestion:  tf_export.PartiallyResourcesDiscoveredSuggestion,
+			}
+			return idsNotFoundError.Error()
 		}
 	}