[willkutler]

This field lacking omitEmpty produces a bug in ROSA when we need to supply a configv1.ImageSpec{}. Since this obj contains the "imageStreamImportMode" field, which is not set to omitempty and but is marked with +openshift:enable:FeatureGate=ImageStreamImportMode , so it is blocked from the CRD unless that featureGate, which is not exposed by hypershift, is enabled. So we have a mismatch where any time the registry management fields are set, we will be setting "imageStreamImportMode": "" on the hostedCluster CR - but that field does not exist in the CRD and it errors.

The field is already omitEmpty in the ImageStatus object so setting it in ImageSpec as well makes sense.

[openshift-merge-bot]

Pipeline controller notification

This PR uses the pipeline controller for second-stage tests. Selection and triggering follow the repository configuration.

Use /test ? to list jobs and /pipeline help for commands.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository YAML (base), Central YAML (inherited)
• Review profile: CHILL
• Plan: Enterprise
• Run ID: c0c142b1-b426-4698-9b0e-ad39b5569d10

📥 Commits

Reviewing files that changed from the base of the PR and between 24b2699 and 9cd268f.

📒 Files selected for processing: 1

• config/v1/types_image.go

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

📝 Walkthrough

ImageSpec.ImageStreamImportMode now omits its JSON field when the value is empty.

Suggested reviewers: prashanth684

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 9cd26

The serialization change does not require a fixture update. No merge-blocking issue remains after normal checks.

Pre-merge checks: 15

Check name	Status	Explanation
Description check		The description explains the serialization bug, the ROSA and HyperShift impact, and why adding omitempty fixes the issue. It directly relates to the changeset.
Title check		The title clearly and concisely identifies the main change: making ImageSpec.ImageStreamImportMode use omitempty.
Docstring Coverage		No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names		The pull request changes only the ImageSpec.ImageStreamImportMode JSON tag in config/v1/types_image.go. The diff adds no Ginkgo tests or test-title declarations, so it introduces no unstable or ov…
Test Structure And Quality		The pull request changes only the JSON tag on ImageSpec.ImageStreamImportMode in config/v1/types_image.go. It adds no Ginkgo tests or test setup, cleanup, waits, or assertions. Therefore, the stat…
Microshift Test Compatibility		The pull request changes only the JSON tag on ImageSpec.ImageStreamImportMode in config/v1/types_image.go. It adds no Ginkgo e2e tests or other test changes, so the MicroShift test compatibility c…
Single Node Openshift (Sno) Test Compatibility		PASS: The pull request changes only the JSON tag in config/v1/types_image.go. It adds no Ginkgo e2e tests or other tests that could make multi-node or HA assumptions.
Topology-Aware Scheduling Compatibility		PASS: The pull request changes only the JSON tag on ImageSpec.ImageStreamImportMode in config/v1/types_image.go. It adds no deployment manifests, operator code, controllers, replica settings, affi…
Ote Binary Stdout Contract		PASS: The pull request changes only the JSON tag on ImageSpec.ImageStreamImportMode in config/v1/types_image.go. It adds omitempty and introduces no process-level code, OTE binary entry point, s…
Ipv6 And Disconnected Network Test Compatibility		The pull request changes only the JSON tag on ImageSpec.ImageStreamImportMode in config/v1/types_image.go. It adds no Ginkgo e2e tests, IPv4 assumptions, or external connectivity requirements.
No-Weak-Crypto		The pull request changes only the JSON tag for ImageSpec.ImageStreamImportMode from json:"imageStreamImportMode" to json:"imageStreamImportMode,omitempty". The diff introduces no MD5, SHA1, DES,…
Container-Privileges		The pull request changes only the JSON tag for ImageSpec.ImageStreamImportMode in config/v1/types_image.go. It adds omitempty and introduces no container or Kubernetes manifest settings such as …
No-Sensitive-Data-In-Logs		PASS. The pull request changes only the JSON tag on ImageSpec.ImageStreamImportMode from json:"imageStreamImportMode" to json:"imageStreamImportMode,omitempty". It adds no logging and introduces…

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

Full details: Stable And Deterministic Test Names

Explanation

The pull request changes only the ImageSpec.ImageStreamImportMode JSON tag in config/v1/types_image.go. The diff adds no Ginkgo tests or test-title declarations, so it introduces no unstable or overly specific test names.

Full details: Test Structure And Quality

Explanation

The pull request changes only the JSON tag on ImageSpec.ImageStreamImportMode in config/v1/types_image.go. It adds no Ginkgo tests or test setup, cleanup, waits, or assertions. Therefore, the stated test-structure requirements are not applicable.

Full details: Microshift Test Compatibility

Explanation

The pull request changes only the JSON tag on ImageSpec.ImageStreamImportMode in config/v1/types_image.go. It adds no Ginkgo e2e tests or other test changes, so the MicroShift test compatibility criteria do not apply.

Full details: Topology-Aware Scheduling Compatibility

Explanation

PASS: The pull request changes only the JSON tag on ImageSpec.ImageStreamImportMode in config/v1/types_image.go. It adds no deployment manifests, operator code, controllers, replica settings, affinity, topology spread constraints, node selectors, tolerations, or PDBs. The topology-aware scheduling check is therefore not applicable.

Full details: Ote Binary Stdout Contract

Explanation

PASS: The pull request changes only the JSON tag on ImageSpec.ImageStreamImportMode in config/v1/types_image.go. It adds omitempty and introduces no process-level code, OTE binary entry point, suite setup, logging, or stdout write.

Full details: No-Weak-Crypto

Explanation

The pull request changes only the JSON tag for ImageSpec.ImageStreamImportMode from json:"imageStreamImportMode" to json:"imageStreamImportMode,omitempty". The diff introduces no MD5, SHA1, DES, RC4, Blowfish, ECB, custom cryptography, or secret/token comparison code.

Full details: Container-Privileges

Explanation

The pull request changes only the JSON tag for ImageSpec.ImageStreamImportMode in config/v1/types_image.go. It adds omitempty and introduces no container or Kubernetes manifest settings such as privileged, host namespace sharing, SYS_ADMIN, root execution, or allowPrivilegeEscalation.

Full details: No-Sensitive-Data-In-Logs

Explanation

PASS. The pull request changes only the JSON tag on ImageSpec.ImageStreamImportMode from json:"imageStreamImportMode" to json:"imageStreamImportMode,omitempty". It adds no logging and introduces no path that exposes passwords, tokens, API keys, PII, session IDs, hostnames, or customer data.

✨ Finishing Touches: 💡 1

🧪 Generate unit tests (beta)

• Create a new PR

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

Warning

Some tools did not complete. Review the errors below.

🔧 golangci-lint (2.14.0)

golangci-lint (2.14.0) skipped for .golangci.yaml: configuration contains disallowed or uninspectable plugin or linter settings. Use supported built-in linters.

_{Comment @coderabbitai help to get the list of available commands.}

[openshift-ci]

Hello @willkutler! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

Do you have a copy of the error message that is presenting here?

[openshift-ci]

@willkutler: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/verify	9cd268f	link	true	/test verify
ci/prow/lint	9cd268f	link	true	/test lint

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.

[JoelSpeed]

This was raised to me again separately yesterday. We need to also remove "" as a valid choice from the enum definition. We will also need a bug report showing the issue to assign as justification for making this change

[willkutler]

@JoelSpeed thanks! I have opened https://redhat.atlassian.net/browse/ROSAENG-74933 to track but not sure what labels etc it needs