Closed tbutler-qontigo closed 1 year ago
Thinking on this a bit more, the SonarQube API isn't great, possibly because they seem to prefer query params rather than bodies in the POST requests to create objects.
api/qualitygates/create
is used to create a quality gate and accepts only the name
parameter.
api/qualitygates/create_condition
is the end point you need to create a single condition
The current implementation of the provider reflects this with sonarqube_qualitygate
and sonarqube_qualitygate_condition
respectively.
In reality, conditions are a child resources of a quality gate and, IMO, the API should reflect that, which unfortunately it does not. A condition cannot exist without being attached to a quality gate so why is it a "top level" resource?
Whilst we cannot change the SonarQube API, we can however refactor the provider to reflect the relationship by changing sonarqube_qualitygate
to accept conditions and removing sonarqube_qualitygate_condition
completely - it serves no useful purpose now.
A resource creation block would now look like this
resource "sonarqube_qualitygate" "default" {
name = "My way"
condition {
error = "1"
id = "AYe-J8gMpq_IgwDfAnKA"
metric = "new_security_rating"
op = "GT"
}
condition {
error = "80"
id = "AYe40uWkpq_IgwDfAktH"
metric = "new_coverage"
op = "LT"
}
etc..
}
thus more accurately modelling the relationship between quality gates and conditions.
This in itself does not solve the problem Sonar have created by automatically adding conditions when a new quality gate is created so we have a few options here:
9.8
and below behaviour effectively by deleting all of the conditions that were auto created at the point in time the sonarqube_qualitygate
resource is created. Doing this, we would not actually need to refactor as described above and could continue with the existing two resource types, with sonarqube_qualtiygate_condition
continuing to be used to add conditions.Assuming we proceed with the refactor though:
conditions
property is optional then we can either a) leave all the auto-created conditions "as is" - they will be recorded in state (or at least they will if https://github.com/jdamata/terraform-provider-sonarqube/pull/157 is merged) though it is a bit weird that they are invisible in the .tf
file itself or b) remove them if none are specified. That's a bit weird too though because why would you want a quality gate with no conditions? (remember the existing conditions resource would be removed)conditions
property is mandatory then we replace the auto-created conditions with whatever ones are supplied in the .tf
block. This is a bit like option 1 in the sense that the conditions are specified by the terraform script and not from the magic creation that SonarQube does at the moment. I'm not sure there is a perfect answer to this but I am inclined towards 3 because:
Thank you for the great write up.
Needing logic to delete/replace a set of auto populated conditions after creating a qualitygate is a little gross. We would need to maintain a list of these populated conditions and make separate calls for each condition.
That being said, I don't think I have a better solution :)
I am leaning towards either option 1 or 3.
Option 1 has the benefit of being backwards compatible and not introducing a breaking change. Users would not need to refactor their terraform when upgrading.
I do agree that Option 3 models the relationship between gate and condition much better.
Is this something you are interested in working on? Ultimately I think option 3 would be the way to go but is significantly more work than option 1. I would be happy to review/accept PRs for either of these options.
Hi @jdamata
Agreed it is nasty - sadly Sonar are taking a very hard line on this - it is "Clean as you code" or nothing and they consider my use case (terraform + sonar) to be unique to me so unwilling to engage in exploring other options :/
I am actually pretty close to a PR for this using option 3 - though it will have to wait until Tuesday now because it is EOD here and a public holiday on Monday :}
Essentially I have to create the quality gate, query the API to get back the conditions that were auto-created and then add/remove/update any that differ from those the user supplied in the .tf
file.
As I have been working on it, I have had option 1 in the back of my mind - it would be relatively easy to pull this apart again and make it backwards compatible All I would need to do is delete the auto-created conditions and everything would "just work" as before.
BUT
I am really not a fan of the Sonar API - I find it pretty hard to read and navigate and reason about so if we can abstract some of that away then all the better.
Doing option 3 also means I've had to implement Update for when people want to update conditions on the quality gate. More complex for sure...but essentially done now.
I will submit a PR for Option 3 on Tuesday and if you really don't like it then I'll have a little cry and then submit an Option 1 version.
thanks
@jdamata - a little later than advertised, I have created a PR for this issue.
There are other issues that affect v10 compatibility - a customKey
property that has been removed - I will look at them separately.
The simplest way to upgrade Quality Gates from v0.15.x to 0.16+ is to:
quality_gate_condition
resources from your .tf
filesterraform apply
to have them removed from state and from Sonarcondition
blocks to each of your quality_gate
resourcesterraform apply
to have the conditions added back to Sonar and into terraform state in the right place
Hi
SonarQube made a change in the
9.9
release (SONAR-17818) which automatically adds their "clean as you code" conditions to any quality gate that you create. This basically makes it impossible to set different values using the terraform provider.This is what we have been doing:
Which will no longer work because the
new_coverage
metric is already added when the gate is created but terraform state doesn't know about it - so you get this:I am not a GO developer but I played around with the provider and it seems like
resource_sonarqube_qualitygate.go
does not populate theconditions
array when a gate is created. It looks like it should but this seems to be a bug: https://github.com/jdamata/terraform-provider-sonarqube/blob/e230cdf842cf6a0beaf46419903d8d0ed8e8ef52/sonarqube/resource_sonarqube_qualitygate.go#L182 at this point I guess the pointer toresp.Body
is at the end of the body soconditions
always remains empty.Anyway I messed around a bit and fixed that to populate from
qualityGateReadResponse.Conditions
and now tf state will contain e.g.when the gate is created. This still doesn't help with updating the conditions however since any new ones you create are in a completely different resource - e.g.:
It feels like the
sonarqube_qualitygate
resource needs to handle the conditions in all cases andsonarqube_qualitygate_condition
should be removed as it would be unnecessay at this point. It is essentially useless for updating any of the conditions that SonarQube adds automatically now anyway.And migrating to a new version of
sonarqube_qualitygate
could be a bit painful/a breaking change.It feels like the sort of change that would require some discussion @jdamata rather than someone simply pushing a PR and hoping it is accepted :)
And of course there may be other solutions available....such as potentially
sonarqube_qualitygate_condition
updating the child resource within thesonarqube_qualitygate
- though that may be a bit hacky even if it is possible?