Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

david-ortiz-saez
Copy link
Contributor

@david-ortiz-saez david-ortiz-saez commented Oct 31, 2023

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/[email protected]/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/[email protected]/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/[email protected]/helper/schema/grpc_provider.go:616 +0x497
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ReadResource(0xc00012d220, {0xdb3740?, 0xc000405950?}, 0xc00068f2c0)
	github.com/hashicorp/[email protected]/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/[email protected]/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/[email protected]/server.go:1337 +0xde3
google.golang.org/grpc.(*Server).handleStream(0xc0003f0000, {0xdb64a0, 0xc00048c1a0}, 0xc0001905a0, 0x0)
	google.golang.org/[email protected]/server.go:1714 +0xa1b
google.golang.org/grpc.(*Server).serveStreams.func1.1()
	google.golang.org/[email protected]/server.go:959 +0x98
created by google.golang.org/grpc.(*Server).serveStreams.func1
	google.golang.org/[email protected]/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
Copy link
Contributor Author

@jdamata kindly review :)

@freeranger
Copy link
Collaborator

@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
Copy link
Collaborator

Also there is no v0.62.3 :}

@david-ortiz-saez
Copy link
Contributor Author

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

@david-ortiz-saez david-ortiz-saez changed the title fix: retrocompatibility break when v0.62.3 was released fix: retrocompatibility break when v0.16.3 was released Oct 31, 2023
@freeranger
Copy link
Collaborator

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

@jdamata jdamata merged commit cd71997 into jdamata:master Dec 18, 2023
11 checks passed
@jdamata jdamata linked an issue Dec 18, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provider crashes after upgrade from 0.16.2 to 0.16.3
3 participants