Skip to content

Commit c6a33e3

Browse files
dvasilasdvasilas
authored
Merge pull request #143 from scality/improvement/LOGC-57-acl-required-e2e-tests
LOGC-57: Add e2e tests for aclRequired field
2 parents 62d4ffc + c9c78fe commit c6a33e3

6 files changed

Lines changed: 271 additions & 67 deletions

File tree

‎env/default/values.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ features:
88
enabled: true
99

1010
cloudserver:
11-
image: ghcr.io/scality/cloudserver:9.2.32
11+
image: ghcr.io/scality/cloudserver:9.2.34
1212

1313
vault:
1414
image: ghcr.io/scality/vault:7.84.0

‎test/e2e/acl_required_test.go‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
package e2e_test
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"fmt"
7+
"time"
8+
9+
"github.com/aws/aws-sdk-go-v2/aws"
10+
"github.com/aws/aws-sdk-go-v2/service/s3"
11+
. "github.com/onsi/ginkgo/v2"
12+
. "github.com/onsi/gomega"
13+
)
14+
15+
var _ = Describe("aclRequired field in access logs", func() {
16+
var testCtx *E2ETestContext
17+
18+
BeforeEach(func() {
19+
testCtx = setupE2ETest()
20+
})
21+
22+
AfterEach(func() {
23+
cleanupE2ETest(testCtx)
24+
})
25+
26+
It("logs aclRequired as dash for bucket owner requests", func(ctx context.Context) {
27+
testKey := "acl-owner-test.txt"
28+
testContent := []byte("owner request test data")
29+
30+
_, err := testCtx.S3Client.PutObject(ctx, &s3.PutObjectInput{
31+
Bucket: aws.String(testCtx.SourceBucket),
32+
Key: aws.String(testKey),
33+
Body: bytes.NewReader(testContent),
34+
})
35+
Expect(err).NotTo(HaveOccurred(), "PUT should succeed")
36+
37+
resp, err := testCtx.S3Client.GetObject(ctx, &s3.GetObjectInput{
38+
Bucket: aws.String(testCtx.SourceBucket),
39+
Key: aws.String(testKey),
40+
})
41+
Expect(err).NotTo(HaveOccurred(), "GET should succeed")
42+
_ = resp.Body.Close()
43+
44+
testCtx.VerifyLogs(
45+
testCtx.ObjectOp("REST.PUT.OBJECT", testKey, 200).
46+
WithObjectSize(int64(len(testContent))).
47+
WithACLRequired("-"),
48+
testCtx.ObjectOp("REST.GET.OBJECT", testKey, 200).
49+
WithBytesSent(int64(len(testContent))).
50+
WithObjectSize(int64(len(testContent))).
51+
WithACLRequired("-"),
52+
)
53+
})
54+
55+
It("logs aclRequired as Yes for IAM user authorized by ACL", func(ctx context.Context) {
56+
testKey := "acl-iam-test.txt"
57+
testContent := []byte("IAM user ACL test data")
58+
59+
// PUT object as bucket owner
60+
_, err := testCtx.S3Client.PutObject(ctx, &s3.PutObjectInput{
61+
Bucket: aws.String(testCtx.SourceBucket),
62+
Key: aws.String(testKey),
63+
Body: bytes.NewReader(testContent),
64+
})
65+
Expect(err).NotTo(HaveOccurred(), "PUT should succeed")
66+
67+
// Create IAM user with s3:GetObject permission
68+
userName := fmt.Sprintf("e2e-acl-test-%d", time.Now().UnixNano())
69+
policy := fmt.Sprintf(`{
70+
"Version": "2012-10-17",
71+
"Statement": [{
72+
"Effect": "Allow",
73+
"Action": "s3:GetObject",
74+
"Resource": "arn:aws:s3:::%s/*"
75+
}]
76+
}`, testCtx.SourceBucket)
77+
iamUser := createIAMUser(ctx, userName, "allow-get-object", policy)
78+
defer iamUser.Cleanup()
79+
80+
// GET object as the IAM user — authorized via ACL (same account)
81+
iamS3Client := iamUser.S3Client
82+
83+
resp, err := iamS3Client.GetObject(ctx, &s3.GetObjectInput{
84+
Bucket: aws.String(testCtx.SourceBucket),
85+
Key: aws.String(testKey),
86+
})
87+
Expect(err).NotTo(HaveOccurred(), "GET as IAM user should succeed")
88+
_ = resp.Body.Close()
89+
90+
logs := testCtx.VerifyLogs(
91+
testCtx.ObjectOp("REST.PUT.OBJECT", testKey, 200).
92+
WithObjectSize(int64(len(testContent))).
93+
WithACLRequired("-"),
94+
testCtx.ObjectOp("REST.GET.OBJECT", testKey, 200).
95+
WithBytesSent(int64(len(testContent))).
96+
WithObjectSize(int64(len(testContent))).
97+
WithACLRequired("Yes"),
98+
)
99+
100+
// Additional verification: the two requests have different requesters
101+
Expect(logs[0].Requester).NotTo(Equal(logs[1].Requester),
102+
"PUT (owner) and GET (IAM user) should have different requesters")
103+
})
104+
105+
It("logs aclRequired as Yes for both sides of a copy by IAM user", func(ctx context.Context) {
106+
sourceKey := "acl-copy-source.txt"
107+
destKey := "acl-copy-dest.txt"
108+
testContent := []byte("copy test data")
109+
110+
// PUT object as bucket owner
111+
_, err := testCtx.S3Client.PutObject(ctx, &s3.PutObjectInput{
112+
Bucket: aws.String(testCtx.SourceBucket),
113+
Key: aws.String(sourceKey),
114+
Body: bytes.NewReader(testContent),
115+
})
116+
Expect(err).NotTo(HaveOccurred(), "PUT should succeed")
117+
118+
// Create IAM user with GetObject + PutObject permissions for the copy
119+
userName := fmt.Sprintf("e2e-acl-copy-%d", time.Now().UnixNano())
120+
policy := fmt.Sprintf(`{
121+
"Version": "2012-10-17",
122+
"Statement": [{
123+
"Effect": "Allow",
124+
"Action": ["s3:GetObject", "s3:PutObject"],
125+
"Resource": "arn:aws:s3:::%s/*"
126+
}]
127+
}`, testCtx.SourceBucket)
128+
iamUser := createIAMUser(ctx, userName, "allow-copy", policy)
129+
defer iamUser.Cleanup()
130+
131+
// Copy as IAM user — both source GET and destination PUT go through ACL path
132+
iamS3Client := iamUser.S3Client
133+
134+
_, err = iamS3Client.CopyObject(ctx, &s3.CopyObjectInput{
135+
Bucket: aws.String(testCtx.SourceBucket),
136+
Key: aws.String(destKey),
137+
CopySource: aws.String(fmt.Sprintf("%s/%s", testCtx.SourceBucket, sourceKey)),
138+
})
139+
Expect(err).NotTo(HaveOccurred(), "COPY as IAM user should succeed")
140+
141+
testCtx.VerifyLogs(
142+
testCtx.ObjectOp("REST.PUT.OBJECT", sourceKey, 200).
143+
WithObjectSize(int64(len(testContent))).
144+
WithACLRequired("-"),
145+
testCtx.ObjectOp("REST.COPY.OBJECT_GET", sourceKey, 200).
146+
WithObjectSize(int64(len(testContent))).
147+
WithACLRequired("Yes"),
148+
testCtx.ObjectOp("REST.COPY.OBJECT", destKey, 200).
149+
WithObjectSize(int64(len(testContent))).
150+
WithACLRequired("Yes"),
151+
)
152+
})
153+
})

