Merge pull request #1069 from timofurrer/cleanup/jira-id

resource/gitlab_service_jira: several cleanup of tech debt

Author: timofurrer

docs/resources/service_jira.md

@@ -46,7 +46,7 @@ resource "gitlab_service_jira" "jira" {
 - `comment_on_event_enabled` (Boolean) Enable comments inside Jira issues on each GitLab event (commit / merge request)
 - `commit_events` (Boolean) Enable notifications for commit events
 - `issues_events` (Boolean) Enable notifications for issues events.
-- `jira_issue_transition_id` (String) The ID of a transition that moves issues to a closed state. You can find this number under the JIRA workflow administration (Administration > Issues > Workflows) by selecting View under Operations of the desired workflow of your project. By default, this ID is set to 2.
+- `jira_issue_transition_id` (String) The ID of a transition that moves issues to a closed state. You can find this number under the JIRA workflow administration (Administration > Issues > Workflows) by selecting View under Operations of the desired workflow of your project. By default, this ID is set to 2. **Note**: importing this field is currently not supported.
 - `job_events` (Boolean) Enable notifications for job events.
 - `merge_requests_events` (Boolean) Enable notifications for merge request events
 - `note_events` (Boolean) Enable notifications for note events.

internal/provider/resource_gitlab_service_jira.go

@@ -20,7 +20,7 @@ var _ = registerResource("gitlab_service_jira", func() *schema.Resource {
 		UpdateContext: resourceGitlabServiceJiraUpdate,
 		DeleteContext: resourceGitlabServiceJiraDelete,
 		Importer: &schema.ResourceImporter{
-			StateContext: resourceGitlabServiceJiraImportState,
+			StateContext: schema.ImportStatePassthroughContext,
 		},
 
 		Schema: map[string]*schema.Schema{
@@ -81,7 +81,7 @@ var _ = registerResource("gitlab_service_jira", func() *schema.Resource {
 				Sensitive:   true,
 			},
 			"jira_issue_transition_id": {
-				Description: "The ID of a transition that moves issues to a closed state. You can find this number under the JIRA workflow administration (Administration > Issues > Workflows) by selecting View under Operations of the desired workflow of your project. By default, this ID is set to 2.",
+				Description: "The ID of a transition that moves issues to a closed state. You can find this number under the JIRA workflow administration (Administration > Issues > Workflows) by selecting View under Operations of the desired workflow of your project. By default, this ID is set to 2. **Note**: importing this field is currently not supported.",
 				Type:        schema.TypeString,
 				Optional:    true,
 			},
@@ -166,46 +166,31 @@ func resourceGitlabServiceJiraCreate(ctx context.Context, d *schema.ResourceData
 
 func resourceGitlabServiceJiraRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	client := meta.(*gitlab.Client)
-	project := d.Get("project").(string)
-
-	p, _, err := client.Projects.GetProject(project, nil, gitlab.WithContext(ctx))
-	if err != nil {
-		if is404(err) {
-			log.Printf("[DEBUG] Removing Gitlab Jira service %s because project %s not found", d.Id(), p.Name)
-			d.SetId("")
-			return nil
-		}
-		return diag.FromErr(err)
-	}
+	project := d.Id()
 
-	log.Printf("[DEBUG] Read Gitlab Jira service %s", d.Id())
+	log.Printf("[DEBUG] Read Gitlab Jira service %s", project)
 
 	jiraService, _, err := client.Services.GetJiraService(project, gitlab.WithContext(ctx))
 	if err != nil {
 		if is404(err) {
-			log.Printf("[DEBUG] gitlab jira service not found %s", project)
+			log.Printf("[DEBUG] gitlab jira service not found %s, removing from state", project)
 			d.SetId("")
 			return nil
 		}
 		return diag.FromErr(err)
 	}
 
-	if v := jiraService.Properties.URL; v != "" {
-		d.Set("url", v)
+	d.Set("project", project)
+	d.Set("url", jiraService.Properties.URL)
+	d.Set("api_url", jiraService.Properties.APIURL)
+	d.Set("username", jiraService.Properties.Username)
+	d.Set("project_key", jiraService.Properties.ProjectKey)
+	// FIXME: The API or go-gitlab doesn't return `jira_issue_transition_id` properly,
+	//        therefore we don't overwrite the value set in the config, in case it's not set in the API
+	//        response. This currently impacts the import of this attribute.
+	if jiraService.Properties.JiraIssueTransitionID != "" {
+		d.Set("jira_issue_transition_id", jiraService.Properties.JiraIssueTransitionID)
 	}
-	if v := jiraService.Properties.APIURL; v != "" {
-		d.Set("api_url", v)
-	}
-	if v := jiraService.Properties.Username; v != "" {
-		d.Set("username", v)
-	}
-	if v := jiraService.Properties.ProjectKey; v != "" {
-		d.Set("project_key", v)
-	}
-	if v := jiraService.Properties.JiraIssueTransitionID; v != "" {
-		d.Set("jira_issue_transition_id", v)
-	}
-
 	d.Set("title", jiraService.Title)
 	d.Set("created_at", jiraService.CreatedAt.String())
 	d.Set("updated_at", jiraService.UpdatedAt.String())
@@ -264,9 +249,3 @@ func expandJiraOptions(d *schema.ResourceData) (*gitlab.SetJiraServiceOptions, e
 
 	return &setJiraServiceOptions, nil
 }
-
-func resourceGitlabServiceJiraImportState(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
-	d.Set("project", d.Id())
-
-	return []*schema.ResourceData{d}, nil
-}

internal/provider/resource_gitlab_service_jira_test.go

@@ -35,6 +35,16 @@ func TestAccGitlabServiceJira_basic(t *testing.T) {
 					resource.TestCheckResourceAttr(jiraResourceName, "comment_on_event_enabled", "false"),
 				),
 			},
+			// Verify Import
+			{
+				ResourceName:      jiraResourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+				// FIXME: there is a bug in the GitLab API which causes the `jira_issue_transition_id` field
+				//        to be empty in the response. Therefore, we ignore it for now in the import.
+				//        See https://gitlab.com/gitlab-org/gitlab/-/issues/362437
+				ImportStateVerifyIgnore: []string{"password", "jira_issue_transition_id"},
+			},
 			// Update the jira service
 			{
 				Config: testAccGitlabServiceJiraUpdateConfig(rInt),
@@ -50,6 +60,16 @@ func TestAccGitlabServiceJira_basic(t *testing.T) {
 					resource.TestCheckResourceAttr(jiraResourceName, "comment_on_event_enabled", "true"),
 				),
 			},
+			// Verify Import
+			{
+				ResourceName:      jiraResourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+				// FIXME: there is a bug in the GitLab API which causes the `jira_issue_transition_id` field
+				//        to be empty in the response. Therefore, we ignore it for now in the import.
+				//        See https://gitlab.com/gitlab-org/gitlab/-/issues/362437
+				ImportStateVerifyIgnore: []string{"password", "jira_issue_transition_id"},
+			},
 			// Update the jira service to get back to previous settings
 			{
 				Config: testAccGitlabServiceJiraConfig(rInt),
@@ -64,30 +84,15 @@ func TestAccGitlabServiceJira_basic(t *testing.T) {
 					resource.TestCheckResourceAttr(jiraResourceName, "comment_on_event_enabled", "false"),
 				),
 			},
-		},
-	})
-}
-
-// lintignore: AT002 // TODO: Resolve this tfproviderlint issue
-func TestAccGitlabServiceJira_import(t *testing.T) {
-	jiraResourceName := "gitlab_service_jira.jira"
-	rInt := acctest.RandInt()
-
-	resource.Test(t, resource.TestCase{
-		ProviderFactories: providerFactories,
-		CheckDestroy:      testAccCheckGitlabServiceJiraDestroy,
-		Steps: []resource.TestStep{
-			{
-				Config: testAccGitlabServiceJiraConfig(rInt),
-			},
+			// Verify Import
 			{
 				ResourceName:      jiraResourceName,
-				ImportStateIdFunc: getJiraProjectID(jiraResourceName),
 				ImportState:       true,
 				ImportStateVerify: true,
-				ImportStateVerifyIgnore: []string{
-					"password",
-				},
+				// FIXME: there is a bug in the GitLab API which causes the `jira_issue_transition_id` field
+				//        to be empty in the response. Therefore, we ignore it for now in the import.
+				//        See https://gitlab.com/gitlab-org/gitlab/-/issues/362437
+				ImportStateVerifyIgnore: []string{"password", "jira_issue_transition_id"},
 			},
 		},
 	})
@@ -136,22 +141,6 @@ func testAccCheckGitlabServiceJiraDestroy(s *terraform.State) error {
 	return nil
 }
 
-func getJiraProjectID(n string) resource.ImportStateIdFunc {
-	return func(s *terraform.State) (string, error) {
-		rs, ok := s.RootModule().Resources[n]
-		if !ok {
-			return "", fmt.Errorf("Not Found: %s", n)
-		}
-
-		project := rs.Primary.Attributes["project"]
-		if project == "" {
-			return "", fmt.Errorf("No project ID is set")
-		}
-
-		return project, nil
-	}
-}
-
 func testAccGitlabServiceJiraConfig(rInt int) string {
 	return fmt.Sprintf(`
 resource "gitlab_project" "foo" {