New Resource: azurerm_kusto_database_principal

[jrauschenbusch]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

Description

Currently there is no out-of-the-box solution to manage Kusto database principals within terraform. I suggest to add a new resource which enables this feature.

New or Affected Resource(s)

• azurerm_kusto_database_principal

Potential Terraform Configuration

resource "azurerm_kusto_database_principal" "test_principal" {
  name = "Kusto"  # Database principal name
  resource_group_name = "${azurerm_resource_group.resource_group.name}"
  cluster_name = "${azurerm_kusto_cluster.cluster.name}"
  database_name = "${azurerm_kusto_cluster_database.database.name}"

  type = "Group" # Allowed: App, Group, User
  role = "Viewer" # Allowed: Admin, Ingestor, Monitor, UnrestrictedViewers, User, Viewer
  fqn = "aadgroup=some_guid" # Database principal full qualified name
  email = "kusto@microsoft.com" # (Optional) Database principal email if exists.
  appId = "" # (Optional) relevant only for application principal type
}

References

• https://github.com/Azure/azure-sdk-for-go/blob/master/services/kusto/mgmt/2019-05-15/kusto/models.go#L843
• https://github.com/Azure/azure-sdk-for-go/blob/master/services/kusto/mgmt/2019-05-15/kusto/kustoapi/interfaces.go#L44
• https://github.com/Azure/azure-rest-api-specs/blob/master/specification/azure-kusto/resource-manager/Microsoft.Kusto/stable/2019-05-15/examples/KustoDatabaseAddPrincipals.json
• New Resource: Azure Data Explorer (kusto) #3856

[jrauschenbusch]

After a first look at the Azure SDK for Go implementation, I'm more into an implementation that adds principals in the context of the resource azure_kusto_database:

resource "azurerm_kusto_database" "database" {
  name                = "my-kusto-database"
  resource_group_name = "my-kusto-rg"
  location            = "northeurope"
  cluster_name        = azurerm_kusto_cluster.cluster.name

  soft_delete_period = "P365D" # optional
  hot_cache_period   = "P31D" # optional

  permissions = [{
      name  = "some_app_principal"
      role = "Admin"
      type = "App"
      app_id = "some_guid_app_id"
      fqn = "aadapp=some_guid_app_id"
  }]
}

@r0bnet Can you maybe give some feedback?

[r0bnet]

Hey @jrauschenbusch,
I'd rather prefer the option that @tracypholmes presented since i had some "bad" experiences with inline resources. That's why i would always choose own resources over inline stuff. It might seem to be a bit overhead because you need to specify name, resource_group_name, cluster_name and database_name but that shouldn't be a problem imho.

[jrauschenbusch]

Ok,

which presented option of @tracypholmes do you mean?

I already started with the implementation of the dedicated resource based principal approach. A first insight was that it's not an dedicated principal which shall created but more a permission assignment which is applied for an already created/existing principal. Hence i think it should be better called permission instead of principal. The Azure portal also calls it permission. A second insight was that it's really hard to solve some terraform resource specific aspects as there is no REST resource for a single permission entity. There is only a list resource. One has always to handle with an array of kusto.DatabasePrincipals. As there is no specific ID there is also some need for an artificial id.

The following 3 API methods must be sufficient to solve all aspects:

client.AddPrincipals(ctx, resourceGroup, clusterName, databaseName, databasePrincipalsToAdd) # array with single element?

client.RemovePrincipals(ctx, resourceGroup, clusterName, databaseName, principalsToRemove)  # array with single element?

client.ListPrincipals(ctx, resourceGroup, clusterName, databaseName) # to be filtered for the Read part

But maybe it's easier to solve than i think 🤷‍♂

[r0bnet]

Sorry, it was a mistake by me. I'm in favor of your first implementation to have a separate resource and not to add it to the database resource itself.

Regarding the implementation you're probably right. You probably have to remove and set them new each time you update the resource (if the permissions changed). You might have no choice but to inline it. Forgot about this implementation detail you mentioned unfortunately.

