Skip to content

Commit 6ccec8e

Browse files
Eneman DonatienEneman Donatien
Eneman Donatien
authored and
Eneman Donatien
committed
merge of main branch
2 parents f6251ce + a683dec commit 6ccec8e

File tree

9 files changed

+541
-402
lines changed

9 files changed

+541
-402
lines changed

‎Makefile‎

Lines changed: 285 additions & 285 deletions
Large diffs are not rendered by default.

‎README.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,10 +229,22 @@ spec:
229229
# Content of the policy, as a multiline string
230230
# This should be IAM compliant JSON - follow the guidelines of the actual
231231
# S3 provider you're using, as sometimes only a subset is available.
232+
The first Statement (Allow ListBucket) should be applied to every user,
233+
# as s3-operator uses this call to verify that credentials are valid when
234+
# reconciling an existing user.
232235
policyContent: >-
233236
{
234237
"Version": "2012-10-17",
235238
"Statement": [
239+
{
240+
"Effect": "Allow",
241+
"Action": [
242+
"s3:ListBucket"
243+
],
244+
"Resource": [
245+
"arn:aws:s3:::*"
246+
]
247+
},
236248
{
237249
"Effect": "Allow",
238250
"Action": [

‎api/v1alpha1/s3user_types.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,20 @@ type S3UserSpec struct {
4545
// +kubebuilder:validation:MinLength=1
4646
// +kubebuilder:validation:MaxLength=127
4747
S3InstanceRef string `json:"s3InstanceRef,omitempty"`
48+
49+
// SecretFieldNameAccessKey associated to the S3User
50+
// Allow overridden the default key to store the accessKey value in the secret
51+
// +kubebuilder:validation:Optional
52+
// +kubebuilder:validation:Type="string"
53+
// +kubebuilder:default="accessKey"
54+
SecretFieldNameAccessKey string `json:"secretFieldNameAccessKey,omitempty"`
55+
56+
// SecretFieldNameSecretKey associated to the S3User
57+
// Allow overridden the default key to store the secretKey value in the secret
58+
// +kubebuilder:validation:Optional
59+
// +kubebuilder:validation:Type="string"
60+
// +kubebuilder:default="secretKey"
61+
SecretFieldNameSecretKey string `json:"secretFieldNameSecretKey,omitempty"`
4862
}
4963

5064
// S3UserStatus defines the observed state of S3User

‎config/crd/bases/s3.onyxia.sh_s3users.yaml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,18 @@ spec:
5757
x-kubernetes-validations:
5858
- message: s3InstanceRef is immutable
5959
rule: self == oldSelf
60+
secretFieldNameAccessKey:
61+
default: accessKey
62+
description: |-
63+
SecretFieldNameAccessKey associated to the S3User
64+
Allow overridden the default key to store the accessKey value in the secret
65+
type: string
66+
secretFieldNameSecretKey:
67+
default: secretKey
68+
description: |-
69+
SecretFieldNameSecretKey associated to the S3User
70+
Allow overridden the default key to store the secretKey value in the secret
71+
type: string
6072
secretName:
6173
description: SecretName associated to the S3User
6274
type: string

‎internal/controller/user/finalizer.go‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,46 @@ func (r *S3UserReconciler) handleDeletion(
7878
)
7979
}
8080

81+
err := r.deleteOldLinkedSecret(ctx, userResource)
82+
if err != nil {
83+
logger.Error(
84+
err,
85+
"An error occurred when trying to clean old secret linked to user",
86+
"userResourceName",
87+
userResource.Name,
88+
"NamespacedName",
89+
req.NamespacedName.String(),
90+
)
91+
return r.SetReconciledCondition(
92+
ctx,
93+
req,
94+
userResource,
95+
s3v1alpha1.DeletionFailure,
96+
"Deletion of old secret associated to user have failed",
97+
err,
98+
)
99+
}
100+
101+
userOwnedSecret, _ := r.getUserSecret(ctx, userResource)
102+
if err := r.deleteSecret(ctx, &userOwnedSecret); err != nil {
103+
logger.Error(
104+
err,
105+
"An error occurred when trying to clean secret linked to user",
106+
"userResourceName",
107+
userResource.Name,
108+
"NamespacedName",
109+
req.NamespacedName.String(),
110+
)
111+
return r.SetReconciledCondition(
112+
ctx,
113+
req,
114+
userResource,
115+
s3v1alpha1.DeletionFailure,
116+
"Deletion of secret associated to user have failed",
117+
err,
118+
)
119+
}
120+
81121
//Remove userFinalizer. Once all finalizers have been removed, the object will be deleted.
82122
if ok := controllerutil.RemoveFinalizer(userResource, userFinalizer); !ok {
83123
logger.Info(
@@ -94,7 +134,7 @@ func (r *S3UserReconciler) handleDeletion(
94134
// calling r.Update() for adding/removal of finalizer is not necessary (an update event is generated
95135
// with the call to AddFinalizer/RemoveFinalizer), and worse, causes "freshness" problem (with the
96136
// "the object has been modified; please apply your changes to the latest version and try again" error)
97-
err := r.Update(ctx, userResource)
137+
err = r.Update(ctx, userResource)
98138
if err != nil {
99139
logger.Error(
100140
err,

‎internal/controller/user/finalizer_test.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,12 @@ func TestHandleDelete(t *testing.T) {
3636
UID: "6c8dceca-f7df-469d-80a5-1afed9e4d710",
3737
},
3838
Spec: s3v1alpha1.S3UserSpec{
39-
AccessKey: "existing-valid-user",
40-
Policies: []string{"admin"},
41-
SecretName: "existing-valid-user-credentials",
42-
S3InstanceRef: "s3-operator/default",
39+
AccessKey: "existing-valid-user",
40+
Policies: []string{"admin"},
41+
SecretName: "existing-valid-user-credentials",
42+
S3InstanceRef: "s3-operator/default",
43+
SecretFieldNameAccessKey: "accessKey",
44+
SecretFieldNameSecretKey: "secretKey",
4345
},
4446
}
4547

‎internal/controller/user/reconcile.go‎

Lines changed: 93 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,26 @@ func (r *S3UserReconciler) handleUpdate(
247247
)
248248
}
249249

250+
err = r.deleteOldLinkedSecret(ctx, userResource)
251+
if err != nil {
252+
logger.Error(
253+
err,
254+
"An error occurred when trying to clean old secret linked to user",
255+
"userResourceName",
256+
userResource.Name,
257+
"NamespacedName",
258+
req.NamespacedName.String(),
259+
)
260+
return r.SetReconciledCondition(
261+
ctx,
262+
req,
263+
userResource,
264+
s3v1alpha1.Unreachable,
265+
"Deletion of old secret associated to user have failed",
266+
err,
267+
)
268+
}
269+
250270
userOwnedSecret, err := r.getUserSecret(ctx, userResource)
251271
if err != nil {
252272
if err.Error() == "SecretListingFailed" {
@@ -359,82 +379,6 @@ func (r *S3UserReconciler) handleUpdate(
359379
return r.handleCreate(ctx, req, userResource)
360380
}
361381

362-
credentialsValid, err := s3Client.CheckUserCredentialsValid(
363-
userResource.Name,
364-
string(userOwnedSecret.Data["accessKey"]),
365-
string(userOwnedSecret.Data["secretKey"]),
366-
)
367-
if err != nil {
368-
logger.Error(
369-
err,
370-
"An error occurred when checking if user credentials were valid",
371-
"userResource",
372-
userResource.Name,
373-
"NamespacedName",
374-
req.NamespacedName.String(),
375-
)
376-
return r.SetReconciledCondition(
377-
ctx,
378-
req,
379-
userResource,
380-
s3v1alpha1.Unreachable,
381-
"Checking credentials on S3 server has failed",
382-
err,
383-
)
384-
}
385-
386-
if !credentialsValid {
387-
logger.Info(
388-
"The secret containing the credentials will be deleted, and the user will be deleted from the S3 backend, then recreated (through another reconcile)",
389-
"userResource",
390-
userResource.Name,
391-
"NamespacedName",
392-
req.NamespacedName.String(),
393-
)
394-
err = r.deleteSecret(ctx, &userOwnedSecret)
395-
if err != nil {
396-
logger.Error(err, "Deletion of secret associated to user have failed", "userResource",
397-
userResource.Name,
398-
"userResourceName",
399-
userResource.Name,
400-
"NamespacedName",
401-
req.NamespacedName.String())
402-
return r.SetReconciledCondition(
403-
ctx,
404-
req,
405-
userResource,
406-
s3v1alpha1.Unreachable,
407-
"Deletion of secret associated to user have failed",
408-
err,
409-
)
410-
411-
}
412-
err = s3Client.DeleteUser(userResource.Spec.AccessKey)
413-
if err != nil {
414-
logger.Error(err, "Could not delete user on S3 server", "userResource",
415-
userResource.Name,
416-
"userResourceName",
417-
userResource.Name,
418-
"NamespacedName",
419-
req.NamespacedName.String())
420-
return r.SetReconciledCondition(
421-
ctx,
422-
req,
423-
userResource,
424-
s3v1alpha1.Unreachable,
425-
fmt.Sprintf(
426-
"Deletion of S3user %s on S3 server has failed",
427-
userResource.Name,
428-
),
429-
err,
430-
)
431-
432-
}
433-
return r.handleCreate(ctx, req, userResource)
434-
}
435-
436-
// --- End Secret management section
437-
438382
logger.Info("Checking user policies", "userResource",
439383
userResource.Name,
440384
"NamespacedName",
@@ -531,28 +475,88 @@ func (r *S3UserReconciler) handleUpdate(
531475
}
532476
}
533477

534-
logger.Info("User was reconciled without error",
535-
"userResource",
478+
credentialsValid, err := s3Client.CheckUserCredentialsValid(
536479
userResource.Name,
537-
"NamespacedName",
538-
req.NamespacedName.String(),
480+
string(userOwnedSecret.Data[userResource.Spec.SecretFieldNameAccessKey]),
481+
string(userOwnedSecret.Data[userResource.Spec.SecretFieldNameSecretKey]),
539482
)
540483

541-
// Re-fetch the S3User to ensure we have the latest state after updating the secret
542-
// This is necessary at least when creating a user with secretName targetting a pre-existing secret
543-
// that has proper form (data.accessKey and data.secretKey) but isn't owned by any other s3user
544-
if err := r.Get(ctx, req.NamespacedName, userResource); err != nil {
484+
if err != nil {
545485
logger.Error(
546486
err,
547-
"Failed to re-fetch userResource",
548-
"userResourceName",
487+
"An error occurred when checking if user credentials were valid",
488+
"userResource",
549489
userResource.Name,
550490
"NamespacedName",
551491
req.NamespacedName.String(),
552492
)
553-
return ctrl.Result{}, err
493+
return r.SetReconciledCondition(
494+
ctx,
495+
req,
496+
userResource,
497+
s3v1alpha1.Unreachable,
498+
"Checking credentials on S3 server has failed",
499+
err,
500+
)
501+
}
502+
503+
if !credentialsValid {
504+
logger.Info(
505+
"The secret containing the credentials will be deleted, and the user will be deleted from the S3 backend, then recreated (through another reconcile)",
506+
"userResource",
507+
userResource.Name,
508+
"NamespacedName",
509+
req.NamespacedName.String(),
510+
)
511+
err = r.deleteSecret(ctx, &userOwnedSecret)
512+
if err != nil {
513+
logger.Error(err, "Deletion of secret associated to user have failed", "userResource",
514+
userResource.Name,
515+
"userResourceName",
516+
userResource.Name,
517+
"NamespacedName",
518+
req.NamespacedName.String())
519+
return r.SetReconciledCondition(
520+
ctx,
521+
req,
522+
userResource,
523+
s3v1alpha1.Unreachable,
524+
"Deletion of secret associated to user have failed",
525+
err,
526+
)
527+
528+
}
529+
err = s3Client.DeleteUser(userResource.Spec.AccessKey)
530+
if err != nil {
531+
logger.Error(err, "Could not delete user on S3 server", "userResource",
532+
userResource.Name,
533+
"userResourceName",
534+
userResource.Name,
535+
"NamespacedName",
536+
req.NamespacedName.String())
537+
return r.SetReconciledCondition(
538+
ctx,
539+
req,
540+
userResource,
541+
s3v1alpha1.Unreachable,
542+
fmt.Sprintf(
543+
"Deletion of S3user %s on S3 server has failed",
544+
userResource.Name,
545+
),
546+
err,
547+
)
548+
549+
}
550+
return r.handleCreate(ctx, req, userResource)
554551
}
555552

553+
logger.Info("User was reconciled without error",
554+
"userResource",
555+
userResource.Name,
556+
"NamespacedName",
557+
req.NamespacedName.String(),
558+
)
559+
556560
return r.SetReconciledCondition(
557561
ctx,
558562
req,
@@ -617,9 +621,8 @@ func (r *S3UserReconciler) handleCreate(
617621
ctx,
618622
userResource,
619623
map[string][]byte{
620-
"accessKey": []byte(userResource.Spec.AccessKey),
621-
"secretKey": []byte(secretKey),
622-
},
624+
userResource.Spec.SecretFieldNameAccessKey: []byte(userResource.Spec.AccessKey),
625+
userResource.Spec.SecretFieldNameSecretKey: []byte(secretKey)},
623626
)
624627
if err != nil {
625628
// Error while creating the Kubernetes secret - requeue the request.

0 commit comments

Comments
 (0)