Provide additional error information in the case of compartment lookup failure during compartment creation

Author: Ravi Tandon

oci/identity_compartment_resource.go

@@ -5,6 +5,7 @@ package provider
 import (
 	"context"
 	"fmt"
+	"log"
 	"strconv"
 	"strings"
 	"time"
@@ -175,9 +176,9 @@ func (s *CompartmentResourceCrud) Create() error {
 	} else { // @next-break: remove
 		// Prevent potentially inferring wrong TenancyOCID from InstancePrincipal
 		if auth := s.Configuration["auth"]; strings.ToLower(auth) == strings.ToLower(authInstancePrincipalSetting) {
-			return fmt.Errorf("compartment_id must be specified for this resource")
+			return fmt.Errorf("compartment_id must be specified for this resource when using with auth as '%s'", authInstancePrincipalSetting)
 		}
-		// Maintain legacy contract of compartment_id defaulting to tenancy ocid if not specified
+		// Maintain legacy contract of compartment_id defaulting to tenancy_ocid if not specified
 		c := *s.Client.ConfigurationProvider()
 		if c == nil {
 			return fmt.Errorf("cannot access tenancyOCID")
@@ -217,43 +218,49 @@ func (s *CompartmentResourceCrud) Create() error {
 	if err != nil {
 		if response.RawResponse != nil && response.RawResponse.StatusCode == 409 {
 
-			// It was determined that not enabling delete should also preserve the implicit importing behavior
-			if enableDelete, ok := s.D.GetOkExists("enable_delete"); !ok || !enableDelete.(bool) {
-
-				// React to name collisions by basically importing that pre-existing compartment into this plan.
-				if strings.Contains(err.Error(), "already exists") ||
-					strings.Contains(err.Error(), "Maximum number of compartment") {
-					// List all compartments using the datasource to find that compartment with the matching name.
-					// CompartmentsDataSourceCrud requires a compartment_id, so forward whatever value was used in
-					// the create attempt above.
-					s.D.Set("compartment_id", request.CompartmentId)
-					dsCrud := &CompartmentsDataSourceCrud{s.D, s.Client, nil}
-					if err := dsCrud.Get(); err != nil {
-						return err
-					}
+			// Return an error if 'enable_delete' was explicitly set to 'true' in case of automatic import on conflict
+			if enableDelete, ok := s.D.GetOkExists("enable_delete"); ok && enableDelete.(bool) {
+				return fmt.Errorf(`%s
 
-					for _, compartment := range dsCrud.Res.Items {
-						if *compartment.Name == *request.Name {
-							s.Res = &compartment
-							//Update with correct description
-							s.D.SetId(s.ID())
-							return s.Update()
-						}
-					}
+If you define a compartment resource in your configurations with 
+the same name as an existing compartment with 'enable_delete' set to 'true', 
+the compartment will no longer be automatically imported. 
+If you intended to manage an existing compartment, use terraform import instead.`, err)
+			}
+
+			// React to name collisions or conflict errors by importing pre-existing compartment into this plan if the name matches.
+			if strings.Contains(err.Error(), "already exists") ||
+				strings.Contains(err.Error(), "Maximum number of compartment") {
+				// List all compartments using the datasource to find that compartment with the matching name.
+				// CompartmentsDataSourceCrud requires a compartment_id, so forward whatever value was used in
+				// the create attempt above.
+				s.D.Set("compartment_id", request.CompartmentId)
+				log.Println(fmt.Sprintf("[DEBUG] The specified compartment with name '%s' may already exist, listing compartments to lookup with name instead.",
+					*request.Name))
+				dsCrud := &CompartmentsDataSourceCrud{s.D, s.Client, nil}
+				if err := dsCrud.Get(); err != nil {
+					return err
 				}
 
-			} else {
+				for _, compartment := range dsCrud.Res.Items {
+					if *compartment.Name == *request.Name {
+						s.Res = &compartment
+						//Update with correct description
+						s.D.SetId(s.ID())
+						return s.Update()
+					}
+				}
+				// Return an error if the lookup failed, to provide user with information on which compartment id and name were used for lookup
 				return fmt.Errorf(`%s
 
-If you define a compartment resource in your configurations with 
-the same name as an existing compartment, the compartment will no
-longer be transparently imported. If you intended to manage 
-an existing compartment, use terraform import instead.`, err)
+failed to lookup the compartment with name: '%s' in compartment_id: '%s'.
+Verify your configuration if the correct 'compartment_id' and 'name' were specified.
+In most cases, the 'compartment_id' will be your 'tenancy_ocid' with the exception of nested compartments.
+Refer to the 'oci_identity_compartment' documentation for more information.`, err, *request.Name, *request.CompartmentId)
 			}
 		}
 		return err
 	}
-
 	s.Res = &response.Compartment
 	return nil
 }

oci/identity_compartment_test.go

@@ -5,6 +5,7 @@ package provider
 import (
 	"context"
 	"fmt"
+	"regexp"
 	"testing"
 	"time"
 
@@ -187,6 +188,15 @@ func TestIdentityCompartmentResource_basic(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "name", "Network"),
 				),
 			},
+			// verify error on existing compartment name where automatic import fails due to enable_delete
+			{
+				Config: config + compartmentIdVariableStr + CompartmentResourceDependencies +
+					generateResourceFromRepresentationMap("oci_identity_compartment", "test_compartment", Required, Create, compartmentRepresentation) +
+					generateResourceFromRepresentationMap("oci_identity_compartment", "test_compartment2", Required, Create,
+						representationCopyWithNewProperties(compartmentRepresentation, map[string]interface{}{
+							"enable_delete": Representation{repType: Required, create: `true`}})),
+				ExpectError: regexp.MustCompile("If you intended to manage an existing compartment, use terraform import instead."),
+			},
 		},
 	})
 }

website/docs/r/identity_compartment.html.markdown

@@ -11,7 +11,9 @@ This resource provides the Compartment resource in Oracle Cloud Infrastructure I
 
 Creates a new compartment in the specified compartment.
 
-**Important:** Compartments cannot be deleted.
+**Important:** Unless `enable_delete` is explicitly set to true:
+* Terraform will not delete compartments on destroy, and
+* For backwards compatibility, an existing compartment with the same name will be automatically imported into the state. Properties of the existing compartment will be updated to what is defined in the new configuration. This can cause a problem if multiple Terraform configurations are using the same compartment, but, for example, specify a different compartment description.
 
 Specify the parent compartment's OCID as the compartment ID in the request object. Remember that the tenancy
 is simply the root compartment. For information about OCIDs, see
@@ -46,12 +48,12 @@ resource "oci_identity_compartment" "test_compartment" {
 
 The following arguments are supported:
 
-* `compartment_id` - (Required) The OCID of the parent compartment containing the compartment.
+* `compartment_id` - (Required) The OCID of the parent compartment containing the compartment. In most cases, the 'compartment_id' will be the 'tenancy_ocid' with the exception of nested compartments.
 * `defined_tags` - (Optional) (Updatable) Defined tags for this resource. Each key is predefined and scoped to a namespace. For more information, see [Resource Tags](https://docs.cloud.oracle.com/iaas/Content/General/Concepts/resourcetags.htm). Example: `{"Operations.CostCenter": "42"}` 
 * `description` - (Required) (Updatable) The description you assign to the compartment during creation. Does not have to be unique, and it's changeable. 
 * `freeform_tags` - (Optional) (Updatable) Free-form tags for this resource. Each tag is a simple key-value pair with no predefined name, type, or namespace. For more information, see [Resource Tags](https://docs.cloud.oracle.com/iaas/Content/General/Concepts/resourcetags.htm). Example: `{"Department": "Finance"}` 
 * `name` - (Required) (Updatable) The name you assign to the compartment during creation. The name must be unique across all compartments in the parent compartment. Avoid entering confidential information.
-* `enable_delete` - (Optional) Defaults to false. If omitted or set to false the provider will implicitly import the compartment if there is a name collision, and will not actually delete the compartment on destroy or removal of the resource declaration. If set to true, the provider will throw an error on a name collision with another compartment, and will attempt to delete the compartment on destroy or removal of the resource declaration.  
+* `enable_delete` - (Optional) Defaults to false. If omitted or set to false the provider will implicitly import the compartment if there is a name collision, and will not actually delete the compartment on destroy or removal of the resource declaration. If set to true, the provider will throw an error on a name collision with another compartment, and will attempt to delete the compartment on destroy or removal of the resource declaration.
 
 
 ** IMPORTANT **