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

Fix: Principal allowed returned multiple when expecting one #30974

Merged
merged 5 commits into from
Apr 26, 2023
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/30974.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_vpc_endpoint_service_allowed_principal: Fix `too many results` error
```
15 changes: 14 additions & 1 deletion internal/service/ec2/find.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
"github.com/hashicorp/terraform-provider-aws/internal/slices"
"github.com/hashicorp/terraform-provider-aws/internal/tfresource"
"github.com/hashicorp/terraform-provider-aws/internal/verify"
)
Expand Down Expand Up @@ -3354,11 +3355,23 @@ func FindVPCEndpointServicePermissionsByServiceID(ctx context.Context, conn *ec2
}

func FindVPCEndpointServicePermission(ctx context.Context, conn *ec2.EC2, serviceID, principalARN string) (*ec2.AllowedPrincipal, error) {
allowedPrincipals, err := FindVPCEndpointServicePermissionsByServiceID(ctx, conn, serviceID)
// Applying a server-side filter on "principal" can lead to errors like
// "An error occurred (InvalidFilter) when calling the DescribeVpcEndpointServicePermissions operation: The filter value arn:aws:iam::123456789012:role/developer contains unsupported characters".
// Apply the filter client-side.
input := &ec2.DescribeVpcEndpointServicePermissionsInput{
ServiceId: aws.String(serviceID),
}

allowedPrincipals, err := FindVPCEndpointServicePermissions(ctx, conn, input)

if err != nil {
return nil, err
}

allowedPrincipals = slices.Filter(allowedPrincipals, func(v *ec2.AllowedPrincipal) bool {
return aws.StringValue(v.Principal) == principalARN
})

return tfresource.AssertSingleResult(allowedPrincipals)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,33 @@ func TestAccVPCEndpointServiceAllowedPrincipal_basic(t *testing.T) {
})
}

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

resourceName := "aws_vpc_endpoint_service_allowed_principal.test"
serviceResourceName := "aws_vpc_endpoint_service.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_Multiple(rName),
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.TestCheckResourceAttr(serviceResourceName, "allowed_principals.#", "1"),
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")
Expand Down Expand Up @@ -185,8 +212,7 @@ func testAccCheckVPCEndpointServiceAllowedPrincipalExists(ctx context.Context, n
}

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

data "aws_iam_session_context" "current" {
Expand All @@ -196,20 +222,55 @@ 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
}
`)
`, rName))
}

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

data "aws_iam_session_context" "current" {
arn = data.aws_caller_identity.current.arn
}

resource "aws_vpc_endpoint_service" "test" {
acceptance_required = false
network_load_balancer_arns = aws_lb.test[*].arn
allowed_principals = ["arn:${data.aws_partition.current.partition}:iam::123456789012:root"]

tags = {
Name = %[1]q
}

lifecycle {
ignore_changes = [
allowed_principals
]
}
}

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
}
`, rName))
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ data "aws_vpc_endpoint_service" "test" {
}

