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

fix: get the linked account if already linked #2735

Closed
wants to merge 5 commits into from

Conversation

chmoder
Copy link

@chmoder chmoder commented Aug 22, 2024

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.

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

Please delete options that are not relevant.

  • My commit message follows conventional commits
  • My code is formatted to Go standards
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes. Go here for instructions on running tests locally.

How to test this change?

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

  • Step 1

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

@CLAassistant
Copy link

CLAassistant commented Aug 23, 2024

CLA assistant check
All committers have signed the CLA.

@pranav-new-relic
Copy link
Member

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
Copy link

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 ⚠️
...elic/resource_newrelic_cloud_azure_link_account.go 0.00% 27 Missing ⚠️
...wrelic/resource_newrelic_cloud_gcp_link_account.go 0.00% 24 Missing ⚠️
Additional details and impacted files
@@            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     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gmanandhar-nr
Copy link
Member

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
Copy link
Author

chmoder commented Sep 8, 2024

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants