diff --git a/pkg/operator/ceph/object/policy.go b/pkg/operator/ceph/object/policy.go index 0f278b414..48299cb44 100644 --- a/pkg/operator/ceph/object/policy.go +++ b/pkg/operator/ceph/object/policy.go @@ -194,21 +194,10 @@ func (s *S3Agent) GetBucketPolicy(bucket string) (*BucketPolicy, error) { return policy, nil } -// ModifyBucketPolicy new and old statement SIDs and overwrites on a match. -// This allows users to Get, modify, and Replace existing statements as well as -// add new ones. +// ModifyBucketPolicy replaces the policy's statements with the given ones, +// rather than merging them into the existing statements by SID. func (bp *BucketPolicy) ModifyBucketPolicy(ps ...PolicyStatement) *BucketPolicy { - for _, newP := range ps { - var match bool - for j, oldP := range bp.Statement { - if newP.Sid == oldP.Sid { - bp.Statement[j] = newP - } - } - if !match { - bp.Statement = append(bp.Statement, newP) - } - } + bp.Statement = append([]PolicyStatement{}, ps...) return bp } diff --git a/pkg/operator/ceph/object/policy_test.go b/pkg/operator/ceph/object/policy_test.go new file mode 100644 index 000000000..5d78d092e --- /dev/null +++ b/pkg/operator/ceph/object/policy_test.go @@ -0,0 +1,53 @@ +/* +Copyright 2026 The Rook Authors. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package object + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestModifyBucketPolicy(t *testing.T) { + t.Run("does not duplicate an existing SID", func(t *testing.T) { + bp := NewBucketPolicy(*NewPolicyStatement().WithSID("s1").Allows().Actions(GetObject)) + bp.ModifyBucketPolicy(*NewPolicyStatement().WithSID("s1").Denies().Actions(PutObject)) + assert.Len(t, bp.Statement, 1) + assert.Equal(t, effectDeny, bp.Statement[0].Effect) + }) + + t.Run("replaces existing statements instead of merging", func(t *testing.T) { + bp := NewBucketPolicy( + *NewPolicyStatement().WithSID("s1").Allows().Actions(GetObject), + *NewPolicyStatement().WithSID("s2").Allows().Actions(GetObject), + ) + bp.ModifyBucketPolicy(*NewPolicyStatement().WithSID("s3").Denies().Actions(PutObject)) + assert.Len(t, bp.Statement, 1) + assert.Equal(t, "s3", bp.Statement[0].Sid) + }) + + t.Run("keeps all passed statements in order", func(t *testing.T) { + bp := NewBucketPolicy(*NewPolicyStatement().WithSID("old").Allows().Actions(GetObject)) + bp.ModifyBucketPolicy( + *NewPolicyStatement().WithSID("a").Allows().Actions(GetObject), + *NewPolicyStatement().WithSID("b").Denies().Actions(PutObject), + ) + assert.Len(t, bp.Statement, 2) + assert.Equal(t, "a", bp.Statement[0].Sid) + assert.Equal(t, "b", bp.Statement[1].Sid) + }) +}