`azuredevops_serviceendpoint_sonarqube` Adding nil check to project ID of sonarqube service endpoint

dawwestk commented 2 months ago

All Submissions:

[x] Have you added an explanation of what your changes do and why you'd like us to include them?
[ ] I have updated the documentation accordingly.
[ ] I have added tests to cover my changes.
[ ] All new and existing tests passed.
[x] My code follows the code style of this project.
[x] I ran lint checks locally prior to submission.
[x] Have you checked to ensure there aren't other open PRs for the same update/change?

What about the current behavior has changed?

The SonarQube service endpoint is now checked for a valid project ID and returns an appropriate error message if one is not found.

This solves the issue shown in the logs below which occurred when trying to import a service endpoint that had been shared between projects. As these resources are imported using their (shared) ID, the error below occurred once the original endpoint had been deleted, thus leaving no associated project ID.

The nil check is the same as for the kubernetes service endpoint.

Does this introduce a change to `go.mod`, `go.sum` or `vendor/`?

[ ] Yes
[x] No

Does this introduce a breaking change?

[ ] Yes
[x] No

Any relevant logs, error output, etc?

Stack trace from the terraform-provider-azuredevops_v1.3.0 plugin:

panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x8 pc=0x153c063]

goroutine 53 [running]:
github.com/microsoft/terraform-provider-azuredevops/azuredevops/internal/service/serviceendpoint.resourceServiceEndpointSonarQubeRead(0xc00019c900, {0x15caf20?, 0xc0000da4e0})   
        github.com/microsoft/terraform-provider-azuredevops/azuredevops/internal/service/serviceendpoint/resource_serviceendpoint_sonarqube.go:87 +0xa3
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).read(0x1945248?, {0x1945248?, 0xc000108090?}, 0xd?, {0x15caf20?, 0xc0000da4e0?})
        github.com/hashicorp/terraform-plugin-sdk/v2@v2.23.0/helper/schema/resource.go:712 +0x15f
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).RefreshWithoutUpgrade(0xc0004ac380, {0x1945248, 0xc000108090}, 0xc000592820, {0x15caf20, 0xc0000da4e0})    
        github.com/hashicorp/terraform-plugin-sdk/v2@v2.23.0/helper/schema/resource.go:1015 +0x51d
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).ReadResource(0xc00011a000, {0x1945248?, 0xc000703f50?}, 0xc0002566c0)
        github.com/hashicorp/terraform-plugin-sdk/v2@v2.23.0/helper/schema/grpc_provider.go:613 +0x4aa
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ReadResource(0xc000176000, {0x1945248?, 0xc0007039b0?}, 0xc0002b02a0)
        github.com/hashicorp/terraform-plugin-go@v0.14.0/tfprotov5/tf5server/server.go:748 +0x488
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ReadResource_Handler({0x1783840, 0xc000176000}, {0x1945248, 0xc0007039b0}, 0xc000210000, 0x0)     
        github.com/hashicorp/terraform-plugin-go@v0.14.0/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:349 +0x1a6
google.golang.org/grpc.(*Server).processUnaryRPC(0xc000150000, {0x1949ba0, 0xc0004b1520}, 0xc0005c4000, 0xc000702b40, 0x1f97ef0, 0x0)
        google.golang.org/grpc@v1.56.3/server.go:1335 +0xdb8
google.golang.org/grpc.(*Server).handleStream(0xc000150000, {0x1949ba0, 0xc0004b1520}, 0xc0005c4000, 0x0)
        google.golang.org/grpc@v1.56.3/server.go:1712 +0x9da
google.golang.org/grpc.(*Server).serveStreams.func1.1()
        google.golang.org/grpc@v1.56.3/server.go:947 +0xbb
created by google.golang.org/grpc.(*Server).serveStreams.func1 in goroutine 11
        google.golang.org/grpc@v1.56.3/server.go:958 +0x136

Error: The terraform-provider-azuredevops_v1.3.0 plugin crashed!

xuzhang3 commented 2 months ago

@dawwestk Can you share more details? I know there is a case where the project is not returned, but I can't reproduce it.

