Skip to content

Commit 0f9df28

Browse files
author
Gabriel Saratura
committed
Further consolidate cnpg code
1 parent 0950c12 commit 0f9df28

File tree

6 files changed

+289
-5
lines changed

6 files changed

+289
-5
lines changed

‎pkg/comp-functions/functions/vshnmariadb/backup.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ func AddBackupMariadb(ctx context.Context, comp *vshnv1.VSHNMariaDB, svc *runtim
2626
if err != nil {
2727
return runtime.NewFatalResult(fmt.Errorf("failed to parse composite: %w", err))
2828
}
29-
29+
3030
// Setting the version may be lost in other functions, reinforce it here
31-
// TODO Fix status field being overwritten every time SetDesiredCompositeStatus() function is called
31+
// TODO Fix status field being overwritten every time SetDesiredCompositeStatus() function is called
3232
if comp.Spec.Parameters.Maintenance.PinImageTag != "" {
3333
comp.Status.MariaDBVersion = comp.Spec.Parameters.Maintenance.PinImageTag
3434
}

‎pkg/comp-functions/functions/vshnmariadb/proxysql.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,9 @@ func AddProxySQL(_ context.Context, comp *vshnv1.VSHNMariaDB, svc *runtime.Servi
9797
if err != nil {
9898
return runtime.NewWarningResult(fmt.Sprintf("cannot create PDB for ProxySQL: %s", err))
9999
}
100-
100+
101101
// Setting the version may be lost in other functions, reinforce it here
102-
// TODO Fix status field being overwritten every time SetDesiredCompositeStatus() function is called
102+
// TODO Fix status field being overwritten every time SetDesiredCompositeStatus() function is called
103103
if comp.Spec.Parameters.Maintenance.PinImageTag != "" {
104104
comp.Status.MariaDBVersion = comp.Spec.Parameters.Maintenance.PinImageTag
105105
}

‎pkg/comp-functions/functions/vshnpostgrescnpg/deploy_test.go‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,118 @@ func Test_sizing(t *testing.T) {
129129
assert.Equal(t, ourDiskSize, values["cluster"].(map[string]any)["storage"].(map[string]any)["size"])
130130
}
131131

132+
func Test_pinImageTag(t *testing.T) {
133+
ctx := context.TODO()
134+
135+
t.Run("no pinImageTag uses default version", func(t *testing.T) {
136+
svc, comp := getSvcCompCnpg(t)
137+
comp.Spec.Parameters.Service.MajorVersion = "15"
138+
comp.Spec.Parameters.Maintenance.PinImageTag = ""
139+
140+
values, err := createCnpgHelmValues(ctx, svc, comp)
141+
assert.NoError(t, err)
142+
assert.NotNil(t, values)
143+
144+
// Check ImageCatalog has default version for major 15
145+
imageCatalog := values["imageCatalog"].(map[string]any)
146+
images := imageCatalog["images"].([]map[string]string)
147+
148+
var found15 bool
149+
for _, img := range images {
150+
if img["major"] == "15" {
151+
// Should use default version "15.9"
152+
assert.Equal(t, "ghcr.io/cloudnative-pg/postgresql:15.9", img["image"])
153+
found15 = true
154+
}
155+
}
156+
assert.True(t, found15, "should have image entry for major version 15")
157+
158+
// Check status is set to default version
159+
assert.Equal(t, "15.9", comp.Status.CurrentVersion)
160+
})
161+
162+
t.Run("pinImageTag overrides default version in ImageCatalog", func(t *testing.T) {
163+
svc, comp := getSvcCompCnpg(t)
164+
comp.Spec.Parameters.Service.MajorVersion = "15"
165+
comp.Spec.Parameters.Maintenance.PinImageTag = "15.13"
166+
167+
values, err := createCnpgHelmValues(ctx, svc, comp)
168+
assert.NoError(t, err)
169+
assert.NotNil(t, values)
170+
171+
// Check ImageCatalog has pinned version for major 15
172+
imageCatalog := values["imageCatalog"].(map[string]any)
173+
images := imageCatalog["images"].([]map[string]string)
174+
175+
var found15 bool
176+
for _, img := range images {
177+
if img["major"] == "15" {
178+
// Should use pinned version "15.13" instead of default "15.9"
179+
assert.Equal(t, "ghcr.io/cloudnative-pg/postgresql:15.13", img["image"])
180+
found15 = true
181+
}
182+
}
183+
assert.True(t, found15, "should have image entry for major version 15")
184+
185+
// Check status is set to pinned version
186+
assert.Equal(t, "15.13", comp.Status.CurrentVersion)
187+
})
188+
189+
t.Run("pinImageTag only affects specified major version", func(t *testing.T) {
190+
svc, comp := getSvcCompCnpg(t)
191+
comp.Spec.Parameters.Service.MajorVersion = "16"
192+
comp.Spec.Parameters.Maintenance.PinImageTag = "16.4"
193+
194+
values, err := createCnpgHelmValues(ctx, svc, comp)
195+
assert.NoError(t, err)
196+
assert.NotNil(t, values)
197+
198+
// Check ImageCatalog
199+
imageCatalog := values["imageCatalog"].(map[string]any)
200+
images := imageCatalog["images"].([]map[string]string)
201+
202+
for _, img := range images {
203+
switch img["major"] {
204+
case "16":
205+
// Major 16 should use pinned version
206+
assert.Equal(t, "ghcr.io/cloudnative-pg/postgresql:16.4", img["image"])
207+
case "15":
208+
// Major 15 should still use default version
209+
assert.Equal(t, "ghcr.io/cloudnative-pg/postgresql:15.9", img["image"])
210+
case "17":
211+
// Major 17 should still use default version
212+
assert.Equal(t, "ghcr.io/cloudnative-pg/postgresql:17.5", img["image"])
213+
}
214+
}
215+
216+
// Check status is set to pinned version
217+
assert.Equal(t, "16.4", comp.Status.CurrentVersion)
218+
})
219+
220+
t.Run("majorVersion is correctly set in helm values", func(t *testing.T) {
221+
svc, comp := getSvcCompCnpg(t)
222+
comp.Spec.Parameters.Service.MajorVersion = "17"
223+
comp.Spec.Parameters.Maintenance.PinImageTag = "17.2"
224+
225+
values, err := createCnpgHelmValues(ctx, svc, comp)
226+
assert.NoError(t, err)
227+
assert.NotNil(t, values)
228+
229+
// Check version in helm values still uses major version
230+
version := values["version"].(map[string]string)
231+
assert.Equal(t, "17", version["postgresql"])
232+
233+
// Check imageCatalogRef is set correctly
234+
cluster := values["cluster"].(map[string]any)
235+
imageCatalogRef := cluster["imageCatalogRef"].(map[string]string)
236+
assert.Equal(t, "ImageCatalog", imageCatalogRef["kind"])
237+
assert.Equal(t, "postgresql", imageCatalogRef["name"])
238+
239+
// Check status is set to pinned version
240+
assert.Equal(t, "17.2", comp.Status.CurrentVersion)
241+
})
242+
}
243+
132244
// Obtain svc and comp for CNPG tests
133245
func getSvcCompCnpg(testing *testing.T) (*runtime.ServiceRuntime, *vshnv1.VSHNPostgreSQL) {
134246
svc, comp := getPostgreSqlComp(testing, testingPath)

‎pkg/comp-functions/functions/vshnredis/maintenance.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ func AddMaintenanceJob(ctx context.Context, comp *vshnv1.VSHNRedis, svc *runtime
1818
}
1919

2020
// Setting the version may be lost in other functions, reinforce it here
21-
// TODO Fix status field being overwritten every time SetDesiredCompositeStatus() function is called
21+
// TODO Fix status field being overwritten every time SetDesiredCompositeStatus() function is called
2222
if comp.Spec.Parameters.Maintenance.PinImageTag != "" {
2323
comp.Status.CurrentReleaseTag = comp.Spec.Parameters.Maintenance.PinImageTag
2424
}

‎pkg/controller/webhooks/postgresql.go‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,14 @@ func (p *PostgreSQLWebhookHandler) validatePostgreSQL(ctx context.Context, newOb
132132
// Validate PostgreSQL configuration
133133
allErrs.Add(validatePgConf(newPg)...)
134134

135+
// Validate pinImageTag matches majorVersion
136+
if err := validatePinImageTag(
137+
newPg.Spec.Parameters.Maintenance.PinImageTag,
138+
newPg.Spec.Parameters.Service.MajorVersion,
139+
); err != nil {
140+
allErrs.Add(err)
141+
}
142+
135143
if !isCreate {
136144
oldPg, ok := oldObj.(*vshnv1.VSHNPostgreSQL)
137145
if !ok {
@@ -380,3 +388,26 @@ func validatePostgreSQLEncryptionChanges(newEncryption, oldEncryption *vshnv1.VS
380388
}
381389
return nil
382390
}
391+
392+
// validatePinImageTag validates that pinImageTag's major version matches the specified majorVersion
393+
func validatePinImageTag(pinImageTag, majorVersion string) *field.Error {
394+
if pinImageTag == "" {
395+
return nil
396+
}
397+
398+
// Extract major version from pinImageTag (e.g., "15.13" -> "15", "16.4" -> "16")
399+
pinMajor := pinImageTag
400+
if idx := strings.Index(pinImageTag, "."); idx > 0 {
401+
pinMajor = pinImageTag[:idx]
402+
}
403+
404+
if pinMajor != majorVersion {
405+
return field.Invalid(
406+
field.NewPath("spec", "parameters", "maintenance", "pinImageTag"),
407+
pinImageTag,
408+
fmt.Sprintf("pinImageTag major version %q must match majorVersion %q", pinMajor, majorVersion),
409+
)
410+
}
411+
412+
return nil
413+
}

‎pkg/controller/webhooks/postgresql_test.go‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,147 @@ func TestPostgreSQLWebhookHandler_ValidateEncryptionChanges(t *testing.T) {
591591
assert.NoError(t, err)
592592
}
593593

594+
func TestValidatePinImageTag(t *testing.T) {
595+
tests := []struct {
596+
name string
597+
pinImageTag string
598+
majorVersion string
599+
expectErr bool
600+
errContains string
601+
}{
602+
{
603+
name: "GivenEmptyPinImageTag_ThenNoError",
604+
pinImageTag: "",
605+
majorVersion: "15",
606+
expectErr: false,
607+
},
608+
{
609+
name: "GivenMatchingMajorVersion_ThenNoError",
610+
pinImageTag: "15.13",
611+
majorVersion: "15",
612+
expectErr: false,
613+
},
614+
{
615+
name: "GivenMatchingMajorVersionDifferentMinor_ThenNoError",
616+
pinImageTag: "15.9",
617+
majorVersion: "15",
618+
expectErr: false,
619+
},
620+
{
621+
name: "GivenMajorVersion16Match_ThenNoError",
622+
pinImageTag: "16.4",
623+
majorVersion: "16",
624+
expectErr: false,
625+
},
626+
{
627+
name: "GivenMajorVersion17Match_ThenNoError",
628+
pinImageTag: "17.2",
629+
majorVersion: "17",
630+
expectErr: false,
631+
},
632+
{
633+
name: "GivenMismatchedMajorVersion_ThenError",
634+
pinImageTag: "16.4",
635+
majorVersion: "15",
636+
expectErr: true,
637+
errContains: "pinImageTag major version \"16\" must match majorVersion \"15\"",
638+
},
639+
{
640+
name: "GivenPinImageTagWithDifferentMajor_ThenError",
641+
pinImageTag: "15.13",
642+
majorVersion: "16",
643+
expectErr: true,
644+
errContains: "pinImageTag major version \"15\" must match majorVersion \"16\"",
645+
},
646+
{
647+
name: "GivenPinImageTagWithMajorOnly_ThenNoError",
648+
pinImageTag: "15",
649+
majorVersion: "15",
650+
expectErr: false,
651+
},
652+
{
653+
name: "GivenPinImageTagWithMajorOnlyMismatch_ThenError",
654+
pinImageTag: "16",
655+
majorVersion: "15",
656+
expectErr: true,
657+
errContains: "pinImageTag major version \"16\" must match majorVersion \"15\"",
658+
},
659+
}
660+
661+
for _, tt := range tests {
662+
t.Run(tt.name, func(t *testing.T) {
663+
err := validatePinImageTag(tt.pinImageTag, tt.majorVersion)
664+
if tt.expectErr {
665+
assert.NotNil(t, err)
666+
assert.Contains(t, err.Error(), tt.errContains)
667+
} else {
668+
assert.Nil(t, err)
669+
}
670+
})
671+
}
672+
}
673+
674+
func TestPostgreSQLWebhookHandler_ValidateCreateWithPinImageTag(t *testing.T) {
675+
ctx := context.TODO()
676+
claimNS := &corev1.Namespace{
677+
ObjectMeta: metav1.ObjectMeta{
678+
Name: "claimns",
679+
Labels: map[string]string{
680+
utils.OrgLabelName: "myorg",
681+
},
682+
},
683+
}
684+
fclient := fake.NewClientBuilder().
685+
WithScheme(pkg.SetupScheme()).
686+
WithObjects(claimNS).
687+
Build()
688+
689+
handler := PostgreSQLWebhookHandler{
690+
DefaultWebhookHandler: DefaultWebhookHandler{
691+
client: fclient,
692+
log: logr.Discard(),
693+
withQuota: false,
694+
obj: &vshnv1.VSHNPostgreSQL{},
695+
name: "postgresql",
696+
nameLength: 30,
697+
},
698+
}
699+
700+
pgBase := &vshnv1.VSHNPostgreSQL{
701+
ObjectMeta: metav1.ObjectMeta{
702+
Name: "myinstance",
703+
Namespace: "claimns",
704+
},
705+
Spec: vshnv1.VSHNPostgreSQLSpec{
706+
Parameters: vshnv1.VSHNPostgreSQLParameters{
707+
Service: vshnv1.VSHNPostgreSQLServiceSpec{
708+
MajorVersion: "15",
709+
RepackEnabled: true,
710+
},
711+
},
712+
},
713+
}
714+
715+
// Test: Valid pinImageTag matching majorVersion
716+
pgValid := pgBase.DeepCopy()
717+
pgValid.Spec.Parameters.Maintenance.PinImageTag = "15.13"
718+
_, err := handler.ValidateCreate(ctx, pgValid)
719+
assert.NoError(t, err)
720+
721+
// Test: Invalid pinImageTag not matching majorVersion
722+
pgInvalid := pgBase.DeepCopy()
723+
pgInvalid.Spec.Parameters.Maintenance.PinImageTag = "16.4"
724+
_, err = handler.ValidateCreate(ctx, pgInvalid)
725+
assert.Error(t, err)
726+
assert.Contains(t, err.Error(), "pinImageTag major version")
727+
728+
// Test: No pinImageTag set (should be valid)
729+
pgNoPin := pgBase.DeepCopy()
730+
pgNoPin.Spec.Parameters.Maintenance.PinImageTag = ""
731+
_, err = handler.ValidateCreate(ctx, pgNoPin)
732+
assert.NoError(t, err)
733+
}
734+
594735
// disabling this temporarily
595736
// func TestPostgreSQLWebhookHandler_ValidateMajorVersionUpgrade(t *testing.T) {
596737
// tests := []struct {

0 commit comments

Comments
 (0)