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

datadog_integration_aws_account import docs are incorrect #2733

Open
jrobison-sb opened this issue Dec 19, 2024 · 12 comments
Open

datadog_integration_aws_account import docs are incorrect #2733

jrobison-sb opened this issue Dec 19, 2024 · 12 comments
Labels

Comments

@jrobison-sb
Copy link

Datadog Terraform Provider Version

v3.50.0

Terraform Version

v1.10.0

What resources or data sources are affected?

  • datadog_integration_aws_account
  • datadog_integration_aws

Terraform Configuration Files

resource "datadog_integration_aws" "old" {
  ...
}

resource "datadog_integration_aws_account" "new" {
  ...
}

Relevant debug or panic output

No response

Expected Behavior

We need to migrate from the old resource seen above which we have been using for a long time, to the new resource seen above which is newly released in the 3.50.0 provider. The import docs state:

AWS Account Config ID can be retrieved by using the List all AWS integrations endpoint and querying by AWS Account ID, or by accessing the id field of an existing datadog_integration_aws resource.

So I expect that I could import the new resource using the id field of an existing datadog_integration_aws resource. Eg. the ID seen here:

$ terraform state show datadog_integration_aws.old | grep -w id
    id                                   = "1234567890:qa-DatadogIntegration"

In the above example, 1234567890 represents our AWS account number, and qa-DatadogIntegration represents the AWS role that the integration uses.

Actual Behavior

I can't import the new resource using the id field of an existing datadog_integration_aws resource.

$ terraform import datadog_integration_aws_account.new "1234567890:qa-DatadogIntegration"
...
│ Error: error retrieving IntegrationAwsAccount
│ 
│ 400 Bad Request: {"errors":[{"status":"400","title":"Invalid Parameter","detail":"invalid parameter \"aws_account_config_id\" in \"path\";
│ expected type \"uuid\""}]}

I can only get the import to work if I use a UUID which is entirely unknown to the old datadog_integration_aws resource type. I have to get the UUID from the API.

AWS_ACCOUNT_NUMBER="1234567890"
DATADOG_API_KEY="foo"
DATADOG_APP_KEY="bar"
INTEGRATION_UUID="$(curl -L -sS https://api.datadoghq.com/api/v2/integration/aws/accounts -H "Content-Type: application/json" -H "DD-API-KEY: $DATADOG_API_KEY" -H "DD-APPLICATION-KEY: $DATADOG_APP_KEY" | jq -r --arg AWS_ACCOUNT_NUMBER $AWS_ACCOUNT_NUMBER '.data[] | select(.attributes.aws_account_id == $AWS_ACCOUNT_NUMBER) | .id')"
terraform import datadog_integration_aws_account.new "$INTEGRATION_UUID"
...
Import successful!

Steps to Reproduce

Try to migrate from a deprecated datadog_integration_aws resource to a new datadog_integration_aws_account resource by accessing the id field of an existing datadog_integration_aws resource for the import command.

Important Factoids

No response

References

#2682

@synesisinc
Copy link

synesisinc commented Dec 19, 2024

I just ran into this issue as well, used the API method to retrieve the "proper" ID. Sadly, once the resource was imported, I'm now running into a 500 error response when trying to update the integration resource :(

@jrobison-sb
Copy link
Author

@synesisinc can you describe and/or paste the modifications and the errors that you're getting?

The reason I ask is because I'll want to hold off on doing this migration if the resource is unmanageable after import. Thanks.

@synesisinc
Copy link

synesisinc commented Dec 19, 2024

@jrobison-sb sure thing - when I imported, it wanted to change a few things, I kept getting a 500 error response, so I got it down to the smallest change set I could, and still got the error. Here is the output of my TF plan:

