[WIP]New Resource: aws_shield_protection, aws_shield_subscription

[atsushi-ishibashi]

#1769
Sorry for adding two resource at a time because I found aws_shield_protection requires aws_shield_subscription during development.
I have a trouble about shield subscription. AWS Shield Advanced costs $3,000/month and I couldn't find the documentation describing the charge after deactivating.
So far, I removed TestAccAWSShieldSubscription.

There isn't shield package so I added it.

govendor fetch github.com/aws/aws-sdk-go/service/shield

[radeksimko]

Hi @atsushi-ishibashi
thanks for the contribution!

Do you mind submitting all changes in vendor in a separate PR to make review of this PR easier (less LOC)?

Thanks.

[atsushi-ishibashi]

@radeksimko Thank you for teaching me. I have submitted #1901

[atsushi-ishibashi]

@radeksimko I asked the AWS support how to use shield api without the subscription of Shield Advanced but it's impossible...

[radeksimko]

Just FYI - we reached out to AWS to ask about this too, in the context of our testing account.
So far we did not hear back and I'm not inclined to merging something we cannot test.

I will keep you posted.

[akataz]

My company has Shield Advanced and we would love to be able to leverage this for our teams. How can I test this?

[akataz]

I was able to test this successfully with make testacc after modifying the makefile to only run the Shield Advanced test. If I try to run it without adding -run=TestAccAWSShieldSubscription it appears to hang for an indefinite amount of time (waited 30 minutes, I realize the timeout is 120 minutes). Any ideas on why it might be hanging when trying to run all of them? @radeksimko if you have any ideas they would be appreciated.

Could there be an issue with the config.go and provider.go files being out of date? I will try to update them to the state of the current master and try again.

[akataz]

Hey Everyone,

Is there anyone available who can also test this PR? Hashicorp has informed us they are waiting for another company to test it. Any help would be much appreciated, we have a lot of projects that would benefit from this.

[bcornils]

Hello, I'm looking for a second +1 on this. If anyone tracking this can do so, that would be wonderful and appreciated.

[jansiwy]

While I'm really looking forward for the aws_shield_protection resource, I'd recommend to exclude the aws_shield_subscription from this pull request: The Shield subscription does not behave very resource-like: It can be created, but it cannot be deleted directly as the DeleteSubscription operation has been deprecated:

https://docs.aws.amazon.com/waf/latest/DDOSAPIReference/API_DeleteSubscription.html

I'd also like to mention that a Shield subscription is only necessary once per AWS organization. Thus, even if a company has multiple AWS accounts, the Shield subscription is only necessary once. This means, it's not really useful to manage this as code.

[akataz]

@bcornils @jansiwy are either of you able to test this(requires shield advanced subscription)/ update the Go code to remove the part about the subscription(I agree, it doesn't make sense to manage that via Terraform)? I haven't worked with Go, though deleting stuff probably wouldn't be too difficult. I have been in contact with Hashicorp support and they accepted my testing, but said that they would like another company to test it as well. Please let me know, I would be happy to test again.

[parabolic]

@akataz @jansiwy @bcornils
I will start the acceptance tests only for the aws_shield_protection if they pass and if @atsushi-ishibashi doesn't respond I'll open a new PR with only the aws_shield_protection feature and we can continue from there.
Does that sound good ?

[parabolic]

I've tried to run the test from the branch -> issue1769 in @atsushi-ishibashi fork and I got the following error.

make testacc TESTARGS='-run=TestAccAWSShieldProtection'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test $(go list ./... |grep -v 'vendor') -v -run=TestAccAWSShieldProtection -timeout 120m
?   	github.com/terraform-providers/terraform-provider-aws	[no test files]
# github.com/terraform-providers/terraform-provider-aws/aws
aws/resource_aws_vpc_endpoint.go:206: Printf format %q has arg input.VpcEndpointId of wrong type *string
aws/resource_aws_api_gateway_resource_test.go:75: Errorf format %q has arg conf.Path of wrong type *string
aws/resource_aws_spot_fleet_request_test.go:493: Errorf format %q has arg placement.Tenancy of wrong type *string
FAIL	github.com/terraform-providers/terraform-provider-aws/aws [build failed]
make: *** [GNUmakefile:15: testacc] Error 2

And it is about files unrelated to shield, so I would suggest if we don't get a response from @atsushi-ishibashi I will take out the needed parts and create a new PR.

[JoshiiSinfield]

Hi,
@parabolic

I still think that the subscription resource is a good resource to have? Because it's required to enable the subscription for every account, even though the organisation is only billed once.
What are your thoughts around this, and how do you plan on enabling it for your accounts?

Cheers,
Josh

[parabolic]

Hi @JoshiiSinfield.
I had to double check the SDK for supported operations regarding a subscription but it boils down to this.
You cannot disable a subscription and when enabling it on an account you commit yourself to 1 year subscription. Which is 3000$

This is not testable by the acceptance tests as:

• it cannot be disabled after the acceptance tests have run.
• 3000$ costs will incur upon enabling it.

At most we can add a tooltip on top of the resource documentation that explains all of this.

https://docs.aws.amazon.com/cli/latest/reference/shield/index.html
https://docs.aws.amazon.com/sdk-for-go/api/service/shield/#Shield.UpdateSubscription
https://docs.aws.amazon.com/waf/latest/developerguide/remove-protection.html

[JoshiiSinfield]

Hi @parabolic ,

No problem I understand.

I'll test the PR.

Cheers,
Josh

[robh007]

With that in mind, does that mean that the aws_shield_subscription resource is not going to make it into terraform?

[parabolic]

@robh007 It's not up to me to decide.
Judging from my experience and some other similar tricky services in AWS most likely not.
In any case it would be better if we move this conversation ( along with the thumbs up ) to the other PR since it's more complete and ready to go.

#7721

Cheers!

[bflad]

For folks following this pull request, #7721 is almost ready for merge and includes a new aws_shield_protection resource. Managing the Shield Advanced subscription (e.g. a aws_shield_subscription resource) itself does not seem like a good fit for Terraform as it requires a year long commitment, the DeleteSubscription API is marked as deprecated, and the maintainers would not be able to appropriately test its functionality over time.

Thanks @atsushi-ishibashi for your initial work here!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!