func testAccVPCEndpointServiceDataSourceConfig_customBase(rName string) string {
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_networkLoadBalancerBase(rName, 1), fmt.Sprintf(`
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_baseNetworkLoadBalancer(rName, 1), fmt.Sprintf(`
resource "aws_vpc_endpoint_service" "test" {
acceptance_required = false
network_load_balancer_arns = aws_lb.test[*].arn
Expand Down
64 changes: 13 additions & 51 deletions internal/service/ec2/vpc_endpoint_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -370,28 +370,8 @@ func testAccCheckVPCEndpointServiceExists(ctx context.Context, n string, v *ec2.
}
}

func testAccVPCEndpointServiceConfig_networkLoadBalancerBase(rName string, count int) string {
return acctest.ConfigCompose(acctest.ConfigAvailableAZsNoOptIn(), fmt.Sprintf(`
resource "aws_vpc" "test" {
cidr_block = "10.0.0.0/16"

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

resource "aws_subnet" "test" {
count = 2

vpc_id = aws_vpc.test.id
cidr_block = cidrsubnet(aws_vpc.test.cidr_block, 8, count.index)
availability_zone = data.aws_availability_zones.available.names[count.index]

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

func testAccVPCEndpointServiceConfig_baseNetworkLoadBalancer(rName string, count int) string {
return acctest.ConfigCompose(acctest.ConfigVPCWithSubnets(rName, 2), fmt.Sprintf(`
resource "aws_lb" "test" {
count = %[2]d

Expand All @@ -411,7 +391,7 @@ resource "aws_lb" "test" {
`, rName, count))
}

func testAccVPCEndpointServiceConfig_supportedIPAddressTypesBase(rName string) string {
func testAccVPCEndpointServiceConfig_baseSupportedIPAddressTypes(rName string) string {
return acctest.ConfigCompose(acctest.ConfigAvailableAZsNoOptIn(), fmt.Sprintf(`
resource "aws_vpc" "test" {
cidr_block = "10.0.0.0/16"
Expand Down Expand Up @@ -455,7 +435,7 @@ resource "aws_lb" "test" {
}

func testAccVPCEndpointServiceConfig_basic(rName string) string {
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_networkLoadBalancerBase(rName, 1), `
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_baseNetworkLoadBalancer(rName, 1), `
resource "aws_vpc_endpoint_service" "test" {
acceptance_required = false
network_load_balancer_arns = aws_lb.test[*].arn
Expand All @@ -464,33 +444,15 @@ resource "aws_vpc_endpoint_service" "test" {
}

func testAccVPCEndpointServiceConfig_gatewayLoadBalancerARNs(rName string, count int) string {
return acctest.ConfigCompose(acctest.ConfigAvailableAZsNoOptIn(), fmt.Sprintf(`
resource "aws_vpc" "test" {
cidr_block = "10.10.10.0/25"

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

resource "aws_subnet" "test" {
availability_zone = data.aws_availability_zones.available.names[0]
cidr_block = cidrsubnet(aws_vpc.test.cidr_block, 2, 0)
vpc_id = aws_vpc.test.id

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

return acctest.ConfigCompose(acctest.ConfigVPCWithSubnets(rName, 1), fmt.Sprintf(`
resource "aws_lb" "test" {
count = %[2]d

load_balancer_type = "gateway"
name = "%[1]s-${count.index}"

subnet_mapping {
subnet_id = aws_subnet.test.id
subnet_id = aws_subnet.test[0].id
}
}

Expand All @@ -506,7 +468,7 @@ resource "aws_vpc_endpoint_service" "test" {
}

func testAccVPCEndpointServiceConfig_networkLoadBalancerARNs(rName string, count int) string {
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_networkLoadBalancerBase(rName, count), fmt.Sprintf(`
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_baseNetworkLoadBalancer(rName, count), fmt.Sprintf(`
resource "aws_vpc_endpoint_service" "test" {
acceptance_required = false
network_load_balancer_arns = aws_lb.test[*].arn
Expand All @@ -519,7 +481,7 @@ resource "aws_vpc_endpoint_service" "test" {
}

func testAccVPCEndpointServiceConfig_supportedIPAddressTypesIPv4(rName string) string {
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_supportedIPAddressTypesBase(rName), fmt.Sprintf(`
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_baseSupportedIPAddressTypes(rName), fmt.Sprintf(`
resource "aws_vpc_endpoint_service" "test" {
acceptance_required = false
network_load_balancer_arns = aws_lb.test[*].arn
Expand All @@ -533,7 +495,7 @@ resource "aws_vpc_endpoint_service" "test" {
}

func testAccVPCEndpointServiceConfig_supportedIPAddressTypesIPv4AndIPv6(rName string) string {
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_supportedIPAddressTypesBase(rName), fmt.Sprintf(`
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_baseSupportedIPAddressTypes(rName), fmt.Sprintf(`
resource "aws_vpc_endpoint_service" "test" {
acceptance_required = false
network_load_balancer_arns = aws_lb.test[*].arn
Expand All @@ -547,7 +509,7 @@ resource "aws_vpc_endpoint_service" "test" {
}

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

data "aws_iam_session_context" "current" {
Expand All @@ -568,7 +530,7 @@ resource "aws_vpc_endpoint_service" "test" {
}

func testAccVPCEndpointServiceConfig_tags1(rName, tagKey1, tagValue1 string) string {
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_networkLoadBalancerBase(rName, 1), fmt.Sprintf(`
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_baseNetworkLoadBalancer(rName, 1), fmt.Sprintf(`
resource "aws_vpc_endpoint_service" "test" {
acceptance_required = false
network_load_balancer_arns = aws_lb.test[*].arn
Expand All @@ -581,7 +543,7 @@ resource "aws_vpc_endpoint_service" "test" {
}

func testAccVPCEndpointServiceConfig_tags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string {
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_networkLoadBalancerBase(rName, 1), fmt.Sprintf(`
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_baseNetworkLoadBalancer(rName, 1), fmt.Sprintf(`
resource "aws_vpc_endpoint_service" "test" {
acceptance_required = false
network_load_balancer_arns = aws_lb.test[*].arn
Expand All @@ -595,7 +557,7 @@ resource "aws_vpc_endpoint_service" "test" {
}

func testAccVPCEndpointServiceConfig_privateDNSName(rName, dnsName string) string {
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_networkLoadBalancerBase(rName, 1), fmt.Sprintf(`
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_baseNetworkLoadBalancer(rName, 1), fmt.Sprintf(`
resource "aws_vpc_endpoint_service" "test" {
acceptance_required = false
network_load_balancer_arns = aws_lb.test[*].arn
Expand Down
2 changes: 1 addition & 1 deletion internal/service/ec2/vpc_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -695,7 +695,7 @@ resource "aws_vpc_endpoint" "test" {
}

func testAccVPCEndpointConfig_ipAddressType(rName, addressType string) string {
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_supportedIPAddressTypesBase(rName), fmt.Sprintf(`
return acctest.ConfigCompose(testAccVPCEndpointServiceConfig_baseSupportedIPAddressTypes(rName), fmt.Sprintf(`
resource "aws_vpc_endpoint_service" "test" {
acceptance_required = false
network_load_balancer_arns = aws_lb.test[*].arn
Expand Down