Proposal for handling server-side-overrides in ACK.

docs/design/proposals/server-side-override/images/server-side-override.drawio

@@ -0,0 +1 @@
+<mxfile host="Electron" modified="2021-07-06T05:08:01.879Z" agent="5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) draw.io/14.6.13 Chrome/89.0.4389.128 Electron/12.0.7 Safari/537.36" etag="VUWGlTwSoyQBP0SmkYqj" version="14.6.13" type="device"><diagram name="Page-1" id="e7e014a7-5840-1c2e-5031-d8a46d1fe8dd">7Vxbc6M2FP41nmkf4kEIcXm0nezutNvpTtLObh8JKLa6GCjIcdxfXwkkAxKOWcfYNHYn00UHIeB8+s5NMiM4W758zPx08VsS4mhkGuHLCN6OTBNYpjnif0a4KSUuskvBPCOh6FQJHsi/WAgNIV2REOeNjjRJIkrSpjBI4hgHtCHzsyxZN7s9JVHzrqk/x5rgIfAjXfqVhHQhpMD2qhOfMJkvxK1dU7zfox98n2fJKhb3G5nwqfivPL305VjiRfOFHybrmgjejeAsSxJaHi1fZjjiupVqK6/7sOPs9rkzHNMuF/xyA9YT2/nbciefw83cnHy6824sMcyzH62wfI/iaelGaog9eMoPV8voM3nCEYlZa8puTElAUr+4Ozt3F1NCN/wMzsgSU5wxeSQu+FLJpusFofgh9QM+6JrNKiZb0GXEWoAdMqCpzy7Jtu0o8tOcPBbPYzBJhoNVlpNnfI/zcj5x6TPmD+RHk4jMYyajCR84Z/ch8fwP3riFNpNE/iOOplvwZkmUZMWLSvjgNFlR/tCz7ZTj4+v6lrpjN8YvNZHQ/0ecsFfONqyLPOuIuSC4AkVzXU08S4gWtSmHZD9fzPX5duQKb3YgIP8R+E0N/g/R6oVJZrfaPGAvyR5o6gv9BkwPBZ5PDLAH0Q20ALEkYchPSiw+4yf+YrCS3It3LUQ0S75jiUqcFJMtS6hP/WoGpAmJaaEMNGV/TD0zY4xGiD3mjLVB1WZ/vHtGGZhsaDax+I2wn9M1zumRYEVNWC0NVbcFVWlHjg8qvHL6dJyGCvjQ7sZpOUuOD7+lwY9pEF7p3BlR2ETUOzOd7SudT0dnZKEmnVFHOqOe4Ecd4K9B06TSrYI1I84imSexH9XR1tX2+kT8AcfY0KWj6xK06BKAvnRpO+9Gl6bZTZleb7p09+uS2YBnHApt4jic8HyN+5rIz3MSNI0KfiH0Gz8em6L1F6f12PRs0b59ETwvGptao4bADkPwKgp5ssoC3GHi4LCRTOpg1dBAbWZCyDIc+ZRZx4aRaoNI3OELn4nVXFBs1LYtRyjfR1xUTwmVcUynOY6jjEP9bI6pNg6D0d/UugmedH5chF59Khu1PlU1Wcv7V1N3q//DZ7OrJ0L/V8sAvXObWUs3s5PZrzypZHFDxmIFpo9rNNo1IDGb8J45GpUW7DWi7LP0O7Wy1w7LclVpmF7rCAdlsIHbTCkAUCo6XS22peQmwFYGOpLJ3ga5ygP3aoSRXowE419XjyzA59f6aRptdhmO2vRqJfkTiSJFpNmanbalLR0qMpMiuDlS5rEtFEui68VBYLRRva/iIOrgEw+m+kCICZWKLFC12ZWYCHlj9PpQO6h5NPboZT9zXGiIpesBYYi9W+pYxtCoo9fgNO1fazbHgt9RnOL5azZIg3/y9WEX/64xrwYoUPis4XnamLdD2ehgR7g3lJW2ZCAeE6keU12c7OoxHTXEVAfq21/qBSzI/eUswz5lztIoKMvUJbKSd+s9bcV7bl1l3X22GdD+3KfXI+H2JpkDIxxUckfz0NxRY27H3PFYhJO3q6FqVYS7Fzz7fUXTFb0gtlnnZpvdtgfIjmgZTsQNHOx/Vny/0rTa/HQTlOqe8Jk+f/ypqO7y+IH9v36MwM/V5exozv9FHP8/07A0uPdbU1ve/TGT/Xx2rfHAQpZVzjXFXpOFsyGhSVyMr3Z/5N0nccyDHML7lB3YZeULlb3e7xxT86Et0883x45QS5ALRDfGGDZWiBzHOsIKUWf3YA/KPSDLGzvKiolxoIeAjjE294zVt5PQqxj25dYA23YInraSYXeoZPSX+wytjO8p8KBDiQbVaqE2VN8800sUzoVWC80273haknXY4/PeKu3bEs7bK+3aUH1zpy09PU3A7BYBc07ieSPCzctLgtr6uhHyaGdJYsxDZrrAGQ+xCW/ECe+78ON5IYrL86N6rePCAmbkKnso2kogJzUJki3XFYSTbOIe2AKCo6fkHwktUuQ02UXD6zrCFtZhrBs4R0h6d2phf+w8LP+v/EzCO9D7A3Vp6LSu39HTU4O75JKd6SpfvF8naTc175zdReopjKZ84RNLjcvfd4LX1R0VZnCvtSw91w3oaj9VU1m4uy9JTooSJbzNyr5te0v7XW1Vag6wJR1y2uym2xeuejbkqRwzYrxuBKxGnuLgYrgHz0++PtfEh+myrEPLqsBVM1ZtqL7dlr7WDYwLKqsCFUl4bvr0ucI9EPqo1VLLhIfxB9oaf9SheuaPdH91/oBLKZeq5DHPTR63w0cr9i4mxuxJvgktFY1yLREYlhRUi4lFa1NvHW85US5x7M3n5E+UBsJuaCN1rdCyDiQ4Qpa6hqmN1TfD9cQOmI3dCZeyHQy5lmptrRbGW0j2Og3n+1yHHAil1A1aln0gnxxLc5jqUH3TSc/NAazodCmbvZAaArXlbW0zqj8eXcBSo2M1f9dsHVps1Ah54mqjqyfZwNqxZa5lG1y5a66oi+zaJNd9T12g7ql7r5xVtwegs2+Hdjt8aqHr5jm5H66Mdj0bjU63c25gESySv5iTQFvKEN3zU3tsq2Mx5XqnNRZ6SeG6ZnjAx9wU9rdUw0+6iOjppYfrJ/re/Im+88Oq1zCuX2l7y1fazo+oqSF6/erJwYmTOTR09frQ9Se9b/hJb4+Asmb1VeUyuqo+XQ3v/gM=</diagram></mxfile>

