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

Refactor Pinpoint application resource to use keyvaluetags package #11368

Merged
merged 4 commits into from
Jan 10, 2020
Merged

Refactor Pinpoint application resource to use keyvaluetags package #11368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
641a495
Add tag listing and updating for Pinpoint to keyvaluetags.
Dec 19, 2019
ac75ba0
r/aws_pinpoint_app: Refactor tagging logic to keyvaluetags package.
Dec 19, 2019
b1142a8
Add ServiceTagInputCustomValue for pinpoint.
Jan 7, 2020
1194412
r/aws_pinpoint_app: Fix acceptance tests.
Jan 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions aws/internal/keyvaluetags/generators/listtags/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ var serviceNames = []string{
"neptune",
"opsworks",
"organizations",
"pinpoint",
"qldb",
"rds",
"resourcegroups",
Expand Down Expand Up @@ -391,6 +392,8 @@ func ServiceListTagsOutputTagsField(serviceName string) string {
return "ResourceTags.Tags"
case "neptune":
return "TagList"
case "pinpoint":
return "TagsModel.Tags"
case "rds":
return "TagList"
case "route53":
Expand Down
5 changes: 5 additions & 0 deletions aws/internal/keyvaluetags/generators/updatetags/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ var serviceNames = []string{
"neptune",
"opsworks",
"organizations",
"pinpoint",
"qldb",
"ram",
"rds",
Expand Down Expand Up @@ -495,6 +496,8 @@ func ServiceTagInputTagsField(serviceName string) string {
return "TagList"
case "glue":
return "TagsToAdd"
case "pinpoint":
return "TagsModel"
case "route53":
return "AddTags"
default:
Expand All @@ -507,6 +510,8 @@ func ServiceTagInputCustomValue(serviceName string) string {
switch serviceName {
case "kinesis":
return "aws.StringMap(chunk.IgnoreAws().Map())"
case "pinpoint":
return "&pinpoint.TagsModel{Tags: updatedTags.IgnoreAws().PinpointTags()}"
default:
return ""
}
Expand Down
18 changes: 18 additions & 0 deletions aws/internal/keyvaluetags/list_tags_gen.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 37 additions & 0 deletions aws/internal/keyvaluetags/update_tags_gen.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 16 additions & 3 deletions aws/provider_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,10 +330,23 @@ func testAccEC2VPCOnlyPreCheck(t *testing.T) {
}
}

func testAccHasServicePreCheck(service string, t *testing.T) {
func testAccPartitionHasServicePreCheck(serviceId string, t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rename to better explain purpose.

if partition, ok := endpoints.PartitionForRegion(endpoints.DefaultPartitions(), testAccGetRegion()); ok {
if _, ok := partition.Services()[service]; !ok {
t.Skip(fmt.Sprintf("skipping tests; partition does not support %s service", service))
if _, ok := partition.Services()[serviceId]; !ok {
t.Skip(fmt.Sprintf("skipping tests; partition %s does not support %s service", partition.ID(), serviceId))
}
}
}

func testAccRegionHasServicePreCheck(serviceId string, t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checks that the acceptance test regions supports the specified service.

Copy link
Contributor

Choose a reason for hiding this comment

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

This addition is okay for this and other particular cases, but experience over time has shown the AWS Go SDK endpoints information continually being out of date for some services, which is why we do not use it in many places. We'll always want to prefer behavioral checks (attempting to call an API) over using the endpoint information. 👍 I will add a comment to this function on merge to note that preference.

regionId := testAccGetRegion()
if partition, ok := endpoints.PartitionForRegion(endpoints.DefaultPartitions(), regionId); ok {
service, ok := partition.Services()[serviceId]
if !ok {
t.Skip(fmt.Sprintf("skipping tests; partition %s does not support %s service", partition.ID(), serviceId))
}
if _, ok := service.Regions()[regionId]; !ok {
t.Skip(fmt.Sprintf("skipping tests; region %s does not support %s service", regionId, serviceId))
}
}
}
Expand Down
49 changes: 34 additions & 15 deletions aws/resource_aws_pinpoint_app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/helper/validation"
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
)

func resourceAwsPinpointApp() *schema.Resource {
Expand Down Expand Up @@ -87,20 +88,24 @@ func resourceAwsPinpointApp() *schema.Resource {
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"daily": {
Type: schema.TypeInt,
Optional: true,
Type: schema.TypeInt,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add validations as described here for CampaignLimits.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Optional: true,
ValidateFunc: validation.IntBetween(0, 100),
},
"maximum_duration": {
Type: schema.TypeInt,
Optional: true,
Type: schema.TypeInt,
Optional: true,
ValidateFunc: validation.IntAtLeast(60),
},
"messages_per_second": {
Type: schema.TypeInt,
Optional: true,
Type: schema.TypeInt,
Optional: true,
ValidateFunc: validation.IntBetween(50, 20000),
},
"total": {
Type: schema.TypeInt,
Optional: true,
Type: schema.TypeInt,
Optional: true,
ValidateFunc: validation.IntBetween(0, 100),
},
},
},
Expand Down Expand Up @@ -158,16 +163,16 @@ func resourceAwsPinpointAppCreate(d *schema.ResourceData, meta interface{}) erro
},
}

if v, ok := d.GetOk("tags"); ok {
req.CreateApplicationRequest.Tags = tagsFromMapPinPointApp(v.(map[string]interface{}))
if v := d.Get("tags").(map[string]interface{}); len(v) > 0 {
req.CreateApplicationRequest.Tags = keyvaluetags.New(v).IgnoreAws().PinpointTags()
}

output, err := pinpointconn.CreateApp(req)
if err != nil {
return fmt.Errorf("error creating Pinpoint app: %s", err)
}

d.SetId(*output.ApplicationResponse.Id)
d.SetId(aws.StringValue(output.ApplicationResponse.Id))
d.Set("arn", output.ApplicationResponse.Arn)

return resourceAwsPinpointAppUpdate(d, meta)
Expand Down Expand Up @@ -204,8 +209,15 @@ func resourceAwsPinpointAppUpdate(d *schema.ResourceData, meta interface{}) erro
return err
}

if err := setTagsPinPointApp(conn, d); err != nil {
return fmt.Errorf("error updating PinPoint Application (%s) tags: %s", d.Id(), err)
if !d.IsNewResource() {
arn := d.Get("arn").(string)
if d.HasChange("tags") {
o, n := d.GetChange("tags")

if err := keyvaluetags.PinpointUpdateTags(conn, arn, o, n); err != nil {
return fmt.Errorf("error updating PinPoint Application (%s) tags: %s", arn, err)
}
}
}

return resourceAwsPinpointAppRead(d, meta)
Expand Down Expand Up @@ -242,9 +254,10 @@ func resourceAwsPinpointAppRead(d *schema.ResourceData, meta interface{}) error
return err
}

arn := aws.StringValue(app.ApplicationResponse.Arn)
d.Set("name", app.ApplicationResponse.Name)
d.Set("application_id", app.ApplicationResponse.Id)
d.Set("arn", app.ApplicationResponse.Arn)
d.Set("arn", arn)

if err := d.Set("campaign_hook", flattenPinpointCampaignHook(settings.ApplicationSettingsResource.CampaignHook)); err != nil {
return fmt.Errorf("error setting campaign_hook: %s", err)
Expand All @@ -256,7 +269,13 @@ func resourceAwsPinpointAppRead(d *schema.ResourceData, meta interface{}) error
return fmt.Errorf("error setting quiet_time: %s", err)
}

if err := getTagsPinPointApp(conn, d); err != nil {
tags, err := keyvaluetags.PinpointListTags(conn, arn)

if err != nil {
return fmt.Errorf("error listing tags for PinPoint Application (%s): %s", arn, err)
}

if err := d.Set("tags", tags.IgnoreAws().Map()); err != nil {
return fmt.Errorf("error setting tags: %s", err)
}

Expand Down
Loading