Skip to content
This repository was archived by the owner on Dec 6, 2024. It is now read-only.

Commit e354c37

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #29 from rrati/bucket-context-nil-fix
Fix handling of nil parameters in B and BA
2 parents 961846a + 74c54f6 commit e354c37

File tree

4 files changed

+150
-6
lines changed

4 files changed

+150
-6
lines changed

Diff for: ‎container-object-storage-interface-provisioner-sidecar/pkg/controller/bucket/bucket_controller.go

+10-2
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ func (bl *bucketListener) Add(ctx context.Context, obj *v1alpha1.Bucket) error {
102102

103103
req := osspec.ProvisionerCreateBucketRequest{
104104
BucketName: obj.Name,
105-
BucketContext: obj.Spec.Parameters,
105+
BucketContext: bl.getParams(obj),
106106
}
107107

108108
req.BucketContext["ProtocolVersion"] = obj.Spec.Protocol.Version
@@ -147,7 +147,7 @@ func (bl *bucketListener) Delete(ctx context.Context, obj *v1alpha1.Bucket) erro
147147
}
148148

149149
req := osspec.ProvisionerDeleteBucketRequest{
150-
BucketContext: obj.Spec.Parameters,
150+
BucketContext: bl.getParams(obj),
151151
}
152152

153153
switch obj.Spec.Protocol.Name {
@@ -198,3 +198,11 @@ func (bl *bucketListener) updateStatus(ctx context.Context, name, msg string, st
198198
})
199199
return err
200200
}
201+
202+
func (bl *bucketListener) getParams(obj *v1alpha1.Bucket) map[string]string {
203+
params := map[string]string{}
204+
if obj.Spec.Parameters != nil {
205+
params = obj.Spec.Parameters
206+
}
207+
return params
208+
}

Diff for: ‎container-object-storage-interface-provisioner-sidecar/pkg/controller/bucket/bucket_controller_test.go

+51-2
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,20 @@ func TestAddValidProtocols(t *testing.T) {
212212
"AnonymousAccessMode": anonAccess,
213213
},
214214
},
215+
{
216+
name: "Empty parameters",
217+
protocolName: v1alpha1.ProtocolNameS3,
218+
createFunc: func(ctx context.Context, in *osspec.ProvisionerCreateBucketRequest, opts ...grpc.CallOption) (*osspec.ProvisionerCreateBucketResponse, error) {
219+
if in.BucketName != bucketName {
220+
t.Errorf("expected %s, got %s", bucketName, in.BucketName)
221+
}
222+
if in.BucketContext["ProtocolVersion"] != protocolVersion {
223+
t.Errorf("expected %s, got %s", protocolVersion, in.BucketContext["ProtocolVersion"])
224+
}
225+
return &osspec.ProvisionerCreateBucketResponse{}, nil
226+
},
227+
params: nil,
228+
},
215229
}
216230

217231
for _, tc := range testCases {
@@ -242,7 +256,7 @@ func TestAddValidProtocols(t *testing.T) {
242256
kubeClient: kubeClient,
243257
}
244258

245-
t.Logf("Testing protocol %s", tc.name)
259+
t.Logf(tc.name)
246260
err := bl.Add(ctx, &b)
247261
if err != nil {
248262
t.Errorf("add returned: %+v", err)
@@ -405,6 +419,41 @@ func TestDeleteValidProtocols(t *testing.T) {
405419
extraParamName: extraParamValue,
406420
},
407421
},
422+
{
423+
name: "Empty parameters",
424+
setProtocol: func(b *v1alpha1.Bucket) {
425+
b.Spec.Protocol.S3 = &v1alpha1.S3Protocol{
426+
Region: region,
427+
Version: protocolVersion,
428+
SignatureVersion: sigVersion,
429+
BucketName: bucketName,
430+
Endpoint: endpoint,
431+
}
432+
},
433+
protocolName: v1alpha1.ProtocolNameS3,
434+
deleteFunc: func(ctx context.Context, in *osspec.ProvisionerDeleteBucketRequest, opts ...grpc.CallOption) (*osspec.ProvisionerDeleteBucketResponse, error) {
435+
if in.BucketName != bucketName {
436+
t.Errorf("expected %s, got %s", bucketName, in.BucketName)
437+
}
438+
if in.BucketContext["Region"] != region {
439+
t.Errorf("expected %s, got %s", region, in.BucketContext["Region"])
440+
}
441+
if in.BucketContext["ProtocolVersion"] != protocolVersion {
442+
t.Errorf("expected %s, got %s", protocolVersion, in.BucketContext["ProtocolVersion"])
443+
}
444+
if in.BucketContext["SignatureVersion"] != string(sigVersion) {
445+
t.Errorf("expected %s, got %s", sigVersion, in.BucketContext["SignatureVersion"])
446+
}
447+
if in.BucketContext["Endpoint"] != endpoint {
448+
t.Errorf("expected %s, got %s", endpoint, in.BucketContext["Endpoint"])
449+
}
450+
if in.BucketContext["ProtocolVersion"] != protocolVersion {
451+
t.Errorf("expected %s, got %s", protocolVersion, in.BucketContext["ProtocolVersion"])
452+
}
453+
return &osspec.ProvisionerDeleteBucketResponse{}, nil
454+
},
455+
params: nil,
456+
},
408457
}
409458