‎test/e2e/error_cases_test.go‎

Lines changed: 3 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"time"
99

1010
"github.com/aws/aws-sdk-go-v2/aws"
11-
"github.com/aws/aws-sdk-go-v2/service/iam"
1211
"github.com/aws/aws-sdk-go-v2/service/s3"
1312
. "github.com/onsi/ginkgo/v2"
1413
. "github.com/onsi/gomega"
@@ -183,56 +182,11 @@ var _ = Describe("Error Cases", func() {
183182
time.Sleep(1 * time.Second)
184183

185184
// Create IAM user with no permissions
186-
iamEndpoint := os.Getenv("E2E_IAM_ENDPOINT")
187-
if iamEndpoint == "" {
188-
iamEndpoint = testIAMEndpoint
189-
}
190-
accessKey := os.Getenv("E2E_S3_ACCESS_KEY_ID")
191-
if accessKey == "" {
192-
accessKey = testAccessKeyID
193-
}
194-
secretKey := os.Getenv("E2E_S3_SECRET_ACCESS_KEY")
195-
if secretKey == "" {
196-
secretKey = testSecretAccessKey
197-
}
198-
199-
iamClient := iam.NewFromConfig(aws.Config{
200-
Region: testRegion,
201-
Credentials: aws.CredentialsProviderFunc(func(ctx context.Context) (aws.Credentials, error) {
202-
return aws.Credentials{
203-
AccessKeyID: accessKey,
204-
SecretAccessKey: secretKey,
205-
}, nil
206-
}),
207-
}, func(o *iam.Options) {
208-
o.BaseEndpoint = aws.String(iamEndpoint)
209-
})
210-
211185
userName := fmt.Sprintf("e2e-test-user-%d", time.Now().UnixNano())
212-
_, err = iamClient.CreateUser(ctx, &iam.CreateUserInput{
213-
UserName: aws.String(userName),
214-
})
215-
Expect(err).NotTo(HaveOccurred(), "CreateUser should succeed")
186+
iamUser := createIAMUser(ctx, userName, "", "")
187+
defer iamUser.Cleanup()
216188

217-
createKeyResp, err := iamClient.CreateAccessKey(ctx, &iam.CreateAccessKeyInput{
218-
UserName: aws.String(userName),
219-
})
220-
Expect(err).NotTo(HaveOccurred(), "CreateAccessKey should succeed")
221-
222-
defer func() {
223-
_, _ = iamClient.DeleteAccessKey(ctx, &iam.DeleteAccessKeyInput{
224-
UserName: aws.String(userName),
225-
AccessKeyId: createKeyResp.AccessKey.AccessKeyId,
226-
})
227-
_, _ = iamClient.DeleteUser(ctx, &iam.DeleteUserInput{
228-
UserName: aws.String(userName),
229-
})
230-
}()
231-
232-
unprivilegedClient := newS3ClientWithCredentials(
233-
*createKeyResp.AccessKey.AccessKeyId,
234-
*createKeyResp.AccessKey.SecretAccessKey,
235-
)
189+
unprivilegedClient := iamUser.S3Client
236190

237191
_, err = unprivilegedClient.GetObject(ctx, &s3.GetObjectInput{
238192
Bucket: aws.String(testCtx.SourceBucket),

‎test/e2e/helpers_test.go‎

Lines changed: 100 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"time"
1414

1515
"github.com/aws/aws-sdk-go-v2/aws"
16+
"github.com/aws/aws-sdk-go-v2/service/iam"
1617
"github.com/aws/aws-sdk-go-v2/service/s3"
1718
"github.com/aws/aws-sdk-go-v2/service/s3/types"
1819
. "github.com/onsi/ginkgo/v2"
@@ -64,7 +65,7 @@ type ParsedLogRecord struct {
6465
HostHeader string
6566
TLSVersion string
6667
AccessPointARN string // Field 25 - always "-" (not supported)
67-
ACLRequired string // Field 26 - always "-" (not supported)
68+
ACLRequired string
6869
BytesSent int64
6970
ObjectSize int64
7071
HTTPStatus int
@@ -74,13 +75,14 @@ type ParsedLogRecord struct {
7475

7576
// ExpectedLog defines expected log record fields for verification
7677
type ExpectedLog struct {
77-
Operation string
78-
Bucket string
79-
Key string // Optional, use "" to skip check
80-
ErrorCode string // Optional, use "" to skip check
81-
HTTPStatus int
82-
BytesSent int64 // Optional, use -1 to skip check
83-
ObjectSize int64 // Optional, use -1 to skip check
78+
Operation string
79+
Bucket string
80+
Key string // Optional, use "" to skip check
81+
ErrorCode string // Optional, use "" to skip check
82+
ACLRequired string // Optional, use "" to skip check
83+
BytesSent int64 // Optional, use -1 to skip check
84+
ObjectSize int64 // Optional, use -1 to skip check
85+
HTTPStatus int
8486
}
8587

8688
// ExpectedLogBuilder allows fluent construction of ExpectedLog
@@ -106,6 +108,12 @@ func (b ExpectedLogBuilder) WithErrorCode(code string) ExpectedLogBuilder {
106108
return b
107109
}
108110

111+
// WithACLRequired sets the expected ACLRequired value
112+
func (b ExpectedLogBuilder) WithACLRequired(value string) ExpectedLogBuilder {
113+
b.log.ACLRequired = value
114+
return b
115+
}
116+
109117
// BucketOp creates an ExpectedLog for bucket-level operations (no key)
110118
func (ctx *E2ETestContext) BucketOp(operation string, httpStatus int) ExpectedLogBuilder {
111119
return ExpectedLogBuilder{
@@ -510,8 +518,8 @@ func verifyLogRecord(actual *ParsedLogRecord, expected ExpectedLogBuilder) {
510518
"HostID should always be '-' (not supported)")
511519
Expect(actual.AccessPointARN).To(Equal("-"),
512520
"AccessPointARN should always be '-' (not supported)")
513-
Expect(actual.ACLRequired).To(Equal("-"),
514-
"ACLRequired should always be '-' (not supported)")
521+
Expect(actual.ACLRequired).To(BeElementOf("-", "Yes"),
522+
"ACLRequired should be '-' or 'Yes'")
515523

516524
// Verify timing fields relationship
517525
Expect(actual.TotalTime).To(BeNumerically(">=", 0),
@@ -530,6 +538,10 @@ func verifyLogRecord(actual *ParsedLogRecord, expected ExpectedLogBuilder) {
530538
Expect(actual.ObjectSize).To(Equal(exp.ObjectSize),
531539
"ObjectSize mismatch")
532540
}
541+
if exp.ACLRequired != "" {
542+
Expect(actual.ACLRequired).To(Equal(exp.ACLRequired),
543+
"ACLRequired mismatch")
544+
}
533545
}
534546

535547
// verifyLogKeys verifies that logs contain exactly the expected keys with no duplicates.
@@ -647,6 +659,84 @@ func newS3ClientWithCredentials(accessKeyID, secretAccessKey string) *s3.Client
647659
})
648660
}
649661

662+
type IAMUserResult struct {
663+
S3Client *s3.Client
664+
Cleanup func()
665+
}
666+
667+
// createIAMUser creates an IAM user with an optional inline policy.
668+
// If policyName and policyDocument are non-empty, the policy is attached.
669+
func createIAMUser(ctx context.Context, userName, policyName, policyDocument string) IAMUserResult {
670+
GinkgoHelper()
671+
672+
iamEndpoint := os.Getenv("E2E_IAM_ENDPOINT")
673+
if iamEndpoint == "" {
674+
iamEndpoint = testIAMEndpoint
675+
}
676+
accessKey := os.Getenv("E2E_S3_ACCESS_KEY_ID")
677+
if accessKey == "" {
678+
accessKey = testAccessKeyID
679+
}
680+
secretKey := os.Getenv("E2E_S3_SECRET_ACCESS_KEY")
681+
if secretKey == "" {
682+
secretKey = testSecretAccessKey
683+
}
684+
685+
iamClient := iam.NewFromConfig(aws.Config{
686+
Region: testRegion,
687+
Credentials: aws.CredentialsProviderFunc(func(ctx context.Context) (aws.Credentials, error) {
688+
return aws.Credentials{
689+
AccessKeyID: accessKey,
690+
SecretAccessKey: secretKey,
691+
}, nil
692+
}),
693+
}, func(o *iam.Options) {
694+
o.BaseEndpoint = aws.String(iamEndpoint)
695+
})
696+
697+
_, err := iamClient.CreateUser(ctx, &iam.CreateUserInput{
698+
UserName: aws.String(userName),
699+
})
700+
Expect(err).NotTo(HaveOccurred(), "CreateUser should succeed")
701+
702+
createKeyResp, err := iamClient.CreateAccessKey(ctx, &iam.CreateAccessKeyInput{
703+
UserName: aws.String(userName),
704+
})
705+
Expect(err).NotTo(HaveOccurred(), "CreateAccessKey should succeed")
706+
707+
if policyName != "" && policyDocument != "" {
708+
_, err = iamClient.PutUserPolicy(ctx, &iam.PutUserPolicyInput{
709+
UserName: aws.String(userName),
710+
PolicyName: aws.String(policyName),
711+
PolicyDocument: aws.String(policyDocument),
712+
})
713+
Expect(err).NotTo(HaveOccurred(), "PutUserPolicy should succeed")
714+
}
715+
716+
cleanup := func() {
717+
if policyName != "" {
718+
_, _ = iamClient.DeleteUserPolicy(ctx, &iam.DeleteUserPolicyInput{
719+
UserName: aws.String(userName),
720+
PolicyName: aws.String(policyName),
721+
})
722+
}
723+
_, _ = iamClient.DeleteAccessKey(ctx, &iam.DeleteAccessKeyInput{
724+
UserName: aws.String(userName),
725+
AccessKeyId: createKeyResp.AccessKey.AccessKeyId,
726+
})
727+
_, _ = iamClient.DeleteUser(ctx, &iam.DeleteUserInput{
728+
UserName: aws.String(userName),
729+
})
730+
}
731+
732+
s3Client := newS3ClientWithCredentials(
733+
*createKeyResp.AccessKey.AccessKeyId,
734+
*createKeyResp.AccessKey.SecretAccessKey,
735+
)
736+
737+
return IAMUserResult{S3Client: s3Client, Cleanup: cleanup}
738+
}
739+
650740
// setupE2ETest creates and initializes an E2E test context
651741
func setupE2ETest() *E2ETestContext {
652742
GinkgoHelper()

0 commit comments

Comments
 (0)