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

provider/aws: Adds arn as an output for aws_elb #5411

Closed
wants to merge 1 commit into from

Conversation

stack72
Copy link
Contributor

@stack72 stack72 commented Mar 2, 2016

As requested in #5406

Simple addition of ARN for the ELB resource

iamconn := meta.(*AWSClient).iamconn
region := meta.(*AWSClient).region
// An zero value GetUserInput{} defers to the currently logged in user
resp, err := iamconn.GetUser(&iam.GetUserInput{})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have historically had issues with this API call - it returns an error when authenticating via an EC2 Instance Profile (since there is no "current user" in that case). My suspicion is that this could break aws_elb for users authenticating in that way.

I see we have quite a few uses in the provider with this pattern:

builtin/providers/aws/resource_aws_db_instance.go:      resp, err := iamconn.GetUser(&iam.GetUserInput{})
builtin/providers/aws/resource_aws_db_parameter_group.go:       resp, err := iamconn.GetUser(&iam.GetUserInput{})
builtin/providers/aws/resource_aws_db_security_group.go:        resp, err := iamconn.GetUser(&iam.GetUserInput{})
builtin/providers/aws/resource_aws_db_subnet_group.go:  resp, err := iamconn.GetUser(&iam.GetUserInput{})
builtin/providers/aws/resource_aws_elasticache_cluster.go:      resp, err := iamconn.GetUser(&iam.GetUserInput{})

For ARN building I think we need is a shared helper that calls GetUser but can also be overridden by an explicit AWS_ACCOUNT_ID env var - and we include that info in the error to let proper ARNs be built for users authenticating w/ instance profiles.

@radeksimko
Copy link
Member

Agreed with @phinze , just adding a few notes:

  1. If you are absolutely sure we need Account ID here, I would suggest holding off until provider/aws: Allow account ID checks on EC2 instances & w/ federated accounts #5030
    Then we can expose the AWS Account ID via a simple function and/or save it into AWSClient object (which is passed into each resource as meta).
  2. I think it's worth questioning and documenting the use-cases for all these ARNs. If AWS doesn't give us ARN for the resource from the API, it may be intention or just a bug. Specifically I have never seen "ELB ARN" being used anywhere - all resources AFAIK expect just ELB name as a reference.
  3. If there's a field that accepts both the name and ARN, I think we can get around that without having to calculate the ARN and asking for account ID due to the way APIs usually work. If user has passed name, ARN will be returned. The solution would be to parse the name back from ARN in Read() if the user has actually used name and not ARN. See
    // Save Lambda function name in the same format
    if strings.HasPrefix(d.Get("function_name").(string), "arn:aws:lambda:") {
    // Strip qualifier off
    trimmedArn := strings.TrimSuffix(statement.Resource, ":"+qualifier)
    d.Set("function_name", trimmedArn)
    } else {
    functionName, err := getFunctionNameFromLambdaArn(statement.Resource)
    if err != nil {
    return err
    }
    d.Set("function_name", functionName)
    }
    for example.

@stack72
Copy link
Contributor Author

stack72 commented May 5, 2016

Closing this off - the original requestor hasn't asked back about it

@stack72 stack72 closed this May 5, 2016
@stack72 stack72 deleted the f-aws-elb-arn branch May 5, 2016 16:55
@ghost
Copy link

ghost commented Apr 26, 2020

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 ghost locked and limited conversation to collaborators Apr 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants