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.

Sign up for GitHub

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

Jump to bottom

provider/aws: reading multiple pages of aws_efs_file_system tags #12328

Merged
merged 1 commit into from Mar 1, 2017
Merged

provider/aws: reading multiple pages of aws_efs_file_system tags #12328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 26 additions & 7 deletions builtin/providers/aws/resource_aws_efs_file_system.go
View file
Open in desktop
Expand Up @@ -149,15 +149,34 @@ func resourceAwsEfsFileSystemRead(d *schema.ResourceData, meta interface{}) erro
return fmt.Errorf("EFS file system %q could not be found.", d.Id())
}

tagsResp, err := conn.DescribeTags(&efs.DescribeTagsInput{
FileSystemId: aws.String(d.Id()),
})
if err != nil {
return fmt.Errorf("Error retrieving EC2 tags for EFS file system (%q): %s",
d.Id(), err.Error())
tags := make([]*efs.Tag, 0)
var marker string
for {
params := &efs.DescribeTagsInput{
FileSystemId: aws.String(d.Id()),
}
if marker != "" {
params.Marker = aws.String(marker)
}

tagsResp, err := conn.DescribeTags(params)
if err != nil {
return fmt.Errorf("Error retrieving EC2 tags for EFS file system (%q): %s",
d.Id(), err.Error())
Copy link
Member

@catsby catsby Mar 1, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A thought: log the failure but break out of the for instead of returning an error. This could allow Terraform to continue creating all the things, but possibly show a diff later because there was some error in retrieving all the tags.

Just a thought, maybe you feel erring when failing to retrieve tags here is correct, so I trust/defer to your call here

}

for _, tag := range tagsResp.Tags {
tags = append(tags, tag)
Copy link
Member

@catsby catsby Mar 1, 2017 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style nit: instead of a range here I believe you can just directly append, since tags is []*efs.Tag:

tags = append(tags, tagsResp.Tags...)

}

if tagsResp.NextMarker != nil {
marker = *tagsResp.NextMarker
} else {
break
}
}

err = d.Set("tags", tagsToMapEFS(tagsResp.Tags))
err = d.Set("tags", tagsToMapEFS(tags))
if err != nil {
return err
}
Expand Down
51 changes: 48 additions & 3 deletions builtin/providers/aws/resource_aws_efs_file_system_test.go
View file
Open in desktop
Expand Up @@ -87,7 +87,7 @@ func TestAccAWSEFSFileSystem_basic(t *testing.T) {
Providers: testAccProviders,
CheckDestroy: testAccCheckEfsFileSystemDestroy,
Steps: []resource.TestStep{
resource.TestStep{
{
Config: testAccAWSEFSFileSystemConfig,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
Expand All @@ -103,7 +103,7 @@ func TestAccAWSEFSFileSystem_basic(t *testing.T) {
),
),
},
resource.TestStep{
{
Config: testAccAWSEFSFileSystemConfigWithTags,
Check: resource.ComposeTestCheckFunc(
testAccCheckEfsFileSystem(
Expand All @@ -122,7 +122,7 @@ func TestAccAWSEFSFileSystem_basic(t *testing.T) {
),
),
},
resource.TestStep{
{
Config: testAccAWSEFSFileSystemConfigWithPerformanceMode,
Check: resource.ComposeTestCheckFunc(
testAccCheckEfsFileSystem(
Expand All @@ -142,6 +142,32 @@ func TestAccAWSEFSFileSystem_basic(t *testing.T) {
})
}

func TestAccAWSEFSFileSystem_pagedTags(t *testing.T) {
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckEfsFileSystemDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEFSFileSystemConfigPagedTags,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(
"aws_efs_file_system.foo",
"tags.%",
"11"),
//testAccCheckEfsFileSystem(
// "aws_efs_file_system.foo",
//),
//testAccCheckEfsFileSystemPerformanceMode(
// "aws_efs_file_system.foo",
// "generalPurpose",
//),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dead code

),
},
},
})
}

func testAccCheckEfsFileSystemDestroy(s *terraform.State) error {
conn := testAccProvider.Meta().(*AWSClient).efsconn
for _, rs := range s.RootModule().Resources {
Expand Down Expand Up @@ -286,6 +312,25 @@ resource "aws_efs_file_system" "foo" {
}
`

const testAccAWSEFSFileSystemConfigPagedTags = `
resource "aws_efs_file_system" "foo" {
creation_token = "radeksimko"
tags {
Name = "foo-efs"
Another = "tag"
Test = "yes"
User = "root"
Page = "1"
Environment = "prod"
CostCenter = "terraform"
AcceptanceTest = "PagedTags"
CreationToken = "radek"
PerfMode = "max"
Region = "us-west-2"
}
}
`

const testAccAWSEFSFileSystemConfigWithTags = `
resource "aws_efs_file_system" "foo-with-tags" {
creation_token = "yada_yada"
Expand Down