GCP BucketPolicyOnly Error on Provisioning

[hasheddan]

What happened?

When creating a Bucket on GCP, validation error occurs on the managed resource when thebucketPolicyOnly field is set. This is due to the lockedTime (metav1.Time) sub-field of bucketPolicyOnly having a zero value of null rather than "".

How can we reproduce it?

1. Create a GCP BucketClass with bucketPolicyOnly defined.
2. Create a Bucket claim that references the class.

What environment did it happen in?

Crossplane version: Master (commit 43aa434)

Error Body

cannot create managed resource: Bucket.storage.gcp.crossplane.io "bucket-f6e19330-065c-49b1-9e7f-977f6ed1f964" is invalid: []: Invalid value: map[string]interface {}{"apiVersion":"storage.gcp.crossplane.io/v1alpha1", "kind":"Bucket", "metadata":map[string]interface {}{"creationTimestamp":"2019-08-27T20:36:59Z", "generation":1, "name":"bucket-f6e19330-065c-49b1-9e7f-977f6ed1f964", "namespace":"crossplane-system", "ownerReferences":[]interface {}{map[string]interface {}{"apiVersion":"storage.crossplane.io/v1alpha1", "kind":"Bucket", "name":"gitlab-artifacts", "uid":"f6e19330-065c-49b1-9e7f-977f6ed1f964"}}, "uid":"95c4267d-e57d-4285-8605-bf13c6ec3a16"}, "spec":map[string]interface {}{"bucketPolicyOnly":map[string]interface {}{"enabled":true, "lockedTime":interface {}(nil)}, "claimRef":map[string]interface {}{"apiVersion":"storage.crossplane.io/v1alpha1", "kind":"Bucket", "name":"gitlab-artifacts", "namespace":"default", "uid":"f6e19330-065c-49b1-9e7f-977f6ed1f964"}, "classRef":map[string]interface {}{"apiVersion":"storage.gcp.crossplane.io/v1alpha1", "kind":"BucketClass", "name":"standard-gcp-bucket", "namespace":"crossplane-system", "uid":"52d3aa4c-4e19-47cf-9e3a-ab0bac25c89a"}, "labels":map[string]interface {}{"app":"gitlab-demo"}, "lifecycle":map[string]interface {}{}, "location":"US", "nameFormat":"gitlab-demo-artifacts-%s", "providerRef":map[string]interface {}{"name":"example", "namespace":"crossplane-system"}, "reclaimPolicy":"Delete", "serviceAccountSecretRef":map[string]interface {}{"name":"demo-gcs-creds"}, "storageClass":"MULTI_REGIONAL", "writeConnectionSecretToRef":map[string]interface {}{"name":"f6e19330-065c-49b1-9e7f-977f6ed1f964"}}}: validation failure list:
spec.bucketPolicyOnly.lockedTime in body must be of type string: "null"

[jbw976]

This may be less pressing than I thought if it only reproduces when bucketPolicyOnly is set. Is it more common to set that field or to leave it unset (and therefore not his this failure)?

[hasheddan]

@jbw976 that is correct. It is hard to say how common it is. It is set on the BucketClass for the Gitlab example, but I am not entirely sure it is necessary. I believe this will be a relatively easy fix, and I will try to address it if I get a moment this sprint. Also, for anyone who is interested in the error, I have added it to the issue body above.

[jbw976]

thanks for the clarity @hasheddan. I included this in the v0.3 milestone in the hopes of fixing it during the final sprint while we are polishing and getting the user guides and experience to a good quality level. I agree that the scope of fixing this isn't large.

[hasheddan]

@muvaf @negz did this end up getting fixed with the v1beta1 updates?

[CarpathianUA]

Experiencing the same

[denist-huma]

@hasheddan I have exactly the same issue you described. It prevents me to make a GCS private bucket with XP, my main use case is not satisfied. I want to have bucket's properties both "not public" and "uniform access control". Currently, I have "subject to object's ACLs" and "fine-grained".

API v1 GCS got rid of "lockedTime" already, see https://cloud.google.com/storage/docs/json_api/v1/buckets/insert

versions

crossplane/provider-gcp:v0.18.0 crossplane/crossplane:v1.4.1

details

storage/browser shows:

my manifest

apiVersion: apiextensions.crossplane.io/v1
kind: Composition
metadata:
  name: compositebuckets.gcp.storage.huma.com
  labels:
    provider: gcp
