Skip to content

Commit

Permalink
fix(storage): update PAP to use inherited instead of unspecified (#4909)
Browse files Browse the repository at this point in the history
* fix(storage): update PAP to use inherited instead of unspecified
  • Loading branch information
BrennaEpp committed Oct 7, 2021
1 parent bff5b1c commit dac26b1
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 19 deletions.
24 changes: 15 additions & 9 deletions storage/bucket.go
Expand Up @@ -378,23 +378,29 @@ const (
// not set in a call to GCS.
PublicAccessPreventionUnknown PublicAccessPrevention = iota

// PublicAccessPreventionUnspecified corresponds to a value of "unspecified"
// and is the default for buckets.
// PublicAccessPreventionUnspecified corresponds to a value of "unspecified".
// Deprecated: use PublicAccessPreventionInherited
PublicAccessPreventionUnspecified

// PublicAccessPreventionEnforced corresponds to a value of "enforced". This
// enforces Public Access Prevention on the bucket.
PublicAccessPreventionEnforced

publicAccessPreventionUnknown string = ""
publicAccessPreventionUnspecified = "unspecified"
publicAccessPreventionEnforced = "enforced"
// PublicAccessPreventionInherited corresponds to a value of "inherited"
// and is the default for buckets.
PublicAccessPreventionInherited

publicAccessPreventionUnknown string = ""
// TODO: remove unspecified when change is fully completed
publicAccessPreventionUnspecified = "unspecified"
publicAccessPreventionEnforced = "enforced"
publicAccessPreventionInherited = "inherited"
)

func (p PublicAccessPrevention) String() string {
switch p {
case PublicAccessPreventionUnspecified:
return publicAccessPreventionUnspecified
case PublicAccessPreventionInherited, PublicAccessPreventionUnspecified:
return publicAccessPreventionInherited
case PublicAccessPreventionEnforced:
return publicAccessPreventionEnforced
default:
Expand Down Expand Up @@ -1214,8 +1220,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:
Expand Down
17 changes: 14 additions & 3 deletions storage/bucket_test.go
Expand Up @@ -257,11 +257,22 @@ func TestBucketAttrsToRawBucket(t *testing.T) {
}

// Test that setting PublicAccessPrevention to "unspecified" leads to the
// setting being propagated in the proto.
// inherited setting being propagated in the proto.
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)
Expand All @@ -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)
Expand Down
13 changes: 6 additions & 7 deletions storage/integration_test.go
Expand Up @@ -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")
Expand Down Expand Up @@ -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)
}
}

Expand Down

0 comments on commit dac26b1

Please sign in to comment.