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

Fix: Allow ACM lookups beyond 100 certs #250

Merged
merged 1 commit into from
Aug 27, 2024
Merged

Conversation

ryansb
Copy link
Contributor

@ryansb ryansb commented Aug 27, 2024

Jira Ticket

REDA-632

Description

The ellation account now has 147 certificates in it, so this is no longer sufficient.

  • Type of change
    • Bug fix
    • New feature
    • Code Cleanup
    • Library updates / upgrades
    • Refactoring

Can this PR be merged / deployed at any time?

  • Yes
  • No, only at specific time (please specify below)
  • No, only after certain conditions are met (please specify below)

Timeframe/Conditions:

How was this tested?

  • Describe the tests you ran to verify your changes.
  • Test environment (e.g. local, staging, proto0)

The `ellation` account now has 147 certificates in it, so this is no
longer sufficient.

Relates to fixing REDA-632
@ryansb ryansb force-pushed the fix/acm-beyond-100-certs branch from 87cedf9 to 23cbbac Compare August 27, 2024 13:49
Comment on lines +88 to +99
next_token = None
while True:
response = acm_client.list_certificates(
CertificateStatuses=['ISSUED'],
MaxItems=100,
NextToken=next_token
)
cert_summaries.extend(response["CertificateSummaryList"])
if response.get("NextToken"):
next_token = response["NextToken"]
else:
break

Choose a reason for hiding this comment

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

Suggested change
next_token = None
while True:
response = acm_client.list_certificates(
CertificateStatuses=['ISSUED'],
MaxItems=100,
NextToken=next_token
)
cert_summaries.extend(response["CertificateSummaryList"])
if response.get("NextToken"):
next_token = response["NextToken"]
else:
break
response =
acm_client.get_paginator('list_certificates').paginate(CertificateStatuses=['ISSUED']).build_full_result()

Boto3 has some cool built in paginator stuff, while loop works tho

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The version ef-open uses doesn't seem to have ACM paginators

@dlutsch dlutsch merged commit eb07c74 into master Aug 27, 2024
2 checks passed
@dlutsch dlutsch deleted the fix/acm-beyond-100-certs branch August 27, 2024 14:44
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.

5 participants