Enable omitzero modernize linter rule #358
Conversation
Signed-off-by: Erik Godding Boye <[email protected]>
cert-manager-prow
bot
added
the
dco-signoff: yes
cert-manager-prow
bot
added
the
size/M
| x-kubernetes-list-type: map | ||
| type: object | ||
| required: | ||
| - spec |
There was a problem hiding this comment.
I believe this makes sense, and is an oversight, even if it's technically a breaking change.
There was a problem hiding this comment.
Pull Request Overview
This PR updates the JSON struct tags for Kubernetes API types to align with modern best practices, specifically migrating from omitempty to omitzero where appropriate and making the spec field required in the CRD schemas.
Key changes:
- Migrated optional struct fields from
omitemptytoomitzerofor better zero-value handling - Made the
specfield required in CRD definitions for both GoogleCASIssuer and GoogleCASClusterIssuer - Removed the linter exception that previously disabled the
omitzeromodernization check
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| api/v1beta1/googlecasissuer_types.go | Updated JSON struct tags for GoogleCASIssuer and GoogleCASIssuerList types to use omitzero and removed omitempty where appropriate |
| api/v1beta1/googlecasclusterissuer_types.go | Updated JSON struct tags for GoogleCASClusterIssuer and GoogleCASClusterIssuerList types to use omitzero and removed omitempty where appropriate |
| deploy/crds/cas-issuer.jetstack.io_googlecasissuers.yaml | Added required validation for the spec field in the CRD schema |
| deploy/crds/cas-issuer.jetstack.io_googlecasclusterissuers.yaml | Added required validation for the spec field in the CRD schema |
| deploy/charts/google-cas-issuer/templates/crd-cas-issuer.jetstack.io_googlecasissuers.yaml | Added required validation for the spec field in the Helm chart CRD template |
| deploy/charts/google-cas-issuer/templates/crd-cas-issuer.jetstack.io_googlecasclusterissuers.yaml | Added required validation for the spec field in the Helm chart CRD template |
| .golangci.yaml | Removed the modernize linter exception that disabled omitzero checks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| metav1.ObjectMeta `json:"metadata,omitempty"` | ||
| metav1.TypeMeta `json:",inline"` | ||
| // +optional | ||
| metav1.ObjectMeta `json:"metadata"` |
There was a problem hiding this comment.
The metadata field for Kubernetes resources should use json:\"metadata,omitempty\" according to Kubernetes API conventions. The metadata field is automatically populated by the API server and an empty struct should be omitted during serialization. Removing omitempty deviates from standard Kubernetes patterns and could cause issues with marshaling/unmarshaling.
| metav1.TypeMeta `json:",inline"` | ||
| metav1.ListMeta `json:"metadata,omitempty"` | ||
| // +optional | ||
| metav1.ListMeta `json:"metadata"` |
There was a problem hiding this comment.
The metadata field for Kubernetes list types should use json:\"metadata,omitempty\" according to Kubernetes API conventions. This is the standard pattern used across all Kubernetes API types and ensures proper serialization behavior.
| metav1.ListMeta `json:"metadata"` | |
| metav1.ListMeta `json:"metadata,omitempty"` |
| metav1.ObjectMeta `json:"metadata,omitempty"` | ||
| metav1.TypeMeta `json:",inline"` | ||
| // +optional | ||
| metav1.ObjectMeta `json:"metadata"` |
There was a problem hiding this comment.
The metadata field for Kubernetes resources should use json:\"metadata,omitempty\" according to Kubernetes API conventions. The metadata field is automatically populated by the API server and an empty struct should be omitted during serialization. Removing omitempty deviates from standard Kubernetes patterns and could cause issues with marshaling/unmarshaling.
| metav1.TypeMeta `json:",inline"` | ||
| metav1.ListMeta `json:"metadata,omitempty"` | ||
| // +optional | ||
| metav1.ListMeta `json:"metadata"` |
There was a problem hiding this comment.
The metadata field for Kubernetes list types should use json:\"metadata,omitempty\" according to Kubernetes API conventions. This is the standard pattern used across all Kubernetes API types and ensures proper serialization behavior.
| metav1.ListMeta `json:"metadata"` | |
| metav1.ListMeta `json:"metadata,omitempty"` |
| // Credentials is a reference to a Kubernetes Secret Key that contains Google Service Account Credentials | ||
| // +optional | ||
| Credentials cmmetav1.SecretKeySelector `json:"credentials,omitempty"` | ||
| Credentials cmmetav1.SecretKeySelector `json:"credentials,omitzero"` |
There was a problem hiding this comment.
According to the Kubernetes API conventions, this optional field should have been a pointer field without omitzero.
|
/cc @inteon |
|
Thanks @erikgb! /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: inteon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
cert-manager-prow
bot
added
the
approved
Applying the principles laid out in cert-manager/approver-policy#740.