Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: polishing tiles config + add extra linting (more golangci-linters + hadolint) #270

Merged
merged 8 commits into from
Jan 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/e2e-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ jobs:
end-to-end-test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4

# Build a local test image for (potential) re-use across end-to-end tests
- name: Set up Docker Buildx
Expand Down
21 changes: 21 additions & 0 deletions .github/workflows/lint-docker.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
name: lint (docker)
on:
push:
branches:
- master
pull_request:
jobs:
lint:
name: lint
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
sparse-checkout: |
Dockerfile
sparse-checkout-cone-mode: false

- uses: hadolint/hadolint-action@v3.1.0
with:
dockerfile: Dockerfile
2 changes: 1 addition & 1 deletion .github/workflows/lint-go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
go-version: '1.23'
cache: false

- uses: actions/checkout@v3
- uses: actions/checkout@v4

- name: Tidy
uses: katexochen/go-tidy-check@v2
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/lint-ts.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
# See supported Node.js release schedule at https://nodejs.org/en/about/releases/

steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v3
with:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test-go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4

- name: Setup cgo dependencies
run: sudo apt-get update && sudo apt-get install libcurl4-openssl-dev libssl-dev libsqlite3-mod-spatialite
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test-ts.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
node-version: [20.x]

steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4

- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v3
Expand Down
7 changes: 7 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ issues:
linters:
- bodyclose
- dupl
- dogsled
- funlen

output:
Expand Down Expand Up @@ -66,22 +67,27 @@ linters:
- cyclop # checks function and package cyclomatic complexity
- dupl # tool for code clone detection
- durationcheck # checks for two durations multiplied together
- dogsled # find assignments/declarations with too many blank identifiers
- errname # checks that sentinel errors are prefixed with the Err and error types are suffixed with the Error
- errorlint # finds code that will cause problems with the error wrapping scheme introduced in Go 1.13
- exhaustive # checks exhaustiveness of enum switch statements
- exptostd # detects functions from golang.org/x/exp/ that can be replaced by std functions
- copyloopvar # checks for pointers to enclosing loop variables
- fatcontext # detects nested contexts in loops and function literals
- forbidigo # forbids identifiers
- funlen # tool for detection of long functions
- gocheckcompilerdirectives # validates go compiler directive comments (//go:)
- goconst # finds repeated strings that could be replaced by a constant
- gocritic # provides diagnostics that check for bugs, performance and style issues
- gofmt # checks if the code is formatted according to 'gofmt' command
- goimports # in addition to fixing imports, goimports also formats your code in the same style as gofmt
- gomoddirectives # manages the use of 'replace', 'retract', and 'excludes' directives in go.mod
- gomodguard # allow and block lists linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations
- goprintffuncname # checks that printf-like functions are named with f at the end
- gosec # inspects source code for security problems
- loggercheck # checks key value pairs for common logger libraries (kitlog,klog,logr,zap)
- makezero # finds slice declarations with non-zero initial length
- mirror # reports wrong mirror patterns of bytes/strings usage
- misspell # finds commonly misspelled English words
- nakedret # finds naked returns in functions greater than a specified function length
- nestif # reports deeply nested if statements
Expand All @@ -96,6 +102,7 @@ linters:
- rowserrcheck # checks whether Err of rows is checked successfully
- sqlclosecheck # checks that sql.Rows and sql.Stmt are closed
- sloglint # A Go linter that ensures consistent code style when using log/slog
- tagliatelle # checks the struct tags.
- tenv # detects using os.Setenv instead of t.Setenv since Go1.17
- testableexamples # checks if examples are testable (have an expected output)
- tparallel # detects inappropriate usage of t.Parallel() method in your Go test codes
Expand Down
12 changes: 6 additions & 6 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ FROM docker.io/node:lts-alpine3.17 AS build-component
RUN mkdir -p /usr/src/app
COPY ./viewer /usr/src/app
WORKDIR /usr/src/app
RUN npm config set registry http://registry.npmjs.org
RUN npm install
RUN npm run build
RUN npm config set registry http://registry.npmjs.org && \
npm install && \
npm run build

####### Go build
FROM docker.io/golang:1.23-bookworm AS build-env
WORKDIR /go/src/service
ADD . /go/src/service
COPY . /go/src/service

