-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
AWSClient.partition and AWSClient.accountid not set on clients using AdRoll's hologram #12704
Closed
joelthompson opened this issue
Mar 14, 2017
· 1 comment
· Fixed by hashicorp/terraform-provider-aws#17
Closed
AWSClient.partition and AWSClient.accountid not set on clients using AdRoll's hologram #12704
joelthompson opened this issue
Mar 14, 2017
· 1 comment
· Fixed by hashicorp/terraform-provider-aws#17
Labels
Comments
joelthompson
added a commit
to joelthompson/terraform
that referenced
this issue
Mar 22, 2017
This fixes hashicorp#12704 by having GetAccountInfo continue on to the GetCallerIdentity method of getting account info if the retrieval of account info via the EC2 instance metadata service fails. Not attempting to add resiliency when skip_requesting_account_id is set to true.
joelthompson
added a commit
to joelthompson/terraform-provider-aws
that referenced
this issue
Jun 15, 2017
This is a port of hashicorp/terraform#12951 into the new repository. Partially fixes hashicorp/terraform#12704 in the case of hologram clients, but doesn't fix the regression when SkipRequestingAccountId is set.
radeksimko
pushed a commit
to hashicorp/terraform-provider-aws
that referenced
this issue
Jun 15, 2017
* Fix handling of AdRoll's hologram clients This is a port of hashicorp/terraform#12951 into the new repository. Partially fixes hashicorp/terraform#12704 in the case of hologram clients, but doesn't fix the regression when SkipRequestingAccountId is set. * Refactoring of tests
bflad
pushed a commit
to hashicorp/aws-sdk-go-base
that referenced
this issue
Feb 18, 2019
* Fix handling of AdRoll's hologram clients This is a port of hashicorp/terraform#12951 into the new repository. Partially fixes hashicorp/terraform#12704 in the case of hologram clients, but doesn't fix the regression when SkipRequestingAccountId is set. * Refactoring of tests
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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Apr 9, 2020
PhillipGameDev
added a commit
to PhillipGameDev/AWS-SDK-GO-BACKEND
that referenced
this issue
Dec 10, 2024
* Fix handling of AdRoll's hologram clients This is a port of hashicorp/terraform#12951 into the new repository. Partially fixes hashicorp/terraform#12704 in the case of hologram clients, but doesn't fix the regression when SkipRequestingAccountId is set. * Refactoring of tests
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Summary
Just a quick summary (though it's not in your template). When running with EC2 credentials, GetCredentials is called, which recognizes the fact that I have something that looks like EC2 instance metadata creds available and injects the EC2RoleProvider into the credential chain. Then, GetAccountInfo is called. It sees that the credential provider is the EC2RoleProvider and tries to get the instance profile ARN out of the metadata service by calling
metadataClient.IAMInfo()
out of the AWS Go SDK. That method merely tries calling the iam/info endpoint, which is an endpoint that Hologram doesn't implement. The result is that metadataClient.IAMInfo returns an error to GetAccountInfo, which then returns the error to Client, which then silently eats the error and skips setting the accountid and partition. This can be seen in the debug logs below where the aws_partition data source is getting set to the empty string and the aws_caller_identity data source raises an error as neither are getting set.The net result of this is that anybody using AdRoll's hologram to get instance credentials locally won't be able to use the aws_caller_identity or aws_partition data sources. We could probably deal with that, but then #11359 changed up a number of resources to depend on the *AWSClient.partition being set to dynamically generate the ARN. The one we're running in to is sns_topic_policy -- trying to destroy an SNS topic policy goes down a code path which dynamically injects *AWSClient.partition into the ARN regex, and when the partition is empty, that regex match fails, which causes the delete to fail. In other words, for anbody who's using Hologram, they'll be able to create an SNS topic policy with Terraform, but not destroy it.
There's an additional subtlety/regression here. The call to GetAccountInfo is guarded by a check to see if SkipRequestingAccountId is set. If so, then the call to GetAccountInfo is skipped and *AWSClient.partition isn't set. This means that if anybody has SkipRequestingAccountId set, they also will see the same behavior -- they'll be able to create SNS topic policies but not destroy them, whereas prior to #11359 being merged, they would be able to do the same. I'd also guess that the whole point of the call to getAccountIdFromSnsTopicArn in the SNS topic provider is precisely to support the use case where SkipRequestingAccountId is set. I suspect that similar regressions will occur with the other resources touched in #11359, but sns_topic_policy is the only one I've examined in detail.
Not sure what the right path is here -- I feel like if the call to
metadataClient.IAMInfo
errors out, the code should just fall through to sts:GetCallerIdentity. However, that addresses our use case, but not the regression where setting SkipRequestingAccountId would still cause this to not be set. I believe the partition can be inferred from the region, so maybe try to infer it from the region?Terraform Version
$ terraform -v
Terraform v0.8.8
Affected Resource(s)
among others.
Terraform Configuration Files
Debug Output
Can provide a full log if you want, doesn't seem necessary here. The relevant bits:
Expected Behavior
In this case, nothing, but the account ID and partition should have been available to me, and TF shouldn't have errored out.
Actual Behavior
TF errored out
Steps to Reproduce
terraform apply
with the above TF on a machine running AdRoll's HologramImportant Factoids
Using AdRoll's hologram for credentials.
References
The text was updated successfully, but these errors were encountered: