Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(storage): update PAP to use inherited instead of unspecified #4909

Merged
merged 9 commits into from Oct 7, 2021
Merged

fix(storage): update PAP to use inherited instead of unspecified #4909

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9a7a9de
fix(storage): update PAP to use inherited instead of unspecified
BrennaEpp Sep 29, 2021
5f89407
fix enum
BrennaEpp Sep 29, 2021
84973c4
add todo
BrennaEpp Sep 29, 2021
46fb45d
typo
BrennaEpp Sep 29, 2021
b548c86
Merge branch 'master' into pap-inherited
BrennaEpp Oct 6, 2021
7bcf2b4
fix comments
BrennaEpp Oct 6, 2021
079223e
Merge branch 'master' into pap-inherited
BrennaEpp Oct 7, 2021
e8b63bb
Merge branch 'master' into pap-inherited
BrennaEpp Oct 7, 2021
2604003
Merge branch 'master' into pap-inherited
BrennaEpp Oct 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 15 additions & 9 deletions storage/bucket.go
View file
Open in desktop
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shaffeeullah maybe we should include a sentence about how this means that the value for PAP is inherited from the project, folder, or org? Do we have something like this in other languages?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i do not believe we have something like that in other languages

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
View file
Open in desktop
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
View file
Open in desktop
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