+optional has no effect on inlined fields

[invidian]

trafficstars

If you follow https://book.kubebuilder.io/quick-start.html, then patch api/v1/cronjob_types.go with the following changes:

diff --git api/v1/cronjob_types.go api/v1/cronjob_types.go
index 4ec4590..56d9a58 100644
--- api/v1/cronjob_types.go
+++ api/v1/cronjob_types.go
@@ -30,6 +30,30 @@ type CronJobSpec struct {

        // Foo is an example field of CronJob. Edit cronjob_types.go to remove/update
        Foo string `json:"foo,omitempty"`
+
+       // +optional
+       Marketplace *AzureMarketplaceImage `json:"marketplace,omitempty"`
+
+       // +optional
+       ComputeGallery *AzureComputeGalleryImage `json:"computeGallery,omitempty"`
+}
+
+type AzureComputeGalleryImage struct {
+       // +optional
+       *ImagePlan `json:",inline,omitempty"`
+}
+
+type AzureMarketplaceImage struct {
+       ImagePlan `json:",inline"`
+}
+
+type ImagePlan struct {
+       // +kubebuilder:validation:MinLength=1
+       Publisher string `json:"publisher,omitempty"`
+       // +kubebuilder:validation:MinLength=1
+       Offer string `json:"offer,omitempty"`
+       // +kubebuilder:validation:MinLength=1
+       SKU string `json:"sku,omitempty"`
 }

You can see the difference in generated types as follows:

diff --git config/crd/bases/batch.tutorial.kubebuilder.io_cronjobs.yaml config/crd/bases/batch.tutorial.kubebuilder.io_cronjobs.yaml
index e668f47..cf2f23d 100644
--- config/crd/bases/batch.tutorial.kubebuilder.io_cronjobs.yaml
+++ config/crd/bases/batch.tutorial.kubebuilder.io_cronjobs.yaml
@@ -35,10 +35,34 @@ spec:
           spec:
             description: CronJobSpec defines the desired state of CronJob
             properties:
+              computeGallery:
+                properties:
+                  offer:
+                    minLength: 1
+                    type: string
+                  publisher:
+                    minLength: 1
+                    type: string
+                  sku:
+                    minLength: 1
+                    type: string
+                type: object
               foo:
                 description: Foo is an example field of CronJob. Edit cronjob_types.go
                   to remove/update
                 type: string
+              marketplace:
+                properties:
+                  offer:
+                    minLength: 1
+                    type: string
+                  publisher:
+                    minLength: 1
+                    type: string
+                  sku:
+                    minLength: 1
+                    type: string
+                type: object
             type: object
           status:
             description: CronJobStatus defines the observed state of CronJob

As you can see above, +optional got completely ignored in this case, resulting the same CRD struct for both fields, where the source type is different. I'd expect either those fields all be marked as optional or a warning/error that this configuration has no effect.

Found while investigating https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/2277#discussion_r866632784.

[Porges]

I believe this should be due to the package-level defaulting to optional or required (i.e. I think they are optional by default), but I can’t actually get that to take effect. There might be another bug around that here…

[k8s-triage-robot]

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

This bot triages issues and 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 issue is closed

You can:

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

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

/lifecycle stale

[invidian]

/remove-lifecycle stale

[k8s-triage-robot]

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

This bot triages issues and 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 issue is closed

You can:

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

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

/lifecycle stale

[invidian]

/remove-lifecycle stale

[camilamacedo86]

This one is documented here: https://github.com/kubernetes-sigs/kubebuilder/blob/master/docs/book/src/reference/markers.md#difference-between--optional-and--kubebuildervalidationoptional

Could you please check it out and let us know if we can close this issue?

[invidian]

Could you please check it out and let us know if we can close this issue?

Thanks for linking the docs, but I don't see anything there mentioning inlined fields, so I think my original idea about producing a warning in this case still stands. I won't insist on it though, feel free to close the issue if you think the behavior is actually correct and it is no-op by design in this case.

[k8s-triage-robot]

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

This bot triages un-triaged issues 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 issue is closed

You can:

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

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

/lifecycle stale

[invidian]

/remove-lifecycle stale

[k8s-triage-robot]

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

This bot triages un-triaged issues 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 issue is closed

You can:

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

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

/lifecycle stale

[k8s-triage-robot]

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

This bot triages un-triaged issues 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 issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle rotten
• Close this issue with /close
• Offer to help out with Issue Triage

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

/lifecycle rotten

[k8s-triage-robot]

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

This bot triages issues 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 issue is closed

You can:

• Reopen this issue with /reopen
• Mark this issue as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

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

/close not-planned

[k8s-ci-robot]

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

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

This bot triages issues 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 issue is closed

You can:

• Reopen this issue with /reopen
• Mark this issue as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

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

/close not-planned

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.