Add support for EKS Pod Identity #416
Conversation
tongshen-stephanie
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #416 +/- ##
==========================================
+ Coverage 49.67% 53.67% +3.99%
==========================================
Files 9 9
Lines 781 911 +130
==========================================
+ Hits 388 489 +101
- Misses 383 404 +21
- Partials 10 18 +8 ☔ View full report in Codecov by Sentry. |
| return nil, err | ||
| } | ||
| credProvider := NewIRSACredentialProvider(p.stsClient, *roleArn, p.region) | ||
| config, err = credProvider.GetAWSConfig(fetcher) |
There was a problem hiding this comment.
config, err = credProvider.GetAWSConfig(fetcher)
if err != nil {
return nil, err
}
is common to both blocks of the if statement, it can be extracted out.
There was a problem hiding this comment.
made change in commit c3ca813
| } | ||
| } | ||
|
|
||
| func (p *PodIdentityCredentialProvider) GetAWSConfig(fetcher *authTokenFetcher) (*aws.Config, error) { |
There was a problem hiding this comment.
Can we move the two credential providers to their own files in a subfolder?
There was a problem hiding this comment.
Do you mean creating a separate folder/package similar to auth? I didn't do it because moving credential providers to a new folder would also require moving authTokenFetcher at the same time (otherwise it causes circular dependency since credential provider and auth try to import each other). auth is pretty much empty after moving out those 2 big pieces.
| return podIdentityAgentEndpointIPv4 | ||
| }() | ||
|
|
||
| var podIdentityAgentEndpoint = defaultPodIdentityAgentEndpoint |
There was a problem hiding this comment.
Is defaultPodIdentityAgentEndpoint used anywhere else? If not you can remove the extra assignment.
| return nil, roleErr | ||
| } | ||
| credProvider := NewIRSACredentialProvider(p.stsClient, *roleArn, p.region) | ||
| config, err = credProvider.GetAWSConfig(fetcher) |
There was a problem hiding this comment.
I think you missed config, err = credProvider.GetAWSConfig(fetcher)
| config, err = credProvider.GetAWSConfig(fetcher) | ||
|
|
||
| } else { | ||
| roleArn, roleErr := p.getRoleARN() |
There was a problem hiding this comment.
This check should be the responsibility of the IRSA credential provider, let's move it to that provider's implementation in its constructor.
Issue #, if available: Closes #300
Description of changes:
Introduce an optional parameter usePodIdentity to support EKS Pod Identity
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.