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.

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

[Enhancement]: Changed id to use the ServicePermissionID - the ID of the actual resource. #27640

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 .changelog/27640.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:enhancement
resource/aws_vpc_endpoint_service_allowed_principal: Changed id to use ServicePermissionId
```
5 changes: 3 additions & 2 deletions internal/service/cognitoidp/find.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,12 @@ func FindCognitoUserPoolClientByName(ctx context.Context, conn *cognitoidentityp
return nil, err
}

if err := tfresource.ExpectSingleResult(clientDescs); err != nil {
client, err := tfresource.AssertSingleResult(clientDescs)
if err != nil {
return nil, err
}

return FindCognitoUserPoolClientByID(ctx, conn, userPoolId, aws.StringValue(clientDescs[0].ClientId))
return FindCognitoUserPoolClientByID(ctx, conn, userPoolId, aws.StringValue(client.ClientId))
}

type cognitoUserPoolClientDescriptionNameFilter func(string) (bool, error)
Expand Down
19 changes: 5 additions & 14 deletions internal/service/ec2/find.go
Original file line number Diff line number Diff line change
Expand Up @@ -3345,30 +3345,21 @@ func FindVPCEndpointServicePermissions(ctx context.Context, conn *ec2.EC2, input
return output, nil
}

func FindVPCEndpointServicePermissionsByID(ctx context.Context, conn *ec2.EC2, id string) ([]*ec2.AllowedPrincipal, error) {
func FindVPCEndpointServicePermissionsByServiceID(ctx context.Context, conn *ec2.EC2, id string) ([]*ec2.AllowedPrincipal, error) {
input := &ec2.DescribeVpcEndpointServicePermissionsInput{
ServiceId: aws.String(id),
}

return FindVPCEndpointServicePermissions(ctx, conn, input)
}

func FindVPCEndpointServicePermissionExists(ctx context.Context, conn *ec2.EC2, serviceID, principalARN string) error {
allowedPrincipals, err := FindVPCEndpointServicePermissionsByID(ctx, conn, serviceID)

func FindVPCEndpointServicePermission(ctx context.Context, conn *ec2.EC2, serviceID, principalARN string) (*ec2.AllowedPrincipal, error) {
allowedPrincipals, err := FindVPCEndpointServicePermissionsByServiceID(ctx, conn, serviceID)
if err != nil {
return err
}

for _, v := range allowedPrincipals {
if aws.StringValue(v.Principal) == principalARN {
return nil
}
return nil, err
}

return &retry.NotFoundError{
LastError: fmt.Errorf("VPC Endpoint Service (%s) Principal (%s) not found", serviceID, principalARN),
}
return tfresource.AssertSingleResult(allowedPrincipals)
}

// FindVPCEndpointRouteTableAssociationExists returns NotFoundError if no association for the specified VPC endpoint and route table IDs is found.
Expand Down
2 changes: 1 addition & 1 deletion internal/service/ec2/vpc_endpoint_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ func resourceVPCEndpointServiceRead(ctx context.Context, d *schema.ResourceData,

SetTagsOut(ctx, svcCfg.Tags)

allowedPrincipals, err := FindVPCEndpointServicePermissionsByID(ctx, conn, d.Id())
allowedPrincipals, err := FindVPCEndpointServicePermissionsByServiceID(ctx, conn, d.Id())

if err != nil {
return sdkdiag.AppendErrorf(diags, "reading EC2 VPC Endpoint Service (%s) permissions: %s", d.Id(), err)
Expand Down
14 changes: 9 additions & 5 deletions internal/service/ec2/vpc_endpoint_service_allowed_principal.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package ec2

import (
"context"
"fmt"
"log"

"github.com/aws/aws-sdk-go/aws"
Expand All @@ -11,7 +10,6 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
"github.com/hashicorp/terraform-provider-aws/internal/create"
"github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag"
"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
)
Expand Down Expand Up @@ -45,7 +43,7 @@ func resourceVPCEndpointServiceAllowedPrincipalCreate(ctx context.Context, d *sc
serviceID := d.Get("vpc_endpoint_service_id").(string)
principalARN := d.Get("principal_arn").(string)

_, err := conn.ModifyVpcEndpointServicePermissionsWithContext(ctx, &ec2.ModifyVpcEndpointServicePermissionsInput{
output, err := conn.ModifyVpcEndpointServicePermissionsWithContext(ctx, &ec2.ModifyVpcEndpointServicePermissionsInput{
AddAllowedPrincipals: aws.StringSlice([]string{principalARN}),
ServiceId: aws.String(serviceID),
})
Expand All @@ -54,7 +52,11 @@ func resourceVPCEndpointServiceAllowedPrincipalCreate(ctx context.Context, d *sc
return sdkdiag.AppendErrorf(diags, "modifying EC2 VPC Endpoint Service (%s) permissions: %s", serviceID, err)
}

d.SetId(fmt.Sprintf("a-%s%d", serviceID, create.StringHashcode(principalARN)))
for _, v := range output.AddedPrincipals {
if aws.StringValue(v.Principal) == principalARN {
d.SetId(aws.StringValue(v.ServicePermissionId))
}
}

return append(diags, resourceVPCEndpointServiceAllowedPrincipalRead(ctx, d, meta)...)
}
Expand All @@ -66,7 +68,7 @@ func resourceVPCEndpointServiceAllowedPrincipalRead(ctx context.Context, d *sche
serviceID := d.Get("vpc_endpoint_service_id").(string)
principalARN := d.Get("principal_arn").(string)

err := FindVPCEndpointServicePermissionExists(ctx, conn, serviceID, principalARN)
output, err := FindVPCEndpointServicePermission(ctx, conn, serviceID, principalARN)

if !d.IsNewResource() && tfresource.NotFound(err) {
log.Printf("[WARN] EC2 VPC Endpoint Service Allowed Principal %s not found, removing from state", d.Id())
Expand All @@ -78,6 +80,8 @@ func resourceVPCEndpointServiceAllowedPrincipalRead(ctx context.Context, d *sche
return sdkdiag.AppendErrorf(diags, "reading EC2 VPC Endpoint Service (%s) Allowed Principal (%s): %s", serviceID, principalARN, err)
}

d.SetId(aws.StringValue(output.ServicePermissionId))

return diags
}

Expand Down
133 changes: 124 additions & 9 deletions internal/service/ec2/vpc_endpoint_service_allowed_principal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package ec2_test
import (
"context"
"fmt"
"regexp"
"testing"

"github.com/aws/aws-sdk-go/service/ec2"
Expand All @@ -17,8 +18,9 @@ import (

func TestAccVPCEndpointServiceAllowedPrincipal_basic(t *testing.T) {
ctx := acctest.Context(t)
rName := sdkacctest.RandomWithPrefix("tfacctest")

resourceName := "aws_vpc_endpoint_service_allowed_principal.test"
rName := sdkacctest.RandomWithPrefix("tfacctest") // 32 character limit

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
Expand All @@ -28,8 +30,109 @@ func TestAccVPCEndpointServiceAllowedPrincipal_basic(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccVPCEndpointServiceAllowedPrincipalConfig_basic(rName),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckVPCEndpointServiceAllowedPrincipalExists(ctx, resourceName),
resource.TestMatchResourceAttr(resourceName, "id", regexp.MustCompile(`^vpce-svc-perm-\w{17}$`)),
resource.TestCheckResourceAttrPair(resourceName, "vpc_endpoint_service_id", "aws_vpc_endpoint_service.test", "id"),
resource.TestCheckResourceAttrPair(resourceName, "principal_arn", "data.aws_iam_session_context.current", "issuer_arn"),
),
},
},
})
}

func TestAccVPCEndpointServiceAllowedPrincipal_tags(t *testing.T) {
ctx := acctest.Context(t)
rName := sdkacctest.RandomWithPrefix("tfacctest")

resourceName := "aws_vpc_endpoint_service_allowed_principal.test"
tagResourceName := "aws_ec2_tag.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckVPCEndpointServiceAllowedPrincipalDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccVPCEndpointServiceAllowedPrincipalConfig_tag(rName),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckVPCEndpointServiceAllowedPrincipalExists(ctx, resourceName),
resource.TestCheckResourceAttrPair(tagResourceName, "resource_id", resourceName, "id"),
resource.TestCheckResourceAttr(tagResourceName, "key", "Name"),
resource.TestCheckResourceAttr(tagResourceName, "value", rName),
),
},
},
})
}

func TestAccVPCEndpointServiceAllowedPrincipal_migrateID(t *testing.T) {
ctx := acctest.Context(t)
rName := sdkacctest.RandomWithPrefix("tfacctest")

resourceName := "aws_vpc_endpoint_service_allowed_principal.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID),
CheckDestroy: testAccCheckVPCEndpointServiceAllowedPrincipalDestroy(ctx),
Steps: []resource.TestStep{
{
ExternalProviders: map[string]resource.ExternalProvider{
"aws": {
Source: "hashicorp/aws",
VersionConstraint: "4.63.0",
},
},
Config: testAccVPCEndpointServiceAllowedPrincipalConfig_basic(rName),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckVPCEndpointServiceAllowedPrincipalExists(ctx, resourceName),
),
},
{
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
Config: testAccVPCEndpointServiceAllowedPrincipalConfig_basic(rName),
PlanOnly: true,
},
},
})
}

// Verify that the resource returns an ID usable for creating an `aws_ec2_tag`
func TestAccVPCEndpointServiceAllowedPrincipal_migrateAndTag(t *testing.T) {
ctx := acctest.Context(t)
rName := sdkacctest.RandomWithPrefix("tfacctest")

resourceName := "aws_vpc_endpoint_service_allowed_principal.test"
tagResourceName := "aws_ec2_tag.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, ec2.EndpointsID),
CheckDestroy: testAccCheckVPCEndpointServiceAllowedPrincipalDestroy(ctx),
Steps: []resource.TestStep{
{
ExternalProviders: map[string]resource.ExternalProvider{
"aws": {
Source: "hashicorp/aws",
VersionConstraint: "4.63.0",
},
},
Config: testAccVPCEndpointServiceAllowedPrincipalConfig_basic(rName),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckVPCEndpointServiceAllowedPrincipalExists(ctx, resourceName),
),
},
{
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
Config: testAccVPCEndpointServiceAllowedPrincipalConfig_tag(rName),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckVPCEndpointServiceAllowedPrincipalExists(ctx, resourceName),
resource.TestMatchResourceAttr(resourceName, "id", regexp.MustCompile(`^vpce-svc-perm-\w{17}$`)),
resource.TestCheckResourceAttrPair(tagResourceName, "resource_id", resourceName, "id"),
resource.TestCheckResourceAttr(tagResourceName, "key", "Name"),
resource.TestCheckResourceAttr(tagResourceName, "value", rName),
),
},
},
Expand All @@ -45,7 +148,7 @@ func testAccCheckVPCEndpointServiceAllowedPrincipalDestroy(ctx context.Context)
continue
}

err := tfec2.FindVPCEndpointServicePermissionExists(ctx, conn, rs.Primary.Attributes["vpc_endpoint_service_id"], rs.Primary.Attributes["principal_arn"])
_, err := tfec2.FindVPCEndpointServicePermission(ctx, conn, rs.Primary.Attributes["vpc_endpoint_service_id"], rs.Primary.Attributes["principal_arn"])

if tfresource.NotFound(err) {
continue
Expand Down Expand Up @@ -75,12 +178,15 @@ func testAccCheckVPCEndpointServiceAllowedPrincipalExists(ctx context.Context, n

conn := acctest.Provider.Meta().(*conns.AWSClient).EC2Conn()

return tfec2.FindVPCEndpointServicePermissionExists(ctx, conn, rs.Primary.Attributes["vpc_endpoint_service_id"], rs.Primary.Attributes["principal_arn"])
_, err := tfec2.FindVPCEndpointServicePermission(ctx, conn, rs.Primary.Attributes["vpc_endpoint_service_id"], rs.Primary.Attributes["principal_arn"])

return err
}
}

func testAccVPCEndpointServiceAllowedPrincipalConfig_basic(rName string) string {
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_networkLoadBalancerBase(rName, 1), fmt.Sprintf(`
return acctest.ConfigCompose(
testAccVPCEndpointServiceConfig_networkLoadBalancerBase(rName, 1), `
data "aws_caller_identity" "current" {}

data "aws_iam_session_context" "current" {
Expand All @@ -90,16 +196,25 @@ data "aws_iam_session_context" "current" {
resource "aws_vpc_endpoint_service" "test" {
acceptance_required = false
network_load_balancer_arns = aws_lb.test[*].arn

tags = {
Name = %[1]q
}
}

resource "aws_vpc_endpoint_service_allowed_principal" "test" {
vpc_endpoint_service_id = aws_vpc_endpoint_service.test.id

principal_arn = data.aws_iam_session_context.current.issuer_arn
}
`)
}

func testAccVPCEndpointServiceAllowedPrincipalConfig_tag(rName string) string {
return acctest.ConfigCompose(
testAccVPCEndpointServiceAllowedPrincipalConfig_basic(rName),
fmt.Sprintf(`
resource "aws_ec2_tag" "test" {
resource_id = aws_vpc_endpoint_service_allowed_principal.test.id

key = "Name"
value = %[1]q
}
`, rName))
}
10 changes: 5 additions & 5 deletions internal/tfresource/not_found_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,13 @@ func SingularDataSourceFindError(resourceType string, err error) error {
return fmt.Errorf("reading %s: %w", resourceType, err)
}

func ExpectSingleResult[T any](a []*T) error {
func AssertSingleResult[T any](a []*T) (*T, error) {
if l := len(a); l == 0 {
return NewEmptyResultError(nil)
return nil, NewEmptyResultError(nil)
} else if l > 1 {
return NewTooManyResultsError(l, nil)
return nil, NewTooManyResultsError(l, nil)
} else if a[0] == nil {
return NewEmptyResultError(nil)
return nil, NewEmptyResultError(nil)
}
return nil
return a[0], nil
}