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

Commit 795a718

Browse files
author
Krish Chowdhary
committed
RBAC cleaning, minor bug fixes, adds minimal logic for grant access
1 parent cb8b128 commit 795a718

File tree

10 files changed

+76
-47
lines changed

10 files changed

+76
-47
lines changed

Diff for: ‎cmd/minio-cosi-driver/internal/provisioner.go

+13-9
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,14 @@ package internal
1616
import (
1717
"context"
1818

19-
"k8s.io/klog/v2"
20-
"sigs.k8s.io/container-object-storage-interface-provisioner-sidecar/cmd/minio-cosi-driver/internal/minio"
21-
cosi "sigs.k8s.io/container-object-storage-interface-spec"
22-
2319
"github.com/pkg/errors"
2420
"google.golang.org/grpc/codes"
2521
"google.golang.org/grpc/status"
22+
"k8s.io/klog/v2"
23+
24+
cosi "sigs.k8s.io/container-object-storage-interface-spec"
25+
26+
"sigs.k8s.io/container-object-storage-interface-provisioner-sidecar/cmd/minio-cosi-driver/internal/minio"
2627
)
2728

2829
type ProvisionerServer struct {
@@ -40,9 +41,6 @@ type ProvisionerServer struct {
4041
func (s *ProvisionerServer) ProvisionerCreateBucket(ctx context.Context,
4142
req *cosi.ProvisionerCreateBucketRequest) (*cosi.ProvisionerCreateBucketResponse, error) {
4243

43-
bucketName := req.GetName()
44-
klog.V(3).InfoS("Create Bucket", "name", bucketName)
45-
4644
protocol := req.GetProtocol()
4745
if protocol == nil {
4846
klog.ErrorS(errors.New("Invalid Argument"), "Protocol is nil")
@@ -54,6 +52,9 @@ func (s *ProvisionerServer) ProvisionerCreateBucket(ctx context.Context,
5452
return nil, status.Error(codes.InvalidArgument, "S3 Protocol is nil")
5553
}
5654

55+
bucketName := s3.BucketName
56+
klog.V(3).InfoS("Create Bucket", "name", bucketName)
57+
5758
options := minio.MakeBucketOptions{}
5859

5960
// MinIO regions, unlike AWS s3 does not strictly require the
@@ -91,7 +92,7 @@ func (s *ProvisionerServer) ProvisionerCreateBucket(ctx context.Context,
9192
klog.InfoS("Bucket already exists", "name", bucketName)
9293
return &cosi.ProvisionerCreateBucketResponse{
9394
BucketId: bucketID,
94-
}, status.Error(codes.AlreadyExists, "Bucket already exists")
95+
}, nil
9596
}
9697
klog.ErrorS(err, "Bucket creation failed")
9798
return nil, status.Error(codes.Internal, "Bucket creation failed")
@@ -111,7 +112,10 @@ func (s *ProvisionerServer) ProvisionerDeleteBucket(ctx context.Context,
111112
func (s *ProvisionerServer) ProvisionerGrantBucketAccess(ctx context.Context,
112113
req *cosi.ProvisionerGrantBucketAccessRequest) (*cosi.ProvisionerGrantBucketAccessResponse, error) {
113114

114-
return &cosi.ProvisionerGrantBucketAccessResponse{}, nil
115+
return &cosi.ProvisionerGrantBucketAccessResponse{
116+
AccountId: "minio",
117+
CredentialsFileContents: "{\"username\":\"minio\", \"password\": \"minio123\"}",
118+
}, nil
115119
}
116120

117121
func (s *ProvisionerServer) ProvisionerRevokeBucketAccess(ctx context.Context,

Diff for: ‎go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,6 @@ require (
2626
k8s.io/apimachinery v0.19.4
2727
k8s.io/client-go v0.19.4
2828
k8s.io/klog/v2 v2.2.0
29-
sigs.k8s.io/container-object-storage-interface-api v0.0.0-20210330175159-2cdabb1a5dc7
29+
sigs.k8s.io/container-object-storage-interface-api v0.0.0-20210417043410-0af83d5058ab
3030
sigs.k8s.io/container-object-storage-interface-spec v0.0.0-20210330184956-b0de747ccee4
3131
)

Diff for: ‎go.sum

+3-2
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,7 @@ k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE=
717717
k8s.io/klog/v2 v2.2.0 h1:XRvcwJozkgZ1UQJmfMGpvRthQHOvihEhYtDfAaxMz/A=
718718
k8s.io/klog/v2 v2.2.0/go.mod h1:Od+F08eJP+W3HUb4pSrPpgp9DGU4GzlpG/TmITuYh/Y=
719719
k8s.io/kube-openapi v0.0.0-20200410145947-61e04a5be9a6/go.mod h1:GRQhZsXIAJ1xR0C9bd8UpWHZ5plfAS9fzPjJuQ6JL3E=
720+
k8s.io/kube-openapi v0.0.0-20200805222855-6aeccd4b50c6 h1:+WnxoVtG8TMiudHBSEtrVL1egv36TkkJm+bA8AxicmQ=
720721
k8s.io/kube-openapi v0.0.0-20200805222855-6aeccd4b50c6/go.mod h1:UuqjUnNftUyPE5H64/qeyjQoUZhGpeFDVdxjTeEVN2o=
721722
k8s.io/kube-openapi v0.0.0-20200923155610-8b5066479488 h1:mNpvQf4lkIHNOXCoM+Veu/UXwA56Yx1J7hY1Tvcs/oM=
722723
k8s.io/kube-openapi v0.0.0-20200923155610-8b5066479488/go.mod h1:UuqjUnNftUyPE5H64/qeyjQoUZhGpeFDVdxjTeEVN2o=
@@ -726,8 +727,8 @@ k8s.io/utils v0.0.0-20200729134348-d5654de09c73 h1:uJmqzgNWG7XyClnU/mLPBWwfKKF1K
726727
k8s.io/utils v0.0.0-20200729134348-d5654de09c73/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
727728
rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=
728729
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.7/go.mod h1:PHgbrJT7lCHcxMU+mDHEm+nx46H4zuuHZkDP6icnhu0=
729-
sigs.k8s.io/container-object-storage-interface-api v0.0.0-20210330175159-2cdabb1a5dc7 h1:M2ZMhWdq9Az8TFj8G6ZffFUpR4XG7Qy8h8ZGsZhi9Xg=
730-
sigs.k8s.io/container-object-storage-interface-api v0.0.0-20210330175159-2cdabb1a5dc7/go.mod h1:5n4lNKN4uOMW2NTqJ9r8qRAiqh5dZRZB7CNOkFihLfM=
730+
sigs.k8s.io/container-object-storage-interface-api v0.0.0-20210417043410-0af83d5058ab h1:mKZ+ua1nJHPYNb5NkyxFsObb0bElpCiwt8k6xeNsH1Y=
731+
sigs.k8s.io/container-object-storage-interface-api v0.0.0-20210417043410-0af83d5058ab/go.mod h1:WTzZGS4Q6MdQqDihJdMh2kCvqMx9Amhx0KIainA4lXQ=
731732
sigs.k8s.io/container-object-storage-interface-spec v0.0.0-20210329232956-3bbacbbc9c19/go.mod h1:kafkL5l/lTUrZXhVi/9p1GzpEE/ts29BkWkL3Ao33WU=
732733
sigs.k8s.io/container-object-storage-interface-spec v0.0.0-20210330184956-b0de747ccee4 h1:U+M87V77xKotSub2dqNlmxHMbb30QeC7wwTWdPGAhSI=
733734
sigs.k8s.io/container-object-storage-interface-spec v0.0.0-20210330184956-b0de747ccee4/go.mod h1:kafkL5l/lTUrZXhVi/9p1GzpEE/ts29BkWkL3Ao33WU=

Diff for: ‎pkg/bucket/bucket_controller.go

+10-6
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ func (b *BucketListener) Add(ctx context.Context, inputBucket *v1alpha1.Bucket)
103103
"bucket", bucket.Name)
104104
return errors.Wrap(err, "Failed to create bucket")
105105
}
106+
106107
}
107108
if rsp == nil {
108109
err := errors.New("ProvisionerCreateBucket returned a nil response")
@@ -111,16 +112,17 @@ func (b *BucketListener) Add(ctx context.Context, inputBucket *v1alpha1.Bucket)
111112
}
112113

113114
if rsp.BucketId != "" {
114-
bucket.Spec.BucketID = rsp.BucketId
115+
bucket.Status.BucketID = rsp.BucketId
115116
}
117+
116118
bucket.Status.Message = "Bucket Provisioned"
117119
bucket.Status.BucketAvailable = true
118120

119121
// if this step fails, then controller will retry with backoff
120-
if _, err := b.Buckets().Update(ctx, bucket, metav1.UpdateOptions{}); err != nil {
121-
klog.ErrorS(err, "Failed to update bucket",
122+
if _, err := b.Buckets().UpdateStatus(ctx, bucket, metav1.UpdateOptions{}); err != nil {
123+
klog.ErrorS(err, "Failed to update bucket status",
122124
"bucket", bucket.Name)
123-
return errors.Wrap(err, "Failed to update bucket")
125+
return errors.Wrap(err, "Failed to update bucket status")
124126
}
125127

126128
return nil
@@ -158,7 +160,7 @@ func (b *BucketListener) Delete(ctx context.Context, inputBucket *v1alpha1.Bucke
158160
}
159161

160162
req := &cosi.ProvisionerDeleteBucketRequest{
161-
BucketId: bucket.Spec.BucketID,
163+
BucketId: bucket.Status.BucketID,
162164
}
163165

164166
if _, err := b.provisionerClient.ProvisionerDeleteBucket(ctx, req); err != nil {
@@ -170,10 +172,12 @@ func (b *BucketListener) Delete(ctx context.Context, inputBucket *v1alpha1.Bucke
170172
}
171173
}
172174

175+
// TODO, check bucket.Spec.DeletionPolicy
176+
173177
bucket.Status.BucketAvailable = false
174178

175179
// if this step fails, then controller will retry with backoff
176-
if _, err := b.Buckets().Update(ctx, bucket, metav1.UpdateOptions{}); err != nil {
180+
if _, err := b.Buckets().UpdateStatus(ctx, bucket, metav1.UpdateOptions{}); err != nil {
177181
klog.ErrorS(err, "Failed to update bucket",
178182
"bucket", bucket.Name)
179183
return errors.Wrap(err, "Failed to update bucket")

Diff for: ‎pkg/bucket/bucket_controller_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ func TestBucketDeletion(t *testing.T) {
139139
{
140140
name: "BucketDeletion",
141141
setFields: func(b *v1alpha1.Bucket) {
142-
b.Spec.BucketID = bucketId
142+
b.Status.BucketID = bucketId
143143
},
144144
deleteFunc: func(ctx context.Context,
145145
req *cosi.ProvisionerDeleteBucketRequest,

Diff for: ‎pkg/bucketaccess/bucketaccess_controller.go

+23-8
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func (bal *BucketAccessListener) Add(ctx context.Context, inputBucketAccess *v1a
8282
"bucket", bucketName,
8383
)
8484

85-
if bucketAccess.Spec.MintedSecretName != "" {
85+
if bucketAccess.Status.MintedSecret != nil {
8686
klog.V(5).InfoS("AccessAlreadyGranted",
8787
"bucketAccess", bucketAccess.Name,
8888
"bucket", bucketName,
@@ -112,7 +112,7 @@ func (bal *BucketAccessListener) Add(ctx context.Context, inputBucketAccess *v1a
112112
return nil
113113
}
114114

115-
if bucket.Spec.BucketID == "" {
115+
if bucket.Status.BucketID == "" {
116116
err := errors.New("BucketID cannot be empty")
117117
klog.ErrorS(err,
118118
"Invalid arguments",
@@ -123,7 +123,7 @@ func (bal *BucketAccessListener) Add(ctx context.Context, inputBucketAccess *v1a
123123
}
124124

125125
req := &cosi.ProvisionerGrantBucketAccessRequest{
126-
BucketId: bucket.Spec.BucketID,
126+
BucketId: bucket.Status.BucketID,
127127
AccountName: bucketAccess.Name,
128128
AccessPolicy: bucketAccess.Spec.PolicyActionsConfigMapData,
129129
}
@@ -177,16 +177,19 @@ func (bal *BucketAccessListener) Add(ctx context.Context, inputBucketAccess *v1a
177177
}
178178
}
179179

180-
bucketAccess.Spec.AccountID = rsp.AccountId
180+
bucketAccess.Status.AccountID = rsp.AccountId
181+
bucketAccess.Status.MintedSecret = &corev1.SecretReference{
182+
Namespace: bal.namespace,
183+
Name: mintedSecretName,
184+
}
181185
bucketAccess.Status.AccessGranted = true
182-
bucketAccess.Spec.MintedSecretName = mintedSecretName
183186

184187
// if this step fails, then controller will retry with backoff
185-
if _, err := bal.BucketAccesses().Update(ctx, bucketAccess, metav1.UpdateOptions{}); err != nil {
186-
klog.ErrorS(err, "Failed to update BucketAccess",
188+
if _, err := bal.BucketAccesses().UpdateStatus(ctx, bucketAccess, metav1.UpdateOptions{}); err != nil {
189+
klog.ErrorS(err, "Failed to update BucketAccess Status",
187190
"bucketAccess", bucketAccess.Name,
188191
"bucket", bucket.Name)
189-
return errors.Wrap(err, "Failed to update BucketAccess")
192+
return errors.Wrap(err, "Failed to update BucketAccess Status")
190193
}
191194

192195
return nil
@@ -212,6 +215,18 @@ func (bal *BucketAccessListener) Delete(ctx context.Context, bucketAccess *v1alp
212215
"name", bucketAccess.Name,
213216
"bucket", bucketAccess.Spec.BucketName,
214217
)
218+
219+
// TODO, check bucketAccess.Spec.DeletionPolicy
220+
221+
bucketAccess.Status.AccessGranted = false
222+
223+
// if this step fails, then controller will retry with backoff
224+
if _, err := bal.BucketAccesses().UpdateStatus(ctx, bucketAccess, metav1.UpdateOptions{}); err != nil {
225+
klog.ErrorS(err, "Failed to update BucketAccess Status",
226+
"bucketAccess", bucketAccess.Name)
227+
return errors.Wrap(err, "Failed to update BucketAccess Status")
228+
}
229+
215230
return nil
216231
}
217232

Diff for: ‎pkg/bucketaccess/bucketaccess_controller_test.go

+13-10
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,18 @@ import (
2121
"strings"
2222
"testing"
2323

24-
"sigs.k8s.io/container-object-storage-interface-api/apis/objectstorage.k8s.io/v1alpha1"
25-
fakebucketclientset "sigs.k8s.io/container-object-storage-interface-api/clientset/fake"
26-
cosi "sigs.k8s.io/container-object-storage-interface-spec"
27-
fakespec "sigs.k8s.io/container-object-storage-interface-spec/fake"
28-
24+
"google.golang.org/grpc"
2925
corev1 "k8s.io/api/core/v1"
3026
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3127
utilversion "k8s.io/apimachinery/pkg/util/version"
3228
"k8s.io/apimachinery/pkg/version"
3329
fakediscovery "k8s.io/client-go/discovery/fake"
3430
fakekubeclientset "k8s.io/client-go/kubernetes/fake"
3531

36-
"google.golang.org/grpc"
32+
"sigs.k8s.io/container-object-storage-interface-api/apis/objectstorage.k8s.io/v1alpha1"
33+
fakebucketclientset "sigs.k8s.io/container-object-storage-interface-api/clientset/fake"
34+
cosi "sigs.k8s.io/container-object-storage-interface-spec"
35+
fakespec "sigs.k8s.io/container-object-storage-interface-spec/fake"
3736
)
3837

3938
func TestInitializeKubeClient(t *testing.T) {
@@ -97,7 +96,9 @@ func TestAddWrongProvisioner(t *testing.T) {
9796
Spec: v1alpha1.BucketSpec{
9897
Provisioner: provisioner + "-invalid",
9998
Protocol: v1alpha1.Protocol{},
100-
BucketID: bucketId,
99+
},
100+
Status: v1alpha1.BucketStatus{
101+
BucketID: bucketId,
101102
},
102103
}
103104

@@ -172,7 +173,9 @@ func TestAddBucketAccess(t *testing.T) {
172173
Spec: v1alpha1.BucketSpec{
173174
Provisioner: provisioner,
174175
Protocol: v1alpha1.Protocol{},
175-
BucketID: bucketId,
176+
},
177+
Status: v1alpha1.BucketStatus{
178+
BucketID: bucketId,
176179
},
177180
}
178181

@@ -211,8 +214,8 @@ func TestAddBucketAccess(t *testing.T) {
211214
if updatedBA.Status.AccessGranted != true {
212215
t.Errorf("Expected %t, got %t", true, ba.Status.AccessGranted)
213216
}
214-
if !strings.EqualFold(updatedBA.Spec.AccountID, accountId) {
215-
t.Errorf("Expected %s, got %s", accountId, updatedBA.Spec.AccountID)
217+
if !strings.EqualFold(updatedBA.Status.AccountID, accountId) {
218+
t.Errorf("Expected %s, got %s", accountId, updatedBA.Status.AccountID)
216219
}
217220

218221
secretName := "ba-" + string(ba.UID)

Diff for: ‎resources/deployment.yaml

+2
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ spec:
3838
containers:
3939
- name: minio-cosi-driver
4040
image: $(IMAGE_ORG)/minio-cosi-driver:$(IMAGE_VERSION)
41+
imagePullPolicy: Always
4142
envFrom:
4243
- secretRef:
4344
name: objectstorage-provisioner
@@ -46,6 +47,7 @@ spec:
4647
name: socket
4748
- name: objectstorage-provisioner-sidecar
4849
image: $(IMAGE_ORG)/objectstorage-sidecar:$(IMAGE_VERSION)
50+
imagePullPolicy: Always
4951
envFrom:
5052
- secretRef:
5153
name: objectstorage-provisioner

Diff for: ‎resources/minio.yaml

+8-2
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ spec:
3434
- /data
3535
env:
3636
- name: MINIO_ACCESS_KEY
37-
value: minio
37+
valueFrom:
38+
secretKeyRef:
39+
name: objectstorage-provisioner
40+
key: MINIO_ACCESS_KEY
3841
- name: MINIO_SECRET_KEY
39-
value: minio123
42+
valueFrom:
43+
secretKeyRef:
44+
name: objectstorage-provisioner
45+
key: MINIO_SECRET_KEY

Diff for: ‎resources/rbac.yaml

+2-8
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,13 @@ metadata:
1010
app.kubernetes.io/name: container-object-storage-interface-provisioner
1111
rules:
1212
- apiGroups: ["objectstorage.k8s.io"]
13-
resources: ["buckets", "bucketaccess"]
13+
resources: ["buckets", "bucketaccesses","buckets/status", "bucketaccesses/status"]
1414
verbs: ["get", "list", "watch", "update", "create", "delete"]
15-
- apiGroups: [""]
16-
resources: ["events"]
17-
verbs: ["list", "watch", "create", "update", "patch"]
1815
- apiGroups: ["coordination.k8s.io"]
1916
resources: ["leases"]
2017
verbs: ["get", "watch", "list", "delete", "update", "create"]
21-
- apiGroups: ["objectstorage.k8s.io"]
22-
resources: ["bucketaccesses"]
23-
verbs: ["get", "watch", "list", "delete", "update", "create"]
2418
- apiGroups: [""]
25-
resources: ["secrets"]
19+
resources: ["secrets", "events"]
2620
verbs: ["get", "delete", "update", "create"]
2721
---
2822
kind: ClusterRoleBinding

0 commit comments

Comments
 (0)