Skip to content

Commit

Permalink
Addresses PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
gdavison committed Jul 13, 2021
1 parent 3556082 commit 4283c8a
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 29 deletions.
12 changes: 9 additions & 3 deletions .semgrep.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 6 additions & 0 deletions aws/internal/service/ec2/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 5 additions & 10 deletions aws/resource_aws_default_security_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -328,32 +328,27 @@ 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)
}
}

if d.HasChange("tags_all") && !d.IsNewResource() {
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)
}
}

Expand Down
4 changes: 1 addition & 3 deletions aws/resource_aws_globalaccelerator_endpoint_group_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package aws

import (
"errors"
"fmt"
"regexp"
"testing"
Expand Down Expand Up @@ -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
}
Expand Down
11 changes: 6 additions & 5 deletions aws/resource_aws_security_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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
Expand Down Expand Up @@ -412,21 +413,21 @@ 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)
}
}

if d.HasChange("tags_all") && !d.IsNewResource() {
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)
}
}

Expand Down
15 changes: 7 additions & 8 deletions aws/resource_aws_security_group_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand Down

0 comments on commit 4283c8a

Please sign in to comment.