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

elb_application_lb with empty security groups list behaves inconsistently on create/update #28

Closed
briantist opened this issue Apr 7, 2020 · 8 comments · Fixed by #971
Labels
affects_2.10 bug This issue/PR relates to a bug module module plugins plugin (any type) waiting_on_contributor Needs help. Feel free to engage to get things unblocked

Comments

@briantist
Copy link
Contributor

Resubmitted from ansible-collections/amazon.aws#10

SUMMARY

elb_application_lb requires the security_groups option when state=present as explained in the docs (although it also says that the default is [] which seems useless since it won't accept the option being omitted).

When creating a new ALB and supplying security_groups: [] explicitly, the ALB is created successfully with the VPC default SG.

Running the same task again will fail with the error that the security_groups option is missing,

I'm not sure if this is reproducible outside of a VPC since I'm not sure there is such a thing as a default SG in that case.

ISSUE TYPE
  • Bug Report
COMPONENT NAME

elb_application_lb

ANSIBLE VERSION

2.9.6

CONFIGURATION
OS / ENVIRONMENT
STEPS TO REPRODUCE
    - elb_application_lb:
        region: "us-east-1"
        name: "repro-delete"
        state: "present"
        subnets: "{{ my_subnets }}"
        listeners:
          - Protocol: HTTP
            Port: 80
            DefaultActions:
              - Type: forward
                TargetGroupName: repro-group-us-east-1a
        scheme: internal
        security_groups: []
        wait: yes
      register: alb
      loop: [1, 2]
EXPECTED RESULTS

ALB is created, then second run is ok.

(an acceptable result might also be that the first run fails with an invalid option value, but that does preclude the possibility of using a "default" SG)

ACTUAL RESULTS

Second run fails.

fatal: [localhost]: FAILED! => {"changed": false, "msg": "state is present but all of the following are missing: security_groups"}
@ansibullbot
Copy link

Files identified in the description:

If these files are inaccurate, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibullbot
Copy link

@ansibullbot ansibullbot added affects_2.10 bug This issue/PR relates to a bug module module needs_triage labels Aug 19, 2020
@ansibullbot ansibullbot added the plugins plugin (any type) label Aug 28, 2020
@gravesm gravesm added waiting_on_contributor Needs help. Feel free to engage to get things unblocked and removed needs_triage labels Jan 29, 2021
@gravesm
Copy link
Member

gravesm commented Jan 29, 2021

@briantist Thank you for reporting this! We have recently this module to our CI so it should be easier to add a test case for this.

@markuman
Copy link
Member

markuman commented Oct 20, 2021

I'm not sure if this is reproducible outside of a VPC since I'm not sure there is such a thing as a default SG in that case.

imo you can create ALB resources only in a VPC.

the only way to solve this issue is imo to not allow an empty array as an input for security_groups parameter.
otherwise and empty array must be treated as "use the VPCs default security group", which must be determined inside the code.

@briantist
Copy link
Contributor Author

I believe that "use the default VPC SG" is a desirable option in some/many use cases, so I wouldn't want to see that option disappear. It would be most useful if the module handled that case correctly, even if internally it means it has to retrieve the VPC default in order to verify its state.

@markuman markuman mentioned this issue Feb 1, 2022
1 task
@jatorcasso
Copy link
Contributor

I can work on this @briantist @markuman. Expected behavior is treat empty array as "use the VPCs default security group" ?

@briantist
Copy link
Contributor Author

I can work on this @briantist @markuman. Expected behavior is treat empty array as "use the VPCs default security group" ?

@jatorcasso kind of yes, that's already what happens on first creation; the problem is that running it again will fail. So when the ALB already exists, the module needs to have a way to verify the current state, and that part is not working now. It may require reading the default SG from the VPC in order to verify.

It might also be desirable to have "use the default VPC" as a separate option and then disallow the empty SG list, not sure...

I might also suggest that no security group option supplied, when the ALB exists, might mean "don't check or change the state of SGs" but that is a behavioral change.

@jatorcasso
Copy link
Contributor

If that's what happens on first creation, then that's what should happen after first creation as well imo to maintain idempotence. I'll work on that and add some integration tests to validate

softwarefactory-project-zuul bot pushed a commit that referenced this issue Mar 14, 2022
elb_application_lb - treat empty security group as VPC default

SUMMARY

Fixes idempotency issue when security_groups = [] by treating [] as using the VPC's default security group (like it does on creation).
Fixes #28
Used same logic as amazon.aws.ec2_vpc_route_table does for using default igw
Added integration tests

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
elb_application_lb

Reviewed-by: Jill R <None>
Reviewed-by: Mark Woolley <[email protected]>
jatorcasso added a commit to jatorcasso/community.aws that referenced this issue Mar 28, 2022
…le-collections#971)

elb_application_lb - treat empty security group as VPC default

SUMMARY

Fixes idempotency issue when security_groups = [] by treating [] as using the VPC's default security group (like it does on creation).
Fixes ansible-collections#28
Used same logic as amazon.aws.ec2_vpc_route_table does for using default igw
Added integration tests

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
elb_application_lb

Reviewed-by: Jill R <None>
Reviewed-by: Mark Woolley <[email protected]>
(cherry picked from commit 20b726a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.10 bug This issue/PR relates to a bug module module plugins plugin (any type) waiting_on_contributor Needs help. Feel free to engage to get things unblocked
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants