Import of integration/hook requires project id, but converts to project name on subsequent plan

[olhado]

GitLab Provider version

3.16.0

GitLab version

Gitlab 14.5.2 EE

Terraform version

1.3.2

Relevant Terraform Configuration

No response

Relevant log output

########## Planning after importing a hook
# module.groups["foo"].gitlab_project_hook.integration["bar"] will be updated in-place
  ~ resource "gitlab_project_hook" "integration" {
        id                         = "111"
      ~ project                    = "1234" -> "bar"
        # (14 unchanged attributes hidden)
    }
####################
########## Planning after importing the jira service of a project
  # module.groups["foo"].gitlab_service_jira.integration["bar"] must be replaced
-/+ resource "gitlab_service_jira" "integration" {
      ~ active                   = true -> (known after apply)
      + api_url                  = (known after apply)
      ~ created_at               = "2022-01-01 00:00:00.001 +0000 UTC" -> (known after apply)
      ~ id                       = "111" -> (known after apply)
      ~ issues_events            = true -> (known after apply)
      ~ job_events               = true -> (known after apply)
      ~ note_events              = true -> (known after apply)
      + password                 = (sensitive value)
      ~ pipeline_events          = true -> (known after apply)
      ~ project                  = "1234" -> "bar" # forces replacement
      ~ push_events              = true -> (known after apply)
      ~ tag_push_events          = true -> (known after apply)
      ~ title                    = "Jira" -> (known after apply)
      ~ updated_at               = "2022-01-07 00:00:00.001 +0000 UTC" -> (known after apply)
        # (4 unchanged attributes hidden)
    }

Description

Using terraform import requires a project id, and will throw an error if a project name is used. However, if you then run a terraform plan, it says the resource is going to change because the project ID is being converted to the project name. For the jira service/integration, this is even worse as it does a destroy/create replace cycle precisely because the project id is being converted to the project name.

It seems like the easiest ways to fix would be to:

• Allow import by project name if possible, though you'd need to validate that the name is unique/handle full-path of project.
• During the import save the project name as the id in the state file. This seems the most straightforward.

[olhado]

I guess, alternatively, the issue is in the plan/apply code, which should not be converting it to a project name, as that could introduce an ambiguity if two projects in different groups have the same name. This likely would affect forks quite a bit.

[RicePatrick]

It looks like this will only impact hooks/integrations where the name is used instead of the ID, but then the ID is used to import the project. The import always imports the ID as the integer value, so if the configuration uses the integer value this bug won't occur. However, if the configuration uses the string value it will store that as the "project" value and cause a mismatch post-import.

You can verify this bug with the following code in the project hook test:

func TestAccGitlabProjectHook_importPreexistingHook(t *testing.T) {
	// Testing a fix for a bad import on pre-existing hook.
	project := testAccCreateProject(t)
	var hook gitlab.ProjectHook

	resource.ParallelTest(t, resource.TestCase{
		ProviderFactories: providerFactories,
		CheckDestroy:      testAccCheckGitlabProjectHookDestroy,
		Steps: []resource.TestStep{
			// Verify import
			{
				Config: fmt.Sprintf(`
				  resource "gitlab_project_hook" "foo" {
				    project = "%s"
				    url = "https://example.test"
				  }
			    `, project.PathWithNamespace),
				Check: testAccCheckGitlabProjectHookExists("gitlab_project_hook.foo", &hook),
			},
			{
				ResourceName: "gitlab_project_hook.foo",
				ImportStateIdFunc: func(s *terraform.State) (string, error) {
					return fmt.Sprintf("%d:%d", project.ID, hook.ID), nil
				},
				ImportState:             true,
				ImportStateVerify:       true,
				ImportStateVerifyIgnore: []string{"token"},
			},
		},
	})
}

Since I've got a valid failing test, I should be able to work on a PR for this shortly, or feel free to submit one @olhado ! This will actually impact more than just these two resources I think, for example I think the gitlab_project_protected_environment resource would be impacted by this the way it's coded as well.

@timofurrer - I think the best way to do this is to create a helper suppress diff function that we would use on these downstream project resource where it checks the types of old vs new values, and if one is the ID and the other is a string value, we run a getProject API call and compare the IDs. Thoughts?

[timofurrer]

@PatrickRice-KSC the suppress diff func would be a neat solution probably - BUT you won't have access to the GitLab client from meta like you do in the crud functions :'(

I think this is something for v4 when we break the interface ... in the meantime I suggest to just use the id instead of the name ...