Immutable fields broken under certain operations #1543
Description
As per a discussion with @a-hilaly and @jaypipes here's a description of how immutable fields are currently broken under some circumstances and what we should do to fix them on the long term.
Describe the bug
Scenario: As a user, I want to make the name field of my resource immutable after object creation in the Kubernetes API.
Example:
resources:
EventBus:
fields:
Name:
is_immutable: trueWhen I do kubectl edit eventbus ... and change the name I was expecting that the controller sets a terminal condition because an immutable field change has been detected (the field code change detection is actually generated correctly).
However, what happens is that sdk.ReadOne makes a DescribeResource call with spec.name as input, which of course returns ResourceNotFoundException because that resource does not exist. That leads to the code entering the Create path instead of failing during Update. A new resource is created with that new spec.name and the result is zombie resources in AWS (the old resource), i.e. resource leaks.
{"level":"debug","ts":"2022-11-08T14:51:24.907+0100","logger":"ackrt","msg":">> rm.ReadOne","account":"331505801289","role":"","region":"eu-central-1","kind":"Rule","namespace":"default","name":"rule-01","is_adopted":false,"generation":2}
{"level":"debug","ts":"2022-11-08T14:51:24.907+0100","logger":"ackrt","msg":">>> rm.sdkFind","account":"331505801289","role":"","region":"eu-central-1","kind":"Rule","namespace":"default","name":"rule-01","is_adopted":false,"generation":2}
{"level":"debug","ts":"2022-11-08T14:51:25.021+0100","logger":"ackrt","msg":"<<< rm.sdkFind","account":"331505801289","role":"","region":"eu-central-1","kind":"Rule","namespace":"default","name":"rule-01","is_adopted":false,"generation":2,"error":"resource not found"}
{"level":"debug","ts":"2022-11-08T14:51:25.021+0100","logger":"ackrt","msg":"<< rm.ReadOne","account":"331505801289","role":"","region":"eu-central-1","kind":"Rule","namespace":"default","name":"rule-01","is_adopted":false,"generation":2,"error":"resource not found"}
{"level":"debug","ts":"2022-11-08T14:51:25.021+0100","logger":"ackrt","msg":">> r.createResource","account":"331505801289","role":"","region":"eu-central-1","kind":"Rule","namespace":"default","name":"rule-01","is_adopted":false,"generation":2}Steps to reproduce
See above.
Expected outcome
The controller should respect field immutability.
Environment
Every ACK/K8s/Resource version is affected.
Solution
Because the way controller runtime (and as such code gen in ACK) works (level-triggered vs. edge), we do not have access to any previous state during Reconcile() (unless we perform list operations against K8s and AWS control planes, which is very expensive for such a feature and has edges with caching and consistency/concurrency).
A common approach is validation webhooks in the admission chain as here we have access to the current and desired state. Since Kubernetes v1.25 an alternative solution is available though using CRD Validation Rules (beta):
The CEL expression is a one-liner: self == oldSelf | Validate that a required field is immutable once it is set
Going forward, ACK should generate both, an admission controller (for Kubernetes <1.25) but also CRD CEL expressions (older K8s API servers simply ignore this new object). A Helm chart can have logic to deploy the validation webhook in K8s versions <1.25.