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

EC2: Add test for DescribeNetworkAcls using the association.subnet-id filter #7679

Merged

Conversation

marcciosilva
Copy link
Contributor

I attempted to add coverage to DescribeNetworkAcls when using the association.subnet-id filter (since this issue was reported a while ago) to ensure it works as expected.

This also fixes a small bug where network ACLs could be deleted even when associated to subnets (according to AWS's documentation, this cannot happen).

…er, fix small bug where network ACLs could be deleted even when associated to subnets
@marcciosilva marcciosilva marked this pull request as ready for review May 7, 2024 15:28
@bblommers
Copy link
Collaborator

Hi @marcciosilva!

(according to AWS's documentation, this cannot happen)

Did you validate this against AWS as well? As the documentation isn't always correct..

@marcciosilva
Copy link
Contributor Author

Hi @marcciosilva!

(according to AWS's documentation, this cannot happen)

Did you validate this against AWS as well? As the documentation isn't always correct..

Hey @bblommers ! Attaching the terratest golang test (along with the associated terraform files) I used to verify how that worked against AWS (it's a simplified version of what I did here - I create a VPC with a subnet, I attach it to a custom network ACL and then I attempt to delete said network ACL, getting the DependencyViolation error):

main.tf:

provider "aws" {
  region = var.region
}

resource "aws_vpc" "example_vpc" {
  cidr_block = "10.0.0.0/16"
  enable_dns_support   = true
  enable_dns_hostnames = true

  tags = {
    Name = "ExampleVPC"
  }
}

resource "aws_subnet" "example_subnet" {
  vpc_id            = aws_vpc.example_vpc.id
  cidr_block        = "10.0.1.0/24"
  availability_zone = data.aws_availability_zones.available.names[0]

  tags = {
    Name = "ExampleSubnet"
  }
}

resource "aws_network_acl" "custom_acl" {
  vpc_id = aws_vpc.example_vpc.id

  tags = {
    Name = "CustomACL"
  }
}

data "aws_availability_zones" "available" {}

output "vpc_id" {
  value = aws_vpc.example_vpc.id
}

output "subnet_id" {
  value = aws_subnet.example_subnet.id
}

output "custom_acl_id" {
  value = aws_network_acl.custom_acl.id
}

data "aws_caller_identity" "current" {}

output "account_id" {
  value = data.aws_caller_identity.current.account_id
}

variables.tf:

variable "region" {
  description = "The AWS region to create resources in."
  type        = string
  default     = "us-west-1"
}
package test

import (
	"errors"
	"github.com/aws/aws-sdk-go/aws"
	"github.com/aws/aws-sdk-go/aws/awserr"
	"github.com/aws/aws-sdk-go/aws/session"
	"github.com/aws/aws-sdk-go/service/ec2"
	"github.com/aws/aws-sdk-go/service/sts"
	"github.com/gruntwork-io/terratest/modules/terraform"
	"github.com/stretchr/testify/assert"
	"testing"
)

func TestVpcAndSubnet(t *testing.T) {
	awsRegion := "us-west-1"
	terraformOptions := &terraform.Options{
		TerraformDir: "./describe-network-acls/",
		Vars: map[string]interface{}{
			"region": awsRegion,
		},
		NoColor: true,
	}
	defer terraform.Destroy(t, terraformOptions)
	terraform.InitAndApply(t, terraformOptions)
	// Outputs from Terraform
	vpcID := terraform.Output(t, terraformOptions, "vpc_id")
	subnetID := terraform.Output(t, terraformOptions, "subnet_id")
	customAclID := terraform.Output(t, terraformOptions, "custom_acl_id")

	// Create an AWS session
	sess := session.Must(session.NewSession(&aws.Config{
		Region: aws.String(awsRegion),
	}))

	// Create an EC2 service client
	ec2Svc := ec2.New(sess)

	printAwsAccountId(t)

	defaultAclID := getDefaultNetworkAclId(t, ec2Svc, vpcID)

	// Verify subnet is initially associated with the default network ACL
	initialAclID := getNetworkAclForSubnet(t, ec2Svc, subnetID)
	assert.Equal(t, defaultAclID, initialAclID, "Subnet should initially be associated with the default network ACL")

	// Associate subnet with custom network ACL
	associateSubnetWithAcl(t, ec2Svc, subnetID, customAclID)

	// Verify subnet is now associated with the custom network ACL
	updatedAclID := getNetworkAclForSubnet(t, ec2Svc, subnetID)
	assert.Equal(t, customAclID, updatedAclID, "Subnet should now be associated with the custom network ACL")

	// Attempt to delete the custom network ACL, expecting an error
	err := deleteNetworkAcl(ec2Svc, customAclID)
	assert.Error(t, err, "Should not be able to delete a network ACL with associated subnets")
	var awsErr awserr.Error
	errors.As(err, &awsErr)
	assert.Equal(t, "DependencyViolation", awsErr.Code())
	assert.Equal(t, "The networkAcl '"+customAclID+"' has dependencies and cannot be deleted.", awsErr.Message())
}

func getDefaultNetworkAclId(t *testing.T, ec2Svc *ec2.EC2, vpcID string) string {
	input := &ec2.DescribeNetworkAclsInput{
		Filters: []*ec2.Filter{
			{
				Name:   aws.String("vpc-id"),
				Values: []*string{aws.String(vpcID)},
			},
		},
	}

	result, err := ec2Svc.DescribeNetworkAcls(input)
	if err != nil {
		t.Fatal(err)
	}

	for _, acl := range result.NetworkAcls {
		if acl.IsDefault != nil && *acl.IsDefault {
			return *acl.NetworkAclId
		}
	}
	return ""
}

func getNetworkAclForSubnet(t *testing.T, ec2Svc *ec2.EC2, subnetID string) string {
	input := &ec2.DescribeNetworkAclsInput{
		Filters: []*ec2.Filter{
			{
				Name:   aws.String("association.subnet-id"),
				Values: []*string{aws.String(subnetID)},
			},
		},
	}

	result, err := ec2Svc.DescribeNetworkAcls(input)
	if err != nil {
		t.Fatal(err)
	}

	if len(result.NetworkAcls) > 0 && len(result.NetworkAcls[0].Associations) > 0 {
		for _, association := range result.NetworkAcls[0].Associations {
			if association.SubnetId != nil && *association.SubnetId == subnetID {
				return *association.NetworkAclId
			}
		}
	}
	return ""
}

func associateSubnetWithAcl(t *testing.T, ec2Svc *ec2.EC2, subnetID string, aclID string) {
	input := &ec2.ReplaceNetworkAclAssociationInput{
		AssociationId: aws.String(getAssociationIdForSubnet(ec2Svc, subnetID)),
		NetworkAclId:  aws.String(aclID),
	}

	_, err := ec2Svc.ReplaceNetworkAclAssociation(input)
	if err != nil {
		t.Fatal(err)
	}
}

func getAssociationIdForSubnet(ec2Svc *ec2.EC2, subnetID string) string {
	input := &ec2.DescribeNetworkAclsInput{
		Filters: []*ec2.Filter{
			{
				Name:   aws.String("association.subnet-id"),
				Values: []*string{aws.String(subnetID)},
			},
		},
	}

	result, err := ec2Svc.DescribeNetworkAcls(input)
	if err != nil {
		panic(err)
	}

	return *result.NetworkAcls[0].Associations[0].NetworkAclAssociationId
}

func deleteNetworkAcl(ec2Svc *ec2.EC2, aclID string) error {
	input := &ec2.DeleteNetworkAclInput{
		NetworkAclId: aws.String(aclID),
	}

	_, err := ec2Svc.DeleteNetworkAcl(input)
	return err
}

func printAwsAccountId(t *testing.T) {
	sess := session.Must(session.NewSession())
	stsSvc := sts.New(sess)
	input := &sts.GetCallerIdentityInput{}

	result, err := stsSvc.GetCallerIdentity(input)
	if err != nil {
		t.Fatal("Failed to get AWS caller identity:", err)
	}

	t.Log("AWS Account ID:", *result.Account)
}

Copy link
Collaborator

@bblommers bblommers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thank you for fixing this and for adding the test case @marcciosilva!

@bblommers bblommers added this to the 5.0.7 milestone May 8, 2024
@bblommers bblommers merged commit fd3563a into getmoto:master May 8, 2024
38 checks passed
Copy link
Contributor

github-actions bot commented May 8, 2024

This is now part of moto >= 5.0.7.dev36

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants