provider/aws: data source for AWS Security Group #9604

mathieuherbert · 2016-10-25T18:03:02Z

To add Security Group data source for AWS provider.

data "aws_security_group" "by_tag" { 
  tags {
    Name = "${aws_security_group.test.tags["Name"]}"
  }
}

The acceptance test

vagrant@terraform:/opt/gopath/src/github.com/hashicorp/terraform$ TF_ACC=true go test -v github.com/hashicorp/terraform/builtin/providers/aws -run ^TestAccDataSourceAwsSecurityGroup$
=== RUN   TestAccDataSourceAwsSecurityGroup
--- PASS: TestAccDataSourceAwsSecurityGroup (25.67s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    25.687s

It's my first pull request, don't hesitate to tell me what can be improved.

sebiwi · 2016-10-26T08:39:23Z

builtin/providers/aws/data_source_aws_security_group.go

+		return fmt.Errorf("no matching SecurityGroup found")
+	}
+	if len(resp.SecurityGroups) > 1 {
+		return fmt.Errorf("multiple Security Groups matched; use additional constraints to reduce matches to a single Security Groups")


You should probably write "...matches to a single Security Group", instead of "...matches to a single Security Groups".

I correct this in f1ac11e commit

kwilczynski · 2016-10-26T09:40:03Z

builtin/providers/aws/data_source_aws_security_group.go

+	conn := meta.(*AWSClient).ec2conn
+	req := &ec2.DescribeSecurityGroupsInput{}
+
+	if id := d.Get("id"); id != "" {


Would GetOk be better here?

I correct this in f1ac11e commit

kwilczynski · 2016-10-26T09:40:58Z

builtin/providers/aws/data_source_aws_security_group.go

+		req.Filters = nil
+	}
+
+	log.Printf("[DEBUG] Describe Security Groups %s\n", req)


There should be %v here most likely.

I correct this in f1ac11e commit

…n and syntax)

mathieuherbert · 2016-10-26T15:31:00Z

Thank you for your comments, I modified the code with these.

kwilczynski · 2016-10-27T16:45:38Z

builtin/providers/aws/data_source_aws_security_group.go

+		Read: dataSourceAwsSecurityGroupRead,
+
+		Schema: map[string]*schema.Schema{
+			"vpc_id": &schema.Schema{


The &schema.Schema can be removed for Go 1.7+.

kwilczynski · 2016-10-27T16:47:32Z

@mathieuherbert hi there! Thank you so much!

The last comment I made is just a nit pick, it is not essential to get this merged.

Great contribution! Thank you again! ❤️

stack72 · 2016-10-27T17:17:56Z

Hi @mathieuherbert

this LGTM! Tests are also great - nice first PR to Terraform :)

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccDataSourceAwsSecurityGroup'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2016/10/27 18:13:59 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccDataSourceAwsSecurityGroup -timeout 120m
=== RUN   TestAccDataSourceAwsSecurityGroup
--- PASS: TestAccDataSourceAwsSecurityGroup (21.74s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    21.765s

* provider/aws: data source for AWS Security Group * provider/aws: add documentation for data source for AWS Security Group * provider/aws: data source for AWS Security Group (improve if condition and syntax) * fix fmt

ghost · 2020-04-21T01:58:11Z

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.

provider/aws: data source for AWS Security Group

a3ffe0c

mathieuherbert force-pushed the master branch from 1255030 to a3ffe0c Compare October 25, 2016 18:38

provider/aws: add documentation for data source for AWS Security Group

b39edfd

sebiwi reviewed Oct 26, 2016

View reviewed changes

kwilczynski reviewed Oct 26, 2016

View reviewed changes

Mathieu Herbert added 2 commits October 26, 2016 14:19

provider/aws: data source for AWS Security Group (improve if conditio…

f1ac11e

…n and syntax)

fix fmt

9169f9b

kwilczynski reviewed Oct 27, 2016

View reviewed changes

stack72 merged commit 7f9baf7 into hashicorp:master Oct 27, 2016

ghost locked and limited conversation to collaborators Apr 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

provider/aws: data source for AWS Security Group #9604

provider/aws: data source for AWS Security Group #9604

mathieuherbert commented Oct 25, 2016 •

edited

Loading

sebiwi Oct 26, 2016

mathieuherbert Oct 27, 2016

kwilczynski Oct 26, 2016

mathieuherbert Oct 27, 2016

kwilczynski Oct 26, 2016

mathieuherbert Oct 27, 2016

mathieuherbert commented Oct 26, 2016

kwilczynski Oct 27, 2016

kwilczynski commented Oct 27, 2016

stack72 commented Oct 27, 2016

ghost commented Apr 21, 2020

provider/aws: data source for AWS Security Group #9604

provider/aws: data source for AWS Security Group #9604

Conversation

mathieuherbert commented Oct 25, 2016 • edited Loading

sebiwi Oct 26, 2016

Choose a reason for hiding this comment

mathieuherbert Oct 27, 2016

Choose a reason for hiding this comment

kwilczynski Oct 26, 2016

Choose a reason for hiding this comment

mathieuherbert Oct 27, 2016

Choose a reason for hiding this comment

kwilczynski Oct 26, 2016

Choose a reason for hiding this comment

mathieuherbert Oct 27, 2016

Choose a reason for hiding this comment

mathieuherbert commented Oct 26, 2016

kwilczynski Oct 27, 2016

Choose a reason for hiding this comment

kwilczynski commented Oct 27, 2016

stack72 commented Oct 27, 2016

ghost commented Apr 21, 2020

mathieuherbert commented Oct 25, 2016 •

edited

Loading