From 9a7a9de07971f98a6db155717c08c8efe7967579 Mon Sep 17 00:00:00 2001 From: Brenna Epp Date: Wed, 29 Sep 2021 13:59:19 -0500 Subject: [PATCH 1/5] fix(storage): update PAP to use inherited instead of unspecified --- storage/bucket.go | 15 ++++++++++----- storage/bucket_test.go | 15 +++++++++++++-- storage/integration_test.go | 13 ++++++------- 3 files changed, 29 insertions(+), 14 deletions(-) diff --git a/storage/bucket.go b/storage/bucket.go index f2a59f3b3d1..f810a3428c9 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -376,8 +376,12 @@ const ( // not set in a call to GCS. PublicAccessPreventionUnknown PublicAccessPrevention = iota - // PublicAccessPreventionUnspecified corresponds to a value of "unspecified" + // PublicAccessPreventionInherited corresponds to a value of "inherited" // and is the default for buckets. + PublicAccessPreventionInherited + + // PublicAccessPreventionUnspecified corresponds to a value of "inherited". + // Deprecated: use PublicAccessPreventionInherited PublicAccessPreventionUnspecified // PublicAccessPreventionEnforced corresponds to a value of "enforced". This @@ -385,14 +389,15 @@ const ( PublicAccessPreventionEnforced publicAccessPreventionUnknown string = "" + publicAccessPreventionInherited = "inherited" publicAccessPreventionUnspecified = "unspecified" publicAccessPreventionEnforced = "enforced" ) func (p PublicAccessPrevention) String() string { switch p { - case PublicAccessPreventionUnspecified: - return publicAccessPreventionUnspecified + case PublicAccessPreventionInherited, PublicAccessPreventionUnspecified: + return publicAccessPreventionInherited case PublicAccessPreventionEnforced: return publicAccessPreventionEnforced default: @@ -1212,8 +1217,8 @@ func toPublicAccessPrevention(b *raw.BucketIamConfiguration) PublicAccessPrevent return PublicAccessPreventionUnknown } switch b.PublicAccessPrevention { - case publicAccessPreventionUnspecified: - return PublicAccessPreventionUnspecified + case publicAccessPreventionInherited, publicAccessPreventionUnspecified: + return PublicAccessPreventionInherited case publicAccessPreventionEnforced: return PublicAccessPreventionEnforced default: diff --git a/storage/bucket_test.go b/storage/bucket_test.go index 3919853e731..a0780b01249 100644 --- a/storage/bucket_test.go +++ b/storage/bucket_test.go @@ -261,7 +261,18 @@ func TestBucketAttrsToRawBucket(t *testing.T) { attrs.PublicAccessPrevention = PublicAccessPreventionUnspecified got = attrs.toRawBucket() want.IamConfiguration = &raw.BucketIamConfiguration{ - PublicAccessPrevention: "unspecified", + PublicAccessPrevention: "inherited", + } + if msg := testutil.Diff(got, want); msg != "" { + t.Errorf(msg) + } + + // Test that setting PublicAccessPrevention to "inherited" leads to the + // setting being propagated in the proto. + attrs.PublicAccessPrevention = PublicAccessPreventionInherited + got = attrs.toRawBucket() + want.IamConfiguration = &raw.BucketIamConfiguration{ + PublicAccessPrevention: "inherited", } if msg := testutil.Diff(got, want); msg != "" { t.Errorf(msg) @@ -274,7 +285,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) { UniformBucketLevelAccess: &raw.BucketIamConfigurationUniformBucketLevelAccess{ Enabled: true, }, - PublicAccessPrevention: "unspecified", + PublicAccessPrevention: "inherited", } if msg := testutil.Diff(got, want); msg != "" { t.Errorf(msg) diff --git a/storage/integration_test.go b/storage/integration_test.go index e608034fb10..4f7d9127481 100644 --- a/storage/integration_test.go +++ b/storage/integration_test.go @@ -645,15 +645,14 @@ func TestIntegration_PublicAccessPrevention(t *testing.T) { if err := a.Set(ctx, AllUsers, RoleReader); err == nil { t.Error("ACL.Set: expected adding AllUsers ACL to object should fail") } - t.Skip("https://github.com/googleapis/google-cloud-go/issues/4890") - // Update PAP setting to unspecified should work and not affect UBLA setting. - attrs, err := bkt.Update(ctx, BucketAttrsToUpdate{PublicAccessPrevention: PublicAccessPreventionUnspecified}) + // Update PAP setting to inherited should work and not affect UBLA setting. + attrs, err := bkt.Update(ctx, BucketAttrsToUpdate{PublicAccessPrevention: PublicAccessPreventionInherited}) if err != nil { t.Fatalf("updating PublicAccessPrevention failed: %v", err) } - if attrs.PublicAccessPrevention != PublicAccessPreventionUnspecified { - t.Errorf("updating PublicAccessPrevention: got %s, want %s", attrs.PublicAccessPrevention, PublicAccessPreventionUnspecified) + if attrs.PublicAccessPrevention != PublicAccessPreventionInherited { + t.Errorf("updating PublicAccessPrevention: got %s, want %s", attrs.PublicAccessPrevention, PublicAccessPreventionInherited) } if attrs.UniformBucketLevelAccess.Enabled || attrs.BucketPolicyOnly.Enabled { t.Error("updating PublicAccessPrevention changed UBLA setting") @@ -689,8 +688,8 @@ func TestIntegration_PublicAccessPrevention(t *testing.T) { if !attrs.UniformBucketLevelAccess.Enabled { t.Error("updating UBLA: got UBLA not enabled, want enabled") } - if attrs.PublicAccessPrevention != PublicAccessPreventionUnspecified { - t.Errorf("updating UBLA: got %s, want %s", attrs.PublicAccessPrevention, PublicAccessPreventionUnspecified) + if attrs.PublicAccessPrevention != PublicAccessPreventionInherited { + t.Errorf("updating UBLA: got %s, want %s", attrs.PublicAccessPrevention, PublicAccessPreventionInherited) } } From 5f894079cf55a455807b2e9a24e087dd96fe6e66 Mon Sep 17 00:00:00 2001 From: Brenna Epp Date: Wed, 29 Sep 2021 14:31:28 -0500 Subject: [PATCH 2/5] fix enum --- storage/bucket.go | 8 ++++---- storage/bucket_test.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/storage/bucket.go b/storage/bucket.go index f810a3428c9..83539c7a402 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -376,10 +376,6 @@ const ( // not set in a call to GCS. PublicAccessPreventionUnknown PublicAccessPrevention = iota - // PublicAccessPreventionInherited corresponds to a value of "inherited" - // and is the default for buckets. - PublicAccessPreventionInherited - // PublicAccessPreventionUnspecified corresponds to a value of "inherited". // Deprecated: use PublicAccessPreventionInherited PublicAccessPreventionUnspecified @@ -388,6 +384,10 @@ const ( // enforces Public Access Prevention on the bucket. PublicAccessPreventionEnforced + // PublicAccessPreventionInherited corresponds to a value of "inherited" + // and is the default for buckets. + PublicAccessPreventionInherited + publicAccessPreventionUnknown string = "" publicAccessPreventionInherited = "inherited" publicAccessPreventionUnspecified = "unspecified" diff --git a/storage/bucket_test.go b/storage/bucket_test.go index a0780b01249..f78430d2240 100644 --- a/storage/bucket_test.go +++ b/storage/bucket_test.go @@ -257,7 +257,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) { } // Test that setting PublicAccessPrevention to "unspecified" leads to the - // setting being propagated in the proto. + // undepecrated setting being propagated in the proto. attrs.PublicAccessPrevention = PublicAccessPreventionUnspecified got = attrs.toRawBucket() want.IamConfiguration = &raw.BucketIamConfiguration{ From 84973c4473057be729b5fc1fd799bcf2b2e6dc7b Mon Sep 17 00:00:00 2001 From: Brenna Epp Date: Wed, 29 Sep 2021 14:34:25 -0500 Subject: [PATCH 3/5] add todo --- storage/bucket.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/storage/bucket.go b/storage/bucket.go index 83539c7a402..b78412ab592 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -388,10 +388,11 @@ const ( // and is the default for buckets. PublicAccessPreventionInherited - publicAccessPreventionUnknown string = "" - publicAccessPreventionInherited = "inherited" - publicAccessPreventionUnspecified = "unspecified" - publicAccessPreventionEnforced = "enforced" + publicAccessPreventionUnknown string = "" + // TODO: remove unspecified when change is fully completed + publicAccessPreventionUnspecified = "unspecified" + publicAccessPreventionEnforced = "enforced" + publicAccessPreventionInherited = "inherited" ) func (p PublicAccessPrevention) String() string { From 46fb45d26fa396140f923cc33be4a1469df8115c Mon Sep 17 00:00:00 2001 From: Brenna Epp Date: Wed, 29 Sep 2021 14:35:30 -0500 Subject: [PATCH 4/5] typo --- storage/bucket_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/bucket_test.go b/storage/bucket_test.go index f78430d2240..4e1c1e013ef 100644 --- a/storage/bucket_test.go +++ b/storage/bucket_test.go @@ -257,7 +257,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) { } // Test that setting PublicAccessPrevention to "unspecified" leads to the - // undepecrated setting being propagated in the proto. + // undeprecrated setting being propagated in the proto. attrs.PublicAccessPrevention = PublicAccessPreventionUnspecified got = attrs.toRawBucket() want.IamConfiguration = &raw.BucketIamConfiguration{ From 7bcf2b4c7197a3ea6768f97f5b90b5b4f3155eb6 Mon Sep 17 00:00:00 2001 From: Brenna Epp Date: Wed, 6 Oct 2021 10:24:25 -0700 Subject: [PATCH 5/5] fix comments --- storage/bucket.go | 2 +- storage/bucket_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/storage/bucket.go b/storage/bucket.go index 3b9f8284e45..bfcddd5aa3f 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -378,7 +378,7 @@ const ( // not set in a call to GCS. PublicAccessPreventionUnknown PublicAccessPrevention = iota - // PublicAccessPreventionUnspecified corresponds to a value of "inherited". + // PublicAccessPreventionUnspecified corresponds to a value of "unspecified". // Deprecated: use PublicAccessPreventionInherited PublicAccessPreventionUnspecified diff --git a/storage/bucket_test.go b/storage/bucket_test.go index 4e1c1e013ef..0e65e8becdf 100644 --- a/storage/bucket_test.go +++ b/storage/bucket_test.go @@ -257,7 +257,7 @@ func TestBucketAttrsToRawBucket(t *testing.T) { } // Test that setting PublicAccessPrevention to "unspecified" leads to the - // undeprecrated setting being propagated in the proto. + // inherited setting being propagated in the proto. attrs.PublicAccessPrevention = PublicAccessPreventionUnspecified got = attrs.toRawBucket() want.IamConfiguration = &raw.BucketIamConfiguration{