-
Notifications
You must be signed in to change notification settings - Fork 74
Check the correctness and consistency of immutability constraints in all of our resources #1448
Comments
We need to do this in order to be actually immutable in higher versions: knative/eventing#3523, knative/eventing#3522 |
I think that needs to be done in v1beta1 and v1... This has just become a high priority bug |
Wondering whether we also need to do that in v1alpha1. I checked the codebase, we write the function |
@bharattkukreja : Just synced with @nachocano offline, the scope of this issue is:
|
I already worked on the UTs to cover all fields in our current code base as the UT does not cover some fields check in our current version in the code base, will submit a PR, and @bharattkukreja will take over this issue. He might need to add more fields if missing and add UTs to them (see point 2 in the above comment) |
Problem
Related to #1441, #1442 and #1433.
When promoting the sources related types to v1, we found that the checkImmutableFields methods between validations are somewhat inconsistent and incomplete (also for the UTs). We need to check them again in the following sprint to make sure they are correct.
Exit Criteria
Make sure the checkImmutability is called in validation logic for all versions of sources (v1, v1beta1 and v1alpha1). @bharattkukreja
Make sure in our code base, the immutable fields that are included in the checkImmutableFields function are correct, complete and consistent among sources, need to add the missing fields if needed. @danyinggu
As mentioned in PR Update immutable fields for sources in v1alpha1 and v1beta1 #1504, we add immutability check ClusterNameAnnotation and AutoscalingClass Annotation (relates to Autoscaling annotation should be made immutable #665) for all versions of resources. We also add more fields to be immutable in all of the versions. (see the PR for details)
Based on 1 and 2, make sure the immutable fields cannot be mutated when using kubectl edit and mutable fields can be mutated. @bharattkukreja
cc @nachocano
The text was updated successfully, but these errors were encountered: