Add discriminated unions structure linter by MatteoFari · Pull Request #230 · kubernetes-sigs/kube-api-linter

MatteoFari · 2026-02-27T08:12:04Z

Description

This PR introduces a new discriminatedunions linter to enforce structural best practices for discriminated unions.

The new linter, discriminatedunions, does the following:

Validates Union Structure: Detects unions from either legacy markers or declarative markers, including field-marker-only unions.
Enforces Discriminator/Member Rules: Requires exactly one discriminator field, requires the discriminator to be marked required, and requires union member fields to be optional (including support for +unionMember,optional).
Configurable Non-Member Policy: Enforces or allows non-member fields via config (nonMemberFields: Forbid | Allow), with Forbid as default.
Safe Rollout: The linter is disabled by default.

This is the first part and intentionally scopes to structure validation only.
CRD reachability and CEL gating checks will follow.

Part of #20

k8s-ci-robot · 2026-02-27T08:12:10Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: MatteoFari
Once this PR has been reviewed and has the lgtm label, please assign everettraven for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

sivchari · 2026-03-16T08:51:13Z

+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"


We have own inspector, so please use it instead of this.

sivchari · 2026-03-16T08:52:17Z

+	legacyUnionMarker                   = "union"
+	legacyUnionDiscriminatorMarker      = "unionDiscriminator"
+	legacyUnionDiscriminatorLowerMarker = "uniondiscriminator"
+	legacyUnionMemberMarker             = "unionMember"
+	k8sUnionDiscriminatorMarker         = "k8s:unionDiscriminator"
+	k8sUnionMemberMarker                = "k8s:unionMember"


Please move these markers to https://github.com/kubernetes-sigs/kube-api-linter/blob/main/pkg/markers/markers.go

sivchari · 2026-03-16T08:54:21Z

+		Name:     name,
+		Doc:      "Validates discriminated-union marker structure.",
+		Run:      a.run,
+		Requires: []*analysis.Analyzer{inspect.Analyzer, markershelper.Analyzer, extractjsontags.Analyzer},


Suggested change

Requires: []*analysis.Analyzer{inspect.Analyzer, markershelper.Analyzer, extractjsontags.Analyzer},

[]*analysis.Analyzer{inspector.Analyzer}

sivchari · 2026-03-16T08:56:26Z

+	inspectPass, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	if !ok {
+		return nil, kalerrors.ErrCouldNotGetInspector
+	}
+
+	markersAccess, ok := pass.ResultOf[markershelper.Analyzer].(markershelper.Markers)
+	if !ok {
+		return nil, kalerrors.ErrCouldNotGetMarkers
+	}
+
+	jsonTags, ok := pass.ResultOf[extractjsontags.Analyzer].(extractjsontags.StructFieldTags)
+	if !ok {
+		return nil, kalerrors.ErrCouldNotGetJSONTags
+	}
+
+	inspectPass.Preorder([]ast.Node{(*ast.TypeSpec)(nil)}, func(n ast.Node) {
+		typeSpec, ok := n.(*ast.TypeSpec)
+		if !ok || typeSpec.Name == nil {
+			return
+		}
+
+		structType, ok := typeSpec.Type.(*ast.StructType)
+		if !ok {
+			return
+		}
+
+		union := buildUnionType(typeSpec, structType, markersAccess, jsonTags)
+		if union == nil {
+			return
+		}
+
+		a.reportStructureViolations(pass, union)
+	})


You can use InspecTypeSpec.

sivchari · 2026-03-16T09:04:12Z

