New validation webhook for route uniqueness

This introduces a duplicate name helper for validating webhooks, to
factor out common code between the app, org/space and new route
webhooks.

Route uniqueness is checked using a combination of host, path, domain
guid and namespace within the cf root namespace. Note we are not
using actual domain name as this could feasbily be changed on the
domain, invalidating the name registry entry for the route.

We have introduced a config property to set the root CF namespace in the
controllers config, and the various config yamls.

We have also moved the duplicate validator and error messages from workloads up
a package level to webhooks, so networking and workloads can share the
code.

There is no way to update route host/path/domain via the API, so we have
an e2e test to trigger the update hook via the add destinations
endpoint. This will exit the hook early as new and old route
host/path/domain remain unchanged.

Issue: #178
Co-authored-by: Danail Branekov <danailster@gmail.com>
Co-authored-by: Giuseppe Capizzi <gcapizzi@pivotal.io>
Co-authored-by: Georgi Sabev <georgethebeatle@gmail.com>

api/apis/app_handler.go

@@ -11,7 +11,7 @@ import (
 
 	"github.com/gorilla/schema"
 
-	"code.cloudfoundry.org/cf-k8s-controllers/controllers/webhooks/workloads"
+	"code.cloudfoundry.org/cf-k8s-controllers/controllers/webhooks"
 
 	"code.cloudfoundry.org/cf-k8s-controllers/api/authorization"
 	"code.cloudfoundry.org/cf-k8s-controllers/api/payloads"
@@ -147,7 +147,7 @@ func (h *AppHandler) appCreateHandler(authInfo authorization.Info, w http.Respon
 
 	appRecord, err := h.appRepo.CreateApp(ctx, authInfo, payload.ToAppCreateMessage())
 	if err != nil {
-		if workloads.HasErrorCode(err, workloads.DuplicateAppError) {
+		if webhooks.HasErrorCode(err, webhooks.DuplicateAppError) {
 			errorDetail := fmt.Sprintf("App with the name '%s' already exists.", payload.Name)
 			h.logger.Error(err, errorDetail, "App Name", payload.Name)
 			writeUniquenessError(w, errorDetail)

api/apis/org_handler.go

@@ -15,7 +15,7 @@ import (
 	"code.cloudfoundry.org/cf-k8s-controllers/api/payloads"
 	"code.cloudfoundry.org/cf-k8s-controllers/api/presenter"
 	"code.cloudfoundry.org/cf-k8s-controllers/api/repositories"
-	"code.cloudfoundry.org/cf-k8s-controllers/controllers/webhooks/workloads"
+	"code.cloudfoundry.org/cf-k8s-controllers/controllers/webhooks"
 )
 
 const (
@@ -62,7 +62,7 @@ func (h *OrgHandler) orgCreateHandler(info authorization.Info, w http.ResponseWr
 
 	record, err := h.orgRepo.CreateOrg(r.Context(), info, org)
 	if err != nil {
-		if workloads.HasErrorCode(err, workloads.DuplicateOrgNameError) {
+		if webhooks.HasErrorCode(err, webhooks.DuplicateOrgNameError) {
 			errorDetail := fmt.Sprintf("Organization '%s' already exists.", org.Name)
 			h.logger.Info(errorDetail)
 			writeUnprocessableEntityError(w, errorDetail)

api/apis/org_handler_test.go

@@ -19,7 +19,7 @@ import (
 	"code.cloudfoundry.org/cf-k8s-controllers/api/apis/fake"
 	"code.cloudfoundry.org/cf-k8s-controllers/api/authorization"
 	"code.cloudfoundry.org/cf-k8s-controllers/api/repositories"
-	"code.cloudfoundry.org/cf-k8s-controllers/controllers/webhooks/workloads"
+	"code.cloudfoundry.org/cf-k8s-controllers/controllers/webhooks"
 )
 
 const (
@@ -111,7 +111,7 @@ var _ = Describe("OrgHandler", func() {
 			BeforeEach(func() {
 				var err error = &k8serrors.StatusError{
 					ErrStatus: metav1.Status{
-						Reason: metav1.StatusReason(fmt.Sprintf(`{"code":%d}`, workloads.DuplicateOrgNameError)),
+						Reason: metav1.StatusReason(fmt.Sprintf(`{"code":%d}`, webhooks.DuplicateOrgNameError)),
 					},
 				}
 				orgRepo.CreateOrgReturns(repositories.OrgRecord{}, err)

api/apis/route_handler.go

@@ -237,7 +237,17 @@ func (h *RouteHandler) routeCreateHandler(authInfo authorization.Info, w http.Re
 			h.logger.Error(err, "no auth to create route")
 			writeNotAuthenticatedErrorResponse(w)
 			return
-		default: // TODO: Catch the error from the (unwritten) validating webhook
+		case repositories.DuplicateError:
+			pathDetails := ""
+			if createRouteMessage.Path != "" {
+				pathDetails = fmt.Sprintf(" and path '%s'", createRouteMessage.Path)
+			}
+			errorDetail := fmt.Sprintf("Route already exists with host '%s'%s for domain '%s'.",
+				createRouteMessage.Host, pathDetails, domain.Name)
+			h.logger.Info(errorDetail)
+			writeUnprocessableEntityError(w, errorDetail)
+			return
+		default:
 			h.logger.Error(err, "Failed to create route", "Route Host", payload.Host)
 			writeUnknownErrorResponse(w)
 			return

api/apis/route_handler_test.go

@@ -958,6 +958,42 @@ var _ = Describe("RouteHandler", func() {
 			})
 		})
 
+		When("CreateRoute returns an already exists error", func() {
+			BeforeEach(func() {
+				routeRepo.CreateRouteReturns(repositories.RouteRecord{}, repositories.NewDuplicateError(repositories.RouteResourceType, nil))
+			})
+
+			It("returns a duplicate resource error", func() {
+				Expect(rr).To(HaveHTTPBody(MatchJSON(`{
+                    "errors": [
+                        {
+                            "title": "CF-UnprocessableEntity",
+                            "detail": "Route already exists with host 'test-route-host' and path '/test-route-path' for domain 'test-domain-name'.",
+                            "code": 10008
+                        }
+                    ]
+                }`)))
+			})
+
+			When("the route path is not set", func() {
+				BeforeEach(func() {
+					requestBody = initializeCreateRouteRequestBody(testRouteHost, "", testSpaceGUID, testDomainGUID, nil, nil)
+				})
+
+				It("returns a duplicate resource error", func() {
+					Expect(rr).To(HaveHTTPBody(MatchJSON(`{
+                    "errors": [
+                        {
+                            "title": "CF-UnprocessableEntity",
+                            "detail": "Route already exists with host 'test-route-host' for domain 'test-domain-name'.",
+                            "code": 10008
+                        }
+                    ]
+                }`)))
+				})
+			})
+		})
+
 		When("CreateRoute returns an unknown error", func() {
 			BeforeEach(func() {
 				routeRepo.CreateRouteReturns(repositories.RouteRecord{},

api/apis/space_handler.go

@@ -17,7 +17,7 @@ import (
 	"code.cloudfoundry.org/cf-k8s-controllers/api/payloads"
 	"code.cloudfoundry.org/cf-k8s-controllers/api/presenter"
 	"code.cloudfoundry.org/cf-k8s-controllers/api/repositories"
-	"code.cloudfoundry.org/cf-k8s-controllers/controllers/webhooks/workloads"
+	"code.cloudfoundry.org/cf-k8s-controllers/controllers/webhooks"
 )
 
 const (
@@ -70,7 +70,7 @@ func (h *SpaceHandler) SpaceCreateHandler(info authorization.Info, w http.Respon
 
 	record, err := h.spaceRepo.CreateSpace(ctx, info, space)
 	if err != nil {
-		if workloads.HasErrorCode(err, workloads.DuplicateSpaceNameError) {
+		if webhooks.HasErrorCode(err, webhooks.DuplicateSpaceNameError) {
 			errorDetail := fmt.Sprintf("Space '%s' already exists.", space.Name)
 			h.logger.Info(errorDetail)
 			writeUnprocessableEntityError(w, errorDetail)

api/apis/space_handler_test.go

@@ -16,7 +16,7 @@ import (
 	"code.cloudfoundry.org/cf-k8s-controllers/api/apis/fake"
 	"code.cloudfoundry.org/cf-k8s-controllers/api/authorization"
 	"code.cloudfoundry.org/cf-k8s-controllers/api/repositories"
-	"code.cloudfoundry.org/cf-k8s-controllers/controllers/webhooks/workloads"
+	"code.cloudfoundry.org/cf-k8s-controllers/controllers/webhooks"
 )
 
 var _ = Describe("Spaces", func() {
@@ -223,7 +223,7 @@ var _ = Describe("Spaces", func() {
 			BeforeEach(func() {
 				var err error = &k8serrors.StatusError{
 					ErrStatus: metav1.Status{
-						Reason: metav1.StatusReason(fmt.Sprintf(`{"code":%d}`, workloads.DuplicateSpaceNameError)),
+						Reason: metav1.StatusReason(fmt.Sprintf(`{"code":%d}`, webhooks.DuplicateSpaceNameError)),
 					},
 				}
 				spaceRepo.CreateSpaceReturns(repositories.SpaceRecord{}, err)

api/repositories/errors.go

@@ -36,16 +36,20 @@ type NotFoundError struct {
 	repoError
 }
 
-type ForbiddenError struct {
-	repoError
-}
-
 func NewNotFoundError(resourceType string, baseError error) NotFoundError {
 	return NotFoundError{
 		repoError: repoError{err: baseError, resourceType: resourceType, message: "not found"},
 	}
 }
 
+func IsNotFoundError(err error) bool {
+	return errors.As(err, &NotFoundError{})
+}
+
+type ForbiddenError struct {
+	repoError
+}
+
 func NewForbiddenError(resourceType string, baseError error) ForbiddenError {
 	return ForbiddenError{
 		repoError: repoError{err: baseError, resourceType: resourceType, message: "forbidden"},
@@ -56,6 +60,16 @@ func IsForbiddenError(err error) bool {
 	return errors.As(err, &ForbiddenError{})
 }
 
-func IsNotFoundError(err error) bool {
-	return errors.As(err, &NotFoundError{})
+type DuplicateError struct {
+	repoError
+}
+
+func NewDuplicateError(resourceType string, baseError error) DuplicateError {
+	return DuplicateError{
+		repoError: repoError{err: baseError, resourceType: resourceType, message: "duplicate"},
+	}
+}
+
+func IsDuplicateError(err error) bool {
+	return errors.As(err, &DuplicateError{})
 }

api/repositories/route_repository.go

@@ -6,6 +6,7 @@ import (
 
 	"code.cloudfoundry.org/cf-k8s-controllers/api/authorization"
 	networkingv1alpha1 "code.cloudfoundry.org/cf-k8s-controllers/controllers/apis/networking/v1alpha1"
+	"code.cloudfoundry.org/cf-k8s-controllers/controllers/webhooks"
 
 	"github.com/google/uuid"
 	v1 "k8s.io/api/core/v1"
@@ -333,6 +334,9 @@ func (f *RouteRepo) CreateRoute(ctx context.Context, authInfo authorization.Info
 	cfRoute := message.toCFRoute()
 	err := f.privilegedClient.Create(ctx, &cfRoute)
 	if err != nil {
+		if webhooks.HasErrorCode(err, webhooks.DuplicateRouteError) {
+			return RouteRecord{}, NewDuplicateError(RouteResourceType, err)
+		}
 		return RouteRecord{}, err
 	}
 

api/tests/e2e/e2e_suite_test.go

@@ -74,14 +74,25 @@ type relationship struct {
 type resourceList struct {
 	Resources []resource `json:"resources"`
 }
+
 type resourceListWithInclusion struct {
 	Resources []resource    `json:"resources"`
 	Included  *includedApps `json:",omitempty"`
 }
+
 type includedApps struct {
 	Apps []resource `json:"apps"`
 }
 
+type bareResource struct {
+	GUID string `json:"guid,omitempty"`
+	Name string `json:"name,omitempty"`
+}
+
+type bareResourceList struct {
+	Resources []bareResource `json:"resources"`
+}
+
 type appResource struct {
 	resource `json:",inline"`
 	State    string `json:"state,omitempty"`
@@ -127,8 +138,17 @@ type manifestRouteResource struct {
 
 type routeResource struct {
 	resource `json:",inline"`
-	Host     string
-	Path     string
+	Host     string `json:"host"`
+	Path     string `json:"path"`
+	URL      string `json:"url,omitempty"`
+}
+
+type destinationsResource struct {
+	Destinations []destination `json:"destinations"`
+}
+
+type destination struct {
+	App bareResource `json:"app"`
 }
 
 type cfErrs struct {
@@ -725,6 +745,12 @@ func createDomain(name string) string {
 	return domain.Name
 }
 
+func deleteDomain(guid string) {
+	Expect(k8sClient.Delete(context.Background(), &networkingv1alpha1.CFDomain{
+		ObjectMeta: metav1.ObjectMeta{Namespace: rootNamespace, Name: guid},
+	})).To(Succeed())
+}
+
 func createRoute(host, path string, spaceGUID, domainGUID string) string {
 	var route resource
 
@@ -742,8 +768,8 @@ func createRoute(host, path string, spaceGUID, domainGUID string) string {
 		SetResult(&route).
 		Post("/v3/routes")
 
-	Expect(err).NotTo(HaveOccurred())
-	Expect(resp).To(HaveRestyStatusCode(http.StatusCreated))
+	ExpectWithOffset(1, err).NotTo(HaveOccurred())
+	ExpectWithOffset(1, resp).To(HaveRestyStatusCode(http.StatusCreated))
 
 	return route.GUID
 }