databricks_service_principal_federation_policy

[AntonBarkan]

Changes

#4489

Tests

• make test run locally
• relevant change in docs/ folder
• covered with integration tests in internal/acceptance
• using Go SDK
• using TF Plugin Framework

[AntonBarkan]

@rauchy Would you be able to review this?

[GFernie]

I'm not sure I understand this. If name is optional and can be user-specified then I don't think we can assume it is the ID ("uid" field). Though, Databricks docs say "if specified in a request, must match the value in the request URL", but they must be referring to account ID and SP ID because policy ID is not in the URL and is not known until the policy is created.

https://docs.databricks.com/api/account/serviceprincipalfederationpolicy/create

It seems simpler to just use the uid field, directly, since this is what the API expects for update/delete requests, not necessarily the last part of the name.

https://pkg.go.dev/github.com/databricks/[email protected]/service/oauth2#FederationPolicy

Suggested change

	d.SetId(name[strings.LastIndex(name, "/")+1:])
	d.SetId(spfp.Uid)

[AntonBarkan]

Thank you for taking the time to review my pull request. This part was very confusing to me as well, and I’d like to explain what I’ve understood.

So, if the name is not set, then the generated name looks like:
accounts/482323ec-21a2-4d23-a4bb-4fdacf0ee506/servicePrincipals/2479342822644814/federationPolicies/388f001e-5f0a-42c3-bc0d-5da1a33a3798,
where the last part equals the value of the uid field. and available at the link https://accounts.cloud.databricks.com/api/2.0/accounts/482323ec-21a2-4d23-a4bb-4fdacf0ee506/servicePrincipals/2479342822644814/federationPolicies/388f001e-5f0a-42c3-bc0d-5da1a33a3798

Now let’s consider a example where the user sets the name manually. Any name that does not start with
accounts/482323ec-21a2-4d23-a4bb-4fdacf0ee506/servicePrincipals/2479342822644814/federationPolicies/
will be rejected.

The last part of the name may only contain letters, numbers, and hyphens.

For example, if I create a policy with the name:
accounts/482323ec-21a2-4d23-a4bb-4fdacf0ee506/servicePrincipals/2479342822644814/federationPolicies/fdfsd,
then it will be accessible via:
https://accounts.cloud.databricks.com/api/2.0/accounts/482323ec-21a2-4d23-a4bb-4fdacf0ee506/servicePrincipals/2479342822644814/federationPolicies/fdfsd

Whereas the link:
https://accounts.cloud.databricks.com/api/2.0/accounts/482323ec-21a2-4d23-a4bb-4fdacf0ee506/servicePrincipals/2479342822644814/federationPolicies/4e09f9a3-bb95-4e3d-a47c-9c437803d7cd
will return 404.

[GFernie]

Oh, you are right, I just double checked with the CLI. Thanks for explaining! It's quite strange because Databricks calls this part "policy id" in their API docs, but the examples suggest it is the end part of the name, like you say.

https://docs.databricks.com/api/account/serviceprincipalfederationpolicy/get

[AntonBarkan]

I’ve updated the code so that the UUID is used as the ID and is unique. I’d really appreciate it if you could review it

[github-actions]

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/terraform

Inputs:

• PR number: 4635
• Commit SHA: 9483125d276b60490d9713facaacbb9820005718

Checks will be approved automatically on success.

[GFernie]

As I understand it, the contribution guide asks that the schema is created using common.StructToSchema.

https://github.com/databricks/terraform-provider-databricks/blob/main/CONTRIBUTING.md#adding-a-new-resource

For example:

terraform-provider-databricks/tokens/resource_service_principal_secret.go

Line 27 in c6f6f77

spnSecretSchema := common.StructToSchema(ServicePrincipalSecret{},

This looks like the correct struct to use:

terraform-provider-databricks/internal/service/oauth2_tf/model.go

Line 994 in c6f6f77

type FederationPolicy struct {