Skip to content

Commit 9069d50

Browse files
Damian SeredynDamian Seredyn
authored andcommitted
feat: Add support for ACL from PVC
Add possibility to use ACL files mounted from PVC, not k8s secret. Implement validation webhook to ensure only one ACL storage type is used. Update E2E tests with the example that uses ACL from prepopulated PVC. Signed-off-by: Damian Seredyn <[email protected]>
1 parent 9112be5 commit 9069d50

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+1457
-53
lines changed

api/common/v1beta2/common_types.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package v1beta2
22

33
import (
4+
"fmt"
5+
46
appsv1 "k8s.io/api/apps/v1"
57
corev1 "k8s.io/api/core/v1"
68
)
@@ -274,5 +276,23 @@ type InitContainer struct {
274276

275277
// +k8s:deepcopy-gen=true
276278
type ACLConfig struct {
279+
// Secret-based ACL configuration
277280
Secret *corev1.SecretVolumeSource `json:"secret,omitempty"`
281+
// PersistentVolumeClaim-based ACL configuration
282+
// Specify the PVC name to mount ACL file from persistent storage
283+
// The operator will automatically mount /etc/redis/user.acl from the PVC
284+
PersistentVolumeClaim *string `json:"persistentVolumeClaim,omitempty"`
285+
}
286+
287+
// Validate checks that only one ACL source is specified
288+
func (a *ACLConfig) Validate() error {
289+
if a == nil {
290+
return nil
291+
}
292+
293+
if a.Secret != nil && a.PersistentVolumeClaim != nil {
294+
return fmt.Errorf("only one of 'secret' or 'persistentVolumeClaim' can be specified in ACL configuration")
295+
}
296+
297+
return nil
278298
}

api/common/v1beta2/common_types_test.go

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -786,3 +786,150 @@ func TestKubernetesConfig_AdditionalVolumeMounts_EdgeCases(t *testing.T) {
786786
})
787787
}
788788
}
789+
790+
func TestACLConfig_PersistentVolumeClaim(t *testing.T) {
791+
tests := []struct {
792+
name string
793+
aclConfig *ACLConfig
794+
expectedPVCName *string
795+
expectedSecretConfigured bool
796+
}{
797+
{
798+
name: "nil ACL config",
799+
aclConfig: nil,
800+
expectedPVCName: nil,
801+
expectedSecretConfigured: false,
802+
},
803+
{
804+
name: "empty ACL config",
805+
aclConfig: &ACLConfig{},
806+
expectedPVCName: nil,
807+
expectedSecretConfigured: false,
808+
},
809+
{
810+
name: "ACL with secret only",
811+
aclConfig: &ACLConfig{
812+
Secret: &corev1.SecretVolumeSource{
813+
SecretName: "redis-acl-secret",
814+
},
815+
},
816+
expectedPVCName: nil,
817+
expectedSecretConfigured: true,
818+
},
819+
{
820+
name: "ACL with PVC only",
821+
aclConfig: &ACLConfig{
822+
PersistentVolumeClaim: ptr.To("redis-acl-pvc"),
823+
},
824+
expectedPVCName: ptr.To("redis-acl-pvc"),
825+
expectedSecretConfigured: false,
826+
},
827+
}
828+
829+
for _, tt := range tests {
830+
t.Run(tt.name, func(t *testing.T) {
831+
if tt.aclConfig == nil {
832+
assert.Nil(t, tt.aclConfig)
833+
return
834+
}
835+
836+
// Verify PVC configuration
837+
if tt.expectedPVCName != nil {
838+
assert.NotNil(t, tt.aclConfig.PersistentVolumeClaim)
839+
assert.Equal(t, *tt.expectedPVCName, *tt.aclConfig.PersistentVolumeClaim)
840+
} else {
841+
assert.Nil(t, tt.aclConfig.PersistentVolumeClaim)
842+
}
843+
844+
// Verify Secret configuration
845+
if tt.expectedSecretConfigured {
846+
assert.NotNil(t, tt.aclConfig.Secret)
847+
} else {
848+
assert.Nil(t, tt.aclConfig.Secret)
849+
}
850+
})
851+
}
852+
}
853+
854+
func TestACLConfig_BothSourcesConfigured(t *testing.T) {
855+
// Test scenario where both Secret and PVC are configured
856+
// Validation should fail
857+
aclConfig := &ACLConfig{
858+
Secret: &corev1.SecretVolumeSource{
859+
SecretName: "redis-acl-secret",
860+
},
861+
PersistentVolumeClaim: ptr.To("redis-acl-pvc"),
862+
}
863+
864+
// Both should be settable (struct level)
865+
assert.NotNil(t, aclConfig.Secret)
866+
assert.NotNil(t, aclConfig.PersistentVolumeClaim)
867+
assert.Equal(t, "redis-acl-secret", aclConfig.Secret.SecretName)
868+
assert.Equal(t, "redis-acl-pvc", *aclConfig.PersistentVolumeClaim)
869+
870+
// But validation should fail
871+
err := aclConfig.Validate()
872+
assert.Error(t, err)
873+
assert.Contains(t, err.Error(), "only one of 'secret' or 'persistentVolumeClaim' can be specified")
874+
}
875+
876+
func TestACLConfig_Validate(t *testing.T) {
877+
tests := []struct {
878+
name string
879+
aclConfig *ACLConfig
880+
wantErr bool
881+
errMsg string
882+
}{
883+
{
884+
name: "nil config is valid",
885+
aclConfig: nil,
886+
wantErr: false,
887+
},
888+
{
889+
name: "empty config is valid",
890+
aclConfig: &ACLConfig{},
891+
wantErr: false,
892+
},
893+
{
894+
name: "only secret is valid",
895+
aclConfig: &ACLConfig{
896+
Secret: &corev1.SecretVolumeSource{
897+
SecretName: "redis-acl-secret",
898+
},
899+
},
900+
wantErr: false,
901+
},
902+
{
903+
name: "only PVC is valid",
904+
aclConfig: &ACLConfig{
905+
PersistentVolumeClaim: ptr.To("redis-acl-pvc"),
906+
},
907+
wantErr: false,
908+
},
909+
{
910+
name: "both secret and PVC is invalid",
911+
aclConfig: &ACLConfig{
912+
Secret: &corev1.SecretVolumeSource{
913+
SecretName: "redis-acl-secret",
914+
},
915+
PersistentVolumeClaim: ptr.To("redis-acl-pvc"),
916+
},
917+
wantErr: true,
918+
errMsg: "only one of 'secret' or 'persistentVolumeClaim' can be specified",
919+
},
920+
}
921+
922+
for _, tt := range tests {
923+
t.Run(tt.name, func(t *testing.T) {
924+
err := tt.aclConfig.Validate()
925+
if tt.wantErr {
926+
assert.Error(t, err)
927+
if tt.errMsg != "" {
928+
assert.Contains(t, err.Error(), tt.errMsg)
929+
}
930+
} else {
931+
assert.NoError(t, err)
932+
}
933+
})
934+
}
935+
}

