Skip to content

Commit 3406aba

Browse files
committed
orc-api-linter: add kustomizeref rule to catch missing markers
A missing '+orc:kustomize:ref=<Kind>' marker on a KubernetesNameRef field produces no diff in the generated kustomizeconfig.yaml (the generator simply never learns about the field), so 'make verify-generated' cannot catch it. Add a 'kustomizeref' analyzer to tools/orc-api-linter, following the existing 'noopenstackidref' pattern: it uses KAL's inspector/markers helpers to flag any KubernetesNameRef, *KubernetesNameRef, or []KubernetesNameRef field in api/v1alpha1 that lacks the marker (fields on Status structs are exempt). KAL's generic marker parser already understands the 'identifier=value' marker syntax used here, so no custom parsing was needed for the check itself - only for the separate generator, which still needs the JSON path context that KAL's per-field marker API doesn't provide. Registered in plugin.go and enabled via .golangci.yml, so it runs automatically as part of 'make lint' / 'make lint-fix' (golangci-lint rebuilds the custom binary via the existing golangci-kal target). Verified: analyzer unit test passes; a full 'make lint' run against the repo is clean; deliberately stripping a marker from router_types.go is correctly flagged at the right file:line by the rebuilt binary, and restoring it returns to clean.
1 parent 2a24c4f commit 3406aba

6 files changed

Lines changed: 309 additions & 0 deletions

File tree

