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

ELBv2: Fix CF bug when passing certificates to ELB listener creation #8106

Merged

Conversation

Ivan-Lukianov
Copy link
Contributor

Context:

Recently our tests started failing with a next error:

pattern = '^arn:(aws|aws-cn|aws-iso|aws-iso-b|aws-us-gov):iam:'
string = [{'CertificateArn': 'arn:aws:acm:us-east-1:123456789012:certificate/<cert_id>', 'certificate_arn': 'arn:aws:acm:us-east-1:123456789012:certificate/<cert_id>'}]
flags = 0

    def search(pattern, string, flags=0):
        """Scan through string looking for a match to the pattern, returning
        a Match object, or None if no match was found."""
>       return _compile(pattern, flags).search(string)
E       TypeError: expected string or bytes-like object, got 'list'

I tracked down an issue to this change - #8054

In my understanding in case of listener creation from cloudformation certificate variable is not a string, but an array and this newly added code is failing.

This PR adds a change so certificate passed as a string to create_listener method in cloudformation case similar to what is in the elb client code.

Copy link

codecov bot commented Sep 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.46%. Comparing base (b574a9a) to head (c0d1837).
Report is 6 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #8106   +/-   ##
=======================================
  Coverage   94.45%   94.46%           
=======================================
  Files        1141     1141           
  Lines       98198    98201    +3     
=======================================
+ Hits        92757    92761    +4     
+ Misses       5441     5440    -1     
Flag Coverage Δ
servertests 28.95% <0.00%> (-0.01%) ⬇️
unittests 94.43% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Ivan-Lukianov Ivan-Lukianov changed the title Fix elb certificates for cloudformation Fix bug: Align certificates usage on ELB listener creation in case of client and cloudformation Sep 9, 2024
Copy link
Collaborator

@bblommers bblommers left a comment

Choose a reason for hiding this comment

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

Hi @Ivan-Lukianov, thank you for the fix and for adding the test case!

It is an interesting decision from AWS to require a list of certificates, and at the same time state that You must provide exactly one certificate...
But with that in mind, the if/else statement that always picks the first certificate makes sense.

@bblommers bblommers changed the title Fix bug: Align certificates usage on ELB listener creation in case of client and cloudformation ELBv2: Fix CF bug when passing certificates to ELB listener creation Sep 9, 2024
@bblommers bblommers merged commit 255d2b8 into getmoto:master Sep 9, 2024
51 checks passed
@bblommers bblommers added this to the 5.0.15 milestone Sep 9, 2024
Copy link
Contributor

github-actions bot commented Sep 9, 2024

This is now part of moto >= 5.0.15.dev2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants