Skip to content

Commit 4cfad07

Browse files
authored
Merge pull request kubernetes-retired#20 from rrati/bac-fixes
BucketAccess Controller fixes
2 parents 40bc9fe + ccde9df commit 4cfad07

File tree

4 files changed

+86
-13
lines changed

4 files changed

+86
-13
lines changed

Diff for: container-object-storage-interface-provisioner-sidecar/go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ go 1.15
44

55
require (
66
github.com/kubernetes-csi/csi-lib-utils v0.9.0
7-
github.com/kubernetes-sigs/container-object-storage-interface-api v0.0.0-20201204201926-43539346a903
7+
github.com/kubernetes-sigs/container-object-storage-interface-api v0.0.0-20201217233824-6b4158ff7e28
88
github.com/kubernetes-sigs/container-object-storage-interface-spec v0.0.0-20201217184109-8cbf84dde8d3
99
golang.org/x/net v0.0.0-20200707034311-ab3426394381
1010
golang.org/x/time v0.0.0-20201208040808-7e3f01d25324

Diff for: container-object-storage-interface-provisioner-sidecar/go.sum

+3-2
Original file line numberDiff line numberDiff line change
@@ -258,8 +258,8 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
258258
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
259259
github.com/kubernetes-csi/csi-lib-utils v0.9.0 h1:TbuDmxoVqM+fvVkzG/7sShyX/8jUln0ElLHuETcsQJI=
260260
github.com/kubernetes-csi/csi-lib-utils v0.9.0/go.mod h1:8E2jVUX9j3QgspwHXa6LwyN7IHQDjW9jX3kwoWnSC+M=
261-
github.com/kubernetes-sigs/container-object-storage-interface-api v0.0.0-20201204201926-43539346a903 h1:kBd9bCHv429J7Y8Mp2w1Xg3QtDiRAhittzYC/45/G2E=
262-
github.com/kubernetes-sigs/container-object-storage-interface-api v0.0.0-20201204201926-43539346a903/go.mod h1:C7tjzC+nLe7H7+3UM/Z6a7F24yxOO8FSK3ZaVZrKDPQ=
261+
github.com/kubernetes-sigs/container-object-storage-interface-api v0.0.0-20201217233824-6b4158ff7e28 h1:ZCH6aKd+cWLi66i3tPV6f0dIuFG5Lk9XOxWacnsR8mg=
262+
github.com/kubernetes-sigs/container-object-storage-interface-api v0.0.0-20201217233824-6b4158ff7e28/go.mod h1:C7tjzC+nLe7H7+3UM/Z6a7F24yxOO8FSK3ZaVZrKDPQ=
263263
github.com/kubernetes-sigs/container-object-storage-interface-spec v0.0.0-20201217184109-8cbf84dde8d3 h1:8uv9nGDukcMLYeg/m9YtTyr2IzXZd5WwUVDBRuJy2Vw=
264264
github.com/kubernetes-sigs/container-object-storage-interface-spec v0.0.0-20201217184109-8cbf84dde8d3/go.mod h1:wojgWDesMMLuFza4p1YnpX3sdPeo0mDWmSbmPsxRDh0=
265265
github.com/magiconair/properties v1.8.0 h1:LLgXmsheXeRoUOBOjtwPQCWIYqM/LU1ayDtDePerRcY=
@@ -589,6 +589,7 @@ gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
589589
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
590590
gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
591591
gopkg.in/yaml.v2 v2.2.5/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
592+
gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10=
592593
gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
593594
gopkg.in/yaml.v2 v2.3.0 h1:clyUAQHOM3G0M3f5vQj7LuJrETvjVot3Z5el9nffUtU=
594595
gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=

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

+13-8
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222
"strings"
2323
"time"
2424

25-
v1 "k8s.io/api/core/v1"
25+
corev1 "k8s.io/api/core/v1"
2626

2727
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2828
"k8s.io/apimachinery/pkg/types"
@@ -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: map[string]string{},
119+
BucketContext: obj.Spec.Parameters,
120120
}
121121

122122
switch bucket.Spec.Protocol.Name {
@@ -148,7 +148,7 @@ func (bal *bucketAccessListener) Add(ctx context.Context, obj *v1alpha1.BucketAc
148148

149149
// Only update the principal in the BucketAccess if it wasn't set because
150150
// that means that the provisioner created one
151-
if len(obj.Spec.Principal) == 0 {
151+
if len(obj.Spec.Principal) == 0 && obj.Spec.ServiceAccount == nil {
152152
err = bal.updatePrincipal(ctx, obj.Name, rsp)
153153
if err != nil {
154154
return err
@@ -158,22 +158,25 @@ func (bal *bucketAccessListener) Add(ctx context.Context, obj *v1alpha1.BucketAc
158158
// Only create the secret with credentials if serviveAccount isn't set.
159159
// If serviceAccount is set then authorization happens out of band in the
160160
// cloud provider
161-
if len(obj.Spec.ServiceAccount) == 0 {
162-
secret := v1.Secret{
161+
if obj.Spec.ServiceAccount == nil {
162+
secret := corev1.Secret{
163163
ObjectMeta: metav1.ObjectMeta{
164164
Name: generateSecretName(obj.UID),
165165
},
166166
StringData: map[string]string{
167167
"CredentialsFilePath": rsp.CredentialsFilePath,
168168
"CredentialsFileContents": rsp.CredentialsFileContents,
169169
},
170-
Type: v1.SecretTypeOpaque,
170+
Type: corev1.SecretTypeOpaque,
171171
}
172+
172173
// It's unlikely but should probably handle retries on rare case of collision
173174
_, err = bal.kubeClient.CoreV1().Secrets("objectstorage-system").Create(ctx, &secret, metav1.CreateOptions{})
174175
if err != nil {
175176
return err
176177
}
178+
179+
// TODO update the mintedSecretName in the BA
177180
}
178181

179182
// update bucket access status to granted
@@ -203,7 +206,7 @@ func (bal *bucketAccessListener) Delete(ctx context.Context, obj *v1alpha1.Bucke
203206

204207
req := osspec.ProvisionerRevokeBucketAccessRequest{
205208
Principal: obj.Spec.Principal,
206-
BucketContext: map[string]string{},
209+
BucketContext: obj.Spec.Parameters,
207210
}
208211

209212
switch bucket.Spec.Protocol.Name {
@@ -234,7 +237,9 @@ func (bal *bucketAccessListener) Delete(ctx context.Context, obj *v1alpha1.Bucke
234237
klog.V(1).Infof("provisioner returned revoke bucket access response %v", rsp)
235238

236239
// Delete the secret
237-
if len(obj.Spec.ServiceAccount) == 0 {
240+
if obj.Spec.ServiceAccount == nil {
241+
// TODO get the minted secret name from the BA
242+
238243
// It's unlikely but should probably handle retries on rare case of collision
239244
err = bal.kubeClient.CoreV1().Secrets("objectstorage-system").Delete(ctx, generateSecretName(obj.UID), metav1.DeleteOptions{})
240245
if err != nil {

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

+69-2
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
osspec "github.com/kubernetes-sigs/container-object-storage-interface-spec"
2929
fakespec "github.com/kubernetes-sigs/container-object-storage-interface-spec/fake"
3030

31+
corev1 "k8s.io/api/core/v1"
3132
v1 "k8s.io/api/core/v1"
3233

3334
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -118,6 +119,8 @@ func TestAdd(t *testing.T) {
118119
generatedPrincipal := "driverPrincipal"
119120
sa := "serviceAccount"
120121
mpc := struct{ fakespec.MockProvisionerClient }{}
122+
extraParamName := "ParamName"
123+
extraParamValue := "ParamValue"
121124

122125
testCases := []struct {
123126
name string
@@ -126,6 +129,7 @@ func TestAdd(t *testing.T) {
126129
grantFunc func(ctx context.Context, in *osspec.ProvisionerGrantBucketAccessRequest, opts ...grpc.CallOption) (*osspec.ProvisionerGrantBucketAccessResponse, error)
127130
principal string
128131
serviceAccount string
132+
params map[string]string
129133
}{
130134
{
131135
name: "S3",
@@ -158,6 +162,9 @@ func TestAdd(t *testing.T) {
158162
if in.BucketContext["Endpoint"] != endpoint {
159163
t.Errorf("expected %s, got %s", endpoint, in.BucketContext["Endpoint"])
160164
}
165+
if in.BucketContext[extraParamName] != extraParamValue {
166+
t.Errorf("expected %s, got %s", extraParamValue, in.BucketContext[extraParamName])
167+
}
161168
return &osspec.ProvisionerGrantBucketAccessResponse{
162169
Principal: principal,
163170
CredentialsFileContents: credsContents,
@@ -166,6 +173,9 @@ func TestAdd(t *testing.T) {
166173
},
167174
principal: principal,
168175
serviceAccount: "",
176+
params: map[string]string{
177+
extraParamName: extraParamValue,
178+
},
169179
},
170180
{
171181
name: "GCS",
@@ -194,6 +204,9 @@ func TestAdd(t *testing.T) {
194204
if in.BucketContext["ProjectID"] != projID {
195205
t.Errorf("expected %s, got %s", projID, in.BucketContext["ProjectID"])
196206
}
207+
if in.BucketContext[extraParamName] != extraParamValue {
208+
t.Errorf("expected %s, got %s", extraParamValue, in.BucketContext[extraParamName])
209+
}
197210
return &osspec.ProvisionerGrantBucketAccessResponse{
198211
Principal: principal,
199212
CredentialsFileContents: credsContents,
@@ -202,6 +215,9 @@ func TestAdd(t *testing.T) {
202215
},
203216
principal: principal,
204217
serviceAccount: "",
218+
params: map[string]string{
219+
extraParamName: extraParamValue,
220+
},
205221
},
206222
{
207223
name: "AzureBlob",
@@ -222,6 +238,9 @@ func TestAdd(t *testing.T) {
222238
if in.BucketContext["StorageAccount"] != account {
223239
t.Errorf("expected %s, got %s", account, in.BucketContext["StorageAccount"])
224240
}
241+
if in.BucketContext[extraParamName] != extraParamValue {
242+
t.Errorf("expected %s, got %s", extraParamValue, in.BucketContext[extraParamName])
243+
}
225244
return &osspec.ProvisionerGrantBucketAccessResponse{
226245
Principal: principal,
227246
CredentialsFileContents: credsContents,
@@ -230,6 +249,9 @@ func TestAdd(t *testing.T) {
230249
},
231250
principal: principal,
232251
serviceAccount: "",
252+
params: map[string]string{
253+
extraParamName: extraParamValue,
254+
},
233255
},
234256
{
235257
name: "No Principal",
@@ -252,6 +274,9 @@ func TestAdd(t *testing.T) {
252274
},
253275
principal: "",
254276
serviceAccount: "",
277+
params: map[string]string{
278+
extraParamName: extraParamValue,
279+
},
255280
},
256281
{
257282
name: "ServiceAccount exists",
@@ -274,6 +299,9 @@ func TestAdd(t *testing.T) {
274299
},
275300
principal: principal,
276301
serviceAccount: sa,
302+
params: map[string]string{
303+
extraParamName: extraParamValue,
304+
},
277305
},
278306
}
279307

@@ -297,10 +325,16 @@ func TestAdd(t *testing.T) {
297325
BucketInstanceName: instanceName,
298326
Provisioner: provisioner,
299327
Principal: tc.principal,
300-
ServiceAccount: tc.serviceAccount,
328+
Parameters: tc.params,
301329
},
302330
}
303331

332+
if len(tc.serviceAccount) > 0 {
333+
ba.Spec.ServiceAccount = &corev1.ObjectReference{
334+
Name: tc.serviceAccount,
335+
}
336+
}
337+
304338
ctx := context.TODO()
305339
tc.setProtocol(&b)
306340
client := fakebucketclientset.NewSimpleClientset(&ba, &b)
@@ -383,13 +417,16 @@ func TestDelete(t *testing.T) {
383417
endpoint := "endpoint1"
384418
instanceName := "instance"
385419
mpc := struct{ fakespec.MockProvisionerClient }{}
420+
extraParamName := "ParamName"
421+
extraParamValue := "ParamValue"
386422

387423
testCases := []struct {
388424
name string
389425
setProtocol func(b *v1alpha1.Bucket)
390426
protocolName v1alpha1.ProtocolName
391427
revokeFunc func(ctx context.Context, in *osspec.ProvisionerRevokeBucketAccessRequest, opts ...grpc.CallOption) (*osspec.ProvisionerRevokeBucketAccessResponse, error)
392428
serviceAccount string
429+
params map[string]string
393430
}{
394431
{
395432
name: "S3",
@@ -422,9 +459,15 @@ func TestDelete(t *testing.T) {
422459
if in.BucketContext["Endpoint"] != endpoint {
423460
t.Errorf("expected %s, got %s", endpoint, in.BucketContext["Endpoint"])
424461
}
462+
if in.BucketContext[extraParamName] != extraParamValue {
463+
t.Errorf("expected %s, got %s", extraParamValue, in.BucketContext[extraParamName])
464+
}
425465
return &osspec.ProvisionerRevokeBucketAccessResponse{}, nil
426466
},
427467
serviceAccount: "",
468+
params: map[string]string{
469+
extraParamName: extraParamValue,
470+
},
428471
},
429472
{
430473
name: "GCS",
@@ -453,9 +496,15 @@ func TestDelete(t *testing.T) {
453496
if in.BucketContext["ProjectID"] != projID {
454497
t.Errorf("expected %s, got %s", projID, in.BucketContext["ProjectID"])
455498
}
499+
if in.BucketContext[extraParamName] != extraParamValue {
500+
t.Errorf("expected %s, got %s", extraParamValue, in.BucketContext[extraParamName])
501+
}
456502
return &osspec.ProvisionerRevokeBucketAccessResponse{}, nil
457503
},
458504
serviceAccount: "",
505+
params: map[string]string{
506+
extraParamName: extraParamValue,
507+
},
459508
},
460509
{
461510
name: "AzureBlob",
@@ -476,9 +525,15 @@ func TestDelete(t *testing.T) {
476525
if in.BucketContext["StorageAccount"] != account {
477526
t.Errorf("expected %s, got %s", account, in.BucketContext["StorageAccount"])
478527
}
528+
if in.BucketContext[extraParamName] != extraParamValue {
529+
t.Errorf("expected %s, got %s", extraParamValue, in.BucketContext[extraParamName])
530+
}
479531
return &osspec.ProvisionerRevokeBucketAccessResponse{}, nil
480532
},
481533
serviceAccount: "",
534+
params: map[string]string{
535+
extraParamName: extraParamValue,
536+
},
482537
},
483538
{
484539
name: "service account exists",
@@ -511,9 +566,15 @@ func TestDelete(t *testing.T) {
511566
if in.BucketContext["Endpoint"] != endpoint {
512567
t.Errorf("expected %s, got %s", endpoint, in.BucketContext["Endpoint"])
513568
}
569+
if in.BucketContext[extraParamName] != extraParamValue {
570+
t.Errorf("expected %s, got %s", extraParamValue, in.BucketContext[extraParamName])
571+
}
514572
return &osspec.ProvisionerRevokeBucketAccessResponse{}, nil
515573
},
516574
serviceAccount: "serviceAccount",
575+
params: map[string]string{
576+
extraParamName: extraParamValue,
577+
},
517578
},
518579
}
519580

@@ -537,12 +598,18 @@ func TestDelete(t *testing.T) {
537598
BucketInstanceName: instanceName,
538599
Provisioner: provisioner,
539600
Principal: principal,
540-
ServiceAccount: tc.serviceAccount,
601+
Parameters: tc.params,
541602
},
542603
Status: v1alpha1.BucketAccessStatus{
543604
AccessGranted: true,
544605
},
545606
}
607+
608+
if len(tc.serviceAccount) > 0 {
609+
ba.Spec.ServiceAccount = &corev1.ObjectReference{
610+
Name: tc.serviceAccount,
611+
}
612+
}
546613
secretName := generateSecretName(ba.UID)
547614
secret := v1.Secret{
548615
ObjectMeta: metav1.ObjectMeta{

0 commit comments

Comments
 (0)