410459
for _, tc := range testCases {
@@ -434,7 +483,7 @@ func TestDeleteValidProtocols(t *testing.T) {
434483
}
435484

436485
tc.setProtocol(&b)
437-
t.Logf("Testing protocol %s", tc.name)
486+
t.Logf(tc.name)
438487
err := bl.Delete(ctx, &b)
439488
if err != nil {
440489
t.Errorf("delete returned: %+v", err)

Diff for: ‎container-object-storage-interface-provisioner-sidecar/pkg/controller/bucketaccess/bucket_access_controller.go

+10-2
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func (bal *bucketAccessListener) Add(ctx context.Context, obj *v1alpha1.BucketAc
116116
req := osspec.ProvisionerGrantBucketAccessRequest{
117117
Principal: obj.Spec.Principal,
118118
AccessPolicy: obj.Spec.PolicyActionsConfigMapData,
119-
BucketContext: obj.Spec.Parameters,
119+
BucketContext: bal.getParams(obj),
120120
}
121121

122122
switch bucket.Spec.Protocol.Name {
@@ -206,7 +206,7 @@ func (bal *bucketAccessListener) Delete(ctx context.Context, obj *v1alpha1.Bucke
206206

207207
req := osspec.ProvisionerRevokeBucketAccessRequest{
208208
Principal: obj.Spec.Principal,
209-
BucketContext: obj.Spec.Parameters,
209+
BucketContext: bal.getParams(obj),
210210
}
211211

212212
switch bucket.Spec.Protocol.Name {
@@ -277,3 +277,11 @@ func (bal *bucketAccessListener) updatePrincipal(ctx context.Context, name strin
277277
})
278278
return err
279279
}
280+
281+
func (bal *bucketAccessListener) getParams(obj *v1alpha1.BucketAccess) map[string]string {
282+
params := map[string]string{}
283+
if obj.Spec.Parameters != nil {
284+
params = obj.Spec.Parameters
285+
}
286+
return params
287+
}

Diff for: ‎container-object-storage-interface-provisioner-sidecar/pkg/controller/bucketaccess/bucket_access_controller_test.go

+79
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,47 @@ func TestAdd(t *testing.T) {
303303
extraParamName: extraParamValue,
304304
},
305305
},
306+
{
307+
name: "Empty parameters",
308+
setProtocol: func(b *v1alpha1.Bucket) {
309+
b.Spec.Protocol.S3 = &v1alpha1.S3Protocol{
310+
Region: region,
311+
Version: protocolVersion,
312+
SignatureVersion: sigVersion,
313+
BucketName: bucketName,
314+
Endpoint: endpoint,
315+
}
316+
},
317+
protocolName: v1alpha1.ProtocolNameS3,
318+
grantFunc: func(ctx context.Context, in *osspec.ProvisionerGrantBucketAccessRequest, opts ...grpc.CallOption) (*osspec.ProvisionerGrantBucketAccessResponse, error) {
319+
if in.BucketName != bucketName {
320+
t.Errorf("expected %s, got %s", bucketName, in.BucketName)
321+
}
322+
if in.BucketContext["Region"] != region {
323+
t.Errorf("expected %s, got %s", region, in.BucketContext["Region"])
324+
}
325+
if in.Principal != principal {
326+
t.Errorf("expected %s, got %s", principal, in.Principal)
327+
}
328+
if in.BucketContext["Version"] != protocolVersion {
329+
t.Errorf("expected %s, got %s", protocolVersion, in.BucketContext["Version"])
330+
}
331+
if in.BucketContext["SignatureVersion"] != string(sigVersion) {
332+
t.Errorf("expected %s, got %s", sigVersion, in.BucketContext["SignatureVersion"])
333+
}
334+
if in.BucketContext["Endpoint"] != endpoint {
335+
t.Errorf("expected %s, got %s", endpoint, in.BucketContext["Endpoint"])
336+
}
337+
return &osspec.ProvisionerGrantBucketAccessResponse{
338+
Principal: principal,
339+
CredentialsFileContents: credsContents,
340+
CredentialsFilePath: credsFile,
341+
}, nil
342+
},
343+
principal: principal,
344+
serviceAccount: "",
345+
params: nil,
346+
},
306347
}
307348

308349
for _, tc := range testCases {
@@ -347,6 +388,7 @@ func TestAdd(t *testing.T) {
347388
kubeClient: kubeClient,
348389
}
349390

391+
t.Logf(tc.name)
350392
err := bal.Add(ctx, &ba)
351393
if err != nil {
352394
t.Errorf("add returned: %+v", err)
@@ -576,6 +618,42 @@ func TestDelete(t *testing.T) {
576618
extraParamName: extraParamValue,
577619
},
578620
},
621+
{
622+
name: "Empty parameters",
623+
setProtocol: func(b *v1alpha1.Bucket) {
624+
b.Spec.Protocol.S3 = &v1alpha1.S3Protocol{
625+
Region: region,
626+
Version: protocolVersion,
627+
SignatureVersion: sigVersion,
628+
BucketName: bucketName,
629+
Endpoint: endpoint,
630+
}
631+
},
632+
protocolName: v1alpha1.ProtocolNameS3,
633+
revokeFunc: func(ctx context.Context, in *osspec.ProvisionerRevokeBucketAccessRequest, opts ...grpc.CallOption) (*osspec.ProvisionerRevokeBucketAccessResponse, error) {
634+
if in.BucketName != bucketName {
635+
t.Errorf("expected %s, got %s", bucketName, in.BucketName)
636+
}
637+
if in.BucketContext["Region"] != region {
638+
t.Errorf("expected %s, got %s", region, in.BucketContext["Region"])
639+
}
640+
if in.Principal != principal {
641+
t.Errorf("expected %s, got %s", principal, in.Principal)
642+
}
643+
if in.BucketContext["Version"] != protocolVersion {
644+
t.Errorf("expected %s, got %s", protocolVersion, in.BucketContext["Version"])
645+
}
646+
if in.BucketContext["SignatureVersion"] != string(sigVersion) {
647+
t.Errorf("expected %s, got %s", sigVersion, in.BucketContext["SignatureVersion"])
648+
}
649+
if in.BucketContext["Endpoint"] != endpoint {
650+
t.Errorf("expected %s, got %s", endpoint, in.BucketContext["Endpoint"])
651+
}
652+
return &osspec.ProvisionerRevokeBucketAccessResponse{}, nil
653+
},
654+
serviceAccount: "",
655+
params: nil,
656+
},
579657
}
580658

581659
for _, tc := range testCases {
@@ -619,6 +697,7 @@ func TestDelete(t *testing.T) {
619697
Type: v1.SecretTypeOpaque,
620698
}
621699

700+
t.Logf(tc.name)
622701
ctx := context.TODO()
623702
tc.setProtocol(&b)
624703
client := fakebucketclientset.NewSimpleClientset(&ba, &b)

0 commit comments

Comments
 (0)