Skip to content

fix: paginate ECR DescribeImages to retrieve all image tags#441

Open
matteosilv wants to merge 6 commits into
jetstack:mainfrom
matteosilv:fix/ecr-pagination
Open

fix: paginate ECR DescribeImages to retrieve all image tags#441
matteosilv wants to merge 6 commits into
jetstack:mainfrom
matteosilv:fix/ecr-pagination

Conversation

@matteosilv

Copy link
Copy Markdown

No description provided.

Copilot AI review requested due to automatic review settings June 8, 2026 10:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds pagination support to the ECR image tag listing so all images/tags are returned across multiple DescribeImages pages.

Changes:

  • Refactors DescribeImages call to loop over pages using NextToken.
  • Accumulates tags across pages into a single map before returning results.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/client/ecr/ecr.go
Comment on lines +67 to 70
images, err := client.DescribeImages(ctx, input)
if err != nil {
return nil, fmt.Errorf("failed to describe images: %s", err)
}
Comment thread pkg/client/ecr/ecr.go Outdated
Comment on lines +93 to +96
if images.NextToken == nil {
break
}
input.NextToken = images.NextToken
Copilot AI review requested due to automatic review settings June 15, 2026 08:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.

@joshw123

Copy link
Copy Markdown
Collaborator

Thanks for the Contribution @matteosilv Can you please review the copilot comments as above and see if they are relevant, thank you! :)

- Use %w for error wrapping so callers can unwrap with errors.Is/As
- Also break on empty-string NextToken to guard against non-nil empty token edge case
Copilot AI review requested due to automatic review settings June 15, 2026 13:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comment thread pkg/client/ecr/ecr.go
Comment on lines +66 to 70
for {
images, err := client.DescribeImages(ctx, input)
if err != nil {
return nil, fmt.Errorf("failed to describe images: %w", err)
}
Comment thread pkg/client/ecr/ecr.go Outdated
Comment on lines +93 to +96
if images.NextToken == nil || aws.ToString(images.NextToken) == "" {
break
}
input.NextToken = images.NextToken
@matteosilv

Copy link
Copy Markdown
Author

@joshw123 both looks valid and I addressed them

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.

4 participants