From c3f0076a330ace04ac16888bcbc2a5b77f8093e1 Mon Sep 17 00:00:00 2001 From: Njal Karevoll Date: Mon, 21 Nov 2022 11:33:09 +0100 Subject: [PATCH] Allow Bucket.PublicAccessPrevention to be fully optional and managed independently Signed-off-by: Njal Karevoll --- apis/storage/v1alpha3/types.go | 27 ++++++++++++++----- .../storage/v1alpha3/zz_generated.deepcopy.go | 5 ++++ .../storage.gcp.crossplane.io_buckets.yaml | 1 - pkg/controller/storage/bucket.go | 11 +++++++- 4 files changed, 36 insertions(+), 8 deletions(-) diff --git a/apis/storage/v1alpha3/types.go b/apis/storage/v1alpha3/types.go index 3831f8b42..6ab81d728 100644 --- a/apis/storage/v1alpha3/types.go +++ b/apis/storage/v1alpha3/types.go @@ -22,6 +22,7 @@ import ( "cloud.google.com/go/storage" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + gcp "github.com/crossplane-contrib/provider-gcp/pkg/clients" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" ) @@ -605,8 +606,7 @@ type BucketUpdatableAttrs struct { // // +optional // +kubebuilder:validation:Enum="";unspecified;inherited;enforced - // +kubebuilder:default:="" - PublicAccessPrevention string `json:"publicAccessPrevention,omitempty"` + PublicAccessPrevention *string `json:"publicAccessPrevention,omitempty"` // RequesterPays reports whether the bucket is a Requester Pays bucket. // Clients performing operations on Requester Pays buckets must provide @@ -646,7 +646,7 @@ func NewBucketUpdatableAttrs(ba *storage.BucketAttrs) *BucketUpdatableAttrs { Logging: NewBucketLogging(ba.Logging), PredefinedACL: ba.PredefinedACL, PredefinedDefaultObjectACL: ba.PredefinedDefaultObjectACL, - PublicAccessPrevention: ba.PublicAccessPrevention.String(), + PublicAccessPrevention: convertPublicAccessPreventionEnumToStringPtr(ba.PublicAccessPrevention), RequesterPays: ba.RequesterPays, RetentionPolicy: NewRetentionPolicy(ba.RetentionPolicy), VersioningEnabled: ba.VersioningEnabled, @@ -656,8 +656,13 @@ func NewBucketUpdatableAttrs(ba *storage.BucketAttrs) *BucketUpdatableAttrs { // convertPublicAccessPreventionStringToEnum converts a string representation of storage.PublicAccessPrevention to its // enum value. -func convertPublicAccessPreventionStringToEnum(pap string) storage.PublicAccessPrevention { - switch pap { +func convertPublicAccessPreventionStringToEnum(pap *string) storage.PublicAccessPrevention { + // if the field is not set, treat it as unknown + if pap == nil { + return storage.PublicAccessPreventionUnknown + } + + switch *pap { case "unspecified", "inherited": return storage.PublicAccessPreventionInherited case "enforced": @@ -667,6 +672,16 @@ func convertPublicAccessPreventionStringToEnum(pap string) storage.PublicAccessP } } +// convertPublicAccessPreventionEnumToStringPtr converts an enum value of storage.PublicAccessPrevention to its +// string pointer value used in BucketUpdatableAttrs. +func convertPublicAccessPreventionEnumToStringPtr(pap storage.PublicAccessPrevention) *string { + if pap == storage.PublicAccessPreventionUnknown { + return nil + } + + return gcp.StringPtr(pap.String()) +} + // CopyToBucketAttrs create a copy in storage format func CopyToBucketAttrs(ba *BucketUpdatableAttrs) *storage.BucketAttrs { if ba == nil { @@ -750,7 +765,7 @@ type BucketSpecAttrs struct { StorageClass string `json:"storageClass,omitempty"` } -// NewBucketSpecAttrs create new instance from storage BuckateAttrs +// NewBucketSpecAttrs create new instance from storage.BucketAttrs func NewBucketSpecAttrs(ba *storage.BucketAttrs) BucketSpecAttrs { if ba == nil { return BucketSpecAttrs{} diff --git a/apis/storage/v1alpha3/zz_generated.deepcopy.go b/apis/storage/v1alpha3/zz_generated.deepcopy.go index 5a37b8128..f6d4ac701 100644 --- a/apis/storage/v1alpha3/zz_generated.deepcopy.go +++ b/apis/storage/v1alpha3/zz_generated.deepcopy.go @@ -292,6 +292,11 @@ func (in *BucketUpdatableAttrs) DeepCopyInto(out *BucketUpdatableAttrs) { *out = new(BucketLogging) **out = **in } + if in.PublicAccessPrevention != nil { + in, out := &in.PublicAccessPrevention, &out.PublicAccessPrevention + *out = new(string) + **out = **in + } if in.RetentionPolicy != nil { in, out := &in.RetentionPolicy, &out.RetentionPolicy *out = new(RetentionPolicy) diff --git a/package/crds/storage.gcp.crossplane.io_buckets.yaml b/package/crds/storage.gcp.crossplane.io_buckets.yaml index 3564fa9cd..9169b0713 100644 --- a/package/crds/storage.gcp.crossplane.io_buckets.yaml +++ b/package/crds/storage.gcp.crossplane.io_buckets.yaml @@ -400,7 +400,6 @@ spec: - name type: object publicAccessPrevention: - default: "" description: PublicAccessPrevention is the setting for the bucket's PublicAccessPrevention policy, which can be used to prevent public access of data in the bucket. See https://cloud.google.com/storage/docs/public-access-prevention diff --git a/pkg/controller/storage/bucket.go b/pkg/controller/storage/bucket.go index 1d227184e..5751d3e18 100644 --- a/pkg/controller/storage/bucket.go +++ b/pkg/controller/storage/bucket.go @@ -142,7 +142,16 @@ func (e *external) Observe(ctx context.Context, mg resource.Managed) (managed.Ex } proposed := cr.Spec.BucketSpecAttrs.DeepCopy() - if err := mergo.Merge(proposed, v1alpha3.NewBucketSpecAttrs(a)); err != nil { + bsa := v1alpha3.NewBucketSpecAttrs(a) + + // If the spec has no value set for the PublicAccessPrevention field, ignore the one stored in GCP API for the + // purposes of comparison. This allows public access prevention to be managed in the GCP console independently of + // the Bucket CR if the field is not set. + if proposed.PublicAccessPrevention == nil { + bsa.PublicAccessPrevention = nil + } + + if err := mergo.Merge(proposed, bsa); err != nil { return managed.ExternalObservation{}, errors.Wrap(err, errLateInit) } if !cmp.Equal(*proposed, cr.Spec.BucketSpecAttrs) {