crossplane-contrib / provider-upjet-gcp

Official GCP Provider for Crossplane by Upbound.
Apache License 2.0
63 stars 68 forks source link

[Bug]: softDeletePolicy for GCS bucket with latest provider-gcp-storage:v1.5.0 not working (missing in v1beta1) #561

Closed dyurchanka closed 1 month ago

dyurchanka commented 1 month ago

Is there an existing issue for this?

Affected Resource(s)

apiVersion: storage.gcp.upbound.io/v1beta2 kind: Bucket

Resource MRs required to reproduce the bug

No response

Steps to Reproduce

1) Create bucket with softDeletePolicy policy

apiVersion: storage.gcp.upbound.io/v1beta2
kind: Bucket
metadata:
  annotations:
    crossplane.io/external-name: test-soft-delete-bucket
  name: test-soft-delete-bucket
spec:
  deletionPolicy: Delete
  forProvider:
    forceDestroy: true
    location: US-EAST1
    storageClass: STANDARD
    uniformBucketLevelAccess: true
    versioning:
      enabled: false
    softDeletePolicy:
      retentionDurationSeconds: 555
  providerConfigRef:
    name: default

2) Observe state: softDeletePolicy missing in spec and it is trying to use default value 604800

❯ k get bucket test-soft-delete-bucket
NAME                      SYNCED   READY   EXTERNAL-NAME             AGE
test-soft-delete-bucket   False    False   test-soft-delete-bucket   2m35s
❯ k get bucket test-soft-delete-bucket  -o yaml
apiVersion: storage.gcp.upbound.io/v1beta2
kind: Bucket
metadata:
  annotations:
    crossplane.io/external-create-failed: "2024-07-11T06:48:59Z"
    crossplane.io/external-create-pending: "2024-07-11T06:48:59Z"
    crossplane.io/external-create-succeeded: "2024-07-11T06:47:59Z"
    crossplane.io/external-name: test-soft-delete-bucket
    kubectl.kubernetes.io/last-applied-configuration: |
      {"apiVersion":"storage.gcp.upbound.io/v1beta2","kind":"Bucket","metadata":{"annotations":{"crossplane.io/external-name":"test-soft-delete-bucket"},"name":"test-soft-delete-bucket"},"spec":{"deletionPolicy":"Delete","forProvider":{"forceDestroy":true,"location":"US-EAST1","softDeletePolicy":{"retentionDurationSeconds":0},"storageClass":"STANDARD","uniformBucketLevelAccess":true,"versioning":{"enabled":false}},"providerConfigRef":{"name":"default"}}}
  creationTimestamp: "2024-07-11T06:46:59Z"
  finalizers:
  - finalizer.managedresource.crossplane.io
  generation: 1
  name: test-soft-delete-bucket
  resourceVersion: "342420467"
  uid: 9bbb9e89-ce80-48e9-960e-fdaf74186d9b
spec:
  deletionPolicy: Delete
  forProvider:
    forceDestroy: true
    location: US-EAST1
    storageClass: STANDARD
    uniformBucketLevelAccess: true
    versioning:
      enabled: false
  initProvider: {}
  managementPolicies:
  - '*'
  providerConfigRef:
    name: default
status:
  atProvider: {}
  conditions:
  - lastTransitionTime: "2024-07-11T06:47:59Z"
    reason: Creating
    status: "False"
    type: Ready
  - lastTransitionTime: "2024-07-11T06:48:59Z"
    message: 'create failed: async create failed: failed to create the resource: [{0
      googleapi: Error 412: ''604800'' violates constraint ''constraints/storage.softDeletePolicySeconds'',
      conditionNotMet  []}]'
    reason: ReconcileError
    status: "False"
    type: Synced
  - lastTransitionTime: "2024-07-11T06:48:59Z"
    message: 'async create failed: failed to create the resource: [{0 googleapi: Error
      412: ''604800'' violates constraint ''constraints/storage.softDeletePolicySeconds'',
      conditionNotMet  []}]'
    reason: AsyncCreateFailure
    status: "False"
    type: LastAsyncOperation

What happened?

retentionDurationSeconds in softDeletePolicy is ignored. It is not reflected in spec of CR. softDeletePolicy was introduced in v1beta2 and missing in v1beta1. Stored version for this CRD is v1beta1, which missing softDeletePolicy definition in schema

k get crd buckets.storage.gcp.upbound.io -o json | jq '.spec.versions[] | .name, .storage'
"v1beta1"
true
"v1beta2"
false

Relevant Error Output Snippet

No response

Crossplane Version

1.16.0

Provider Version

1.5.0

Kubernetes Version

1.28.11

Kubernetes Distribution

GKE

Additional Info

Please note, that 412 error is because ORG policy, that allows only 0 value for softDeletePolicy retention, ignore it.

Provider info:

❯ k get Provider
NAME                          INSTALLED   HEALTHY   PACKAGE                                                          AGE
provider-gcp-storage          True        True      xpkg.upbound.io/upbound/provider-gcp-storage:v1.5.0              21h
upbound-provider-family-gcp   True        True      xpkg.upbound.io/upbound/provider-family-gcp:v1.5.0               21h
nicolasadeo commented 1 month ago

I have the same issue. StoredVersion is v1beta1 and thus the softDeletePolicy is ignored.

mergenci commented 1 month ago

Interesting 🤔 soft_delete_policy is a singleton list field. It is converted to an object in v1beta2, but it doesn't exist in the v1beta1 schema in the first place. Therefore, conversion to storage version v1beta1 is lossy.

mergenci commented 1 month ago

We discussed the issue with @ulucinar and @turkenf. First of all, this is a known issue. On the surface, the issue may seem fixable by upgrading the storage version. Even though doing so will relieve some of the symptoms, some will remain and others are likely to surface. The fundamental issue is the lack of a lossless conversion mechanism between API versions. Therefore, the storage version doesn't matter.

To address the issue, we will implement conversion webhooks for affected APIs. Ideally, we would like to implement a generic solution, so that appropriate conversion mechanisms can be configured wherever applicable.

mergenci commented 1 month ago

To resolve this issue, we added softDeletePolicy to v1beta1:

https://github.com/crossplane-contrib/provider-upjet-gcp/blob/v1.7.0/apis/storage/v1beta1/zz_bucket_types.go#L301

Conversion is handled by the identity conversion function.