From ac21af638efdbf378f194b88ed12b70b5b0ae50a Mon Sep 17 00:00:00 2001 From: Ian Kosen Date: Fri, 15 Mar 2024 14:53:16 -0700 Subject: [PATCH] feat: Support TargetGroupBinding on targets outside the cluster's VPC (#3479) * feat: Support TargetGroupBinding on targets outside the cluster's VPC * Update docs * Make vpcid optional * Add vpcid missing test case - update docs * Fix failing e2e test * fix to use k8s API convention * Add tests to improve coverage * generate crds --- .../elbv2/v1beta1/targetgroupbinding_types.go | 4 + .../elbv2.k8s.aws_targetgroupbindings.yaml | 4 + .../targetgroupbinding/targetgroupbinding.md | 26 +- .../crds/crds.yaml | 4 + .../elbv2/target_group_binding_manager.go | 1 + pkg/ingress/model_build_target_group.go | 1 + pkg/ingress/model_builder_test.go | 9 +- pkg/model/elbv2/target_group_binding.go | 4 + pkg/service/model_build_target_group.go | 1 + pkg/service/model_builder_test.go | 38 ++- pkg/targetgroupbinding/resource_manager.go | 14 +- webhooks/elbv2/targetgroupbinding_mutator.go | 23 ++ .../elbv2/targetgroupbinding_mutator_test.go | 146 +++++++++++- .../elbv2/targetgroupbinding_validator.go | 30 +++ .../targetgroupbinding_validator_test.go | 223 ++++++++++++++++++ 15 files changed, 520 insertions(+), 8 deletions(-) diff --git a/apis/elbv2/v1beta1/targetgroupbinding_types.go b/apis/elbv2/v1beta1/targetgroupbinding_types.go index d1cf2f8d1..0ec065709 100644 --- a/apis/elbv2/v1beta1/targetgroupbinding_types.go +++ b/apis/elbv2/v1beta1/targetgroupbinding_types.go @@ -145,6 +145,10 @@ type TargetGroupBindingSpec struct { // ipAddressType specifies whether the target group is of type IPv4 or IPv6. If unspecified, it will be automatically inferred. // +optional IPAddressType *TargetGroupIPAddressType `json:"ipAddressType,omitempty"` + + // VpcID is the VPC of the TargetGroup. If unspecified, it will be automatically inferred. + // +optional + VpcID string `json:"vpcID,omitempty"` } // TargetGroupBindingStatus defines the observed state of TargetGroupBinding diff --git a/config/crd/bases/elbv2.k8s.aws_targetgroupbindings.yaml b/config/crd/bases/elbv2.k8s.aws_targetgroupbindings.yaml index f079fedb2..a5943667a 100644 --- a/config/crd/bases/elbv2.k8s.aws_targetgroupbindings.yaml +++ b/config/crd/bases/elbv2.k8s.aws_targetgroupbindings.yaml @@ -386,6 +386,10 @@ spec: - instance - ip type: string + vpcID: + description: VpcID is the VPC of the TargetGroup. If unspecified, + it will be automatically inferred. + type: string required: - serviceRef - targetGroupARN diff --git a/docs/guide/targetgroupbinding/targetgroupbinding.md b/docs/guide/targetgroupbinding/targetgroupbinding.md index a85ff8885..36cdf065d 100644 --- a/docs/guide/targetgroupbinding/targetgroupbinding.md +++ b/docs/guide/targetgroupbinding/targetgroupbinding.md @@ -5,8 +5,8 @@ This will allow you to provision the load balancer infrastructure completely out !!!tip "usage to support Ingress and Service" The AWS LoadBalancer controller internally used TargetGroupBinding to support the functionality for Ingress and Service resource as well. - It automatically creates TargetGroupBinding in the same namespace of the Service used. - + It automatically creates TargetGroupBinding in the same namespace of the Service used. + You can view all TargetGroupBindings in a namespace by `kubectl get targetgroupbindings -n -o wide` @@ -31,6 +31,28 @@ spec: ``` +## VpcID +TargetGroupBinding CR supports the explicit definition of the Virtual Private Cloud (VPC) of your TargetGroup. + +!!!tip "" + If the VpcID is not explicitly specified, a mutating webhook will automatically call AWS API to find the VpcID for your TargetGroup and set it to correct value. + + +## Sample YAML +```yaml +apiVersion: elbv2.k8s.aws/v1beta1 +kind: TargetGroupBinding +metadata: + name: my-tgb +spec: + serviceRef: + name: awesome-service # route traffic to the awesome-service + port: 80 + targetGroupARN: + vpcID: +``` + + ## NodeSelector ### Default Node Selector diff --git a/helm/aws-load-balancer-controller/crds/crds.yaml b/helm/aws-load-balancer-controller/crds/crds.yaml index 4a7a24f40..28e534269 100644 --- a/helm/aws-load-balancer-controller/crds/crds.yaml +++ b/helm/aws-load-balancer-controller/crds/crds.yaml @@ -587,6 +587,10 @@ spec: - instance - ip type: string + vpcID: + description: VpcID is the VPC of the TargetGroup. If unspecified, + it will be automatically inferred. + type: string required: - serviceRef - targetGroupARN diff --git a/pkg/deploy/elbv2/target_group_binding_manager.go b/pkg/deploy/elbv2/target_group_binding_manager.go index 7081fbb2e..8a7fdab01 100644 --- a/pkg/deploy/elbv2/target_group_binding_manager.go +++ b/pkg/deploy/elbv2/target_group_binding_manager.go @@ -187,6 +187,7 @@ func buildK8sTargetGroupBindingSpec(ctx context.Context, resTGB *elbv2model.Targ } k8sTGBSpec.NodeSelector = resTGB.Spec.Template.Spec.NodeSelector k8sTGBSpec.IPAddressType = resTGB.Spec.Template.Spec.IPAddressType + k8sTGBSpec.VpcID = resTGB.Spec.Template.Spec.VpcID return k8sTGBSpec, nil } diff --git a/pkg/ingress/model_build_target_group.go b/pkg/ingress/model_build_target_group.go index b855efb29..32c8e0d4d 100644 --- a/pkg/ingress/model_build_target_group.go +++ b/pkg/ingress/model_build_target_group.go @@ -77,6 +77,7 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingSpec(ctx context.Context, Networking: tgbNetworking, NodeSelector: nodeSelector, IPAddressType: (*elbv2api.TargetGroupIPAddressType)(tg.Spec.IPAddressType), + VpcID: t.vpcID, }, }, } diff --git a/pkg/ingress/model_builder_test.go b/pkg/ingress/model_builder_test.go index f82ceac32..ca55473ba 100644 --- a/pkg/ingress/model_builder_test.go +++ b/pkg/ingress/model_builder_test.go @@ -308,6 +308,7 @@ const baseStackJSON = ` "$ref":"#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/ns-1/ing-1-svc-1:http/status/targetGroupARN" }, "targetType":"instance", + "vpcID": "vpc-dummy", "ipAddressType":"ipv4", "serviceRef":{ "name":"svc-1", @@ -350,6 +351,7 @@ const baseStackJSON = ` }, "targetType":"instance", "ipAddressType":"ipv4", + "vpcID": "vpc-dummy", "serviceRef":{ "name":"svc-2", "port":"http" @@ -390,6 +392,7 @@ const baseStackJSON = ` "$ref":"#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/ns-1/ing-1-svc-3:https/status/targetGroupARN" }, "targetType":"ip", + "vpcID": "vpc-dummy", "ipAddressType":"ipv4", "serviceRef":{ "name":"svc-3", @@ -1131,7 +1134,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) { "port": 443, "protocol": "HTTPS", "sslPolicy": "ELBSecurityPolicy-2016-08", - "mutualAuthentication" : { + "mutualAuthentication" : { "mode" : "off" } } @@ -1442,6 +1445,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) { }, "spec": { "ipAddressType": "ipv4", + "vpcID": "vpc-dummy", "networking": { "ingress": [ { @@ -2429,6 +2433,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) { }, "spec": { "ipAddressType": "ipv6", + "vpcID": "vpc-dummy", "networking": { "ingress": [ { @@ -2695,6 +2700,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) { }, "spec": { "ipAddressType": "ipv4", + "vpcID": "vpc-dummy", "networking": { "ingress": [ { @@ -2854,6 +2860,7 @@ func Test_defaultModelBuilder_Build(t *testing.T) { }, "spec": { "ipAddressType": "ipv4", + "vpcID": "vpc-dummy", "networking": { "ingress": [ { diff --git a/pkg/model/elbv2/target_group_binding.go b/pkg/model/elbv2/target_group_binding.go index 939947614..30d8d159b 100644 --- a/pkg/model/elbv2/target_group_binding.go +++ b/pkg/model/elbv2/target_group_binding.go @@ -103,6 +103,10 @@ type TargetGroupBindingSpec struct { // ipAddressType specifies whether the target group is of type IPv4 or IPv6. If unspecified, it will be automatically inferred. // +optional IPAddressType *elbv2api.TargetGroupIPAddressType `json:"ipAddressType,omitempty"` + + // VpcID is the VPC of the TargetGroup. If unspecified, it will be automatically inferred. + // +optional + VpcID string `json:"vpcID,omitempty"` } // Template for TargetGroupBinding Custom Resource. diff --git a/pkg/service/model_build_target_group.go b/pkg/service/model_build_target_group.go index f03991cb0..16682eb6e 100644 --- a/pkg/service/model_build_target_group.go +++ b/pkg/service/model_build_target_group.go @@ -444,6 +444,7 @@ func (t *defaultModelBuildTask) buildTargetGroupBindingSpec(ctx context.Context, Networking: tgbNetworking, NodeSelector: nodeSelector, IPAddressType: (*elbv2api.TargetGroupIPAddressType)(targetGroup.Spec.IPAddressType), + VpcID: t.vpcID, }, }, }, nil diff --git a/pkg/service/model_builder_test.go b/pkg/service/model_builder_test.go index 3bbe57fcf..2c14c4950 100644 --- a/pkg/service/model_builder_test.go +++ b/pkg/service/model_builder_test.go @@ -233,6 +233,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"ip", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"nlb-ip-svc-tls", "port":80 @@ -380,6 +381,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"ip", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"nlb-ip-svc-tls", "port":80 @@ -598,6 +600,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"ip", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"nlb-ip-svc", "port":80 @@ -664,6 +667,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"ip", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"nlb-ip-svc", "port":83 @@ -942,6 +946,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "$ref":"#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/default/nlb-ip-svc-tls:80/status/targetGroupARN" }, "targetType":"ip", + "vpcID": "vpc-xxx", "ipAddressType":"ipv4", "serviceRef":{ "name":"nlb-ip-svc-tls", @@ -994,6 +999,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"ip", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"nlb-ip-svc-tls", "port":83 @@ -1276,6 +1282,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"instance", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"instance-mode", "port":80 @@ -1317,6 +1324,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"instance", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"instance-mode", "port":83 @@ -1554,6 +1562,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "$ref":"#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/app/traffic-local:80/status/targetGroupARN" }, "targetType":"instance", + "vpcID": "vpc-xxx", "ipAddressType":"ipv4", "serviceRef":{ "name":"traffic-local", @@ -1626,6 +1635,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"instance", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"traffic-local", "port":83 @@ -1815,6 +1825,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"ip", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"nlb-ip-svc-tls", "port":80 @@ -1935,6 +1946,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "spec": { "targetType": "ip", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "targetGroupARN": { "$ref": "#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/default/ip-target:80/status/targetGroupARN" }, @@ -2108,6 +2120,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "spec": { "targetType": "ip", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "targetGroupARN": { "$ref": "#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/default/default-ip-target:80/status/targetGroupARN" }, @@ -2473,6 +2486,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "$ref": "#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/default/traffic-local:80/status/targetGroupARN" }, "targetType": "instance", + "vpcID": "vpc-xxx", "serviceRef": { "name": "traffic-local", "port": 80 @@ -2622,6 +2636,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "$ref": "#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/default/traffic-local:80/status/targetGroupARN" }, "targetType": "instance", + "vpcID": "vpc-xxx", "serviceRef": { "name": "traffic-local", "port": 80 @@ -2824,6 +2839,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "$ref": "#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/awesome/lb-with-class:80/status/targetGroupARN" }, "targetType": "instance", + "vpcID": "vpc-xxx", "serviceRef": { "name": "lb-with-class", "port": 80 @@ -2992,6 +3008,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"ip", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"manual-sg-rule", "port":80 @@ -3145,6 +3162,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"ip", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"nlb-ip-svc-tls", "port":80 @@ -3326,6 +3344,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "$ref":"#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/default/nlb-ip-svc-tls:80/status/targetGroupARN" }, "targetType":"ip", + "vpcID": "vpc-xxx", "ipAddressType":"ipv4", "serviceRef":{ "name":"nlb-ip-svc-tls", @@ -3580,6 +3599,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"ip", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"nlb-ip-svc", "port":80 @@ -3624,6 +3644,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "$ref":"#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/default/nlb-ip-svc:83/status/targetGroupARN" }, "targetType":"ip", + "vpcID": "vpc-xxx", "ipAddressType":"ipv4", "serviceRef":{ "name":"nlb-ip-svc", @@ -3917,6 +3938,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "$ref":"#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/default/nlb-ip-svc-tls:80/status/targetGroupARN" }, "targetType":"ip", + "vpcID": "vpc-xxx", "ipAddressType":"ipv4", "serviceRef":{ "name":"nlb-ip-svc-tls", @@ -3958,6 +3980,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "$ref":"#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/default/nlb-ip-svc-tls:83/status/targetGroupARN" }, "targetType":"ip", + "vpcID": "vpc-xxx", "ipAddressType":"ipv4", "serviceRef":{ "name":"nlb-ip-svc-tls", @@ -4220,6 +4243,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"instance", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"instance-mode", "port":80 @@ -4262,6 +4286,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"instance", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"instance-mode", "port":83 @@ -4557,6 +4582,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "$ref":"#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/app/traffic-local:80/status/targetGroupARN" }, "targetType":"instance", + "vpcID": "vpc-xxx", "ipAddressType":"ipv4", "serviceRef":{ "name":"traffic-local", @@ -4605,6 +4631,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"instance", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"traffic-local", "port":83 @@ -4906,6 +4933,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"instance", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"traffic-local", "port":80 @@ -4953,6 +4981,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { }, "targetType":"instance", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "serviceRef":{ "name":"traffic-local", "port":83 @@ -5135,6 +5164,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "$ref":"#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/default/nlb-ip-svc-tls:80/status/targetGroupARN" }, "targetType":"ip", + "vpcID": "vpc-xxx", "ipAddressType":"ipv4", "serviceRef":{ "name":"nlb-ip-svc-tls", @@ -5275,6 +5305,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "spec": { "targetType": "ip", "ipAddressType":"ipv4", + "vpcID": "vpc-xxx", "targetGroupARN": { "$ref": "#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/default/ip-target:80/status/targetGroupARN" }, @@ -5526,6 +5557,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "$ref": "#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/default/traffic-local:80/status/targetGroupARN" }, "targetType": "instance", + "vpcID": "vpc-xxx", "serviceRef": { "name": "traffic-local", "port": 80 @@ -5709,6 +5741,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "$ref": "#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/default/traffic-local:80/status/targetGroupARN" }, "targetType": "instance", + "vpcID": "vpc-xxx", "serviceRef": { "name": "traffic-local", "port": 80 @@ -5891,6 +5924,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "$ref": "#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/awesome/lb-with-class:80/status/targetGroupARN" }, "targetType": "instance", + "vpcID": "vpc-xxx", "serviceRef": { "name": "lb-with-class", "port": 80 @@ -6085,6 +6119,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "$ref": "#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/default/manual-security-groups:80/status/targetGroupARN" }, "targetType": "ip", + "vpcID": "vpc-xxx", "serviceRef": { "name": "manual-security-groups", "port": 80 @@ -6224,6 +6259,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { "$ref": "#/resources/AWS::ElasticLoadBalancingV2::TargetGroup/default/manual-security-groups:80/status/targetGroupARN" }, "targetType": "ip", + "vpcID": "vpc-xxx", "serviceRef": { "name": "manual-security-groups", "port": 80 @@ -6252,7 +6288,7 @@ func Test_defaultModelBuilderTask_Build(t *testing.T) { } } } - } + } } } `, diff --git a/pkg/targetgroupbinding/resource_manager.go b/pkg/targetgroupbinding/resource_manager.go index 8b439bda0..7f9216625 100644 --- a/pkg/targetgroupbinding/resource_manager.go +++ b/pkg/targetgroupbinding/resource_manager.go @@ -127,6 +127,7 @@ func (m *defaultResourceManager) reconcileWithIPTargetType(ctx context.Context, } tgARN := tgb.Spec.TargetGroupARN + vpcID := tgb.Spec.VpcID targets, err := m.targetsManager.ListTargets(ctx, tgARN) if err != nil { return err @@ -145,7 +146,7 @@ func (m *defaultResourceManager) reconcileWithIPTargetType(ctx context.Context, } } if len(unmatchedEndpoints) > 0 { - if err := m.registerPodEndpoints(ctx, tgARN, unmatchedEndpoints); err != nil { + if err := m.registerPodEndpoints(ctx, tgARN, vpcID, unmatchedEndpoints); err != nil { return err } } @@ -382,8 +383,15 @@ func (m *defaultResourceManager) deregisterTargets(ctx context.Context, tgARN st return m.targetsManager.DeregisterTargets(ctx, tgARN, sdkTargets) } -func (m *defaultResourceManager) registerPodEndpoints(ctx context.Context, tgARN string, endpoints []backend.PodEndpoint) error { - vpcInfo, err := m.vpcInfoProvider.FetchVPCInfo(ctx, m.vpcID) +func (m *defaultResourceManager) registerPodEndpoints(ctx context.Context, tgARN, tgVpcID string, endpoints []backend.PodEndpoint) error { + vpcID := m.vpcID + // Target group is in a different VPC from the cluster's VPC + if tgVpcID != "" && tgVpcID != m.vpcID { + vpcID = tgVpcID + m.logger.Info("registering endpoints using the targetGroup's vpcID", tgVpcID, + "which is different from the cluster's vpcID", m.vpcID) + } + vpcInfo, err := m.vpcInfoProvider.FetchVPCInfo(ctx, vpcID) if err != nil { return err } diff --git a/webhooks/elbv2/targetgroupbinding_mutator.go b/webhooks/elbv2/targetgroupbinding_mutator.go index 2d46faf8a..f798274ac 100644 --- a/webhooks/elbv2/targetgroupbinding_mutator.go +++ b/webhooks/elbv2/targetgroupbinding_mutator.go @@ -43,6 +43,9 @@ func (m *targetGroupBindingMutator) MutateCreate(ctx context.Context, obj runtim if err := m.defaultingIPAddressType(ctx, tgb); err != nil { return nil, err } + if err := m.defaultingVpcID(ctx, tgb); err != nil { + return nil, err + } return tgb, nil } @@ -85,6 +88,18 @@ func (m *targetGroupBindingMutator) defaultingIPAddressType(ctx context.Context, return nil } +func (m *targetGroupBindingMutator) defaultingVpcID(ctx context.Context, tgb *elbv2api.TargetGroupBinding) error { + if tgb.Spec.VpcID != "" { + return nil + } + vpcId, err := m.getVpcIDFromAWS(ctx, tgb.Spec.TargetGroupARN) + if err != nil { + return errors.Wrap(err, "unable to get target group VpcID") + } + tgb.Spec.VpcID = vpcId + return nil +} + func (m *targetGroupBindingMutator) obtainSDKTargetTypeFromAWS(ctx context.Context, tgARN string) (string, error) { targetGroup, err := m.getTargetGroupFromAWS(ctx, tgARN) if err != nil { @@ -125,6 +140,14 @@ func (m *targetGroupBindingMutator) getTargetGroupFromAWS(ctx context.Context, t return tgList[0], nil } +func (m *targetGroupBindingMutator) getVpcIDFromAWS(ctx context.Context, tgARN string) (string, error) { + targetGroup, err := m.getTargetGroupFromAWS(ctx, tgARN) + if err != nil { + return "", err + } + return awssdk.StringValue(targetGroup.VpcId), nil +} + // +kubebuilder:webhook:path=/mutate-elbv2-k8s-aws-v1beta1-targetgroupbinding,mutating=true,failurePolicy=fail,groups=elbv2.k8s.aws,resources=targetgroupbindings,verbs=create;update,versions=v1beta1,name=mtargetgroupbinding.elbv2.k8s.aws,sideEffects=None,webhookVersions=v1,admissionReviewVersions=v1beta1 func (m *targetGroupBindingMutator) SetupWithManager(mgr ctrl.Manager) { diff --git a/webhooks/elbv2/targetgroupbinding_mutator_test.go b/webhooks/elbv2/targetgroupbinding_mutator_test.go index d4d7f498f..65fe47c8b 100644 --- a/webhooks/elbv2/targetgroupbinding_mutator_test.go +++ b/webhooks/elbv2/targetgroupbinding_mutator_test.go @@ -41,7 +41,7 @@ func Test_targetGroupBindingMutator_MutateCreate(t *testing.T) { wantErr error }{ { - name: "targetGroupBinding with TargetType and ipAddressType already set", + name: "targetGroupBinding with TargetType and ipAddressType and vpcID already set", fields: fields{ describeTargetGroupsAsListCalls: nil, }, @@ -51,6 +51,7 @@ func Test_targetGroupBindingMutator_MutateCreate(t *testing.T) { TargetGroupARN: "tg-1", TargetType: &instanceTargetType, IPAddressType: &targetGroupIPAddressTypeIPv4, + VpcID: "vpcid-01", }, }, }, @@ -59,6 +60,7 @@ func Test_targetGroupBindingMutator_MutateCreate(t *testing.T) { TargetGroupARN: "tg-1", TargetType: &instanceTargetType, IPAddressType: &targetGroupIPAddressTypeIPv4, + VpcID: "vpcid-01", }, }, }, @@ -166,6 +168,7 @@ func Test_targetGroupBindingMutator_MutateCreate(t *testing.T) { TargetGroupARN: "tg-1", TargetType: &instanceTargetType, IPAddressType: &targetGroupIPAddressTypeIPv6, + VpcID: "vpcid-01", }, }, }, @@ -174,9 +177,67 @@ func Test_targetGroupBindingMutator_MutateCreate(t *testing.T) { TargetGroupARN: "tg-1", TargetType: &instanceTargetType, IPAddressType: &targetGroupIPAddressTypeIPv6, + VpcID: "vpcid-01", }, }, }, + { + name: "targetGroupBinding with VpcID absent will be defaulted via AWS API", + fields: fields{ + describeTargetGroupsAsListCalls: []describeTargetGroupsAsListCall{ + { + req: &elbv2sdk.DescribeTargetGroupsInput{ + TargetGroupArns: awssdk.StringSlice([]string{"tg-1"}), + }, + resp: []*elbv2sdk.TargetGroup{ + { + VpcId: awssdk.String("vpcid-01"), + }, + }, + }, + }, + }, + args: args{ + obj: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{ + TargetGroupARN: "tg-1", + TargetType: &instanceTargetType, + IPAddressType: &targetGroupIPAddressTypeIPv4, + }, + }, + }, + want: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{ + TargetGroupARN: "tg-1", + TargetType: &instanceTargetType, + IPAddressType: &targetGroupIPAddressTypeIPv4, + VpcID: "vpcid-01", + }, + }, + }, + { + name: "targetGroupBinding with VpcID absent will be defaulted via AWS API - error", + fields: fields{ + describeTargetGroupsAsListCalls: []describeTargetGroupsAsListCall{ + { + req: &elbv2sdk.DescribeTargetGroupsInput{ + TargetGroupArns: awssdk.StringSlice([]string{"tg-1"}), + }, + err: errors.New("vpcid not found"), + }, + }, + }, + args: args{ + obj: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{ + TargetGroupARN: "tg-1", + TargetType: &instanceTargetType, + IPAddressType: &targetGroupIPAddressTypeIPv4, + }, + }, + }, + wantErr: errors.New("unable to get target group VpcID: vpcid not found"), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -432,3 +493,86 @@ func Test_targetGroupBindingMutator_getIPAddressTypeFromAWS(t *testing.T) { }) } } + +func Test_targetGroupBindingMutator_obtainSDKVpcIDFromAWS(t *testing.T) { + type describeTargetGroupsAsListCall struct { + req *elbv2sdk.DescribeTargetGroupsInput + resp []*elbv2sdk.TargetGroup + err error + } + + type fields struct { + describeTargetGroupsAsListCalls []describeTargetGroupsAsListCall + } + type args struct { + tgARN string + } + tests := []struct { + name string + fields fields + args args + want string + wantErr error + }{ + { + name: "fetch vpcid from aws", + fields: fields{ + describeTargetGroupsAsListCalls: []describeTargetGroupsAsListCall{ + { + req: &elbv2sdk.DescribeTargetGroupsInput{ + TargetGroupArns: awssdk.StringSlice([]string{"tg-1"}), + }, + resp: []*elbv2sdk.TargetGroup{ + { + VpcId: awssdk.String("vpcid-01"), + }, + }, + }, + }, + }, + args: args{ + tgARN: "tg-1", + }, + want: "vpcid-01", + }, + { + name: "some error while fetching vpcId", + fields: fields{ + describeTargetGroupsAsListCalls: []describeTargetGroupsAsListCall{ + { + req: &elbv2sdk.DescribeTargetGroupsInput{ + TargetGroupArns: awssdk.StringSlice([]string{"tg-1"}), + }, + err: errors.New("vpcid not found"), + }, + }, + }, + args: args{ + tgARN: "tg-1", + }, + wantErr: errors.New("vpcid not found"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + elbv2Client := services.NewMockELBV2(ctrl) + for _, call := range tt.fields.describeTargetGroupsAsListCalls { + elbv2Client.EXPECT().DescribeTargetGroupsAsList(gomock.Any(), call.req).Return(call.resp, call.err) + } + + m := &targetGroupBindingMutator{ + elbv2Client: elbv2Client, + logger: logr.New(&log.NullLogSink{}), + } + got, err := m.getVpcIDFromAWS(context.Background(), tt.args.tgARN) + if tt.wantErr != nil { + assert.EqualError(t, err, tt.wantErr.Error()) + } else { + assert.NoError(t, err) + assert.Equal(t, tt.want, got) + } + }) + } +} diff --git a/webhooks/elbv2/targetgroupbinding_validator.go b/webhooks/elbv2/targetgroupbinding_validator.go index b24ad3072..58629f3b2 100644 --- a/webhooks/elbv2/targetgroupbinding_validator.go +++ b/webhooks/elbv2/targetgroupbinding_validator.go @@ -55,6 +55,9 @@ func (v *targetGroupBindingValidator) ValidateCreate(ctx context.Context, obj ru if err := v.checkTargetGroupIPAddressType(ctx, tgb); err != nil { return err } + if err := v.checkTargetGroupVpcID(ctx, tgb); err != nil { + return err + } return nil } @@ -108,6 +111,10 @@ func (v *targetGroupBindingValidator) checkImmutableFields(tgb *elbv2api.TargetG if oldTGB.Spec.IPAddressType != nil && tgb.Spec.IPAddressType != nil && (*oldTGB.Spec.IPAddressType) != (*tgb.Spec.IPAddressType) { changedImmutableFields = append(changedImmutableFields, "spec.ipAddressType") } + if (tgb.Spec.VpcID != "" && oldTGB.Spec.VpcID != "" && (tgb.Spec.VpcID) != (oldTGB.Spec.VpcID)) || + (tgb.Spec.VpcID == "") != (oldTGB.Spec.VpcID == "") { + changedImmutableFields = append(changedImmutableFields, "spec.vpcID") + } if len(changedImmutableFields) != 0 { return errors.Errorf("%s update may not change these fields: %s", "TargetGroupBinding", strings.Join(changedImmutableFields, ",")) } @@ -150,6 +157,21 @@ func (v *targetGroupBindingValidator) checkTargetGroupIPAddressType(ctx context. return nil } +// checkTargetGroupVpcID ensures VpcID matches with that on the AWS target group +func (v *targetGroupBindingValidator) checkTargetGroupVpcID(ctx context.Context, tgb *elbv2api.TargetGroupBinding) error { + if tgb.Spec.VpcID == "" { + return nil + } + vpcID, err := v.getVpcIDFromAWS(ctx, tgb.Spec.TargetGroupARN) + if err != nil { + return errors.Wrap(err, "unable to get target group VpcID") + } + if vpcID != tgb.Spec.VpcID { + return errors.Errorf("invalid VpcID %v doesnt match VpcID from TargetGroup %v", tgb.Spec.VpcID, tgb.Spec.TargetGroupARN) + } + return nil +} + // getTargetGroupIPAddressTypeFromAWS returns the target group IP address type of AWS target group func (v *targetGroupBindingValidator) getTargetGroupIPAddressTypeFromAWS(ctx context.Context, tgARN string) (elbv2api.TargetGroupIPAddressType, error) { targetGroup, err := v.getTargetGroupFromAWS(ctx, tgARN) @@ -183,6 +205,14 @@ func (v *targetGroupBindingValidator) getTargetGroupFromAWS(ctx context.Context, return tgList[0], nil } +func (v *targetGroupBindingValidator) getVpcIDFromAWS(ctx context.Context, tgARN string) (string, error) { + targetGroup, err := v.getTargetGroupFromAWS(ctx, tgARN) + if err != nil { + return "", err + } + return awssdk.StringValue(targetGroup.VpcId), nil +} + // +kubebuilder:webhook:path=/validate-elbv2-k8s-aws-v1beta1-targetgroupbinding,mutating=false,failurePolicy=fail,groups=elbv2.k8s.aws,resources=targetgroupbindings,verbs=create;update,versions=v1beta1,name=vtargetgroupbinding.elbv2.k8s.aws,sideEffects=None,webhookVersions=v1,admissionReviewVersions=v1beta1 func (v *targetGroupBindingValidator) SetupWithManager(mgr ctrl.Manager) { diff --git a/webhooks/elbv2/targetgroupbinding_validator_test.go b/webhooks/elbv2/targetgroupbinding_validator_test.go index e8f9c46a3..1cf07a45a 100644 --- a/webhooks/elbv2/targetgroupbinding_validator_test.go +++ b/webhooks/elbv2/targetgroupbinding_validator_test.go @@ -181,6 +181,94 @@ func Test_targetGroupBindingValidator_ValidateCreate(t *testing.T) { }, wantErr: errors.New("invalid IP address type ipv6 for TargetGroup tg-2"), }, + { + name: "VpcID in spec matches with TG vpc", + fields: fields{ + describeTargetGroupsAsListCalls: []describeTargetGroupsAsListCall{ + { + req: &elbv2sdk.DescribeTargetGroupsInput{ + TargetGroupArns: awssdk.StringSlice([]string{"tg-2"}), + }, + resp: []*elbv2sdk.TargetGroup{ + { + TargetGroupArn: awssdk.String("tg-2"), + TargetType: awssdk.String("instance"), + IpAddressType: awssdk.String("ipv6"), + VpcId: awssdk.String("vpcid-02"), + }, + }, + }, + { + req: &elbv2sdk.DescribeTargetGroupsInput{ + TargetGroupArns: awssdk.StringSlice([]string{"tg-2"}), + }, + resp: []*elbv2sdk.TargetGroup{ + { + TargetGroupArn: awssdk.String("tg-2"), + TargetType: awssdk.String("instance"), + IpAddressType: awssdk.String("ipv6"), + VpcId: awssdk.String("vpcid-02"), + }, + }, + }, + }, + }, + args: args{ + obj: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{ + TargetGroupARN: "tg-2", + TargetType: &instanceTargetType, + IPAddressType: &targetGroupIPAddressTypeIPv6, + VpcID: "vpcid-02", + }, + }, + }, + wantErr: nil, + }, + { + name: "VpcID provided doesnt match TG VpcID mismatch", + fields: fields{ + describeTargetGroupsAsListCalls: []describeTargetGroupsAsListCall{ + { + req: &elbv2sdk.DescribeTargetGroupsInput{ + TargetGroupArns: awssdk.StringSlice([]string{"tg-2"}), + }, + resp: []*elbv2sdk.TargetGroup{ + { + TargetGroupArn: awssdk.String("tg-2"), + TargetType: awssdk.String("instance"), + IpAddressType: awssdk.String("ipv6"), + VpcId: awssdk.String("vpcid-02"), + }, + }, + }, + { + req: &elbv2sdk.DescribeTargetGroupsInput{ + TargetGroupArns: awssdk.StringSlice([]string{"tg-2"}), + }, + resp: []*elbv2sdk.TargetGroup{ + { + TargetGroupArn: awssdk.String("tg-2"), + TargetType: awssdk.String("instance"), + IpAddressType: awssdk.String("ipv6"), + VpcId: awssdk.String("vpcid-02"), + }, + }, + }, + }, + }, + args: args{ + obj: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{ + TargetGroupARN: "tg-2", + TargetType: &instanceTargetType, + IPAddressType: &targetGroupIPAddressTypeIPv6, + VpcID: "vpcid-01", + }, + }, + }, + wantErr: errors.New("invalid VpcID vpcid-01 doesnt match VpcID from TargetGroup tg-2"), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -600,6 +688,64 @@ func Test_targetGroupBindingValidator_checkImmutableFields(t *testing.T) { }, wantErr: errors.New("TargetGroupBinding update may not change these fields: spec.ipAddressType"), }, + { + name: "VpcID modified from vpc-01 to vpc-02", + args: args{ + tgb: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{ + TargetGroupARN: "tg-2", + TargetType: &ipTargetType, + VpcID: "vpc-02", + }, + }, + oldTGB: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{ + TargetGroupARN: "tg-2", + TargetType: &ipTargetType, + VpcID: "vpc-01", + }, + }, + }, + wantErr: errors.New("TargetGroupBinding update may not change these fields: spec.vpcID"), + }, + { + name: "VpcID modified from vpc-01 to nil", + args: args{ + tgb: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{ + TargetGroupARN: "tg-2", + TargetType: &ipTargetType, + }, + }, + oldTGB: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{ + TargetGroupARN: "tg-2", + TargetType: &ipTargetType, + VpcID: "vpc-01", + }, + }, + }, + wantErr: errors.New("TargetGroupBinding update may not change these fields: spec.vpcID"), + }, + { + name: "VpcID modified from nil to vpc-01", + args: args{ + tgb: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{ + TargetGroupARN: "tg-2", + TargetType: &ipTargetType, + VpcID: "vpc-01", + }, + }, + oldTGB: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{ + TargetGroupARN: "tg-2", + TargetType: &ipTargetType, + }, + }, + }, + wantErr: errors.New("TargetGroupBinding update may not change these fields: spec.vpcID"), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -906,3 +1052,80 @@ func Test_targetGroupBindingValidator_checkExistingTargetGroups(t *testing.T) { }) } } + +func Test_targetGroupBindingValidator_checkTargetGroupVpcID(t *testing.T) { + type args struct { + obj *elbv2api.TargetGroupBinding + } + type describeTargetGroupsAsListCall struct { + req *elbv2sdk.DescribeTargetGroupsInput + resp []*elbv2sdk.TargetGroup + err error + } + type fields struct { + describeTargetGroupsAsListCalls []describeTargetGroupsAsListCall + } + tests := []struct { + name string + fields fields + args args + wantErr error + }{ + { + name: "[ok] VpcID is not set", + args: args{ + obj: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{}, + }, + }, + wantErr: nil, + }, + { + name: "[err] vpcID is not found", + fields: fields{ + describeTargetGroupsAsListCalls: []describeTargetGroupsAsListCall{ + { + req: &elbv2sdk.DescribeTargetGroupsInput{ + TargetGroupArns: awssdk.StringSlice([]string{"tg-2"}), + }, + err: errors.New("vpcid not found"), + }, + }, + }, + args: args{ + obj: &elbv2api.TargetGroupBinding{ + Spec: elbv2api.TargetGroupBindingSpec{ + TargetGroupARN: "tg-2", + VpcID: "vpcid-01", + }, + }, + }, + wantErr: errors.New("unable to get target group VpcID: vpcid not found"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + k8sSchema := runtime.NewScheme() + clientgoscheme.AddToScheme(k8sSchema) + elbv2api.AddToScheme(k8sSchema) + k8sClient := testclient.NewFakeClientWithScheme(k8sSchema) + elbv2Client := services.NewMockELBV2(ctrl) + for _, call := range tt.fields.describeTargetGroupsAsListCalls { + elbv2Client.EXPECT().DescribeTargetGroupsAsList(gomock.Any(), call.req).Return(call.resp, call.err) + } + v := &targetGroupBindingValidator{ + k8sClient: k8sClient, + elbv2Client: elbv2Client, + logger: logr.New(&log.NullLogSink{}), + } + err := v.checkTargetGroupVpcID(context.Background(), tt.args.obj) + if tt.wantErr != nil { + assert.EqualError(t, err, tt.wantErr.Error()) + } else { + assert.NoError(t, err) + } + }) + } +}