api/common/v1beta2/zz_generated.deepcopy.go

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/redis/v1beta2/redis_webhook.go

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,80 @@ limitations under the License.
1717
package v1beta2
1818

1919
import (
20+
apierrors "k8s.io/apimachinery/pkg/api/errors"
21+
"k8s.io/apimachinery/pkg/runtime"
22+
"k8s.io/apimachinery/pkg/runtime/schema"
23+
"k8s.io/apimachinery/pkg/util/validation/field"
2024
ctrl "sigs.k8s.io/controller-runtime"
25+
logf "sigs.k8s.io/controller-runtime/pkg/log"
26+
"sigs.k8s.io/controller-runtime/pkg/webhook"
27+
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
2128
)
2229

30+
const (
31+
webhookPath = "/validate-redis-redis-opstreelabs-in-v1beta2-redis"
32+
)
33+
34+
// log is for logging in this package.
35+
var redislog = logf.Log.WithName("redis-v1beta2-validation")
36+
37+
// +kubebuilder:webhook:path=/validate-redis-redis-opstreelabs-in-v1beta2-redis,mutating=false,failurePolicy=fail,sideEffects=None,groups=redis.redis.opstreelabs.in,resources=redis,verbs=create;update,versions=v1beta2,name=validate-redis.redis.opstreelabs.in,admissionReviewVersions=v1
38+
2339
func (r *Redis) SetupWebhookWithManager(mgr ctrl.Manager) error {
2440
return ctrl.NewWebhookManagedBy(mgr).
2541
For(r).
2642
Complete()
2743
}
2844

