SSO: Improve Azure AD validation

[mgyongyosi]

This modifies the validation rules for azuread by ensuring the field api_url is empty. There are no changes for the other providers.

[github-actions]

In order to lower resource usage and have a faster runtime, PRs will not run Cloud tests automatically.
To do so, a Grafana Labs employee must trigger the cloud acceptance tests workflow manually.

[julienduchesne]

I think all of these validations should go in a CustomizeDiff function. example:

CreateContext: UpdateSSOSettings,
		ReadContext:   ReadSSOSettings,
		UpdateContext: UpdateSSOSettings,
		DeleteContext: DeleteSSOSettings,
		Importer: &schema.ResourceImporter{
			StateContext: schema.ImportStatePassthroughContext,
		},

		CustomizeDiff: func(ctx context.Context, rd *schema.ResourceDiff, i interface{}) error {
			providerName := rd.Get(providerKey).(string)
			// validation + relevant error message for each failure
		},

Then, you should have more than one test for various failing validations

[dmihai]

@julienduchesne I did a research on how we can use CustomizeDiff to validate the input. The first thing I found was that the CustomizeDiff function is executed after a plan has been generated. This means we cannot fail early in case the input from the terraform config is invalid. I tried to find some other ways to perform conditional validations in terraform plugin sdk v2 but apparently there are none.

For that reason I think we should keep the way we do validations now. Please let me know if I'm missing something, I might have misunderstood your suggestion.

[julienduchesne]

👍 That works. I think the ideal validation scenario would've been to not have a provider_name attribute and instead have a different block for each kind of auth (github { ... } OR gitlab { ... } ...). Then, we could've had ValidateFunc for attributes in each one (terraform validate would then work). Up to you if you want to refactor that way or not, major version is coming soon and the SSO resource is new, so it's the right time to change that.

If not, my one comment then is that I'd like all of these validations to have SSO in their name because they are specific to the SSO resource or be grouped in a helper struct (SSOResourceValidations.bla or something like that)

[dmihai]

👍 That works. I think the ideal validation scenario would've been to not have a provider_name attribute and instead have a different block for each kind of auth (github { ... } OR gitlab { ... } ...). Then, we could've had ValidateFunc for attributes in each one (terraform validate would then work). Up to you if you want to refactor that way or not, major version is coming soon and the SSO resource is new, so it's the right time to change that.

If not, my one comment then is that I'd like all of these validations to have SSO in their name because they are specific to the SSO resource or be grouped in a helper struct (SSOResourceValidations.bla or something like that)

I don't think it's a good idea to change the structure of the sso resource now because some users are already using it. I'll try to group the validators to make the code more clear.

[julienduchesne]