Add conflict between project parent kinds, allow migrating between them (#5323) (#10373)

Signed-off-by: Modular Magician <[email protected]>

Author: modular-magician

.changelog/5323.txt

@@ -0,0 +1,6 @@
+```release-note:breaking-change
+resourcemanager: added conflict between `org_id`, `folder_id` at plan time in `google_project`
+```
+```release-note:breaking-change
+resourcemanager: `org_id`, `folder_id` are unset when removed from config in `google_project`
+```

google/resource_google_project.go

@@ -68,17 +68,17 @@ func resourceGoogleProject() *schema.Resource {
 				Description:  `The display name of the project.`,
 			},
 			"org_id": {
-				Type:        schema.TypeString,
-				Optional:    true,
-				Computed:    true,
-				Description: `The numeric ID of the organization this project belongs to. Changing this forces a new project to be created.  Only one of org_id or folder_id may be specified. If the org_id is specified then the project is created at the top level. Changing this forces the project to be migrated to the newly specified organization.`,
+				Type:          schema.TypeString,
+				Optional:      true,
+				ConflictsWith: []string{"folder_id"},
+				Description:   `The numeric ID of the organization this project belongs to. Changing this forces a new project to be created.  Only one of org_id or folder_id may be specified. If the org_id is specified then the project is created at the top level. Changing this forces the project to be migrated to the newly specified organization.`,
 			},
 			"folder_id": {
-				Type:        schema.TypeString,
-				Optional:    true,
-				Computed:    true,
-				StateFunc:   parseFolderId,
-				Description: `The numeric ID of the folder this project should be created under. Only one of org_id or folder_id may be specified. If the folder_id is specified, then the project is created under the specified folder. Changing this forces the project to be migrated to the newly specified folder.`,
+				Type:          schema.TypeString,
+				Optional:      true,
+				StateFunc:     parseFolderId,
+				ConflictsWith: []string{"org_id"},
+				Description:   `The numeric ID of the folder this project should be created under. Only one of org_id or folder_id may be specified. If the folder_id is specified, then the project is created under the specified folder. Changing this forces the project to be migrated to the newly specified folder.`,
 			},
 			"number": {
 				Type:        schema.TypeString,

google/resource_google_project_test.go

@@ -240,6 +240,47 @@ func TestAccProject_parentFolder(t *testing.T) {
 	})
 }
 
+func TestAccProject_migrateParent(t *testing.T) {
+	t.Parallel()
+
+	org := getTestOrgFromEnv(t)
+	pid := fmt.Sprintf("%s-%d", testPrefix, randInt(t))
+	folderDisplayName := testPrefix + randString(t, 10)
+	vcrTest(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccProject_migrateParentFolder(pid, pname, folderDisplayName, org),
+			},
+			{
+				ResourceName:            "google_project.acceptance",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"skip_delete"},
+			},
+			{
+				Config: testAccProject_migrateParentOrg(pid, pname, folderDisplayName, org),
+			},
+			{
+				ResourceName:            "google_project.acceptance",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"skip_delete"},
+			},
+			{
+				Config: testAccProject_migrateParentFolder(pid, pname, folderDisplayName, org),
+			},
+			{
+				ResourceName:            "google_project.acceptance",
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"skip_delete"},
+			},
+		},
+	})
+}
+
 func testAccCheckGoogleProjectExists(r, pid string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[r]
@@ -412,8 +453,6 @@ resource "google_project" "acceptance" {
   project_id = "%s"
   name       = "%s"
 
-  # ensures we can set both org_id and folder_id as long as only one is not empty.
-  org_id    = ""
   folder_id = google_folder.folder1.id
 }
 
@@ -424,6 +463,38 @@ resource "google_folder" "folder1" {
 `, pid, projectName, folderName, org)
 }
 
+func testAccProject_migrateParentFolder(pid, projectName, folderName, org string) string {
+	return fmt.Sprintf(`
+resource "google_project" "acceptance" {
+  project_id = "%s"
+  name       = "%s"
+
+  folder_id = google_folder.folder1.id
+}
+
+resource "google_folder" "folder1" {
+  display_name = "%s"
+  parent       = "organizations/%s"
+}
+`, pid, projectName, folderName, org)
+}
+
+func testAccProject_migrateParentOrg(pid, projectName, folderName, org string) string {
+	return fmt.Sprintf(`
+resource "google_project" "acceptance" {
+  project_id = "%s"
+  name       = "%s"
+
+  org_id = "%s"
+}
+
+resource "google_folder" "folder1" {
+  display_name = "%s"
+  parent       = "organizations/%s"
+}
+`, pid, projectName, org, folderName, org)
+}
+
 func skipIfEnvNotSet(t *testing.T, envs ...string) {
 	if t == nil {
 		log.Printf("[DEBUG] Not running inside of test - skip skipping")

website/docs/guides/version_4_upgrade.html.markdown

@@ -47,6 +47,9 @@ description: |-
     - [`node_config.workload_metadata_config.node_metadata` is now removed](#node_configworkload_metadata_confignode_metadata-is-now-removed)
     - [`workload_identity_config.0.identity_namespace` is now removed](#workload_identity_config0identity_namespace-is-now-removed)
     - [`pod_security_policy_config` is removed from the GA provider](#pod_security_policy_config-is-removed-from-the-ga-provider)
+  - [Resource: `google_project`](#resource-google_project)
+    - [`org_id`, `folder_id` now conflict at plan time](#org_id-folder_id-now-confict-at-plan-time)
+    - [`org_id`, `folder_id` are unset when removed from config](#org_id-folder_id-are-unset-when-removed-from-config)
   - [Resource: `google_project_service`](#resource-google_project_service)
     - [`bigquery-json.googleapis.com` is no longer a valid service name](#bigquery-json.googleapis.com-is-no-longer-a-valid-service-name)
   - [Resource: `google_data_loss_prevention_trigger`](#resource-google_data_loss_prevention_trigger)
@@ -345,6 +348,20 @@ The provider will now enforce at plan time that one of these fields be set.
 ### At least one of `patch_config.0.post_step.0.linux_exec_step_config` or `patch_config.0.post_step.0.windows_exec_step_config` is required
 The provider will now enforce at plan time that one of these fields be set.
 
+## Resource: `google_project`
+
+### `org_id`, `folder_id` now conflict at plan time
+
+Previously, they were only checked for conflicts at apply time. Terraform will
+now report an error at plan time.
+
+### `org_id`, `folder_id` are unset when removed from config
+
+Previously, these fields kept their old value in state when they were removed
+from config, changing the value on next refresh. Going forward, removing one of
+the values or switching values will generate a correct plan that removes the
+value.
+
 ## Resource: `google_project_service`
 
 ### `bigquery-json.googleapis.com` is no longer a valid service name