jdamata / terraform-provider-sonarqube

Terraform provider for managing Sonarqube configuration
GNU General Public License v3.0
62 stars 50 forks source link

fix: retrocompatibility break when v0.16.3 was released #218

Closed david-ortiz-saez closed 6 months ago

david-ortiz-saez commented 8 months ago

WHAT

This is fixing ISSUE-216.

In v0.16.3 release this was changed.

As a consequence, when new qualityprofile_project_associations are created, it will provide idSlice[2] with the language of the qualityProfile.

However, for qualityProfileAssociations that were created using this module before that association, the idSlice lenght is only 2, being idSlice[0] the name and idSlice[1] the key. As a consequence, when we try to upgrade from v0.16.2 to v0.16.3 we're getting an error:

Stack trace from the terraform-provider-sonarqube_v0.16.3 plugin:

panic: runtime error: index out of range [2] with length 2

goroutine 298 [running]:
github.com/jdamata/terraform-provider-sonarqube/sonarqube.resourceSonarqubeQualityProfileProjectAssociationRead(0xc000277500, {0xb52ee0?, 0xc0001af380})
    github.com/jdamata/terraform-provider-sonarqube/sonarqube/resource_sonarqube_qualityprofile_project_association.go:133 +0xfaf
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).read(0xdb3740?, {0xdb3740?, 0xc0002c2960?}, 0xd?, {0xb52ee0?, 0xc0001af380?})
    github.com/hashicorp/terraform-plugin-sdk/v2@v2.27.0/helper/schema/resource.go:738 +0x178
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).RefreshWithoutUpgrade(0xc00012f0a0, {0xdb3740, 0xc0002c2960}, 0xc00026a5b0, {0xb52ee0, 0xc0001af380})
    github.com/hashicorp/terraform-plugin-sdk/v2@v2.27.0/helper/schema/resource.go:1044 +0x59e
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).ReadResource(0xc000125878, {0xdb3740?, 0xc0002c2810?}, 0xc0002750c0)
    github.com/hashicorp/terraform-plugin-sdk/v2@v2.27.0/helper/schema/grpc_provider.go:616 +0x497
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ReadResource(0xc00012d220, {0xdb3740?, 0xc000405950?}, 0xc00068f2c0)
    github.com/hashicorp/terraform-plugin-go@v0.16.0/tfprotov5/tf5server/server.go:751 +0x49e
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ReadResource_Handler({0xc508a0?, 0xc00012d220}, {0xdb3740, 0xc000405950}, 0xc0005435e0, 0x0)
    github.com/hashicorp/terraform-plugin-go@v0.16.0/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:386 +0x170
google.golang.org/grpc.(*Server).processUnaryRPC(0xc0003f0000, {0xdb64a0, 0xc00048c1a0}, 0xc0001905a0, 0xc0003dbfb0, 0x12595d0, 0x0)
    google.golang.org/grpc@v1.56.0/server.go:1337 +0xde3
google.golang.org/grpc.(*Server).handleStream(0xc0003f0000, {0xdb64a0, 0xc00048c1a0}, 0xc0001905a0, 0x0)
    google.golang.org/grpc@v1.56.0/server.go:1714 +0xa1b
google.golang.org/grpc.(*Server).serveStreams.func1.1()
    google.golang.org/grpc@v1.56.0/server.go:959 +0x98
created by google.golang.org/grpc.(*Server).serveStreams.func1
    google.golang.org/grpc@v1.56.0/server.go:957 +0x18c

Error: The terraform-provider-sonarqube_v0.16.3 plugin crashed!

This is always indicative of a bug within the plugin. It would be immensely
helpful if you could report the crash with the plugin's maintainers so that it
can be fixed. The output above should help diagnose the issue.

This Pr is fixing the issue, if idSlice[2] doesn't exist, it will use the Get method to populate the language

david-ortiz-saez commented 8 months ago

@jdamata kindly review :)

freeranger commented 8 months ago

@david-ortiz-saez Can you add one or more unit tests to demonstrate that you have fixed the problem you have reported? Automated tests are a good way to ensure that the expected outcome is indeed the actual outcome :)

freeranger commented 8 months ago

Also there is no v0.62.3 :}

david-ortiz-saez commented 8 months ago

hi @freeranger, thanks for pointing out these errors. I've fixed them.

Regarding the unit tests, this method is already covered with tests.

We have built the plugin and run the terraform plan that was failing and now it doesn't

freeranger commented 8 months ago

hi @david-ortiz-saez

Regarding the unit tests, this method is already covered with tests.

Since those tests pass with or without your change, that suggests that the use case is not covered by the tests - hence I am suggesting you add additional tests to cover this and demonstrate that the code works in all cases. thanks

david-ortiz-saez commented 8 months ago

Hi @freeranger! I've added a test case where we take the language from the sonarqube project itself, same as my changes are doing. Hope that helps

david-ortiz-saez commented 8 months ago

hi @freeranger, @jdamata. After further reviewing it I've reached the conclusion that sadly we cannot create a test for this use case.

This PR is fixing the provider to work with the already existing sonarqube_qualityprofile_project_association.

Since v0.16.3 each new sonarqube_qualityprofile_project_association that is created will have an id in terraform state like this one: "Quality_Profile/Project/Language".

So when we try to access idSlice[2] there is no problem.

However, for those sonarqube_qualityprofile_project_association that were created using provider version v0.16.2 or lower, their id in terraform state is only "Quality_Profile/Project" without language, and trying to access idSlice[2] make the program crash.

As this id is automatically created inside the code only when a new sonarqube_qualityprofile_project_association is created, we have no way to simulate these already existing resources with language in the id over tests. Because each new resource that we try to create will already have the id in the new format.

To test that this pr is working we have built the provider locally and use it inside our terraform code.