diff --git a/docs/development/external-name-handling.md b/docs/development/external-name-handling.md new file mode 100644 index 00000000..76c153b2 --- /dev/null +++ b/docs/development/external-name-handling.md @@ -0,0 +1,110 @@ +# External Name Handling + +## Context +`crossplane.io/external-name` is an annotation for Managed Resources. Across providers we lack a unified definition and implementation guideline. This ADR proposes a consistent definition, expected behavior, and implementation approach. + +## Background + +### Definitions +Common interpretations of the external name: +1. Human-facing name of the resource shown in the external provider UI. [1] +2. Unique identifier generated by the external system (GUID). [2] + +### Default Behavior in Crossplane +- By default, the external name is initialized (by crossplane-runtime) to the Kubernetes `metadata.name` of the Managed Resource (MR) if not explicitly set. +- This default conflicts with cases where the external system uses a different unique identifier. + +### Importing Existing External Resources (Official Flow) [3] +1. Create an MR manifest with `spec.managementPolicies: Observe`. +2. Add the external-name annotation with the external resource identifier. If not globally unique, also supply distinguishing `spec.forProvider` fields. +3. Apply the MR. Controller populates `status.atProvider`. +4. Copy the fields from `status.atProvider` to `spec.forProvider` and change `managementPolicies` to `*` to gain full control. + +## Our Approach + +### Definition +External-name represents the unique identifier given by the external-system. If this does not exist, it is a compound key of values from spec.forProvider that uniquely identify the external resource. This annotation is used for API Get requests (or filter List requests). + +In Create(), the external name is set based on the response of the API (if possible). Observe() use the external-name to match the external resource, Update() and Delete() also use the external-name. [4] +This is easy in the case of a unique identifier. If we have a compound key, we deconstruct the compound key to perform the API requests. + +### Special Case: defaulting of external-name +If external-name is unset, crossplane-runtime will run a default initializer that will set the external name to metadata.name before the first Observe() is called. + +This poses two problems: +- If we set any kind of initializer ourselves later, we don’t get the default initializer automatically and therefore might run into bugs if we don’t check for empty initializer +- At this moment, the finalizer would break our definition as the metadata.name is only an identifier within the Kubernetes cluster, not external system. + +**TODO: how do we handle this?** + +### Implementation guideline + +#### Observe() +1. Check if external-name is empty. + - If we need backwards compatibility, we create our API query from the spec.forProvider fields. + - If resource exists we perform the normal needsCreation/needsUpdate and set the external name to the correct value. + - If resource does not exist, it needsCreation. + - Else no backwards compatibility needed, return resourceExist: false. +2. External-name is set, we check its format. + - If format is not correct (not a valid GUID or not a valid compound key): return error. +3. Build the Get API Request from the external-name (by either parsing the compound key or using the GUID directly, do not use the spec.forProvider). + - If resource is not found, that is a drift, not an error case, we therefore return resourceExist: false to trigger a Create() call. + - Else, updateObservation and perform needsCreate()/needsUpdate() check as usual. + +#### Create() +We know that the resource does not exist so we perform the Create API Request and store the necessary GUID or compound key from the API response/spec.ForProvider. The case that we don’t have all the necessary information to set the key can not happen because than we would not be able to check the resource for being created. + +If the response is creation failed – resource already exist, we treat it as an error. In the next reconcile Observe(), the resource will be shown as resourceExist: true. + +#### Update() +If the diff contains a non-updateable field, we return error without executing the update, staying in an error loop until the user fixes the wrong state. This would put the resource into Ready: false, representing the inability to correct the drift. + +Perform the UPDATE request, return error if it returns error. + +If the external-name is a compound key, update the compound key. This is necessary if parts of the compound key were updated. + +#### Delete() +If our external resource has a field indicating it is in deletion state, we return. + +We use external-name to perform DELETE request. + +If the response is 404 – not found (or the equivalent API response for this API), we do NOT treat this as an error. This only happens when the resource was externally removed in the meantime since after Observe(), Delete() is not called if the resource does not exist. Technically we have a drift in here but it can be safely ignored since the drift already covers the desired state. + +If our delete request is successful, we return EVEN IF the deletion operation is performed async by the external system (in the next reconcile Observe(), the resourceExist field will be checked by crossplane-runtime and Delete() is called again if the resource still exists). Should the resource still exist in next reconcile, Delete() is triggered again but we check for the deletion state in the external system (= status.observation being something like `DELETING` or so) so we don’t perform another Delete(). If this is not possible, we have to perform another DELETE API call, treat a possible error response as error and return it. Once the resource is deleted in the external system, the Observe() function will determine resourceExist: false and crossplane-runtime removed the MR. + +Else, if we have an error in our request, return with error. + +### Importing of existing external resources +For importing, setting the right external-name will match the external resource no matter the values in `spec.ForProvider`. + +If we set the `spec.forProvider` as required, the user can not create managed resources without those values and therefore must also set the required fields. The official import guide [3] talks about setting fields that are required for unique matching but the validation rules might require more fields. On the other hand, setting it as optional would allow to Observe an external resource and deleting them (using the managementPolicies: Observe, Delete) without having to fill out the spec. + +This is not super important for us right now since we plan to have an export cli to create the spec for us but might help to have a more common behaviour accross our providers. + +Since the matching between MR and external resource is done based on external-name, a wrongly filled spec would result in Update() calls to the resource changing it. To prevent this, the user would not set the managementPolicy to Update. If there is then a missmatch, the Observe() method would determines this, sets the externalObservation.Diff and it would be visible in the debug log. TODO: this should be put as a warning event on the resource maybe, because crossplane-runtime only does a log.Debug with the Diff. As a user adopting existing landscapes it would be good to know if the current spec WOULD trigger an update/diff for confidence. + +## Open Questions, Things not considered +These might not be important for our APIs if we don’t have this scenarios. + +### ID can’t be determined in Create() +Maybe the ID can not be determined in Create() after the API request, maybe it only has a job id/temporal id and later gets a real GUID, the external name needs to be swapped. + +### external-name compound key +The delimiter can collide with the field values if they can be in there too. Case sensitive/insensitive matching, leading/trailing spaces. Length limit of annotation, maybe base64 encoded parts of the compound key to avoid delimiter collisions (or escaping of the delimiter character). + +Also error case if the compound key matches more than one external resource needs to be handled if a List API is used. + +### External-name annotation manually edited +Observe() would return a not found and the resource would be created again with the same `spec.forProvider`. This would probably lead to a creation error. Is this what we want? + +### Export tool considerations + +The export tool must generate managed resource definitions based on external resource state retrieved through the external system API. To ensure these definitions can be imported successfully, the tool must generate proper `crossplane.io/external-name` annotations. + +The export tool **must** be able to derive the `crossplane.io/external-name` annotation value from information obtained through standard API operations (GET, LIST) on external resources. + +## References +[1] https://github.com/crossplane/crossplane/blob/main/design/one-pager-managed-resource-api-design.md?plain=1#L151 +[2] https://github.com/crossplane/crossplane/issues/1640 +[3] https://docs.crossplane.io/latest/guides/import-existing-resources/ +[4] https://github.com/SAP/crossplane-provider-cloudfoundry/blob/main/docs/resource-names.md#crossplaneioexternal-name-annotation