Lack of default value serialization causes certain requests to fail

[lucix-aws]

Describe the bug

In the AWS SDK for Go v2, "optional" fields in an operation request are exposed as nilable (pointer e.g. *bool) types. When left as their nil zero value, these fields will not be serialized in an outgoing request.

Additionally, specific fields may be marked as having a "default" in their smithy model. For these fields, we instead generate them as scalar types (e.g. bool). We then omit serialization of these values if they are set as the zero value in the input (whether they were done so explicitly or otherwise, as we do not implement "presence tracking" of set values in the Go v2 SDK).

Certain operations, however, will fail on validation if the input is configured in such a way that certain values are set as defaults and end up not being serialized. This can be confusing to the SDK user, who from their perspective, may be explicitly setting values in a request, only to have the service reject with the complaint that their submitted config is incomplete.

The following operations have been observed to behave incorrectly in this manner, though this list is certainly nowhere near exhaustive:

• S3::PutPublicAccessBlock: main input's PublicAccessBlockConfiguration field
• S3Control::PutPublicAccessBlock: main input's PublicAccessBlockConfiguration field
• S3::SelectObjectContent: main input's ScanRange field
• MediaConvert::CreateJob in general (this operation's input is large and likely has several trigger points for this issue)
• We were notified of a customer issue for this same reason within the Lambda service, but didn't receive follow-up to identify the operation(s) at fault.
• APIGateway operations have had this issue in the past, but have reportedly been fixed in the model.
• There have historically been issues with the EC2 service in this regard as well. We have applied a customization to make every field in every input nilable because of this.
• SQS::ChangeMessageVisibilityBatch: input.Entries[].VisibilityTimeout (reported in sqs.ChangeMessageVisibilityBatch fails when entries' VisibilityTimeout is zero #2250)
• IVS::UpdateChannel: TranscodePreset (reported in SDK UpdateChannel Call Fails to Update IVS Channel Type #2262)

Expected Behavior

n/a

Current Behavior

n/a

Reproduction Steps

This is one example for PutPublicAccessBlock:

package main

import (
    "context"
    "log"
    "github.com/aws/aws-sdk-go-v2/aws"
    "github.com/aws/aws-sdk-go-v2/config"
    "github.com/aws/aws-sdk-go-v2/service/s3control"
    "github.com/aws/aws-sdk-go-v2/service/s3control/types"
)

func main() {
    cfg, err := config.LoadDefaultConfig(context.Background(),
        config.WithRegion("us-east-1"),
        config.WithClientLogMode(aws.LogRequest|aws.LogRequestWithBody|aws.LogResponse))
    if err != nil {
        log.Fatal(err)
    }

    s3 := s3control.NewFromConfig(cfg)

    _, err = s3.PutPublicAccessBlock(context.Background(), &s3control.PutPublicAccessBlockInput{
        AccountId:                      aws.String("..."),
        PublicAccessBlockConfiguration: &types.PublicAccessBlockConfiguration{BlockPublicAcls: false},
    })
    if err != nil {
        log.Fatal(err)
    }
}

This snippet, when run, will produce an error:

operation error S3 Control: PutPublicAccessBlock, https response error StatusCode: 400, RequestID: ..., HostID: ..., api error InvalidRequest: Must specify at least one configuration.

Because the request serialized PublicAccessBlockConfiguration as an empty container (rather than explicitly setting BlockPublicAcls to false within).

Possible Solution

There are several ways forward here, listed in order of increasing impact radius:

• Patch the problematic fields on a per-op basis
• Abandon the notion of default values for affected services and instead generate every input field as nilable
• Option 2, but for every service

All of these options would constitute a break-fix.

Note that simply switching to serialize default values is not an option, as this is liable to break operations whose default value behavior is the inverse of the operations recorded here.

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

latest

Compiler and Version used

n/a

Operating System and version

n/a

[object-Object]

This also occurs with lifecycle rules containing AbortIncompleteMultipartUpload.

Go reproduction steps:

package main

import (
	"context"
	"log"

	"github.com/aws/aws-sdk-go-v2/aws"
	"github.com/aws/aws-sdk-go-v2/config"
	"github.com/aws/aws-sdk-go-v2/service/s3"
	"github.com/aws/aws-sdk-go-v2/service/s3/types"
)

func main() {
	bucketName := "..."

	ctx := context.Background()
	cfg, err := config.LoadDefaultConfig(ctx)
	if err != nil {
		log.Fatal(err)
	}

	client := s3.NewFromConfig(cfg)

	_, err = client.PutBucketLifecycleConfiguration(ctx, &s3.PutBucketLifecycleConfigurationInput{
		Bucket: &bucketName,
		LifecycleConfiguration: &types.BucketLifecycleConfiguration{
			Rules: []types.LifecycleRule{
				{
					ID:     aws.String("rule"),
					Status: types.ExpirationStatusEnabled,
					Filter: &types.LifecycleRuleFilterMemberPrefix{
						Value: "prefix",
					},
					AbortIncompleteMultipartUpload: &types.AbortIncompleteMultipartUpload{
						DaysAfterInitiation: 1,
					},
					Expiration: &types.LifecycleExpiration{ // problem occurs here
						ExpiredObjectDeleteMarker: false,
					},
				},
			},
		},
	})
	if err != nil {
		log.Fatal(err)
	}
}

This is an issue when deserializing lifecycle rules created through the console (or likely other SDKs which don't have Go's zero value semantics). When created through the console, this type of rule returns the following XML, corresponding to the above Go code.

<LifecycleConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01/">
    <Rule>
        <ID>pass</ID>
        <Filter>
            <Prefix>prefix</Prefix>
        </Filter>
        <Status>Enabled</Status>
        <AbortIncompleteMultipartUpload>
            <DaysAfterInitiation>4</DaysAfterInitiation>
        </AbortIncompleteMultipartUpload>
        <Expiration>
            <ExpiredObjectDeleteMarker>false</ExpiredObjectDeleteMarker>
        </Expiration>
    </Rule>
</LifecycleConfiguration>

The following XML is what's sent by the Go code, which returns a MalformedXML error.

<LifecycleConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01">
    <Rule>
        <ID>pass</ID>
        <Filter>
            <Prefix>prefix</Prefix>
        </Filter>
        <Status>Enabled</Status>
        <AbortIncompleteMultipartUpload>
            <DaysAfterInitiation>4</DaysAfterInitiation>
        </AbortIncompleteMultipartUpload>
        <Expiration></Expiration>
    </Rule>
</LifecycleConfiguration>

This means (as with #2874) that getting the lifecycle rules from a bucket then putting those same rules to the bucket, which should logically be a no-op, returns an error. Our use case is dynamically appending lifecycle rules to a bucket while preserving existing rules.

Workaround, for others running into this issue:

for i, rule := range response.Rules {
	if rule.Expiration != nil && rule.Expiration.Date == nil && rule.Expiration.Days == 0 && !rule.Expiration.ExpiredObjectDeleteMarker {
		response.Rules[i].Expiration = nil
	}
}

[RanVaknin]

related aws/aws-sdk#577

[lucix-aws]

Yesterday's release addressed a significant number of these. A followup to address S3 specifically is forthcoming.

[lucix-aws]

Today's release included another round of fixes to various services, including S3: https://github.com/aws/aws-sdk-go-v2/releases/tag/release-2023-11-17

[wolfeidau]

Wow this change caught me off guard when starting a new project, by breaking another library I used in some interesting ways.

1. I added github.com/aws/aws-sdk-go-v2/service/s3
2. I added github.com/wolfeidau/s3iofs which uses an older version of v2 s3.
3. Pretty confusing compile error...

# github.com/wolfeidau/s3iofs
../../../go/pkg/mod/github.com/wolfeidau/[email protected]/s3iofs.go:82:13: cannot use res.ContentLength (variable of type *int64) as int64 value in struct literal
../../../go/pkg/mod/github.com/wolfeidau/[email protected]/s3iofs.go:150:13: cannot use obj.Size (variable of type *int64) as int64 value in struct literal
../../../go/pkg/mod/github.com/wolfeidau/[email protected]/s3iofs.go:172:14: cannot use 1 (untyped int constant) as *int32 value in struct literal
../../../go/pkg/mod/github.com/wolfeidau/[email protected]/s3iofs.go:193:13: cannot use list.Contents[0].Size (variable of type *int64) as int64 value in struct literal
make: *** [build] Error 1

Not sure if i was the only one to run into this.

[at-wat]

I believe API breaking changes should be released with a new major version.

I also have a library importing aws-sdk-go-v2 and have compatibility issue due to this.

[lucix-aws]

See my comments on #2370 (comment). This issue is another supporting data point for getting a proper major versioning strategy in place.

[lucix-aws]

Through the efforts of the smithy team we've now fixed default value issues of this nature across 100 services (including s3).

I'm going to close this since we've addressed the problem at large. Problems with this behavior in the future should be addressed through individual issues.

[github-actions]

⚠️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.