From 4543218101b1c29c754dce8046ffacff6d4993e7 Mon Sep 17 00:00:00 2001 From: Ben Blattberg Date: Thu, 7 Oct 2021 16:04:12 -0500 Subject: [PATCH 1/3] Enable postgis extensions automatically on postgis images Issue: [sc-12487] --- .github/workflows/test.yaml | 1 + .../controller/postgrescluster/postgres.go | 17 +++++- internal/postgis/postgis.go | 53 ++++++++++++++++++ internal/postgis/postgis_test.go | 54 +++++++++++++++++++ 4 files changed, 123 insertions(+), 2 deletions(-) create mode 100644 internal/postgis/postgis.go create mode 100644 internal/postgis/postgis_test.go diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 1ac5a3aaad..ba904a7f40 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -47,6 +47,7 @@ jobs: - uses: nolar/setup-k3d-k3s@v1 with: version: "${{ matrix.kubernetes }}" + k3d-tag: v4.4.8 k3d-args: --no-lb - name: Prefetch container images run: > diff --git a/internal/controller/postgrescluster/postgres.go b/internal/controller/postgrescluster/postgres.go index 78b55d7796..280ab14ee7 100644 --- a/internal/controller/postgrescluster/postgres.go +++ b/internal/controller/postgrescluster/postgres.go @@ -37,6 +37,7 @@ import ( "github.com/crunchydata/postgres-operator/internal/logging" "github.com/crunchydata/postgres-operator/internal/naming" "github.com/crunchydata/postgres-operator/internal/pgaudit" + "github.com/crunchydata/postgres-operator/internal/postgis" "github.com/crunchydata/postgres-operator/internal/postgres" pgpassword "github.com/crunchydata/postgres-operator/internal/postgres/password" "github.com/crunchydata/postgres-operator/internal/util" @@ -188,7 +189,7 @@ func (r *Reconciler) reconcilePostgresDatabases( // Calculate a hash of the SQL that should be executed in PostgreSQL. - var pgAuditOK bool + var pgAuditOK, postgisInstallOK bool create := func(ctx context.Context, exec postgres.Executor) error { if pgAuditOK = pgaudit.EnableInPostgreSQL(ctx, exec) == nil; !pgAuditOK { // pgAudit can only be enabled after its shared library is loaded, @@ -201,6 +202,18 @@ func (r *Reconciler) reconcilePostgresDatabases( "Unable to install pgAudit; try restarting PostgreSQL") } + // Enabling PostGIS extensions is a one-way operation + // e.g., you can take a PostgresCluster and turn it into a PostGISCluster, + // but you cannot reverse the process, as that would potentially remove an extension + // that is being used by some database/tables + if cluster.Spec.PostGISVersion != "" { + if postgisInstallOK = postgis.EnableInPostgreSQL(ctx, exec) == nil; !postgisInstallOK { + // TODO(benjb): Investigate under what conditions postgis would fail install + r.Recorder.Event(cluster, corev1.EventTypeWarning, "postgisDisabled", + "Unable to install postgis") + } + } + return postgres.CreateDatabasesInPostgreSQL(ctx, exec, databases.List()) } @@ -232,7 +245,7 @@ func (r *Reconciler) reconcilePostgresDatabases( log := logging.FromContext(ctx).WithValues("revision", revision) err = errors.WithStack(create(logging.NewContext(ctx, log), podExecutor)) } - if err == nil && pgAuditOK { + if err == nil && pgAuditOK && postgisInstallOK { cluster.Status.DatabaseRevision = revision } diff --git a/internal/postgis/postgis.go b/internal/postgis/postgis.go new file mode 100644 index 0000000000..076a9cb856 --- /dev/null +++ b/internal/postgis/postgis.go @@ -0,0 +1,53 @@ +/* + Copyright 2021 Crunchy Data Solutions, Inc. + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package postgis + +import ( + "context" + "strings" + + "github.com/crunchydata/postgres-operator/internal/logging" + "github.com/crunchydata/postgres-operator/internal/postgres" +) + +// EnableInPostgreSQL installs triggers for the following extensions into every database: +// - postgis +// - postgis_topology +// - fuzzystrmatch +// - postgis_tiger_geocoder +func EnableInPostgreSQL(ctx context.Context, exec postgres.Executor) error { + log := logging.FromContext(ctx) + + stdout, stderr, err := exec.ExecInAllDatabases(ctx, + strings.Join([]string{ + // Quiet NOTICE messages from IF NOT EXISTS statements. + // - https://www.postgresql.org/docs/current/runtime-config-client.html + `SET client_min_messages = WARNING;`, + + `CREATE EXTENSION IF NOT EXISTS postgis;`, + `CREATE EXTENSION IF NOT EXISTS postgis_topology;`, + `CREATE EXTENSION IF NOT EXISTS fuzzystrmatch;`, + `CREATE EXTENSION IF NOT EXISTS postgis_tiger_geocoder;`, + }, "\n"), + map[string]string{ + "ON_ERROR_STOP": "on", // Abort when any one statement fails. + "QUIET": "on", // Do not print successful statements to stdout. + }) + + log.V(1).Info("enabled postgis and related extensions", "stdout", stdout, "stderr", stderr) + + return err +} diff --git a/internal/postgis/postgis_test.go b/internal/postgis/postgis_test.go new file mode 100644 index 0000000000..d7e09e69ed --- /dev/null +++ b/internal/postgis/postgis_test.go @@ -0,0 +1,54 @@ +/* + Copyright 2021 Crunchy Data Solutions, Inc. + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package postgis + +import ( + "context" + "errors" + "io" + "io/ioutil" + "strings" + "testing" + + "gotest.tools/v3/assert" +) + +func TestEnableInPostgreSQL(t *testing.T) { + expected := errors.New("whoops") + exec := func( + _ context.Context, stdin io.Reader, stdout, stderr io.Writer, command ...string, + ) error { + assert.Assert(t, stdout != nil, "should capture stdout") + assert.Assert(t, stderr != nil, "should capture stderr") + + assert.Assert(t, strings.Contains(strings.Join(command, "\n"), + `SELECT datname FROM pg_catalog.pg_database`, + ), "expected all databases and templates") + + b, err := ioutil.ReadAll(stdin) + assert.NilError(t, err) + assert.Equal(t, string(b), `SET client_min_messages = WARNING; +CREATE EXTENSION IF NOT EXISTS postgis; +CREATE EXTENSION IF NOT EXISTS postgis_topology; +CREATE EXTENSION IF NOT EXISTS fuzzystrmatch; +CREATE EXTENSION IF NOT EXISTS postgis_tiger_geocoder;`) + + return expected + } + + ctx := context.Background() + assert.Equal(t, expected, EnableInPostgreSQL(ctx, exec)) +} From fd93613a78eb80e4a146c47a964911bdb1332207 Mon Sep 17 00:00:00 2001 From: Ben Blattberg Date: Thu, 7 Oct 2021 16:27:49 -0500 Subject: [PATCH 2/3] Fix indentation Issue [sc-12487] --- internal/postgis/postgis.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/postgis/postgis.go b/internal/postgis/postgis.go index 076a9cb856..8d1b910d1e 100644 --- a/internal/postgis/postgis.go +++ b/internal/postgis/postgis.go @@ -24,8 +24,8 @@ import ( ) // EnableInPostgreSQL installs triggers for the following extensions into every database: -// - postgis -// - postgis_topology +// - postgis +// - postgis_topology // - fuzzystrmatch // - postgis_tiger_geocoder func EnableInPostgreSQL(ctx context.Context, exec postgres.Executor) error { From 41940ce75f23341a0dca2add23692372cd217a63 Mon Sep 17 00:00:00 2001 From: Ben Blattberg Date: Thu, 7 Oct 2021 20:32:56 -0500 Subject: [PATCH 3/3] Fix PostGIS capitalization Issue [sc-12487] --- internal/controller/postgrescluster/postgres.go | 4 ++-- internal/postgis/postgis.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/controller/postgrescluster/postgres.go b/internal/controller/postgrescluster/postgres.go index 280ab14ee7..56e03c4077 100644 --- a/internal/controller/postgrescluster/postgres.go +++ b/internal/controller/postgrescluster/postgres.go @@ -209,8 +209,8 @@ func (r *Reconciler) reconcilePostgresDatabases( if cluster.Spec.PostGISVersion != "" { if postgisInstallOK = postgis.EnableInPostgreSQL(ctx, exec) == nil; !postgisInstallOK { // TODO(benjb): Investigate under what conditions postgis would fail install - r.Recorder.Event(cluster, corev1.EventTypeWarning, "postgisDisabled", - "Unable to install postgis") + r.Recorder.Event(cluster, corev1.EventTypeWarning, "PostGISDisabled", + "Unable to install PostGIS") } } diff --git a/internal/postgis/postgis.go b/internal/postgis/postgis.go index 8d1b910d1e..b9abec57f0 100644 --- a/internal/postgis/postgis.go +++ b/internal/postgis/postgis.go @@ -47,7 +47,7 @@ func EnableInPostgreSQL(ctx context.Context, exec postgres.Executor) error { "QUIET": "on", // Do not print successful statements to stdout. }) - log.V(1).Info("enabled postgis and related extensions", "stdout", stdout, "stderr", stderr) + log.V(1).Info("enabled PostGIS and related extensions", "stdout", stdout, "stderr", stderr) return err }