One-pager for ignore changes on updates

Signed-off-by: lsviben <sviben.lovro@gmail.com>

design/one-pager-ignore-changes.md

@@ -0,0 +1,199 @@
+# Ignore field changes
+
+* Owners: Lovro Sviben (@lsviben)
+* Reviewers: Crossplane Maintainers
+* Status: Draft
+
+## Background
+
+When Crossplane manages a resource, it reconciles all of the parameters under `spec.forProvider` to the external providers both 
+during the creation and subsequent updates to the managed resource. But in some cases, a parameter of the external resource
+could change due to it being managed outside of Crossplane. In the current state, if that happens, Crossplane will try to 
+"fix" the situation, and update the parameter back to the one it has in the `spec.forProvider` as it is designed to be
+the only source of truth.
+
+Some examples of this case:
+- [AutoScalingGroup DesireCapacity], where when using an autoscaler, the parameter gets changed 
+externally.
+- [DynamoDB Tables] with using autoscaling ([Policy] + [Target]). The autoscaler scales the read and write capacity,
+and then Crossplane resets it back to the `readCapacity` and `writeCapacity` defined on the Table, which effectively makes autoscaling useless.
+- [NodeGroup] while using a node autoscaler. There is a race condition between crossplane and node autoscaler. 
+Node autoscaler increases the `desiredSize` due to resource requirement but crossplane decrease it to `desiredSize` set in composition.
+
+In these cases, it would make sense to make Crossplane ignore select parameters during updates. 
+Terraform addresses this use case with its [ignore_changes] functionality. 
+
+An additional benefit of ignoring some fields would be that it would be possible to have different Crossplane
+Compositions managing a single external resource. This could open up some interesting new possibilities for Crossplane.
+
+So in this document, the aim will be to introduce a solution in Crossplane for ignoring some managed resource parameters
+during updates.
+
+## Goals
+
+* Design a functionality that allows for ignoring some fields during resource updates.
+* Try and make it as easy as possible to apply the ignoring functionality on existing Crossplane providers. In other words, to 
+design a standard way on how to update the providers code to support `ignore changes`.
+
+## Solution
+
+The idea is to introduce a new `ignoreChanges` string array field in the `spec` of the managed resource, that would work similarly
+to Terraform's [ignore_changes]. The items in the array would be the field paths of the fields that should
+be ignored on updates. 
+
+```yaml
+apiVersion: eks.aws.crossplane.io/v1alpha1
+kind: NodeGroup
+metadata:
+  name: my-group
+spec:
+  ignoreChanges:
+    - scalingConfig.desiredSize
+  forProvider:
+    region: us-east-1
+    clusterNameRef:
+      name: sample-cluster
+    subnetRefs:
+      - name: sample-subnet1
+    nodeRoleRef:
+      name: somenoderole
+    scalingConfig:
+      desiredSize: 1
+      maxSize: 5
+      minSize: 1
+    updateConfig:
+      maxUnavailablePercentage: 50
+      force: true
+  providerConfigRef:
+    name: example
+```
+
+The solution should affect just the Update part of the Crossplane lifecycle, so create and late initialization should work 
+as before, even though a field is marked under `ignoreChanges`. Additionally, as Crossplane will soon support `status.atProvider`,
+users will be able to observe the actual values of the ignored fields.
+
+#### Ignoring required fields
+
+Required fields should not be put into `ignoreChanges`, as that could compromise the functionality of the provider. If such
+a need arises, it should be handled as a special case on the provider side, and not through the `ignoreChanges` feature.
+
+Of course, some fields could be marked as required, but they are actually required only for create. As there currently is
+no standard on differentiating them, it is advised to use `ignoreChanges` on required fields only if certain that it's not
+required for update.
+
+### Implementation
+
+The implementation would be done on the provider level, so we would need to update all the providers over time to use the 
+new field. 
+
+For providers using Upjet the solution will be to leverage Terraform's [ignore_changes] lifecycle field. So Upjet
+generation should be updated with a step that transfers the `spec.ignoreChanges` field into Terraform's `ignore_changes`.
+Something like this in the Upjet method that writes the main TF file:
+
+```go
+// WriteMainTF writes the content main configuration file that has the desired
+// state configuration for Terraform.
+func (fp *FileProducer) WriteMainTF() error {
+
+    fp.parameters["lifecycle"] = map[string]interface{}{
+        // If the resource is in a deletion process, we need to remove the deletion
+        // protection.
+        "prevent_destroy": !meta.WasDeleted(fp.Resource),
+        // Add fields which should be ignored on updates.
+        "ignore_changes": fp.Resource.GetIgnored(),
+    }
+...
+```
+
+For other providers we can implement a helper function in the [crossplane-runtime] repo that would use the existing code
+in [crossplane-runtime/fieldpath] to unset the fields of a managed resource that are under `ignoreChanges`. That way
+provider maintainers could use this function in the Update method without the need to invent their own solution.
+
+Helper function would look something this (WIP):
+```go
+func UnsetIgnored(managed resource.Managed) error {
+	p, err := PaveObject(managed)
+	if err != nil {
+		return err
+	}
+
+	for _, s := range managed.GetIgnored() {
+		err = p.DeleteField(s)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+```
+
+#### Feature Gating
+
+Similar to all other new features being added to Crossplane, we will ship this
+new policy as an alpha feature that will be off by default and will be
+controlled by the `--enable-alpha-ignore-changes` flag in Providers.
+
+This will not prevent the field from appearing in the schema of the managed
+resources. However, we will ignore the `spec.ignoreChanges` when the feature
+is not enabled.
+
+### Implementation alternative considered
+
+An alternative was considered where the above helper function which unsets fields in the managed resource would be used
+directly in the [crossplane-runtime] reconciler, and then a version of the managed resource without the ignored fields
+would be passed to the Update method.
+
+While this solution would enable the functionality without the need to change all the providers, it has a few 
+drawbacks:
+* It doesn't allow for flexibility or handling special cases in the provider code, as the providers will just receive a 
+managed resource which does not have the ignored fields set. There could be cases where the wish is to not update the field
+on the external resource, but is useful in some other way.
+* Won't work well with Upjet as it uses Terraform underneath which would work better with its native [ignore_changes].
+
+## Solution alternative considered
+
+The idea here was to do something similar to K8s `initContainers` and add a field `initProvider` to the managed resources.
+The parameters under `initProvider` would be used only during creation.
+
+```yaml
+apiVersion: eks.aws.crossplane.io/v1alpha1
+kind: NodeGroup
+metadata:
+  name: my-group
+spec:
+  initProvider:
+    scalingConfig:
+      scalingConfig.desiredSize: 1
+  forProvider:
+    region: us-east-1
+    clusterNameRef:
+      name: sample-cluster
+    subnetRefs:
+      - name: sample-subnet1
+    nodeRoleRef:
+      name: somenoderole
+    scalingConfig:
+      maxSize: 5
+      minSize: 1
+    updateConfig:
+      maxUnavailablePercentage: 50
+      force: true
+  providerConfigRef:
+    name: example
+```
+
+The great thing about this solution is that it feels natural to add `initProvider` as we already have `forProvider`,
+`atProvider`. But on the other hand a thing that would be bothersome with this solution is that a value is needed in the 
+`initProvider`. A field cannot just be marked to be ignored, without setting the initial value.
+
+One more thing to note here is how would this solution interact with *late initialization*, which sets some of the `forProvider`
+fields. To avoid conflicts, the fields that are in `initProviders` would be ignored during *late initialize*
+
+[AutoScalingGroup DesireCapacity]: https://doc.crds.dev/github.com/upbound/provider-aws/autoscaling.aws.upbound.io/AutoscalingGroup/v1beta1@v0.21.0#spec-forProvider-desiredCapacity
+[DynamoDB Tables]: https://doc.crds.dev/github.com/upbound/provider-aws/dynamodb.aws.upbound.io/Table/v1beta1@v0.24.0
+[Policy]: https://doc.crds.dev/github.com/upbound/provider-aws/appautoscaling.aws.upbound.io/Policy/v1beta1@v0.24.0
+[Target]: https://doc.crds.dev/github.com/upbound/provider-aws/appautoscaling.aws.upbound.io/Target/v1beta1@v0.24.0
+[NodeGroup]: https://doc.crds.dev/github.com/upbound/provider-aws/eks.aws.upbound.io/NodeGroup/v1beta1@v0.24.0
+[ignore_changes]: https://developer.hashicorp.com/terraform/language/meta-arguments/lifecycle#ignore_changes
+[crossplane-runtime]: https://github.com/crossplane/crossplane-runtime/
+[crossplane-runtime/fieldpath]: https://github.com/crossplane/crossplane-runtime/tree/master/pkg/fieldpath