Skip to content

Commit 96b28d3

Browse files
limak9182limak9182
committed
remove API reader
1 parent 0c51361 commit 96b28d3

10 files changed

Lines changed: 135 additions & 215 deletions

File tree

‎cmd/main.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,6 @@ func main() {
348348
ReadTimeout: readTimeout,
349349
WriteTimeout: writeTimeout,
350350
Client: mgr.GetClient(),
351-
Reader: mgr.GetAPIReader(),
352351
})
353352

354353
if err := mgr.Add(manager.RunnableFunc(func(ctx context.Context) error {

‎pkg/postgresql/cluster/adapter/webhook/postgres_webhook_integration_test.go‎

Lines changed: 15 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3030
"k8s.io/apimachinery/pkg/runtime"
3131
"k8s.io/apimachinery/pkg/types"
32+
"k8s.io/utils/ptr"
3233
"sigs.k8s.io/controller-runtime/pkg/client/fake"
3334

3435
enterpriseApi "github.com/splunk/splunk-operator/api/v4"
@@ -574,15 +575,6 @@ func TestPostgresClusterClassPgHBAUpdateIntegration(t *testing.T) {
574575
})
575576
}
576577

577-
func ptrBool(b bool) *bool { return &b }
578-
func ptrString(s string) *string { return &s }
579-
func ptrInt32(i int32) *int32 { return &i }
580-
581-
func ptrQuantity(s string) *resource.Quantity {
582-
q := resource.MustParse(s)
583-
return &q
584-
}
585-
586578
func newFakeReader(objects ...runtime.Object) *fake.ClientBuilder {
587579
s := runtime.NewScheme()
588580
enterpriseApi.AddToScheme(s)
@@ -599,20 +591,20 @@ func TestCrossResourceValidationIntegration(t *testing.T) {
599591
Spec: enterpriseApi.PostgresClusterClassSpec{
600592
Provisioner: "postgresql.cnpg.io",
601593
Config: &enterpriseApi.PostgresClusterClassConfig{
602-
Instances: ptrInt32(3),
603-
Storage: ptrQuantity("50Gi"),
604-
PostgresVersion: ptrString("17"),
605-
ConnectionPoolerEnabled: ptrBool(false),
594+
Instances: ptr.To(int32(3)),
595+
Storage: ptr.To(resource.MustParse("50Gi")),
596+
PostgresVersion: ptr.To("17"),
597+
ConnectionPoolerEnabled: ptr.To(false),
606598
},
607599
},
608600
}
609601

610-
reader := newFakeReader(prodClass).Build()
602+
fakeClient := newFakeReader(prodClass).Build()
611603

612604
server := validation.NewWebhookServer(validation.WebhookServerOptions{
613605
Port: 9443,
614606
Validators: validation.DefaultValidators,
615-
Reader: reader,
607+
Client: fakeClient,
616608
})
617609

618610
tests := []struct {
@@ -640,56 +632,31 @@ func TestCrossResourceValidationIntegration(t *testing.T) {
640632
wantAllowed: false,
641633
wantMessage: "PostgresClusterClass not found",
642634
},
643-
{
644-
name: "rejected - storage below class floor",
645-
obj: &enterpriseApi.PostgresCluster{
646-
TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"},
647-
ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"},
648-
Spec: enterpriseApi.PostgresClusterSpec{
649-
Class: "prod",
650-
Storage: ptrQuantity("10Gi"),
651-
},
652-
},
653-
wantAllowed: false,
654-
wantMessage: "storage cannot be lower than class default",
655-
},
656635
{
657636
name: "rejected - version below class floor",
658637
obj: &enterpriseApi.PostgresCluster{
659638
TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"},
660639
ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"},
661640
Spec: enterpriseApi.PostgresClusterSpec{
662641
Class: "prod",
663-
PostgresVersion: ptrString("16"),
642+
PostgresVersion: ptr.To("16"),
664643
},
665644
},
666645
wantAllowed: false,
667646
wantMessage: "postgresVersion cannot be lower than class default",
668647
},
669648
{
670-
name: "rejected - pooler enabled when class disables",
649+
name: "rejected - pooler enabled but class has no cnpg.connectionPooler",
671650
obj: &enterpriseApi.PostgresCluster{
672651
TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"},
673652
ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"},
674653
Spec: enterpriseApi.PostgresClusterSpec{
675654
Class: "prod",
676-
ConnectionPoolerEnabled: ptrBool(true),
655+
ConnectionPoolerEnabled: ptr.To(true),
677656
},
678657
},
679658
wantAllowed: false,
680-
wantMessage: "connectionPoolerEnabled cannot be enabled",
681-
},
682-
{
683-
name: "allowed - storage equal to class",
684-
obj: &enterpriseApi.PostgresCluster{
685-
TypeMeta: metav1.TypeMeta{APIVersion: "enterprise.splunk.com/v4", Kind: "PostgresCluster"},
686-
ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"},
687-
Spec: enterpriseApi.PostgresClusterSpec{
688-
Class: "prod",
689-
Storage: ptrQuantity("50Gi"),
690-
},
691-
},
692-
wantAllowed: true,
659+
wantMessage: "connection pooler requires cnpg.connectionPooler configuration",
693660
},
694661
{
695662
name: "allowed - higher version",
@@ -698,7 +665,7 @@ func TestCrossResourceValidationIntegration(t *testing.T) {
698665
ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "default"},
699666
Spec: enterpriseApi.PostgresClusterSpec{
700667
Class: "prod",
701-
PostgresVersion: ptrString("18"),
668+
PostgresVersion: ptr.To("18"),
702669
},
703670
},
704671
wantAllowed: true,
@@ -719,7 +686,7 @@ func TestCrossResourceValidationIntegration(t *testing.T) {
719686
}
720687
}
721688

722-
func TestCrossResourceValidationDisabledWithoutReader(t *testing.T) {
689+
func TestCrossResourceValidationDisabledWithoutClient(t *testing.T) {
723690
server := validation.NewWebhookServer(validation.WebhookServerOptions{
724691
Port: 9443,
725692
Validators: validation.DefaultValidators,
@@ -731,8 +698,8 @@ func TestCrossResourceValidationDisabledWithoutReader(t *testing.T) {
731698
Spec: enterpriseApi.PostgresClusterSpec{Class: "nonexistent"},
732699
}
733700

734-
ar := newPostgresClusterAdmissionReview(t, "uid-no-reader", admissionv1.Create, obj, nil)
701+
ar := newPostgresClusterAdmissionReview(t, "uid-no-client", admissionv1.Create, obj, nil)
735702
resp := sendAdmissionReview(t, server, ar)
736703

737-
assert.True(t, resp.Allowed, "without a reader, cross-resource validation should be skipped")
704+
assert.True(t, resp.Allowed, "without a client, cross-resource validation should be skipped")
738705
}

‎pkg/postgresql/cluster/adapter/webhook/postgrescluster_validation.go‎

Lines changed: 42 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,15 @@ package webhook
1818

1919
import (
2020
"context"
21+
"fmt"
2122
"strconv"
2223

24+
apierrors "k8s.io/apimachinery/pkg/api/errors"
2325
"k8s.io/apimachinery/pkg/util/validation/field"
2426
"sigs.k8s.io/controller-runtime/pkg/client"
2527

2628
enterpriseApi "github.com/splunk/splunk-operator/api/v4"
27-
hba "github.com/splunk/splunk-operator/pkg/postgresql/cluster/core"
29+
core "github.com/splunk/splunk-operator/pkg/postgresql/cluster/core"
2830
)
2931

3032
// ValidatePostgresClusterCreate validates a PostgresCluster on CREATE.
@@ -33,7 +35,7 @@ func ValidatePostgresClusterCreate(obj *enterpriseApi.PostgresCluster, reader cl
3335

3436
if len(obj.Spec.PgHBA) > 0 {
3537
pgHBAPath := field.NewPath("spec").Child("pgHBA")
36-
for _, re := range hba.ValidateRules(obj.Spec.PgHBA) {
38+
for _, re := range core.ValidateRules(obj.Spec.PgHBA) {
3739
allErrs = append(allErrs, field.Invalid(
3840
pgHBAPath.Index(re.Index),
3941
obj.Spec.PgHBA[re.Index],
@@ -58,81 +60,62 @@ func validateAgainstClass(obj *enterpriseApi.PostgresCluster, reader client.Read
5860

5961
class := &enterpriseApi.PostgresClusterClass{}
6062
if err := reader.Get(context.Background(), client.ObjectKey{Name: obj.Spec.Class}, class); err != nil {
61-
allErrs = append(allErrs, field.Invalid(
62-
field.NewPath("spec").Child("class"),
63-
obj.Spec.Class,
64-
"referenced PostgresClusterClass not found"))
63+
classPath := field.NewPath("spec").Child("class")
64+
if apierrors.IsNotFound(err) {
65+
allErrs = append(allErrs, field.Invalid(classPath, obj.Spec.Class,
66+
"referenced PostgresClusterClass not found"))
67+
} else {
68+
allErrs = append(allErrs, field.InternalError(classPath,
69+
fmt.Errorf("failed to look up PostgresClusterClass %q: %w", obj.Spec.Class, err)))
70+
}
6571
return allErrs
6672
}
6773

68-
classConfig := class.Spec.Config
69-
70-
mergedInstances := obj.Spec.Instances
71-
mergedVersion := obj.Spec.PostgresVersion
72-
mergedStorage := obj.Spec.Storage
73-
if classConfig != nil {
74-
if mergedInstances == nil {
75-
mergedInstances = classConfig.Instances
74+
merged, err := core.GetMergedConfig(class, obj)
75+
if err != nil {
76+
specPath := field.NewPath("spec")
77+
if merged == nil || merged.Spec.Instances == nil {
78+
allErrs = append(allErrs, field.Required(specPath.Child("instances"),
79+
"must be set in PostgresCluster or PostgresClusterClass"))
7680
}
77-
if mergedVersion == nil {
78-
mergedVersion = classConfig.PostgresVersion
81+
if merged == nil || merged.Spec.PostgresVersion == nil {
82+
allErrs = append(allErrs, field.Required(specPath.Child("postgresVersion"),
83+
"must be set in PostgresCluster or PostgresClusterClass"))
7984
}
80-
if mergedStorage == nil {
81-
mergedStorage = classConfig.Storage
85+
if merged == nil || merged.Spec.Storage == nil {
86+
allErrs = append(allErrs, field.Required(specPath.Child("storage"),
87+
"must be set in PostgresCluster or PostgresClusterClass"))
8288
}
83-
}
84-
specPath := field.NewPath("spec")
85-
if mergedInstances == nil {
86-
allErrs = append(allErrs, field.Required(specPath.Child("instances"),
87-
"must be set in PostgresCluster or PostgresClusterClass"))
88-
}
89-
if mergedVersion == nil {
90-
allErrs = append(allErrs, field.Required(specPath.Child("postgresVersion"),
91-
"must be set in PostgresCluster or PostgresClusterClass"))
92-
}
93-
if mergedStorage == nil {
94-
allErrs = append(allErrs, field.Required(specPath.Child("storage"),
95-
"must be set in PostgresCluster or PostgresClusterClass"))
96-
}
97-
98-
if classConfig == nil {
9989
return allErrs
10090
}
10191

102-
if obj.Spec.Storage != nil && classConfig.Storage != nil {
103-
if obj.Spec.Storage.Cmp(*classConfig.Storage) < 0 {
104-
allErrs = append(allErrs, field.Invalid(
105-
field.NewPath("spec").Child("storage"),
106-
obj.Spec.Storage.String(),
107-
"storage cannot be lower than class default ("+classConfig.Storage.String()+")"))
108-
}
109-
}
110-
111-
if obj.Spec.PostgresVersion != nil && classConfig.PostgresVersion != nil {
112-
clusterMajor, clusterMinor := parseVersion(*obj.Spec.PostgresVersion)
113-
classMajor, classMinor := parseVersion(*classConfig.PostgresVersion)
114-
if clusterMajor > 0 && classMajor > 0 {
115-
versionTooLow := clusterMajor < classMajor ||
116-
(clusterMajor == classMajor && classMinor >= 0 && clusterMinor >= 0 && clusterMinor < classMinor)
117-
if versionTooLow {
118-
allErrs = append(allErrs, field.Invalid(
119-
field.NewPath("spec").Child("postgresVersion"),
120-
*obj.Spec.PostgresVersion,
121-
"postgresVersion cannot be lower than class default ("+*classConfig.PostgresVersion+")"))
92+
if classConfig := class.Spec.Config; classConfig != nil {
93+
if obj.Spec.PostgresVersion != nil && classConfig.PostgresVersion != nil {
94+
clusterMajor, clusterMinor := parseVersion(*obj.Spec.PostgresVersion)
95+
classMajor, classMinor := parseVersion(*classConfig.PostgresVersion)
96+
if clusterMajor > 0 && classMajor > 0 {
97+
versionTooLow := clusterMajor < classMajor ||
98+
(clusterMajor == classMajor && classMinor >= 0 && clusterMinor >= 0 && clusterMinor < classMinor)
99+
if versionTooLow {
100+
allErrs = append(allErrs, field.Invalid(
101+
field.NewPath("spec").Child("postgresVersion"),
102+
*obj.Spec.PostgresVersion,
103+
"postgresVersion cannot be lower than class default ("+*classConfig.PostgresVersion+")"))
104+
}
122105
}
123106
}
124-
}
125107

126-
if obj.Spec.ConnectionPoolerEnabled != nil && *obj.Spec.ConnectionPoolerEnabled {
127-
classDisabled := classConfig.ConnectionPoolerEnabled == nil || !*classConfig.ConnectionPoolerEnabled
128-
if classDisabled {
108+
poolerEnabled := (obj.Spec.ConnectionPoolerEnabled != nil && *obj.Spec.ConnectionPoolerEnabled) ||
109+
(obj.Spec.ConnectionPoolerEnabled == nil && classConfig.ConnectionPoolerEnabled != nil && *classConfig.ConnectionPoolerEnabled)
110+
if poolerEnabled && (class.Spec.CNPG == nil || class.Spec.CNPG.ConnectionPooler == nil) {
129111
allErrs = append(allErrs, field.Invalid(
130112
field.NewPath("spec").Child("connectionPoolerEnabled"),
131113
true,
132-
"connectionPoolerEnabled cannot be enabled when disabled in PostgresClusterClass"))
114+
"connection pooler requires cnpg.connectionPooler configuration in PostgresClusterClass"))
133115
}
134116
}
135117

118+
_ = merged
136119
return allErrs
137120
}
138121

0 commit comments

Comments
 (0)