Drop v1alpha1 or add conversion webhooks

[jcooklin]

Drop v1alpha1 or add conversion webhooks

[jcooklin]

If we were voting I would +1 both:

• Drop v1alpha1 as the issue title suggests - tested, and it works
• Implement a migration in Crossplane, which would update the API version in the managed fields with the composed API version

🚀

[haarchri]

We faced a problem with the provider-helm in one environment, we using a few old v1alpha1 releases.
In the past we introduced v1beta1 releases and later introduced new fields like skipCreateNamespace, which were not implemented into v1alpha1. This has become now an issue issue since v1alpha1 releases are still in etcd. We updated the resources to v1beta1 in their composition, but an automatic storage version migration in etcd did not happen alone ...
As a result, we have now facing an error: failed to convert new object: .spec.forProvider.skipCreateNamespace: field not declared in schema Do we have plans to remove v1alpha1 and add a storage version migration like in crossplane ? crossplane/crossplane#4402 cc: @turkenh

[haarchri]

kubectl get --raw /apis/helm.crossplane.io/v1alpha1/releases/xyz | jq
{
  "apiVersion": "helm.crossplane.io/v1alpha1",
  "kind": "Release",
  "metadata": {
    "annotations": {
      "crossplane.io/composition-resource-name": "xyz",
      "crossplane.io/external-create-pending": "2024-03-12T22:29:06Z",
      "crossplane.io/external-create-succeeded": "2024-03-12T22:29:08Z",
      "crossplane.io/external-name": "xyz"
    },
    "creationTimestamp": "2024-03-12T22:29:06Z",
    "finalizers": [
      "finalizer.managedresource.crossplane.io"
    ],

[turkenh]

IIUC, the problem here is different than what crossplane/crossplane#4402 solves. We bumped from v1alpha1 to v1beta1 and marked v1beta1 as the storage version and have both versions in the version list different than what was happening on Crossplane side.

I suspect the problem here is still having v1alpha1 in the resource ref of composites. The Releases stored as v1beta1 in the cluster are tried to be read as v1alpha1 by the composite controller due to that reference, and a conversion from v1beta1 to v1alpha1 fails. Can you check if this is the case, i.e., resourceRefs still has v1alpha1 of this resource? Indeed, I would expect Crossplane to update the referenced version once the composed resources are bumped 🤔

[haarchri]

The info was more that if we drop v1alpha1 we need to do something - we see in resourceRefs v1beta1 - so something is strange with fields we only have in v1beta1 when the resource was initial created as v1alpha1 - i guess in etcd the resource is still v1alpha1 ?

[haarchri]

Resource Refs:
    API Version:  helm.crossplane.io/v1beta1
    Kind:         Release
    Name:         nrf777-lr8td-zcnjz
...
Status:
  Conditions:
    Last Transition Time:  2024-03-22T22:50:52Z
    Message:               cannot compose resources: cannot apply composed resource "spotinstance-controller": failed to convert new object: .spec.forProvider.skipCreateNamespace: field not declared in schema
    Reason:                ReconcileError
    Status:                False
    Type:                  Synced

[turkenh]

i guess in etcd the resource is still v1alpha1 ?

Assuming a recent enough provider-helm is running, I would be surprised if that is the case since we are marking v1beta1 as the storage version.

Having some reproduction steps would be helpful to dig further on my side, currently, I don't have much idea on what is going on TBH.

[haarchri]

@turkenh remember we set storage version v1beta1 lot of releases ago - what happen with users still using v1alpha1 before this time and want to switch to v1beta1 ?

[sttts]

This looks like an instance of the misunderstanding of Kube versions in the helm provider types, i.e. what I tweeted in https://twitter.com/the_sttts/status/1771257423614836820.

In code, these forProviders fields diverged in v1alpha1 and v1beta1:

https://github.com/crossplane-contrib/provider-helm/blob/master/apis/release/v1beta1/types.go#L77
https://github.com/crossplane-contrib/provider-helm/blob/master/apis/release/v1alpha1/types.go#L70

This is "forbidden" in Kube API versions.

[turkenh]

Probably I am missing something, but I didn't understand why we would need multiple versions if there won't be a divergence 🤔

I remember recently I worked on the following, was it also wrong?

[turkenh]

We had an async chat with @sttts, and now I have a better understanding of what he means.

For this specific case, though, I would like to better understand what is really going on before taking an action.
So, I would appreciate some steps to reproduce the issue (if possible) before we talk about the solutions.

[haarchri]

https://github.com/haarchri/issue-helm-213

create release first with v1alpha1 https://github.com/haarchri/issue-helm-213/blob/main/apis/composition.yaml#L22
then change to v1beta1 and add a field which is only available in v1beta1 https://github.com/haarchri/issue-helm-213/blob/main/apis/composition-update.yaml#L22 and https://github.com/haarchri/issue-helm-213/blob/main/apis/composition-update.yaml#L31

reproduce the issue:

./setup.sh
wait that the release is ready/synced true 
kubectl apply -f apis/composition-update.yaml
kubectl describe xacmedatabases
...
  Resource Refs:
    API Version:  helm.crossplane.io/v1beta1
    Kind:         Release
    Name:         acme-db-prod-b64nn-nxqnk
...
Status:
  Conditions:
    Last Transition Time:  2024-03-25T14:26:22Z
    Message:               cannot compose resources: cannot apply composed resource "test": failed to convert merged object to last applied version: .spec.forProvider.skipCreateNamespace: field not declared in schema

[turkenh]

Thanks @haarchri!

I believe this is something happening with function pipeline only which uses Server Side Apply.

In the release object managed fields, I see we still have v1alpha1 as the version for composed manager, which was persisted when the object was applied for the first time:

apiVersion: helm.crossplane.io/v1beta1
kind: Release
metadata:
  ...
  managedFields:
  - apiVersion: helm.crossplane.io/v1alpha1
    fieldsType: FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          f:crossplane.io/composition-resource-name: {}
        f:generateName: {}
        f:labels:
          f:crossplane.io/claim-name: {}
          f:crossplane.io/claim-namespace: {}
          f:crossplane.io/composite: {}
        f:ownerReferences:
          k:{"uid":"7b28334e-bdea-4f5d-825e-856c343ee931"}: {}
      f:spec:
        f:forProvider:
          f:chart:
            f:name: {}
            f:repository: {}
            f:version: {}
          f:namespace: {}
    manager: apiextensions.crossplane.io/composed/3231d80411f15ed19fe66e36da324b1ced4cf15c0f94724e4afd3e7db990005d
    operation: Apply
    time: "2024-03-26T06:41:32Z"
   ...

I see the following options here:

1. Make sure all versions are convertible between each other, as @sttts pointed out above.
2. Implement a migration in Crossplane, which would update the API version in the managed fields with the composed API version.
3. Drop v1alpha1 as the issue title suggests - tested, and it works.

None of the options are exclusive to each other, but any of them would fix the issue.
Having a migration in Crossplane would be beneficial in any case, as there is no reason to keep the old version there once we start working with the new version.

A 4th option is running a manual migration as follows (which could be used as a workaround):

# assuming it is the 0th element, not sure if it is always the case. Update the index otherwise.
kubectl patch releases.helm.crossplane.io <release-name> --type='json' -p='[{"op": "replace", "path": "/metadata/managedFields/0/apiVersion", "value": "helm.crossplane.io/v1beta1"}]'