feature/ec2/imds: Fix Client's response handling and operation timeout race

[jasdel]

Fixes #1253 race between reading a IMDS response, and the operationTimeout middleware cleaning up its timeout context. Changes the IMDS client to always buffer the response body received, before the result is deserialized. This ensures that the consumer of the operation's response body will not race with context cleanup within the middleware stack.

Also updates the IMDS client to respect passed in Context Deadline/Timeout. Updates the IMDS Client operations to not override the passed in Context's Deadline or Timeout options. If an Client operation is called with a Context with a Deadline or Timeout, the client will no longer override it with the client's default timeout.

[bcap]

<3 thank you for allowing timeouts to be externally configurable!

[bcap]

maybe this could use the constant defaultOperationTimeout directly and remove the timeout var entirely from operationTimeout struct? Seems m.DefaultTimeout will always be defaultOperationTimeout in the end.

Not sure though of the impact/necessity on tests

[jasdel]

Having the DefaultTimeout member on operationTimeout middleware is handy if we want to change the timeout depending on the operation. I'm not aware of a need for this at the moment. Since operationTimeout is not exported, having the member is preferred so we can reuse the operationTimeout middleware later.

There is a test that uses a custom timeout, but I think this test needs to be tweaked a little bit to more closely align with the intention behind it. e.g. if there was a context with timeout use it, if there wasn't use a default. This test is relying on the behavior of a canceled context, and the configured nanosecond timeout input.

aws-sdk-go-v2/feature/ec2/imds/request_middleware_test.go

Lines 128 to 152 in dccd10a

	func TestOperationTimeoutMiddleware(t *testing.T) {
	m := &operationTimeout{
	DefaultTimeout: time.Nanosecond,
	}
	_, _, err := m.HandleInitialize(context.Background(), middleware.InitializeInput{},
	middleware.InitializeHandlerFunc(func(
	ctx context.Context, input middleware.InitializeInput,
	) (
	out middleware.InitializeOutput, metadata middleware.Metadata, err error,
	) {
	if err := sdk.SleepWithContext(ctx, time.Second); err != nil {
	return out, metadata, err
	}
	return out, metadata, nil
	}))
	if err == nil {
	t.Fatalf("expect error got none")
	}
	if e, a := "deadline exceeded", err.Error(); !strings.Contains(a, e) {
	t.Errorf("expect %q error in %q", e, a)
	}
	}

[jasdel]

Updated operationTimeout to not set a default timeout, if DefaultTimeout is zero. I think this was the missing behavior in keeping DefaultTimeout member in operationTimeout.