-
Notifications
You must be signed in to change notification settings - Fork 197
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
Added AWS Credentials Support for Scanning Private Registry (ECR) #1103
Open
VF-mbrauer
wants to merge
7
commits into
aquasecurity:main
Choose a base branch
from
VF-mbrauer:ecr_cred_support
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
b875dc8
Added support for AWS Private Registry (ECR)
VF-mbrauer 1411d12
AWS-API Throttling feature implemented.
VF-mbrauer d361d7f
Corrected Redundant parentheses and wrong bracket closings
VF-mbrauer 23bce93
Merged Date function in existing go-file. Adapted error handlings.
VF-mbrauer 58fb2c0
Unnecessary parenthesis removed.
VF-mbrauer bc24ddd
Set EcrTokenGen after successfull GetAuthorizationToken. Removed glob…
VF-mbrauer 10768d2
Update Chart.yaml
VF-mbrauer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ecrTokenRefreshTTLhas to be shorter then 12h? as Trivy might want to authenticate with an expired credentialThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, maybe a retry logic should be placed for
GetAuthorizationTokenin case tokens are expired unless we force refresh before 12hThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, Trivy will never use an expired token as in the original part I pushed, the Token creation was part of "each" individual request Trivy did per repository. The only reason we have this timer now in place is to reduce the number of requests (Throttling) against the ECR-API.
So, if you want to detect an expired token and you want to make it part of a retry to refresh the token now you need to get the first trivy job triggered which tries to get the image and will run into the error.
In this case, a Token Refresh needs to be triggered. But this is a completely new scenario, which needs to be plugged into the trivy process in combination with the PlugIn routines.
Also, a drawback and a negative aspect are here is, that each request needs to be sent towards API to check if still valid or not. But this will be contradicting the Throttling which I wanted to solve.
As of now, I see no point in that we need that. The Throttling does its job, and it is up to the implementer to configure the TTLs on their own. In my view, this is the best approach.