+}
+
+func markerSpecifiesOptionalMember(marker markershelper.Marker) bool {
+	if strings.Contains(strings.ToLower(marker.String()), ",optional") {


I don't prefer string matching pattern. Is marker.Arguments insufficient ?

sivchari · 2026-03-16T09:04:39Z

+	return false
+}
+
+func isTruthyMarkerValue(value string) bool {


Why do we need it ? I could not come up with pattern.

to normalize a couple of possible encodings in one place, but that was broader than this linter really needed.

sivchari · 2026-03-16T09:53:29Z

+	legacyUnionMarker                   = "union"
+	legacyUnionDiscriminatorMarker      = "unionDiscriminator"
+	legacyUnionDiscriminatorLowerMarker = "uniondiscriminator"
+	legacyUnionMemberMarker             = "unionMember"


Suggested change

legacyUnionMarker = "union"

legacyUnionDiscriminatorMarker = "unionDiscriminator"

legacyUnionDiscriminatorLowerMarker = "uniondiscriminator"

legacyUnionMemberMarker = "unionMember"

UnionMarker = "union"

UnionDiscriminatorMarker = "unionDiscriminator"

UnionDiscriminatorLowerMarker = "uniondiscriminator"

UnionMemberMarker = "unionMember"

sivchari · 2026-03-16T09:58:28Z

+
+func qualifyFieldName(typeName string, field *ast.Field) string {
+	fieldName := utils.FieldName(field)
+	if fieldName == "" {


Why do you fallback fieldName ?

MatteoFari · 2026-03-16T16:59:08Z

@sivchari if the changes are good i can continue with CRD reachability and CEL enforcement.

sivchari · 2026-03-17T07:26:01Z

Please remove it.

sivchari · 2026-03-17T07:27:45Z

+}
+
+func normalizedMarkerValue(value string) string {
+	return strings.ToLower(strings.TrimSpace(strings.Trim(value, `"'`)))


I'm not sure why we make value lower.

sivchari · 2026-03-17T07:32:11Z

+	UnionDiscriminatorMarker = "unionDiscriminator"
+
+	// UnionDiscriminatorLowerMarker is the lowercase variant of UnionDiscriminatorMarker accepted by existing APIs.
+	UnionDiscriminatorLowerMarker = "uniondiscriminator"


Is it right ? At least, I've only found useDiscriminator in k/k
https://github.com/search?q=repo%3Akubernetes%2Fkubernetes+uniondiscriminator&type=code

yeah it was just too defensive

sivchari · 2026-03-17T07:32:38Z

Please add license to header.

sivchari · 2026-03-17T07:35:49Z

+	if ok {
+		return normalizedMarkerValue(value) == "true"
+	}


optional does not have value.

k8s-triage-robot · 2026-06-18T10:05:32Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

JoelSpeed · 2026-07-03T14:17:42Z

/remove-lifecycle stale

JoelSpeed

How would we feel about optionally also enforcing CEL validations for the union membership?

There are two patterns we use in openshift, one for required members, one for optional members. They are tied only to the discriminator value and then the field name itself, so scale well.

The linter could automatically detect these/configure these for users possibly?

Signed-off-by: Matteo Fari <matteofari06@gmail.com>

kubernetes-prow · 2026-07-10T07:56:23Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: MatteoFari
Once this PR has been reviewed and has the lgtm label, please assign everettraven for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

MatteoFari · 2026-07-10T08:01:53Z

How would we feel about optionally also enforcing CEL validations for the union membership?

There are two patterns we use in openshift, one for required members, one for optional members. They are tied only to the discriminator value and then the field name itself, so scale well.

The linter could automatically detect these/configure these for users possibly?

I originally thought about this PR as a first part and later add CRD reachability and CEL gating checks. Do you prefer that I include them in this PR?

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 27, 2026

k8s-ci-robot requested review from everettraven and sivchari February 27, 2026 08:12

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 27, 2026

MatteoFari marked this pull request as ready for review February 27, 2026 08:19

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 27, 2026

k8s-ci-robot requested a review from jpbetz February 27, 2026 08:19

sivchari reviewed Mar 16, 2026

View reviewed changes

MatteoFari force-pushed the discriminatedunions-linter branch from db91ce1 to e29cad4 Compare March 16, 2026 16:51

sivchari reviewed Mar 17, 2026

View reviewed changes

MatteoFari force-pushed the discriminatedunions-linter branch 2 times, most recently from cb6425b to 594b077 Compare March 17, 2026 09:37

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 20, 2026

MatteoFari force-pushed the discriminatedunions-linter branch from 594b077 to f66067a Compare March 20, 2026 09:20

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 20, 2026

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 18, 2026

kubernetes-prow Bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 3, 2026

JoelSpeed reviewed Jul 3, 2026

View reviewed changes

Add discriminated unions structure linter

c17d507

Signed-off-by: Matteo Fari <matteofari06@gmail.com>

MatteoFari force-pushed the discriminatedunions-linter branch from f66067a to c17d507 Compare July 10, 2026 07:56

	Requires: []*analysis.Analyzer{inspect.Analyzer, markershelper.Analyzer, extractjsontags.Analyzer},
	[]*analysis.Analyzer{inspector.Analyzer}

Uh oh!

Conversation

MatteoFari commented Feb 27, 2026

Description

Uh oh!

k8s-ci-robot commented Feb 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatteoFari commented Mar 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-triage-robot commented Jun 18, 2026

Uh oh!

JoelSpeed commented Jul 3, 2026

Uh oh!

JoelSpeed left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kubernetes-prow Bot commented Jul 10, 2026

Uh oh!

MatteoFari commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants