From 4283c8a4c9f5e7589b422ec761c36eca25dd823b Mon Sep 17 00:00:00 2001 From: Graham Davison Date: Mon, 12 Jul 2021 22:38:16 -0700 Subject: [PATCH] Addresses PR feedback --- .semgrep.yml | 12 +++++++++--- aws/internal/service/ec2/errors.go | 6 ++++++ aws/resource_aws_default_security_group.go | 15 +++++---------- ...e_aws_globalaccelerator_endpoint_group_test.go | 4 +--- aws/resource_aws_security_group.go | 11 ++++++----- aws/resource_aws_security_group_rule.go | 15 +++++++-------- 6 files changed, 34 insertions(+), 29 deletions(-) diff --git a/.semgrep.yml b/.semgrep.yml index 45f0978901f..718408630ad 100644 --- a/.semgrep.yml +++ b/.semgrep.yml @@ -327,10 +327,16 @@ rules: - aws/resource_aws_athena_*.go - aws/resource_aws_autoscaling_*.go - aws/resource_aws_autoscalingplans_scaling_plan.go - - aws/resource_aws_[b-g]*.go + - aws/resource_aws_[b-ce-g]*.go + - aws/resource_aws_d[a-df-z]*.go + - aws/resource_aws_devicefarm*.go - aws/resource_aws_i*.go - - aws/resource_aws_[k-t]*.go - - aws/resource_aws_[v-x]*.go + - aws/resource_aws_[k-r]*.go + - aws/resource_aws_s[a-df-z3]*.go + - aws/resource_aws_se[d-z]*.go + - aws/resource_aws_sec[a-t]*.go + - aws/resource_aws_securityhub*.go + - aws/resource_aws_[t-x]*.go include: - aws/resource*.go patterns: diff --git a/aws/internal/service/ec2/errors.go b/aws/internal/service/ec2/errors.go index e4c94a55408..d7f9a9a110d 100644 --- a/aws/internal/service/ec2/errors.go +++ b/aws/internal/service/ec2/errors.go @@ -83,6 +83,12 @@ const ( InvalidVpnGatewayIDNotFound = "InvalidVpnGatewayID.NotFound" ) +const ( + ErrCodeInvalidPermissionDuplicate = "InvalidPermission.Duplicate" + ErrCodeInvalidPermissionMalformed = "InvalidPermission.Malformed" + ErrCodeInvalidPermissionNotFound = "InvalidPermission.NotFound" +) + func UnsuccessfulItemError(apiObject *ec2.UnsuccessfulItemError) error { if apiObject == nil { return nil diff --git a/aws/resource_aws_default_security_group.go b/aws/resource_aws_default_security_group.go index 97be5dc68d3..d08c0b858ba 100644 --- a/aws/resource_aws_default_security_group.go +++ b/aws/resource_aws_default_security_group.go @@ -268,7 +268,7 @@ func resourceAwsDefaultSecurityGroupRead(d *schema.ResourceData, meta interface{ ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig group, err := finder.SecurityGroupByID(conn, d.Id()) - if tfresource.NotFound(err) { + if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] Security group (%s) not found, removing from state", d.Id()) d.SetId("") return nil @@ -328,24 +328,19 @@ func resourceAwsDefaultSecurityGroupUpdate(d *schema.ResourceData, meta interfac conn := meta.(*AWSClient).ec2conn group, err := finder.SecurityGroupByID(conn, d.Id()) - if tfresource.NotFound(err) { - log.Printf("[WARN] Security group (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil - } if err != nil { - return err + return fmt.Errorf("error updating Default Security Group (%s): %w", d.Id(), err) } err = resourceAwsSecurityGroupUpdateRules(d, "ingress", meta, group) if err != nil { - return err + return fmt.Errorf("error updating Default Security Group (%s): %w", d.Id(), err) } if d.Get("vpc_id") != nil { err = resourceAwsSecurityGroupUpdateRules(d, "egress", meta, group) if err != nil { - return err + return fmt.Errorf("error updating Default Security Group (%s): %w", d.Id(), err) } } @@ -353,7 +348,7 @@ func resourceAwsDefaultSecurityGroupUpdate(d *schema.ResourceData, meta interfac o, n := d.GetChange("tags_all") if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), o, n); err != nil { - return fmt.Errorf("error updating EC2 Security Group (%s) tags: %w", d.Id(), err) + return fmt.Errorf("error updating Default Security Group (%s) tags: %w", d.Id(), err) } } diff --git a/aws/resource_aws_globalaccelerator_endpoint_group_test.go b/aws/resource_aws_globalaccelerator_endpoint_group_test.go index aac2d49854b..0c12f664605 100644 --- a/aws/resource_aws_globalaccelerator_endpoint_group_test.go +++ b/aws/resource_aws_globalaccelerator_endpoint_group_test.go @@ -1,7 +1,6 @@ package aws import ( - "errors" "fmt" "regexp" "testing" @@ -472,8 +471,7 @@ func testAccCheckGlobalAcceleratorEndpointGroupDeleteGlobalAcceleratorSecurityGr conn := testAccProvider.Meta().(*AWSClient).ec2conn sg, err := ec2finder.SecurityGroupByNameAndVpcID(conn, "GlobalAccelerator", aws.StringValue(vpc.VpcId)) - var nfe *resource.NotFoundError - if errors.As(err, &nfe) { + if tfresource.NotFound(err) { // Already gone. return nil } diff --git a/aws/resource_aws_security_group.go b/aws/resource_aws_security_group.go index ffb90175518..784f43dbf93 100644 --- a/aws/resource_aws_security_group.go +++ b/aws/resource_aws_security_group.go @@ -20,6 +20,7 @@ import ( "github.com/terraform-providers/terraform-provider-aws/aws/internal/hashcode" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" "github.com/terraform-providers/terraform-provider-aws/aws/internal/naming" + tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/waiter" ) @@ -328,7 +329,7 @@ func resourceAwsSecurityGroupCreate(d *schema.ResourceData, meta interface{}) er if err != nil { //If we have a NotFound or InvalidParameterValue, then we are trying to remove the default IPv6 egress of a non-IPv6 //enabled SG - if !tfawserr.ErrCodeEquals(err, "InvalidPermission.NotFound") && !tfawserr.ErrMessageContains(err, "InvalidParameterValue", "remote-ipv6-range") { + if !tfawserr.ErrCodeEquals(err, tfec2.ErrCodeInvalidPermissionNotFound) && !tfawserr.ErrMessageContains(err, tfec2.ErrCodeInvalidParameterValue, "remote-ipv6-range") { return fmt.Errorf("Error revoking default IPv6 egress rule for Security Group (%s): %w", d.Id(), err) } } @@ -345,7 +346,7 @@ func resourceAwsSecurityGroupRead(d *schema.ResourceData, meta interface{}) erro sg, err := finder.SecurityGroupByID(conn, d.Id()) var nfe *resource.NotFoundError - if errors.As(err, &nfe) { + if !d.IsNewResource() && errors.As(err, &nfe) { log.Printf("[WARN] Security group (%s) not found, removing from state", d.Id()) d.SetId("") return nil @@ -412,13 +413,13 @@ func resourceAwsSecurityGroupUpdate(d *schema.ResourceData, meta interface{}) er err = resourceAwsSecurityGroupUpdateRules(d, "ingress", meta, group) if err != nil { - return err + return fmt.Errorf("error updating Security Group (%s): %w", d.Id(), err) } if d.Get("vpc_id") != nil { err = resourceAwsSecurityGroupUpdateRules(d, "egress", meta, group) if err != nil { - return err + return fmt.Errorf("error updating Security Group (%s): %w", d.Id(), err) } } @@ -426,7 +427,7 @@ func resourceAwsSecurityGroupUpdate(d *schema.ResourceData, meta interface{}) er o, n := d.GetChange("tags_all") if err := keyvaluetags.Ec2UpdateTags(conn, d.Id(), o, n); err != nil { - return fmt.Errorf("error updating EC2 Security Group (%s) tags: %w", d.Id(), err) + return fmt.Errorf("error updating Security Group (%s) tags: %w", d.Id(), err) } } diff --git a/aws/resource_aws_security_group_rule.go b/aws/resource_aws_security_group_rule.go index 7f3bfdf8c4d..e6844c405bc 100644 --- a/aws/resource_aws_security_group_rule.go +++ b/aws/resource_aws_security_group_rule.go @@ -16,6 +16,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/hashcode" + tfec2 "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2" "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/ec2/finder" "github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource" ) @@ -211,7 +212,7 @@ func resourceAwsSecurityGroupRuleCreate(d *schema.ResourceData, meta interface{} return fmt.Errorf("Security Group Rule must be type 'ingress' or type 'egress'") } - if tfawserr.ErrCodeEquals(autherr, "InvalidPermission.Duplicate") { + if tfawserr.ErrCodeEquals(autherr, tfec2.ErrCodeInvalidPermissionDuplicate) { return fmt.Errorf(`[WARN] A duplicate Security Group rule was found on (%s). This may be a side effect of a now-fixed Terraform issue causing two security groups with identical attributes but different source_security_group_ids to overwrite each @@ -281,7 +282,7 @@ func resourceAwsSecurityGroupRuleRead(d *schema.ResourceData, meta interface{}) conn := meta.(*AWSClient).ec2conn sg_id := d.Get("security_group_id").(string) sg, err := finder.SecurityGroupByID(conn, sg_id) - if tfresource.NotFound(err) { + if !d.IsNewResource() && tfresource.NotFound(err) { log.Printf("[WARN] Security Group (%s) not found, removing Rule (%s) from state", sg_id, d.Id()) d.SetId("") return nil @@ -308,18 +309,16 @@ func resourceAwsSecurityGroupRuleRead(d *schema.ResourceData, meta interface{}) return err } - if len(rules) == 0 { - log.Printf("[WARN] No %s rules were found for Security Group (%s) looking for Security Group Rule (%s)", - ruleType, *sg.GroupName, d.Id()) + if !d.IsNewResource() && len(rules) == 0 { + log.Printf("[WARN] No %s rules were found for Security Group (%s) looking for Security Group Rule (%s)", ruleType, aws.StringValue(sg.GroupName), d.Id()) d.SetId("") return nil } rule = findRuleMatch(p, rules, isVPC) - if rule == nil { - log.Printf("[DEBUG] Unable to find matching %s Security Group Rule (%s) for Group %s", - ruleType, d.Id(), sg_id) + if !d.IsNewResource() && rule == nil { + log.Printf("[DEBUG] Unable to find matching %s Security Group Rule (%s) for Group %s", ruleType, d.Id(), sg_id) d.SetId("") return nil }