diff --git a/api/v2beta1/wfs_conversion.go b/api/v2beta1/wfs_conversion.go index 5d046e3..9d089d2 100644 --- a/api/v2beta1/wfs_conversion.go +++ b/api/v2beta1/wfs_conversion.go @@ -177,7 +177,7 @@ func convertV2FeatureTypeToV3(src FeatureType) pdoknlv3.FeatureType { MetadataIdentifier: src.DatasetMetadataIdentifier, }, }, - Data: pdoknlv3.Data{}, + Data: pdoknlv3.BaseData{}, } if src.Extent != nil { @@ -186,7 +186,7 @@ func convertV2FeatureTypeToV3(src FeatureType) pdoknlv3.FeatureType { } } - featureTypeV3.Data = ConvertV2DataToV3(src.Data) + featureTypeV3.Data = ConvertV2DataToV3(src.Data).BaseData return featureTypeV3 } @@ -260,7 +260,7 @@ func (dst *WFS) ConvertFrom(srcRaw conversion.Hub) error { Keywords: featureType.Keywords, DatasetMetadataIdentifier: featureType.DatasetMetadataURL.CSW.MetadataIdentifier, SourceMetadataIdentifier: "", - Data: ConvertV3DataToV2(featureType.Data), + Data: ConvertV3DataToV2(pdoknlv3.Data{BaseData: featureType.Data}), } if src.Spec.Service.Inspire != nil { diff --git a/api/v3/shared_types.go b/api/v3/shared_types.go index e17ee51..30fb550 100644 --- a/api/v3/shared_types.go +++ b/api/v3/shared_types.go @@ -182,14 +182,19 @@ type Custom struct { Type string `json:"type"` } -// Data holds the data source configuration -// +kubebuilder:validation:XValidation:rule="has(self.gpkg) || has(self.tif) || has(self.postgis)", message="Atleast one of the datasource should be provided (postgis, gpkg, tif)" -type Data struct { +// BaseData holds the data source configuration for gpkg and postgis +type BaseData struct { // Gpkg configures a GeoPackage file source Gpkg *Gpkg `json:"gpkg,omitempty"` // Postgis configures a Postgis table source Postgis *Postgis `json:"postgis,omitempty"` +} + +// Data holds the data source configuration +// +kubebuilder:validation:XValidation:rule="has(self.gpkg) || has(self.tif) || has(self.postgis)", message="Atleast one of the datasource should be provided (postgis, gpkg, tif)" +type Data struct { + BaseData `json:",inline"` // TIF configures a GeoTIF raster source TIF *TIF `json:"tif,omitempty"` @@ -276,7 +281,7 @@ func GetHost(includeProtocol bool) string { return host } -func (d *Data) GetColumns() *[]Column { +func (d *BaseData) GetColumns() *[]Column { switch { case d.Gpkg != nil: return &d.Gpkg.Columns @@ -287,7 +292,7 @@ func (d *Data) GetColumns() *[]Column { } } -func (d *Data) GetTableName() *string { +func (d *BaseData) GetTableName() *string { switch { case d.Gpkg != nil: return &d.Gpkg.TableName @@ -298,7 +303,7 @@ func (d *Data) GetTableName() *string { } } -func (d *Data) GetGeometryType() *string { +func (d *BaseData) GetGeometryType() *string { switch { case d.Gpkg != nil: return &d.Gpkg.GeometryType diff --git a/api/v3/shared_validation.go b/api/v3/shared_validation.go index 5ea0dbe..b1c3020 100644 --- a/api/v3/shared_validation.go +++ b/api/v3/shared_validation.go @@ -46,7 +46,7 @@ func ValidateUpdate[W WMSWFS](c client.Client, newW, oldW W, validate func(W, *[ allErrs := field.ErrorList{} // Make sure no ingressRouteURLs have been removed - sharedValidation.ValidateIngressRouteURLsNotRemoved(oldW.IngressRouteURLs(true), newW.IngressRouteURLs(true), &allErrs, nil) + sharedValidation.ValidateIngressRouteURLsNotRemoved(oldW.IngressRouteURLs(false), newW.IngressRouteURLs(true), &allErrs, nil) if len(newW.IngressRouteURLs(false)) == 0 { // There are no ingressRouteURLs given, spec.service.url is immutable is that case. diff --git a/api/v3/wfs_types.go b/api/v3/wfs_types.go index 7405947..e63127b 100644 --- a/api/v3/wfs_types.go +++ b/api/v3/wfs_types.go @@ -187,7 +187,8 @@ type FeatureType struct { // FeatureType data connection // +kubebuilder:validation:Type=object - Data Data `json:"data"` + // +kubebuilder:validation:XValidation:rule="has(self.gpkg) || has(self.postgis)", message="At least one of the datasource should be provided (postgis, gpkg)" + Data BaseData `json:"data"` } // FeatureBbox is the optional featureType bounding box, if provided it overrides the default extent diff --git a/api/v3/wfs_validation.go b/api/v3/wfs_validation.go index e5ef329..e9f99f2 100644 --- a/api/v3/wfs_validation.go +++ b/api/v3/wfs_validation.go @@ -34,11 +34,20 @@ func ValidateWFS(wfs *WFS, warnings *[]string, allErrs *field.ErrorList) { path := field.NewPath("spec").Child("service") if service.Mapfile == nil && service.DefaultCrs != "EPSG:28992" && service.Bbox == nil { - *allErrs = append(*allErrs, field.Required(path.Child("bbox").Child("defaultCRS"), "when service.defaultCRS is not 'EPSG:28992'")) + *allErrs = append(*allErrs, field.Required( + path.Child("bbox").Child("defaultCRS"), + "when service.defaultCRS is not 'EPSG:28992'", + )) } if service.Mapfile != nil && service.Bbox != nil { - sharedValidation.AddWarning(warnings, *path.Child("bbox"), "is not used when service.mapfile is configured", wfs.GroupVersionKind(), wfs.GetName()) + sharedValidation.AddWarning( + warnings, + *path.Child("bbox"), + "is not used when service.mapfile is configured", + wfs.GroupVersionKind(), + wfs.GetName(), + ) } ValidateInspire(wfs, allErrs) @@ -76,37 +85,5 @@ func ValidateFeatureTypes(wfs *WFS, warnings *[]string, allErrs *field.ErrorList ) } - if tif := featureType.Data.TIF; tif != nil { - if tif.Resample != "NEAREST" { - sharedValidation.AddWarning( - warnings, - *path.Index(index).Child("data").Child("tif").Child("resample"), - "is not used when service.mapfile is configured", - wfs.GroupVersionKind(), - wfs.GetName(), - ) - } - - if tif.Offsite != nil { - sharedValidation.AddWarning( - warnings, - *path.Index(index).Child("data").Child("tif").Child("offsite"), - "is not used when service.mapfile is configured", - wfs.GroupVersionKind(), - wfs.GetName(), - ) - } - - if tif.GetFeatureInfoIncludesClass { - sharedValidation.AddWarning( - warnings, - *path.Index(index).Child("data").Child("tif").Child("getFeatureInfoIncludesClass"), - "is not used when service.mapfile is configured", - wfs.GroupVersionKind(), - wfs.GetName(), - ) - } - } - } } diff --git a/api/v3/wms_types.go b/api/v3/wms_types.go index e1ae48f..0d3d8f1 100644 --- a/api/v3/wms_types.go +++ b/api/v3/wms_types.go @@ -108,6 +108,8 @@ type WMSSpec struct { Service WMSService `json:"service"` } +// +kubebuilder:validation:XValidation:message="service requires styling, either through service.mapfile, or stylingAssets.configMapRefs",rule=has(self.mapfile) || (has(self.stylingAssets) && has(self.stylingAssets.configMapRefs)) +// +kubebuilder:validation:XValidation:message="when using service.mapfile, don't include stylingAssets.configMapRefs",rule=!has(self.mapfile) || (!has(self.stylingAssets) || !has(self.stylingAssets.configMapRefs)) type WMSService struct { BaseService `json:",inline"` @@ -187,9 +189,9 @@ type ConfigMapRef struct { // +kubebuilder:validation:XValidation:message="A layer should have exactly one of sublayers or data", rule="(has(self.data) || has(self.layers)) && !(has(self.data) && has(self.layers))" // +kubebuilder:validation:XValidation:message="A layer with data attribute should have styling", rule="!has(self.data) || has(self.styles)" -// +kubebuilder:validation:XValidation:message="A layer should have keywords when visible", rule="!self.visible || has(self.keywords)" // +kubebuilder:validation:XValidation:message="A layer should have a title when visible", rule="!self.visible || has(self.title)" // +kubebuilder:validation:XValidation:message="A layer should have an abstract when visible", rule="!self.visible || has(self.abstract)" +// +kubebuilder:validation:XValidation:message="A layer should have keywords when visible", rule="!self.visible || has(self.keywords)" type Layer struct { // Name of the layer, required for layers on the 2nd or 3rd level // +kubebuilder:validation:MinLength:=1 @@ -326,7 +328,7 @@ type WMSOptions struct { func (wmsService *WMSService) GetBoundingBox() WMSBoundingBox { var boundingBox *WMSBoundingBox - allLayers := wmsService.GetAllLayers() + allLayers := wmsService.GetAnnotatedLayers() for _, layer := range allLayers { if len(layer.BoundingBoxes) > 0 { for _, bbox := range wmsService.Layer.BoundingBoxes { @@ -354,6 +356,16 @@ func (wmsService *WMSService) GetBoundingBox() WMSBoundingBox { } } +func (stylingAssets *StylingAssets) GetAllConfigMapRefKeys() []string { + keys := []string{} + if stylingAssets != nil { + for _, cmRef := range stylingAssets.ConfigMapRefs { + keys = append(keys, cmRef.Keys...) + } + } + return keys +} + type AnnotatedLayer struct { // The name of the group that this layer belongs to, nil if it is not a member of a group. Groups can be a member of the toplayer as a group GroupName *string @@ -363,42 +375,36 @@ type AnnotatedLayer struct { IsGroupLayer bool // Contains actual data IsDataLayer bool - Layer Layer + Layer } func (wmsService *WMSService) GetAnnotatedLayers() []AnnotatedLayer { result := make([]AnnotatedLayer, 0) - if wmsService.Layer.Name != nil && len(*wmsService.Layer.Name) > 0 { - firstLayer := AnnotatedLayer{ - GroupName: nil, - IsTopLayer: wmsService.Layer.IsTopLayer(), - IsGroupLayer: wmsService.Layer.IsGroupLayer(), - IsDataLayer: wmsService.Layer.IsDataLayer(), - Layer: wmsService.Layer, - } - result = append(result, firstLayer) - } + result = append(result, AnnotatedLayer{ + GroupName: nil, + IsTopLayer: true, + IsGroupLayer: true, + IsDataLayer: false, + Layer: wmsService.Layer, + }) - for _, subLayer := range wmsService.Layer.Layers { - groupName := wmsService.Layer.Name - isGroupLayer := subLayer.IsGroupLayer() - isDataLayer := !isGroupLayer + for _, middleLayer := range wmsService.Layer.Layers { result = append(result, AnnotatedLayer{ - GroupName: groupName, + GroupName: wmsService.Layer.Name, IsTopLayer: false, - IsGroupLayer: isGroupLayer, - IsDataLayer: isDataLayer, - Layer: subLayer, + IsGroupLayer: middleLayer.IsGroupLayer(), + IsDataLayer: middleLayer.IsDataLayer(), + Layer: middleLayer, }) - for _, subSubLayer := range subLayer.Layers { + for _, bottomLayer := range middleLayer.Layers { result = append(result, AnnotatedLayer{ - GroupName: subLayer.Name, + GroupName: middleLayer.Name, IsTopLayer: false, IsGroupLayer: false, IsDataLayer: true, - Layer: subSubLayer, + Layer: bottomLayer, }) } } @@ -406,19 +412,6 @@ func (wmsService *WMSService) GetAnnotatedLayers() []AnnotatedLayer { return result } -func (wmsService *WMSService) GetAllLayers() (layers []Layer) { - return wmsService.Layer.FlattenLayers() -} - -// FlattenLayers - flattens the layer and its sublayers into one array -func (layer *Layer) FlattenLayers() []Layer { - layers := []Layer{*layer} - for _, childLayer := range layer.Layers { - layers = append(layers, childLayer.FlattenLayers()...) - } - return layers -} - // GetAllSublayers - get all sublayers of a layer, the result does not include the layer itself func (layer *Layer) GetAllSublayers() []Layer { layers := layer.Layers @@ -469,17 +462,6 @@ func (layer *Layer) hasTIFData() bool { return layer.Data.TIF != nil && layer.Data.TIF.BlobKey != "" } -func (layer *Layer) GetLayerType(service *WMSService) (layerType string) { - switch { - case layer.IsDataLayer(): - return DataLayer - case layer.Name == service.Layer.Name: - return TopLayer - default: - return GroupLayer - } -} - func (layer *Layer) IsDataLayer() bool { return layer.hasData() && len(layer.Layers) == 0 } @@ -530,8 +512,8 @@ func (layer *Layer) setInheritedBoundingBoxes() { layer.Layers = updatedLayers } -func (wms *WMS) GetAllLayersWithLegend() (layers []Layer) { - for _, layer := range wms.Spec.Service.GetAllLayers() { +func (wms *WMS) GetAllLayersWithLegend() (layers []AnnotatedLayer) { + for _, layer := range wms.Spec.Service.GetAnnotatedLayers() { if !layer.hasData() || len(layer.Styles) == 0 { continue } @@ -547,7 +529,7 @@ func (wms *WMS) GetAllLayersWithLegend() (layers []Layer) { func (wms *WMS) GetUniqueTiffBlobKeys() []string { blobKeys := map[string]bool{} - for _, layer := range wms.Spec.Service.GetAllLayers() { + for _, layer := range wms.Spec.Service.GetAnnotatedLayers() { if layer.hasTIFData() { blobKeys[layer.Data.TIF.BlobKey] = true } @@ -578,7 +560,7 @@ func (wms *WMS) GetAuthority() *Authority { } func (wms *WMS) HasPostgisData() bool { - for _, layer := range wms.Spec.Service.GetAllLayers() { + for _, layer := range wms.Spec.Service.GetAnnotatedLayers() { if layer.Data != nil && layer.Data.Postgis != nil { return true } @@ -639,7 +621,7 @@ func (wms *WMS) URL() smoothoperatormodel.URL { func (wms *WMS) DatasetMetadataIDs() []string { ids := []string{} - for _, layer := range wms.Spec.Service.GetAllLayers() { + for _, layer := range wms.Spec.Service.GetAnnotatedLayers() { if layer.DatasetMetadataURL != nil && layer.DatasetMetadataURL.CSW != nil { if id := layer.DatasetMetadataURL.CSW.MetadataIdentifier; !slices.Contains(ids, id) { ids = append(ids, id) @@ -684,8 +666,8 @@ func (wms *WMS) ReadinessQueryString() (string, string, error) { } firstDataLayerName := "" - for _, layer := range wms.Spec.Service.GetAllLayers() { - if layer.IsDataLayer() { + for _, layer := range wms.Spec.Service.GetAnnotatedLayers() { + if layer.IsDataLayer { firstDataLayerName = *layer.Name break } diff --git a/api/v3/wms_validation.go b/api/v3/wms_validation.go index bebd4b6..400fb10 100644 --- a/api/v3/wms_validation.go +++ b/api/v3/wms_validation.go @@ -2,7 +2,6 @@ package v3 import ( "fmt" - "maps" "strings" "sigs.k8s.io/controller-runtime/pkg/client" @@ -20,9 +19,6 @@ func (wms *WMS) ValidateUpdate(c client.Client, wmsOld *WMS) ([]string, error) { return ValidateUpdate(c, wms, wmsOld, ValidateWMS) } -// TODO fix linting (cyclop,funlen) -// -//nolint:cyclop,funlen func ValidateWMS(wms *WMS, warnings *[]string, allErrs *field.ErrorList) { if strings.Contains(wms.GetName(), "wms") { sharedValidation.AddWarning( @@ -34,10 +30,9 @@ func ValidateWMS(wms *WMS, warnings *[]string, allErrs *field.ErrorList) { ) } - service := wms.Spec.Service - path := field.NewPath("spec").Child("service") - - if service.Mapfile != nil { + if wms.Mapfile() != nil { + service := wms.Spec.Service + path := field.NewPath("spec").Child("service") if service.Resolution != nil { sharedValidation.AddWarning( warnings, @@ -58,257 +53,270 @@ func ValidateWMS(wms *WMS, warnings *[]string, allErrs *field.ErrorList) { } } - var rewriteGroupToDataLayers = wms.Options().RewriteGroupToDataLayers - var validateChildStyleNameEqual = wms.Options().ValidateChildStyleNameEqual - - equalChildStyleNames := map[string][]string{} - if rewriteGroupToDataLayers && validateChildStyleNameEqual { - findEqualChildStyleNames(&wms.Spec.Service.Layer, &equalChildStyleNames) + ValidateInspire(wms, allErrs) + if wms.HorizontalPodAutoscalerPatch() != nil { + ValidateHorizontalPodAutoscalerPatch(*wms.HorizontalPodAutoscalerPatch(), allErrs) } + ValidateEphemeralStorage(wms.PodSpecPatch(), allErrs) - var names []string + validateLayers(wms, warnings, allErrs) +} + +func validateLayers(wms *WMS, warnings *[]string, allErrs *field.ErrorList) { + + layerNames := []string{} hasVisibleLayer := false - wms.Spec.Service.Layer.setInheritedBoundingBoxes() - for _, layer := range wms.Spec.Service.GetAllLayers() { - path = path.Child("layers") - var layerErrs field.ErrorList - - layerType := layer.GetLayerType(&service) - var layerName string - if layer.Name == nil { - if layerType != TopLayer { - layerErrs = append(layerErrs, field.Required( - path.Child("[*]").Child("name"), - "(except for the topLayer)", - )) - } - layerName = "unnamed:" + layerType - } else { - layerName = *layer.Name - } - path = path.Child(fmt.Sprintf("[%s]", layerName)) + topLayer := AnnotatedLayer{ + GroupName: nil, + IsTopLayer: true, + IsGroupLayer: true, + IsDataLayer: false, + Layer: wms.Spec.Service.Layer, + } - if slices.Contains(names, layerName) { - layerErrs = append(layerErrs, field.Duplicate( - path, - layerName, - )) - } - names = append(names, layerName) + validateLayer(topLayer, field.NewPath("spec").Child("service").Child("layer"), []string{}, &layerNames, &hasVisibleLayer, wms, warnings, allErrs) - if service.Mapfile != nil && layer.BoundingBoxes != nil { - sharedValidation.AddWarning( - warnings, - *path.Child("boundingBoxes"), - "is not used when service.mapfile is configured", - wms.GroupVersionKind(), - wms.GetName(), - ) - } - if service.Mapfile == nil && service.DataEPSG != "EPSG:28992" && !layer.hasBoundingBoxForCRS(service.DataEPSG) { - layerErrs = append(layerErrs, field.Required( - path.Child("boundingBoxes").Child("crs"), - fmt.Sprintf("must contain a boundingBox for CRS %s when service.dataEPSG is not 'EPSG:28992'", service.DataEPSG), - )) - } + if !hasVisibleLayer { + *allErrs = append(*allErrs, field.Required( + field.NewPath("spec").Child("service").Child("layer").Child("layers[*]").Child("visible"), + "at least one layer must be visible", + )) + } +} - //nolint:nestif - if !layer.Visible { - if layer.Title != nil { - sharedValidation.AddWarning( - warnings, - *path.Child("title"), - "is not used when layer.visible=false", - wms.GroupVersionKind(), - wms.GetName(), - ) - } - if layer.Abstract != nil { - sharedValidation.AddWarning( - warnings, - *path.Child("abstrct"), - "is not used when layer.visible=false", - wms.GroupVersionKind(), - wms.GetName(), - ) - } - if layer.Keywords != nil { - sharedValidation.AddWarning( - warnings, - *path.Child("keywords"), - "is not used when layer.visible=false", - wms.GroupVersionKind(), - wms.GetName(), - ) - } - if layer.DatasetMetadataURL != nil { - sharedValidation.AddWarning( - warnings, - *path.Child("datasetMetadataURL"), - "is not used when layer.visible=false", - wms.GroupVersionKind(), - wms.GetName(), - ) - } - if layer.Authority != nil && layer.Authority.SpatialDatasetIdentifier != "" { - sharedValidation.AddWarning( - warnings, - *path.Child("authority").Child("spatialDatasetIdentifier"), - "is not used when layer.visible=false", - wms.GroupVersionKind(), - wms.GetName(), - ) - } +func validateLayer(layer AnnotatedLayer, path *field.Path, groupStyles []string, layerNames *[]string, hasVisibleLayer *bool, wms *WMS, warnings *[]string, allErrs *field.ErrorList) { + service := wms.Spec.Service - for i, style := range layer.Styles { - if style.Title != nil { - sharedValidation.AddWarning( - warnings, - *path.Child("styles").Index(i).Child("title"), - "is not used when layer.visible=false", - wms.GroupVersionKind(), - wms.GetName(), - ) - } - if style.Abstract != nil { - sharedValidation.AddWarning( - warnings, - *path.Child("styles").Index(i).Child("abstract"), - "is not used when layer.visible=false", - wms.GroupVersionKind(), - wms.GetName(), - ) - } - } - } + var layerName string + if layer.IsTopLayer && layer.Name == nil { + layerName = "unnamed: " + TopLayer + } else { + layerName = *layer.Name + } - // TODO fix linting (nestif) - //nolint:nestif - if layer.Visible { - hasVisibleLayer = true + if slices.Contains(*layerNames, layerName) { + *allErrs = append(*allErrs, field.Duplicate( + path.Child("name"), + layerName, + )) + } else { + *layerNames = append(*layerNames, layerName) + } - if layer.Title == nil { - layerErrs = append(layerErrs, field.Required(path.Child("title"), "required if layer.visible=true")) - } - if layer.Abstract == nil { - layerErrs = append(layerErrs, field.Required(path.Child("abstract"), "required if layer.visible=true")) - } - if layer.Keywords == nil { - layerErrs = append(layerErrs, field.Required(path.Child("keywords"), "required if layer.visible=true")) - } - for i, style := range layer.Styles { - if style.Title == nil { - layerErrs = append(layerErrs, field.Required( - path.Child("styles").Index(i), - "required if layer.visible=true", - )) - } - } - if !rewriteGroupToDataLayers && validateChildStyleNameEqual { - equalStylesNames, ok := equalChildStyleNames[layerName] - if ok { - for _, styleName := range equalStylesNames { - layerErrs = append(layerErrs, field.Invalid( - path.Child("styles").Child(styleName), - styleName, - "style.name from parent layer must not be set on a a child layer style", - )) - } - } - } - } + if layer.IsGroupLayer && layer.Data != nil { + *allErrs = append(*allErrs, field.Invalid( + path.Child("data"), + layer.Data, + "must not be set on a GroupLayer", + )) + } - if layer.IsDataLayer() { - for i, style := range layer.Styles { - if wms.Spec.Service.Mapfile == nil && style.Visualization == nil { - layerErrs = append(layerErrs, field.Required( - path.Child("styles").Index(i).Child("visualization"), - "must be set on a dataLayer", - )) - } - if wms.Spec.Service.Mapfile != nil && style.Visualization != nil { - layerErrs = append(layerErrs, field.Invalid( - path.Child("styles").Index(i).Child("visualization"), - style.Visualization, - "must not be set on a layer with a static mapfile", - )) - } - } + validateLayerWithMapfile(layer, path, wms, warnings, allErrs) + + if layer.Visible { + if !layer.IsTopLayer { + *hasVisibleLayer = true } + } else { + validateNotVisibleLayer(layer, path, wms, warnings, allErrs) + } - if layerType == GroupLayer || layerType == TopLayer { - if !layer.Visible { - layerErrs = append(layerErrs, field.Invalid( - path.Child("visible"), - layer.Visible, - "must be true for a "+layerType, - )) - } - if layer.Data != nil { - layerErrs = append(layerErrs, field.Invalid( - path.Child("data"), - "", - "must not be set on a grouplayer", + styleNames := []string{} + for i, style := range layer.Styles { + stylePath := path.Child("styles").Index(i) + validateStyle(style, stylePath, &styleNames, &groupStyles, service.StylingAssets.GetAllConfigMapRefKeys(), layer, service.Mapfile != nil, allErrs) + } + + if layer.IsDataLayer { + for _, groupStyle := range groupStyles { + if !slices.Contains(styleNames, groupStyle) { + *allErrs = append(*allErrs, field.Invalid( + path.Child("styles"), + nil, + fmt.Sprintf("dataLayer must implement style: %s, defined by a parent layer", groupStyle), )) } - for i, style := range layer.Styles { - if style.Visualization != nil { - layerErrs = append(layerErrs, field.Invalid( - path.Child("styles").Index(i), - style.Visualization, - "must not be set on a groupLayer", - )) - } - } } - if len(layerErrs) != 0 { - *allErrs = append(*allErrs, layerErrs...) + } + + for i, subLayer := range layer.Layers { + annotatedSubLayer := AnnotatedLayer{ + GroupName: layer.Name, + IsTopLayer: false, + IsGroupLayer: subLayer.IsGroupLayer(), + IsDataLayer: subLayer.IsDataLayer(), + Layer: subLayer, } + validateLayer(annotatedSubLayer, path.Child("layers").Index(i), groupStyles, layerNames, hasVisibleLayer, wms, warnings, allErrs) } - if !hasVisibleLayer { - *allErrs = append(*allErrs, field.Invalid( - path.Child("layers"), - "", - "at least one layer must be visible", +} + +func validateLayerWithMapfile(layer AnnotatedLayer, path *field.Path, wms *WMS, warnings *[]string, allErrs *field.ErrorList) { + service := wms.Spec.Service + hasCustomMapfile := service.Mapfile != nil + if hasCustomMapfile && layer.BoundingBoxes != nil { + sharedValidation.AddWarning( + warnings, + *path.Child("boundingBoxes"), + "is not used when service.mapfile is configured", + wms.GroupVersionKind(), + wms.GetName(), + ) + } + if !hasCustomMapfile && service.DataEPSG != "EPSG:28992" && !layer.hasBoundingBoxForCRS(service.DataEPSG) && layer.Name != nil { + *allErrs = append(*allErrs, field.Required( + path.Child("boundingBoxes").Child("crs"), + fmt.Sprintf("must contain a boundingBox for CRS %s when service.dataEPSG is not 'EPSG:28992'", service.DataEPSG), )) } - ValidateInspire(wms, allErrs) + if layer.IsDataLayer && hasCustomMapfile { + if tif := layer.Data.TIF; tif != nil { + tifWarnings(tif, path, wms, warnings) + } + } + +} - if wms.Spec.HorizontalPodAutoscalerPatch != nil { - ValidateHorizontalPodAutoscalerPatch(*wms.Spec.HorizontalPodAutoscalerPatch, allErrs) +func tifWarnings(tif *TIF, path *field.Path, wms *WMS, warnings *[]string) { + if tif.Resample != "NEAREST" { + sharedValidation.AddWarning( + warnings, + *path.Child("data").Child("tif").Child("resample"), + "is not used when service.mapfile is configured", + wms.GroupVersionKind(), + wms.GetName(), + ) } - podSpecPatch := wms.Spec.PodSpecPatch - ValidateEphemeralStorage(podSpecPatch, allErrs) + if tif.Offsite != nil { + sharedValidation.AddWarning( + warnings, + *path.Child("data").Child("tif").Child("offsite"), + "is not used when service.mapfile is configured", + wms.GroupVersionKind(), + wms.GetName(), + ) + } + + if tif.GetFeatureInfoIncludesClass { + sharedValidation.AddWarning( + warnings, + *path.Child("data").Child("tif").Child("getFeatureInfoIncludesClass"), + "is not used when service.mapfile is configured", + wms.GroupVersionKind(), + wms.GetName(), + ) + } } -func findEqualChildStyleNames(layer *Layer, equalStyleNames *map[string][]string) { - if len(layer.Layers) == 0 { - return +func validateStyle(style Style, path *field.Path, styleNames *[]string, groupStyles *[]string, stylingFiles []string, layer AnnotatedLayer, usesCustomMapfile bool, allErrs *field.ErrorList) { + if slices.Contains(*styleNames, style.Name) { + *allErrs = append(*allErrs, field.Invalid( + path.Child("name"), + style.Name, + "A Layer can't use the same style name multiple times", + )) + } else { + *styleNames = append(*styleNames, style.Name) + } + + if layer.Visible && !slices.Contains(*groupStyles, style.Name) && style.Title == nil { + *allErrs = append(*allErrs, field.Required( + path.Child("title"), + "A Style must have a title on the highest visible Layer", + )) } - equalChildStyleNames := map[string][]string{} - for _, childLayer := range layer.Layers { - if childLayer.Name == nil { - // Name check is done elsewhere - // To prevent errors here we just continue - continue + + if layer.IsGroupLayer { + if slices.Contains(*groupStyles, style.Name) { + *allErrs = append(*allErrs, field.Invalid( + path.Child("name"), + style.Name, + "A GroupLayer can't redefine the same style as a parent layer", + )) + } else { + *groupStyles = append(*groupStyles, style.Name) } - var equalStyles []string - for _, style := range layer.Styles { - for _, childStyle := range childLayer.Styles { - if style.Name == childStyle.Name { - equalStyles = append(equalStyles, style.Name) - } - } + if style.Visualization != nil { + *allErrs = append(*allErrs, field.Invalid( + path.Child("visualization"), + style.Visualization, + "GroupLayers must not have a visualization", + )) + } + } + + if layer.IsDataLayer { + switch { + case usesCustomMapfile && style.Visualization != nil: + *allErrs = append(*allErrs, field.Invalid( + path.Child("visualization"), + style.Visualization, + "is not used when spec.service.mapfile is used", + )) + case !usesCustomMapfile && style.Visualization == nil: + *allErrs = append(*allErrs, field.Required( + path.Child("visualization"), + "on DataLayers when spec.service.mapfile is not used", + )) + case !usesCustomMapfile && !slices.Contains(stylingFiles, *style.Visualization): + *allErrs = append(*allErrs, field.Invalid( + path.Child("visualization"), + style.Visualization, + "must be defined be in spec.service.stylingAssets.configMapKeyRefs.Keys", + )) + } + + } +} + +func validateNotVisibleLayer(layer AnnotatedLayer, path *field.Path, wms *WMS, warnings *[]string, allErrs *field.ErrorList) { + if layer.IsGroupLayer { + *allErrs = append(*allErrs, field.Invalid( + path.Child("visible"), + layer.Visible, + "must be true for a "+GroupLayer, + )) + } + paths := []field.Path{} + + if layer.Title != nil { + paths = append(paths, *path.Child("title")) + } + if layer.Abstract != nil { + paths = append(paths, *path.Child("abstract")) + } + if layer.Keywords != nil { + paths = append(paths, *path.Child("keywords")) + } + if layer.DatasetMetadataURL != nil { + paths = append(paths, *path.Child("datasetMetadataURL")) + } + if layer.Authority != nil { + paths = append(paths, *path.Child("authority")) + } + + for i, style := range layer.Styles { + if style.Title != nil { + paths = append(paths, *path.Child("styles").Index(i).Child("title")) } - if len(equalStyles) > 0 { - equalChildStyleNames[*childLayer.Name] = equalStyles + if style.Abstract != nil { + paths = append(paths, *path.Child("styles").Index(i).Child("abstract")) } - findEqualChildStyleNames(&childLayer, equalStyleNames) } - maps.Copy(*equalStyleNames, equalChildStyleNames) + + for _, path := range paths { + sharedValidation.AddWarning( + warnings, + path, + "is not used when layer.visible=false", + wms.GroupVersionKind(), + wms.GetName(), + ) + } + } diff --git a/api/v3/wms_validation_test.go b/api/v3/wms_validation_test.go deleted file mode 100644 index 25593fb..0000000 --- a/api/v3/wms_validation_test.go +++ /dev/null @@ -1,95 +0,0 @@ -package v3 - -import ( - "reflect" - "testing" - - smoothoperatorutils "github.com/pdok/smooth-operator/pkg/util" -) - -func Test_getEqualChildStyleNames(t *testing.T) { - type args struct { - layer *Layer - equalStyleNames map[string][]string - } - tests := []struct { - name string - args args - want map[string][]string - }{ - { - name: "Test equal style names", - args: args{ - layer: &Layer{ - Name: smoothoperatorutils.Pointer("toplayer"), - Styles: []Style{ - {Name: "stylename-1"}, - {Name: "stylename-2"}, - }, - Layers: []Layer{ - { - Name: smoothoperatorutils.Pointer("childlayer-1"), - Styles: []Style{ - {Name: "stylename-2"}, - {Name: "stylename-3"}, - }, - Layers: []Layer{ - { - Name: smoothoperatorutils.Pointer("childlayer-2"), - Styles: []Style{ - {Name: "stylename-3"}, - {Name: "stylename-4"}, - }, - }, - }, - }, - }, - }, - equalStyleNames: map[string][]string{}, - }, - want: map[string][]string{ - "childlayer-1": {"stylename-2"}, - "childlayer-2": {"stylename-3"}, - }, - }, - { - name: "Test no equal style names", - args: args{ - layer: &Layer{ - Name: smoothoperatorutils.Pointer("toplayer"), - Styles: []Style{ - {Name: "stylename-1"}, - {Name: "stylename-2"}, - }, - Layers: []Layer{ - { - Name: smoothoperatorutils.Pointer("childlayer-1"), - Styles: []Style{ - {Name: "stylename-3"}, - {Name: "stylename-4"}, - }, - Layers: []Layer{ - { - Name: smoothoperatorutils.Pointer("childlayer-2"), - Styles: []Style{ - {Name: "stylename-5"}, - {Name: "stylename-6"}, - }, - }, - }, - }, - }, - }, - equalStyleNames: map[string][]string{}, - }, - want: map[string][]string{}, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if findEqualChildStyleNames(tt.args.layer, &tt.args.equalStyleNames); !reflect.DeepEqual(tt.args.equalStyleNames, tt.want) { - t.Errorf("findEqualChildStyleNames() = %v, want %v", tt.args.equalStyleNames, tt.want) - } - }) - } -} diff --git a/api/v3/zz_generated.deepcopy.go b/api/v3/zz_generated.deepcopy.go index 8dfe8eb..137e8a6 100644 --- a/api/v3/zz_generated.deepcopy.go +++ b/api/v3/zz_generated.deepcopy.go @@ -70,6 +70,31 @@ func (in *Authority) DeepCopy() *Authority { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BaseData) DeepCopyInto(out *BaseData) { + *out = *in + if in.Gpkg != nil { + in, out := &in.Gpkg, &out.Gpkg + *out = new(Gpkg) + (*in).DeepCopyInto(*out) + } + if in.Postgis != nil { + in, out := &in.Postgis, &out.Postgis + *out = new(Postgis) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BaseData. +func (in *BaseData) DeepCopy() *BaseData { + if in == nil { + return nil + } + out := new(BaseData) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BaseOptions) DeepCopyInto(out *BaseOptions) { *out = *in @@ -192,16 +217,7 @@ func (in *Custom) DeepCopy() *Custom { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Data) DeepCopyInto(out *Data) { *out = *in - if in.Gpkg != nil { - in, out := &in.Gpkg, &out.Gpkg - *out = new(Gpkg) - (*in).DeepCopyInto(*out) - } - if in.Postgis != nil { - in, out := &in.Postgis, &out.Postgis - *out = new(Postgis) - (*in).DeepCopyInto(*out) - } + in.BaseData.DeepCopyInto(&out.BaseData) if in.TIF != nil { in, out := &in.TIF, &out.TIF *out = new(TIF) diff --git a/config/crd/bases/pdok.nl_wfs.yaml b/config/crd/bases/pdok.nl_wfs.yaml index 6a21384..73ed1a1 100644 --- a/config/crd/bases/pdok.nl_wfs.yaml +++ b/config/crd/bases/pdok.nl_wfs.yaml @@ -1286,33 +1286,10 @@ spec: - geometryType - tableName type: object - tif: - description: TIF configures a GeoTIF raster source - properties: - blobKey: - description: BlobKey to the TIFF file - pattern: ^.+\/.+\/.+\.(tif?f|vrt)$ - type: string - getFeatureInfoIncludesClass: - default: false - description: '"When a band represents nominal or ordinal data the class name (from styling) can be included in the getFeatureInfo"' - type: boolean - offsite: - description: Sets the color index to treat as transparent for raster layers, optional, hex or rgb - pattern: (#[0-9A-F]{6}([0-9A-F]{2})?)|([0-9]{1,3}\s[0-9]{1,3}\s[0-9]{1,3}) - type: string - resample: - default: NEAREST - description: This option can be used to control the resampling kernel used sampling raster images, optional - pattern: (NEAREST|AVERAGE|BILINEAR) - type: string - required: - - blobKey - type: object type: object x-kubernetes-validations: - - message: Atleast one of the datasource should be provided (postgis, gpkg, tif) - rule: has(self.gpkg) || has(self.tif) || has(self.postgis) + - message: At least one of the datasource should be provided (postgis, gpkg) + rule: has(self.gpkg) || has(self.postgis) datasetMetadataUrl: description: Metadata URL properties: diff --git a/config/crd/bases/pdok.nl_wms.yaml b/config/crd/bases/pdok.nl_wms.yaml index 326d403..aae0d1d 100644 --- a/config/crd/bases/pdok.nl_wms.yaml +++ b/config/crd/bases/pdok.nl_wms.yaml @@ -1841,16 +1841,17 @@ spec: type: boolean required: - name + - styles type: object x-kubernetes-validations: - message: A layer with data attribute should have styling rule: '!has(self.data) || has(self.styles)' - - message: A layer should have keywords when visible - rule: '!self.visible || has(self.keywords)' - message: A layer should have a title when visible rule: '!self.visible || has(self.title)' - message: A layer should have an abstract when visible rule: '!self.visible || has(self.abstract)' + - message: A layer should have keywords when visible + rule: '!self.visible || has(self.keywords)' minItems: 1 type: array maxscaledenominator: @@ -1917,18 +1918,19 @@ spec: type: boolean required: - name + - styles type: object x-kubernetes-validations: - message: A layer should have exactly one of sublayers or data rule: (has(self.data) || has(self.layers)) && !(has(self.data) && has(self.layers)) - message: A layer with data attribute should have styling rule: '!has(self.data) || has(self.styles)' - - message: A layer should have keywords when visible - rule: '!self.visible || has(self.keywords)' - message: A layer should have a title when visible rule: '!self.visible || has(self.title)' - message: A layer should have an abstract when visible rule: '!self.visible || has(self.abstract)' + - message: A layer should have keywords when visible + rule: '!self.visible || has(self.keywords)' minItems: 1 type: array maxscaledenominator: @@ -2003,6 +2005,9 @@ spec: - fieldPath: .visible message: TopLayer must be visible rule: self.visible + - fieldPath: .styles + message: If TopLayer has a name, it must have styles + rule: '!has(self.name) || has(self.styles)' mapfile: description: External Mapfile reference properties: @@ -2094,6 +2099,11 @@ spec: - title - url type: object + x-kubernetes-validations: + - message: service requires styling, either through service.mapfile, or stylingAssets.configMapRefs + rule: has(self.mapfile) || (has(self.stylingAssets) && has(self.stylingAssets.configMapRefs)) + - message: when using service.mapfile, don't include stylingAssets.configMapRefs + rule: '!has(self.mapfile) || (!has(self.stylingAssets) || !has(self.stylingAssets.configMapRefs))' required: - podSpecPatch - service diff --git a/config/crd/update_openapi.go b/config/crd/update_openapi.go index 8e2cfea..001397a 100644 --- a/config/crd/update_openapi.go +++ b/config/crd/update_openapi.go @@ -72,6 +72,7 @@ func updateLayersV3(version *v1.CustomResourceDefinitionVersion) { // Level 3 layerSpecLevel3 := layer.DeepCopy() layerSpecLevel3.Required = append(layerSpecLevel3.Required, "name") + layerSpecLevel3.Required = append(layerSpecLevel3.Required, "styles") delete(layerSpecLevel3.Properties, "layers") xvals := v1.ValidationRules{} for _, xval := range layerSpecLevel3.XValidations { @@ -84,6 +85,7 @@ func updateLayersV3(version *v1.CustomResourceDefinitionVersion) { // Level 2 layerSpecLevel2 := layer.DeepCopy() layerSpecLevel2.Required = append(layerSpecLevel2.Required, "name") + layerSpecLevel2.Required = append(layerSpecLevel2.Required, "styles") bottomLayers := layerSpecLevel2.Properties["layers"] bottomLayers.Description = "[OpenAPI spec injected by mapserver-operator/cmd/update_openapi.go]" bottomLayers.Items = &v1.JSONSchemaPropsOrArray{Schema: layerSpecLevel3} @@ -94,6 +96,7 @@ func updateLayersV3(version *v1.CustomResourceDefinitionVersion) { layerSpecLevel1.Required = append(layerSpecLevel1.Required, "title", "abstract", "keywords", "layers") layerSpecLevel1.XValidations = []v1.ValidationRule{ {Rule: "self.visible", Message: "TopLayer must be visible", FieldPath: ".visible"}, + {Rule: "!has(self.name) || has(self.styles)", Message: "If TopLayer has a name, it must have styles", FieldPath: ".styles"}, } delete(layerSpecLevel1.Properties, "data") delete(layerSpecLevel1.Properties, "labelNoClip") diff --git a/config/samples/v3_wms.yaml b/config/samples/v3_wms.yaml index 9580629..bc6585a 100644 --- a/config/samples/v3_wms.yaml +++ b/config/samples/v3_wms.yaml @@ -13,6 +13,11 @@ spec: - "keyword" ownerInfoRef: "owner" dataEPSG: "EPSG:28992" + stylingAssets: + configMapRefs: + - name: configmap + keys: + - file.symbol layer: title: "title" abstract: "abstract" @@ -35,6 +40,7 @@ spec: tableName: "table" styles: - name: "name" + title: "title" - name: "not visible" visible: false data: diff --git a/internal/controller/blobdownload/blob_download_test.go b/internal/controller/blobdownload/blob_download_test.go index b33701f..86341cd 100644 --- a/internal/controller/blobdownload/blob_download_test.go +++ b/internal/controller/blobdownload/blob_download_test.go @@ -172,10 +172,10 @@ func TestGetArgsForWMS(t *testing.T) { }, }, }, - Data: &pdoknlv3.Data{ + Data: &pdoknlv3.Data{BaseData: pdoknlv3.BaseData{ Gpkg: &pdoknlv3.Gpkg{ BlobKey: "geopackages-bucket/key/gpkg-layer-1-data.gpkg", - }, + }}, }, }, { @@ -190,10 +190,10 @@ func TestGetArgsForWMS(t *testing.T) { }, }, }, - Data: &pdoknlv3.Data{ + Data: &pdoknlv3.Data{BaseData: pdoknlv3.BaseData{ Gpkg: &pdoknlv3.Gpkg{ BlobKey: "geopackages-bucket/key/gpkg-layer-2-data.gpkg", - }, + }}, }, }, }, diff --git a/internal/controller/featureinfogenerator/featureinfo_generator_test.go b/internal/controller/featureinfogenerator/featureinfo_generator_test.go index 8a040ac..5fb0045 100644 --- a/internal/controller/featureinfogenerator/featureinfo_generator_test.go +++ b/internal/controller/featureinfogenerator/featureinfo_generator_test.go @@ -86,24 +86,24 @@ func TestGetInput(t *testing.T) { Layers: []pdoknlv3.Layer{ { Name: smoothoperatorutils.Pointer("gpkg-layer-name"), - Data: &pdoknlv3.Data{ + Data: &pdoknlv3.Data{BaseData: pdoknlv3.BaseData{ Gpkg: &pdoknlv3.Gpkg{ Columns: []pdoknlv3.Column{ {Name: "column-1", Alias: smoothoperatorutils.Pointer("ALIAS_column-1")}, {Name: "column-2"}, }, - }, + }}, }, }, { Name: smoothoperatorutils.Pointer("postgis-layer-name"), - Data: &pdoknlv3.Data{ + Data: &pdoknlv3.Data{BaseData: pdoknlv3.BaseData{ Postgis: &pdoknlv3.Postgis{ Columns: []pdoknlv3.Column{ {Name: "column-1"}, {Name: "column-2"}, }, - }, + }}, }, }, { diff --git a/internal/controller/featureinfogenerator/mapper.go b/internal/controller/featureinfogenerator/mapper.go index 1d6b2d1..188ddcd 100644 --- a/internal/controller/featureinfogenerator/mapper.go +++ b/internal/controller/featureinfogenerator/mapper.go @@ -18,18 +18,17 @@ func MapWMSToFeatureinfoGeneratorInput(wms *pdoknlv3.WMS) (*featureinfo.Scheme, Layers: []featureinfo.Layer{}, } - for _, layer := range wms.Spec.Service.GetAllLayers() { - if !layer.IsDataLayer() { + for _, layer := range wms.Spec.Service.GetAnnotatedLayers() { + if !layer.IsDataLayer { continue } l := featureinfo.Layer{ Name: *layer.Name, - Properties: getProperties(&layer), + Properties: getProperties(&layer.Layer), } - parentLayer := wms.Spec.Service.GetParentLayer(layer) - if parentLayer != nil && parentLayer.IsGroupLayer() && parentLayer.Name != wms.Spec.Service.Layer.Name { - l.GroupName = smoothoperatorutils.PointerVal(parentLayer.Name, "") + if layer.GroupName != nil && layer.GroupName != wms.Spec.Service.Layer.Name { + l.GroupName = smoothoperatorutils.PointerVal(layer.GroupName, "") } input.Layers = append(input.Layers, l) diff --git a/internal/controller/mapfilegenerator/mapper.go b/internal/controller/mapfilegenerator/mapper.go index dda65dd..c8872a4 100644 --- a/internal/controller/mapfilegenerator/mapper.go +++ b/internal/controller/mapfilegenerator/mapper.go @@ -90,7 +90,7 @@ func getWFSLayers(service pdoknlv3.WFSService) (layers []WFSLayer) { Columns: getColumns(featureType.Data), TableName: featureType.Data.GetTableName(), GeometryType: featureType.Data.GetGeometryType(), - GeopackagePath: getGeopackagePath(featureType.Data), + GeopackagePath: getGeopackagePath(featureType.Data.Gpkg), }, } if featureType.Data.Postgis != nil { @@ -123,7 +123,7 @@ func getWMSExtent(serviceLayer pdoknlv3.Layer, serviceExtent string) string { return defaultExtent } -func getColumns(data pdoknlv3.Data) []Column { +func getColumns(data pdoknlv3.BaseData) []Column { columns := []Column{{Name: "fuuid"}} if data.GetColumns() != nil { for _, column := range *data.GetColumns() { @@ -135,12 +135,12 @@ func getColumns(data pdoknlv3.Data) []Column { return columns } -func getGeopackagePath(data pdoknlv3.Data) *string { - if data.Gpkg == nil { +func getGeopackagePath(gpkg *pdoknlv3.Gpkg) *string { + if gpkg == nil { return nil } - index := strings.LastIndex(data.Gpkg.BlobKey, "/") + 1 - blobName := data.Gpkg.BlobKey[index:] + index := strings.LastIndex(gpkg.BlobKey, "/") + 1 + blobName := gpkg.BlobKey[index:] return smoothoperatorutils.Pointer(geopackagePath + "/" + blobName) } @@ -212,7 +212,7 @@ func MapWMSToMapfileGeneratorInput(wms *pdoknlv3.WMS, ownerInfo *smoothoperatorv MaxSize: maxSize, } - if wms.Spec.Service.Layer.IsTopLayer() && wms.Spec.Service.Layer.Name != nil { + if wms.Spec.Service.Layer.Name != nil { result.TopLevelName = *wms.Spec.Service.Layer.Name } @@ -257,7 +257,7 @@ func getWMSLayer(serviceLayer pdoknlv3.Layer, serviceExtent string, wms *pdoknlv var columns []Column if serviceLayer.Data != nil { - columns = getColumns(*serviceLayer.Data) + columns = getColumns(serviceLayer.Data.BaseData) } var tableName *string diff --git a/internal/controller/mapserver/deployment.go b/internal/controller/mapserver/deployment.go index 70ae627..a6253a3 100644 --- a/internal/controller/mapserver/deployment.go +++ b/internal/controller/mapserver/deployment.go @@ -175,7 +175,7 @@ func getStartupProbeForWMS(wms *pdoknlv3.WMS) (*corev1.Probe, error) { } var layerNames []string - for _, layer := range wms.Spec.Service.GetAllLayers() { + for _, layer := range wms.Spec.Service.GetAnnotatedLayers() { if layer.Name != nil { layerNames = append(layerNames, *layer.Name) } diff --git a/internal/controller/ogcwebserviceproxy/ogc_webservice_proxy.go b/internal/controller/ogcwebserviceproxy/ogc_webservice_proxy.go index e0101af..408d83d 100644 --- a/internal/controller/ogcwebserviceproxy/ogc_webservice_proxy.go +++ b/internal/controller/ogcwebserviceproxy/ogc_webservice_proxy.go @@ -5,7 +5,6 @@ import ( "github.com/pdok/mapserver-operator/internal/controller/constants" "github.com/pdok/mapserver-operator/internal/controller/types" "github.com/pdok/mapserver-operator/internal/controller/utils" - smoothoperatorutils "github.com/pdok/smooth-operator/pkg/util" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" yaml "sigs.k8s.io/yaml/goyaml.v3" @@ -78,17 +77,13 @@ func MapWMSToOgcWebserviceProxyConfig(wms *pdoknlv3.WMS) (config Config, err err } config.GroupLayers = make(map[string][]string) - for _, layer := range wms.Spec.Service.GetAllLayers() { - if layer.IsGroupLayer() && wms.Spec.Service.GetParentLayer(layer) != nil { - if dataLayers := dataLayersForGroupLayer(layer); len(dataLayers) > 0 { - config.GroupLayers[smoothoperatorutils.PointerVal(layer.Name, "")] = dataLayers - } + for _, layer := range wms.Spec.Service.GetAnnotatedLayers() { + if !layer.IsTopLayer && layer.IsGroupLayer && layer.Name != nil { + config.GroupLayers[*layer.Name] = dataLayersForGroupLayer(layer.Layer) } } if wms.Spec.Service.Layer.Name != nil { - if dataLayers := dataLayersForGroupLayer(wms.Spec.Service.Layer); len(dataLayers) > 0 { - config.GroupLayers[smoothoperatorutils.PointerVal(wms.Spec.Service.Layer.Name, "")] = dataLayers - } + config.GroupLayers[*wms.Spec.Service.Layer.Name] = dataLayersForGroupLayer(wms.Spec.Service.Layer) } return } diff --git a/internal/controller/test_data/wms/complete/expected/configmap-capabilities-generator.yaml b/internal/controller/test_data/wms/complete/expected/configmap-capabilities-generator.yaml index 465c1d5..acd4844 100644 --- a/internal/controller/test_data/wms/complete/expected/configmap-capabilities-generator.yaml +++ b/internal/controller/test_data/wms/complete/expected/configmap-capabilities-generator.yaml @@ -233,7 +233,7 @@ metadata: service-type: wms service-version: v1_0 theme: '2016' - name: complete-wms-capabilities-generator-657b9fg7m8 + name: complete-wms-capabilities-generator-h2tmmd2f44 namespace: default ownerReferences: - apiVersion: pdok.nl/v3 diff --git a/internal/controller/test_data/wms/complete/expected/configmap-legend-generator.yaml b/internal/controller/test_data/wms/complete/expected/configmap-legend-generator.yaml index 306ac8a..ab1ad53 100644 --- a/internal/controller/test_data/wms/complete/expected/configmap-legend-generator.yaml +++ b/internal/controller/test_data/wms/complete/expected/configmap-legend-generator.yaml @@ -75,7 +75,7 @@ metadata: service-type: wms service-version: v1_0 theme: '2016' - name: complete-wms-legend-generator-k47kg87hb4 + name: complete-wms-legend-generator-bmg7f9t24k namespace: default ownerReferences: - apiVersion: pdok.nl/v3 diff --git a/internal/controller/test_data/wms/complete/expected/configmap-mapfile-generator.yaml b/internal/controller/test_data/wms/complete/expected/configmap-mapfile-generator.yaml index e44fdf6..f971c0a 100644 --- a/internal/controller/test_data/wms/complete/expected/configmap-mapfile-generator.yaml +++ b/internal/controller/test_data/wms/complete/expected/configmap-mapfile-generator.yaml @@ -168,7 +168,7 @@ metadata: service-type: wms service-version: v1_0 theme: '2016' - name: complete-wms-mapfile-generator-gd8tbt5k2b + name: complete-wms-mapfile-generator-26fdbg5546 namespace: default ownerReferences: - apiVersion: pdok.nl/v3 diff --git a/internal/controller/test_data/wms/complete/expected/deployment.yaml b/internal/controller/test_data/wms/complete/expected/deployment.yaml index aa4ff80..1fe7d2c 100644 --- a/internal/controller/test_data/wms/complete/expected/deployment.yaml +++ b/internal/controller/test_data/wms/complete/expected/deployment.yaml @@ -378,11 +378,11 @@ spec: name: complete-wms-init-scripts-fft29bbtdd name: init-scripts - configMap: - name: complete-wms-capabilities-generator-657b9fg7m8 + name: complete-wms-capabilities-generator-h2tmmd2f44 defaultMode: 420 name: capabilities-generator-config - configMap: - name: complete-wms-mapfile-generator-gd8tbt5k2b + name: complete-wms-mapfile-generator-26fdbg5546 defaultMode: 420 name: mapfile-generator-config - name: styling-files @@ -392,11 +392,13 @@ spec: name: gpkg-styling - configMap: name: tif-styling + - configMap: + name: postgis-styling - configMap: name: complete-wms-featureinfo-generator-257f6m6228 defaultMode: 420 name: featureinfo-generator-config - configMap: - name: complete-wms-legend-generator-k47kg87hb4 + name: complete-wms-legend-generator-bmg7f9t24k defaultMode: 420 name: legend-generator-config diff --git a/internal/controller/test_data/wms/complete/input/wms.yaml b/internal/controller/test_data/wms/complete/input/wms.yaml index 7342554..6430e4d 100644 --- a/internal/controller/test_data/wms/complete/input/wms.yaml +++ b/internal/controller/test_data/wms/complete/input/wms.yaml @@ -145,6 +145,12 @@ spec: name: group-layer-style-2-name title: gpkg-layer-style-4-title visualization: gpkg-layer-style-4.style + - name: top-layer-style-2-name + visualization: top-layer-style-2.style + - name: group-layer-style-1-name + visualization: group-layer-style-1.style + - name: group-layer-style-3-name + visualization: group-layer-style-3.style title: gpkg-layer-title "2" visible: true - abstract: postgis-layer-abstract @@ -183,6 +189,16 @@ spec: - name: postgis-layer-style-2-name title: postgis-layer-style-2-title visualization: postgis-layer-style-2.style + - name: top-layer-style-1-name + visualization: top-layer-style-1.style + - name: top-layer-style-2-name + visualization: top-layer-style-2.style + - name: group-layer-style-1-name + visualization: group-layer-style-1.style + - name: group-layer-style-2-name + visualization: group-layer-style-1.style + - name: group-layer-style-3-name + visualization: group-layer-style-1.style title: postgis-layer-title visible: true maxscaledenominator: "50" @@ -194,8 +210,8 @@ spec: - name: group-layer-style-2-name title: group-layer-style-2-title - abstract: group-layer-style-3-abstract - name: top-layer-style-1-name - title: group-layer-style-3-name + name: group-layer-style-3-name + title: group-layer-style-3-title title: group layer title "1" visible: true - boundingBoxes: @@ -224,6 +240,10 @@ spec: visualization: tif-layer-style-1.style - name: tif-layer-style-2-name visualization: tif-layer-style-2.style + - name: top-layer-style-1-name + visualization: top-layer-style-1.style + - name: top-layer-style-2-name + visualization: top-layer-style-2.style visible: false maxscaledenominator: "50" name: top-layer-name @@ -246,14 +266,24 @@ spec: - ${BLOBS_RESOURCES_BUCKET}/key/font-2.ttf configMapRefs: - keys: - - gpgk-layer-style-1.style - - gpgk-layer-style-2.style + - gpkg-layer-style-1.style + - gpkg-layer-style-2.style + - gpkg-layer-style-3.style + - gpkg-layer-style-4.style - gpkg-layer-symbol.symbol + - top-layer-style-1.style + - top-layer-style-2.style + - group-layer-style-1.style + - group-layer-style-3.style name: gpkg-styling - keys: - tif-layer-style-1.style - tif-layer-style-2.style - tif-layer-symbol.symbol name: tif-styling + - keys: + - postgis-layer-style-1.style + - postgis-layer-style-2.style + name: postgis-styling title: some service title url: http://localhost:32788/datasetOwner/dataset/2016/wms/v1_0 diff --git a/internal/controller/test_data/wms/custom-mapfile/expected/configmap-capabilities-generator.yaml b/internal/controller/test_data/wms/custom-mapfile/expected/configmap-capabilities-generator.yaml index 9e6361b..75a1321 100644 --- a/internal/controller/test_data/wms/custom-mapfile/expected/configmap-capabilities-generator.yaml +++ b/internal/controller/test_data/wms/custom-mapfile/expected/configmap-capabilities-generator.yaml @@ -139,7 +139,7 @@ metadata: inspire: "false" service-type: wms service-version: v1_0 - name: custom-mapfile-wms-capabilities-generator-f244chh92c + name: custom-mapfile-wms-capabilities-generator-745hbmh8kd namespace: default ownerReferences: - apiVersion: pdok.nl/v3 diff --git a/internal/controller/test_data/wms/custom-mapfile/expected/configmap-legend-generator.yaml b/internal/controller/test_data/wms/custom-mapfile/expected/configmap-legend-generator.yaml index 1d651a5..1e99e14 100644 --- a/internal/controller/test_data/wms/custom-mapfile/expected/configmap-legend-generator.yaml +++ b/internal/controller/test_data/wms/custom-mapfile/expected/configmap-legend-generator.yaml @@ -15,7 +15,7 @@ metadata: inspire: "false" service-type: wms service-version: v1_0 - name: custom-mapfile-wms-legend-generator-6cf9f5k5h5 + name: custom-mapfile-wms-legend-generator-82hh8mg962 namespace: default ownerReferences: - apiVersion: pdok.nl/v3 diff --git a/internal/controller/test_data/wms/custom-mapfile/expected/deployment.yaml b/internal/controller/test_data/wms/custom-mapfile/expected/deployment.yaml index 58915d9..f0ec37c 100644 --- a/internal/controller/test_data/wms/custom-mapfile/expected/deployment.yaml +++ b/internal/controller/test_data/wms/custom-mapfile/expected/deployment.yaml @@ -328,7 +328,7 @@ spec: name: custom-mapfile-wms-init-scripts-fft29bbtdd name: init-scripts - configMap: - name: custom-mapfile-wms-capabilities-generator-f244chh92c + name: custom-mapfile-wms-capabilities-generator-745hbmh8kd defaultMode: 420 name: capabilities-generator-config - configMap: @@ -336,6 +336,6 @@ spec: defaultMode: 420 name: featureinfo-generator-config - configMap: - name: custom-mapfile-wms-legend-generator-6cf9f5k5h5 + name: custom-mapfile-wms-legend-generator-82hh8mg962 defaultMode: 420 name: legend-generator-config diff --git a/internal/controller/test_data/wms/custom-mapfile/input/wms.yaml b/internal/controller/test_data/wms/custom-mapfile/input/wms.yaml index 4306992..379a413 100644 --- a/internal/controller/test_data/wms/custom-mapfile/input/wms.yaml +++ b/internal/controller/test_data/wms/custom-mapfile/input/wms.yaml @@ -83,6 +83,8 @@ spec: styles: - name: group-child title: group-child + - name: style + title: style title: group-child visible: true keywords: @@ -90,6 +92,9 @@ spec: name: group title: group visible: true + styles: + - name: style + title: style title: service-title visible: true mapfile: @@ -97,10 +102,5 @@ spec: name: configMap key: mapfile.map ownerInfoRef: owner - stylingAssets: - configMapRefs: - - keys: - - layer-style.style - name: styling title: service-title url: http://localhost:32788/datasetOwner/dataset/wms/v1_0 \ No newline at end of file diff --git a/internal/controller/test_data/wms/minimal/expected/configmap-capabilities-generator.yaml b/internal/controller/test_data/wms/minimal/expected/configmap-capabilities-generator.yaml index 78c9274..0bad664 100644 --- a/internal/controller/test_data/wms/minimal/expected/configmap-capabilities-generator.yaml +++ b/internal/controller/test_data/wms/minimal/expected/configmap-capabilities-generator.yaml @@ -139,7 +139,7 @@ metadata: inspire: "false" service-type: wms service-version: v1_0 - name: minimal-wms-capabilities-generator-f244chh92c + name: minimal-wms-capabilities-generator-745hbmh8kd namespace: default ownerReferences: - apiVersion: pdok.nl/v3 diff --git a/internal/controller/test_data/wms/minimal/expected/configmap-legend-generator.yaml b/internal/controller/test_data/wms/minimal/expected/configmap-legend-generator.yaml index 420a993..73b37bf 100644 --- a/internal/controller/test_data/wms/minimal/expected/configmap-legend-generator.yaml +++ b/internal/controller/test_data/wms/minimal/expected/configmap-legend-generator.yaml @@ -15,7 +15,7 @@ metadata: inspire: "false" service-type: wms service-version: v1_0 - name: minimal-wms-legend-generator-6cf9f5k5h5 + name: minimal-wms-legend-generator-82hh8mg962 namespace: default ownerReferences: - apiVersion: pdok.nl/v3 diff --git a/internal/controller/test_data/wms/minimal/expected/configmap-mapfile-generator.yaml b/internal/controller/test_data/wms/minimal/expected/configmap-mapfile-generator.yaml index d56ebef..2b67165 100644 --- a/internal/controller/test_data/wms/minimal/expected/configmap-mapfile-generator.yaml +++ b/internal/controller/test_data/wms/minimal/expected/configmap-mapfile-generator.yaml @@ -107,7 +107,7 @@ metadata: inspire: "false" service-type: wms service-version: v1_0 - name: minimal-wms-mapfile-generator-kfkkm2kf78 + name: minimal-wms-mapfile-generator-hkt264tk5b namespace: default ownerReferences: - apiVersion: pdok.nl/v3 diff --git a/internal/controller/test_data/wms/minimal/expected/deployment.yaml b/internal/controller/test_data/wms/minimal/expected/deployment.yaml index 63ab811..96b3f92 100644 --- a/internal/controller/test_data/wms/minimal/expected/deployment.yaml +++ b/internal/controller/test_data/wms/minimal/expected/deployment.yaml @@ -342,11 +342,11 @@ spec: name: minimal-wms-init-scripts-fft29bbtdd name: init-scripts - configMap: - name: minimal-wms-capabilities-generator-f244chh92c + name: minimal-wms-capabilities-generator-745hbmh8kd defaultMode: 420 name: capabilities-generator-config - configMap: - name: minimal-wms-mapfile-generator-kfkkm2kf78 + name: minimal-wms-mapfile-generator-hkt264tk5b defaultMode: 420 name: mapfile-generator-config - name: styling-files @@ -359,6 +359,6 @@ spec: defaultMode: 420 name: featureinfo-generator-config - configMap: - name: minimal-wms-legend-generator-6cf9f5k5h5 + name: minimal-wms-legend-generator-82hh8mg962 defaultMode: 420 name: legend-generator-config diff --git a/internal/controller/test_data/wms/minimal/input/wms.yaml b/internal/controller/test_data/wms/minimal/input/wms.yaml index 1a1363e..05c5451 100644 --- a/internal/controller/test_data/wms/minimal/input/wms.yaml +++ b/internal/controller/test_data/wms/minimal/input/wms.yaml @@ -85,6 +85,9 @@ spec: - name: group-child title: group-child visualization: layer-style.style + - name: style + title: style + visualization: style.style title: group-child visible: true keywords: @@ -92,6 +95,9 @@ spec: name: group title: group visible: true + styles: + - name: style + title: style title: service-title visible: true ownerInfoRef: owner @@ -99,6 +105,7 @@ spec: configMapRefs: - keys: - layer-style.style + - style.style name: styling title: service-title url: http://localhost:32788/datasetOwner/dataset/wms/v1_0 \ No newline at end of file diff --git a/internal/controller/test_data/wms/noprefetch/expected/configmap-capabilities-generator.yaml b/internal/controller/test_data/wms/noprefetch/expected/configmap-capabilities-generator.yaml index 131dd30..411a680 100644 --- a/internal/controller/test_data/wms/noprefetch/expected/configmap-capabilities-generator.yaml +++ b/internal/controller/test_data/wms/noprefetch/expected/configmap-capabilities-generator.yaml @@ -139,7 +139,7 @@ metadata: inspire: "false" service-type: wms service-version: v1_0 - name: noprefetch-wms-capabilities-generator-f244chh92c + name: noprefetch-wms-capabilities-generator-745hbmh8kd namespace: default ownerReferences: - apiVersion: pdok.nl/v3 diff --git a/internal/controller/test_data/wms/noprefetch/expected/configmap-legend-generator.yaml b/internal/controller/test_data/wms/noprefetch/expected/configmap-legend-generator.yaml index 7e76825..28ff622 100644 --- a/internal/controller/test_data/wms/noprefetch/expected/configmap-legend-generator.yaml +++ b/internal/controller/test_data/wms/noprefetch/expected/configmap-legend-generator.yaml @@ -15,7 +15,7 @@ metadata: inspire: "false" service-type: wms service-version: v1_0 - name: noprefetch-wms-legend-generator-6cf9f5k5h5 + name: noprefetch-wms-legend-generator-82hh8mg962 namespace: default ownerReferences: - apiVersion: pdok.nl/v3 diff --git a/internal/controller/test_data/wms/noprefetch/expected/configmap-mapfile-generator.yaml b/internal/controller/test_data/wms/noprefetch/expected/configmap-mapfile-generator.yaml index 6471f53..0748fd5 100644 --- a/internal/controller/test_data/wms/noprefetch/expected/configmap-mapfile-generator.yaml +++ b/internal/controller/test_data/wms/noprefetch/expected/configmap-mapfile-generator.yaml @@ -107,7 +107,7 @@ metadata: inspire: "false" service-type: wms service-version: v1_0 - name: noprefetch-wms-mapfile-generator-d47d4264kh + name: noprefetch-wms-mapfile-generator-874tg4ghb5 namespace: default ownerReferences: - apiVersion: pdok.nl/v3 diff --git a/internal/controller/test_data/wms/noprefetch/expected/deployment.yaml b/internal/controller/test_data/wms/noprefetch/expected/deployment.yaml index 9a38f26..e564bfe 100644 --- a/internal/controller/test_data/wms/noprefetch/expected/deployment.yaml +++ b/internal/controller/test_data/wms/noprefetch/expected/deployment.yaml @@ -334,11 +334,11 @@ spec: defaultMode: 420 name: ogc-webservice-proxy-config - configMap: - name: noprefetch-wms-capabilities-generator-f244chh92c + name: noprefetch-wms-capabilities-generator-745hbmh8kd defaultMode: 420 name: capabilities-generator-config - configMap: - name: noprefetch-wms-mapfile-generator-d47d4264kh + name: noprefetch-wms-mapfile-generator-874tg4ghb5 defaultMode: 420 name: mapfile-generator-config - name: styling-files @@ -351,6 +351,6 @@ spec: defaultMode: 420 name: featureinfo-generator-config - configMap: - name: noprefetch-wms-legend-generator-6cf9f5k5h5 + name: noprefetch-wms-legend-generator-82hh8mg962 defaultMode: 420 name: legend-generator-config diff --git a/internal/controller/test_data/wms/noprefetch/input/wms.yaml b/internal/controller/test_data/wms/noprefetch/input/wms.yaml index 6408b8e..de64255 100644 --- a/internal/controller/test_data/wms/noprefetch/input/wms.yaml +++ b/internal/controller/test_data/wms/noprefetch/input/wms.yaml @@ -86,6 +86,9 @@ spec: - name: group-child title: group-child visualization: layer-style.style + - name: style + title: style + visualization: style.style title: group-child visible: true keywords: @@ -93,6 +96,9 @@ spec: name: group title: group visible: true + styles: + - name: style + title: style title: service-title visible: true ownerInfoRef: owner @@ -100,6 +106,7 @@ spec: configMapRefs: - keys: - layer-style.style + - style.style name: styling title: service-title url: http://localhost:32788/datasetOwner/dataset/wms/v1_0 \ No newline at end of file diff --git a/internal/webhook/v3/test_data/v3_wms.yaml b/internal/webhook/v3/test_data/v3_wms.yaml index 9a4d079..444a4c3 100644 --- a/internal/webhook/v3/test_data/v3_wms.yaml +++ b/internal/webhook/v3/test_data/v3_wms.yaml @@ -1,153 +1,71 @@ apiVersion: pdok.nl/v3 kind: WMS metadata: - labels: - app.kubernetes.io/name: mapserver-operator - app.kubernetes.io/managed-by: kustomize - dataset: dataset - dataset-owner: owner - service-type: wms - service-version: 1.0.0 - name: sample-v3 + name: sample namespace: default + labels: + sample: sample spec: - lifecycle: - ttlInDays: 21 podSpecPatch: containers: - name: mapserver resources: limits: - memory: 12M - ephemeral-storage: 2G - horizontalPodAutoscalerPatch: - scaleTargetRef: - apiVersion: apps/v1 - kind: Deployment - name: wms-sample-v3 - maxReplicas: 5 - minReplicas: 2 - metrics: - - type: "Resource" - resource: - name: cpu - target: - type: Utilization - averageUtilization: 60 - options: - automaticCasing: true - prefetchData: false - includeIngress: false - rewriteGroupToDataLayers: true - validateChildStyleNameEqual: true + ephemeral-storage: 1G + ingressRouteUrls: + - url: "https://test.test/path" service: - url: https://service.pdok.nl/owner/dataset/wms/1.0.0 - title: "Dataset" - abstract: "Dataset abstract ..." + prefix: "prefix" + url: "https://test.test/path" + title: "title" + abstract: "abstract" keywords: - - keyword1 - - keyword2 - ownerInfoRef: owner - fees: "" - accessConstraints: "http://creativecommons.org/publicdomain/zero/1.0/deed.nl" - maxSize: - inspire: - serviceMetadataUrl: - csw: - metadataIdentifier: 1234abcd-1234-abcd-1234-abcd1234abcd - spatialDatasetIdentifier: abcd1234-abcd-1234-abcd-1234abcd1234 - language: "dut" - dataEPSG: EPSG:28992 - resolution: - defResolution: + - "keyword" + ownerInfoRef: "owner" + dataEPSG: "EPSG:28992" stylingAssets: - mapfile: + configMapRefs: + - name: configmap + keys: + - file.style layer: - name: top-layer-name - title: Top "Layer" Title - abstract: Top "Layer" Abstract + title: "title" + abstract: "abstract" keywords: - - top-layer-keyword-1 - - top-layer-keyword-2 - boundingBoxes: - - crs: EPSG:28992 - bbox: - minx: "482.06" - maxx: "306602.42" - miny: "284182.97" - maxy: "637049.52" + - "keyword" visible: true - authority: - datasetMetadataUrl: - minscaledenominator: - maxscaledenominator: - style: - labelNoClip: false - data: layers: - - name: group-layer-name - title: Group "Layer" Title - abstract: Group "Abstract" Abstract + - name: "visible" + visible: true + title: "title" + abstract: "abstract" keywords: - - group-layer-keyword-1 - - group-layer-keyword-2 - boundingBoxes: + - keyword + data: + gpkg: + blobKey: "container/path/file.gpkg" + columns: + - name: "column" + geometryType: "Point" + tableName: "table" + styles: + - name: "name" + title: "title" + visualization: "file.style" + - name: "visible Group Layer" visible: true - authority: - datasetMetadataUrl: - minscaledenominator: - maxscaledenominator: styles: - labelNoClip: false + - name: style + title: style layers: - - name: gpkg-layer-name - title: GPKG "Layer" Title - abstract: GPKG "Abstract" Abstract - keywords: - - gpkg-layer-keyword-1 - - gpkg-layer-keyword-2 - boundingBoxes: - visible: true - authority: - datasetMetadataUrl: - minscaledenominator: - maxscaledenominator: - styles: - - name: gpkg-layer-style-name - title: gpkg-layer-style-title - visualization: gpkg-layer-style.style - labelNoClip: false + - name: "not visible" + visible: false data: - gpkg: - blobKey: "geopackages-bucket/key/gpkg-layer-data.gpkg" - tableName: "table-1" - geometryType: "MultiPolygon" + postgis: columns: - - name: "column-1" - alias: "alias-column-1" - - name: "column-2" - - name: "column-3" - alias: "alias-column-3" - - name: tif-layer-name - title: TIF "Layer" Title - abstract: TIF "Abstract" Abstract - keywords: - - tif-layer-keyword-1 - - tif-layer-keyword-2 - boundingBoxes: - visible: true - authority: - datasetMetadataUrl: - minscaledenominator: - maxscaledenominator: + - name: "column" + geometryType: "Point" + tableName: "table" styles: - - name: tif-layer-style-1-name - title: tif-layer-style-1-title - visualization: tif-layer-style-1.style - labelNoClip: false - data: - tif: - blobKey: "tifs-bucket/key/tif-layer-data.tif" - offsite: "#FF00FF" - resample: "AVERAGE" - getFeatureInfoIncludesClass: true + - name: "style" + visualization: "file.style" diff --git a/internal/webhook/v3/webhook_suite_test.go b/internal/webhook/v3/webhook_suite_test.go index 020ba2c..ea12f53 100644 --- a/internal/webhook/v3/webhook_suite_test.go +++ b/internal/webhook/v3/webhook_suite_test.go @@ -38,6 +38,12 @@ import ( "testing" "time" + "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" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + smoothoperatorv1 "github.com/pdok/smooth-operator/api/v1" "golang.org/x/tools/go/packages" @@ -229,3 +235,16 @@ func must[T any](t T, err error) T { } return t } + +func getValidationError[O pdoknlv3.WMSWFS](obj O, errorList *field.Error) error { + return apierrors.NewInvalid(obj.GroupKind(), obj.GetName(), field.ErrorList{errorList}) +} + +func getValidationWarnings[O pdoknlv3.WMSWFS](obj O, path field.Path, warning string, warnings []string) admission.Warnings { + validation.AddWarning(&warnings, path, warning, schema.GroupVersionKind{ + Group: obj.GroupKind().Group, + Version: "v3", + Kind: obj.GroupKind().Kind, + }, obj.GetName()) + return warnings +} diff --git a/internal/webhook/v3/wfs_webhook_test.go b/internal/webhook/v3/wfs_webhook_test.go index 9b87b07..27c849a 100644 --- a/internal/webhook/v3/wfs_webhook_test.go +++ b/internal/webhook/v3/wfs_webhook_test.go @@ -27,13 +27,16 @@ package v3 //nolint:revive // Complains about the dot imports import ( "context" + "fmt" + + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/utils/ptr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" pdoknlv3 "github.com/pdok/mapserver-operator/api/v3" smoothoperatorv1 "github.com/pdok/smooth-operator/api/v1" smoothoperatormodel "github.com/pdok/smooth-operator/model" - smoothoperatorutils "github.com/pdok/smooth-operator/pkg/util" corev1 "k8s.io/api/core/v1" ) @@ -83,17 +86,27 @@ var _ = Describe("WFS Webhook", func() { It("Should deny creation if there are no labels", func() { obj.Labels = nil warnings, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("metadata").Child("labels"), + "can't be empty", + )))) Expect(warnings).To(BeEmpty()) }) - It("Should deny creation if Url not in IngressRouteUrls", func() { - Expect(obj.Spec.IngressRouteURLs).NotTo(BeNil()) - url, err := smoothoperatormodel.ParseURL("https://new/new") - Expect(err).To(Not(HaveOccurred())) + It("Should deny Create when URL not in IngressRouteURLs", func() { + url, err := smoothoperatormodel.ParseURL("http://changed/changed") + Expect(err).To(BeNil()) + obj.Spec.IngressRouteURLs = []smoothoperatormodel.IngressRouteURL{{URL: smoothoperatormodel.URL{URL: url}}} + url, err = smoothoperatormodel.ParseURL("http://sample/sample") + Expect(err).To(BeNil()) obj.Spec.Service.URL = smoothoperatormodel.URL{URL: url} + warnings, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("ingressRouteUrls"), + fmt.Sprint(obj.Spec.IngressRouteURLs), + fmt.Sprintf("must contain baseURL: %s", url), + )))) Expect(warnings).To(BeEmpty()) }) @@ -101,14 +114,22 @@ var _ = Describe("WFS Webhook", func() { obj.Name += "-wfs" warnings, err := validator.ValidateCreate(ctx, obj) Expect(err).To(BeNil()) - Expect(len(warnings)).To(Equal(1)) + Expect(warnings).To(Equal(getValidationWarnings( + obj, + *field.NewPath("metadata").Child("name"), + "name should not contain wfs", + []string{}, + ))) }) It("Should deny creation if there is no bounding box and the defaultCRS is not EPSG:28992", func() { obj.Spec.Service.DefaultCrs = "EPSG:1234" obj.Spec.Service.Bbox = nil warnings, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("spec").Child("service").Child("bbox").Child("defaultCRS"), + "when service.defaultCRS is not 'EPSG:28992'", + )))) Expect(warnings).To(BeEmpty()) }) @@ -119,7 +140,16 @@ var _ = Describe("WFS Webhook", func() { obj.Spec.Service.Mapfile = &pdoknlv3.Mapfile{} warnings, err := validator.ValidateCreate(ctx, obj) Expect(err).To(BeNil()) - Expect(len(warnings)).To(Equal(2)) + Expect(warnings).To(Equal(getValidationWarnings( + obj, + *field.NewPath("spec").Child("service").Child("featureTypes").Index(0).Child("bbox").Child("defaultCrs"), + "is not used when service.mapfile is configured", + getValidationWarnings( + obj, + *field.NewPath("spec").Child("service").Child("bbox"), + "is not used when service.mapfile is configured", + []string{}, + )))) }) It("Should deny creation if SpatialID is also used as a featureType datasetMetadataID", func() { @@ -128,7 +158,11 @@ var _ = Describe("WFS Webhook", func() { Expect(obj.Spec.Service.FeatureTypes[0].DatasetMetadataURL.CSW).NotTo(BeNil()) obj.Spec.Service.Inspire.SpatialDatasetIdentifier = obj.Spec.Service.FeatureTypes[0].DatasetMetadataURL.CSW.MetadataIdentifier warnings, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("service").Child("inspire").Child("spatialDatasetIdentifier"), + obj.Spec.Service.Inspire.SpatialDatasetIdentifier, + "spatialDatasetIdentifier cannot also be used as an datasetMetadataUrl.csw.metadataIdentifier", + )))) Expect(warnings).To(BeEmpty()) }) @@ -139,7 +173,11 @@ var _ = Describe("WFS Webhook", func() { Expect(obj.Spec.Service.FeatureTypes[0].DatasetMetadataURL.CSW).NotTo(BeNil()) obj.Spec.Service.Inspire.ServiceMetadataURL.CSW.MetadataIdentifier = obj.Spec.Service.FeatureTypes[0].DatasetMetadataURL.CSW.MetadataIdentifier warnings, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("service").Child("inspire").Child("csw").Child("metadataIdentifier"), + obj.Spec.Service.Inspire.ServiceMetadataURL.CSW.MetadataIdentifier, + "serviceMetadataUrl.csw.metadataIdentifier cannot also be used as an datasetMetadataUrl.csw.metadataIdentifier", + )))) Expect(warnings).To(BeEmpty()) }) @@ -148,7 +186,11 @@ var _ = Describe("WFS Webhook", func() { Expect(obj.Inspire().ServiceMetadataURL.CSW).NotTo(BeNil()) obj.Spec.Service.Inspire.ServiceMetadataURL.CSW.MetadataIdentifier = obj.Spec.Service.Inspire.SpatialDatasetIdentifier warnings, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("service").Child("inspire").Child("csw").Child("metadataIdentifier"), + obj.Spec.Service.Inspire.ServiceMetadataURL.CSW.MetadataIdentifier, + "serviceMetadataUrl.csw.metadataIdentifier cannot also be used as the spatialDatasetIdentifier", + )))) Expect(warnings).To(BeEmpty()) }) @@ -162,24 +204,40 @@ var _ = Describe("WFS Webhook", func() { Expect(obj.Spec.Service.FeatureTypes[1].DatasetMetadataURL.CSW).NotTo(BeNil()) obj.Spec.Service.FeatureTypes[0].DatasetMetadataURL.CSW.MetadataIdentifier = "" warnings, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("service").Child("featureTypes[*]").Child("datasetMetadataUrl").Child("csw").Child("metadataIdentifier"), + obj.DatasetMetadataIDs(), + "when Inspire, all featureTypes need use the same datasetMetadataUrl.csw.metadataIdentifier", + )))) Expect(warnings).To(BeEmpty()) }) - It("Should deny creation if maxReplicas < minReplicas in HPAPatch", func() { + It("Should deny Create when minReplicas are larger than maxReplicas", func() { obj.Spec.HorizontalPodAutoscalerPatch = &pdoknlv3.HorizontalPodAutoscalerPatch{ - MinReplicas: smoothoperatorutils.Pointer(int32(5)), - MaxReplicas: smoothoperatorutils.Pointer(int32(1)), + MinReplicas: ptr.To(int32(10)), + MaxReplicas: ptr.To(int32(5)), } + warnings, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("horizontalPodAutoscaler"), + fmt.Sprintf("minReplicas: %d, maxReplicas: %d", 10, 5), + "maxReplicas cannot be less than minReplicas", + )))) Expect(warnings).To(BeEmpty()) }) - It("Should deny creation if no ephemeralStorage is set on the mapserver container", func() { - obj.Spec.PodSpecPatch.Containers = []corev1.Container{} + It("Should deny Create when mapserver container doesn't have ephemeral storage", func() { + obj.Spec.PodSpecPatch = corev1.PodSpec{} + warnings, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Required(field.NewPath("spec"). + Child("podSpecPatch"). + Child("containers"). + Key("mapserver"). + Child("resources"). + Child("limits"). + Child(corev1.ResourceEphemeralStorage.String()), "")))) Expect(warnings).To(BeEmpty()) }) @@ -187,142 +245,207 @@ var _ = Describe("WFS Webhook", func() { Expect(len(obj.Spec.Service.FeatureTypes)).To(BeNumerically(">", 1)) obj.Spec.Service.FeatureTypes[1].Name = obj.Spec.Service.FeatureTypes[0].Name warnings, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Duplicate( + field.NewPath("spec").Child("service").Child("featureTypes").Index(1).Child("name"), + obj.Spec.Service.FeatureTypes[1].Name, + )))) Expect(warnings).To(BeEmpty()) }) - It("Warns if the mapfile and featuretype.tif extra settings are both set", func() { - Expect(len(obj.Spec.Service.FeatureTypes)).To(BeNumerically(">", 1)) - Expect(obj.Spec.Service.FeatureTypes[1].Data.TIF).NotTo(BeNil()) - obj.Spec.Service.FeatureTypes[1].Data.TIF = &pdoknlv3.TIF{ - BlobKey: obj.Spec.Service.FeatureTypes[1].Data.TIF.BlobKey, - Resample: "AVERAGE", - Offsite: smoothoperatorutils.Pointer("#555555"), - GetFeatureInfoIncludesClass: true, - } + It("Should deny create if the OwnerInfoRef doesn't exist", func() { + obj.Spec.Service.OwnerInfoRef = "changed" + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.NotFound( + field.NewPath("spec").Child("service").Child("ownerInfoRef"), + obj.Spec.Service.OwnerInfoRef, + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny create if the OwnerInfoRef misses namespaceTemplate", func() { + ownerInfo.Spec.NamespaceTemplate = nil + + Expect(updateOwnerInfo(ctx, k8sClient, ownerInfo)).To(Succeed()) + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("spec").Child("service").Child("ownerInfoRef"), + "spec.namespaceTemplate missing in "+ownerInfo.Name, + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny create if the OwnerInfoRef misses csw metadataTemplate", func() { + obj.Spec.Service.Inspire = &pdoknlv3.WFSInspire{Inspire: pdoknlv3.Inspire{ServiceMetadataURL: pdoknlv3.MetadataURL{CSW: &pdoknlv3.Metadata{MetadataIdentifier: "metadata"}}}} + ownerInfo.Spec.MetadataUrls.CSW = nil + + Expect(updateOwnerInfo(ctx, k8sClient, ownerInfo)).To(Succeed()) warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("spec").Child("service").Child("ownerInfoRef"), + "spec.metadataUrls.csw missing in "+ownerInfo.Name, + )))) + Expect(warnings).To(BeEmpty()) + + ownerInfo.Spec.MetadataUrls = nil + Expect(updateOwnerInfo(ctx, k8sClient, ownerInfo)).To(Succeed()) + warnings, err = validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("spec").Child("service").Child("ownerInfoRef"), + "spec.metadataUrls.csw missing in "+ownerInfo.Name, + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny create if the OwnerInfoRef misses WMS", func() { + ownerInfo.Spec.WFS = nil + Expect(updateOwnerInfo(ctx, k8sClient, ownerInfo)).To(Succeed()) + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("spec").Child("service").Child("ownerInfoRef"), + "spec.WFS missing in "+ownerInfo.Name, + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny update if a ingressRouteURL was removed", func() { + url, err := smoothoperatormodel.ParseURL("http://new.url/path") Expect(err).To(BeNil()) - Expect(len(warnings)).To(Equal(3)) + oldObj.Spec.IngressRouteURLs = []smoothoperatormodel.IngressRouteURL{ + {URL: obj.URL()}, + {URL: smoothoperatormodel.URL{URL: url}}, + } + obj.Spec.IngressRouteURLs = []smoothoperatormodel.IngressRouteURL{{URL: obj.URL()}} + warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("ingressRouteUrls"), + fmt.Sprint(obj.Spec.IngressRouteURLs), + fmt.Sprintf("urls cannot be removed, missing: %s", smoothoperatormodel.IngressRouteURL{URL: smoothoperatormodel.URL{URL: url}}), + )))) + Expect(warnings).To(BeEmpty()) }) - It("Should deny update if a url was changed and ingressRouteUrls = nil", func() { - url, err := smoothoperatormodel.ParseURL("http://old/path") + It("Should accept update if a url was changed when it's in ingressRouteUrls", func() { + url, err := smoothoperatormodel.ParseURL("http://new.url/path") Expect(err).To(BeNil()) + oldObj.Spec.IngressRouteURLs = []smoothoperatormodel.IngressRouteURL{ + {URL: obj.URL()}, + {URL: smoothoperatormodel.URL{URL: url}}, + } + obj.Spec.IngressRouteURLs = oldObj.Spec.IngressRouteURLs + oldObj.Spec.Service.URL = obj.URL() obj.Spec.Service.URL = smoothoperatormodel.URL{URL: url} + warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) Expect(err).To(BeNil()) Expect(warnings).To(BeEmpty()) + }) - oldObj.Spec.IngressRouteURLs = nil + It("Should deny update if a url was changed and ingressRouteUrls = nil", func() { + url, err := smoothoperatormodel.ParseURL("http://new.url/path") + Expect(err).To(BeNil()) + obj.Spec.Service.URL = smoothoperatormodel.URL{URL: url} obj.Spec.IngressRouteURLs = nil - warnings, err = validator.ValidateUpdate(ctx, oldObj, obj) - Expect(err).To(HaveOccurred()) - Expect(warnings).To(BeEmpty()) - }) + oldObj.Spec.IngressRouteURLs = nil - It("Should deny update if a ingressRouteURL was removed", func() { - Expect(len(oldObj.Spec.IngressRouteURLs)).To(Equal(2)) - obj.Spec.IngressRouteURLs = []smoothoperatormodel.IngressRouteURL{{URL: obj.URL()}} warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Forbidden( + field.NewPath("spec").Child("service").Child("url"), + "is immutable", + )))) Expect(warnings).To(BeEmpty()) }) It("Should deny update url was changed but not added to ingressRouteURLs", func() { - url, err := smoothoperatormodel.ParseURL("http://old/changed") + url, err := smoothoperatormodel.ParseURL("http://new.url/path") Expect(err).ToNot(HaveOccurred()) oldObj.Spec.IngressRouteURLs = nil obj.Spec.IngressRouteURLs = []smoothoperatormodel.IngressRouteURL{{URL: oldObj.Spec.Service.URL}} obj.Spec.Service.URL = smoothoperatormodel.URL{URL: url} warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("ingressRouteUrls"), + fmt.Sprint(obj.Spec.IngressRouteURLs), + fmt.Sprintf("must contain baseURL: %s", obj.URL()), + )))) Expect(warnings).To(BeEmpty()) obj.Spec.IngressRouteURLs = []smoothoperatormodel.IngressRouteURL{{URL: smoothoperatormodel.URL{URL: url}}} warnings, err = validator.ValidateUpdate(ctx, oldObj, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("ingressRouteUrls"), + fmt.Sprint(obj.Spec.IngressRouteURLs), + fmt.Sprintf("must contain baseURL: %s", oldObj.URL()), + )))) Expect(warnings).To(BeEmpty()) }) It("Should deny update if a label was removed", func() { + oldKey := "" for label := range obj.Labels { + oldKey = label delete(obj.Labels, label) break } warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("metadata").Child("labels").Child(oldKey), + "labels cannot be removed", + )))) Expect(warnings).To(BeEmpty()) }) It("Should deny update if a label changed", func() { + oldKey := "" + oldValue := "" + newValue := "" for label, val := range obj.Labels { - obj.Labels[label] = val + "-newval" + oldKey = label + oldValue = val + newValue = val + "-newval" + obj.Labels[label] = newValue break } warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("metadata").Child("labels").Child(oldKey), + newValue, + "immutable: should be: "+oldValue, + )))) Expect(warnings).To(BeEmpty()) }) It("Should deny update if a label was added", func() { - obj.Labels["new-label"] = "test" + newKey := "new-label" + obj.Labels[newKey] = "test" warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Forbidden( + field.NewPath("metadata").Child("labels").Child(newKey), + "new labels cannot be added", + )))) Expect(warnings).To(BeEmpty()) }) It("Should deny update if an inspire block was added", func() { - Expect(obj.Spec.Service.Inspire).NotTo(BeNil()) + obj.Spec.Service.Inspire = &pdoknlv3.WFSInspire{} oldObj.Spec.Service.Inspire = nil warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Forbidden( + field.NewPath("spec").Child("service").Child("inspire"), + "cannot change from inspire to not inspire or the other way around", + )))) Expect(warnings).To(BeEmpty()) }) It("Should deny update if an inspire block was removed", func() { - Expect(oldObj.Spec.Service.Inspire).NotTo(BeNil()) + oldObj.Spec.Service.Inspire = &pdoknlv3.WFSInspire{} obj.Spec.Service.Inspire = nil warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) - Expect(err).To(HaveOccurred()) - Expect(warnings).To(BeEmpty()) - }) - - It("Should deny create if the OwnerInfoRef doesn't exist", func() { - obj.Spec.Service.OwnerInfoRef = "changed" - warnings, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) - Expect(warnings).To(BeEmpty()) - }) - - It("Should deny create if the OwnerInfoRef misses namespaceTemplate", func() { - ownerInfo.Spec.NamespaceTemplate = nil - Expect(updateOwnerInfo(ctx, k8sClient, ownerInfo)).To(Succeed()) - warnings, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) - Expect(warnings).To(BeEmpty()) - }) - - It("Should deny create if the OwnerInfoRef misses csw metadataTemplate", func() { - Expect(obj.Inspire().ServiceMetadataURL.CSW).ToNot(BeNil()) - ownerInfo.Spec.MetadataUrls.CSW = nil - Expect(updateOwnerInfo(ctx, k8sClient, ownerInfo)).To(Succeed()) - warnings, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) - Expect(warnings).To(BeEmpty()) - - ownerInfo.Spec.MetadataUrls = nil - Expect(updateOwnerInfo(ctx, k8sClient, ownerInfo)).To(Succeed()) - warnings, err = validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) - Expect(warnings).To(BeEmpty()) - }) - - It("Should deny create if the OwnerInfoRef misses Wfs", func() { - ownerInfo.Spec.WFS = nil - Expect(updateOwnerInfo(ctx, k8sClient, ownerInfo)).To(Succeed()) - warnings, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + Expect(err).To(Equal(getValidationError(obj, field.Forbidden( + field.NewPath("spec").Child("service").Child("inspire"), + "cannot change from inspire to not inspire or the other way around", + )))) Expect(warnings).To(BeEmpty()) }) }) diff --git a/internal/webhook/v3/wms_webhook_test.go b/internal/webhook/v3/wms_webhook_test.go index 13bba8f..e170353 100644 --- a/internal/webhook/v3/wms_webhook_test.go +++ b/internal/webhook/v3/wms_webhook_test.go @@ -27,13 +27,17 @@ package v3 //nolint:revive // Complains about the dot imports import ( "context" + "fmt" + + smoothoperatormodel "github.com/pdok/smooth-operator/model" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/utils/ptr" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" pdoknlv3 "github.com/pdok/mapserver-operator/api/v3" smoothoperatorv1 "github.com/pdok/smooth-operator/api/v1" - smoothoperatorutils "github.com/pdok/smooth-operator/pkg/util" - corev1 "k8s.io/api/core/v1" ) var _ = Describe("WMS Webhook", func() { @@ -73,149 +77,578 @@ var _ = Describe("WMS Webhook", func() { ctx := context.Background() It("Creates the WMS from the sample", func() { - _, err := validator.ValidateCreate(ctx, obj) + warnings, err := validator.ValidateCreate(ctx, obj) Expect(err).To(BeNil()) + Expect(warnings).To(BeEmpty()) }) - It("Warns if the name contains WMS", func() { + It("Should Deny Create when Labels are empty", func() { + obj.Labels = nil + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("metadata").Child("labels"), + "can't be empty", + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny Create when URL not in IngressRouteURLs", func() { + url, err := smoothoperatormodel.ParseURL("http://changed/changed") + Expect(err).To(BeNil()) + obj.Spec.IngressRouteURLs = []smoothoperatormodel.IngressRouteURL{{URL: smoothoperatormodel.URL{URL: url}}} + url, err = smoothoperatormodel.ParseURL("http://sample/sample") + Expect(err).To(BeNil()) + obj.Spec.Service.URL = smoothoperatormodel.URL{URL: url} + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("ingressRouteUrls"), + fmt.Sprint(obj.Spec.IngressRouteURLs), + fmt.Sprintf("must contain baseURL: %s", url), + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Warns when the name contains WMS", func() { obj.Name += "-wms" warnings, err := validator.ValidateCreate(ctx, obj) Expect(err).To(BeNil()) - Expect(len(warnings)).To(BeNumerically(">", 0)) + Expect(warnings).To(Equal(getValidationWarnings( + obj, + *field.NewPath("metadata").Child("name"), + "name should not contain wms", + []string{}, + ))) }) - It("Should deny creation if there are no labels", func() { - obj.Labels = map[string]string{} - _, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + It("Warns when mapfile and resolution are set", func() { + withMapfile(obj) + obj.Spec.Service.Resolution = ptr.To(int32(5)) + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(BeNil()) + Expect(warnings).To(Equal(getValidationWarnings( + obj, + *field.NewPath("spec").Child("service").Child("resolution"), + "not used when service.mapfile is configured", + []string{}, + ))) }) - It("Should deny creation if layer names are not unique", func() { - childLayers := obj.Spec.Service.Layer.Layers - secondLayer := childLayers[0] - obj.Spec.Service.Layer.Name = secondLayer.Name - _, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + It("Warns when mapfile and defResolution are set", func() { + withMapfile(obj) + obj.Spec.Service.DefResolution = ptr.To(int32(5)) + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(BeNil()) + Expect(warnings).To(Equal(getValidationWarnings( + obj, + *field.NewPath("spec").Child("service").Child("defResolution"), + "not used when service.mapfile is configured", + []string{}, + ))) + }) + + It("Should deny Create when URL not in IngressRouteURLs", func() { + obj.Spec.Service.Inspire = &pdoknlv3.Inspire{ServiceMetadataURL: pdoknlv3.MetadataURL{CSW: &pdoknlv3.Metadata{MetadataIdentifier: "metadata"}}} + obj.Spec.Service.Layer.Layers[0].DatasetMetadataURL = &pdoknlv3.MetadataURL{CSW: &pdoknlv3.Metadata{MetadataIdentifier: "metadata"}} + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("service").Child("inspire").Child("csw").Child("metadataIdentifier"), + "metadata", + "serviceMetadataUrl.csw.metadataIdentifier cannot also be used as an datasetMetadataUrl.csw.metadataIdentifier", + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny Create when minReplicas are larger than maxReplicas", func() { + obj.Spec.HorizontalPodAutoscalerPatch = &pdoknlv3.HorizontalPodAutoscalerPatch{ + MinReplicas: ptr.To(int32(10)), + MaxReplicas: ptr.To(int32(5)), + } + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("horizontalPodAutoscaler"), + fmt.Sprintf("minReplicas: %d, maxReplicas: %d", 10, 5), + "maxReplicas cannot be less than minReplicas", + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny Create when mapserver container doesn't have ephemeral storage", func() { + obj.Spec.PodSpecPatch = corev1.PodSpec{} + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Required(field.NewPath("spec"). + Child("podSpecPatch"). + Child("containers"). + Key("mapserver"). + Child("resources"). + Child("limits"). + Child(corev1.ResourceEphemeralStorage.String()), "")))) + Expect(warnings).To(BeEmpty()) }) - It("Should deny creation if defaultCRS is not EPSG:28992 and layer has no boundingbox defined for the corresponding CRS", func() { - obj.Spec.Service.DataEPSG = "EPSG:4326" - _, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + It("Should deny Create when multiple layers have the same name", func() { + layerName := "equal" + obj.Spec.Service.Layer.Layers[0].Name = &layerName + obj.Spec.Service.Layer.Layers[1].Name = &layerName + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Duplicate( + field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(1).Child("name"), + layerName, + )))) + Expect(warnings).To(BeEmpty()) }) - It("Should deny creation if layer is visible and has no value for required field title", func() { - obj.Spec.Service.Layer.Title = nil - _, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + It("Should deny Create when Group Layer has data set", func() { + data := pdoknlv3.Data{} + obj.Spec.Service.Layer.Layers[1].Data = &data + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(1).Child("data"), + data, + "must not be set on a GroupLayer", + )))) + Expect(warnings).To(BeEmpty()) }) - It("Should deny creation if layer is visible and has no value for required field abstract", func() { - obj.Spec.Service.Layer.Abstract = nil - _, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + It("Warns when mapfile and layer boundingboxes are both set", func() { + withMapfile(obj) + obj.Spec.Service.Layer.BoundingBoxes = []pdoknlv3.WMSBoundingBox{{ + CRS: "", + BBox: smoothoperatormodel.BBox{}, + }} + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(BeNil()) + Expect(warnings).To(Equal(getValidationWarnings( + obj, + *field.NewPath("spec").Child("service").Child("layer").Child("boundingBoxes"), + "is not used when service.mapfile is configured", + []string{}, + ))) }) - It("Should deny creation if layer is visible and has no value for required field keywords", func() { - obj.Spec.Service.Layer.Keywords = nil - _, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + It("Should deny Create when there is no layer boundingbox set for dataepsg and no custom mapfile", func() { + obj.Spec.Service.Mapfile = nil + obj.Spec.Service.DataEPSG = "EPSG:1234" + obj.Spec.Service.Layer.Layers = []pdoknlv3.Layer{obj.Spec.Service.Layer.Layers[0]} + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(0).Child("boundingBoxes").Child("crs"), + fmt.Sprintf("must contain a boundingBox for CRS %s when service.dataEPSG is not 'EPSG:28992'", obj.Spec.Service.DataEPSG), + )))) + Expect(warnings).To(BeEmpty()) }) - It("Should deny creation if there is a visible layer without a style title", func() { - nestedLayers1 := obj.Spec.Service.Layer.Layers - nestedLayers2 := nestedLayers1[0].Layers - nestedLayers2[0].Styles[0].Title = nil - _, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + It("Warns when unused fields are set on a tiff connection when using a custom mapfile", func() { + withMapfile(obj) + obj.Spec.Service.Layer.Layers[0].Data = &pdoknlv3.Data{TIF: &pdoknlv3.TIF{ + BlobKey: "blobkey", + Resample: "AVERAGE", + Offsite: ptr.To("offsite"), + GetFeatureInfoIncludesClass: true, + }} + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(BeNil()) + Expect(warnings).To(Equal(getValidationWarnings( + obj, + *field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(0).Child("data").Child("tif").Child("getFeatureInfoIncludesClass"), + "is not used when service.mapfile is configured", + getValidationWarnings( + obj, + *field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(0).Child("data").Child("tif").Child("offsite"), + "is not used when service.mapfile is configured", + getValidationWarnings( + obj, + *field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(0).Child("data").Child("tif").Child("resample"), + "is not used when service.mapfile is configured", + []string{}, + ))))) }) - It("Should deny creation if layer has parent layer with same style name as child layer", func() { - nestedLayers1 := obj.Spec.Service.Layer.Layers - nestedLayers2 := nestedLayers1[0].Layers - nestedLayers1[0].Styles = []pdoknlv3.Style{{Name: nestedLayers2[0].Styles[0].Name}} - _, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + It("Should deny Create when there is a Group Layer that is not visible", func() { + obj.Spec.Service.Layer.Layers[1].Visible = false + obj.Spec.Service.Layer.Layers[1].Styles[0].Title = nil + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(1).Child("visible"), + false, + "must be true for a "+pdoknlv3.GroupLayer, + )))) + Expect(warnings).To(BeEmpty()) }) - It("Should deny creation if datalayer has style without visualization but there is no mapfile set", func() { - nestedLayers1 := obj.Spec.Service.Layer.Layers - nestedLayers2 := nestedLayers1[0].Layers - nestedLayers2[0].Styles[0].Visualization = nil - _, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + It("Warns when unused fields are set on a layer that is not visible", func() { + obj.Spec.Service.Layer.Layers[0].Visible = false + obj.Spec.Service.Layer.Layers[0].Title = ptr.To("title") + obj.Spec.Service.Layer.Layers[0].Abstract = ptr.To("abstract") + obj.Spec.Service.Layer.Layers[0].Keywords = []string{"keyword"} + obj.Spec.Service.Layer.Layers[0].DatasetMetadataURL = &pdoknlv3.MetadataURL{} + obj.Spec.Service.Layer.Layers[0].Authority = &pdoknlv3.Authority{} + obj.Spec.Service.Layer.Layers[0].Styles[0].Title = ptr.To("title") + obj.Spec.Service.Layer.Layers[0].Styles[0].Abstract = ptr.To("abstract") + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(BeNil()) + Expect(warnings).To(Equal(getValidationWarnings( + obj, + *field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(0).Child("styles").Index(0).Child("abstract"), + "is not used when layer.visible=false", getValidationWarnings( + obj, + *field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(0).Child("styles").Index(0).Child("title"), + "is not used when layer.visible=false", + getValidationWarnings( + obj, + *field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(0).Child("authority"), + "is not used when layer.visible=false", + getValidationWarnings( + obj, + *field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(0).Child("datasetMetadataURL"), + "is not used when layer.visible=false", + getValidationWarnings( + obj, + *field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(0).Child("keywords"), + "is not used when layer.visible=false", + getValidationWarnings( + obj, + *field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(0).Child("abstract"), + "is not used when layer.visible=false", + getValidationWarnings( + obj, + *field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(0).Child("title"), + "is not used when layer.visible=false", + []string{}, + ))))))))) }) - It("Should deny creation if datalayer has style with visualization but there is also a mapfile set", func() { - obj.Spec.Service.Mapfile = &pdoknlv3.Mapfile{ConfigMapKeyRef: corev1.ConfigMapKeySelector{Key: "mapfile.map"}} - _, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + It("Should deny Create when a Layer uses the same style name multiple times", func() { + styleName := "duplicate" + style := pdoknlv3.Style{ + Name: styleName, + Title: ptr.To("Title"), + Visualization: obj.Spec.Service.Layer.Layers[0].Styles[0].Visualization, + } + obj.Spec.Service.Layer.Layers[0].Styles = []pdoknlv3.Style{style, style} + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(0).Child("styles").Index(1).Child("name"), + styleName, + "A Layer can't use the same style name multiple times", + )))) + Expect(warnings).To(BeEmpty()) }) - It("Should deny creation if grouplayer is not visible", func() { - nestedLayers1 := obj.Spec.Service.Layer.Layers - nestedLayers1[0].Visible = false - _, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + It("Should deny Create when a Style doesn't have a title on its highest visible layer", func() { + obj.Spec.Service.Layer.Layers[1].Styles[0].Title = nil + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(1).Child("styles").Index(0).Child("title"), + "A Style must have a title on the highest visible Layer", + )))) + Expect(warnings).To(BeEmpty()) }) - It("Should deny creation if grouplayer has data", func() { - nestedLayers1 := obj.Spec.Service.Layer.Layers - nestedLayers1[0].Data = &pdoknlv3.Data{} - _, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + It("Should deny Create when a GroupLayer Style uses the same name as a Style from a parent Layer", func() { + styleName := "duplicate" + obj.Spec.Service.Layer.Styles = []pdoknlv3.Style{{Name: styleName, Title: ptr.To("title")}} + obj.Spec.Service.Layer.Layers[1].Styles = []pdoknlv3.Style{{Name: styleName, Title: ptr.To("title")}} + obj.Spec.Service.Layer.Layers[0].Styles[0].Name = styleName + obj.Spec.Service.Layer.Layers[1].Layers[0].Styles[0].Name = styleName + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(1).Child("styles").Index(0).Child("name"), + styleName, + "A GroupLayer can't redefine the same style as a parent layer", + )))) + Expect(warnings).To(BeEmpty()) }) - It("Should deny creation if grouplayer has style with visualization", func() { - nestedLayers1 := obj.Spec.Service.Layer.Layers - nestedLayers1[0].Styles = []pdoknlv3.Style{{Visualization: smoothoperatorutils.Pointer("visualization.style")}} - _, err := validator.ValidateCreate(ctx, obj) - Expect(err).To(HaveOccurred()) + It("Should deny Create when a GroupLayer Style has visualization", func() { + visualization := "file.style" + obj.Spec.Service.Layer.Layers[1].Styles[0].Visualization = &visualization + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(1).Child("styles").Index(0).Child("visualization"), + visualization, + "GroupLayers must not have a visualization", + )))) + Expect(warnings).To(BeEmpty()) }) - It("Should deny update if a label changed", func() { - for label, val := range obj.Labels { - obj.Labels[label] = val + "-newval" - break + It("Should deny Create when a Style has a visualization while a custom mapfile is configured", func() { + visualization := "file.style" + withMapfile(obj) + obj.Spec.Service.Layer.Layers[0].Styles[0].Visualization = &visualization + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(0).Child("styles").Index(0).Child("visualization"), + visualization, + "is not used when spec.service.mapfile is used", + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny Create when a Data Layer has a Style with no visualization while a no custom mapfile is configured", func() { + obj.Spec.Service.Layer.Layers[0].Styles[0].Visualization = nil + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(0).Child("styles").Index(0).Child("visualization"), + "on DataLayers when spec.service.mapfile is not used", + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny Create when a when a Visualization file is not defined in the stylingassets", func() { + visualization := "new.style" + obj.Spec.Service.Layer.Layers[0].Styles[0].Visualization = &visualization + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(0).Child("styles").Index(0).Child("visualization"), + visualization, + "must be defined be in spec.service.stylingAssets.configMapKeyRefs.Keys", + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny Create when a when a Group Layer style isn't implemented in a sub Data Layer", func() { + obj.Spec.Service.Layer.Layers[1].Styles[0].Name = "new" + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("service").Child("layer").Child("layers").Index(1).Child("layers").Index(0).Child("styles"), + nil, + fmt.Sprintf("dataLayer must implement style: %s, defined by a parent layer", "new"), + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny Create when there are no visible layers", func() { + obj.Spec.Service.Layer.Layers = []pdoknlv3.Layer{obj.Spec.Service.Layer.Layers[1].Layers[0]} + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("spec").Child("service").Child("layer").Child("layers[*]").Child("visible"), + "at least one layer must be visible", + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny create if the OwnerInfoRef doesn't exist", func() { + obj.Spec.Service.OwnerInfoRef = "changed" + + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.NotFound( + field.NewPath("spec").Child("service").Child("ownerInfoRef"), + "changed", + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny create if the OwnerInfoRef misses namespaceTemplate", func() { + ownerInfo.Spec.NamespaceTemplate = nil + + Expect(updateOwnerInfo(ctx, k8sClient, ownerInfo)).To(Succeed()) + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("spec").Child("service").Child("ownerInfoRef"), + "spec.namespaceTemplate missing in "+ownerInfo.Name, + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny create if the OwnerInfoRef misses csw metadataTemplate", func() { + obj.Spec.Service.Inspire = &pdoknlv3.Inspire{ServiceMetadataURL: pdoknlv3.MetadataURL{CSW: &pdoknlv3.Metadata{MetadataIdentifier: "metadata"}}} + ownerInfo.Spec.MetadataUrls.CSW = nil + + Expect(updateOwnerInfo(ctx, k8sClient, ownerInfo)).To(Succeed()) + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("spec").Child("service").Child("ownerInfoRef"), + "spec.metadataUrls.csw missing in "+ownerInfo.Name, + )))) + Expect(warnings).To(BeEmpty()) + + ownerInfo.Spec.MetadataUrls = nil + Expect(updateOwnerInfo(ctx, k8sClient, ownerInfo)).To(Succeed()) + warnings, err = validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("spec").Child("service").Child("ownerInfoRef"), + "spec.metadataUrls.csw missing in "+ownerInfo.Name, + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny create if the OwnerInfoRef misses WMS", func() { + ownerInfo.Spec.WMS = nil + Expect(updateOwnerInfo(ctx, k8sClient, ownerInfo)).To(Succeed()) + warnings, err := validator.ValidateCreate(ctx, obj) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("spec").Child("service").Child("ownerInfoRef"), + "spec.WMS missing in "+ownerInfo.Name, + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny update if a ingressRouteURL was removed", func() { + url, err := smoothoperatormodel.ParseURL("http://new.url/path") + Expect(err).To(BeNil()) + oldObj.Spec.IngressRouteURLs = []smoothoperatormodel.IngressRouteURL{ + {URL: obj.URL()}, + {URL: smoothoperatormodel.URL{URL: url}}, } - _, err := validator.ValidateUpdate(ctx, oldObj, obj) - Expect(err).To(HaveOccurred()) + obj.Spec.IngressRouteURLs = []smoothoperatormodel.IngressRouteURL{{URL: obj.URL()}} + warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("ingressRouteUrls"), + fmt.Sprint(obj.Spec.IngressRouteURLs), + fmt.Sprintf("urls cannot be removed, missing: %s", smoothoperatormodel.IngressRouteURL{URL: smoothoperatormodel.URL{URL: url}}), + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should accept update if a url was changed when it's in ingressRouteUrls", func() { + url, err := smoothoperatormodel.ParseURL("http://new.url/path") + Expect(err).To(BeNil()) + oldObj.Spec.IngressRouteURLs = []smoothoperatormodel.IngressRouteURL{ + {URL: obj.URL()}, + {URL: smoothoperatormodel.URL{URL: url}}, + } + obj.Spec.IngressRouteURLs = oldObj.Spec.IngressRouteURLs + oldObj.Spec.Service.URL = obj.URL() + obj.Spec.Service.URL = smoothoperatormodel.URL{URL: url} + + warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) + Expect(err).To(BeNil()) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny update if a url was changed and ingressRouteUrls = nil", func() { + url, err := smoothoperatormodel.ParseURL("http://new.url/path") + Expect(err).To(BeNil()) + obj.Spec.Service.URL = smoothoperatormodel.URL{URL: url} + obj.Spec.IngressRouteURLs = nil + oldObj.Spec.IngressRouteURLs = nil + + warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) + Expect(err).To(Equal(getValidationError(obj, field.Forbidden( + field.NewPath("spec").Child("service").Child("url"), + "is immutable", + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny update url was changed but not added to ingressRouteURLs", func() { + url, err := smoothoperatormodel.ParseURL("http://new.url/path") + Expect(err).ToNot(HaveOccurred()) + oldObj.Spec.IngressRouteURLs = nil + obj.Spec.IngressRouteURLs = []smoothoperatormodel.IngressRouteURL{{URL: oldObj.Spec.Service.URL}} + obj.Spec.Service.URL = smoothoperatormodel.URL{URL: url} + warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("ingressRouteUrls"), + fmt.Sprint(obj.Spec.IngressRouteURLs), + fmt.Sprintf("must contain baseURL: %s", obj.URL()), + )))) + Expect(warnings).To(BeEmpty()) + + obj.Spec.IngressRouteURLs = []smoothoperatormodel.IngressRouteURL{{URL: smoothoperatormodel.URL{URL: url}}} + warnings, err = validator.ValidateUpdate(ctx, oldObj, obj) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("spec").Child("ingressRouteUrls"), + fmt.Sprint(obj.Spec.IngressRouteURLs), + fmt.Sprintf("must contain baseURL: %s", oldObj.URL()), + )))) + Expect(warnings).To(BeEmpty()) + }) It("Should deny update if a label was removed", func() { + oldKey := "" for label := range obj.Labels { + oldKey = label delete(obj.Labels, label) break } - _, err := validator.ValidateUpdate(ctx, oldObj, obj) - Expect(err).To(HaveOccurred()) + warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) + Expect(err).To(Equal(getValidationError(obj, field.Required( + field.NewPath("metadata").Child("labels").Child(oldKey), + "labels cannot be removed", + )))) + Expect(warnings).To(BeEmpty()) + }) + + It("Should deny update if a label changed", func() { + oldKey := "" + oldValue := "" + newValue := "" + for label, val := range obj.Labels { + oldKey = label + oldValue = val + newValue = val + "-newval" + obj.Labels[label] = newValue + break + } + warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) + Expect(err).To(Equal(getValidationError(obj, field.Invalid( + field.NewPath("metadata").Child("labels").Child(oldKey), + newValue, + "immutable: should be: "+oldValue, + )))) + Expect(warnings).To(BeEmpty()) }) It("Should deny update if a label was added", func() { - obj.Labels["new-label"] = "test" - _, err := validator.ValidateUpdate(ctx, oldObj, obj) - Expect(err).To(HaveOccurred()) + newKey := "new-label" + obj.Labels[newKey] = "test" + warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) + Expect(err).To(Equal(getValidationError(obj, field.Forbidden( + field.NewPath("metadata").Child("labels").Child(newKey), + "new labels cannot be added", + )))) + Expect(warnings).To(BeEmpty()) }) It("Should deny update if an inspire block was added", func() { + obj.Spec.Service.Inspire = &pdoknlv3.Inspire{} oldObj.Spec.Service.Inspire = nil - _, err := validator.ValidateUpdate(ctx, oldObj, obj) - Expect(err).To(HaveOccurred()) + warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) + Expect(err).To(Equal(getValidationError(obj, field.Forbidden( + field.NewPath("spec").Child("service").Child("inspire"), + "cannot change from inspire to not inspire or the other way around", + )))) + Expect(warnings).To(BeEmpty()) }) It("Should deny update if an inspire block was removed", func() { + oldObj.Spec.Service.Inspire = &pdoknlv3.Inspire{} obj.Spec.Service.Inspire = nil - _, err := validator.ValidateUpdate(ctx, oldObj, obj) - Expect(err).To(HaveOccurred()) + warnings, err := validator.ValidateUpdate(ctx, oldObj, obj) + Expect(err).To(Equal(getValidationError(obj, field.Forbidden( + field.NewPath("spec").Child("service").Child("inspire"), + "cannot change from inspire to not inspire or the other way around", + )))) + Expect(warnings).To(BeEmpty()) }) - It("Should deny creation if there are no visible layers", func() { - obj.Spec.Service.Layer.Layers = nil - obj.Spec.Service.Layer.Visible = false - - _, err := validator.ValidateUpdate(ctx, oldObj, obj) - Expect(err).To(HaveOccurred()) - }) }) }) + +func withMapfile(wms *pdoknlv3.WMS) { + wms.Spec.Service.Mapfile = &pdoknlv3.Mapfile{} + wms.Spec.Service.Layer.Layers[0].Styles[0].Visualization = nil + wms.Spec.Service.Layer.Layers[1].Layers[0].Styles[0].Visualization = nil +}