Terraform will perform the following actions:

  # datadog_integration_aws_account.<scrubbed> will be updated in-place
  ~ resource "datadog_integration_aws_account" "<scrubbed>" {
        id             = "<scrubbed>"
        # (3 unchanged attributes hidden)

      ~ metrics_config {
            # (4 unchanged attributes hidden)

          ~ namespace_filters {
              ~ exclude_only = [
                  - "AWS/ElasticMapReduce",
                    "AWS/SQS",
                  + "AWS/ElasticMapReduce",
                ]
            }
        }

        # (5 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

and here is the result of the TF apply:

datadog_integration_aws_account.<scrubbed>: Modifying... [id=<scrubbed>]
datadog_integration_aws_account.<scrubbed>: Still modifying... [id=<scrubbed>, 10s elapsed]
datadog_integration_aws_account.<scrubbed>: Still modifying... [id=<scrubbed>, 20s elapsed]
╷
│ Error: error retrieving IntegrationAwsAccount
│ 
│   with datadog_integration_aws_account.<scrubbed>,
│   on dd-integrations.tf line 12, in resource "datadog_integration_aws_account" "<scrubbed>":
│   12: resource "datadog_integration_aws_account" "<scrubbed>" {
│ 
│ 500 Internal Server Error: {"errors":[{"status":"500","title":"Internal error","detail":"Please try again"}]}
╵

@jrobison-sb
Copy link
Author

@synesisinc thanks.

Coincidentally I got a similar diff where it wanted to re-order the metrics_config.namespace_filters.exclude_only list, and I was able to get past it by re-ordering the elements in the list in my HCL. Eg:

exclude_only = [
  "AWS/SQS",
  "AWS/ElasticMapReduce",
]

After I re-ordered the items in my HCL, a subsequent terraform plan returned zero diffs.

Though I don't know if the 500 error is a result of the API getting confused by item re-ordering or if something else is going on.

@ktmq
Copy link
Contributor

ktmq commented Dec 19, 2024

Hey all, first of all, thanks for trying out the new provider version and this new resource! We really appreciate your feedback. I'll definitely correct the import docs issue about the resource ID from an existing datadog_integration_aws resource, that's my mistake. I can also look into adding a datasource for retrieving the UUID via the AWS Account ID.

I'm also investigating the 500 API errors you received. If you're able to create support ticket (link) and include your Datadog Organization name, that will help our investigation.

@synesisinc
Copy link

@jrobison-sb yeah good point - did you try applying that order change and get the 500 as well? Would be good to get confirmation that we're seeing the same issue. I could also modify the order in my code to remove that portion of the diff, and then try a different change to the resource to see if the error is limited to that exclusion list.

@synesisinc
Copy link

synesisinc commented Dec 19, 2024

@ktmq thanks, will do! Felt a little bad hijacking this thread with that issue, but decided it could be useful feedback at least for the OP, in case he wanted to hold off upgrading :)

The link you provided goes to DD ticketing, do you prefer that over an issue here in GitHub? or both?

@jrobison-sb
Copy link
Author

@synesisinc

did you try applying that order change and get the 500 as well?

I didn't get that far. I tested the terraform import using the UUID and got to a zero-diff terraform plan for the new resource, but didn't get as far as doing the terraform state rm on the old resource or doing any subsequent terraform apply. Then I noticed your reply about the 500 error while I was waiting for my internal PR to get approved.

I would be curious to know if a change other than simply reordering the exclude_only list would work though.

@synesisinc
Copy link

@ktmq request submitted, #1973571

@synesisinc
Copy link

@jrobison-sb @ktmq I did some additional testing, more bad news :(

I swapped the order of the metrics_config.namespace_filters.exclude_only filter to get a no-op like you did.

Then, to test a different area of the config, I changed the value of resources_config.cloud_security_posture_management_collection from true to false - and it applied successfully - yay! So I figured the issue was limited to the metrics_config schema block.

Finally, I reverted my change to set resources_config.cloud_security_posture_management_collection back to true, and once again ran into the API error response :( I re-tried 5 more times, with the same error response (in case it was a transient/timing issue).

So, the issue seems more nuanced, allowing me to change an attribute in one direction, but not the other.

@jrobison-sb
Copy link
Author

jrobison-sb commented Dec 19, 2024

@synesisinc thanks for the additional info.

@ktmq initially this GitHub issue only addressed the docs, but if 500 errors are in play when modifying the resource going forward, I'd probably prefer to wait until those are resolved before we start using the new resource in production.

Are these errors the kind of thing where you might expect to push a bugfix in a subsequent provider version in the coming days/weeks? Would you be able to reply here when those errors are resolved?

Thanks.

@ktmq
Copy link
Contributor

ktmq commented Dec 20, 2024

@synesisinc @jrobison-sb Thank you both for the additional details and investigation! 🙏

I've merged a fix for the docs issue, which will go out with our next release in January.

Are these errors the kind of thing where you might expect to push a bugfix in a subsequent provider version in the coming days/weeks? Would you be able to reply here when those errors are resolved?

Yes, absolutely! We're targeting first week of January to push the fix, so I'll reply with an update at that time.

Also:

The link you provided goes to DD ticketing, do you prefer that over an issue here in GitHub? or both?

A DD support ticket is helpful for us if we need to gather more information about your org or specific configuration.

Coincidentally I got a similar diff where it wanted to re-order the metrics_config.namespace_filters.exclude_only list, and I was able to get past it by re-ordering the elements in the list in my HCL.

I'll also look into this and see if we can avoid any diff on ordering changes.

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

No branches or pull requests

3 participants