Skip to content
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

pricing.GetProducts includes breaking changes in v1.44.46 #4480

Closed
bendrucker opened this issue Jul 14, 2022 · 5 comments · Fixed by #4486 or #4487
Closed

pricing.GetProducts includes breaking changes in v1.44.46 #4480

bendrucker opened this issue Jul 14, 2022 · 5 comments · Fixed by #4486 or #4487
Assignees
Labels
bug This issue is a bug.

Comments

@bendrucker
Copy link

Describe the bug

Calling pricing.GetProducts() returns a different type beginning in v1.44.46, breaking existing programs. The PriceList field in pricing.GetProductsOutput is a *string, where it was previously an aws.JSONValue, which is a map[string]interface{}. Clients relying on the previous type are broken by this change:

products, _ := pricing.GetProducts(input)
b, _ := json.Marshal(products.PriceList)
fmt.Println(string(b)

Where previously this would print a JSON array where each element is an object, it now prints a JSON array where each element is a string (a stringified JSON object).

Expected Behavior

Either behavior is acceptable, but I would expect the type signature for all functions to remain the same across all revisions within a major version.

Current Behavior

Currently, an array of strings is returned

Reproduction Steps

For any pricing query (input)

products, _ := pricing.GetProducts(input)
b, _ := json.Marshal(products.PriceList)
fmt.Println(string(b))

This prints an array of strings

Possible Solution

No response

Additional Information/Context

Originally reported in hashicorp/terraform-provider-aws#25771

SDK version used

v1.44.46

Environment details (Version of Go (go version)? OS name and version, etc.)

go version go1.18.3 darwin/arm64

@bendrucker bendrucker added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 14, 2022
@bendrucker
Copy link
Author

bendrucker commented Jul 14, 2022

Worth noting, the change results from a change to the model, which was also applied to SDK v2:

aws/aws-sdk-go-v2@cbf4363#diff-16b9ee4d242312b1c334f2f00f3de6e2206b94b2662771d1b4b517f663147916

The issue is with the model, which causes a breaking change for all AWS SDKs. Seems like the API itself has never changed:

https://web.archive.org/web/20201126170559/https://docs.aws.amazon.com/aws-cost-management/latest/APIReference/API_pricing_GetProducts.html

@skmcgrail
Copy link
Member

skmcgrail commented Jul 19, 2022

This type flipped from aws.JSONValue to *string due to the service team changing the target shape within the model, which broke our legacy allowlist of services that were using aws.JSONValue. This would explain why this value type flipped in the Go SDK. I would like us to understand more how this change broke other SDKs though, as this shouldn't have been impacted by this modeling change in this manner. As while the model was changed, the effective net result should have been zero.

https://github.com/aws/aws-sdk-go/blob/main/private/model/api/legacy_jsonvalue.go#L139-L144

@vudh1 vudh1 removed the needs-triage This issue or PR still needs to be triaged. label Jul 19, 2022
@bendrucker
Copy link
Author

bendrucker commented Jul 19, 2022

I would like us to understand more how this change broke other SDKs though

In hindsight this seems like it was a bad assumption on my part and may only affect the Go SDK. The legacy allowlist makes a lot of sense and explains why the behavior differed here.

v2 of the Go SDK has always been a []string:

https://github.com/aws/aws-sdk-go-v2/blame/88b5ad7c6610bed957942d641b43516d46d38848/service/pricing/api_op_GetProducts.go#L65

@bendrucker
Copy link
Author

Appreciate the quick diagnosis and fix for this!

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

aws-sdk-go-automation pushed a commit that referenced this issue Jul 20, 2022
===

### Service Client Updates
* `service/acm-pca`: Updates service documentation
* `service/iot`: Updates service API and documentation
  * GA release the ability to enable/disable IoT Fleet Indexing for Device Defender and Named Shadow information, and search them through IoT Fleet Indexing APIs. This includes Named Shadow Selection as a part of the UpdateIndexingConfiguration API.

### SDK Bugs
* `service/pricing`: Fixes a bug that caused `GetProductsOutput.PriceList` to be generated incorrectly. ([#4486](#4486))
  * The [v1.44.46](https://github.com/aws/aws-sdk-go/releases/tag/v1.44.46) release incorrectly resulted in the `PriceList` field's type changing from `[]aws.JSONValue` to `[]*string`.
  * This release reverts this change, with the field now correctly updated to `[]aws.JSONValue`.
  * Fixes [#4480](#4480)
aws-sdk-go-automation added a commit that referenced this issue Jul 20, 2022
Release v1.44.59 (2022-07-20)
===

### Service Client Updates
* `service/acm-pca`: Updates service documentation
* `service/iot`: Updates service API and documentation
  * GA release the ability to enable/disable IoT Fleet Indexing for Device Defender and Named Shadow information, and search them through IoT Fleet Indexing APIs. This includes Named Shadow Selection as a part of the UpdateIndexingConfiguration API.

### SDK Bugs
* `service/pricing`: Fixes a bug that caused `GetProductsOutput.PriceList` to be generated incorrectly. ([#4486](#4486))
  * The [v1.44.46](https://github.com/aws/aws-sdk-go/releases/tag/v1.44.46) release incorrectly resulted in the `PriceList` field's type changing from `[]aws.JSONValue` to `[]*string`.
  * This release reverts this change, with the field now correctly updated to `[]aws.JSONValue`.
  * Fixes [#4480](#4480)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug.
Projects
None yet
3 participants