Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Application Insights API Key management #2547

Closed
BzSpi opened this issue Dec 19, 2018 · 9 comments
Closed

Implement Application Insights API Key management #2547

BzSpi opened this issue Dec 19, 2018 · 9 comments

Comments

@BzSpi
Copy link
Contributor

BzSpi commented Dec 19, 2018

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

We should have the availability to create an API Key in Terraform.

New or Affected Resource(s)

  • azurerm_application_insights_api_key

Potential Terraform Configuration

resource "azurerm_application_insights" "my_application_insights" {
  name = "my-application-insights"

  location            = "${var.location}"
  resource_group_name = "${var.resource_group_name}"

  application_type = "Other"
}

resource "azurerm_application_insights_api_key" "my_api_key" {
  name = "my-api-key"

  application_insights_id = "${azurerm_application_insights.my_application_insights.id}"

  resource_group_name = "${var.resource_group_name}"

  read_telemetry = false
  write_annotations = false
  authenticate_sdk_control_channel = false
}

output "my_api_key_id" {
  value = "${azurerm_application_insights_api_key.my_api_key.id}"
}

output "my_api_key_api_key" {
  value = "${azurerm_application_insights_api_key.my_api_key.apikey}"
}

output "my_api_key_name" {
  value = "${azurerm_application_insights_api_key.my_api_key.name}"
}

output "my_api_key_linked_read_properties" {
  value = "${azurerm_application_insights_api_key.my_api_key.linked_read_properties}"
}

output "my_api_key_linked_write_properties" {
  value = "${azurerm_application_insights_api_key.my_api_key.linked_write_properties}"
}```
@pdecat
Copy link
Contributor

pdecat commented Dec 20, 2018

Working on a PR right now.

@pdecat
Copy link
Contributor

pdecat commented Dec 20, 2018

After studying the Azure App Insights API and while implementing the feature, I reckon it would be more natural and future ready to expose raw linked_read_properties and linked_write_properties fields:

# Read telemetry example
resource "azurerm_application_insights_api_key" "read_telemetry" {
  name = "my-api-key-read-telemetry"

  application_insights_name = "${azurerm_application_insights.my_application_insights.name}"

  resource_group_name = "${var.resource_group_name}"

  linked_read_properties  = ["aggregate", "api", "draft", "extendqueries", "search"]
  linked_write_properties = []
}
# Write annotations example
resource "azurerm_application_insights_api_key" "write_annotations" {
  name = "my-api-key-write-annotations"

  application_insights_name = "${azurerm_application_insights.my_application_insights.name}"

  resource_group_name = "${var.resource_group_name}"

  linked_read_properties  = []
  linked_write_properties = ["annotations"]
}
# Authenticate SDK control channel
resource "azurerm_application_insights_api_key" "authenticate_sdk_control_channel" {
  name = "my-api-key-authenticate"

  application_insights_name = "${azurerm_application_insights.my_application_insights.name}"

  resource_group_name = "${var.resource_group_name}"

  linked_read_properties  = ["agentconfig"]
  linked_write_properties = []
}
# Full example (read telemetry, write annotations & authenticate SDK control channel)
resource "azurerm_application_insights_api_key" "full" {
  name = "my-api-key-full"

  application_insights_id = "${azurerm_application_insights.my_application_insights.id}"

  resource_group_name = "${var.resource_group_name}"

  linked_read_properties  = ["agentconfig", "aggregate", "api", "draft", "extendqueries", "search"]
  linked_write_properties = ["annotations"]
}

This avoids to hard code combinations that may change in the future.

@tombuildsstuff
Copy link
Contributor

@pdecat thanks for taking a look into this :)

Whilst it's a deviation from the API - I'd suggest the field names would be better as read_permissions and write_permissions here, since that'd be clearer - what do you think? In either instance we can add validation to these fields to ensure the casing is correct (as we're doing for all new properties) for these values

@pdecat
Copy link
Contributor

pdecat commented Dec 20, 2018

Submitted #2556

@BzSpi
Copy link
Contributor Author

BzSpi commented Dec 21, 2018

The PR works well. Thanks @pdecat !

One thing that bothers me is that if I only ask for the api permission, I have a diff on the next apply since Azure automatically sets the permissions aggregate, draft, extendqueries and search.
If I automatically set the permissions ["aggregate", "api", "draft", "extendqueries", "search"] I don't have any issue, even if I change the list order.

I haven't found any documentation on this subject.

My code

resource "azurerm_application_insights_api_key" "my_api_key" {
  name = "my-api-key"

  application_insights_id = "${azurerm_application_insights.appinsights_functions.id}"

  read_permissions  = ["api"]
}

First apply

Terraform will perform the following actions:

  + azurerm_application_insights_api_key.my_api_key
      id:                          <computed>
      api_key:                     <computed>
      application_insights_id:     "xxxxxxxxxxxxxxxxxxxxxx"
      name:                        "my-api-key"
      read_permissions.#:          "1"
      read_permissions.2902841359: "api"

Second apply

-/+ azurerm_application_insights_api_key.my_api_key (new resource required)
      id:                          "xxxxxxxxxxxxxxxxxxxxxx" => <computed> (forces new resource)
      api_key:                     <sensitive> => <computed> (attribute changed)
      application_insights_id:     "xxxxxxxxxxxxxxxxxxxxxx" => "xxxxxxxxxxxxxxxxxxxxxx"
      name:                        "my-api-key" => "my-api-key"
      read_permissions.#:          "5" => "1" (forces new resource)
      read_permissions.1182570132: "draft" => "" (forces new resource)
      read_permissions.2421119739: "extendqueries" => "" (forces new resource)
      read_permissions.2902841359: "api" => "api"
      read_permissions.3035683751: "search" => "" (forces new resource)
      read_permissions.3078179327: "aggregate" => "" (forces new resource)

@tombuildsstuff Any idea on how to handle this ?

@pdecat
Copy link
Contributor

pdecat commented Dec 21, 2018

Maybe with a DiffSuppressFunc specific to this known case?

The supported actions for Application Insights seem to be listed at https://docs.microsoft.com/en-us/azure/role-based-access-control/resource-provider-operations#microsoftinsights

And here's an example from the REST API specification: https://github.com/Azure/azure-rest-api-specs/blob/master/specification/applicationinsights/resource-manager/Microsoft.Insights/stable/2015-05-01/examples/APIKeysGet.json

@tombuildsstuff
Copy link
Contributor

@BzSpi are those permissions still present if you apply the changes? If not, we should just be able to get the Create method to call the Update method to ensure the permissions are valid

@pdecat
Copy link
Contributor

pdecat commented Dec 22, 2018

There is no Update method on API keys (hence the "forces new resource"): https://github.com/Azure/azure-sdk-for-go/blob/master/services/appinsights/mgmt/2015-05-01/insights/apikeys.go#L29

It seems the Azure API is implicitly translating a request for read permission api as also having aggregate, draft, extendqueries and search permissions.

@ghost
Copy link

ghost commented Mar 5, 2019

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 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants