Skip to content

fix(api): add omitempty to SingleValue optional fields - #2293

Merged
openshift-merge-bot[bot] merged 2 commits into
shipwright-io:mainfrom
psrvere:singlevalue-omitempty
Aug 19, 2026
Merged

fix(api): add omitempty to SingleValue optional fields#2293
openshift-merge-bot[bot] merged 2 commits into
shipwright-io:mainfrom
psrvere:singlevalue-omitempty

Conversation

@psrvere

@psrvere psrvere commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Changes

The Value, ConfigMapValue and SecretValue fields of the SingleValue
type are mutually exclusive and optional, but their JSON tags lacked
omitempty. As a result, serialized build parameter values always rendered
the two unset fields as explicit nulls:

- name: build-args
  values:
  - configMapValue: null
    secretValue: null
    value: BUILD_VERSION=1.0.0

This adds omitempty to all three fields so only the field that is actually
set appears in the output, producing cleaner Build/BuildRun YAML.

This is a serialization-only change — the CRD schema is unaffected, since
optionality is already driven by the +optional markers (verified: no CRD
regeneration diff). A unit test covering the value, configMap and secret
cases is included.

Submitter Checklist

  • Includes tests if functionality changed/was added
  • Includes docs if changes are user-facing
  • Set a kind label on this PR
  • Release notes block has been filled in, or marked NONE

See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.

Release Notes

Unset optional build parameter value fields are now omitted from serialized output instead of appearing as `null`.

Co-Authored-By: Claude

@openshift-ci openshift-ci Bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Label for when a PR has specified a release note labels Aug 17, 2026
@pull-request-size pull-request-size Bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 17, 2026
@psrvere

psrvere commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

/kind cleanup

@openshift-ci openshift-ci Bot added the kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. label Aug 17, 2026
@psrvere

psrvere commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@psrvere
psrvere marked this pull request as ready for review August 18, 2026 16:42
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 18, 2026
@openshift-ci
openshift-ci Bot requested review from karanibm6 and rxinui August 18, 2026 16:46
The Value, ConfigMapValue and SecretValue fields of SingleValue are
mutually exclusive and optional, but their JSON tags lacked omitempty.
As a result, serialized build parameters always rendered the two unset
fields as explicit nulls, for example:

    - name: build-args
      values:
      - configMapValue: null
        secretValue: null
        value: BUILD_VERSION=1.0.0

Adding omitempty makes only the field that is actually set appear in the
output, producing cleaner Build/BuildRun YAML. This is a serialization-
only change; the CRD schema is unaffected (optionality is already driven
by the +optional markers). A unit test covering the value, configMap and
secret cases is added.

Co-Authored-By: Claude
Signed-off-by: Prateek Singh Rathore <prateek.singh.rathore@gmail.com>
@SaschaSchwarze0

Copy link
Copy Markdown
Member

Rebased ^^

@SaschaSchwarze0

Copy link
Copy Markdown
Member

@psrvere out of interest, which client is rendering the null values for you? I am totally understanding your change and kind of surprised we don't have the fields defined like that. But basically, I do not understand why I never observed this issue with kubectl.

grafik

Comment thread pkg/apis/build/v1beta1/parameter_test.go Outdated
@psrvere

psrvere commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Hi @SaschaSchwarze0, it's not a kubectl client. I hit this while generating YAML directly from the Go API types.

I am building a migration tool that converts OpenShift BuildConfigs to OpenShift Builds (which uses Shipwright under the hood): https://github.com/migtools/crane-plugin-buildconfig-to-shipwright.

It constructs Build objects in Go and marshals them with sigs.k8s.io/yaml to write manifest files. Without omitempty, the unset pointer fields render as explicit configMapValue: null / secretValue: null in the generated YAML.

I will work on refactoring the tests.

…Table

Signed-off-by: Prateek Singh Rathore <prateek.singh.rathore@gmail.com>
@psrvere
psrvere force-pushed the singlevalue-omitempty branch from b5b9f09 to cfc4524 Compare August 19, 2026 16:18
@SaschaSchwarze0 SaschaSchwarze0 added this to the release-v0.21.0 milestone Aug 19, 2026

@SaschaSchwarze0 SaschaSchwarze0 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve
/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 19, 2026
@openshift-ci

openshift-ci Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SaschaSchwarze0

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 19, 2026
@openshift-merge-bot
openshift-merge-bot Bot merged commit 3a4da66 into shipwright-io:main Aug 19, 2026
15 checks passed
@github-project-automation github-project-automation Bot moved this to Done in Issues Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. release-note Label for when a PR has specified a release note size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants