From 74df169b8d74a188f3dcc81c6e76a9a2173c3397 Mon Sep 17 00:00:00 2001 From: Marco Nenciarini Date: Wed, 30 Oct 2024 18:28:23 +0100 Subject: [PATCH] test: review Signed-off-by: Marco Nenciarini --- api/v1/publication_types.go | 3 +- .../postgresql.cnpg.io_publications.yaml | 3 ++ .../publication_controller_sql_test.go | 18 +++++----- .../subscription_controller_sql_test.go | 34 +++++++++---------- .../subscription_controller_test.go | 5 +-- 5 files changed, 33 insertions(+), 30 deletions(-) diff --git a/api/v1/publication_types.go b/api/v1/publication_types.go index 2b4b168924..4ac56a5be0 100644 --- a/api/v1/publication_types.go +++ b/api/v1/publication_types.go @@ -70,13 +70,14 @@ type PublicationSpec struct { // +kubebuilder:validation:XValidation:rule="(has(self.allTables) && !has(self.objects)) || (!has(self.allTables) && has(self.objects))",message="allTables and objects are mutually exclusive" type PublicationTarget struct { // All tables should be published + // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="allTables is immutable" // +optional AllTables bool `json:"allTables,omitempty"` // Just the following schema objects - // +optional // +kubebuilder:validation:XValidation:rule="!(self.exists(o, has(o.table) && has(o.table.columns)) && self.exists(o, has(o.tablesInSchema)))",message="specifying a column list when the publication also publishes tablesInSchema is not supported" // +kubebuilder:validation:MaxItems=100000 + // +optional Objects []PublicationTargetObject `json:"objects,omitempty"` } diff --git a/config/crd/bases/postgresql.cnpg.io_publications.yaml b/config/crd/bases/postgresql.cnpg.io_publications.yaml index 50d89e6daf..9b8ed22fd9 100644 --- a/config/crd/bases/postgresql.cnpg.io_publications.yaml +++ b/config/crd/bases/postgresql.cnpg.io_publications.yaml @@ -103,6 +103,9 @@ spec: allTables: description: All tables should be published type: boolean + x-kubernetes-validations: + - message: allTables is immutable + rule: self == oldSelf objects: description: Just the following schema objects items: diff --git a/internal/management/controller/publication_controller_sql_test.go b/internal/management/controller/publication_controller_sql_test.go index e00b9df8d3..1ede219832 100644 --- a/internal/management/controller/publication_controller_sql_test.go +++ b/internal/management/controller/publication_controller_sql_test.go @@ -86,7 +86,7 @@ var _ = Describe("publication sql", func() { } sqls := toPublicationAlterSQL(obj) - Expect(sqls).To(ContainElement("ALTER PUBLICATION \"test_pub\" SET TABLES IN SCHEMA \"public\"")) + Expect(sqls).To(ContainElement(`ALTER PUBLICATION "test_pub" SET TABLES IN SCHEMA "public"`)) }) It("generates correct SQL for altering publication with owner", func() { @@ -98,7 +98,7 @@ var _ = Describe("publication sql", func() { } sqls := toPublicationAlterSQL(obj) - Expect(sqls).To(ContainElement("ALTER PUBLICATION \"test_pub\" OWNER TO \"new_owner\"")) + Expect(sqls).To(ContainElement(`ALTER PUBLICATION "test_pub" OWNER TO "new_owner"`)) }) It("generates correct SQL for altering publication with parameters", func() { @@ -113,7 +113,7 @@ var _ = Describe("publication sql", func() { } sqls := toPublicationAlterSQL(obj) - Expect(sqls).To(ContainElement("ALTER PUBLICATION \"test_pub\" SET (param1 = 'value1', param2 = 'value2')")) + Expect(sqls).To(ContainElement(`ALTER PUBLICATION "test_pub" SET (param1 = 'value1', param2 = 'value2')`)) }) It("returns empty SQL list when no alterations are needed", func() { @@ -140,7 +140,7 @@ var _ = Describe("publication sql", func() { } sqls := toPublicationCreateSQL(obj) - Expect(sqls).To(ContainElement("CREATE PUBLICATION \"test_pub\" FOR TABLES IN SCHEMA \"public\"")) + Expect(sqls).To(ContainElement(`CREATE PUBLICATION "test_pub" FOR TABLES IN SCHEMA "public"`)) }) It("generates correct SQL for creating publication with target table", func() { @@ -162,13 +162,15 @@ var _ = Describe("publication sql", func() { It("generates correct SQL for creating publication with owner", func() { obj := &apiv1.Publication{ Spec: apiv1.PublicationSpec{ - Name: "test_pub", - Owner: "new_owner", + Name: "test_pub", + Owner: "new_owner", + Target: apiv1.PublicationTarget{AllTables: true}, }, } sqls := toPublicationCreateSQL(obj) - Expect(sqls).To(ContainElement("ALTER PUBLICATION \"test_pub\" OWNER to \"new_owner\"")) + Expect(sqls).To(ContainElement(`CREATE PUBLICATION "test_pub" FOR ALL TABLES`)) + Expect(sqls).To(ContainElement(`ALTER PUBLICATION "test_pub" OWNER to "new_owner"`)) }) It("generates correct SQL for creating publication with parameters", func() { @@ -189,7 +191,7 @@ var _ = Describe("publication sql", func() { sqls := toPublicationCreateSQL(obj) Expect(sqls).To(ContainElement( - "CREATE PUBLICATION \"test_pub\" FOR TABLES IN SCHEMA \"public\" WITH (param1 = 'value1', param2 = 'value2')", + `CREATE PUBLICATION "test_pub" FOR TABLES IN SCHEMA "public" WITH (param1 = 'value1', param2 = 'value2')`, )) }) }) diff --git a/internal/management/controller/subscription_controller_sql_test.go b/internal/management/controller/subscription_controller_sql_test.go index 5c40e114b3..0c1f663263 100644 --- a/internal/management/controller/subscription_controller_sql_test.go +++ b/internal/management/controller/subscription_controller_sql_test.go @@ -83,7 +83,7 @@ var _ = Describe("subscription sql", func() { sqls := toSubscriptionCreateSQL(obj, connString) Expect(sqls).To(ContainElement( - "CREATE SUBSCRIPTION \"test_sub\" CONNECTION 'host=localhost user=test dbname=test' PUBLICATION \"test_pub\"")) + `CREATE SUBSCRIPTION "test_sub" CONNECTION 'host=localhost user=test dbname=test' PUBLICATION "test_pub"`)) }) It("generates correct SQL for creating subscription with parameters", func() { @@ -100,9 +100,9 @@ var _ = Describe("subscription sql", func() { connString := "host=localhost user=test dbname=test" sqls := toSubscriptionCreateSQL(obj, connString) - expectedElement := "CREATE SUBSCRIPTION \"test_sub\" " + - "CONNECTION 'host=localhost user=test dbname=test' " + - "PUBLICATION \"test_pub\" WITH (param1 = 'value1', param2 = 'value2')" + expectedElement := `CREATE SUBSCRIPTION "test_sub" ` + + `CONNECTION 'host=localhost user=test dbname=test' ` + + `PUBLICATION "test_pub" WITH (param1 = 'value1', param2 = 'value2')` Expect(sqls).To(ContainElement(expectedElement)) }) @@ -118,8 +118,8 @@ var _ = Describe("subscription sql", func() { sqls := toSubscriptionCreateSQL(obj, connString) Expect(sqls).To(ContainElement( - "CREATE SUBSCRIPTION \"test_sub\" CONNECTION 'host=localhost user=test dbname=test' PUBLICATION \"test_pub\"")) - Expect(sqls).To(ContainElement("ALTER SUBSCRIPTION \"test_sub\" OWNER TO \"new_owner\"")) + `CREATE SUBSCRIPTION "test_sub" CONNECTION 'host=localhost user=test dbname=test' PUBLICATION "test_pub"`)) + Expect(sqls).To(ContainElement(`ALTER SUBSCRIPTION "test_sub" OWNER TO "new_owner"`)) }) It("returns correct SQL for creating subscription with no owner or parameters", func() { @@ -133,7 +133,7 @@ var _ = Describe("subscription sql", func() { sqls := toSubscriptionCreateSQL(obj, connString) Expect(sqls).To(ContainElement( - "CREATE SUBSCRIPTION \"test_sub\" CONNECTION 'host=localhost user=test dbname=test' PUBLICATION \"test_pub\"")) + `CREATE SUBSCRIPTION "test_sub" CONNECTION 'host=localhost user=test dbname=test' PUBLICATION "test_pub"`)) }) It("generates correct SQL for altering subscription with publication and connection string", func() { @@ -146,8 +146,8 @@ var _ = Describe("subscription sql", func() { connString := "host=localhost user=test dbname=test" sqls := toSubscriptionAlterSQL(obj, connString) - Expect(sqls).To(ContainElement("ALTER SUBSCRIPTION \"test_sub\" SET PUBLICATION \"test_pub\"")) - Expect(sqls).To(ContainElement("ALTER SUBSCRIPTION \"test_sub\" CONNECTION 'host=localhost user=test dbname=test'")) + Expect(sqls).To(ContainElement(`ALTER SUBSCRIPTION "test_sub" SET PUBLICATION "test_pub"`)) + Expect(sqls).To(ContainElement(`ALTER SUBSCRIPTION "test_sub" CONNECTION 'host=localhost user=test dbname=test'`)) }) It("generates correct SQL for altering subscription with owner", func() { @@ -161,9 +161,9 @@ var _ = Describe("subscription sql", func() { connString := "host=localhost user=test dbname=test" sqls := toSubscriptionAlterSQL(obj, connString) - Expect(sqls).To(ContainElement("ALTER SUBSCRIPTION \"test_sub\" SET PUBLICATION \"test_pub\"")) - Expect(sqls).To(ContainElement("ALTER SUBSCRIPTION \"test_sub\" CONNECTION 'host=localhost user=test dbname=test'")) - Expect(sqls).To(ContainElement("ALTER SUBSCRIPTION \"test_sub\" OWNER TO \"new_owner\"")) + Expect(sqls).To(ContainElement(`ALTER SUBSCRIPTION "test_sub" SET PUBLICATION "test_pub"`)) + Expect(sqls).To(ContainElement(`ALTER SUBSCRIPTION "test_sub" CONNECTION 'host=localhost user=test dbname=test'`)) + Expect(sqls).To(ContainElement(`ALTER SUBSCRIPTION "test_sub" OWNER TO "new_owner"`)) }) It("generates correct SQL for altering subscription with parameters", func() { @@ -180,9 +180,9 @@ var _ = Describe("subscription sql", func() { connString := "host=localhost user=test dbname=test" sqls := toSubscriptionAlterSQL(obj, connString) - Expect(sqls).To(ContainElement("ALTER SUBSCRIPTION \"test_sub\" SET PUBLICATION \"test_pub\"")) - Expect(sqls).To(ContainElement("ALTER SUBSCRIPTION \"test_sub\" CONNECTION 'host=localhost user=test dbname=test'")) - Expect(sqls).To(ContainElement("ALTER SUBSCRIPTION \"test_sub\" SET (param1 = 'value1', param2 = 'value2')")) + Expect(sqls).To(ContainElement(`ALTER SUBSCRIPTION "test_sub" SET PUBLICATION "test_pub"`)) + Expect(sqls).To(ContainElement(`ALTER SUBSCRIPTION "test_sub" CONNECTION 'host=localhost user=test dbname=test'`)) + Expect(sqls).To(ContainElement(`ALTER SUBSCRIPTION "test_sub" SET (param1 = 'value1', param2 = 'value2')`)) }) It("returns correct SQL for altering subscription with no owner or parameters", func() { @@ -195,7 +195,7 @@ var _ = Describe("subscription sql", func() { connString := "host=localhost user=test dbname=test" sqls := toSubscriptionAlterSQL(obj, connString) - Expect(sqls).To(ContainElement("ALTER SUBSCRIPTION \"test_sub\" SET PUBLICATION \"test_pub\"")) - Expect(sqls).To(ContainElement("ALTER SUBSCRIPTION \"test_sub\" CONNECTION 'host=localhost user=test dbname=test'")) + Expect(sqls).To(ContainElement(`ALTER SUBSCRIPTION "test_sub" SET PUBLICATION "test_pub"`)) + Expect(sqls).To(ContainElement(`ALTER SUBSCRIPTION "test_sub" CONNECTION 'host=localhost user=test dbname=test'`)) }) }) diff --git a/internal/management/controller/subscription_controller_test.go b/internal/management/controller/subscription_controller_test.go index 230dc496ea..c2d8b147bf 100644 --- a/internal/management/controller/subscription_controller_test.go +++ b/internal/management/controller/subscription_controller_test.go @@ -27,9 +27,6 @@ var _ = Describe("Conversion of PG parameters from map to string of key/value pa "a": "1", "b": "2", } res := toPostgresParameters(m) - Expect(res).To(BeElementOf([]string{ - `a = '1', b = '2'`, - `b = '2', a = '1'`, - })) + Expect(res).To(Equal(`a = '1', b = '2'`)) }) })