29-
// TODO(user): EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!
45+
var _ webhook.Validator = &Redis{}
46+
47+
// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
48+
func (r *Redis) ValidateCreate() (admission.Warnings, error) {
49+
redislog.Info("validate create", "name", r.Name)
50+
51+
return r.validate(nil)
52+
}
53+
54+
// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
55+
func (r *Redis) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
56+
redislog.Info("validate update", "name", r.Name)
57+
58+
return r.validate(old.(*Redis))
59+
}
60+
61+
// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
62+
func (r *Redis) ValidateDelete() (admission.Warnings, error) {
63+
redislog.Info("validate delete", "name", r.Name)
64+
65+
return nil, nil
66+
}
67+
68+
// validate validates the Redis CR
69+
func (r *Redis) validate(_ *Redis) (admission.Warnings, error) {
70+
var errors field.ErrorList
71+
72+
// Validate ACL configuration
73+
if r.Spec.ACL != nil {
74+
if err := r.Spec.ACL.Validate(); err != nil {
75+
errors = append(errors, field.Invalid(
76+
field.NewPath("spec").Child("acl"),
77+
r.Spec.ACL,
78+
err.Error(),
79+
))
80+
}
81+
}
82+
83+
if len(errors) == 0 {
84+
return nil, nil
85+
}
86+
87+
return nil, apierrors.NewInvalid(
88+
schema.GroupKind{Group: "redis.redis.opstreelabs.in", Kind: "Redis"},
89+
r.Name,
90+
errors,
91+
)
92+
}
93+
94+
func (r *Redis) WebhookPath() string {
95+
return webhookPath
96+
}
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package v1beta2_test
2+
3+
import (
4+
"encoding/json"
5+
"fmt"
6+
"testing"
7+
8+
common "github.com/OT-CONTAINER-KIT/redis-operator/api/common/v1beta2"
9+
v1beta2 "github.com/OT-CONTAINER-KIT/redis-operator/api/redis/v1beta2"
10+
"github.com/OT-CONTAINER-KIT/redis-operator/internal/testutil/webhook"
11+
"github.com/stretchr/testify/require"
12+
admissionv1beta1 "k8s.io/api/admission/v1beta1"
13+
corev1 "k8s.io/api/core/v1"
14+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
15+
"k8s.io/apimachinery/pkg/types"
16+
"k8s.io/utils/ptr"
17+
)
18+
19+
func TestRedisWebhook(t *testing.T) {
20+
cases := []webhook.ValidationWebhookTestCase{
21+
{
22+
Name: "failed-create-v1beta2-redis-acl-both-sources",
23+
Operation: admissionv1beta1.Create,
24+
Object: func(t *testing.T, uid string) []byte {
25+
t.Helper()
26+
redis := mkRedis(uid)
27+
redis.Spec.ACL = mkACLWithBothSources()
28+
return marshal(t, redis)
29+
},
30+
Check: webhook.ValidationWebhookFailed("only one of 'secret' or 'persistentVolumeClaim' can be specified"),
31+
},
32+
}
33+
34+
gvk := metav1.GroupVersionKind{
35+
Group: "redis.redis.opstreelabs.in",
36+
Version: "v1beta2",
37+
Kind: "Redis",
38+
}
39+
40+
redis := &v1beta2.Redis{}
41+
webhook.RunValidationWebhookTests(t, gvk, redis, cases...)
42+
}
43+
44+
func mkRedis(uid string) *v1beta2.Redis {
45+
return &v1beta2.Redis{
46+
ObjectMeta: metav1.ObjectMeta{
47+
Name: fmt.Sprintf("test-%s", uid),
48+
UID: types.UID(fmt.Sprintf("test-%s", uid)),
49+
},
50+
Spec: v1beta2.RedisSpec{},
51+
}
52+
}
53+
54+
func marshal(t *testing.T, obj interface{}) []byte {
55+
t.Helper()
56+
bytes, err := json.Marshal(obj)
57+
require.NoError(t, err)
58+
return bytes
59+
}
60+
61+
func mkACLWithBothSources() *common.ACLConfig {
62+
return &common.ACLConfig{
63+
Secret: &corev1.SecretVolumeSource{
64+
SecretName: "test-secret",
65+
},
66+
PersistentVolumeClaim: ptr.To("test-pvc"),
67+
}
68+
}

api/redis/v1beta2/zz_generated.deepcopy.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

api/rediscluster/v1beta2/rediscluster_webhook.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,17 @@ func (r *RedisCluster) validate(_ *RedisCluster) (admission.Warnings, error) {
8484
))
8585
}
8686

87+
// Validate ACL configuration
88+
if r.Spec.ACL != nil {
89+
if err := r.Spec.ACL.Validate(); err != nil {
90+
errors = append(errors, field.Invalid(
91+
field.NewPath("spec").Child("acl"),
92+
r.Spec.ACL,
93+
err.Error(),
94+
))
95+
}
96+
}
97+
8798
if len(errors) == 0 {
8899
return nil, nil
89100
}

0 commit comments

Comments
 (0)