Ensure shot host with ref gateway domain used for occupied vs check (#…

…1318) * Ensure shot host also includes ref gateway domain when checking for occupied VS host * More readable * Update internal/validation/v2alpha1/hosts.go Co-authored-by: Bartosz Chwila <[email protected]> --------- Co-authored-by: Bartosz Chwila <[email protected]>
kyma-project · Sep 26, 2024 · 49dbeff · 49dbeff
1 parent e4d8e37
commit 49dbeff
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 50 deletions.
diff --git a/internal/validation/v2alpha1/hosts.go b/internal/validation/v2alpha1/hosts.go
@@ -7,6 +7,7 @@ import (
 	gatewayv1beta1 "github.com/kyma-project/api-gateway/apis/gateway/v1beta1"
 	gatewayv2alpha1 "github.com/kyma-project/api-gateway/apis/gateway/v2alpha1"
 	"github.com/kyma-project/api-gateway/internal/helpers"
+	"github.com/kyma-project/api-gateway/internal/processing/default_domain"
 	"github.com/kyma-project/api-gateway/internal/validation"
 	networkingv1beta1 "istio.io/client-go/pkg/apis/networking/v1beta1"
 )
@@ -25,6 +26,7 @@ func validateHosts(parentAttributePath string, vsList networkingv1beta1.VirtualS
 	}
 
 	for hostIndex, host := range hosts {
+		gatewayDomain := ""
 		if helpers.IsShortHostName(string(*host)) {
 			gateway := findGateway(*apiRule.Spec.Gateway, gwList)
 			if gateway == nil {
@@ -40,6 +42,7 @@ func validateHosts(parentAttributePath string, vsList networkingv1beta1.VirtualS
 					Message:       "Lowercase RFC 1123 label is only supported as the APIRule host when selected Gateway has a single host definition matching *.<fqdn> format",
 				})
 			}
+			gatewayDomain = getGatewayDomain(gateway)
 		} else if !helpers.IsFqdnHostName(string(*host)) {
 			hostAttributePath := fmt.Sprintf("%s[%d]", hostsAttributePath, hostIndex)
 			failures = append(failures, validation.Failure{
@@ -48,7 +51,8 @@ func validateHosts(parentAttributePath string, vsList networkingv1beta1.VirtualS
 			})
 		}
 		for _, vs := range vsList.Items {
-			if occupiesHost(vs, string(*host)) && !ownedBy(vs, apiRule) {
+			hostWithDomain := default_domain.GetHostWithDomain(string(*host), gatewayDomain)
+			if occupiesHost(vs, hostWithDomain) && !ownedBy(vs, apiRule) {
 				hostAttributePath := fmt.Sprintf("%s[%d]", hostsAttributePath, hostIndex)
 				failures = append(failures, validation.Failure{
 					AttributePath: hostAttributePath,
@@ -61,6 +65,17 @@ func validateHosts(parentAttributePath string, vsList networkingv1beta1.VirtualS
 	return failures
 }
 
+func getGatewayDomain(gateway *networkingv1beta1.Gateway) string {
+	if gateway != nil {
+		for _, server := range gateway.Spec.Servers {
+			if len(server.Hosts) > 0 {
+				return strings.TrimPrefix(server.Hosts[0], "*.")
+			}
+		}
+	}
+	return ""
+}
+
 func hasSingleHostDefinitionWithCorrectPrefix(gateway *networkingv1beta1.Gateway) bool {
 	host := ""
 	for _, server := range gateway.Spec.Servers {

diff --git a/internal/validation/v2alpha1/hosts_test.go b/internal/validation/v2alpha1/hosts_test.go
@@ -2,6 +2,7 @@ package v2alpha1
 
 import (
 	"github.com/kyma-project/api-gateway/apis/gateway/v2alpha1"
+	"github.com/kyma-project/api-gateway/internal/validation"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"istio.io/api/networking/v1beta1"
@@ -200,94 +201,108 @@ var _ = Describe("Validate hosts", func() {
 		Expect(problems[0].Message).To(Equal("Lowercase RFC 1123 label is only supported as the APIRule host when selected Gateway has a single host definition matching *.<fqdn> format"))
 	})
 
-	It("Should fail if any host that is occupied by any Virtual Service exposed by another resource", func() {
+	validateHostsHelper := func(hosts []*v2alpha1.Host, useVsOwnerLabel bool) []validation.Failure {
 		//given
 		apiRule := &v2alpha1.APIRule{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "some-name",
 				Namespace: "some-ns",
 			},
 			Spec: v2alpha1.APIRuleSpec{
-				Hosts: []*v2alpha1.Host{
-					ptr.To(v2alpha1.Host("host.example.com")),
-					ptr.To(v2alpha1.Host("occupied.example.com")),
-				},
+				Gateway: ptr.To("gateway-ns/gateway-name"),
+				Hosts:   hosts,
 			},
 		}
 		virtualService1 := &networkingv1beta1.VirtualService{
 			Spec: v1beta1.VirtualService{
 				Hosts: []string{
 					"not-occupied1.example.com",
-					"not-occupied2.example.com"},
+					"not-occupied2.example.com",
+				},
 			},
 		}
 		virtualService2 := &networkingv1beta1.VirtualService{
 			Spec: v1beta1.VirtualService{
 				Hosts: []string{
 					"not-occupied3.example.com",
-					"occupied.example.com"},
+					"occupied.example.com",
+				},
 			},
 		}
-
+		if useVsOwnerLabel {
+			virtualService2.ObjectMeta.Labels = getMapWithOwnerLabel(apiRule)
+		}
 		virtualServiceList := networkingv1beta1.VirtualServiceList{
 			Items: []*networkingv1beta1.VirtualService{
 				virtualService1,
 				virtualService2,
 			},
 		}
+		gwList := networkingv1beta1.GatewayList{
+			Items: []*networkingv1beta1.Gateway{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "gateway-name",
+						Namespace: "gateway-ns",
+					},
+					Spec: v1beta1.Gateway{
+						Servers: []*v1beta1.Server{
+							{
+								Hosts: []string{"*.example.com"},
+							},
+						},
+					},
+				},
+			},
+		}
+
+		return validateHosts(".spec", virtualServiceList, gwList, apiRule)
+	}
+
+	It("Should succeed if a host is occupied by a Virtual Service related to the same API Rule", func() {
+		//when
+		problems := validateHostsHelper([]*v2alpha1.Host{
+			ptr.To(v2alpha1.Host("host.example.com")),
+			ptr.To(v2alpha1.Host("occupied.example.com")),
+		}, true)
+
+		//then
+		Expect(problems).To(HaveLen(0))
+	})
+
+	It("Should succeed if a shot host name is occupied by a Virtual Service related to the same API Rule", func() {
+		//when
+		problems := validateHostsHelper([]*v2alpha1.Host{
+			ptr.To(v2alpha1.Host("occupied")),
+		}, true)
+
+		//then
+		Expect(problems).To(HaveLen(0))
+	})
 
+	It("Should fail if any host that is occupied by any Virtual Service exposed by another resource", func() {
 		//when
-		problems := validateHosts(".spec", virtualServiceList, networkingv1beta1.GatewayList{}, apiRule)
+		problems := validateHostsHelper([]*v2alpha1.Host{
+			ptr.To(v2alpha1.Host("host.example.com")),
+			ptr.To(v2alpha1.Host("occupied.example.com")),
+		}, false)
 
 		//then
 		Expect(problems).To(HaveLen(1))
 		Expect(problems[0].AttributePath).To(Equal(".spec.hosts[1]"))
 		Expect(problems[0].Message).To(Equal("Host is occupied by another Virtual Service"))
 	})
 
-	It("Should not fail if a host is occupied by the Virtual Service related to the same API Rule", func() {
-		//given
-		apiRule := &v2alpha1.APIRule{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "some-name",
-				Namespace: "some-ns",
-			},
-			Spec: v2alpha1.APIRuleSpec{
-				Hosts: []*v2alpha1.Host{
-					ptr.To(v2alpha1.Host("host.example.com")),
-					ptr.To(v2alpha1.Host("occupied.example.com")),
-				},
-			},
-		}
-		virtualService1 := &networkingv1beta1.VirtualService{
-			Spec: v1beta1.VirtualService{
-				Hosts: []string{
-					"not-occupied1.example.com",
-					"not-occupied2.example.com"},
-			},
-		}
-		virtualService2 := &networkingv1beta1.VirtualService{
-			Spec: v1beta1.VirtualService{
-				Hosts: []string{
-					"not-occupied3.example.com",
-					"occupied.example.com"},
-			},
-			ObjectMeta: metav1.ObjectMeta{
-				Labels: getMapWithOwnerLabel(apiRule),
-			},
-		}
-		virtualServiceList := networkingv1beta1.VirtualServiceList{
-			Items: []*networkingv1beta1.VirtualService{
-				virtualService1,
-				virtualService2,
-			},
-		}
-
+	It("Should fail if any short host name that is occupied by any Virtual Service exposed by another resource", func() {
 		//when
-		problems := validateHosts(".spec", virtualServiceList, networkingv1beta1.GatewayList{}, apiRule)
+		problems := validateHostsHelper([]*v2alpha1.Host{
+			ptr.To(v2alpha1.Host("occupied")),
+		}, false)
 
 		//then
-		Expect(problems).To(HaveLen(0))
+		Expect(problems).To(HaveLen(1))
+		Expect(problems[0].AttributePath).To(Equal(".spec.hosts[0]"))
+		Expect(problems[0].Message).To(Equal("Host is occupied by another Virtual Service"))
 	})
 })