jdamata / terraform-provider-sonarqube

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

Support Sonarqube portfolios #143

Closed felixlut closed 1 year ago

felixlut commented 1 year ago

This PR introduces support for Portolios. Worth to note are the following:

Resolves #140

jdamata commented 1 year ago

This is elegantly written and i appreciate you keeping the PR on the smaller side for me to review.

As for the sonarqube:enterprise I spent a fair amount of time poking around to see if it would be an issue for us to use this version of sonarqube but couldn't find anything. Im trying to reach out to their support to see if they would have a problem with it. I think we should go ahead and update the CI to the enterprise docker version and get the tests in a passing. I can always revert the CI change and figure out what to do later if I hear back from sonarsource

felixlut commented 1 year ago

I could implement something like whats done here, to make the tests only run against enterprise editions. Ì looked into it a bit, the system/info endpoint exposes which edition is running (enterprise/developer/community). And the same for the resoruce file here.

The edition could be exposed in the same way the sonarQubeVersion is.

Then the tests can be run on multiple editions with something like this.

jdamata commented 1 year ago

I could implement something like whats done here, to make the tests only run against enterprise editions. Ì looked into it a bit, the system/info endpoint exposes which edition is running (enterprise/developer/community). And the same for the resoruce file here.

The edition could be exposed in the same way the sonarQubeVersion is.

Then the tests can be run on multiple editions with something like this.

This is something the provider does not do currently. I am positive there are existing resources/tests already in this provider that wont work with all versions of sonarqube. (IE: https://github.com/jdamata/terraform-provider-sonarqube/issues/137)

I think this is a great idea. Inevitably when users of this provider run into issues with this resource, being able to return a helpful error of only supported in enterprise edition is going to save me and the user some time.

It would probably make sense to do this in another PR? Otherwise when you change the CI, you'll get stuck trying to fix other resources that are not compatible with a specific version of sonarqube

felixlut commented 1 year ago

I could implement something like whats done here, to make the tests only run against enterprise editions. Ì looked into it a bit, the system/info endpoint exposes which edition is running (enterprise/developer/community). And the same for the resoruce file here. The edition could be exposed in the same way the sonarQubeVersion is. Then the tests can be run on multiple editions with something like this.

This is something the provider does not do currently. I am positive there are existing resources/tests already in this provider that wont work with all versions of sonarqube. (IE: #137)

I think this is a great idea. Inevitably when users of this provider run into issues with this resource, being able to return a helpful error of only supported in enterprise edition is going to save me and the user some time.

It would probably make sense to do this in another PR? Otherwise when you change the CI, you'll get stuck trying to fix other resources that are not compatible with a specific version of sonarqube

Agreed. I've changed the CI on this PR to use the enterprise image. I can look into adding a PR changing the CI to use multiple images.

So basically, the second CI will do the following: