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

Cannot create S3 bucket inventory with SSE-S3 encryption #2015

Closed
danielwhite opened this issue Jun 28, 2018 · 1 comment · Fixed by #2078
Closed

Cannot create S3 bucket inventory with SSE-S3 encryption #2015

danielwhite opened this issue Jun 28, 2018 · 1 comment · Fixed by #2078
Labels
bug This issue is a bug.

Comments

@danielwhite
Copy link

danielwhite commented Jun 28, 2018

Version of AWS SDK for Go?

  • 1.14.8
  • v1.14.15-1-g0625b326

Version of Go (go version)?

$ go version
go version go1.10 darwin/amd64

What issue did you see?

The SDK produces a malformed request when configuring S3 bucket inventory where the destination is using SSE-S3 encryption.

Given the following s3.PutBucketInventoryConfigurationInput:

{
  Bucket: "example-inventory-bucket",
  Id: "SSE-S3 Encryption",
  InventoryConfiguration: {
    Destination: {
      S3BucketDestination: {
        Bucket: "aws:s3:::example-inventory-bucket",
        Encryption: {
          SSES3: {

          }
        },
        Format: "ORC"
      }
    },
    Id: "SSE-S3 Encryption",
    IncludedObjectVersions: "All",
    IsEnabled: true,
    Schedule: {
      Frequency: "Daily"
    }
  }
}

The operation produces the following error:

2018/06/29 02:33:43 DEBUG: Request s3/PutBucketInventoryConfiguration Details:
---[ REQUEST POST-SIGN ]-----------------------------
PUT /?id=SSE-S3%20Encryption&inventory= HTTP/1.1
[ ... ]
<InventoryConfiguration xmlns="http://s3.amazonaws.com/doc/2006-03-01/"><Schedule><Frequency>Daily</Frequency></Schedule><Destination><S3BucketDestination><Bucket>aws:s3:::example-inventory-bucket</Bucket><Encryption></Encryption><Format>ORC</Format></S3BucketDestination></Destination><Id>SSE-S3 Encryption</Id><IncludedObjectVersions>All</IncludedObjectVersions><IsEnabled>true</IsEnabled></InventoryConfiguration>
-----------------------------------------------------
2018/06/29 02:33:49 DEBUG: Response s3/PutBucketInventoryConfiguration Details:
---[ RESPONSE ]--------------------------------------
HTTP/1.1 400 Bad Request
[ ... ]
-----------------------------------------------------
2018/06/29 02:33:49 <?xml version="1.0" encoding="UTF-8"?>
<Error><Code>MalformedXML</Code><Message>The XML you provided was not well-formed or did not validate against our published schema</Message><RequestId>8D2C362D0151D051</RequestId><HostId>vBEciPKXOl72I+Uj3A2Qhn8T56Clm5S4I9XhmskMAKuw1ayBhDYAHbFqHi7CizUORMa1mp2J+FU=</HostId></Error>

At first glance, the main offender seems to be a check in xmlutil.buildStruct to prevent marshaling of structs where no child fields have been marshaled.

if fieldAdded { // only append this child if we have one ore more valid members
current.AddChild(child)
}

Steps to reproduce

The following will produce the error described above:

package main

import (
	"flag"

	"github.com/aws/aws-sdk-go/aws"
	"github.com/aws/aws-sdk-go/aws/awserr"
	"github.com/aws/aws-sdk-go/aws/session"
	"github.com/aws/aws-sdk-go/service/s3"
)

func main() {
	bucket := flag.String("bucket", "example-inventory-bucket", "Name of bucket to create")
	flag.Parse()

	sess := session.Must(session.NewSessionWithOptions(session.Options{
		SharedConfigState: session.SharedConfigEnable,
	}))
	cfg := aws.NewConfig().WithLogLevel(aws.LogDebugWithHTTPBody)
	client := s3.New(sess, cfg)

	// Set up a bucket to use as an S3 inventory source and destination.
	if _, err := client.CreateBucket(&s3.CreateBucketInput{
		Bucket: bucket,
		ACL:    aws.String(s3.BucketCannedACLPrivate),
	}); err != nil {
		if aerr, ok := err.(awserr.Error); ok && aerr.Code() == s3.ErrCodeBucketAlreadyOwnedByYou {
			// second run
		} else {
			panic(err)
		}
	}

	if err := client.WaitUntilBucketExists(&s3.HeadBucketInput{
		Bucket: bucket,
	}); err != nil {
		panic(err)
	}

	// Configure inventory to use SSE-S3 encryption when writing.
	input := &s3.PutBucketInventoryConfigurationInput{
		Bucket: bucket,
		Id:     aws.String("SSE-S3 Encryption"),
		InventoryConfiguration: &s3.InventoryConfiguration{
			Id:                     aws.String("SSE-S3 Encryption"),
			IsEnabled:              aws.Bool(true),
			IncludedObjectVersions: aws.String(s3.InventoryIncludedObjectVersionsAll),
			Schedule: &s3.InventorySchedule{
				Frequency: aws.String(s3.InventoryFrequencyDaily),
			},
			Destination: &s3.InventoryDestination{
				S3BucketDestination: &s3.InventoryS3BucketDestination{
					Format: aws.String(s3.InventoryFormatOrc),
					Bucket: aws.String("aws:s3:::" + *bucket),
					Encryption: &s3.InventoryEncryption{
						SSES3: &s3.SSES3{}, // This is not marshaled.
					},
				},
			},
		},
	}

	log.Printf("%#v", input)

	if _, err := client.PutBucketInventoryConfiguration(input); err != nil {
		panic(err)
	}
}
@jasdel
Copy link
Contributor

