lucix-aws
commented
1 month ago The use of the #v formatter here is misleading. What you're seeing when you print a document value in a response directly like that is the un-mutated interface{} value for the manifest field that was decoded from the service's JSON response. The deserializers for JSON services all use json.Decoder.UseNumber, so numeric tokens are decoded by the stdlib into json.Number which is an alias for string.
For whatever reason the #v represents these json.Number values with quotes. Couldn't tell you why.
- EDIT: Apparently the
#vis for a "Go-syntax" representation of the value, so I guess it just looks past the alias and the fact that json.Number has a String() method on it and just prints the literal, with quotes. It seems like they could at least have it print asjson.Number("90")etc. but that's out of my control.
If you actually use UnmarshalSmithyDocument to unwrap the response document into something, it'll recognize the numericness of that inner field just fine. Example at the end.
I can't really speak to these terraform issues. Perhaps they're doing something wrong in regards to this field, but that would surprise me -- the value you're showing through #v really doesn't mean anything programmatically, under the hood I assume they're using the document unmarshaling API as intended in some way to actually do things with this document field, and they can definitely unmarshal the inner fields in question into numeric types as the example shows.
(for this example I just mocked the wire response instead of creating one of these landing zones, which I know nothing about and it seemed involved/impossible for me without additional setup):
package main
import (
"context"
"io"
"log"
"net/http"
"strings"
"github.com/aws/aws-sdk-go-v2/aws"
"github.com/aws/aws-sdk-go-v2/config"
"github.com/aws/aws-sdk-go-v2/service/controltower"
)
const lz = `
{
"landingZone": {
"arn": "$landing_zone_arn",
"driftStatus": {
"status": "IN_SYNC"
},
"latestAvailableVersion": "3.3",
"manifest": {
"accessManagement": {
"enabled": false
},
"securityRoles": {
"accountId": "$account_id"
},
"governedRegions": [
"eu-west-1",
"eu-central-1",
"us-east-1",
"us-west-2"
],
"organizationStructure": {
"security": {
"name": "Security"
}
},
"centralizedLogging": {
"accountId": "$account_id",
"configurations": {
"loggingBucket": {
"retentionDays": 90
},
"kmsKeyArn": "$kms_arn",
"accessLoggingBucket": {
"retentionDays": 90
}
},
"enabled": true
}
},
"status": "ACTIVE",
"version": "3.3"
}
}
`
type mockHTTP struct{}
func (mockHTTP) Do(r *http.Request) (*http.Response, error) {
return &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(strings.NewReader(lz)),
}, nil
}
type Manifest struct {
// omitting stuff we don't care about in this example
CentralizedLogging struct {
Configurations struct {
LoggingBucket struct {
RetentionDays int
}
}
}
}
func main() {
cfg, err := config.LoadDefaultConfig(context.Background(), config.WithClientLogMode(aws.LogResponseWithBody))
if err != nil {
log.Fatal(err)
}
client := controltower.NewFromConfig(cfg)
input := &controltower.GetLandingZoneInput{
LandingZoneIdentifier: aws.String("foo"),
}
output, err := client.GetLandingZone(context.Background(), input, func(o *controltower.Options) {
o.HTTPClient = mockHTTP{}
})
if err != nil {
log.Fatal(err)
}
var manifest Manifest
if err := output.LandingZone.Manifest.UnmarshalSmithyDocument(&manifest); err != nil {
panic(err)
}
log.Printf("\n\nsmithy document: %#v", output.LandingZone.Manifest)
println("retention days", manifest.CentralizedLogging.Configurations.LoggingBucket.RetentionDays)
}
markwellis
github-actions[bot]
Acknowledgements
go get -u github.com/aws/aws-sdk-go-v2/...)Describe the bug
The landing zone manifest has two
retentionDaysfields that are numbers.centralizedLogging.configurations.accessLoggingBucket.retentionDayscentralizedLogging.configurations.loggingBucket.retentionDaysThe AWS API returns them as numbers
aws controltower get-landing-zone --landing-zone-identifier '$landing_zone_arn' --output jsonoutput with sensitive data redacted:
when using the go sdk, they are converted to string which is incorrect:
you can see both
retentionDaysare"90"instead of90Expected Behavior
both
centralizedLogging.configurations.accessLoggingBucket.retentionDays¢ralizedLogging.configurations.loggingBucket.retentionDaysin the manifest should remain numbers and not be stringsCurrent Behavior
both
centralizedLogging.configurations.accessLoggingBucket.retentionDays¢ralizedLogging.configurations.loggingBucket.retentionDaysin the manifest are converted to stringsReproduction Steps
./main "$landing_zone_arnoutput shows the LogResponseWithBody debug logging that the api response is correct. both{"retentionDays":90}are numbers and not strings (formatted for readability)but the smithy document is incorrect, both
{"retentionDays":"90"}are strings and not numbersPossible Solution
No response
Additional Information/Context
this is causing half of this terraform issue https://github.com/hashicorp/terraform-provider-aws/issues/35763 - specifically this part of the plan, trying to change the type from a string to a number, because when terraform reads the current manifest using the aws sdk the type is wrong so terraform tries to change it back. It is not wrong at aws, but the go aws sdk is converting them to a string
AWS Go SDK V2 Module Versions Used
go mod graphCompiler and Version used
go version go1.22.3 linux/amd64
Operating System and version
Fedora Linux 40