Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add validation to CRDUpgradeSafety preflight check to prevent removal of existing fields #922

Conversation

rashmigottipati
Copy link
Contributor

What this PR does / why we need it:

Since the base CRDUpgradeSafety preflight check is in place and merged into the develop branch, we are at a point where we can continue adding validation logic based on the CRDUpgradeSafety preflight check proposal.
The validation logic in this PR focuses on adding a check using the CRD schema checker to ensure that no existing fields are removed from a particular version of a CRD's schema during an upgrade operation.

Which issue(s) this PR fixes:

Fixes #912

Does this PR introduce a user-facing change?

Add validation to `CRDUpgradeSafety` preflight check to prevent removal of existing fields from a particular version of a CRD schema during an upgrade operation

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.:


@rashmigottipati
Copy link
Contributor Author

cc @everettraven

Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

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

LGTM!

… of existing fields

Signed-off-by: Rashmi Gottipati <chowdary.grashmi@gmail.com>
Copy link
Member

@praveenrewar praveenrewar left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for your contribution 🙏🏻
I added some comments on Bryce's PR that maybe relevant to this as well (package name for test files and capitalising error messages), although no action item needed as of now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add validation to CRDUpgradeSafety preflight check to prevent removal of existing fields
3 participants