jdamata / terraform-provider-sonarqube

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

feat: add initial sonarqube quality gate usergroup association #122

Closed EvertonSA closed 1 year ago

EvertonSA commented 1 year ago

relates to https://github.com/jdamata/terraform-provider-sonarqube/issues/109

EvertonSA commented 1 year ago

@jdamata I've never programmed in go before, let alone terraform providers. is it possible to add an importer if we want to make it flexible for users and groups? I don't think so, because a user can have the same name as a group.

EvertonSA commented 1 year ago

great, it only works on the latest, lts will not work and enterprise works only for on add_user endpoint. will keep this here for future reference.

jdamata commented 1 year ago

great, it only works on the latest, lts will not work and enterprise works only for on add_user endpoint. will keep this here for future reference.

That's a real shame after all this work you did. So this doesn't work on LTS or enterprise eh.

Should we close this, keep the issue open and link this PR in the issue? That way someday when this API endpoint matures we can reopen this.

jdamata commented 1 year ago

Someone noticed that lts changed to 9.9 overnight: https://github.com/jdamata/terraform-provider-sonarqube/pull/119

So maybe this works on the newer version of LTS? Might be worth trying.

EvertonSA commented 1 year ago

will give it a try

EvertonSA commented 1 year ago

@jdamata thanks, it passed now, but I'm unsure about changing the tag to a different one. let me tag back to 8.9.10-developer and see if it works.

EvertonSA commented 1 year ago

TODO: add documentation

EvertonSA commented 1 year ago

dont know what to do with this sonarqube image.

my understanding is that acceptance-test should run on developer and acceptance-test-lts should run on developer-lts.

it was changed recently here https://github.com/jdamata/terraform-provider-sonarqube/commit/d2e493e756e4518dc9f82a7f05be982eb5727961.

jdamata commented 1 year ago

LGTM. Just some comments about the log messages being more specific to a user/group association to a quality gate and some if conditions that might not be necessary

gilfthde commented 1 year ago

I've submitted an implementation for the same feature because I did not notice this PR. Sorry for that. Maybe we can merge both because mine also includes import and I have a test against the SQ version in it.

jdamata commented 1 year ago

I've submitted an implementation for the same feature because I did not notice this PR. Sorry for that. Maybe we can merge both because mine also includes import and I have a test against the SQ version in it.

Do you mind rebasing your changes after this one goes in? I am interested in getting the import and sonarqube version tests in as well, they are very useful.

I prefer the simplicity of having to only supply gatename and login_name or group_name. Instead of having to provide gatename, type and subject.

gilfthde commented 1 year ago

Do you mind rebasing your changes after this one goes in? I am interested in getting the import and sonarqube version tests in as well, they are very useful.

No problem. Will do so.

EvertonSA commented 1 year ago

sorry for my absence, will update with the comments above

EvertonSA commented 1 year ago

im aware my programming skills suck, the switch case on https://github.com/jdamata/terraform-provider-sonarqube/pull/127/files looks better. due to my lack of internet, and if more changes are needed, I'm ok with someone else taking charge of this

EvertonSA commented 1 year ago

@jdamata @gilfthde ready for review

gilfthde commented 1 year ago

LGTM. @EvertonSA maybe run go fmt on the code to fix some indentions.

jdamata commented 1 year ago

ill run go fmt after merge. Thanks @EvertonSA and @gilfthde for your work on this!

EvertonSA commented 1 year ago

thanks