Skip to content

Run updateValidation when updating Package and PackageMetadata - #1888

Open
himsngh wants to merge 1 commit into
carvel-dev:developfrom
himsngh:himsngh/fix-package-update-validation
Open

himsngh wants to merge 1 commit into
carvel-dev:developfrom
himsngh:himsngh/fix-package-update-validation

Conversation

@himsngh

@himsngh himsngh commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

PackageCRDREST.Update and PackageMetadataCRDREST.Update run createValidation when the object does not exist in the namespace and the update is turned into a create, but never run updateValidation when an existing object is updated. updateValidation is how the generic apiserver applies validating admission (validating webhooks and ValidatingAdmissionPolicy) to update requests, so admission configured for UPDATE on packages or packagemetadatas in data.packaging.carvel.dev was skipped for updates of existing objects.

Call updateValidation with the updated and the existing object before the update is persisted, in both REST implementations.

Changes:

  1. pkg/apiserver/registry/datapackaging/package_crd_rest.go

  2. pkg/apiserver/registry/datapackaging/package_metadata_crd_rest.go

    • Call updateValidation when updating an existing object
  3. pkg/apiserver/registry/datapackaging/package_crd_rest_test.go

  4. pkg/apiserver/registry/datapackaging/package_metadata_crd_rest_test.go

    • Add tests verifying updateValidation is called with the updated and existing objects, and that a validation error is returned and the stored object is left unchanged

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?


Additional Notes for your reviewer:

Review Checklist:
  • Follows the developer guidelines
  • Relevant tests are added or updated
  • Relevant docs in this repo added or updated
  • Relevant carvel.dev docs added or updated in a separate PR and there's
    a link to that PR
  • Code is at least as readable and maintainable as it was before this
    change

Additional documentation e.g., Proposal, usage docs, etc.:


PackageCRDREST.Update and PackageMetadataCRDREST.Update run
createValidation when the object does not exist in the namespace and
the update is turned into a create, but never run updateValidation
when an existing object is updated. updateValidation is how the
generic apiserver applies validating admission (validating webhooks
and ValidatingAdmissionPolicy) to update requests, so admission
configured for UPDATE on packages or packagemetadatas in
data.packaging.carvel.dev was skipped for updates of existing objects.

Call updateValidation with the updated and the existing object before
the update is persisted, in both REST implementations.

Changes:

1. pkg/apiserver/registry/datapackaging/package_crd_rest.go
2. pkg/apiserver/registry/datapackaging/package_metadata_crd_rest.go
   - Call updateValidation when updating an existing object

3. pkg/apiserver/registry/datapackaging/package_crd_rest_test.go
4. pkg/apiserver/registry/datapackaging/package_metadata_crd_rest_test.go
   - Add tests verifying updateValidation is called with the updated
     and existing objects, and that a validation error is returned and
     the stored object is left unchanged

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Himanshu Singh <himansh.singh3@gmail.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant