diff --git a/go.mod b/go.mod index 3d31bc302c..f233723cfe 100644 --- a/go.mod +++ b/go.mod @@ -35,7 +35,7 @@ require ( k8s.io/klog/v2 v2.1.0 k8s.io/utils v0.0.0-20210709001253-0e1f9d693477 sigs.k8s.io/aws-iam-authenticator v0.5.3 - sigs.k8s.io/cluster-api v0.3.21 + sigs.k8s.io/cluster-api v0.3.22 sigs.k8s.io/controller-runtime v0.5.14 sigs.k8s.io/yaml v1.2.0 ) diff --git a/go.sum b/go.sum index 021ff14bfa..2d1eb3c302 100644 --- a/go.sum +++ b/go.sum @@ -1186,8 +1186,8 @@ modernc.org/xc v1.0.0/go.mod h1:mRNCo0bvLjGhHO9WsyuKVU4q0ceiDDDoEeWDJHrNx8I= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= sigs.k8s.io/aws-iam-authenticator v0.5.3 h1:EyqQ/uxzbe2mDETZZmuMnv0xHITnyLhZfPlGb6Mma20= sigs.k8s.io/aws-iam-authenticator v0.5.3/go.mod h1:DIq7gy0lvnyaG88AgFyJzUVeix+ia5msHEp4RL0102I= -sigs.k8s.io/cluster-api v0.3.21 h1:AOumFs+wJ/kJDNNzdqk1IQjhM+0x6nZbzaSpytX9Xms= -sigs.k8s.io/cluster-api v0.3.21/go.mod h1:1DBZEj6GmcWxe77d8YOeac1JIa8ttP21uTHUlAyji2g= +sigs.k8s.io/cluster-api v0.3.22 h1:yxl/YHujBqTx+OBcX1FVlJ0DSwXH4LXg8hmDeh8Lick= +sigs.k8s.io/cluster-api v0.3.22/go.mod h1:1DBZEj6GmcWxe77d8YOeac1JIa8ttP21uTHUlAyji2g= sigs.k8s.io/controller-runtime v0.5.14 h1:lmoRaPvLg9877ZOnjFivjtyIdqyLbWfcCEilxHXTEj4= sigs.k8s.io/controller-runtime v0.5.14/go.mod h1:OTqxLuz7gVcrq+BHGUgedRu6b2VIKCEc7Pu4Jbwui0A= sigs.k8s.io/kind v0.7.1-0.20200303021537-981bd80d3802 h1:L6/8hETA7jvdx3xBcbDifrIN2xaYHE7tA58n+Kdp2Zw= diff --git a/pkg/cloud/services/securitygroup/securitygroups.go b/pkg/cloud/services/securitygroup/securitygroups.go index 9180533794..c79205414b 100644 --- a/pkg/cloud/services/securitygroup/securitygroups.go +++ b/pkg/cloud/services/securitygroup/securitygroups.go @@ -326,7 +326,7 @@ func (s *Service) describeClusterOwnedSecurityGroups() ([]infrav1.SecurityGroup, input := &ec2.DescribeSecurityGroupsInput{ Filters: []*ec2.Filter{ filter.EC2.VPC(s.scope.VPC().ID), - filter.EC2.ProviderOwned(s.scope.Name()), + filter.EC2.ClusterOwned(s.scope.Name()), }, } diff --git a/pkg/cloud/services/securitygroup/securitygroups_test.go b/pkg/cloud/services/securitygroup/securitygroups_test.go index 00da2c6bb1..707d0ee203 100644 --- a/pkg/cloud/services/securitygroup/securitygroups_test.go +++ b/pkg/cloud/services/securitygroup/securitygroups_test.go @@ -17,20 +17,23 @@ limitations under the License. package securitygroup import ( - "github.com/pkg/errors" + "context" "strings" "testing" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" "github.com/golang/mock/gomock" + "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/sets" infrav1 "sigs.k8s.io/cluster-api-provider-aws/api/v1alpha3" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/scope" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services" "sigs.k8s.io/cluster-api-provider-aws/pkg/cloud/services/ec2/mock_ec2iface" clusterv1 "sigs.k8s.io/cluster-api/api/v1alpha3" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) func TestReconcileSecurityGroups(t *testing.T) { @@ -385,3 +388,123 @@ func TestControlPlaneSecurityGroupNotOpenToAnyCIDR(t *testing.T) { } } } + +func TestDeleteSecurityGroups(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + testCases := []struct { + name string + input *infrav1.NetworkSpec + expect func(m *mock_ec2iface.MockEC2APIMockRecorder) + err error + }{ + { + name: "do not delete overridden security groups, only delete 'owned' SGs", + input: &infrav1.NetworkSpec{ + VPC: infrav1.VPCSpec{ + ID: "vpc-securitygroups", + InternetGatewayID: aws.String("igw-01"), + }, + Subnets: infrav1.Subnets{ + &infrav1.SubnetSpec{ + ID: "subnet-securitygroups-private", + IsPublic: false, + AvailabilityZone: "us-east-1a", + }, + &infrav1.SubnetSpec{ + ID: "subnet-securitygroups-public", + IsPublic: true, + NatGatewayID: aws.String("nat-01"), + AvailabilityZone: "us-east-1a", + }, + }, + SecurityGroupOverrides: map[infrav1.SecurityGroupRole]string{ + infrav1.SecurityGroupBastion: "sg-bastion", + infrav1.SecurityGroupAPIServerLB: "sg-apiserver-lb", + infrav1.SecurityGroupLB: "sg-lb", + infrav1.SecurityGroupControlPlane: "sg-control", + infrav1.SecurityGroupNode: "sg-node", + }, + }, + expect: func(m *mock_ec2iface.MockEC2APIMockRecorder) { + m.DescribeSecurityGroupsPages(gomock.Any(), gomock.Any()).Do(func(_, y interface{}) { + funct := y.(func(output *ec2.DescribeSecurityGroupsOutput, lastPage bool) bool) + funct(&ec2.DescribeSecurityGroupsOutput{ + SecurityGroups: []*ec2.SecurityGroup{ + { + GroupName: aws.String("sg-bastion"), + GroupId: aws.String("sg-bastion"), + + Tags: []*ec2.Tag{ + { + Key: aws.String("Name"), + Value: aws.String("test-cluster-nat"), + }, + { + Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/cluster/test-cluster"), + Value: aws.String("owned"), + }, + { + Key: aws.String("sigs.k8s.io/cluster-api-provider-aws/role"), + Value: aws.String("common"), + }, + }, + }, + }, + }, true) + }).Return(nil) + + m.DescribeSecurityGroups(gomock.Any()).Return(&ec2.DescribeSecurityGroupsOutput{}, nil) + m.DeleteSecurityGroup(gomock.Any()).Return(nil, nil) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ec2Mock := mock_ec2iface.NewMockEC2API(mockCtrl) + + scheme := runtime.NewScheme() + _ = infrav1.AddToScheme(scheme) + awsCluster := &infrav1.AWSCluster{ + TypeMeta: metav1.TypeMeta{ + APIVersion: infrav1.GroupVersion.String(), + Kind: "AWSCluster", + }, + ObjectMeta: metav1.ObjectMeta{Name: "test"}, + Spec: infrav1.AWSClusterSpec{ + NetworkSpec: *tc.input, + }, + } + client := fake.NewFakeClientWithScheme(scheme, awsCluster) + + ctx := context.TODO() + client.Create(ctx, awsCluster) + + scope, err := scope.NewClusterScope(scope.ClusterScopeParams{ + Client: client, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "test-cluster"}, + }, + AWSCluster: awsCluster, + }) + if err != nil { + t.Fatalf("Failed to create test context: %v", err) + } + + tc.expect(ec2Mock.EXPECT()) + + s := NewService(scope) + s.EC2Client = ec2Mock + + if err := s.DeleteSecurityGroups(); err != nil && tc.err != nil { + if !strings.Contains(err.Error(), tc.err.Error()) { + t.Fatalf("was expecting error to look like '%v', but got '%v'", tc.err, err) + } + } else if err != nil { + t.Fatalf("got an unexpected error: %v", err) + } + }) + } +} diff --git a/test/e2e/data/e2e_conf.yaml b/test/e2e/data/e2e_conf.yaml index 79567e39cd..d367c54980 100644 --- a/test/e2e/data/e2e_conf.yaml +++ b/test/e2e/data/e2e_conf.yaml @@ -14,11 +14,11 @@ # For more details, run `go run ./cmd/clusterawsadm bootstrap credentials` images: - - name: gcr.io/k8s-staging-cluster-api/cluster-api-controller:v0.3.21 + - name: gcr.io/k8s-staging-cluster-api/cluster-api-controller:v0.3.22 loadBehavior: tryLoad - - name: gcr.io/k8s-staging-cluster-api/kubeadm-bootstrap-controller:v0.3.21 + - name: gcr.io/k8s-staging-cluster-api/kubeadm-bootstrap-controller:v0.3.22 loadBehavior: tryLoad - - name: gcr.io/k8s-staging-cluster-api/kubeadm-control-plane-controller:v0.3.21 + - name: gcr.io/k8s-staging-cluster-api/kubeadm-control-plane-controller:v0.3.22 loadBehavior: tryLoad # Use local dev images built source tree; - name: gcr.io/k8s-staging-cluster-api/capa-manager:e2e @@ -35,9 +35,9 @@ providers: - name: cluster-api type: CoreProvider versions: - - name: v0.3.21 + - name: v0.3.22 # Use manifest from source files - value: "https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.3.21/core-components.yaml" + value: "https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.3.22/core-components.yaml" type: "url" replacements: - old: "imagePullPolicy: Always" @@ -50,9 +50,9 @@ providers: - name: kubeadm type: BootstrapProvider versions: - - name: v0.3.21 + - name: v0.3.22 # Use manifest from source files - value: "https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.3.21/bootstrap-components.yaml" + value: "https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.3.22/bootstrap-components.yaml" type: "url" replacements: - old: "imagePullPolicy: Always" @@ -65,9 +65,9 @@ providers: - name: kubeadm type: ControlPlaneProvider versions: - - name: v0.3.21 + - name: v0.3.22 # Use manifest from source files - value: "https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.3.21/control-plane-components.yaml" + value: "https://github.com/kubernetes-sigs/cluster-api/releases/download/v0.3.22/control-plane-components.yaml" type: "url" replacements: - old: "imagePullPolicy: Always" @@ -116,21 +116,21 @@ providers: - sourcePath: "./infrastructure-aws/cluster-template-nested-multitenancy.yaml" targetName: "cluster-template-nested-multitenancy.yaml" variables: - KUBERNETES_VERSION: "v1.21.2" + KUBERNETES_VERSION: "v1.21.3" CNI: "../../data/cni/calico.yaml" EXP_CLUSTER_RESOURCE_SET: "true" EVENT_BRIDGE_INSTANCE_STATE: "true" AWS_CONTROL_PLANE_MACHINE_TYPE: t3.large AWS_NODE_MACHINE_TYPE: t3.large AWS_SSH_KEY_NAME: "cluster-api-provider-aws-sigs-k8s-io" - CONFORMANCE_CI_ARTIFACTS_KUBERNETES_VERSION: "v1.21.2" + CONFORMANCE_CI_ARTIFACTS_KUBERNETES_VERSION: "v1.21.3" CONFORMANCE_WORKER_MACHINE_COUNT: "5" CONFORMANCE_CONTROL_PLANE_MACHINE_COUNT: "1" EXP_MACHINE_POOL: "true" ETCD_VERSION_UPGRADE_TO: "3.4.13-0" COREDNS_VERSION_UPGRADE_TO: "1.7.1" - KUBERNETES_VERSION_UPGRADE_TO: "v1.21.2" - KUBERNETES_VERSION_UPGRADE_FROM: "v1.20.8" + KUBERNETES_VERSION_UPGRADE_TO: "v1.21.3" + KUBERNETES_VERSION_UPGRADE_FROM: "v1.20.9" MULTI_TENANCY_ROLE_NAME: "multi-tenancy-role" MULTI_TENANCY_NESTED_ROLE_NAME: "multi-tenancy-nested-role"