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

when aws_instance security_groups is not a list, terraform crashes #691

Closed
bitglue opened this issue Dec 17, 2014 · 17 comments
Closed

when aws_instance security_groups is not a list, terraform crashes #691

bitglue opened this issue Dec 17, 2014 · 17 comments

Comments

@bitglue
Copy link

bitglue commented Dec 17, 2014

It seems that if aws_instance is provided a string (not a list) for security_groups, terraform crashes on plan or apply.

Example:

resource "aws_instance" "coreos" {
    ami = "${lookup(var.coreos-ami, var.region)}"
    instance_type = "t2.micro"
    count = "${var.cluster-size}"
    subnet_id = "${element(aws_subnet.core.*.id, count.index)}"
    key_name = "phil"
    associate_public_ip_address = true
    user_data = "${file(\"cloud-config\")}"
    security_groups = "${aws_security_group.ssh.id}"

    tags {
        Name = "terraform test"
    }
}

if that's changed to

    security_groups = ["${aws_security_group.ssh.id}"]

then there is no crash.

@svanharmelen
Copy link
Contributor

@bitglue that is correct, but that's inline with the docs and the schema of this resource. So this is actually by design I would say... Of course a user friendly error would be a lot better then a crash, but I guess that is a different issue right?

@bitglue
Copy link
Author

bitglue commented Dec 17, 2014

No, that's the issue here. Crashing is never by design. Invalid user input should be punished by an error with a concise explanation and a line number, not a crash log spew.

@mitchellh
Copy link
Contributor

Agreed with @bitglue. We should fix the crash and show a user friendly error. Tagged as crashing.

@svanharmelen
Copy link
Contributor

Check! So guess I misinterpreted your issue a little 😉

Was just thinking to myself I should double check anyway, as there is a validation being done which clearly misses this one.

Thanks!

@bitglue
Copy link
Author

bitglue commented Dec 17, 2014

I would also add as a related and relatively minor note: the syntax for making "a list" is arcane because:

  • it's not introduced in the getting started guide, and
  • examples are pretty rare

Additionally it's called an "array" instead of a "list" in the page that defines the syntax, which so far as I've found is the only place to call it an "array". The inconsistency in terminology thwarted my Googling attempts and I thought the glob syntax was the only way to make a list until I performed an exhaustive read of the documentation and found the solution by coincidence.

A little clarification of the docs probably would have avoided the mistake that caused me to discover this error in the first place.

@yahyapo
Copy link

yahyapo commented Dec 29, 2014

In fact, I believe this is not only for "security groups" in "aws_instance" but for all other attributes in resources which have their type as Set. There is validation for type "List" but not "Set".

@svanharmelen
Copy link
Contributor

Yes, this is a generic issue that we should fix in the validation of the resource...

@yahyapo
Copy link

yahyapo commented Dec 30, 2014

@mitchellh, I investigated and it seems there is some issue with the below lines of code in resource.go (terraform/terraform/resource.go) file :

func (c *ResourceConfig) IsComputed(k string) bool {
    _, ok := c.get(k, c.Config)
    _, okRaw := c.get(k, c.Raw)
    return !ok && okRaw
}

This is what is causing the crash. I changed the return condition to return !ok and !okRaw and it solved it! But it would be better if you have a look at it as I could not completely comprehend what the above code was actually doing.

@mitchellh
Copy link
Contributor

I'm not sure when this was fixed, but it was fixed in 0.3.6:

terraform master → terraform plan
There are warnings and/or errors related to your configuration. Please
fix these before continuing.

Errors:

  * 'aws_instance.coreos' error: security_groups: should be a list

@yahyapo
Copy link

yahyapo commented Jan 11, 2015

Yes, it is fixed. Thanks!

@ceh
Copy link
Contributor

ceh commented Jan 12, 2015

@mitchellh It was never fixed?

I can reproduce this on both Terraform v0.3.6 and Terraform v0.3.7.dev (a909ce8) by running terraform plan on

provider "aws" {
    access_key = "a"
    secret_key = "b"
    region = "us-east-1"
}

resource "aws_instance" "foo" {
    ami = "ami-foo"
    instance_type = "t2.micro"
    security_groups = "${aws_security_group.foo.name}"
}

resource "aws_security_group" "foo" {
    name = "foobar"
    description = "foobar"
}

@mitchellh
Copy link
Contributor

@ceh Sure, opening it. It was fixed for the repro case in the root comment, but this is new. :)

@mitchellh mitchellh reopened this Jan 12, 2015
@mitchellh
Copy link
Contributor

Fixed by #773

@cheburakshu
Copy link

This seems to be happening still.
I am on Terrform v0.9.2

module "launch_configuration" {
  source = "./modules/aws_launch_configuration"
  name = "${module.cluster.asg_name}"
  image_id = "${var.images[var.region]}"
  iam_instance_profile = "${module.cluster.id}"
  instance_type = "${var.size["minion"]}"
  key_name = "${module.key_pair.key_name}"
# There is a bug. This is not allowing me to send a list.
  security_groups = ["${module.minion_security_group.id}"]
  associate_public_ip_address = "${var.associate_public_ip_address}"
  spot_price = "${var.spot_price}"
  user_data = ""
}

Module:

variable "name" {}
variable "image_id" {}
variable "iam_instance_profile" {}
variable "instance_type" {}
variable "key_name" {}
variable "security_groups" {type="list"}
variable "associate_public_ip_address" {}
variable "spot_price" {}
variable "user_data" {}

resource "aws_launch_configuration" "aws_launch_configuration" {
  name = "${var.name}"
  image_id = "${var.image_id}"
  iam_instance_profile = "${var.iam_instance_profile}"
  instance_type = "${var.instance_type}"
  key_name = "${var.key_name}"
  security_groups = "${var.security_groups}"
  associate_public_ip_address = "${var.associate_public_ip_address}"
  spot_price = "${var.spot_price}"
  user_data = "${var.user_data}"
}

Output:

1 error(s) occurred:

* module.launch_configuration.aws_launch_configuration.aws_launch_configuration: security_groups: should be a list

Change security groups as a list, explicitly in module:

variable "name" {}
variable "image_id" {}
variable "iam_instance_profile" {}
variable "instance_type" {}
variable "key_name" {}
variable "security_groups" {type="list"}
variable "associate_public_ip_address" {}
variable "spot_price" {}
variable "user_data" {}

resource "aws_launch_configuration" "aws_launch_configuration" {
  name = "${var.name}"
  image_id = "${var.image_id}"
  iam_instance_profile = "${var.iam_instance_profile}"
  instance_type = "${var.instance_type}"
  key_name = "${var.key_name}"
# Change as explicit list
  security_groups = ["${var.security_groups}"]
  associate_public_ip_address = "${var.associate_public_ip_address}"
  spot_price = "${var.spot_price}"
  user_data = "${var.user_data}"
}

And it works!

Get: file:///home/sud/cloud/projects/terrakube/modules/aws_launch_configuration
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

@niralkk
Copy link

niralkk commented Dec 11, 2019

thwarted

Googling attempts and I thought the glob syntax was the only way to make a list until I performed an exhaustive read of the documentation and found the solution by coincidence.

Hi @bitglue (Phil) would you mind sharing your findings by any example or the link that you follow. I'm in the same situation.

@bitglue
Copy link
Author

bitglue commented Dec 11, 2019

I don't think I have any configs left still using security_groups -- they all use vpc_security_group_ids now. I don't remember experiencing any crashes in a long time, so I assume it was fixed some time in the past five years.

@ghost
Copy link

ghost commented Dec 12, 2019

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.

@ghost ghost locked and limited conversation to collaborators Dec 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants