From 84f1f5b69dcfe6100ab10906e13f8acb824ad804 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Thu, 5 Feb 2026 16:33:11 +0100 Subject: [PATCH 1/5] Add ORC API linter to enforce API design philosophy Add a custom golangci-lint plugin that flags OpenStack ID references in spec structs, enforcing ORC's API design philosophy that spec fields should only reference ORC Kubernetes objects, not OpenStack resources directly by UUID. The noopenstackidref linter flags fields like 'ProjectID *string' in spec/filter structs and suggests using '*KubernetesNameRef' with a 'Ref' suffix instead (e.g., 'ProjectRef *KubernetesNameRef'). Status structs are exempt, as they are expected to report OpenStack UUIDs. See: https://k-orc.cloud/development/architecture/#api-design-philosophy --- .golangci.yml | 1 + Makefile | 8 +- api/v1alpha1/zz_generated.domain-resource.go | 2 +- api/v1alpha1/zz_generated.flavor-resource.go | 2 +- .../zz_generated.floatingip-resource.go | 2 +- api/v1alpha1/zz_generated.group-resource.go | 2 +- api/v1alpha1/zz_generated.image-resource.go | 2 +- api/v1alpha1/zz_generated.keypair-resource.go | 2 +- api/v1alpha1/zz_generated.network-resource.go | 2 +- api/v1alpha1/zz_generated.port-resource.go | 2 +- api/v1alpha1/zz_generated.project-resource.go | 2 +- api/v1alpha1/zz_generated.role-resource.go | 2 +- api/v1alpha1/zz_generated.router-resource.go | 2 +- .../zz_generated.securitygroup-resource.go | 2 +- api/v1alpha1/zz_generated.server-resource.go | 2 +- .../zz_generated.servergroup-resource.go | 2 +- api/v1alpha1/zz_generated.service-resource.go | 2 +- api/v1alpha1/zz_generated.subnet-resource.go | 2 +- api/v1alpha1/zz_generated.trunk-resource.go | 2 +- api/v1alpha1/zz_generated.volume-resource.go | 2 +- .../zz_generated.volumetype-resource.go | 2 +- cmd/resource-generator/data/api.template | 4 +- tools/orc-api-linter/go.mod | 18 ++ tools/orc-api-linter/go.sum | 43 +++++ .../pkg/analysis/noopenstackidref/analyzer.go | 170 ++++++++++++++++++ .../noopenstackidref/analyzer_test.go | 28 +++ .../pkg/analysis/noopenstackidref/doc.go | 57 ++++++ .../noopenstackidref/testdata/src/a/a.go | 131 ++++++++++++++ tools/orc-api-linter/plugin.go | 40 +++++ website/docs/development/api-design.md | 1 + 30 files changed, 514 insertions(+), 25 deletions(-) create mode 100644 tools/orc-api-linter/go.mod create mode 100644 tools/orc-api-linter/go.sum create mode 100644 tools/orc-api-linter/pkg/analysis/noopenstackidref/analyzer.go create mode 100644 tools/orc-api-linter/pkg/analysis/noopenstackidref/analyzer_test.go create mode 100644 tools/orc-api-linter/pkg/analysis/noopenstackidref/doc.go create mode 100644 tools/orc-api-linter/pkg/analysis/noopenstackidref/testdata/src/a/a.go create mode 100644 tools/orc-api-linter/plugin.go diff --git a/.golangci.yml b/.golangci.yml index ac59808d0..0adca1fc8 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -62,6 +62,7 @@ linters: - statusoptional - statussubresource - uniquemarkers + - noopenstackidref lintersConfig: conditions: isFirstField: Warn diff --git a/Makefile b/Makefile index d1b7e775b..0f1cb5a12 100644 --- a/Makefile +++ b/Makefile @@ -139,7 +139,7 @@ lint: golangci-kal ## Run golangci-kal linter $(GOLANGCI_KAL) run .PHONY: lint-fix -lint-fix: golangci-kal ## Run golangci-lint linter and perform fixes +lint-fix: golangci-kal ## Run golangci-kal linter and perform fixes $(GOLANGCI_KAL) run --fix ##@ Build @@ -315,7 +315,6 @@ KUSTOMIZE_VERSION ?= v5.6.0 CONTROLLER_TOOLS_VERSION ?= v0.17.1 ENVTEST_VERSION ?= release-0.22 GOLANGCI_LINT_VERSION ?= v2.7.2 -KAL_VERSION ?= v0.0.0-20260205134631-d65d24a9df89 MOCKGEN_VERSION ?= v0.5.0 KUTTL_VERSION ?= v0.24.0 GOVULNCHECK_VERSION ?= v1.1.4 @@ -346,8 +345,8 @@ version: $(GOLANGCI_LINT_VERSION) name: golangci-kube-api-linter destination: $(LOCALBIN) plugins: -- module: 'sigs.k8s.io/kube-api-linter' - version: $(KAL_VERSION) +- module: 'github.com/k-orc/openstack-resource-controller/v2/tools/orc-api-linter' + path: ./tools/orc-api-linter endef export custom-gcl @@ -357,6 +356,7 @@ CUSTOM_GCL_FILE ?= $(shell pwd)/.custom-gcl.yml golangci-kal: $(GOLANGCI_KAL) $(GOLANGCI_KAL): $(LOCALBIN) $(GOLANGCI_LINT) $(file >$(CUSTOM_GCL_FILE),$(custom-gcl)) + cd tools/orc-api-linter && go mod tidy $(GOLANGCI_LINT) custom .PHONY: mockgen diff --git a/api/v1alpha1/zz_generated.domain-resource.go b/api/v1alpha1/zz_generated.domain-resource.go index 07505e07b..ae2e5fc4e 100644 --- a/api/v1alpha1/zz_generated.domain-resource.go +++ b/api/v1alpha1/zz_generated.domain-resource.go @@ -32,7 +32,7 @@ type DomainImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.flavor-resource.go b/api/v1alpha1/zz_generated.flavor-resource.go index c3ca4a8b6..6ae9d1fd8 100644 --- a/api/v1alpha1/zz_generated.flavor-resource.go +++ b/api/v1alpha1/zz_generated.flavor-resource.go @@ -32,7 +32,7 @@ type FlavorImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.floatingip-resource.go b/api/v1alpha1/zz_generated.floatingip-resource.go index 6d7501526..d502e9b65 100644 --- a/api/v1alpha1/zz_generated.floatingip-resource.go +++ b/api/v1alpha1/zz_generated.floatingip-resource.go @@ -32,7 +32,7 @@ type FloatingIPImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.group-resource.go b/api/v1alpha1/zz_generated.group-resource.go index 51e19eedd..653bea813 100644 --- a/api/v1alpha1/zz_generated.group-resource.go +++ b/api/v1alpha1/zz_generated.group-resource.go @@ -32,7 +32,7 @@ type GroupImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.image-resource.go b/api/v1alpha1/zz_generated.image-resource.go index a74b4cd38..e9a65eff8 100644 --- a/api/v1alpha1/zz_generated.image-resource.go +++ b/api/v1alpha1/zz_generated.image-resource.go @@ -32,7 +32,7 @@ type ImageImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.keypair-resource.go b/api/v1alpha1/zz_generated.keypair-resource.go index 4703a9f17..57d13fde6 100644 --- a/api/v1alpha1/zz_generated.keypair-resource.go +++ b/api/v1alpha1/zz_generated.keypair-resource.go @@ -32,7 +32,7 @@ type KeyPairImport struct { // The ORC object will enter an error state if the resource does not exist. // +kubebuilder:validation:MaxLength:=1024 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.network-resource.go b/api/v1alpha1/zz_generated.network-resource.go index 5e0248b92..bc60852dc 100644 --- a/api/v1alpha1/zz_generated.network-resource.go +++ b/api/v1alpha1/zz_generated.network-resource.go @@ -32,7 +32,7 @@ type NetworkImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.port-resource.go b/api/v1alpha1/zz_generated.port-resource.go index 5f707e564..8b1c25ca4 100644 --- a/api/v1alpha1/zz_generated.port-resource.go +++ b/api/v1alpha1/zz_generated.port-resource.go @@ -32,7 +32,7 @@ type PortImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.project-resource.go b/api/v1alpha1/zz_generated.project-resource.go index 473442d35..33fce32e2 100644 --- a/api/v1alpha1/zz_generated.project-resource.go +++ b/api/v1alpha1/zz_generated.project-resource.go @@ -32,7 +32,7 @@ type ProjectImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.role-resource.go b/api/v1alpha1/zz_generated.role-resource.go index 31a915d25..5891a418b 100644 --- a/api/v1alpha1/zz_generated.role-resource.go +++ b/api/v1alpha1/zz_generated.role-resource.go @@ -32,7 +32,7 @@ type RoleImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.router-resource.go b/api/v1alpha1/zz_generated.router-resource.go index 83b0681de..68d83bd53 100644 --- a/api/v1alpha1/zz_generated.router-resource.go +++ b/api/v1alpha1/zz_generated.router-resource.go @@ -32,7 +32,7 @@ type RouterImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.securitygroup-resource.go b/api/v1alpha1/zz_generated.securitygroup-resource.go index f378c1b29..ac0a921a6 100644 --- a/api/v1alpha1/zz_generated.securitygroup-resource.go +++ b/api/v1alpha1/zz_generated.securitygroup-resource.go @@ -32,7 +32,7 @@ type SecurityGroupImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.server-resource.go b/api/v1alpha1/zz_generated.server-resource.go index d00f9f934..011c5896d 100644 --- a/api/v1alpha1/zz_generated.server-resource.go +++ b/api/v1alpha1/zz_generated.server-resource.go @@ -32,7 +32,7 @@ type ServerImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.servergroup-resource.go b/api/v1alpha1/zz_generated.servergroup-resource.go index 88f875511..9f478e276 100644 --- a/api/v1alpha1/zz_generated.servergroup-resource.go +++ b/api/v1alpha1/zz_generated.servergroup-resource.go @@ -32,7 +32,7 @@ type ServerGroupImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.service-resource.go b/api/v1alpha1/zz_generated.service-resource.go index 1b9275360..0c0182818 100644 --- a/api/v1alpha1/zz_generated.service-resource.go +++ b/api/v1alpha1/zz_generated.service-resource.go @@ -32,7 +32,7 @@ type ServiceImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.subnet-resource.go b/api/v1alpha1/zz_generated.subnet-resource.go index c6fcc4090..0151f3064 100644 --- a/api/v1alpha1/zz_generated.subnet-resource.go +++ b/api/v1alpha1/zz_generated.subnet-resource.go @@ -32,7 +32,7 @@ type SubnetImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.trunk-resource.go b/api/v1alpha1/zz_generated.trunk-resource.go index 00aefdd24..eb4c5e844 100644 --- a/api/v1alpha1/zz_generated.trunk-resource.go +++ b/api/v1alpha1/zz_generated.trunk-resource.go @@ -32,7 +32,7 @@ type TrunkImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.volume-resource.go b/api/v1alpha1/zz_generated.volume-resource.go index 839df23da..9451474fb 100644 --- a/api/v1alpha1/zz_generated.volume-resource.go +++ b/api/v1alpha1/zz_generated.volume-resource.go @@ -32,7 +32,7 @@ type VolumeImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/api/v1alpha1/zz_generated.volumetype-resource.go b/api/v1alpha1/zz_generated.volumetype-resource.go index 4ae6b2a85..5f583a2bd 100644 --- a/api/v1alpha1/zz_generated.volumetype-resource.go +++ b/api/v1alpha1/zz_generated.volumetype-resource.go @@ -32,7 +32,7 @@ type VolumeTypeImport struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter // filter contains a resource query which is expected to return a single // result. The controller will continue to retry if filter returns no diff --git a/cmd/resource-generator/data/api.template b/cmd/resource-generator/data/api.template index ecdc787b2..9abb00d7a 100644 --- a/cmd/resource-generator/data/api.template +++ b/cmd/resource-generator/data/api.template @@ -32,7 +32,7 @@ type {{ .Name }}Import struct { // The ORC object will enter an error state if the resource does not exist. // +kubebuilder:validation:MaxLength:=1024 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter {{- else }} // id contains the unique identifier of an existing OpenStack resource. Note // that when specifying an import by ID, the resource MUST already exist. @@ -40,7 +40,7 @@ type {{ .Name }}Import struct { // +kubebuilder:validation:Format:=uuid // +kubebuilder:validation:MaxLength:=36 // +optional - ID *string `json:"id,omitempty"` + ID *string `json:"id,omitempty"` //nolint:kubeapilinter {{- end }} // filter contains a resource query which is expected to return a single diff --git a/tools/orc-api-linter/go.mod b/tools/orc-api-linter/go.mod new file mode 100644 index 000000000..06dc79143 --- /dev/null +++ b/tools/orc-api-linter/go.mod @@ -0,0 +1,18 @@ +module github.com/k-orc/openstack-resource-controller/v2/tools/orc-api-linter + +go 1.24.0 + +require ( + golang.org/x/tools v0.41.0 + sigs.k8s.io/kube-api-linter v0.0.0-20260205134631-d65d24a9df89 +) + +require ( + github.com/golangci/plugin-module-register v0.1.2 // indirect + golang.org/x/mod v0.32.0 // indirect + golang.org/x/sync v0.19.0 // indirect + k8s.io/apimachinery v0.32.3 // indirect + k8s.io/gengo/v2 v2.0.0-20250922181213-ec3ebc5fd46b // indirect + k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 // indirect + sigs.k8s.io/yaml v1.4.0 // indirect +) diff --git a/tools/orc-api-linter/go.sum b/tools/orc-api-linter/go.sum new file mode 100644 index 000000000..fce1ec281 --- /dev/null +++ b/tools/orc-api-linter/go.sum @@ -0,0 +1,43 @@ +github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= +github.com/go-logr/logr v1.4.3/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= +github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= +github.com/golangci/plugin-module-register v0.1.2 h1:e5WM6PO6NIAEcij3B053CohVp3HIYbzSuP53UAYgOpg= +github.com/golangci/plugin-module-register v0.1.2/go.mod h1:1+QGTsKBvAIvPvoY/os+G5eoqxWn70HYDm2uvUyGuVw= +github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= +github.com/google/pprof v0.0.0-20250607225305-033d6d78b36a h1://KbezygeMJZCSHH+HgUZiTeSoiuFspbMg1ge+eFj18= +github.com/google/pprof v0.0.0-20250607225305-033d6d78b36a/go.mod h1:5hDyRhoBCxViHszMt12TnOpEI4VVi+U8Gm9iphldiMA= +github.com/onsi/ginkgo/v2 v2.23.4 h1:ktYTpKJAVZnDT4VjxSbiBenUjmlL/5QkBEocaWXiQus= +github.com/onsi/ginkgo/v2 v2.23.4/go.mod h1:Bt66ApGPBFzHyR+JO10Zbt0Gsp4uWxu5mIOTusL46e8= +github.com/onsi/gomega v1.38.0 h1:c/WX+w8SLAinvuKKQFh77WEucCnPk4j2OTUr7lt7BeY= +github.com/onsi/gomega v1.38.0/go.mod h1:OcXcwId0b9QsE7Y49u+BTrL4IdKOBOKnD6VQNTJEB6o= +go.uber.org/automaxprocs v1.6.0 h1:O3y2/QNTOdbF+e/dpXNNW7Rx2hZ4sTIPyybbxyNqTUs= +go.uber.org/automaxprocs v1.6.0/go.mod h1:ifeIMSnPZuznNm6jmdzmU3/bfk01Fe2fotchwEFJ8r8= +golang.org/x/mod v0.32.0 h1:9F4d3PHLljb6x//jOyokMv3eX+YDeepZSEo3mFJy93c= +golang.org/x/mod v0.32.0/go.mod h1:SgipZ/3h2Ci89DlEtEXWUk/HteuRin+HHhN+WbNhguU= +golang.org/x/net v0.49.0 h1:eeHFmOGUTtaaPSGNmjBKpbng9MulQsJURQUAfUwY++o= +golang.org/x/net v0.49.0/go.mod h1:/ysNB2EvaqvesRkuLAyjI1ycPZlQHM3q01F02UY/MV8= +golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= +golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= +golang.org/x/sys v0.40.0 h1:DBZZqJ2Rkml6QMQsZywtnjnnGvHza6BTfYFWY9kjEWQ= +golang.org/x/sys v0.40.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/text v0.29.0 h1:1neNs90w9YzJ9BocxfsQNHKuAT4pkghyXc4nhZ6sJvk= +golang.org/x/text v0.29.0/go.mod h1:7MhJOA9CD2qZyOKYazxdYMF85OwPdEr9jTtBpO7ydH4= +golang.org/x/tools v0.41.0 h1:a9b8iMweWG+S0OBnlU36rzLp20z1Rp10w+IY2czHTQc= +golang.org/x/tools v0.41.0/go.mod h1:XSY6eDqxVNiYgezAVqqCeihT4j1U2CCsqvH3WhQpnlg= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +k8s.io/apimachinery v0.32.3 h1:JmDuDarhDmA/Li7j3aPrwhpNBA94Nvk5zLeOge9HH1U= +k8s.io/apimachinery v0.32.3/go.mod h1:GpHVgxoKlTxClKcteaeuF1Ul/lDVb74KpZcxcmLDElE= +k8s.io/gengo/v2 v2.0.0-20250922181213-ec3ebc5fd46b h1:gMplByicHV/TJBizHd9aVEsTYoJBnnUAT5MHlTkbjhQ= +k8s.io/gengo/v2 v2.0.0-20250922181213-ec3ebc5fd46b/go.mod h1:CgujABENc3KuTrcsdpGmrrASjtQsWCT7R99mEV4U/fM= +k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 h1:M3sRQVHv7vB20Xc2ybTt7ODCeFj6JSWYFzOFnYeS6Ro= +k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= +sigs.k8s.io/kube-api-linter v0.0.0-20260205134631-d65d24a9df89 h1:QuWBEzbBkQyuwWPKDEaUBGr8QdHilkc4CdJYCeU1SIo= +sigs.k8s.io/kube-api-linter v0.0.0-20260205134631-d65d24a9df89/go.mod h1:5mP60UakkCye+eOcZ5p98VnV2O49qreW1gq9TdsUf7Q= +sigs.k8s.io/yaml v1.4.0 h1:Mk1wCc2gy/F0THH0TAp1QYyJNzRm2KCLy3o5ASXVI5E= +sigs.k8s.io/yaml v1.4.0/go.mod h1:Ejl7/uTz7PSA4eKMyQCUTnhZYNmLIl+5c2lQPGR2BPY= diff --git a/tools/orc-api-linter/pkg/analysis/noopenstackidref/analyzer.go b/tools/orc-api-linter/pkg/analysis/noopenstackidref/analyzer.go new file mode 100644 index 000000000..7d920611b --- /dev/null +++ b/tools/orc-api-linter/pkg/analysis/noopenstackidref/analyzer.go @@ -0,0 +1,170 @@ +/* +Copyright The ORC Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package noopenstackidref + +import ( + "go/ast" + "regexp" + "strings" + + "golang.org/x/tools/go/analysis" + "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags" + "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector" + "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers" + "sigs.k8s.io/kube-api-linter/pkg/analysis/initializer" + "sigs.k8s.io/kube-api-linter/pkg/analysis/registry" +) + +const ( + name = "noopenstackidref" + doc = `Flags OpenStack ID references in spec structs. + +ORC's API design philosophy states that spec fields should only reference +ORC Kubernetes objects, not OpenStack resources directly by UUID. + +Fields ending with 'ID' (like ProjectID, NetworkID) in spec structs should +instead use KubernetesNameRef type with a 'Ref' suffix (like ProjectRef, NetworkRef). + +See: https://k-orc.cloud/development/api-design/` +) + +// openstackIDPattern matches field names that end with "ID" and are likely +// references to OpenStack resources by UUID. These should instead use +// KubernetesNameRef with a "Ref" suffix to reference ORC objects. +var openstackIDPattern = regexp.MustCompile(`ID$`) + +// excludedIDPatterns contains field name patterns that end in "ID" but are +// not OpenStack resource references. +var excludedIDPatterns = []string{ + "SegmentationID", // VLAN segmentation ID, not an OpenStack resource +} + +// excludedStructs contains struct names that should not be checked even though +// they don't have "Status" in their name. These are typically nested types used +// exclusively within status structs. +var excludedStructs = []string{ + "ServerInterfaceFixedIP", // Used only in ServerInterfaceStatus.FixedIPs +} + +// Analyzer is the analyzer for the noopenstackidref linter. +var Analyzer = &analysis.Analyzer{ + Name: name, + Doc: doc, + Run: run, + Requires: []*analysis.Analyzer{inspector.Analyzer}, +} + +func init() { + registry.DefaultRegistry().RegisterLinter(initializer.NewInitializer( + name, + Analyzer, + false, // not enabled by default - must be explicitly enabled + )) +} + +func run(pass *analysis.Pass) (any, error) { + inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector) + if !ok { + return nil, nil + } + + inspect.InspectFieldsIncludingListTypes(func(field *ast.Field, _ extractjsontags.FieldTagInfo, _ markers.Markers, qualifiedFieldName string) { + checkField(pass, field, qualifiedFieldName) + }) + + return nil, nil +} + +func checkField(pass *analysis.Pass, field *ast.Field, qualifiedFieldName string) { + // qualifiedFieldName is in the form "StructName.FieldName" + parts := strings.SplitN(qualifiedFieldName, ".", 2) + if len(parts) != 2 { + return + } + + structName := parts[0] + fieldName := parts[1] + + // Only check spec-related structs, not status structs + if !isSpecStruct(structName) { + return + } + + // Check if field name matches OpenStack ID pattern + if !openstackIDPattern.MatchString(fieldName) { + return + } + + // Check if field name is in the exclusion list + for _, excluded := range excludedIDPatterns { + if fieldName == excluded { + return + } + } + + // Allow *KubernetesNameRef type (correct type, even if name ends in ID) + if isKubernetesNameRefType(field.Type) { + return + } + + suggestedRef := strings.TrimSuffix(fieldName, "ID") + "Ref" + pass.Reportf(field.Pos(), + "field %s references OpenStack resource by ID in spec; "+ + "use *KubernetesNameRef with %s instead; "+ + "see https://k-orc.cloud/development/api-design/", + qualifiedFieldName, suggestedRef) +} + +// isSpecStruct returns true if the struct name indicates it's a spec-related struct +// (where OpenStack ID references should be flagged), not a status struct +// (where OpenStack IDs are expected and valid). +func isSpecStruct(structName string) bool { + // Status structs are allowed to have OpenStack IDs + if strings.HasSuffix(structName, "Status") || + strings.Contains(structName, "Status") { + return false + } + + // Check excluded structs (nested types used only in status contexts) + for _, excluded := range excludedStructs { + if structName == excluded { + return false + } + } + + // All other structs should use KubernetesNameRef for references + return true +} + +// isKubernetesNameRefType checks if the expression is KubernetesNameRef or *KubernetesNameRef. +// This is the only acceptable type for fields that might look like ID references. +func isKubernetesNameRefType(expr ast.Expr) bool { + // Check for *KubernetesNameRef + if starExpr, ok := expr.(*ast.StarExpr); ok { + if ident, ok := starExpr.X.(*ast.Ident); ok { + return ident.Name == "KubernetesNameRef" + } + return false + } + + // Check for KubernetesNameRef (non-pointer) + if ident, ok := expr.(*ast.Ident); ok { + return ident.Name == "KubernetesNameRef" + } + + return false +} diff --git a/tools/orc-api-linter/pkg/analysis/noopenstackidref/analyzer_test.go b/tools/orc-api-linter/pkg/analysis/noopenstackidref/analyzer_test.go new file mode 100644 index 000000000..ba8ee5e79 --- /dev/null +++ b/tools/orc-api-linter/pkg/analysis/noopenstackidref/analyzer_test.go @@ -0,0 +1,28 @@ +/* +Copyright The ORC Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package noopenstackidref + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestAnalyzer(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, Analyzer, "a") +} diff --git a/tools/orc-api-linter/pkg/analysis/noopenstackidref/doc.go b/tools/orc-api-linter/pkg/analysis/noopenstackidref/doc.go new file mode 100644 index 000000000..6e0938616 --- /dev/null +++ b/tools/orc-api-linter/pkg/analysis/noopenstackidref/doc.go @@ -0,0 +1,57 @@ +/* +Copyright The ORC Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package noopenstackidref provides a linter that enforces ORC's API design +// philosophy of referencing ORC Kubernetes objects rather than OpenStack +// resources directly by UUID. +// +// # Overview +// +// ORC (OpenStack Resource Controller) manages OpenStack resources through +// Kubernetes custom resources. The API design philosophy states that spec +// fields should only reference other ORC objects, not OpenStack resources +// directly by UUID. +// +// # What this linter checks +// +// The linter flags fields in spec-related structs that: +// - Have names matching OpenStack resource ID patterns (e.g., ProjectID, NetworkID) +// - Are of type *string +// +// These should instead use *KubernetesNameRef with a 'Ref' suffix. +// +// # Examples +// +// Bad (will be flagged): +// +// type UserResourceSpec struct { +// DefaultProjectID *string `json:"defaultProjectID,omitempty"` +// } +// +// Good (correct pattern): +// +// type UserResourceSpec struct { +// DefaultProjectRef *KubernetesNameRef `json:"defaultProjectRef,omitempty"` +// } +// +// # Status structs are exempt +// +// Fields in status structs (ending with 'Status' or 'ResourceStatus') are +// allowed to have OpenStack IDs, as they report what OpenStack returned. +// +// See https://k-orc.cloud/development/architecture/#api-design-philosophy +// for more details on ORC's API design philosophy. +package noopenstackidref diff --git a/tools/orc-api-linter/pkg/analysis/noopenstackidref/testdata/src/a/a.go b/tools/orc-api-linter/pkg/analysis/noopenstackidref/testdata/src/a/a.go new file mode 100644 index 000000000..501001210 --- /dev/null +++ b/tools/orc-api-linter/pkg/analysis/noopenstackidref/testdata/src/a/a.go @@ -0,0 +1,131 @@ +/* +Copyright The ORC Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package a + +// KubernetesNameRef is a reference to a Kubernetes object by name. +type KubernetesNameRef string + +// HostID is a custom struct type (simulating ORC's HostID pattern). +// The bare ID field inside is flagged and requires a nolint comment if intentional. +type HostID struct { + ID string `json:"id,omitempty"` // want `field HostID.ID references OpenStack resource by ID in spec` + ServerRef KubernetesNameRef `json:"serverRef,omitempty"` +} + +// ---- Spec structs: OpenStack IDs should be flagged ---- + +// UserResourceSpec is a spec struct that should be checked. +type UserResourceSpec struct { + // Name is fine, not an OpenStack ID reference. + Name *string `json:"name,omitempty"` + + DefaultProjectID *string `json:"defaultProjectID,omitempty"` // want `field UserResourceSpec.DefaultProjectID references OpenStack resource by ID in spec` + + // DomainRef is good - uses KubernetesNameRef. + DomainRef *KubernetesNameRef `json:"domainRef,omitempty"` +} + +// PortResourceSpec has multiple violations. +type PortResourceSpec struct { + NetworkID *string `json:"networkID,omitempty"` // want `field PortResourceSpec.NetworkID references OpenStack resource by ID in spec` + + SubnetID *string `json:"subnetID,omitempty"` // want `field PortResourceSpec.SubnetID references OpenStack resource by ID in spec` + + // ProjectRef is correct. + ProjectRef *KubernetesNameRef `json:"projectRef,omitempty"` +} + +// ServerSpec tests shortened Spec suffix. +type ServerSpec struct { + ImageID *string `json:"imageID,omitempty"` // want `field ServerSpec.ImageID references OpenStack resource by ID in spec` + + FlavorID *string `json:"flavorID,omitempty"` // want `field ServerSpec.FlavorID references OpenStack resource by ID in spec` +} + +// ---- Filter structs: OpenStack IDs should also be flagged ---- + +// NetworkFilter is a filter struct that should be checked. +type NetworkFilter struct { + ProjectID *string `json:"projectID,omitempty"` // want `field NetworkFilter.ProjectID references OpenStack resource by ID in spec` + + // Name is fine. + Name *string `json:"name,omitempty"` +} + +// ---- Status structs: OpenStack IDs are allowed ---- + +// UserResourceStatus is a status struct where OpenStack IDs are expected. +type UserResourceStatus struct { + // DefaultProjectID is allowed in status - it reports what OpenStack returned. + DefaultProjectID string `json:"defaultProjectID,omitempty"` + + // DomainID is allowed in status. + DomainID string `json:"domainID,omitempty"` +} + +// PortStatus tests shortened Status suffix. +type PortStatus struct { + // NetworkID is allowed in status. + NetworkID string `json:"networkID,omitempty"` +} + +// ---- Nested types used in specs: should also be flagged ---- + +// ServerBlockDevice is a nested type used in ServerResourceSpec. +type ServerBlockDevice struct { + VolumeID *string `json:"volumeID,omitempty"` // want `field ServerBlockDevice.VolumeID references OpenStack resource by ID in spec` + + // Device is fine. + Device *string `json:"device,omitempty"` +} + +// SecurityGroupRule is a nested type. +type SecurityGroupRule struct { + RemoteGroupID *string `json:"remoteGroupID,omitempty"` // want `field SecurityGroupRule.RemoteGroupID references OpenStack resource by ID in spec` +} + +// ---- Edge cases ---- + +// NonPointerIDSpec has non-pointer ID fields which should also be flagged. +type NonPointerIDSpec struct { + ProjectID string `json:"projectID,omitempty"` // want `field NonPointerIDSpec.ProjectID references OpenStack resource by ID in spec` +} + +// UnrelatedIDStruct has ID fields that don't look like OpenStack resources, +// but they are still flagged because any *ID pattern could be a reference. +// Users should add //nolint:noopenstackidref if these are intentional. +type UnrelatedIDStruct struct { + ExternalID *string `json:"externalID,omitempty"` // want `field UnrelatedIDStruct.ExternalID references OpenStack resource by ID in spec` +} + +// StructTypeIDSpec tests that struct-typed ID fields are also flagged. +type StructTypeIDSpec struct { + HostID *HostID `json:"hostID,omitempty"` // want `field StructTypeIDSpec.HostID references OpenStack resource by ID in spec` +} + +// BareIDSpec tests that bare "ID" field is also flagged. +// Use //nolint:noopenstackidref for legitimate cases like spec.import.id. +type BareIDSpec struct { + ID *string `json:"id,omitempty"` // want `field BareIDSpec.ID references OpenStack resource by ID in spec` +} + +// WrongNameCorrectTypeSpec tests that *KubernetesNameRef with wrong name is allowed. +// This is acceptable because the type is correct even if naming is unconventional. +type WrongNameCorrectTypeSpec struct { + // ProjectID with *KubernetesNameRef type is allowed (type takes precedence). + ProjectID *KubernetesNameRef `json:"projectID,omitempty"` +} diff --git a/tools/orc-api-linter/plugin.go b/tools/orc-api-linter/plugin.go new file mode 100644 index 000000000..d7d0e22bb --- /dev/null +++ b/tools/orc-api-linter/plugin.go @@ -0,0 +1,40 @@ +/* +Copyright The ORC Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package orcapilinter is a golangci-lint plugin that extends kube-api-linter +// with ORC-specific API design rules. +// +// It imports the base kube-api-linter plugin and registers additional ORC-specific +// linters with the registry. This allows all linters to be configured through +// the kubeapilinter section in .golangci.yml. +package orcapilinter + +import ( + pluginbase "sigs.k8s.io/kube-api-linter/pkg/plugin/base" + + // Import the default kube-api-linter linters. + _ "sigs.k8s.io/kube-api-linter/pkg/registration" + + // Import ORC-specific linters to register them with the registry. + _ "github.com/k-orc/openstack-resource-controller/v2/tools/orc-api-linter/pkg/analysis/noopenstackidref" +) + +// New is the entrypoint for the plugin. +// We use the base kube-api-linter plugin which will include both the standard +// KAL linters and our ORC-specific linters registered via init(). +// +//nolint:gochecknoglobals +var New = pluginbase.New diff --git a/website/docs/development/api-design.md b/website/docs/development/api-design.md index 2fdd30b2b..5f867fb67 100644 --- a/website/docs/development/api-design.md +++ b/website/docs/development/api-design.md @@ -83,3 +83,4 @@ You should update `examples/components/kustomizeconfig/kustomizeconfig.yaml` wit * Do not use unsigned integers: use `intN` with a kubebuilder marker validating for a minimum of 0. * Optional fields should have the `omitempty` tag. * Optional fields should be pointers, unless their zero-value is also the OpenStack default, or we can be very confident that we will never need to distinguish between empty and unset values. e.g. Will we ever want to set a value explicitly to the empty string? +* ResourceSpec and Filter fields must reference other ORC objects using `*KubernetesNameRef` with a `Ref` suffix (e.g., `ProjectRef`), not OpenStack resources directly by UUID (e.g., `ProjectID *string`). Exceptions include bare `ID` fields (used for `spec.import.id`) and non-resource IDs like `SegmentationID`. From dffe27e921706dba5a34454a379a205dd06142b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Fri, 6 Feb 2026 10:37:20 +0100 Subject: [PATCH 2/5] Extend noopenstackidref linter to flag plural *IDs fields Fields like NetworkIDs, SubnetIDs should use NetworkRefs, SubnetRefs instead. This ensures consistency with the singular form (NetworkID -> NetworkRef). --- .../pkg/analysis/noopenstackidref/analyzer.go | 24 ++++++++++++------- .../noopenstackidref/testdata/src/a/a.go | 23 ++++++++++++++++++ 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/tools/orc-api-linter/pkg/analysis/noopenstackidref/analyzer.go b/tools/orc-api-linter/pkg/analysis/noopenstackidref/analyzer.go index 7d920611b..f46a18d41 100644 --- a/tools/orc-api-linter/pkg/analysis/noopenstackidref/analyzer.go +++ b/tools/orc-api-linter/pkg/analysis/noopenstackidref/analyzer.go @@ -36,21 +36,22 @@ const ( ORC's API design philosophy states that spec fields should only reference ORC Kubernetes objects, not OpenStack resources directly by UUID. -Fields ending with 'ID' (like ProjectID, NetworkID) in spec structs should -instead use KubernetesNameRef type with a 'Ref' suffix (like ProjectRef, NetworkRef). +Fields ending with 'ID' or 'IDs' (like ProjectID, NetworkIDs) in spec structs should +instead use KubernetesNameRef type with a 'Ref' or 'Refs' suffix (like ProjectRef, NetworkRefs). See: https://k-orc.cloud/development/api-design/` ) -// openstackIDPattern matches field names that end with "ID" and are likely +// openstackIDPattern matches field names that end with "ID" or "IDs" and are likely // references to OpenStack resources by UUID. These should instead use -// KubernetesNameRef with a "Ref" suffix to reference ORC objects. -var openstackIDPattern = regexp.MustCompile(`ID$`) +// KubernetesNameRef with a "Ref" or "Refs" suffix to reference ORC objects. +var openstackIDPattern = regexp.MustCompile(`IDs?$`) -// excludedIDPatterns contains field name patterns that end in "ID" but are +// excludedIDPatterns contains field name patterns that end in "ID" or "IDs" but are // not OpenStack resource references. var excludedIDPatterns = []string{ - "SegmentationID", // VLAN segmentation ID, not an OpenStack resource + "SegmentationID", // VLAN segmentation ID, not an OpenStack resource + "SegmentationIDs", // Plural form of the above } // excludedStructs contains struct names that should not be checked even though @@ -121,7 +122,14 @@ func checkField(pass *analysis.Pass, field *ast.Field, qualifiedFieldName string return } - suggestedRef := strings.TrimSuffix(fieldName, "ID") + "Ref" + // Generate the suggested Ref/Refs name based on singular/plural + var suggestedRef string + if strings.HasSuffix(fieldName, "IDs") { + suggestedRef = strings.TrimSuffix(fieldName, "IDs") + "Refs" + } else { + suggestedRef = strings.TrimSuffix(fieldName, "ID") + "Ref" + } + pass.Reportf(field.Pos(), "field %s references OpenStack resource by ID in spec; "+ "use *KubernetesNameRef with %s instead; "+ diff --git a/tools/orc-api-linter/pkg/analysis/noopenstackidref/testdata/src/a/a.go b/tools/orc-api-linter/pkg/analysis/noopenstackidref/testdata/src/a/a.go index 501001210..162b4d4fb 100644 --- a/tools/orc-api-linter/pkg/analysis/noopenstackidref/testdata/src/a/a.go +++ b/tools/orc-api-linter/pkg/analysis/noopenstackidref/testdata/src/a/a.go @@ -129,3 +129,26 @@ type WrongNameCorrectTypeSpec struct { // ProjectID with *KubernetesNameRef type is allowed (type takes precedence). ProjectID *KubernetesNameRef `json:"projectID,omitempty"` } + +// ---- Plural ID fields: should also be flagged ---- + +// PluralIDsSpec tests that plural IDs fields are flagged. +type PluralIDsSpec struct { + NetworkIDs []string `json:"networkIDs,omitempty"` // want `field PluralIDsSpec.NetworkIDs references OpenStack resource by ID in spec` + + SubnetIDs []string `json:"subnetIDs,omitempty"` // want `field PluralIDsSpec.SubnetIDs references OpenStack resource by ID in spec` + + // SecurityGroupRefs is correct - uses the Refs suffix. + SecurityGroupRefs []KubernetesNameRef `json:"securityGroupRefs,omitempty"` +} + +// PluralIDsStatus tests that plural IDs in status are allowed. +type PluralIDsStatus struct { + // NetworkIDs is allowed in status. + NetworkIDs []string `json:"networkIDs,omitempty"` +} + +// SegmentationIDsSpec tests that SegmentationIDs is excluded (not an OpenStack resource). +type SegmentationIDsSpec struct { + SegmentationIDs []int `json:"segmentationIDs,omitempty"` +} From 364f4f2682677ba8dcdfeb6c22fc230db4aac56a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Fri, 6 Feb 2026 10:48:53 +0100 Subject: [PATCH 3/5] Flag *Ref/*Refs fields that don't use KubernetesNameRef type Fields ending in Ref or Refs should use KubernetesNameRef type to reference ORC objects. This catches cases where the naming convention is correct but the type is wrong (e.g., SecurityGroupRefs []OpenStackName should be []KubernetesNameRef). CloudCredentialsRef is excluded as it intentionally uses a different type. --- .../pkg/analysis/noopenstackidref/analyzer.go | 44 +++++++++++++++++++ .../noopenstackidref/testdata/src/a/a.go | 38 ++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/tools/orc-api-linter/pkg/analysis/noopenstackidref/analyzer.go b/tools/orc-api-linter/pkg/analysis/noopenstackidref/analyzer.go index f46a18d41..adb507486 100644 --- a/tools/orc-api-linter/pkg/analysis/noopenstackidref/analyzer.go +++ b/tools/orc-api-linter/pkg/analysis/noopenstackidref/analyzer.go @@ -39,6 +39,9 @@ ORC Kubernetes objects, not OpenStack resources directly by UUID. Fields ending with 'ID' or 'IDs' (like ProjectID, NetworkIDs) in spec structs should instead use KubernetesNameRef type with a 'Ref' or 'Refs' suffix (like ProjectRef, NetworkRefs). +Additionally, fields ending with 'Ref' or 'Refs' must use the KubernetesNameRef type, +not other types like OpenStackName or string. + See: https://k-orc.cloud/development/api-design/` ) @@ -47,6 +50,10 @@ See: https://k-orc.cloud/development/api-design/` // KubernetesNameRef with a "Ref" or "Refs" suffix to reference ORC objects. var openstackIDPattern = regexp.MustCompile(`IDs?$`) +// refPattern matches field names that end with "Ref" or "Refs". +// These fields should use KubernetesNameRef type. +var refPattern = regexp.MustCompile(`Refs?$`) + // excludedIDPatterns contains field name patterns that end in "ID" or "IDs" but are // not OpenStack resource references. var excludedIDPatterns = []string{ @@ -54,6 +61,12 @@ var excludedIDPatterns = []string{ "SegmentationIDs", // Plural form of the above } +// excludedRefPatterns contains field name patterns that end in "Ref" or "Refs" but +// intentionally use a different type than KubernetesNameRef. +var excludedRefPatterns = []string{ + "CloudCredentialsRef", // References a credentials secret, not an ORC object +} + // excludedStructs contains struct names that should not be checked even though // they don't have "Status" in their name. These are typically nested types used // exclusively within status structs. @@ -105,6 +118,24 @@ func checkField(pass *analysis.Pass, field *ast.Field, qualifiedFieldName string return } + // Check if field name ends in Ref/Refs but uses wrong type + if refPattern.MatchString(fieldName) { + // Check if field name is in the Ref exclusion list + for _, excluded := range excludedRefPatterns { + if fieldName == excluded { + return + } + } + + if !isKubernetesNameRefTypeOrSlice(field.Type) { + pass.Reportf(field.Pos(), + "field %s has Ref suffix but does not use KubernetesNameRef type; "+ + "see https://k-orc.cloud/development/api-design/", + qualifiedFieldName) + } + return + } + // Check if field name matches OpenStack ID pattern if !openstackIDPattern.MatchString(fieldName) { return @@ -176,3 +207,16 @@ func isKubernetesNameRefType(expr ast.Expr) bool { return false } + +// isKubernetesNameRefTypeOrSlice checks if the expression is KubernetesNameRef, +// *KubernetesNameRef, or []KubernetesNameRef. This is used for Ref/Refs fields +// which may be singular or plural. +func isKubernetesNameRefTypeOrSlice(expr ast.Expr) bool { + // Check for []KubernetesNameRef + if arrayType, ok := expr.(*ast.ArrayType); ok { + return isKubernetesNameRefType(arrayType.Elt) + } + + // Check for KubernetesNameRef or *KubernetesNameRef + return isKubernetesNameRefType(expr) +} diff --git a/tools/orc-api-linter/pkg/analysis/noopenstackidref/testdata/src/a/a.go b/tools/orc-api-linter/pkg/analysis/noopenstackidref/testdata/src/a/a.go index 162b4d4fb..a81778c60 100644 --- a/tools/orc-api-linter/pkg/analysis/noopenstackidref/testdata/src/a/a.go +++ b/tools/orc-api-linter/pkg/analysis/noopenstackidref/testdata/src/a/a.go @@ -152,3 +152,41 @@ type PluralIDsStatus struct { type SegmentationIDsSpec struct { SegmentationIDs []int `json:"segmentationIDs,omitempty"` } + +// ---- Ref/Refs fields with wrong type: should be flagged ---- + +// OpenStackName simulates the ORC OpenStackName type (wrong type for Refs). +type OpenStackName string + +// WrongTypeRefSpec tests that Ref fields with wrong type are flagged. +type WrongTypeRefSpec struct { + // ProjectRef with *string type is wrong - should use *KubernetesNameRef. + ProjectRef *string `json:"projectRef,omitempty"` // want `field WrongTypeRefSpec.ProjectRef has Ref suffix but does not use KubernetesNameRef type` + + // NetworkRef with OpenStackName type is wrong - should use KubernetesNameRef. + NetworkRef OpenStackName `json:"networkRef,omitempty"` // want `field WrongTypeRefSpec.NetworkRef has Ref suffix but does not use KubernetesNameRef type` + + // SubnetRef is correct - uses KubernetesNameRef. + SubnetRef KubernetesNameRef `json:"subnetRef,omitempty"` + + // RouterRef is correct - uses *KubernetesNameRef. + RouterRef *KubernetesNameRef `json:"routerRef,omitempty"` +} + +// WrongTypeRefsSpec tests that plural Refs fields with wrong type are flagged. +type WrongTypeRefsSpec struct { + // SecurityGroupRefs with []OpenStackName type is wrong - should use []KubernetesNameRef. + SecurityGroupRefs []OpenStackName `json:"securityGroupRefs,omitempty"` // want `field WrongTypeRefsSpec.SecurityGroupRefs has Ref suffix but does not use KubernetesNameRef type` + + // NetworkRefs with []string type is wrong - should use []KubernetesNameRef. + NetworkRefs []string `json:"networkRefs,omitempty"` // want `field WrongTypeRefsSpec.NetworkRefs has Ref suffix but does not use KubernetesNameRef type` + + // SubnetRefs is correct - uses []KubernetesNameRef. + SubnetRefs []KubernetesNameRef `json:"subnetRefs,omitempty"` +} + +// WrongTypeRefsStatus tests that Refs in status with wrong type are allowed. +type WrongTypeRefsStatus struct { + // SecurityGroupRefs is allowed in status even with wrong type. + SecurityGroupRefs []OpenStackName `json:"securityGroupRefs,omitempty"` +} From 967d344a3cf5a1f406a8bb9471fcdf1416cb3860 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Fri, 6 Feb 2026 11:32:00 +0100 Subject: [PATCH 4/5] Add nolint comments for intentional API design choices - HostID.ID: Allows raw hypervisor host ID as alternative to ServerRef - PortResourceSpec.HostID: The HostID struct intentionally provides both options - SecurityGroupRefs: Known issue #438, breaking change planned for next API version --- api/v1alpha1/port_types.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/v1alpha1/port_types.go b/api/v1alpha1/port_types.go index d54f3cf95..e312b5812 100644 --- a/api/v1alpha1/port_types.go +++ b/api/v1alpha1/port_types.go @@ -59,7 +59,7 @@ type HostID struct { // This is mutually exclusive with serverRef. // +kubebuilder:validation:MaxLength=36 // +optional - ID string `json:"id,omitempty"` + ID string `json:"id,omitempty"` //nolint:kubeapilinter // intentionally allow raw ID // serverRef is a reference to an ORC Server resource from which to // retrieve the hostID for port binding. The hostID will be read from @@ -167,7 +167,7 @@ type PortResourceSpec struct { // +kubebuilder:validation:MaxItems:=64 // +listType=set // +optional - SecurityGroupRefs []OpenStackName `json:"securityGroupRefs,omitempty"` + SecurityGroupRefs []OpenStackName `json:"securityGroupRefs,omitempty"` //nolint:kubeapilinter // https://github.com/k-orc/openstack-resource-controller/issues/438 // vnicType specifies the type of vNIC which this port should be // attached to. This is used to determine which mechanism driver(s) to @@ -203,7 +203,7 @@ type PortResourceSpec struct { // hostID specifies the host where the port will be bound. // +optional - HostID *HostID `json:"hostID,omitempty"` + HostID *HostID `json:"hostID,omitempty"` //nolint:kubeapilinter // HostID provides both raw ID and ServerRef options } type PortResourceStatus struct { From 54ca4f45373cfa1ee51a649a7b568d14c89c00b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Andr=C3=A9?= Date: Fri, 6 Feb 2026 11:46:02 +0100 Subject: [PATCH 5/5] Clarify listType guidance for struct vs primitive arrays Update API design documentation to align with ssatags linter behavior: listType=set is discouraged for object arrays due to SSA merge issues, so recommend listType=map for structs and listType=set only for primitives. --- website/docs/development/api-design.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/development/api-design.md b/website/docs/development/api-design.md index 5f867fb67..79597cb84 100644 --- a/website/docs/development/api-design.md +++ b/website/docs/development/api-design.md @@ -32,7 +32,7 @@ This is located at `spec.resource` in the base object. It is only defined for [m * Where relevant, the `ResourceSpec` should include a `name` field to allow object name to be overridden. * All fields should use pre-defined validated types where possible, e.g. `OpenStackName`, `NeutronDescription`, `IPvAny`. -* Lists should have type `set` or `map` where possible, but `atomic` lists may be necessary where a struct has no merge key. +* Lists of structs should use `listType=map` with an appropriate `listMapKey` where possible. `listType=set` should only be used for lists of primitives (e.g., strings). `listType=atomic` may be necessary where a struct has no suitable merge key. ### ResourceStatus