[jrauschenbusch]

I've created a ticket on the azure-rest-api-specs repo to clarify the question why the API for permissions is not resource-based. Maybe there will be some feedback which helps to define which approach fits best.

If the API stays as it is i would prefer the inlined design, as currently a permission is only uniquely identifiable via multiple properties (subscription, resourceGroup, kusto_cluster, kusto_database, role & fqn).

Especially the required combination of role and fqn is a problem. In a resource-based approach using the current API i would have to define an artificial id /subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Kusto/clusters/{clusterName}/databases/{database}/Fqns/{fqn}/Roles/{role}
to be able to restore the relevant attributes for filtering the permission from the list of permissions during a read or delete.

[jrauschenbusch]

Hey @r0bnet

Although i think the Azure Kusto API for Database Permissions will not change that fast (or if ever) i tried to create an initial resource-based implementation:
https://github.com/jrauschenbusch/terraform-provider-azurerm/blob/kusto-database-principal/azurerm/resource_arm_kusto_database_principal.go

It's working. I've used the artificial id pattern as described in last post. Not that optimal, but i didn't found another way to solve the unique id problem. Maybe you can give some feedback about the implementation details.

Interestingly enough, my API design questions regarding the list of required attributes in the azure-rest-api-specs repo is already implemented in some way, as correct values are only required for the FQN and Role attributes. Although all other values must be present in the HTTP request, they are apparently derived from the FQN in the Azure backend.

[jackbatzner]

@jrauschenbusch / @r0bnet - Are either of you working on this? I have a use case for this resource and can try implementing it if you haven't already.

[jrauschenbusch]

@Brunhil I started with a resource implementation (see my last posted link) But as there is no dedicated Kusto principal resource on the Azure API it’s hard to fit it to the concept of a terraform resource. The way with an artificial namespace is imho a little bit hacky. After having a first draft of a dedicated resource i think it would be better to use an embedded a principal attribute inside the kusto_database resource to be able to use the DB namespace to store the state.

Regarding your question: I‘ll proceed on this after my honeymoon. Cheers from Maui 😊

[jrauschenbusch]

As promised here the initial kusto_database based implementation: jrauschenbusch@dbf182d

Usage:

resource "azurerm_kusto_database" "azurerm_kusto_test_db" {
  name                = "azurerm-kusto-test-db"
  resource_group_name = "azurerm-kusto-test"
  location            = "west-us"
  cluster_name        = "azurerm-kusto-test-cluster"

  permission {
    role = "Ingestor"
    fqn  = "aaduser=37d4d51e-64ca-4aab-b3c4-111671e5ce31"
  }

  permission {
    role = "Viewer"
    fqn  = "aaduser=37d4d51e-64ca-4aab-b3c4-111671e5ce31"
  }
}

As the implementation is based on the database resource, the diff of principalsToAdd and principalsToDelete has to be implemented in the database provider. Using a dedicated kusto principal resource, this would be totally handled by terraform.

Maybe @Brunhil and @r0bnet can have a final look over both approaches to vote for the best of both:

• Dedicated principal resource (required artificial resource id, which could lead to collisions in the future)

or

• Database-related permissions (required permissions sync, )

[mbfrahry]

Hey @jrauschenbusch, I'd like to throw my hat into this as well and suggest going with a separate resource even though the sdk/api isn't a perfect match for the Terraform model. We've done this type of setup before with other resources recently (see iothub_route).

[mbfrahry]

Hey @jrauschenbusch, how're we looking for this resource? I've got some time to work on it if you wanted an extra pair of hands

[jrauschenbusch]

Hey @mbfrahry,

Thank you for driving this topic, and finally to a mergable state. Unfortunately, I did not have the time to continue. But I'm really glad that maybe it will be released with v1.40.0.

[tombuildsstuff]

Fixed via #5242 which will ship as a part of v1.40 in the near future

[ghost]

This has been released in version 1.40.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 1.40.0"
}
# ... other configuration ...

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!