From 2aa0317c944e895f147fe3cb4a72f69a45f43bd5 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 12 Jan 2022 18:52:37 -0500 Subject: [PATCH 1/6] cloudwatch: ISO-friendly tagging --- .../service/cloudwatch/composite_alarm.go | 46 ++++++++++++++++++- .../service/cloudwatch/{enum.go => const.go} | 4 ++ 2 files changed, 48 insertions(+), 2 deletions(-) rename internal/service/cloudwatch/{enum.go => const.go} (92%) diff --git a/internal/service/cloudwatch/composite_alarm.go b/internal/service/cloudwatch/composite_alarm.go index 2b982546b69..5cfc781c3ba 100644 --- a/internal/service/cloudwatch/composite_alarm.go +++ b/internal/service/cloudwatch/composite_alarm.go @@ -100,12 +100,39 @@ func resourceCompositeAlarmCreate(ctx context.Context, d *schema.ResourceData, m input := expandPutCompositeAlarmInput(d, meta) _, err := conn.PutCompositeAlarmWithContext(ctx, &input) + + // Some partitions (i.e., ISO) may not support tag-on-create + if input.Tags != nil && (tfawserr.ErrCodeContains(err, errCodeAccessDenied) || tfawserr.ErrCodeContains(err, cloudwatch.ErrCodeInternalServiceFault)) { + log.Printf("[WARN] CloudWatch Composite Alarm (%s) create failed (%s) with tags. Trying create without tags.", d.Id(), err) + input.Tags = nil + + _, err = conn.PutCompositeAlarmWithContext(ctx, &input) + } + if err != nil { return diag.Errorf("error creating CloudWatch Composite Alarm (%s): %s", name, err) } d.SetId(name) + defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig + tags := defaultTagsConfig.MergeTags(tftags.New(d.Get("tags").(map[string]interface{}))) + + // Some partitions (i.e., ISO) may not support tag-on-create, attempt tag after create + if input.Tags == nil && len(tags) > 0 { + err := UpdateTags(conn, d.Id(), nil, tags) + + // If default tags only, log and continue. Otherwise, error. + if v, ok := d.GetOk("tags"); (!ok || len(v.(map[string]interface{})) == 0) && (tfawserr.ErrCodeContains(err, errCodeAccessDenied) || tfawserr.ErrCodeContains(err, cloudwatch.ErrCodeInternalServiceFault)) { + log.Printf("[WARN] error adding tags after create for CloudWatch Composite Alarm (%s): %s", d.Id(), err) + return resourceCompositeAlarmRead(ctx, d, meta) + } + + if err != nil { + return diag.Errorf("error creating CloudWatch Composite Alarm (%s) tags: %s", name, err) + } + } + return resourceCompositeAlarmRead(ctx, d, meta) } @@ -156,6 +183,13 @@ func resourceCompositeAlarmRead(ctx context.Context, d *schema.ResourceData, met } tags, err := ListTags(conn, aws.StringValue(alarm.AlarmArn)) + + // Some partitions (i.e., ISO) may not support tagging, giving error + if tfawserr.ErrCodeContains(err, errCodeAccessDenied) || tfawserr.ErrCodeContains(err, cloudwatch.ErrCodeInternalServiceFault) { + log.Printf("[WARN] Unable to list tags for CloudWatch Composite Alarm %s: %s", d.Id(), err) + return nil + } + if err != nil { return diag.Errorf("error listing tags of alarm: %s", err) } @@ -189,8 +223,16 @@ func resourceCompositeAlarmUpdate(ctx context.Context, d *schema.ResourceData, m if d.HasChange("tags_all") { o, n := d.GetChange("tags_all") - if err := UpdateTags(conn, arn, o, n); err != nil { - return diag.Errorf("error updating tags: %s", err) + err := UpdateTags(conn, arn, o, n) + + // Some partitions (i.e., ISO) may not support tagging, giving error + if tfawserr.ErrCodeContains(err, errCodeAccessDenied) || tfawserr.ErrCodeContains(err, cloudwatch.ErrCodeInternalServiceFault) { + log.Printf("[WARN] Unable to update tags for CloudWatch Composite Alarm %s: %s", arn, err) + return resourceCompositeAlarmRead(ctx, d, meta) + } + + if err != nil { + return diag.Errorf("error updating CloudWatch Composite Alarm (%s) tags: %s", arn, err) } } diff --git a/internal/service/cloudwatch/enum.go b/internal/service/cloudwatch/const.go similarity index 92% rename from internal/service/cloudwatch/enum.go rename to internal/service/cloudwatch/const.go index f017be18f25..0ca90cd7b43 100644 --- a/internal/service/cloudwatch/enum.go +++ b/internal/service/cloudwatch/const.go @@ -1,5 +1,9 @@ package cloudwatch +const ( + errCodeAccessDenied = "AccessDenied" +) + const ( lowSampleCountPercentilesEvaluate = "evaluate" lowSampleCountPercentilesmissingDataIgnore = "ignore" From 14228998b6ac2babc0aeab665a1bc7e7f3acbe26 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 12 Jan 2022 18:53:43 -0500 Subject: [PATCH 2/6] Add changelog --- .changelog/22556.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/22556.txt diff --git a/.changelog/22556.txt b/.changelog/22556.txt new file mode 100644 index 00000000000..e0c636886c0 --- /dev/null +++ b/.changelog/22556.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_cloudwatch_composite_alarm: Attempt `tags`-on-create, fallback to tag after create, and allow some `tags` errors to be non-fatal to support non-standard AWS partitions (i.e., ISO) +``` \ No newline at end of file From b6f166439f0be71a22945be4968afc20c802248e Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 12 Jan 2022 19:11:04 -0500 Subject: [PATCH 3/6] metric_alarm: ISO-friendly tagging --- .changelog/22556.txt | 4 ++ .../service/cloudwatch/composite_alarm.go | 5 ++- internal/service/cloudwatch/metric_alarm.go | 45 ++++++++++++++++++- 3 files changed, 51 insertions(+), 3 deletions(-) diff --git a/.changelog/22556.txt b/.changelog/22556.txt index e0c636886c0..3f900694973 100644 --- a/.changelog/22556.txt +++ b/.changelog/22556.txt @@ -1,3 +1,7 @@ ```release-note:enhancement resource/aws_cloudwatch_composite_alarm: Attempt `tags`-on-create, fallback to tag after create, and allow some `tags` errors to be non-fatal to support non-standard AWS partitions (i.e., ISO) +``` + +```release-note:enhancement +resource/aws_cloudwatch_metric_alarm: Attempt `tags`-on-create, fallback to tag after create, and allow some `tags` errors to be non-fatal to support non-standard AWS partitions (i.e., ISO) ``` \ No newline at end of file diff --git a/internal/service/cloudwatch/composite_alarm.go b/internal/service/cloudwatch/composite_alarm.go index 5cfc781c3ba..cda1ecad928 100644 --- a/internal/service/cloudwatch/composite_alarm.go +++ b/internal/service/cloudwatch/composite_alarm.go @@ -120,7 +120,8 @@ func resourceCompositeAlarmCreate(ctx context.Context, d *schema.ResourceData, m // Some partitions (i.e., ISO) may not support tag-on-create, attempt tag after create if input.Tags == nil && len(tags) > 0 { - err := UpdateTags(conn, d.Id(), nil, tags) + arn := d.Get("arn").(string) + err := UpdateTags(conn, arn, nil, tags) // If default tags only, log and continue. Otherwise, error. if v, ok := d.GetOk("tags"); (!ok || len(v.(map[string]interface{})) == 0) && (tfawserr.ErrCodeContains(err, errCodeAccessDenied) || tfawserr.ErrCodeContains(err, cloudwatch.ErrCodeInternalServiceFault)) { @@ -129,7 +130,7 @@ func resourceCompositeAlarmCreate(ctx context.Context, d *schema.ResourceData, m } if err != nil { - return diag.Errorf("error creating CloudWatch Composite Alarm (%s) tags: %s", name, err) + return diag.Errorf("error creating CloudWatch Composite Alarm (%s) tags: %s", d.Id(), err) } } diff --git a/internal/service/cloudwatch/metric_alarm.go b/internal/service/cloudwatch/metric_alarm.go index f3fdd443bee..87f3906b564 100644 --- a/internal/service/cloudwatch/metric_alarm.go +++ b/internal/service/cloudwatch/metric_alarm.go @@ -294,12 +294,41 @@ func resourceMetricAlarmCreate(d *schema.ResourceData, meta interface{}) error { log.Printf("[DEBUG] Creating CloudWatch Metric Alarm: %#v", params) _, err = conn.PutMetricAlarm(¶ms) + + // Some partitions (i.e., ISO) may not support tag-on-create + if params.Tags != nil && (tfawserr.ErrCodeContains(err, errCodeAccessDenied) || tfawserr.ErrCodeContains(err, cloudwatch.ErrCodeInternalServiceFault)) { + log.Printf("[WARN] CloudWatch Metric Alarm (%s) create failed (%s) with tags. Trying create without tags.", d.Id(), err) + params.Tags = nil + + _, err = conn.PutMetricAlarm(¶ms) + } + if err != nil { return fmt.Errorf("Creating metric alarm failed: %w", err) } + d.SetId(d.Get("alarm_name").(string)) log.Println("[INFO] CloudWatch Metric Alarm created") + defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig + tags := defaultTagsConfig.MergeTags(tftags.New(d.Get("tags").(map[string]interface{}))) + + // Some partitions (i.e., ISO) may not support tag-on-create, attempt tag after create + if params.Tags == nil && len(tags) > 0 { + arn := d.Get("arn").(string) + err := UpdateTags(conn, arn, nil, tags) + + // If default tags only, log and continue. Otherwise, error. + if v, ok := d.GetOk("tags"); (!ok || len(v.(map[string]interface{})) == 0) && (tfawserr.ErrCodeContains(err, errCodeAccessDenied) || tfawserr.ErrCodeContains(err, cloudwatch.ErrCodeInternalServiceFault)) { + log.Printf("[WARN] error adding tags after create for CloudWatch Metric Alarm (%s): %s", d.Id(), err) + return resourceMetricAlarmRead(d, meta) + } + + if err != nil { + return fmt.Errorf("error creating CloudWatch Metric Alarm (%s) tags: %s", d.Id(), err) + } + } + return resourceMetricAlarmRead(d, meta) } @@ -372,6 +401,12 @@ func resourceMetricAlarmRead(d *schema.ResourceData, meta interface{}) error { tags = tags.IgnoreAWS().IgnoreConfig(ignoreTagsConfig) + // Some partitions (i.e., ISO) may not support tagging, giving error + if tfawserr.ErrCodeContains(err, errCodeAccessDenied) || tfawserr.ErrCodeContains(err, cloudwatch.ErrCodeInternalServiceFault) { + log.Printf("[WARN] Unable to list tags for CloudWatch Metric Alarm %s: %s", d.Id(), err) + return nil + } + //lintignore:AWSR002 if err := d.Set("tags", tags.RemoveDefaultConfig(defaultTagsConfig).Map()); err != nil { return fmt.Errorf("error setting tags: %w", err) @@ -399,7 +434,15 @@ func resourceMetricAlarmUpdate(d *schema.ResourceData, meta interface{}) error { if d.HasChange("tags_all") { o, n := d.GetChange("tags_all") - if err := UpdateTags(conn, arn, o, n); err != nil { + err := UpdateTags(conn, arn, o, n) + + // Some partitions (i.e., ISO) may not support tagging, giving error + if tfawserr.ErrCodeContains(err, errCodeAccessDenied) || tfawserr.ErrCodeContains(err, cloudwatch.ErrCodeInternalServiceFault) { + log.Printf("[WARN] Unable to update tags for CloudWatch Metric Alarm %s: %s", arn, err) + return resourceMetricAlarmRead(d, meta) + } + + if err != nil { return fmt.Errorf("error updating CloudWatch Metric Alarm (%s) tags: %w", arn, err) } } From 452a3982e56a1d202ee8b488f8ea99714ceecbe2 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 12 Jan 2022 19:26:19 -0500 Subject: [PATCH 4/6] metric_stream: ISO-friendly tagging --- internal/service/cloudwatch/metric_stream.go | 48 +++++++++++++++++--- 1 file changed, 41 insertions(+), 7 deletions(-) diff --git a/internal/service/cloudwatch/metric_stream.go b/internal/service/cloudwatch/metric_stream.go index 375acb4eadc..ed729984f2f 100644 --- a/internal/service/cloudwatch/metric_stream.go +++ b/internal/service/cloudwatch/metric_stream.go @@ -131,7 +131,10 @@ func resourceMetricStreamCreate(ctx context.Context, d *schema.ResourceData, met FirehoseArn: aws.String(d.Get("firehose_arn").(string)), RoleArn: aws.String(d.Get("role_arn").(string)), OutputFormat: aws.String(d.Get("output_format").(string)), - Tags: Tags(tags.IgnoreAWS()), + } + + if len(tags) > 0 { + params.Tags = Tags(tags.IgnoreAWS()) } if v, ok := d.GetOk("include_filter"); ok && v.(*schema.Set).Len() > 0 { @@ -142,13 +145,38 @@ func resourceMetricStreamCreate(ctx context.Context, d *schema.ResourceData, met params.ExcludeFilters = expandCloudWatchMetricStreamFilters(v.(*schema.Set)) } - log.Printf("[DEBUG] Putting CloudWatch MetricStream: %#v", params) - _, err := conn.PutMetricStreamWithContext(ctx, ¶ms) + log.Printf("[DEBUG] Putting CloudWatch Metric Stream: %#v", params) + output, err := conn.PutMetricStreamWithContext(ctx, ¶ms) + + // Some partitions (i.e., ISO) may not support tag-on-create + if params.Tags != nil && (tfawserr.ErrCodeContains(err, errCodeAccessDenied) || tfawserr.ErrCodeContains(err, cloudwatch.ErrCodeInternalServiceFault)) { + log.Printf("[WARN] CloudWatch Metric Stream (%s) create failed (%s) with tags. Trying create without tags.", d.Id(), err) + params.Tags = nil + + output, err = conn.PutMetricStreamWithContext(ctx, ¶ms) + } + if err != nil { return diag.FromErr(fmt.Errorf("putting metric_stream failed: %s", err)) } + d.SetId(name) - log.Println("[INFO] CloudWatch MetricStream put finished") + log.Println("[INFO] CloudWatch Metric Stream put finished") + + // Some partitions (i.e., ISO) may not support tag-on-create, attempt tag after create + if params.Tags == nil && len(tags) > 0 { + err := UpdateTags(conn, aws.StringValue(output.Arn), nil, tags) + + // If default tags only, log and continue. Otherwise, error. + if v, ok := d.GetOk("tags"); (!ok || len(v.(map[string]interface{})) == 0) && (tfawserr.ErrCodeContains(err, errCodeAccessDenied) || tfawserr.ErrCodeContains(err, cloudwatch.ErrCodeInternalServiceFault)) { + log.Printf("[WARN] error adding tags after create for CloudWatch Metric Stream (%s): %s", d.Id(), err) + return resourceMetricStreamRead(ctx, d, meta) + } + + if err != nil { + return diag.Errorf("error creating CloudWatch Metric Stream (%s) tags: %s", d.Id(), err) + } + } return resourceMetricStreamRead(ctx, d, meta) } @@ -198,6 +226,12 @@ func resourceMetricStreamRead(ctx context.Context, d *schema.ResourceData, meta tags, err := ListTags(conn, aws.StringValue(output.Arn)) + // Some partitions (i.e., ISO) may not support tagging, giving error + if tfawserr.ErrCodeContains(err, errCodeAccessDenied) || tfawserr.ErrCodeContains(err, cloudwatch.ErrCodeInternalServiceFault) { + log.Printf("[WARN] Unable to list tags for CloudWatch Metric Stream %s: %s", d.Id(), err) + return nil + } + if err != nil { return diag.FromErr(fmt.Errorf("error listing tags for CloudWatch Metric Stream (%s): %w", d.Id(), err)) } @@ -217,21 +251,21 @@ func resourceMetricStreamRead(ctx context.Context, d *schema.ResourceData, meta } func resourceMetricStreamDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { - log.Printf("[INFO] Deleting CloudWatch MetricStream %s", d.Id()) + log.Printf("[INFO] Deleting CloudWatch Metric Stream %s", d.Id()) conn := meta.(*conns.AWSClient).CloudWatchConn params := cloudwatch.DeleteMetricStreamInput{ Name: aws.String(d.Id()), } if _, err := conn.DeleteMetricStreamWithContext(ctx, ¶ms); err != nil { - return diag.FromErr(fmt.Errorf("error deleting CloudWatch MetricStream: %s", err)) + return diag.FromErr(fmt.Errorf("error deleting CloudWatch Metric Stream: %s", err)) } if _, err := WaitMetricStreamDeleted(ctx, conn, d.Id()); err != nil { return diag.FromErr(fmt.Errorf("error while waiting for CloudWatch Metric Stream (%s) to become deleted: %w", d.Id(), err)) } - log.Printf("[INFO] CloudWatch MetricStream %s deleted", d.Id()) + log.Printf("[INFO] CloudWatch Metric Stream %s deleted", d.Id()) return nil } From 39b07aec02fb68812ecb590282e7d86d8d36079c Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 12 Jan 2022 19:28:36 -0500 Subject: [PATCH 5/6] Update changelog --- .changelog/22556.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.changelog/22556.txt b/.changelog/22556.txt index 3f900694973..c25d5d6a44b 100644 --- a/.changelog/22556.txt +++ b/.changelog/22556.txt @@ -4,4 +4,8 @@ resource/aws_cloudwatch_composite_alarm: Attempt `tags`-on-create, fallback to t ```release-note:enhancement resource/aws_cloudwatch_metric_alarm: Attempt `tags`-on-create, fallback to tag after create, and allow some `tags` errors to be non-fatal to support non-standard AWS partitions (i.e., ISO) +``` + +```release-note:enhancement +resource/aws_cloudwatch_metric_stream: Attempt `tags`-on-create, fallback to tag after create, and allow some `tags` errors to be non-fatal to support non-standard AWS partitions (i.e., ISO) ``` \ No newline at end of file From 4f4350b5e2c027a8216db7ca69ccf340c97d3924 Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 12 Jan 2022 19:57:26 -0500 Subject: [PATCH 6/6] Fix tag-after-create logic --- internal/service/cloudwatch/composite_alarm.go | 9 +++++++-- internal/service/cloudwatch/metric_alarm.go | 18 +++++++++++++----- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/internal/service/cloudwatch/composite_alarm.go b/internal/service/cloudwatch/composite_alarm.go index cda1ecad928..e02dd1c00fa 100644 --- a/internal/service/cloudwatch/composite_alarm.go +++ b/internal/service/cloudwatch/composite_alarm.go @@ -120,8 +120,13 @@ func resourceCompositeAlarmCreate(ctx context.Context, d *schema.ResourceData, m // Some partitions (i.e., ISO) may not support tag-on-create, attempt tag after create if input.Tags == nil && len(tags) > 0 { - arn := d.Get("arn").(string) - err := UpdateTags(conn, arn, nil, tags) + alarm, err := FindCompositeAlarmByName(ctx, conn, name) + + if err != nil { + return diag.Errorf("error reading CloudWatch Composite Alarm (%s): %s", name, err) + } + + err = UpdateTags(conn, aws.StringValue(alarm.AlarmArn), nil, tags) // If default tags only, log and continue. Otherwise, error. if v, ok := d.GetOk("tags"); (!ok || len(v.(map[string]interface{})) == 0) && (tfawserr.ErrCodeContains(err, errCodeAccessDenied) || tfawserr.ErrCodeContains(err, cloudwatch.ErrCodeInternalServiceFault)) { diff --git a/internal/service/cloudwatch/metric_alarm.go b/internal/service/cloudwatch/metric_alarm.go index 87f3906b564..a266db360a7 100644 --- a/internal/service/cloudwatch/metric_alarm.go +++ b/internal/service/cloudwatch/metric_alarm.go @@ -315,17 +315,22 @@ func resourceMetricAlarmCreate(d *schema.ResourceData, meta interface{}) error { // Some partitions (i.e., ISO) may not support tag-on-create, attempt tag after create if params.Tags == nil && len(tags) > 0 { - arn := d.Get("arn").(string) - err := UpdateTags(conn, arn, nil, tags) + resp, err := FindMetricAlarmByName(conn, d.Id()) + + if err != nil { + return fmt.Errorf("while finding metric alarm (%s): %w", d.Id(), err) + } + + err = UpdateTags(conn, aws.StringValue(resp.AlarmArn), nil, tags) // If default tags only, log and continue. Otherwise, error. if v, ok := d.GetOk("tags"); (!ok || len(v.(map[string]interface{})) == 0) && (tfawserr.ErrCodeContains(err, errCodeAccessDenied) || tfawserr.ErrCodeContains(err, cloudwatch.ErrCodeInternalServiceFault)) { - log.Printf("[WARN] error adding tags after create for CloudWatch Metric Alarm (%s): %s", d.Id(), err) + log.Printf("[WARN] could not add tags after create for CloudWatch Metric Alarm (%s): %s", d.Id(), err) return resourceMetricAlarmRead(d, meta) } if err != nil { - return fmt.Errorf("error creating CloudWatch Metric Alarm (%s) tags: %s", d.Id(), err) + return fmt.Errorf("creating CloudWatch Metric Alarm (%s) tags: %w", d.Id(), err) } } @@ -478,7 +483,10 @@ func getPutMetricAlarmInput(d *schema.ResourceData, meta interface{}) cloudwatch ComparisonOperator: aws.String(d.Get("comparison_operator").(string)), EvaluationPeriods: aws.Int64(int64(d.Get("evaluation_periods").(int))), TreatMissingData: aws.String(d.Get("treat_missing_data").(string)), - Tags: Tags(tags.IgnoreAWS()), + } + + if len(tags) > 0 { + params.Tags = Tags(tags.IgnoreAWS()) } if v := d.Get("actions_enabled"); v != nil {