jasdel commented Jun 28, 2018

Thanks for reporting this issue @danielwhite I think you're correct the SDK's XML marshaler is intentionally excluding elements from the XML document if that structure does not have nested values. I know the SDK's JSON marshaler are able to handle this case to marshaler empty objects.

PR #1998 adds additional tests from XML marshaling that we can use for validating this functionality. The SDK's xml marshaler is pretty fragile, additional tests need to be included with the fix for this.

danielwhite pushed a commit to danielwhite/terraform-provider-aws that referenced this issue Jun 29, 2018
…tion is enabled

This prevents importing a resource that has the feature enabled so
that future updates don't accidentally stomp on the data.

The SDK currently fails to marshal this option correctly in a
create/update request.

The risk is minimal since the create/update request fails anyway, but
this will avoid a user being surprised by a failed update _after_
they've imported it into their state.

See: aws/aws-sdk-go#2015
jasdel added a commit to jasdel/aws-sdk-go that referenced this issue Jul 26, 2018
Updates the S3 Upload Manager's default behavior for MaxNumParts, and
ensurs that the Uploader.MaxNumPart's member value is initialized
properly if the type was created via struct initialization instead of
using the `NewUploader` function.

Fix aws#2015
jasdel added a commit that referenced this issue Jul 26, 2018
…rts (#2077)

Updates the S3 Upload Manager's default behavior for MaxNumParts, and
ensurs that the Uploader.MaxNumPart's member value is initialized
properly if the type was created via struct initialization instead of
using the `NewUploader` function.

Fix #2015
@awstools awstools mentioned this issue Jul 26, 2018
jasdel added a commit to jasdel/aws-sdk-go that referenced this issue Jul 27, 2018
Fixes the SDK's marshaling of types without members. This corrects the
issue where the SDK would not marshal an XML tag for a type, if that
type did not have any exported members.

Fix aws#2015
jasdel added a commit to jasdel/aws-sdk-go that referenced this issue Jul 31, 2018
Fixes the SDK's marshaling of types without members. This corrects the
issue where the SDK would not marshal an XML tag for a type, if that
type did not have any exported members.

Fix aws#2015
jasdel added a commit that referenced this issue Jul 31, 2018
Fixes the SDK's marshaling of types without members. This corrects the
issue where the SDK would not marshal an XML tag for a type, if that
type did not have any exported members.

Fix #2015
@awstools awstools mentioned this issue Jul 31, 2018
xibz pushed a commit to xibz/aws-sdk-go that referenced this issue Sep 12, 2018
…rts (aws#2077)

Updates the S3 Upload Manager's default behavior for MaxNumParts, and
ensurs that the Uploader.MaxNumPart's member value is initialized
properly if the type was created via struct initialization instead of
using the `NewUploader` function.

Fix aws#2015
xibz pushed a commit to xibz/aws-sdk-go that referenced this issue Sep 12, 2018
…2081)

Fixes the SDK's marshaling of types without members. This corrects the
issue where the SDK would not marshal an XML tag for a type, if that
type did not have any exported members.

Fix aws#2015
usrenmae pushed a commit to usrenmae/aws-sdk-go that referenced this issue Sep 23, 2018
…rts (aws#2077)

Updates the S3 Upload Manager's default behavior for MaxNumParts, and
ensurs that the Uploader.MaxNumPart's member value is initialized
properly if the type was created via struct initialization instead of
using the `NewUploader` function.

Fix aws#2015
usrenmae pushed a commit to usrenmae/aws-sdk-go that referenced this issue Sep 23, 2018
…2081)

Fixes the SDK's marshaling of types without members. This corrects the
issue where the SDK would not marshal an XML tag for a type, if that
type did not have any exported members.

Fix aws#2015
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
Development

Successfully merging a pull request may close this issue.

2 participants