.golangci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ linters:
4545
- duplicatemarkers
4646
- integers
4747
- jsontags
48+
- kustomizeref
4849
- maxlength
4950
# NOTE: we have a number of boolean fields. Should we convert them to
5051
# string?
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
/*
2+
Copyright The ORC Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package kustomizeref
18+
19+
import (
20+
"go/ast"
21+
"strings"
22+
23+
"golang.org/x/tools/go/analysis"
24+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
25+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
26+
"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
27+
"sigs.k8s.io/kube-api-linter/pkg/analysis/initializer"
28+
"sigs.k8s.io/kube-api-linter/pkg/analysis/registry"
29+
)
30+
31+
const (
32+
name = "kustomizeref"
33+
34+
// kustomizeRefMarker is the identifier of the marker which must be
35+
// present on every field which references another ORC object (or a
36+
// core Secret) by KubernetesNameRef, naming the Kind it refers to.
37+
//
38+
// It is consumed by cmd/kustomizeconfig-generator to generate
39+
// examples/components/kustomizeconfig/kustomizeconfig.yaml.
40+
kustomizeRefMarker = "orc:kustomize:ref"
41+
42+
doc = `Requires every KubernetesNameRef field to carry an +orc:kustomize:ref=<Kind> marker.
43+
44+
examples/components/kustomizeconfig/kustomizeconfig.yaml is generated from
45+
these markers (see cmd/kustomizeconfig-generator) so that kustomize can
46+
correctly rewrite cross-object name references (e.g. when a nameSuffix or
47+
nameReference transformer is applied to the examples). A missing marker
48+
produces no diff in the generated file, so this can't be caught by
49+
'make verify-generated' alone.
50+
51+
Fields declared on a struct whose name contains "Status" are exempt, since
52+
status fields are server-observed and not subject to kustomize name
53+
substitution.
54+
55+
See: https://k-orc.cloud/development/api-design/`
56+
)
57+
58+
// Analyzer is the analyzer for the kustomizeref linter.
59+
var Analyzer = &analysis.Analyzer{
60+
Name: name,
61+
Doc: doc,
62+
Run: run,
63+
Requires: []*analysis.Analyzer{inspector.Analyzer},
64+
}
65+
66+
func init() {
67+
registry.DefaultRegistry().RegisterLinter(initializer.NewInitializer(
68+
name,
69+
Analyzer,
70+
false, // not enabled by default - must be explicitly enabled
71+
))
72+
}
73+
74+
func run(pass *analysis.Pass) (any, error) {
75+
inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
76+
if !ok {
77+
return nil, nil
78+
}
79+
80+
inspect.InspectFieldsIncludingListTypes(func(field *ast.Field, _ extractjsontags.FieldTagInfo, markersAccess markers.Markers, qualifiedFieldName string) {
81+
checkField(pass, field, markersAccess, qualifiedFieldName)
82+
})
83+
84+
return nil, nil
85+
}
86+
87+
func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Markers, qualifiedFieldName string) {
88+
// qualifiedFieldName is in the form "StructName.FieldName"
89+
parts := strings.SplitN(qualifiedFieldName, ".", 2)
90+
if len(parts) != 2 {
91+
return
92+
}
93+
94+
structName := parts[0]
95+
96+
// Status fields are server-observed and are never rewritten by
97+
// kustomize name substitution.
98+
if strings.Contains(structName, "Status") {
99+
return
100+
}
101+
102+
if !isKubernetesNameRefType(field.Type) {
103+
return
104+
}
105+
106+
if hasKustomizeRefMarker(markersAccess.FieldMarkers(field)) {
107+
return
108+
}
109+
110+
pass.Reportf(field.Pos(),
111+
"field %s references another object by KubernetesNameRef but has no +%s=<Kind> marker; "+
112+
"see https://k-orc.cloud/development/api-design/",
113+
qualifiedFieldName, kustomizeRefMarker)
114+
}
115+
116+
func hasKustomizeRefMarker(fieldMarkers markers.MarkerSet) bool {
117+
for _, m := range fieldMarkers.Get(kustomizeRefMarker) {
118+
if m.Payload.Value != "" {
119+
return true
120+
}
121+
}
122+
123+
return false
124+
}
125+
126+
// isKubernetesNameRefType checks if the expression is KubernetesNameRef,
127+
// *KubernetesNameRef, or []KubernetesNameRef.
128+
func isKubernetesNameRefType(expr ast.Expr) bool {
129+
switch e := expr.(type) {
130+
case *ast.Ident:
131+
return e.Name == "KubernetesNameRef"
132+
case *ast.StarExpr:
133+
return isKubernetesNameRefType(e.X)
134+
case *ast.ArrayType:
135+
return isKubernetesNameRefType(e.Elt)
136+
default:
137+
return false
138+
}
139+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
Copyright The ORC Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package kustomizeref
18+
19+
import (
20+
"testing"
21+
22+
"golang.org/x/tools/go/analysis/analysistest"
23+
)
24+
25+
func TestAnalyzer(t *testing.T) {
26+
testdata := analysistest.TestData()
27+
analysistest.Run(t, testdata, Analyzer, "a")
28+
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/*
2+
Copyright The ORC Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
// Package kustomizeref provides a linter that ensures every KubernetesNameRef
18+
// field in api/v1alpha1 carries an +orc:kustomize:ref=<Kind> marker.
19+
//
20+
// # Overview
21+
//
22+
// examples/components/kustomizeconfig/kustomizeconfig.yaml teaches kustomize
23+
// how to rewrite cross-object name references (e.g. under a nameSuffix or
24+
// nameReference transformer) between ORC objects, and between ORC objects and
25+
// core Secrets. It is generated by cmd/kustomizeconfig-generator from
26+
// "+orc:kustomize:ref=<Kind>" markers on the API types.
27+
//
28+
// # What this linter checks
29+
//
30+
// The linter flags any field typed KubernetesNameRef, *KubernetesNameRef, or
31+
// []KubernetesNameRef which does not carry a "+orc:kustomize:ref=<Kind>"
32+
// marker naming the Kind it references.
33+
//
34+
// A missing marker produces no diff in the generated kustomizeconfig.yaml
35+
// (the generator simply never learns about the field), so 'make
36+
// verify-generated' alone cannot catch it. This linter closes that gap.
37+
//
38+
// # Examples
39+
//
40+
// Bad (will be flagged):
41+
//
42+
// type ProjectResourceSpec struct {
43+
// DomainRef *KubernetesNameRef `json:"domainRef,omitempty"`
44+
// }
45+
//
46+
// Good (correct pattern):
47+
//
48+
// type ProjectResourceSpec struct {
49+
// // +orc:kustomize:ref=Domain
50+
// DomainRef *KubernetesNameRef `json:"domainRef,omitempty"`
51+
// }
52+
//
53+
// # Status structs are exempt
54+
//
55+
// Fields declared on a struct whose name contains "Status" are exempt, as
56+
// they are server-observed and not subject to kustomize name substitution.
57+
//
58+
// See https://k-orc.cloud/development/api-design/ for more details.
59+
package kustomizeref
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/*
2+
Copyright The ORC Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package a
18+
19+
// KubernetesNameRef is a reference to a Kubernetes object by name.
20+
type KubernetesNameRef string
21+
22+
// ---- Spec structs: missing markers should be flagged ----
23+
24+
// ProjectResourceSpec has a marked field and an unmarked field.
25+
type ProjectResourceSpec struct {
26+
// name is fine, not a reference field.
27+
Name *string `json:"name,omitempty"`
28+
29+
// domainRef is correctly marked.
30+
// +orc:kustomize:ref=Domain
31+
DomainRef *KubernetesNameRef `json:"domainRef,omitempty"`
32+
33+
// projectRef has no marker and should be flagged.
34+
ProjectRef *KubernetesNameRef `json:"projectRef,omitempty"` // want `field ProjectResourceSpec.ProjectRef references another object by KubernetesNameRef but has no \+orc:kustomize:ref=<Kind> marker`
35+
}
36+
37+
// PortResourceSpec tests the non-pointer and slice variants.
38+
type PortResourceSpec struct {
39+
// networkRef is correctly marked and non-pointer.
40+
// +orc:kustomize:ref=Network
41+
NetworkRef KubernetesNameRef `json:"networkRef,omitempty"`
42+
43+
// securityGroupRefs is correctly marked and a slice.
44+
// +orc:kustomize:ref=SecurityGroup
45+
SecurityGroupRefs []KubernetesNameRef `json:"securityGroupRefs,omitempty"`
46+
47+
// subnetRef is a slice with no marker and should be flagged.
48+
SubnetRefs []KubernetesNameRef `json:"subnetRefs,omitempty"` // want `field PortResourceSpec.SubnetRefs references another object by KubernetesNameRef but has no \+orc:kustomize:ref=<Kind> marker`
49+
}
50+
51+
// ---- Filter structs: missing markers should also be flagged ----
52+
53+
// NetworkFilter is a filter struct that should be checked.
54+
type NetworkFilter struct {
55+
// name is fine.
56+
Name *string `json:"name,omitempty"`
57+
58+
// projectRef has no marker and should be flagged.
59+
ProjectRef *KubernetesNameRef `json:"projectRef,omitempty"` // want `field NetworkFilter.ProjectRef references another object by KubernetesNameRef but has no \+orc:kustomize:ref=<Kind> marker`
60+
}
61+
62+
// ---- Status structs: exempt ----
63+
64+
// ProjectResourceStatus is a status struct, exempt even without a marker.
65+
type ProjectResourceStatus struct {
66+
// domainRef is allowed without a marker in status.
67+
DomainRef KubernetesNameRef `json:"domainRef,omitempty"`
68+
}
69+
70+
// ---- Edge cases ----
71+
72+
// EmptyMarkerSpec tests that a marker with no value is treated as missing.
73+
type EmptyMarkerSpec struct {
74+
// +orc:kustomize:ref
75+
ProjectRef *KubernetesNameRef `json:"projectRef,omitempty"` // want `field EmptyMarkerSpec.ProjectRef references another object by KubernetesNameRef but has no \+orc:kustomize:ref=<Kind> marker`
76+
}
77+
78+
// UnrelatedFieldSpec has non-KubernetesNameRef fields which are never flagged.
79+
type UnrelatedFieldSpec struct {
80+
ProjectID *string `json:"projectID,omitempty"`
81+
}

tools/orc-api-linter/plugin.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
_ "sigs.k8s.io/kube-api-linter/pkg/registration"
3030

3131
// Import ORC-specific linters to register them with the registry.
32+
_ "github.com/k-orc/openstack-resource-controller/v2/tools/orc-api-linter/pkg/analysis/kustomizeref"
3233
_ "github.com/k-orc/openstack-resource-controller/v2/tools/orc-api-linter/pkg/analysis/noopenstackidref"
3334
)
3435

0 commit comments

Comments
 (0)