docs/design/proposals/server-side-override/server-side-override.md

@@ -0,0 +1,224 @@
+# ACK Handling Server Side Defaults
+
+## Problem
+
+During reconciliation, ACK service controller consumes the custom resource spec (i.e. desired state) from “etcd” and
+then creates/updates the AWS resource to match that desired state. ACK service controller uses the custom resource spec
+fields to construct Create/Update AWS API request.
+
+However, some AWS API’s behavior does not completely rely on the fields present in Create/Update AWS API request alone.
+In the backend, these AWS APIs sometime update the value of field present in request
+with newly calculated value which represents the latest state of the resource in AWS. AWS APIs return these updated 
+values with Create/Update API output and Read/List/Describe API output after resource creation.
+
+Following problem arise due to this behavior of AWS APIs,
+
+* When an AWS API does not strictly use the property value as is in the backend, but updates it, the desired spec of
+the K8s resource in the “etcd” does not correctly represent the actual state of resource in AWS. This is a problem because
+for K8s user, the spec of custom resource is the source of truth for state of K8s resource.
+
+
+## Solution Requirements
+
+1. Consistency with K8s conventions
+2. Consistency across ACK service controllers
+3. GitOps compatibility
+4. User experience for custom resource lifecycle 
+
+## Solution
+
+In this solution, If an existing value is updated as part of Create/Update API output, ACK service controller will persist 
+the details of server-side-overrides in the custom resource’s Annotation, add a condition in custom resource  status to 
+indicate that server override some spec fieds.
+
+> Note: The original value provided by the k8s user in desired spec will not be updated, even though server udated that value. 
+
+
+1. *Consistency with K8s conventions* ✅
+    1. In K8s convention, the spec fields are not duplicated into status fields.
+2. *Consistency across ACK service controllers* ✅
+3. *GitOps compatibility* ✅
+    1. There should be no complexity in implementing GitOps for ACK custom resources. 
+    2. Approach mentioned in low level design below should work.
+4. *User experience for custom resource lifecycle* ✅
+    1. Applying updates to custom resource will not be confusing for the user. Users will update the spec and service
+     controller will create Update request based on new spec.
+
+### High Level Design
+
+![High Level Design](./images/server-side-override.png)
+[source](./images/server-side-override.drawio)
+
+
+### Low Level Design
+
+To implement the flow described in high level design, we will walk through reconciliation of an example resource through
+following steps and layout in detail how these steps will be achieved by ACK controller. 
+We assume that Fluxcd is used to apply desired state of custom resource.
+
+1. Assume there is a custom resource with following spec and status fields.
+
+    “fieldA” and “fieldC” are regular spec and status field respectively. “fieldA” is provided by K8s user and “fieldD” 
+    is updated in status based on latest observation.
+
+    overrideFieldB is modified by reconciler whenever the value is provided by the k8s user. Ex: “-xyz” gets appended to
+    it’s value. 
+
+    ```yaml
+    Spec:
+     fieldA string
+     overrideFieldB string
+    Status:
+     fieldC string
+    ```
+
+    and the manifest file(desired state) looks like,
+
+    ```yaml
+    Spec:
+     fieldA: valueA
+     overrideFieldB: valueB
+   ```
+
+2.  When this resource is reconciled by the ACK service controller, we expect the CreateOutput to have following value 
+    for these fields. fieldA → “valueA“, overrideFieldB → “valueB-xyz“, fieldC → ”valueC“
+
+    Based on this result, ACK service controller will persist the following object in etcd:
+
+    Annotations:
+
+    ```yaml
+    services.k8s.aws/spec-overrides: 
+       “{ 
+           \“overrideFieldB\”:
+               {
+                   \“userInput\”:\“valueB\”,
+                   \“serverOverride\”:\“valueB-xyz\”
+               }
+       }”
+       Spec:
+        fieldA: valueA
+        overrideFieldB: valueB
+       Status:
+        fieldC: valueC
+        conditions:
+        - type: ACK.ServerSideOverride
+          status: True
+          reason: ServerSideSpecOverride
+          message: Spec override values are present in services.aws.k8s/spec-overrides annotation
+    ```
+
+    > Note: the annotation above has single string value. It is been broken into multiple lines to showcase the structure better.
+
+3. During the next reconciliation of manifest mentioned in #1, the desired state will be constructed not just from spec 
+   in etcd but also from the `services.k8s.aws/spec-overrides` Annotations. The desired object will look like the object
+   persisted in etcd in #2
+
+   After reading the desired state from etcd, the reconciler code will go through all the spec fields and see whether 
+   they exist in the spec-override annotation map. If a corresponding entry exists, reconciler will marshall current 
+   desired value of spec field and compare with “userInput” in annotation override, 
+
+   * If there is no difference between “userInput” and marshalled desiredValue, reconciler will update the desired state
+     with serverOverride from spec-override annotation. 
+
+   * If the marshalled string does `not` match “userInput”, it means there was some change in manifest by the user, and 
+     serverOverrides from the spec-override map will not be used for desired state.
+
+   In this case, There will be no delta between desired and latest object, And no AWS updates will be triggered.
+
+4. Now let’s assume that User updates the manifest. The manifest will look like
+
+    ```yaml
+    Spec:
+     fieldA: valueA
+     overrideFieldB: updatedValueB
+    ```
+
+    Now, during reconciliation, when the desired object is being constructed by parsing Annotations, reconciler will see
+    that overrideFieldB was updated because the userInput inside AnnotationMap is not equal to the current value in manifest.
+    Reconciler will remove the annotation for overrideFieldB.
+
+5. At this time, the desired object looks different from latest object in AWS and an Update will happen. The result of 
+   UpdateOutput will contain following fields, fieldA → “valueA”, overrideFieldB → “updatedValueB-xyz”
+
+   During sdkUpdate, Annotations will be updated again. 
+
+   The object persisted in etcd will look like
+
+    ```yaml
+    Annotations:
+    services.k8s.aws/spec-overrides: 
+    “{
+        \“overrideFieldB\”:
+            {
+                \“userInput\”:\“updatedValueB\”,
+                \“serverOverride\”:\“updatedValueB-xyz\”
+            }
+    }”
+    Spec:
+     fieldA: valueA
+     overrideFieldB: updatedValueB-xyz
+    Status:
+     fieldC: valueC
+     conditions:
+     - type: ACK.ServerSideOverride
+       status: True
+       reason: ServerSideSpecOverride
+       message: Spec override values are present in services.aws.k8s/spec-overrides annotation
+    ```
+
+    And that is how reconciler will handle simple case of server overrides of desired state.
+
+### Pseudo Code Changes
+TBD
+
+## Alternate Solutions Considered
+
+### Solution 1
+
+In this solution, the status fields will include all the fields which are CreateOutput shape members. Current status 
+fields are the CreateOutput shape members which are not present in CreateInput shape (spec fields).
+
+During reconciliation, the Read(One|Many),Update request inputs will be created from desired state but giving precedence
+to status fields over spec in case of conflict.
+
+
+1. *Consistency with K8s conventions* ❌ 
+    1. In K8s convention, the spec fields are not duplicated into status fields. This will confuse k8s users when they 
+    describe their custom resources.
+2. *Consistency across ACK service controllers* ✅
+    1. There should be no complexity in following this approach across all ACK service controllers.
+3. *GitOps compatibility* ✅
+    1. There should be no complexity in implementing GitOps for ACK custom resources. 
+    2. Approach mentioned in low level design below should work.
+4. *User experience for custom resource lifecycle* ✅
+    1. Applying updates to custom resource can become confusing for the user.
+    2. To update custom resource spec, K8s user will have to check resource status because status will have most updated
+     value of all the fields.
+
+Other Cons :
+
+* The number of status fields will be very large and there will be a lot of duplication in custom resource description
+
+
+### Solution 2
+
+This solution builds on top of “Solution 1”, with the assumption that “Problem” mentioned above is not very common 
+amongst AWS APIs.
+
+In this solution, service teams can specify the spec fields which will be duplicated in both spec and status of custom
+resource using generator.yaml
+
+
+1. *Consistency with K8s conventions* ❌ 
+    1. In K8s convention, the spec fields are not duplicated into status fields. This will confuse k8s users when they 
+    describe their custom resources.
+2. *Consistency across ACK service controllers* ❌
+    1. Since all AWS APIs do not have this “Problem”, behavior across different service controllers will not seem consistent.
+3. *GitOps compatibility* ✅
+    1. There should be no complexity in implementing GitOps for ACK custom resources. 
+    2. Approach mentioned in low level design below should work.
+4. *User experience for custom resource lifecycle* ✅
+    1. Applying updates to custom resource can become confusing for the user.
+    2. To update custom resource spec, K8s user will have to check resource status because status will have most updated
+     value for some of the fields.