🐛 fix +listType marker not working on type alias fields by sivchari · Pull Request #1359 · kubernetes-sigs/controller-tools

sivchari · 2026-03-08T12:37:18Z

Include underlying type info (array/object) in schema for slice/map type aliases, allowing field-level markers like +listType to work.

Fixes #988

k8s-ci-robot · 2026-03-08T12:37:28Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: sivchari
Once this PR has been reviewed and has the lgtm label, please assign sbueringer 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

JoelSpeed · 2026-03-16T14:36:54Z

Code change looks good but we do need to fix the commit message please

sivchari · 2026-03-17T03:10:26Z

Sorry, fixed.

k8s-triage-robot · 2026-06-15T03:24:20Z

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

camilamacedo86 · 2026-06-15T09:53:18Z

+		// For type aliases to slice/map types, include the underlying type information
+		// so that markers like +listType can be applied at the field level.


Suggested change

// For type aliases to slice/map types, include the underlying type information

// so that markers like +listType can be applied at the field level.

▎ // Set the type for slice and map aliases so field-level markers like +listType

▎ // and +mapType can work. Structs are skipped — they use a $ref only.

I think we sould need to shape it here.

camilamacedo86 · 2026-06-15T09:55:13Z

+	// For type aliases to slice/map types, include the underlying type information
+	// so that markers like +listType can be applied at the field level.
+	props := &apiextensionsv1.JSONSchemaProps{


Suggested change

// For type aliases to slice/map types, include the underlying type information

// so that markers like +listType can be applied at the field level.

props := &apiextensionsv1.JSONSchemaProps{

▎ // Set the type for slice and map aliases so field-level markers like +listType

▎ // and +mapType can work. Structs are skipped — they use a $ref only.

props := &apiextensionsv1.JSONSchemaProps{

here too?

camilamacedo86 · 2026-06-15T09:57:47Z

@@ -312,26 +315,31 @@ func localNamedToSchema(ctx *schemaContext, ident *ast.Ident) *apiextensionsv1.J
 		ctx.requestSchema(pkgPath, typeNameInfo.Name())
 		link := TypeRefLink(pkgPath, typeNameInfo.Name())



Suggested change

resolvedType := typeInfoRaw

if aliasInfo, isAlias := typeInfoRaw.(*types.Alias); isAlias {

resolvedType = aliasInfo.Rhs()

}

I think we need to handle the case where Go 1.23+ represents type aliases as *types.Alias instead of *types.Named. Right now if someone has type Foo = []T in an external package, isNamed is false, typ stays empty and +listType still fails — same as before this fix. Could you add the alias unwrap before the switch? That seems like a blocker for merging this one.

@sivchari did you check this one ?

Yes, addressed with types.Unalias() so slice/map aliases resolve correctly and +listType/+mapType apply

camilamacedo86 · 2026-06-15T09:58:32Z

@@ -207,6 +207,12 @@ type CronJobSpec struct {
 	// This tests that associative lists work via a nested type.


The PR adds props.Type = "object" for map aliases to make +mapType work at the field level, but there's no test covering that path. Could you add a field with a map alias type and +mapType on it? Without it a regression there would go unnoticed. Could you please add ?

camilamacedo86 · 2026-06-15T10:01:48Z

+
 // +kubebuilder:validation:MinLength=4
 // This tests that markers that are allowed on both fields and types are applied to types
 type LongerString string


Hey, I was looking at flattenAllOfInto and noticed XListType and XListMapKeys don't have their own case there. If both the type and the field have +listType set, they'll hit default: and the annotation gets wiped from the top level of the CRD.

Could it not be a silent failure? XMapType has its own case already so it handles this fine, could we do the same for these two?

camilamacedo86

@sivchari

Thank you so much for looking on this one 🎉

I just try to do a full review. Could you please check it out?
Can you please rebase with main and address those ones?

Then, I would be happy to LGTM this one.

Thank you 🚀

camilamacedo86 · 2026-06-16T06:28:45Z

/remove-lifecycle stale

sivchari · 2026-06-18T05:00:24Z

I addressed these comments. Thnaks.

camilamacedo86 · 2026-06-18T05:28:28Z

Hi @sivchari

Could you please ensure that pass in the lint check and the tests?

Include underlying type info (array/object) in schema for slice/map type aliases, allowing field-level markers like +listType to work. Signed-off-by: sivchari <shibuuuu5@gmail.com>

sivchari · 2026-06-23T02:33:00Z

CI green now.

k8s-ci-robot requested review from alvaroaleman and vincepri March 8, 2026 12:37

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 8, 2026

sivchari force-pushed the fix-issue-988-listtype-type-alias branch from 06bfcda to a39b281 Compare March 9, 2026 02:23

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

sivchari force-pushed the fix-issue-988-listtype-type-alias branch from a39b281 to 639035d Compare March 9, 2026 02:40

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

sivchari force-pushed the fix-issue-988-listtype-type-alias branch from 639035d to 4d23c3a Compare March 16, 2026 08:37

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

sivchari force-pushed the fix-issue-988-listtype-type-alias branch from 4d23c3a to 98002b0 Compare March 17, 2026 03:10

k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Mar 17, 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 15, 2026

camilamacedo86 reviewed Jun 15, 2026

View reviewed changes

camilamacedo86 suggested changes Jun 15, 2026

View reviewed changes

k8s-ci-robot assigned camilamacedo86 Jun 15, 2026

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

sivchari force-pushed the fix-issue-988-listtype-type-alias branch from 98002b0 to 9284ccd Compare June 18, 2026 05:00

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 18, 2026

sivchari force-pushed the fix-issue-988-listtype-type-alias branch 3 times, most recently from bb50795 to d7f3da5 Compare June 18, 2026 06:07

Fix +listType marker not working on type alias fields

1d22cac

Include underlying type info (array/object) in schema for slice/map type aliases, allowing field-level markers like +listType to work. Signed-off-by: sivchari <shibuuuu5@gmail.com>

sivchari force-pushed the fix-issue-988-listtype-type-alias branch from d7f3da5 to 1d22cac Compare June 18, 2026 06:19

		// For type aliases to slice/map types, include the underlying type information
		// so that markers like +listType can be applied at the field level.

		@@ -312,26 +315,31 @@ func localNamedToSchema(ctx schemaContext, ident ast.Ident) *apiextensionsv1.J
		ctx.requestSchema(pkgPath, typeNameInfo.Name())
		link := TypeRefLink(pkgPath, typeNameInfo.Name())

+      resolvedType := typeInfoRaw
+      if aliasInfo, isAlias := typeInfoRaw.(*types.Alias); isAlias {
+            resolvedType = aliasInfo.Rhs()
+      }

		@@ -207,6 +207,12 @@ type CronJobSpec struct {
		// This tests that associative lists work via a nested type.

Uh oh!

Conversation

sivchari commented Mar 8, 2026

Uh oh!

k8s-ci-robot commented Mar 8, 2026

Uh oh!

JoelSpeed commented Mar 16, 2026

Uh oh!

sivchari commented Mar 17, 2026

Uh oh!

k8s-triage-robot commented Jun 15, 2026

Uh oh!

camilamacedo86 Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

sivchari Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 left a comment

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 commented Jun 16, 2026

Uh oh!

sivchari commented Jun 18, 2026

Uh oh!

camilamacedo86 commented Jun 18, 2026

Uh oh!

sivchari commented Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants