Skip to content

Commit fe99d41

Browse files
mikeshootzzmikeshootzz
committed
Add tests for xvshnpostgresql webhook
1 parent 1896854 commit fe99d41

File tree

6 files changed

+119
-76
lines changed

6 files changed

+119
-76
lines changed

‎pkg/comp-functions/functions/common/postgresql.go‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -163,16 +163,15 @@ func (a *PostgreSQLDependencyBuilder) CreateDependency() (string, error) {
163163
params.Instances = a.comp.GetInstances()
164164

165165
// Preserve the compositionRef of an already-existing nested PostgreSQL composite.
166-
// If we always wrote defaultPGComposition, switching that default (e.g. Stackgres → CNPG)
167-
// would silently migrate live instances on the next reconcile, which is destructive.
168-
// If the observed composite exists but has no compositionRef we return an error rather than
169-
// falling back to the default — an empty ref on an existing instance is unexpected and
170-
// silently overwriting it could trigger an unintended backend migration.
171166
compositionName := a.svc.Config.Data["defaultPGComposition"]
172167
observedPg := &vshnv1.XVSHNPostgreSQL{}
173-
if err := a.svc.GetObservedComposedResource(observedPg, a.comp.GetName()+PgInstanceNameSuffix); err == nil {
168+
err := a.svc.GetObservedComposedResource(observedPg, a.comp.GetName()+PgInstanceNameSuffix)
169+
if err != nil && err != runtime.ErrNotFound {
170+
return "", fmt.Errorf("couldn't read observed nested postgresql composite: %w", err)
171+
}
172+
if err == nil {
174173
if observedPg.Spec.CompositionRef.Name == "" {
175-
return "", fmt.Errorf("observed nested postgresql composite %q has no compositionRef, refusing to overwrite with default to prevent unintended backend migration", observedPg.GetName())
174+
return "", fmt.Errorf("observed nested postgresql composite %q has no compositionRef, refusing to overwrite with default", observedPg.GetName())
176175
}
177176
compositionName = observedPg.Spec.CompositionRef.Name
178177
}
@@ -204,7 +203,7 @@ func (a *PostgreSQLDependencyBuilder) CreateDependency() (string, error) {
204203
pg.Labels[runtime.ProviderConfigLabel] = v
205204
}
206205

207-
err := CustomCreateNetworkPolicy([]string{a.comp.GetInstanceNamespace()}, pg.GetInstanceNamespace(), pg.GetName()+"-"+a.comp.GetServiceName(), "", false, a.svc)
206+
err = CustomCreateNetworkPolicy([]string{a.comp.GetInstanceNamespace()}, pg.GetInstanceNamespace(), pg.GetName()+"-"+a.comp.GetServiceName(), "", false, a.svc)
208207
if err != nil {
209208
return "", err
210209
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ func AddConnectionSecrets(ctx context.Context, comp *vshnv1.VSHNPostgreSQL, svc
5151
port := string(cd[PostgresqlPort])
5252
db := string(cd[PostgresqlDb])
5353
if user != "" && password != "" && port != "" {
54-
svc.SetConnectionDetail(PostgresqlURL, []byte(fmt.Sprintf("postgresql://%s:%s@%s:%s/%s", user, password, host, port, db)))
54+
svc.SetConnectionDetail(PostgresqlURL, []byte(fmt.Sprintf("postgresql://%s:%s@%s:%s/%s",
55+
user, password, host, port, db)))
5556
}
5657

5758
return nil

‎pkg/controller/webhooks/postgresql.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,7 @@ import (
2424
// See https://book.kubebuilder.io/reference/markers/webhook for docs
2525
//+kubebuilder:webhook:verbs=create;update;delete,path=/validate-vshn-appcat-vshn-io-v1-vshnpostgresql,mutating=false,failurePolicy=fail,groups=vshn.appcat.vshn.io,resources=vshnpostgresqls,versions=v1,name=postgresql.vshn.appcat.vshn.io,sideEffects=None,admissionReviewVersions=v1
2626

27-
// Protect the nested XVSHNPostgreSQL composite from having its compositionRef changed once set.
28-
// This prevents accidental backend migrations (e.g. StackGres → CNPG) on live instances.
27+
// Protect the XVSHNPostgreSQL composite from having its compositionRef changed once set.
2928
//+kubebuilder:webhook:verbs=update,path=/validate-vshn-appcat-vshn-io-v1-xvshnpostgresql,mutating=false,failurePolicy=fail,groups=vshn.appcat.vshn.io,resources=xvshnpostgresqls,versions=v1,name=xvshnpostgresql.vshn.appcat.vshn.io,sideEffects=None,admissionReviewVersions=v1
3029

3130
//RBAC
@@ -91,8 +90,6 @@ func SetupPostgreSQLWebhookHandlerWithManager(mgr ctrl.Manager, withQuota bool)
9190
}
9291

9392
// XVSHNPostgreSQLWebhookHandler validates the nested XVSHNPostgreSQL composite resource.
94-
// Its sole purpose is to make compositionRef immutable once set, preventing accidental
95-
// backend migrations on live nested instances (e.g. those created by VSHNKeycloak).
9693
type XVSHNPostgreSQLWebhookHandler struct{}
9794

9895
// SetupXVSHNPostgreSQLWebhookHandlerWithManager registers the XVSHNPostgreSQL validation webhook.
@@ -122,7 +119,9 @@ func (x *XVSHNPostgreSQLWebhookHandler) ValidateUpdate(_ context.Context, oldObj
122119
}
123120

124121
allErrs := newFielErrors(newPg.Name, xpgGK)
125-
allErrs.Add(validateCompositionRefImmutability(oldPg.Spec.CompositionRef.Name, newPg.Spec.CompositionRef.Name))
122+
if err := validateCompositionRefImmutability(oldPg.Spec.CompositionRef.Name, newPg.Spec.CompositionRef.Name); err != nil {
123+
allErrs.Add(err)
124+
}
126125
return nil, allErrs.Get()
127126
}
128127

@@ -197,7 +196,9 @@ func (p *PostgreSQLWebhookHandler) validatePostgreSQL(ctx context.Context, newOb
197196

198197
// Do not allow changing compositionRef if it has been set previously.
199198
// When creating a new VSHNPostgresQL, crossplane will automatically set this field if unset.
200-
allErrs.Add(validateCompositionRefImmutability(oldPg.Spec.CompositionRef.Name, newPg.Spec.CompositionRef.Name))
199+
if err := validateCompositionRefImmutability(oldPg.Spec.CompositionRef.Name, newPg.Spec.CompositionRef.Name); err != nil {
200+
allErrs.Add(err)
201+
}
201202

202203
// Check for disk downsizing
203204
if diskErr := p.DefaultWebhookHandler.ValidateDiskDownsizing(ctx, oldPg, newPg, p.gk.Kind); diskErr != nil {

‎pkg/controller/webhooks/postgresql_test.go‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -732,6 +732,97 @@ func TestPostgreSQLWebhookHandler_ValidateCreateWithPinImageTag(t *testing.T) {
732732
assert.NoError(t, err)
733733
}
734734

735+
func TestValidateCompositionRefImmutability(t *testing.T) {
736+
tests := []struct {
737+
name string
738+
oldRef string
739+
newRef string
740+
expectErr bool
741+
}{
742+
{
743+
name: "GivenBothEmpty_ThenNoError",
744+
oldRef: "",
745+
newRef: "",
746+
expectErr: false,
747+
},
748+
{
749+
name: "GivenOldEmptyNewSet_ThenNoError",
750+
oldRef: "",
751+
newRef: "vshnpostgrescnpg.vshn.appcat.vshn.io",
752+
expectErr: false,
753+
},
754+
{
755+
name: "GivenSameRef_ThenNoError",
756+
oldRef: "vshnpostgres.vshn.appcat.vshn.io",
757+
newRef: "vshnpostgres.vshn.appcat.vshn.io",
758+
expectErr: false,
759+
},
760+
{
761+
name: "GivenOldSetNewDifferent_ThenError",
762+
oldRef: "vshnpostgres.vshn.appcat.vshn.io",
763+
newRef: "vshnpostgrescnpg.vshn.appcat.vshn.io",
764+
expectErr: true,
765+
},
766+
{
767+
name: "GivenOldSetNewEmpty_ThenError",
768+
oldRef: "vshnpostgres.vshn.appcat.vshn.io",
769+
newRef: "",
770+
expectErr: true,
771+
},
772+
}
773+
for _, tt := range tests {
774+
t.Run(tt.name, func(t *testing.T) {
775+
err := validateCompositionRefImmutability(tt.oldRef, tt.newRef)
776+
if tt.expectErr {
777+
assert.NotNil(t, err)
778+
assert.Contains(t, err.Error(), "compositionRef is immutable")
779+
} else {
780+
assert.Nil(t, err)
781+
}
782+
})
783+
}
784+
}
785+
786+
func TestXVSHNPostgreSQLWebhookHandler_ValidateUpdate(t *testing.T) {
787+
ctx := context.TODO()
788+
handler := XVSHNPostgreSQLWebhookHandler{}
789+
790+
base := &vshnv1.XVSHNPostgreSQL{
791+
ObjectMeta: metav1.ObjectMeta{
792+
Name: "keycloak-pg",
793+
Namespace: "vshn-keycloak-myinstance",
794+
},
795+
}
796+
797+
// Allow: compositionRef not yet set on either side
798+
_, err := handler.ValidateUpdate(ctx, base, base.DeepCopy())
799+
assert.NoError(t, err)
800+
801+
// Allow: first assignment (old empty, new set)
802+
withRef := base.DeepCopy()
803+
withRef.Spec.CompositionRef.Name = "vshnpostgrescnpg.vshn.appcat.vshn.io"
804+
_, err = handler.ValidateUpdate(ctx, base, withRef)
805+
assert.NoError(t, err)
806+
807+
// Allow: compositionRef unchanged
808+
_, err = handler.ValidateUpdate(ctx, withRef, withRef.DeepCopy())
809+
assert.NoError(t, err)
810+
811+
// Reject: compositionRef changed on existing instance
812+
changedRef := base.DeepCopy()
813+
changedRef.Spec.CompositionRef.Name = "vshnpostgres.vshn.appcat.vshn.io"
814+
_, err = handler.ValidateUpdate(ctx, withRef, changedRef)
815+
assert.Error(t, err)
816+
assert.Contains(t, err.Error(), "compositionRef is immutable")
817+
818+
// Allow: object is being deleted even if compositionRef would change
819+
deletingObj := changedRef.DeepCopy()
820+
now := metav1.Now()
821+
deletingObj.DeletionTimestamp = &now
822+
_, err = handler.ValidateUpdate(ctx, withRef, deletingObj)
823+
assert.NoError(t, err)
824+
}
825+
735826
// disabling this temporarily
736827
// func TestPostgreSQLWebhookHandler_ValidateMajorVersionUpgrade(t *testing.T) {
737828
// tests := []struct {

‎test/functions/vshnkeycloak/01_default.yaml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,14 @@ input:
1818
observed:
1919
resources:
2020
mycloak-pg:
21+
resource:
22+
apiVersion: vshn.appcat.vshn.io/v1
23+
kind: XVSHNPostgreSQL
24+
metadata:
25+
name: mycloak-pg
26+
spec:
27+
compositionRef:
28+
name: vshnpostgrescnpg.vshn.appcat.vshn.io
2129
connection_details:
2230
foo: YmFyCg==
2331
# necessary for keycloak custom env variables

‎test/functions/vshnnextcloud/01_default.yaml‎

Lines changed: 4 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -103,70 +103,13 @@ observed:
103103
admin: czNjcjN0UGFzcwo=
104104
nextcloud-gc9x4-pg:
105105
resource:
106-
apiVersion: v1
107-
kind: Object
106+
apiVersion: vshn.appcat.vshn.io/v1
107+
kind: XVSHNPostgreSQL
108108
metadata:
109109
name: "nextcloud-gc9x4-pg"
110110
spec:
111-
forProvider:
112-
manifest:
113-
apiVersion: vshn.appcat.vshn.io/v1
114-
kind: VSHNNextcloud
115-
metadata:
116-
name: "nextcloud-gc9x4-pg"
117-
spec:
118-
parameters:
119-
service:
120-
fqdn:
121-
- nextcloud.example.com
122-
repackEnabled: false
123-
vacuumEnabled: false
124-
backup:
125-
retention: 6
126-
deletionProtection: true
127-
deletionRetention: 7
128-
encryption:
129-
enabled: false
130-
instances: 1
131-
security:
132-
allowAllNamespaces: false
133-
status:
134-
atProvider:
135-
manifest:
136-
apiVersion: vshn.appcat.vshn.io/v1
137-
kind: VSHNNextcloud
138-
metadata:
139-
annotations: null
140-
creationTimestamp: "2024-07-08T16:30:28Z"
141-
generation: 8
142-
name: "nextcloud-gc9x4-pg"
143-
namespace: vshn-postgresql-nextcloud-gc9x4
144-
resourceVersion: "583272583"
145-
uid: 44ead047-98de-4e73-9cc0-d99454090a36
146-
spec:
147-
parameters:
148-
service:
149-
repackEnabled: false
150-
vacuumEnabled: false
151-
backup:
152-
retention: 6
153-
deletionProtection: true
154-
deletionRetention: 7
155-
encryption:
156-
enabled: false
157-
instances: 1
158-
security:
159-
allowAllNamespaces: false
160-
status:
161-
conditions:
162-
- lastTransitionTime: "2024-07-08T14:50:19Z"
163-
reason: Available
164-
status: "True"
165-
type: Ready
166-
- lastTransitionTime: "2024-07-08T14:50:18Z"
167-
reason: ReconcileSuccess
168-
status: "True"
169-
type: Synced
111+
compositionRef:
112+
name: vshnpostgres.vshn.appcat.vshn.io
170113
connection_details:
171114
POSTGRESQL_HOST: bmV4dGNsb3VkLWdjOXg0LnZzaG4tcG9zdGdyZXNxbC1uZXh0Y2xvdWQtZ2N4NC5zdmMuY2x1c3Rlci5sb2NhbAo=
172115
POSTGRESQL_DB: cG9zdGdyZXMK

0 commit comments

Comments
 (0)