Merge pull request #4243 from zac-nixon/main

clear confusion around using ClusterIP service and Instance targets

Author: k8s-ci-robot

pkg/gateway/model/model_build_target_group.go

@@ -283,6 +283,14 @@ func (builder *targetGroupBuilderImpl) buildTargetGroupSpec(gw *gwv1.Gateway, ro
 	}
 	tgPort := builder.buildTargetGroupPort(targetType, *backend.ServicePort)
 	name := builder.buildTargetGroupName(targetGroupProps, k8s.NamespacedName(gw), route.GetRouteNamespacedName(), k8s.NamespacedName(backend.Service), tgPort, targetType, tgProtocol, tgProtocolVersion)
+
+	if tgPort == 0 {
+		if targetType == elbv2model.TargetTypeIP {
+			return elbv2model.TargetGroupSpec{}, errors.Errorf("TargetGroup port is empty. Are you using the correct service type?")
+		}
+		return elbv2model.TargetGroupSpec{}, errors.Errorf("TargetGroup port is empty. When using Instance targets, your service be must of type 'NodePort' or 'LoadBalancer'")
+	}
+
 	return elbv2model.TargetGroupSpec{
 		Name:                  name,
 		TargetType:            targetType,
@@ -359,10 +367,6 @@ func (builder *targetGroupBuilderImpl) buildTargetGroupIPAddressType(svc *corev1
 // and we do our best to use the most appropriate port as targetGroup's port to avoid UX confusing.
 func (builder *targetGroupBuilderImpl) buildTargetGroupPort(targetType elbv2model.TargetType, svcPort corev1.ServicePort) int32 {
 	if targetType == elbv2model.TargetTypeInstance {
-		// Maybe an error? Because the service has no node port, instance type targets don't work.
-		if svcPort.NodePort == 0 {
-			return 1
-		}
 		return svcPort.NodePort
 	}
 	if svcPort.TargetPort.Type == intstr.Int {

pkg/gateway/model/model_build_target_group_test.go

@@ -263,6 +263,41 @@ func Test_buildTargetGroupSpec(t *testing.T) {
 				Tags:                  make(map[string]string),
 			},
 		},
+		{
+			name:                     "wrong svc type for instance should produce an error",
+			tags:                     make(map[string]string),
+			lbType:                   elbv2model.LoadBalancerTypeNetwork,
+			disableRestrictedSGRules: false,
+			defaultTargetType:        string(elbv2model.TargetTypeInstance),
+			gateway: &gwv1.Gateway{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "my-gw-ns",
+					Name:      "my-gw",
+				},
+			},
+			route: &routeutils.MockRoute{
+				Kind:      routeutils.TCPRouteKind,
+				Name:      "my-route",
+				Namespace: "my-route-ns",
+			},
+			backend: routeutils.Backend{
+				Service: &corev1.Service{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "my-svc-ns",
+						Name:      "my-svc",
+					},
+				},
+				ServicePort: &corev1.ServicePort{
+					Protocol: corev1.ProtocolTCP,
+					Port:     80,
+					TargetPort: intstr.IntOrString{
+						IntVal: 80,
+						Type:   intstr.Int,
+					},
+				},
+			},
+			expectErr: true,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -1207,7 +1242,7 @@ func Test_buildTargetGroupPort(t *testing.T) {
 			name:       "instance - no node port",
 			svcPort:    corev1.ServicePort{},
 			targetType: elbv2model.TargetTypeInstance,
-			expected:   1,
+			expected:   0,
 		},
 		{
 			name: "ip",

pkg/ingress/model_build_target_group.go

@@ -181,6 +181,14 @@ func (t *defaultModelBuildTask) buildTargetGroupSpec(ctx context.Context,
 	}
 	tgPort := t.buildTargetGroupPort(ctx, targetType, svcPort)
 	name := t.buildTargetGroupName(ctx, k8s.NamespacedName(ing.Ing), svc, port, tgPort, targetType, tgProtocol, tgProtocolVersion)
+
+	if tgPort == 0 {
+		if targetType == elbv2model.TargetTypeIP {
+			return elbv2model.TargetGroupSpec{}, errors.Errorf("TargetGroup port is empty. Are you using the correct service type?")
+		}
+		return elbv2model.TargetGroupSpec{}, errors.Errorf("TargetGroup port is empty. When using Instance targets, your service be must of type 'NodePort' or 'LoadBalancer'")
+	}
+
 	return elbv2model.TargetGroupSpec{
 		Name:                  name,
 		TargetType:            targetType,

pkg/service/model_build_target_group.go

@@ -72,6 +72,14 @@ func (t *defaultModelBuildTask) buildTargetGroupSpec(ctx context.Context, tgProt
 	if err != nil {
 		return elbv2model.TargetGroupSpec{}, err
 	}
+
+	if targetPort == 0 {
+		if targetType == elbv2model.TargetTypeIP {
+			return elbv2model.TargetGroupSpec{}, errors.Errorf("TargetGroup port is empty. Are you using the correct service type?")
+		}
+		return elbv2model.TargetGroupSpec{}, errors.Errorf("TargetGroup port is empty. When using Instance targets, your service be must of type 'NodePort' or 'LoadBalancer'")
+	}
+
 	return elbv2model.TargetGroupSpec{
 		Name:                  tgName,
 		TargetType:            targetType,