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 allow changing an existing required field to be optional #915

Closed
everettraven opened this issue Mar 29, 2024 · 0 comments · Fixed by #933
Labels
carvel accepted This issue should be considered for future work and that the triage process has been completed enhancement This issue is a feature request

Comments

@everettraven
Copy link
Contributor

Now that there is a base CRDUpgradeSafety preflight check in place, we can continue adding validation logic based on the CRDUpgradeSafety preflight check proposal.

This issue focuses on adding a validation to ensure that updates to an existing field are allowed when the update performed is changing it from required to optional

As a potential source of inspiration, here is how a couple of the existing validations are implemented:

  • Definitions:
    func NoScopeChange(old, new v1.CustomResourceDefinition) error {
    if old.Spec.Scope != new.Spec.Scope {
    return fmt.Errorf("scope changed from %q to %q", old.Spec.Scope, new.Spec.Scope)
    }
    return nil
    }
    func NoStoredVersionRemoved(old, new v1.CustomResourceDefinition) error {
    newVersions := sets.New[string]()
    for _, version := range new.Spec.Versions {
    if !newVersions.Has(version.Name) {
    newVersions.Insert(version.Name)
    }
    }
    for _, storedVersion := range old.Status.StoredVersions {
    if !newVersions.Has(storedVersion) {
    return fmt.Errorf("stored version %q removed", storedVersion)
    }
    }
    return nil
    }
  • Consumption:
    Validations: []Validation{
    NewValidationFunc("NoScopeChange", NoScopeChange),
    NewValidationFunc("NoStoredVersionRemoved", NoStoredVersionRemoved),
    },

Note

It may be useful to consider a more generic check that allows for easily extending the validations that are run against updates to existing fields in a CRD schema

@everettraven everettraven added the carvel triage This issue has not yet been reviewed for validity label Mar 29, 2024
@praveenrewar praveenrewar added enhancement This issue is a feature request carvel accepted This issue should be considered for future work and that the triage process has been completed and removed carvel triage This issue has not yet been reviewed for validity labels Apr 8, 2024
@github-project-automation github-project-automation bot moved this to Closed in Carvel Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
carvel accepted This issue should be considered for future work and that the triage process has been completed enhancement This issue is a feature request
Projects
Archived in project
2 participants