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

providers/aws: Provide source security group id for ELBs #3780

Merged
merged 4 commits into from
Nov 10, 2015

Conversation

catsby
Copy link
Contributor

@catsby catsby commented Nov 5, 2015

The source_security_group returned for an ELB is the name of the security group... the API only returns the name to us.

You cannot use the Security Group name as a reference in the AWS API for security group ingress unless you're in EC2 Classic, or the default VPC. So, if you're trying to ingress from this ELB, and you are not in classic or default VPC, you will hit error(s) (see #2420)

This PR looks up the SG Id and adds source_security_group_id as an attribute, if found successfully

Remaining:

  • Docs
  • Tests to verify

@catsby
Copy link
Contributor Author

catsby commented Nov 6, 2015

Updated with Docs and Test

}
sgId, err := sourceSGIdByName(meta, *lb.SourceSecurityGroup.GroupName, elbVpc)
if err != nil {
log.Printf("[WARN] Error looking up ELB Security Group ID: %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to a Q I asked elsewhere today - if there was an error, here, isn't that a big enough deal to stop? Perhaps the answer is "no" but I figure it's worth considering.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I upgraded it to an error in 7152674

@phinze
Copy link
Contributor

phinze commented Nov 10, 2015

LGTM generally! One inline question.

@phinze
Copy link
Contributor

phinze commented Nov 10, 2015

🚢

catsby added a commit that referenced this pull request Nov 10, 2015
providers/aws: Provide source security group id for ELBs
@catsby catsby merged commit bea8e0b into master Nov 10, 2015
@catsby catsby deleted the b-aws-elb-source-sg-id branch November 10, 2015 22:44
@ghost
Copy link

ghost commented Apr 30, 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 30, 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.

2 participants