newrelic / terraform-provider-newrelic

Terraform provider for New Relic
https://registry.terraform.io/providers/newrelic/newrelic/latest/docs
Mozilla Public License 2.0
202 stars 245 forks source link

fix: get the linked account if already linked #2735

Closed chmoder closed 3 weeks ago

chmoder commented 1 month ago

Description

This is my first time working on a "Go" project, terraform provider, and the new relic api. Please review my work and feel free to improve it and/or suggest improvements from me.

Uses client.Cloud.GetLinkedAccounts to get the already linked account if ERR_INVALID_DATA is the response from client.Cloud.CloudLinkAccountWithContext.

resource "newrelic_cloud_gcp_link_account" "example" {
  account_id      = var.nr_account_id
  project_id      = var.project_id
  name            = "chmoder"
}

resource "newrelic_cloud_gcp_integrations" "example" {
  account_id        = var.nr_account_id
  linked_account_id = newrelic_cloud_gcp_link_account.example.id
}

Fixes #2733

Type of change

Please delete options that are not relevant.

Checklist:

Please delete options that are not relevant.

How to test this change?

Please describe how to test your changes. Include any relevant steps in the UI, HCL file(s), commands, etc

Try to link an already linked account using newrelic_cloud_gcp_link_account and reference the output id field in another resource.

CLAassistant commented 1 month ago

CLA assistant check
All committers have signed the CLA.

pranav-new-relic commented 1 month ago

Hi @chmoder, thank you for the contribution. We're currently working on a key release, so we shall be able to get to reviewing this hopefully soon. We'll get to this as soon as we can and shall keep you posted. Thanks!

codecov-commenter commented 1 month ago

Codecov Report

Attention: Patch coverage is 0% with 79 lines in your changes missing coverage. Please review.

Project coverage is 36.50%. Comparing base (92361de) to head (d8d2e6d). Report is 82 commits behind head on main.

Files with missing lines Patch % Lines
...source_newrelic_cloud_aws_govcloud_link_account.go 0.00% 28 Missing :warning:
...elic/resource_newrelic_cloud_azure_link_account.go 0.00% 27 Missing :warning:
...wrelic/resource_newrelic_cloud_gcp_link_account.go 0.00% 24 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2735 +/- ## ========================================== + Coverage 32.82% 36.50% +3.68% ========================================== Files 98 98 Lines 26884 21923 -4961 ========================================== - Hits 8824 8003 -821 + Misses 17902 13756 -4146 - Partials 158 164 +6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

gmanandhar-nr commented 4 weeks ago

Hi @chmoder , thanks for the contribution again! We looked deeper in the issue, and decided "resources" block should only be used to create a new resource, based on terraform design principles and the error message "Error: ERR_INVALID_DATA Validation failed: The account has already been linked." would be the right error to show. If you really must need to use linkedAccountId that was created outside terraform, you can use terraform's graphql provider to query the NerdGraph and get the right linkedAccountId by filtering with account_id and project_id. Please find sample code below:

locals {
  response = jsondecode(data.graphql_query.basic_query.query_response)
  filtered_ids = flatten([
    for item in local.response["data"]["actor"]["account"]["cloud"]["linkedAccounts"] : item.id if item.name == var.name && item.nrAccountId == var.nr_account_id
  ])[0]
}

provider "graphql" {
  url = "https://api.newrelic.com/graphql"
  headers = {
    "Content-Type" = "application/json"
    "API-Key" = "your-api-key"
  }
}

data "graphql_query" "basic_query" {
  query_variables = {}
  query = <<EOF
    query {
  actor {
    user {
      name
    }
    account(id: your-account-id) {
      cloud {
        linkedAccounts {
          id
          name
          nrAccountId
        }
      }
    }
  }
}
    EOF
}

output "filteredId" {
  value = local.filtered_ids
}

You can pass local.filtered_ids to linked_account_id in newrelic_cloud_gcp_integrations resource block. Thanks!

chmoder commented 3 weeks ago

Thank you @gmanandhar-nr, for the review and the sample solution using terraforms graphql provider. Here is my final verison if you are interested:

locals {
  response        = jsondecode(data.graphql_query.account_id_query.query_response)
  linked_accounts = local.response["data"]["actor"]["account"]["cloud"]["linkedAccounts"]

  linked_account_ids = flatten([
    for linked_account in local.linked_accounts :
    linked_account.id if linked_account.name == var.project_id && linked_account.nrAccountId == var.nr_account_id
  ])

  linked_account_id = length(local.linked_account_ids) > 0 ? local.linked_account_ids[0] : null
}

This is a good solution and maybe this is just my lack of experience. But with other terraform resources like an IP address for example, if I create one with terraform apply, and run it again I don't get an error about the ip address already existing. But here we get an error saying the account has already been linked. I think we can close this PR and the issue, but can you please help me understand the difference between these cases?

I may need to try using the API to see if there is a linked_account_id already and if not, then create a new one using newrelic_cloud_gcp_link_account.

gmanandhar-nr commented 3 weeks ago

Hi @chmoder,

I may need to try using the API to see if there is a linked_account_id already and if not, then create a new one using newrelic_cloud_gcp_link_account. this is a great idea!

The reason we wanted to show the error, and not simply override is because someone else in the user's organization may have linked the gcp account and integrated multiple gcp resources, outside of terraform, and not showing the error would mean someone else might overwrite those without their permission.

This is one way of doing it. Another way could be to use terraform import command to import the pre-existing resources. You can read more about it here: https://developer.hashicorp.com/terraform/cli/commands/import https://developer.hashicorp.com/terraform/language/import/generating-configuration

However, the first way should satisfy your use case. As discussed, closing this issue and PR. If you have more questions, feel free to reopen it.