Make elasticache_subnet_group subnet_ids as required argument

[dtan4]

WHAT

Make elasticache_subnet_group subnet_ids as required argument.

WHY

Terraform v0.5.3 describes that elasticache_subnet_group subnet_ids is optional argument.
However, AWS API document (CreateCacheSubnetGroup - Amazon ElastiCache) describes that subnet_ids is required argument.

I tried to create ElastiCache Subnet Group without subnet_ids with the below code:

resource "aws_elasticache_subnet_group" "hoge" {
    name        = "hoge"
    description = "Group for hoge"
}

and then I got the the below error by terraform apply:

aws_elasticache_subnet_group.hoge: Creating...
  description:  "" => "Group for hoge"
  name:         "" => "hoge"
  subnet_ids.#: "" => "<computed>"
aws_elasticache_subnet_group.hoge: Error: 1 error(s) occurred:

* Error creating CacheSubnetGroup: InvalidParameterValue: The parameter SubnetIds must be provided.
    status code: 400, request id: [6107dfa2-1b1f-11e5-a9d3-d5506ffa70a9]
Error applying plan:

1 error(s) occurred:

* 1 error(s) occurred:

* 1 error(s) occurred:

* Error creating CacheSubnetGroup: InvalidParameterValue: The parameter SubnetIds must be provided.
    status code: 400, request id: [6107dfa2-1b1f-11e5-a9d3-d5506ffa70a9]

[radeksimko]

Isn't it really optional for old EC2-Classic accounts which have no knowledge of subnets/VPCs?

It feels like it would be really helpful to have a detection for EC2-Classic / VPC accounts in place, then we could do validation based on that.

[dtan4]

AFAIK this resource is designed for VPC-inside EC cluster only, and Subnet is a part of VPC.

Isn't it really optional for old EC2-Classic accounts which have no knowledge of subnets/VPCs?

Is it expected to create a new Subnet automatically when no subnet is specified? If that is so this is an another bug.

It feels like it would be really helpful to have a detection for EC2-Classic / VPC accounts in place, then we could do validation based on that.

This EC2-classic/VPC validation you mentioned works at terraform apply by current approach. By my approach, this validation works at terraform plan (because EC2-classic user cannot choose any subnets and fill subnet_ids). It seems more useful and helpful.

[radeksimko]

You're right: http://docs.aws.amazon.com/cli/latest/reference/elasticache/create-cache-subnet-group.html
Security groups are for non-VPC only: http://docs.aws.amazon.com/cli/latest/reference/elasticache/create-cache-security-group.html

I don't know what happens if user tries using this in EC2 Classic... but like I said, we don't have validation yet in place anyway and more importantly that's not a subject of this PR.

[radeksimko]

Thanks for noticing! 👍

cc @tmtk75 who originally created this resource.

[radeksimko]

Overall it LGTM, I'm just giving some time to @tmtk75 to react in case he had some reasons to keep it optional. In case of no reaction, it will be merged.

[tmtk75]

@radeksimko Hi, thanks a lot for your notice! I'm totally fine with this change 👍
@dtan4 Thank you for this improvement!

[dtan4]

Great 👍 Thanks for merging ❗

[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.