spec:
  # This Composition declares that it satisfies the CompositeBucket
  # resource defined above - i.e. it patches "from" a CompositeBucket.
  # Note that the version in apiVersion must be the referenceable version of the
  # XRD.
  compositeTypeRef:
    apiVersion: storage.huma.com/v1alpha1
    kind: CompositeBucket
  # When an XR is created in response to a claim Crossplane needs to know where
  # it should create the XR's connection secret. This is configured using the
  # 'writeConnectionSecretsToNamespace' field.
  writeConnectionSecretsToNamespace: crossplane-system
  resources:
    - name: bucket
      base:
        apiVersion: storage.gcp.crossplane.io/v1alpha3
        kind: Bucket
        spec:
          location: EUROPE-WEST2
          storageClass: STANDARD
          versioningEnabled: true
          labels:
            managed-by: crossplane

    - name: sa
      base:
        apiVersion: iam.gcp.crossplane.io/v1alpha1
        kind: ServiceAccount
        spec:
          forProvider:
            description: "Crosspalne managed SA for private bucket access"
      patches:
        - fromFieldPath: "spec.resourceRefs[0].name"
          toFieldPath: "spec.forProvider.displayName"
          transforms:
            - type: string
              string:
                fmt: "%s-sa"

    - name: key
      base:
        apiVersion: iam.gcp.crossplane.io/v1alpha1
        kind: ServiceAccountKey
        spec:
          writeConnectionSecretToRef:
            namespace: crossplane-system
      patches:
        - fromFieldPath: "spec.resourceRefs[1].name"
          toFieldPath: "spec.forProvider.serviceAccountRef.name"
        - fromFieldPath: "metadata.uid"
          toFieldPath: "spec.writeConnectionSecretToRef.name"
          transforms:
            - type: string
              string:
                fmt: "%s-key"
      connectionDetails:
        - fromConnectionSecretKey: privateKey
        - fromConnectionSecretKey: privateKeyType
        - fromConnectionSecretKey: publicKey
        - fromConnectionSecretKey: publicKeyType

    - name: binding
      base:
        apiVersion: storage.gcp.crossplane.io/v1alpha1
        kind: BucketPolicyMember
        metadata:
          name: crossplane-example-bucket-bind-member-to-role
        spec:
          forProvider:
            # https://cloud.google.com/storage/docs/access-control/iam#project-level_roles_vs_bucket-level_roles
            role: roles/storage.objectAdmin
      patches:
        - fromFieldPath: "spec.resourceRefs[0].name"
          toFieldPath: "spec.forProvider.bucketRef.name"
        - fromFieldPath: "spec.resourceRefs[1].name"
          toFieldPath: "spec.forProvider.serviceAccountMemberRef.name"

[ghost]

same for me!

[denist-huma]

@muvaf I saw you worked on provider-gcp, it is easy to fix?

I am thinking to do it myself if this takes reasonable time usually. I haven't contributed to providers yet, but since you have no spare hands, I am willing to pitch in to solve my blocker

Can you please help me to estimate? How long it can take me to learn to dev and ship the fix? I do operator development with Kubebuilder for 5 months now.

[Feggah]

Hey @denist-huma , I think that you would need to make sure that when the spec.ForProvider.BucketPolicyOnly.LockedTime is empty, it should be "" instead of nil, as @hasheddan described in the issue description.

I didn't debug the code to see exactly where it is receiving the nil value, but I would recommend you to investigate further the function https://github.com/crossplane/provider-gcp/blob/c44badd048c7dadce1e799958e269a61938aaa62/apis/storage/v1alpha3/types.go#L743-L752

This functions is responsible to create the bucket (i.e the payload to the API) and its values. As you are defining values in your composition from the struct BucketUpdatableAttrs (versioningEnabled and labels), and the field BucketPolicyOnly is part of this struct, the problem may be in the functions that are copying values from one struct to another (the struct in this provider and the struct from the SDK). As you can see in this lines: https://github.com/crossplane/provider-gcp/blob/c44badd048c7dadce1e799958e269a61938aaa62/apis/storage/v1alpha3/types.go#L552-L560

The easiest way that I think you can check where exactly this nil is coming from is setting a breakpoint in the function CopyBucketSpecAttrs and stepping line by line to see where it is.