Fix: modifying existing application lb using certificates now properl…

[tsifrer]

…y sets certificates

SUMMARY

Fixed an issue where modifying an existing application load balancer fails because certificates are not copied to a new dictionary correctly.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

lib/ansible/modules/cloud/amazon/elb_application_lb.py

ANSIBLE VERSION

2.4.0

[ansibot]

cc @willthames @wimnat
click here for bot help

[wimnat]

Can you better explain the problem please? Did the module fail or were the results unexpected?

[tsifrer]

When modifying an existing application LB that has a certificate set for HTTPS connection, the script raised an exception KeyError: 'Certificates' which causes the module to fail. As a result of failure, it doesn't update the application LB.

Example:

- local_action:
    module: elb_application_lb
    name: "my-alb"
    state: present
    listeners:
      - Protocol: HTTPS
        Port: 443
        SslPolicy: ELBSecurityPolicy-2016-08
        Certificates:
          - CertificateArn: "arn:aws:iam::my-account-id:certificate/my-certificate"
        DefaultActions:
          - Type: forward
            TargetGroupName: "my-target-group"

This will eventually call compare_listener function in the module which will raise a KeyError: 'Certificates' exception on line 541 (in this case, but the same would happen on line 544 if we would be changing HTTP listener to HTTPS one). The problem happens because the compare_listener function is creating a new modified_listener dictionary but in cases where you have certificates set for HTTPS connection, it doesn't correctly set the Certificates key on the new modified_listener dictionary.

Sorry for not explaining it better in the summary. Hope this helps.

[s-hertel]

@wimnat I think this issue looks akin to #27332.
^ Nope.

[s-hertel]

Can you just do something like modified_listener['Certificates'] = [{'CertificateArn': new_listener['Certificates'][0]['CertificateArn']}] rather than making it an empty list and appending an empty dict?

[s-hertel]

I see that it's the same as done elsewhere in the module though. Fine as-is.

[s-hertel]

@wimnat This fix makes sense to me. Basically the same as what's happening on line 549-551: https://github.com/ansible/ansible/pull/28217/files#diff-a1bcebad578f0e6bfd841049d327a587L549
Are you happy with it?

[wimnat]

Yep i'm ok with it shipit