-
Notifications
You must be signed in to change notification settings - Fork 634
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
internal/ini: Normalize Section keys to lowercase #495
internal/ini: Normalize Section keys to lowercase #495
Conversation
@@ -1,6 +1,6 @@ | |||
{ | |||
"ʃʉʍΡιξ": { | |||
"ϰϪϧ": "Ϯϴϖ", | |||
"ϰϫϧ": "Ϯϴϖ", |
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.
wish I didn't have to change this, but, it has a capitalized UTF-8 character.
This is to match the behavior of the parsing in Python, and the boto based AWS CLI, where the keys are normalized to lowercase. We had a case where things were working using the CLI but when we tried to do the same action through the Go SDK it didn't work because we had UPPERCASED the access_key_id.
11b9ba0
to
037eeb9
Compare
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.
Thanks for taking the time to create this PR @gaqzi. The change to support caseless keys is good. It makes sense to change that test file.
It would be nice to have a way to do case-less keys without the extra allocations of ToLower, but since they are storied in a map for lookup I'm pretty sure this is the best solution currently. In the SDKs lifetime the the INI files are rarely read or used so the potential additional allocations won't be significant.
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.
Sorry realized after reviewing this that we probably also want to update the pkg's Section
type should probably be updated to down case the passed in key parameter before doing the map lookup.
=== Breaking Change --- * `aws/endpoints`: Several functions and types have been removed * Removes `DecodeModel` and `DecodeModelOptions` from the package ([#509](#509)) * Remove Region Constants, Partition Constants, and types use for exploring the endpoint data model ([#512](#512)) * `service/s3/s3crypto`: Package and associated encryption/decryption clients have been removed from the SDK ([#511](#511)) * `aws/external`: Removes several export constants and types ([#508](#508)) * No longer exports AWS environment constants used by the external environment configuration loader * `DefaultSharedConfigProfile` is now defined an exported constant * `aws`: `ErrMissingRegion`, `ErrMissingEndpoint`, `ErrStaticCredentialsEmpty` are now concrete error types ([#510](#510)) Services --- * Synced the V2 SDK with latest AWS service API definitions. SDK Features --- * `aws/signer/v4`: New methods `SignHTTP` and `PresignHTTP` have been added ([#519](#519)) * `SignHTTP` replaces `Sign`, and usage of `Sign` should be migrated before it's removal at a later date * `PresignHTTP` replaces `Presign`, and usage of `Presign` should be migrated before it's removal at a later date * `DisableRequestBodyOverwrite` and `UnsignedPayload` are now deprecated options and have no effect on `SignHTTP` or `PresignHTTP`. These options will be removed at a later date. * `aws/external`: Add Support for setting a default fallback region and resolving region from EC2 IMDS ([#523](#523)) * `WithDefaultRegion` helper has been added which can be passed to `LoadDefaultAWSConfig` * This helper can be used to configure a default fallback region in the event a region fails to be resolved from other sources * Support has been added to resolve region using EC2 IMDS when available * The IMDS region will be used if region as not found configured in either the shared config or the process environment. * Fixes [#244](#244) * Fixes [#515](#515) SDK Enhancements --- * `service/dynamodb/expression`: Add IsSet helper for ConditionBuilder and KeyConditionBuilder ([#494](#494)) * Adds a IsSet helper for ConditionBuilder and KeyConditionBuilder to make it easier to determine if the condition builders have any conditions added to them. * Implements [#493](#493). * `internal/ini`: Normalize Section keys to lowercase ([#495](#495)) * Update's SDK's ini utility to store all keys as lowercase. This brings the SDK inline with the AWS CLI's behavior. SDK Bugs --- * `internal/sdk`: Fix SDK's UUID utility to handle partial read ([#536](#536)) * Fixes the SDK's UUID utility to correctly handle partial reads from its crypto rand source. This error was sometimes causing the SDK's InvocationID value to fail to be obtained, due to a partial read from crypto.Rand. * Fix [#534](#534) * `aws/defaults`: Fix request metadata headers causing signature errors ([#536](#536)) * Fixes the SDK's adding the request metadata headers in the wrong location within the request handler stack. This created a situation where a request that was retried would sign the new attempt using the old value of the header. The header value would then be changed before sending the request. * Fix [#533](#533) * Fix [#521](#521)
Breaking Change --- * `aws/endpoints`: Several functions and types have been removed * Removes `DecodeModel` and `DecodeModelOptions` from the package ([#509](#509)) * Remove Region Constants, Partition Constants, and types use for exploring the endpoint data model ([#512](#512)) * `service/s3/s3crypto`: Package and associated encryption/decryption clients have been removed from the SDK ([#511](#511)) * `aws/external`: Removes several export constants and types ([#508](#508)) * No longer exports AWS environment constants used by the external environment configuration loader * `DefaultSharedConfigProfile` is now defined an exported constant * `aws`: `ErrMissingRegion`, `ErrMissingEndpoint`, `ErrStaticCredentialsEmpty` are now concrete error types ([#510](#510)) Services --- * Synced the V2 SDK with latest AWS service API definitions. SDK Features --- * `aws/signer/v4`: New methods `SignHTTP` and `PresignHTTP` have been added ([#519](#519)) * `SignHTTP` replaces `Sign`, and usage of `Sign` should be migrated before it's removal at a later date * `PresignHTTP` replaces `Presign`, and usage of `Presign` should be migrated before it's removal at a later date * `DisableRequestBodyOverwrite` and `UnsignedPayload` are now deprecated options and have no effect on `SignHTTP` or `PresignHTTP`. These options will be removed at a later date. * `aws/external`: Add Support for setting a default fallback region and resolving region from EC2 IMDS ([#523](#523)) * `WithDefaultRegion` helper has been added which can be passed to `LoadDefaultAWSConfig` * This helper can be used to configure a default fallback region in the event a region fails to be resolved from other sources * Support has been added to resolve region using EC2 IMDS when available * The IMDS region will be used if region as not found configured in either the shared config or the process environment. * Fixes [#244](#244) * Fixes [#515](#515) SDK Enhancements --- * `service/dynamodb/expression`: Add IsSet helper for ConditionBuilder and KeyConditionBuilder ([#494](#494)) * Adds a IsSet helper for ConditionBuilder and KeyConditionBuilder to make it easier to determine if the condition builders have any conditions added to them. * Implements [#493](#493). * `internal/ini`: Normalize Section keys to lowercase ([#495](#495)) * Update's SDK's ini utility to store all keys as lowercase. This brings the SDK inline with the AWS CLI's behavior. SDK Bugs --- * `internal/sdk`: Fix SDK's UUID utility to handle partial read ([#536](#536)) * Fixes the SDK's UUID utility to correctly handle partial reads from its crypto rand source. This error was sometimes causing the SDK's InvocationID value to fail to be obtained, due to a partial read from crypto.Rand. * Fix [#534](#534) * `aws/defaults`: Fix request metadata headers causing signature errors ([#536](#536)) * Fixes the SDK's adding the request metadata headers in the wrong location within the request handler stack. This created a situation where a request that was retried would sign the new attempt using the old value of the header. The header value would then be changed before sending the request. * Fix [#533](#533) * Fix [#521](#521)
This is to match the behavior of the parsing in Python and the boto
based AWS CLI, where the keys are normalized to lowercase. Per the config parser documentation in Python:
Because this indicates that only keys are case-insensitive in Python, I didn't touch the sections themselves (and I had a lot more failing tests if I did, so I opted for the smallest possible change that required changing existing tests).
We had a case where things were working using the CLI but when we tried
to do the same action, list S3 bucket through Terraform, which relies on the Go SDK, it didn't work because we had UPPERCASED the access_key_id.
Note: This issue is also present in the v1 SDK as the INI parser seems to be copied from there, but I figured I would start by making this PR here and if it's accepted handle the backport. :)