provider/aws: Reworked validateArn function to handle empty values

[Ninir]

Context

This is a proposal to fix #10806.

Chronologically, #10356 enforced that kms_key* attributes should be ARNs.
Then, hashicorp/hil#38 added the support for comparison and boolean operations.

Since then, resources were way more flexible and we could set attributes depending on other ones (for instance the env/stack).
As it is not possible to pass null values in resource arguments, the validateArn function is called against an empty value ("") and then returning errors.
Having this empty argument is allowed, as the code is taking care of getting the value if set/changed.

Work

This work adds a validateArnIfSet method, allowing parameters with empty values.
It is just a simple function checking if the value is empty, and running validateArn internally if not.

Tests

If needed, I can add per-resource tests, checking for the parameter validation/emptyness.

• Run aws_cloudtrail acceptance tests

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSCloudTrail_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/12/19 14:39:20 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSCloudTrail_ -timeout 120m
=== RUN   TestAccAWSCloudTrail_importBasic
--- PASS: TestAccAWSCloudTrail_importBasic (56.29s)
=== RUN   TestAccAWSCloudTrail_basic
--- PASS: TestAccAWSCloudTrail_basic (91.50s)
=== RUN   TestAccAWSCloudTrail_enable_logging
--- PASS: TestAccAWSCloudTrail_enable_logging (118.18s)
=== RUN   TestAccAWSCloudTrail_is_multi_region
--- PASS: TestAccAWSCloudTrail_is_multi_region (118.46s)
=== RUN   TestAccAWSCloudTrail_logValidation
--- PASS: TestAccAWSCloudTrail_logValidation (87.33s)
=== RUN   TestAccAWSCloudTrail_kmsKey
--- PASS: TestAccAWSCloudTrail_kmsKey (84.40s)
=== RUN   TestAccAWSCloudTrail_tags
--- PASS: TestAccAWSCloudTrail_tags (118.14s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	674.357s

• Run aws_ebs_volume acceptance tests

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSEBSVolume_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/12/19 15:02:49 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSEBSVolume_ -timeout 120m
=== RUN   TestAccAWSEBSVolume_importBasic
--- PASS: TestAccAWSEBSVolume_importBasic (39.50s)
=== RUN   TestAccAWSEBSVolume_basic
--- PASS: TestAccAWSEBSVolume_basic (37.88s)
=== RUN   TestAccAWSEBSVolume_kmsKey
--- PASS: TestAccAWSEBSVolume_kmsKey (84.51s)
=== RUN   TestAccAWSEBSVolume_NoIops
--- PASS: TestAccAWSEBSVolume_NoIops (39.81s)
=== RUN   TestAccAWSEBSVolume_withTags
--- PASS: TestAccAWSEBSVolume_withTags (38.90s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	240.630s

• Run aws_s3_bucket_object acceptance tests

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSS3BucketObject_'  
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/12/19 15:18:09 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSS3BucketObject_ -timeout 120m
=== RUN   TestAccAWSS3BucketObject_source
--- PASS: TestAccAWSS3BucketObject_source (65.57s)
=== RUN   TestAccAWSS3BucketObject_content
--- PASS: TestAccAWSS3BucketObject_content (68.89s)
=== RUN   TestAccAWSS3BucketObject_withContentCharacteristics
--- PASS: TestAccAWSS3BucketObject_withContentCharacteristics (67.63s)
=== RUN   TestAccAWSS3BucketObject_updates
--- PASS: TestAccAWSS3BucketObject_updates (108.71s)
=== RUN   TestAccAWSS3BucketObject_updatesWithVersioning
--- PASS: TestAccAWSS3BucketObject_updatesWithVersioning (108.61s)
=== RUN   TestAccAWSS3BucketObject_kms
--- PASS: TestAccAWSS3BucketObject_kms (97.77s)
=== RUN   TestAccAWSS3BucketObject_acl
--- PASS: TestAccAWSS3BucketObject_acl (110.39s)
=== RUN   TestAccAWSS3BucketObject_storageClass
--- PASS: TestAccAWSS3BucketObject_storageClass (112.17s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	739.754s              

• Run aws_ses_receipt_rule acceptance tests

$ make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSSESReceiptRule_'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/12/19 15:09:01 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSSESReceiptRule_ -timeout 120m
=== RUN   TestAccAWSSESReceiptRule_basic
--- PASS: TestAccAWSSESReceiptRule_basic (35.93s)
=== RUN   TestAccAWSSESReceiptRule_order
--- PASS: TestAccAWSSESReceiptRule_order (51.97s)
=== RUN   TestAccAWSSESReceiptRule_actions
--- PASS: TestAccAWSSESReceiptRule_actions (63.63s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	151.555s

[stack72]

Hi @Ninir

I am not sure this is the best way to handle this - this is wrapping a validatefunc in another validatefunc. I think the existing func should be extended to handle this IMO

Thoughts?

Paul

[Ninir]

@stack72 Did it, now running acceptance tests, updating the description for each.
Tell me if it's good for you.

Thanks!

[stack72]

This LGTM now @Ninir - thanks

[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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.