dawwestk commented 2 months ago

Hi @xuzhang3, no problem, here are the steps to reproduce:

Enable the SonarQube extension in your Azure DevOps org
Create 2 Azure DevOps Projects
Manually create a SonarQube service connection in one Project, then share it with the other (the connection will now appear in both Projects with the same ID)
Use import blocks to bring the existing/legacy connections into terraform state

resource "azuredevops_serviceendpoint_sonarqube" "sonarqube" {
  for_each              = local.svc_conns
  project_id            = each.value.project
  service_endpoint_name = "sonarqube"
  url                   = "https://my-sonarqube.com"
  token                 = "token1234"
  description           = "managed by Terraform"
}

import {
  for_each = local.svc_conns
  to       = azuredevops_serviceendpoint_sonarqube.sonarqube[each.key]
  id       = "${each.value.project}/${each.value.id}"
}

locals {
  svc_conns = {
    "projectA" = {
      "project" = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
      "id"      = "<svc-conn-ID>"
    }
    "projectB" = {
      "project" = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb",
      "id"      = "<svc-conn-ID>"
    }
  }
}

Attempting to apply the code above should fail with the error message in my original comment. With my update it should give a normal terraform error.

Let me know if you need any more info - cheers

xuzhang3 commented 2 months ago

Hi @xuzhang3, no problem, here are the steps to reproduce:

Enable the SonarQube extension in your Azure DevOps org

Create 2 Azure DevOps Projects

Manually create a SonarQube service connection in one Project, then share it with the other (the connection will now appear in both Projects with the same ID)

Use import blocks to bring the existing/legacy connections into terraform state
resource "azuredevops_serviceendpoint_sonarqube" "sonarqube" {
  for_each              = local.svc_conns
  project_id            = each.value.project
  service_endpoint_name = "sonarqube"
  url                   = "https://my-sonarqube.com"
  token                 = "token1234"
  description           = "managed by Terraform"
}

import {
  for_each = local.svc_conns
  to       = azuredevops_serviceendpoint_sonarqube.sonarqube[each.key]
  id       = "${each.value.project}/${each.value.id}"
}

locals {
  svc_conns = {
    "projectA" = {
      "project" = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa",
      "id"      = "<svc-conn-ID>"
    }
    "projectB" = {
      "project" = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb",
      "id"      = "<svc-conn-ID>"
    }
  }
}
Attempting to apply the code above should fail with the error message in my original comment. With my update it should give a normal terraform error.

Let me know if you need any more info - cheers

I know why this happens. The service connection needs to be import was destroyed, but the service does not return a 404, but a 200. We need to handle the response correctly

dawwestk commented 2 months ago

Thanks @xuzhang3 - updated as per your comment

xuzhang3 commented 2 months ago

=== RUN   TestAccServiceEndpointSonarQube_basic
=== PAUSE TestAccServiceEndpointSonarQube_basic
=== RUN   TestAccServiceEndpointSonarQube_complete
=== PAUSE TestAccServiceEndpointSonarQube_complete
=== RUN   TestAccServiceEndpointSonarQube_update
=== PAUSE TestAccServiceEndpointSonarQube_update
=== RUN   TestAccServiceEndpointSonarQube_RequiresImportErrorStep
=== PAUSE TestAccServiceEndpointSonarQube_RequiresImportErrorStep
=== CONT  TestAccServiceEndpointSonarQube_basic
=== CONT  TestAccServiceEndpointSonarQube_update
=== CONT  TestAccServiceEndpointSonarQube_complete
=== CONT  TestAccServiceEndpointSonarQube_RequiresImportErrorStep
--- PASS: TestAccServiceEndpointSonarQube_complete (84.29s)
--- PASS: TestAccServiceEndpointSonarQube_RequiresImportErrorStep (90.73s)
--- PASS: TestAccServiceEndpointSonarQube_basic (123.55s)
--- PASS: TestAccServiceEndpointSonarQube_update (165.15s)
PASS
ok      github.com/microsoft/terraform-provider-azuredevops/azuredevops/internal/acceptancetests        165.605s

microsoft / terraform-provider-azuredevops