Skip to content

fix(operator): preserve metadata in partial conversion responses - #12432

Merged
julienmancuso merged 2 commits into
mainfrom
jsm/12351
Jul 31, 2026
Merged

fix(operator): preserve metadata in partial conversion responses#12432
julienmancuso merged 2 commits into
mainfrom
jsm/12351

Conversation

@julienmancuso

@julienmancuso julienmancuso commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #12351.

Kubernetes Server-Side Apply can send partial objects through the CRD conversion webhook while reconciling fields owned under another API version. The v1beta1 JSON normalizer removed the empty top-level metadata: {} object, causing the API server to reject the conversion response with:

returned invalid metadata: missing metadata in converted object

This change:

  • restores the top-level metadata envelope after normalizing Kubernetes runtime.Object values;
  • preserves the removal of nested encoder artifacts such as podTemplate.metadata: {};
  • adds marshal regression coverage for all v1beta1 root resource types; and
  • adds conversion-webhook regression coverage for partial DGD, DCD, and DGDR objects.

Validation

go test ./api/... -count=1

All Operator API tests pass.


Open in Devin Review

Summary by CodeRabbit

  • Bug Fixes

    • Preserved the top-level metadata field as an empty object when converting Kubernetes objects with minimal metadata.
    • Ensured conversion webhook responses retain valid runtime-object structure for partial objects.
  • Tests

    • Added coverage for metadata preservation across supported API versions and object kinds.
    • Added validation for successful webhook conversions and returned object counts.

@julienmancuso
julienmancuso requested a review from a team as a code owner July 30, 2026 17:21
@github-actions github-actions Bot added fix deployment::k8s Relates to dynamo deployment in kubernetes labels Jul 30, 2026
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Changes

Metadata preservation

Layer / File(s) Summary
Runtime-object metadata normalization
deploy/operator/api/v1beta1/marshal.go, deploy/operator/api/v1beta1/marshal_test.go
The normalizer restores missing or nil root metadata for runtime objects, with tests covering multiple typed objects.
Partial-object webhook validation
deploy/operator/api/conversion_webhook_test.go
The webhook test verifies successful conversion of minimal objects for multiple kinds while retaining empty metadata.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately captures the main change: preserving metadata in partial conversion responses.
Description check ✅ Passed The description covers the bug, fix, linked issue, and validation; it only misses the exact template headings and reviewer-start note.
Linked Issues check ✅ Passed The changes address #12351 by restoring top-level metadata on partial converted objects and adding regression tests for the reported failure.
Out of Scope Changes check ✅ Passed The added normalization logic and tests are directly tied to the metadata-preservation fix, with no clear unrelated scope creep.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
deploy/operator/api/conversion_webhook_test.go (1)

25-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add t.Log headings for each test step.

  • deploy/operator/api/conversion_webhook_test.go#L25-L107: add headings before scheme setup, request construction/execution, and response validation.
  • deploy/operator/api/v1beta1/marshal_test.go#L52-L115: add headings before marshaling and metadata assertions in each subtest.

As per coding guidelines, “Use t.Log to tell each Go test's story, with one heading before every block implementing a test step.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@deploy/operator/api/conversion_webhook_test.go` around lines 25 - 107, The
tests lack step headings describing their execution flow. In
deploy/operator/api/conversion_webhook_test.go:25-107, add t.Log headings before
scheme setup, request construction and execution, and response validation; in
deploy/operator/api/v1beta1/marshal_test.go:52-115, add t.Log headings before
marshaling and metadata assertions in each subtest, using one heading for each
test-step block.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@deploy/operator/api/conversion_webhook_test.go`:
- Around line 25-107: The tests lack step headings describing their execution
flow. In deploy/operator/api/conversion_webhook_test.go:25-107, add t.Log
headings before scheme setup, request construction and execution, and response
validation; in deploy/operator/api/v1beta1/marshal_test.go:52-115, add t.Log
headings before marshaling and metadata assertions in each subtest, using one
heading for each test-step block.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b6ae6db3-0399-4c2d-ad97-92d86f640c23

📥 Commits

Reviewing files that changed from the base of the PR and between a387b06 and 3a1b28f.

📒 Files selected for processing (3)
  • deploy/operator/api/conversion_webhook_test.go
  • deploy/operator/api/v1beta1/marshal.go
  • deploy/operator/api/v1beta1/marshal_test.go

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

Open in Devin Review

Comment thread deploy/operator/api/conversion_webhook_test.go
@julienmancuso
julienmancuso merged commit 2b66136 into main Jul 31, 2026
99 checks passed
@julienmancuso
julienmancuso deleted the jsm/12351 branch July 31, 2026 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deployment::k8s Relates to dynamo deployment in kubernetes fix size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: Webhook returns partial objects without metadata

2 participants