# enable cgo in order to interface with sqlite
ENV CGO_ENABLED=1
Expand All @@ -19,7 +19,7 @@ ENV GOOS=linux
# install sqlite-related compile-time dependencies
RUN set -eux && \
apt-get update && \
apt-get install -y libcurl4-openssl-dev libssl-dev libsqlite3-mod-spatialite && \
apt-get install --no-install-recommends -y libcurl4-openssl-dev=* libssl-dev=* libsqlite3-mod-spatialite=* && \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Die hadolint is een mooie aanvulling, kunnen we voor meer projecten gebruiken.
Ik vind de "=*" alleen niet zo veel toevoegen. Misschien willen we rule DL3008 disabelen?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ja die regel uitzetten kan ook idd. Voor mij om het even. Opzich leek mij dit ook en heldere oplossing. Snap het idee om versions te pinnen maar we willen juist ook up to date zijn en ik verwacht ook niet snel breaking issues met curl/openssl/spatialite.

En idd +1 om vaker hadolint te gebruiken.

rm -rf /var/lib/apt/lists/*

# install controller-gen (used by go generate)
Expand All @@ -41,7 +41,7 @@ FROM docker.io/debian:bookworm-slim
# install sqlite-related runtime dependencies
RUN set -eux && \
apt-get update && \
apt-get install -y libcurl4 curl openssl libsqlite3-mod-spatialite && \
apt-get install --no-install-recommends -y libcurl4=* curl=* openssl=* ca-certificates=* libsqlite3-mod-spatialite=* && \
rm -rf /var/lib/apt/lists/*

EXPOSE 8080
Expand Down
5 changes: 3 additions & 2 deletions config/README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Config

This config package is used to validate and unmarshall a GoKoala YAML config file to structs.
This config package is used to validate and unmarshal a GoKoala YAML config file to Go structs.

In addition, this package is imported as a _library_ in the PDOK [OGCAPI operator](https://github.com/PDOK/ogcapi-operator)
to validate the `OGCAPI` Custom Resource (CR) in order to orchestrate GoKoala in Kubernetes.
to validate the `OGCAPI` Custom Resource (CR) in order to orchestrate GoKoala in Kubernetes.
For this reason the structs in the package are annotated with [Kubebuilder markers](https://book.kubebuilder.io/reference/markers).
26 changes: 3 additions & 23 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,36 +226,16 @@ func setDefaults(config *Config) error {
if len(config.AvailableLanguages) == 0 {
config.AvailableLanguages = append(config.AvailableLanguages, Language{language.Dutch}) // default to Dutch only
}
if config.OgcAPI.Tiles != nil && config.OgcAPI.Tiles.DatasetTiles != nil && config.OgcAPI.Tiles.DatasetTiles.HealthCheck.Srs == DefaultSrs &&
config.OgcAPI.Tiles.DatasetTiles.HealthCheck.TilePath == nil {
setHealthCheckTilePath(config.OgcAPI.Tiles.DatasetTiles)
} else if config.OgcAPI.Tiles != nil && config.OgcAPI.Tiles.Collections != nil {
for _, coll := range config.OgcAPI.Tiles.Collections {
if coll.Tiles.GeoDataTiles.HealthCheck.Srs == DefaultSrs && coll.Tiles.GeoDataTiles.HealthCheck.TilePath == nil {
setHealthCheckTilePath(&coll.Tiles.GeoDataTiles)
}
}
if config.OgcAPI.Tiles != nil {
config.OgcAPI.Tiles.Defaults()
}
return nil
}

func setHealthCheckTilePath(tilesConfig *Tiles) {
var deepestZoomLevel int
for _, srs := range tilesConfig.SupportedSrs {
if srs.Srs == DefaultSrs {
deepestZoomLevel = srs.ZoomLevelRange.End
}
}
defaultTile := HealthCheckDefaultTiles[deepestZoomLevel]
tileMatrixSet := AllTileProjections[DefaultSrs]
tilePath := fmt.Sprintf("/%s/%d/%d/%d.pbf", tileMatrixSet, deepestZoomLevel, defaultTile.x, defaultTile.y)
tilesConfig.HealthCheck.TilePath = &tilePath
}

func validate(config *Config) error {
// process 'validate' tags
v := validator.New()
err := RegisterAllValidators(v)
err := v.RegisterValidation(lowercaseID, LowercaseID)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func TestGeoSpatialCollection_Marshalling_JSON(t *testing.T) {
}

type TestEmbeddedGeoSpatialCollection struct {
C GeoSpatialCollection `json:"C"`
C GeoSpatialCollection `json:"c"`
}

func TestGeoSpatialCollection_Unmarshalling_JSON(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions config/duration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
)

type TestEmbeddedDuration struct {
D Duration `json:"D" yaml:"D"`
D Duration `json:"d" yaml:"d"`
}

func TestDuration_DeepCopy(t *testing.T) {
Expand Down Expand Up @@ -77,7 +77,7 @@ func TestDuration_Marshalling_JSON(t *testing.T) {

// non-pointer
unmarshalledEmbedded := &TestEmbeddedDuration{}
err = yaml.Unmarshal([]byte(`{"D": `+tt.want+`}`), unmarshalledEmbedded)
err = yaml.Unmarshal([]byte(`{"d": `+tt.want+`}`), unmarshalledEmbedded)
if !tt.wantErr(t, err, errors.New("yaml.Unmarshal")) {
return
}
Expand Down
4 changes: 2 additions & 2 deletions config/language_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

type TestEmbeddedLanguage struct {
L Language `json:"L" yaml:"L"`
L Language `json:"l" yaml:"l"`
}

func TestLanguage_DeepCopy(t *testing.T) {
Expand Down Expand Up @@ -78,7 +78,7 @@ func TestLanguage_Marshalling_JSON(t *testing.T) {

// non-pointer
unmarshalledEmbedded := &TestEmbeddedLanguage{}
err = yaml.Unmarshal([]byte(`{"L": `+tt.want+`}`), unmarshalledEmbedded)
err = yaml.Unmarshal([]byte(`{"l": `+tt.want+`}`), unmarshalledEmbedded)
if !tt.wantErr(t, err, errors.New("yaml.Unmarshal")) {
return
}
Expand Down
4 changes: 0 additions & 4 deletions config/validators.go → config/lowercase_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,6 @@ const (
lowercaseID = "lowercase_id"
)

func RegisterAllValidators(v *validator.Validate) error {
return v.RegisterValidation(lowercaseID, LowercaseID)
}

// LowercaseID is the validation function for validating if the current field
// is not empty and contains only lowercase chars, numbers, hyphens or underscores.
// It's similar to RFC 1035 DNS label but not the same.
Expand Down
4 changes: 2 additions & 2 deletions config/mediatype_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

type TestEmbeddedMediaType struct {
M MediaType `json:"M" yaml:"M"`
M MediaType `json:"m" yaml:"m"`
}

func TestMediaType_DeepCopy(t *testing.T) {
Expand Down Expand Up @@ -174,7 +174,7 @@ func TestMediaType_Unmarshalling_YAML(t *testing.T) {

// non-pointer
unmarshalledEmbedded := &TestEmbeddedMediaType{}
err = yaml.Unmarshal([]byte(`{"M": `+tt.mediaType+`}`), unmarshalledEmbedded)
err = yaml.Unmarshal([]byte(`{"m": `+tt.mediaType+`}`), unmarshalledEmbedded)
if !tt.wantErr(t, err, errors.New("yaml.Unmarshal")) {
return
}
Expand Down
2 changes: 1 addition & 1 deletion config/ogcapi_3dgeovolumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type CollectionEntry3dGeoVolumes struct {

// URI template for digital terrain model (DTM) in Quantized Mesh format, REQUIRED when you want to serve a DTM.
// +optional
URITemplateDTM *string `yaml:"uriTemplateDTM,omitempty" json:"uriTemplateDTM,omitempty" validate:"required_without_all=URITemplate3dTiles"`
URITemplateDTM *string `yaml:"uriTemplateDTM,omitempty" json:"uriTemplateDTM,omitempty" validate:"required_without_all=URITemplate3dTiles"` //nolint:tagliatelle // grandfathered

// Optional URL to 3D viewer to visualize the given collection of 3D Tiles.
// +optional
Expand Down
2 changes: 1 addition & 1 deletion config/ogcapi_features.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ type CollectionEntryFeatures struct {
type Datasources struct {
// Features should always be available in WGS84 (according to spec).
// This specifies the datasource to be used for features in the WGS84 projection
DefaultWGS84 Datasource `yaml:"defaultWGS84" json:"defaultWGS84" validate:"required"`
DefaultWGS84 Datasource `yaml:"defaultWGS84" json:"defaultWGS84" validate:"required"` //nolint:tagliatelle // grandfathered

// One or more additional datasources for features in other projections. GoKoala doesn't do
// any on-the-fly reprojection so additional datasources need to be reprojected ahead of time.
Expand Down
27 changes: 27 additions & 0 deletions config/ogcapi_tiles.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package config

import (
"fmt"
"slices"
"sort"

Expand All @@ -18,6 +19,19 @@ type OgcAPITiles struct {
Collections GeoSpatialCollections `yaml:"collections,omitempty" json:"collections,omitempty"`
}

func (o *OgcAPITiles) Defaults() {
if o.DatasetTiles != nil && o.DatasetTiles.HealthCheck.Srs == DefaultSrs &&
o.DatasetTiles.HealthCheck.TilePath == nil {
o.DatasetTiles.deriveHealthCheckTilePath()
} else if o.Collections != nil {
for _, coll := range o.Collections {
if coll.Tiles.GeoDataTiles.HealthCheck.Srs == DefaultSrs && coll.Tiles.GeoDataTiles.HealthCheck.TilePath == nil {
coll.Tiles.GeoDataTiles.deriveHealthCheckTilePath()
}
}
}
}

// +kubebuilder:object:generate=true
type CollectionEntryTiles struct {

Expand Down Expand Up @@ -112,6 +126,19 @@ type Tiles struct {
HealthCheck HealthCheck `yaml:"healthCheck" json:"healthCheck"`
}

func (t *Tiles) deriveHealthCheckTilePath() {
var deepestZoomLevel int
for _, srs := range t.SupportedSrs {
if srs.Srs == DefaultSrs {
deepestZoomLevel = srs.ZoomLevelRange.End
}
}
defaultTile := HealthCheckDefaultTiles[deepestZoomLevel]
tileMatrixSet := AllTileProjections[DefaultSrs]
tilePath := fmt.Sprintf("/%s/%d/%d/%d.pbf", tileMatrixSet, deepestZoomLevel, defaultTile.x, defaultTile.y)
t.HealthCheck.TilePath = &tilePath
}

// +kubebuilder:object:generate=true
type SupportedSrs struct {
// Projection (SRS/CRS) used
Expand Down
4 changes: 2 additions & 2 deletions config/url_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

type TestEmbeddedURL struct {
U URL `json:"U" yaml:"U"`
U URL `json:"u" yaml:"u"`
}

func TestURL_DeepCopy(t *testing.T) {
Expand Down Expand Up @@ -174,7 +174,7 @@ func TestURL_Unmarshalling_YAML(t *testing.T) {

// non-pointer
unmarshalledEmbedded := &TestEmbeddedURL{}
err = yaml.Unmarshal([]byte(`{"U": `+tt.url+`}`), unmarshalledEmbedded)
err = yaml.Unmarshal([]byte(`{"u": `+tt.url+`}`), unmarshalledEmbedded)
if !tt.wantErr(t, err, errors.New("yaml.Unmarshal")) {
return
}
Expand Down
17 changes: 4 additions & 13 deletions examples/config_3d.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
version: 1.0.0
title: New York in 3D
title: Example 3D
# shortened title, used in breadcrumb path
serviceIdentifier: 3D
abstract: >-
Expand All @@ -23,16 +23,7 @@ availableLanguages:
- en
ogcApi:
3dgeovolumes:
tileServer: https://maps.ecere.com/3DAPI/collections/
tileServer: https://api.pdok.nl/kadaster/3d-basisvoorziening/ogc/v1/collections
collections:
- id: newyork
# optional basepath to 3D tiles on the tileserver. Defaults to the collection ID.
tileServerPath: "NewYork/3DTiles"
# URI template for individual 3D tiles
uriTemplate3dTiles: "3DTiles/{level}/{x}/{y}.b3m"

# optional URI template for subtrees, only required when "implicit tiling" extension is used
# uriTemplateImplicitTilingSubtree: ""

# URI template for digital terrain model (DTM) in Quantized Mesh format, REQUIRED when you want to serve a DTM
# uriTemplateDTM: ""
- id: gebouwen
uriTemplate3dTiles: "t/{level}/{x}/{y}.glb"
Loading
Loading