Merge pull request #40 from PDOK/wr/validation

Wr/validation

Author: Shalucik

api/v2beta1/atom_conversion.go

@@ -25,7 +25,6 @@ SOFTWARE.
 package v2beta1
 
 import (
-	"fmt"
 	"log"
 	"strconv"
 	"time"
@@ -60,9 +59,14 @@ func (a *Atom) ToV3(dst *pdoknlv3.Atom) error {
 		}
 	}
 
+	baseURL, err := createBaseURL(pdoknlv3.GetBaseURL(), a.Spec.General)
+	if err != nil {
+		return err
+	}
+
 	// Service
 	dst.Spec.Service = pdoknlv3.Service{
-		BaseURL:      createBaseURL(pdoknlv3.GetBaseURL(), a.Spec.General),
+		BaseURL:      *baseURL,
 		Lang:         "nl",
 		Title:        a.Spec.Service.Title,
 		Subtitle:     a.Spec.Service.Subtitle,
@@ -91,10 +95,14 @@ func (a *Atom) ToV3(dst *pdoknlv3.Atom) error {
 
 		// Map the links
 		for _, srcLink := range srcDataset.Links {
+			href, err := smoothoperatormodel.ParseURL(srcLink.URI)
+			if err != nil {
+				return err
+			}
 			dstLink := pdoknlv3.Link{
 				Rel:   "describedby",
 				Title: &srcLink.Type,
-				Href:  srcLink.URI,
+				Href:  smoothoperatormodel.URL{URL: href},
 			}
 			if srcLink.ContentType != nil {
 				dstLink.Type = *srcLink.ContentType
@@ -108,11 +116,17 @@ func (a *Atom) ToV3(dst *pdoknlv3.Atom) error {
 
 		// Map the entries
 		for _, srcDownload := range srcDataset.Downloads {
+
+			uri, err := smoothoperatormodel.ParseURL(srcDownload.Srs.URI)
+			if err != nil {
+				return err
+			}
+
 			dstEntry := pdoknlv3.Entry{
 				TechnicalName: srcDownload.Name,
 				Content:       srcDownload.Content,
 				SRS: pdoknlv3.SRS{
-					URI:  srcDownload.Srs.URI,
+					URI:  smoothoperatormodel.URL{URL: uri},
 					Name: srcDownload.Srs.Code,
 				},
 				Polygon: pdoknlv3.Polygon{
@@ -234,7 +248,7 @@ func (a *Atom) ConvertFrom(srcRaw conversion.Hub) error {
 		for _, srcLink := range srcDatasetFeed.Links {
 			dstDataset.Links = append(dstDataset.Links, OtherLink{
 				Type:        smoothutil.PointerVal(srcLink.Title, ""),
-				URI:         srcLink.Href,
+				URI:         srcLink.Href.String(),
 				ContentType: &srcLink.Type,
 				Language:    srcLink.Hreflang,
 			})
@@ -263,7 +277,7 @@ func (a *Atom) ConvertFrom(srcRaw conversion.Hub) error {
 			dstDownload.Updated = &updatedString
 
 			dstDownload.Srs = Srs{
-				URI:  srcEntry.SRS.URI,
+				URI:  srcEntry.SRS.URI.String(),
 				Code: srcEntry.SRS.Name,
 			}
 
@@ -307,19 +321,22 @@ func (a *Atom) ConvertFrom(srcRaw conversion.Hub) error {
 	return nil
 }
 
-func createBaseURL(host string, general General) (baseURL string) {
-	atomURI := fmt.Sprintf("%s/%s", general.DatasetOwner, general.Dataset)
+func createBaseURL(host string, general General) (*smoothoperatormodel.URL, error) {
+	baseURL, err := smoothoperatormodel.ParseURL(host)
+	if err != nil {
+		return nil, err
+	}
+	baseURL = baseURL.JoinPath(general.DatasetOwner, general.Dataset)
 	if general.Theme != nil {
-		atomURI += "/" + *general.Theme
+		baseURL = baseURL.JoinPath(*general.Theme)
 	}
-	atomURI += "/atom"
+	baseURL = baseURL.JoinPath("atom")
 
 	if general.ServiceVersion != nil {
-		atomURI += "/" + *general.ServiceVersion
+		baseURL = baseURL.JoinPath(*general.ServiceVersion)
 	}
 
-	baseURL = fmt.Sprintf("%s/%s/", host, atomURI)
-	return
+	return &smoothoperatormodel.URL{URL: baseURL}, nil
 }
 
 func GetInt32Pointer(value int32) *int32 {

api/v2beta1/atom_conversion_test.go

@@ -14,6 +14,7 @@ func TestAtom_ConvertTo(t *testing.T) {
 	convertFromAtom := getTestAtomV2()
 	convertToAtom := &pdoknlv3.Atom{}
 	dstRaw := conversion.Hub(convertToAtom)
+	pdoknlv3.SetBaseURL("https://test.com/test")
 	err := convertFromAtom.ConvertTo(dstRaw)
 	if err != nil {
 		t.Errorf("ConvertTo() error = %v", err)

api/v3/atom_types.go

@@ -25,7 +25,6 @@ SOFTWARE.
 package v3
 
 import (
-	"net/url"
 	"strings"
 
 	smoothoperatormodel "github.com/pdok/smooth-operator/model"
@@ -47,16 +46,15 @@ type AtomSpec struct {
 // Service defines the service configuration for the Atom feed
 type Service struct {
 	// BaseURL of the Atom service. Will be suffixed with index.xml for the index.
-	// +kubebuilder:validation:Pattern:="https?://.*"
-	BaseURL string `json:"baseUrl"` // TODO use URL type
+	BaseURL smoothoperatormodel.URL `json:"baseUrl"`
 
 	// Language of the service
 	// +kubebuilder:default:="nl"
 	// +kubebuilder:validation:MinLength:=2
-	Lang string `json:"lang"`
+	Lang string `json:"lang,omitempty"`
 
 	// Optional link to a stylesheet used in pages generated by the service.
-	Stylesheet *string `json:"stylesheet,omitempty"`
+	Stylesheet *smoothoperatormodel.URL `json:"stylesheet,omitempty"`
 
 	// Title of the service
 	// +kubebuilder:validation:MinLength:=1
@@ -88,8 +86,7 @@ type Service struct {
 // Link represents a link in the service or dataset feed
 type Link struct {
 	// Actual href of the link
-	// +kubebuilder:validation:Pattern:="https?://.*"
-	Href string `json:"href"`
+	Href smoothoperatormodel.URL `json:"href"`
 
 	// Relation (type) of the link, for example: describedby, self or alternate
 	// +kubebuilder:validation:MinLength:=1
@@ -136,6 +133,7 @@ type DatasetFeed struct {
 	// +kubebuilder:validation:Pattern:=`^[0-9a-zA-Z]{8}\-[0-9a-zA-Z]{4}\-[0-9a-zA-Z]{4}\-[0-9a-zA-Z]{4}\-[0-9a-zA-Z]{12}$`
 	SpatialDatasetIdentifierCode *string `json:"spatialDatasetIdentifierCode,omitempty"`
 
+	// TODO user URL type
 	// SpatialDatasetIdentifierNamespace
 	// +kubebuilder:validation:Format:=uri
 	SpatialDatasetIdentifierNamespace *string `json:"spatialDatasetIdentifierNamespace,omitempty"`
@@ -153,6 +151,7 @@ type MetadataLink struct {
 
 	// Metadata templates to use
 	// +kubebuilder:validation:MinItems:=1
+	// +kubebuilder:validation:items:Enum:=csw;opensearch;html
 	Templates []string `json:"templates"`
 }
 
@@ -189,7 +188,7 @@ type Entry struct {
 // DownloadLink specifies download information for entries
 type DownloadLink struct {
 	// URL to the data
-	// +kubebuilder:validation:MinLength:=1
+	// +kubebuilder:validation:Pattern:=^[^\/]+\/.+\/.+
 	Data string `json:"data"`
 
 	// Optional relation if the link, for example: describedby, self or alternate
@@ -212,8 +211,7 @@ type Polygon struct {
 // SRS describes the Spatial Reference System for an entry
 type SRS struct {
 	// URI of the SRS
-	// +kubebuilder:validation:Pattern:="https?://.*"
-	URI string `json:"uri"`
+	URI smoothoperatormodel.URL `json:"uri"`
 
 	// Name of the SRS
 	// +kubebuilder:validation:MinLength:=1
@@ -265,14 +263,9 @@ func GetBlobEndpoint() string {
 	return blobEndpoint
 }
 
-func (r *Atom) GetBaseURL() url.URL {
-	baseURL, _ := url.Parse(r.Spec.Service.BaseURL)
-	return *baseURL
-}
-
 //nolint:revive
 func (r *Atom) GetBaseUrl() string {
-	return r.Spec.Service.BaseURL
+	return r.Spec.Service.BaseURL.String()
 }
 
 func (r *Atom) GetDownloadLinks() (downloadLinks []DownloadLink) {

api/v3/atom_validation.go

@@ -2,9 +2,13 @@ package v3
 
 import (
 	"fmt"
+	"slices"
 
 	smoothoperatorv1 "github.com/pdok/smooth-operator/api/v1"
 	smoothoperatorvalidation "github.com/pdok/smooth-operator/pkg/validation"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 
 	"strings"
 
@@ -13,79 +17,121 @@ import (
 )
 
 func (atom *Atom) ValidateCreate(c client.Client) ([]string, error) {
-	warnings := []string{}
-	reasons := []string{}
+	var warnings []string
+	var allErrs field.ErrorList
 
 	err := smoothoperatorvalidation.ValidateLabelsOnCreate(atom.Labels)
 	if err != nil {
-		reasons = append(reasons, fmt.Sprintf("%v", err))
+		allErrs = append(allErrs, err)
 	}
 
-	ValidateAtom(c, atom, &warnings, &reasons)
+	ValidateAtom(c, atom, &warnings, &allErrs)
 
-	if len(reasons) > 0 {
-		return warnings, fmt.Errorf("%s", strings.Join(reasons, ". "))
+	if len(allErrs) == 0 {
+		return warnings, nil
 	}
 
-	return warnings, nil
+	return warnings, apierrors.NewInvalid(
+		schema.GroupKind{Group: "pdok.nl", Kind: "Atom"},
+		atom.Name, allErrs)
 }
 
 func (atom *Atom) ValidateUpdate(c client.Client, atomOld *Atom) ([]string, error) {
-	warnings := []string{}
-	reasons := []string{}
+	var warnings []string
+	var allErrs field.ErrorList
+	smoothoperatorvalidation.ValidateLabelsOnUpdate(atomOld.Labels, atom.Labels, &allErrs)
 
-	// Check labels did not change
-	err := smoothoperatorvalidation.ValidateLabelsOnUpdate(atomOld.Labels, atom.Labels)
-	if err != nil {
-		reasons = append(reasons, fmt.Sprintf("%v", err))
-	}
-
-	smoothoperatorvalidation.CheckBaseUrlImmutability(atomOld, atom, &reasons)
+	smoothoperatorvalidation.CheckBaseUrlImmutability(atomOld, atom, &allErrs)
 
-	ValidateAtom(c, atom, &warnings, &reasons)
+	ValidateAtom(c, atom, &warnings, &allErrs)
 
-	if len(reasons) > 0 {
-		return warnings, fmt.Errorf("%s", strings.Join(reasons, ". "))
+	if len(allErrs) == 0 {
+		return warnings, nil
 	}
 
-	return warnings, nil
+	return warnings, apierrors.NewInvalid(
+		schema.GroupKind{Group: "pdok.nl", Kind: "Atom"},
+		atom.Name, allErrs)
 }
 
-func ValidateAtom(c client.Client, atom *Atom, warnings *[]string, reasons *[]string) {
-	ValidateAtomWithoutClusterChecks(atom, warnings, reasons)
+func ValidateAtom(c client.Client, atom *Atom, warnings *[]string, allErrs *field.ErrorList) {
+	ValidateAtomWithoutClusterChecks(atom, warnings, allErrs)
 
+	ownerInfoRef := atom.Spec.Service.OwnerInfoRef
 	ownerInfo := &smoothoperatorv1.OwnerInfo{}
 	objectKey := client.ObjectKey{
 		Namespace: atom.Namespace,
-		Name:      atom.Spec.Service.OwnerInfoRef,
+		Name:      ownerInfoRef,
 	}
 	ctx := context.Background()
 	err := c.Get(ctx, objectKey, ownerInfo)
+	fieldPath := field.NewPath("spec").Child("service").Child("ownerInfoRef")
 	if err != nil {
-		*reasons = append(*reasons, fmt.Sprintf("%v", err))
+		*allErrs = append(*allErrs, field.NotFound(fieldPath, ownerInfoRef))
+		return
 	}
 
 	if ownerInfo.Spec.Atom == nil {
-		*reasons = append(*reasons, "no atom settings in ownerInfo: "+ownerInfo.Name)
+		*allErrs = append(*allErrs, field.Required(fieldPath, "spec.Atom missing in "+ownerInfo.Name))
 	}
 }
 
-func ValidateAtomWithoutClusterChecks(atom *Atom, warnings *[]string, reasons *[]string) {
+func ValidateAtomWithoutClusterChecks(atom *Atom, warnings *[]string, allErrs *field.ErrorList) {
+	var fieldPath *field.Path
 	if strings.Contains(atom.GetName(), "atom") {
-		*warnings = append(*warnings, smoothoperatorvalidation.FormatValidationWarning("name should not contain atom", atom.GroupVersionKind(), atom.GetName()))
+		fieldPath = field.NewPath("metadata").Child("name")
+		smoothoperatorvalidation.AddWarning(warnings, *fieldPath, "should not contain atom", atom.GroupVersionKind(), atom.GetName())
 	}
+	var feedNames []string
+	for i, datasetFeed := range atom.Spec.Service.DatasetFeeds {
+		fieldPath = field.NewPath("spec").Child("service").Child("datasetFeeds").Index(i)
 
-	for _, datasetFeed := range atom.Spec.Service.DatasetFeeds {
-		for _, entry := range datasetFeed.Entries {
-			if linkCount := len(entry.DownloadLinks); linkCount > 1 && entry.Content == nil {
-				*reasons = append(*reasons, "content is required for an Entry with more than 1 DownloadLink")
+		if slices.Contains(feedNames, datasetFeed.TechnicalName) {
+			*allErrs = append(*allErrs, field.Duplicate(fieldPath.Child("technicalName"), datasetFeed.TechnicalName))
+		}
+
+		feedNames = append(feedNames, datasetFeed.TechnicalName)
+
+		if datasetFeed.DatasetMetadataLinks != nil && atom.Spec.Service.ServiceMetadataLinks != nil {
+			if datasetFeed.DatasetMetadataLinks.MetadataIdentifier == atom.Spec.Service.ServiceMetadataLinks.MetadataIdentifier {
+				*allErrs = append(*allErrs, field.Invalid(
+					fieldPath.Child("datasetMetadataLinks").Child("metadataIdentifier"),
+					datasetFeed.DatasetMetadataLinks.MetadataIdentifier,
+					fmt.Sprintf("should not be the same as %s", field.NewPath("spec").
+						Child("service").Child("serviceMetadataLinks").Child("metadataIdentifier")),
+				))
 			}
 		}
-	}
 
-	service := atom.Spec.Service
-	err := smoothoperatorvalidation.ValidateBaseURL(service.BaseURL)
-	if err != nil {
-		*reasons = append(*reasons, fmt.Sprintf("%v", err))
+		if datasetFeed.DatasetMetadataLinks != nil && datasetFeed.SpatialDatasetIdentifierCode == nil {
+			*allErrs = append(*allErrs, field.Required(
+				fieldPath.Child("spatialDatasetIdentifierCode"),
+				fmt.Sprintf("when %s exists", fieldPath.Child("datasetMetadataLinks").String()),
+			))
+		}
+
+		if datasetFeed.SpatialDatasetIdentifierCode != nil && datasetFeed.SpatialDatasetIdentifierNamespace == nil {
+			*allErrs = append(*allErrs, field.Required(
+				fieldPath.Child("spatialDatasetIdentifierNamespace"),
+				fmt.Sprintf("when %s exists", fieldPath.Child("spatialDatasetIdentifierCode").String()),
+			))
+		}
+
+		var entryNames []string
+		for in, entry := range datasetFeed.Entries {
+			fieldPath = fieldPath.Child("entries").Index(in)
+			if linkCount := len(entry.DownloadLinks); linkCount > 1 && entry.Content == nil {
+				*allErrs = append(*allErrs, field.Required(
+					fieldPath.Child("content"),
+					fmt.Sprintf("when %s has 2 or more elements", fieldPath.Child("downloadlinks").String()),
+				))
+			}
+
+			if slices.Contains(entryNames, entry.TechnicalName) {
+				*allErrs = append(*allErrs, field.Duplicate(fieldPath.Child("technicalName"), entry.TechnicalName))
+			}
+
+			entryNames = append(entryNames, entry.TechnicalName)
+		}
 	}
 }