Modify http cred provider resolving logic #4961
Conversation
wty-Bryant
force-pushed
the
feat-http-cred
branch
from
bc5dbbc to
23c87aa
Compare
wty-Bryant
force-pushed
the
feat-http-cred
branch
from
33721f9 to
0be20ae
Compare
Release v1.44.332 (2023-08-25) === ### Service Client Updates * `service/cloudtrail`: Updates service API and documentation * Add ThrottlingException with error code 429 to handle CloudTrail Delegated Admin request rate exceeded on organization resources. * `service/detective`: Updates service API * `service/monitoring`: Updates service documentation * Doc-only update to get doc bug fixes into the SDK docs
Release v1.44.333 (2023-08-28) === ### Service Client Updates * `service/backup`: Updates service API and documentation * `service/compute-optimizer`: Updates service API and documentation * `service/organizations`: Updates service documentation * Documentation updates for permissions and links. * `service/securitylake`: Updates service API * `service/service-quotas`: Updates service API and documentation * `service/workspaces-web`: Updates service API and documentation
Release v1.44.334 (2023-08-29) === ### Service Client Updates * `service/cognito-idp`: Updates service API, documentation, and examples * `service/fsx`: Updates service documentation * `service/omics`: Updates service API and documentation * `service/sesv2`: Updates service API, documentation, paginators, and examples
|
i dont understand the PR description. what does this mean:
|
It means (1) allow eks/ecs host url while creating http credential provider (2) enable retrieving authorization token from both env var and auth token file |
CHANGELOG_PENDING.md
Outdated
| @@ -3,3 +3,5 @@ | |||
| ### SDK Enhancements | |||
|
|
|||
| ### SDK Bugs | |||
| * `aws/defaults`: Modify http credential provider resolving logic to allow EKS/ECS container host and auth token file. | |||
| * Fix http cred provider only allows local hosts endpoint and auth token directly set in env var | |||
There was a problem hiding this comment.
i understand that short-hand is often effective in our own internal conversations. but with customer-facing release notes, lets use correct grammar with complete thoughts. Additionally, since this is more of a feature and not a simple bug fix, the language should emphasize the overall problem being solved.
Modify http credential provider resolving logic to allow EKS/ECS container host and auth token file.
The way this is written sounds like this PR is just fixing a bug. However, as i understand it, were actually adding a feature to the HTTP credential provider. Since this is a feature, and not merely a bug-fix, it should relate back to the higher-level problem. For example,
In the HTTP credential provider, support is added for the EKS container host URL along with support for dynamic loading of an authorization token from a file or environment variable.
And this
Fix http cred provider only allows local hosts endpoint and auth token directly set in env var
This reads like a run-on sentence and sounds like its just explaining implementation details. And because its a run-on sentence, im not sure if auth token directly set in env var is something that already exists or is something that you are adding.
Since most of the feature is explained in what I wrote above, you just add a clarification here for the motivation. For example:
These changes enables use of IRSAv2 AuthNZ in EKS.
| if p.AuthorizationTokenProvider != nil { | ||
| authToken, err = p.AuthorizationTokenProvider.GetToken() | ||
| if err != nil { | ||
| return nil, fmt.Errorf("get authorization token: %v", err) |
There was a problem hiding this comment.
i dont understand what this error message is conveying
There was a problem hiding this comment.
This is just standard error wrapping to include context for the underlying error. I had previously asked that we get better about doing this, it makes identifying failure points a lot easier.
This is done to fix authentication issues related to EKS Pod identity. Picking aws/aws-sdk-go#4961 Signed-off-by: Arnaud Meukam <[email protected]>
This is done to fix authentication issues related to EKS Pod identity. Picking aws/aws-sdk-go#4961 Signed-off-by: Arnaud Meukam <[email protected]>
Modify http cred provider resolving logic to allow EKS/ECS host url and auth token from a file, which can be retrieved from env var