From 2dc9d246a09508b9801ac1568d9d81b5edf105e0 Mon Sep 17 00:00:00 2001 From: Pavlo Golub Date: Wed, 18 Jun 2025 15:10:11 +0200 Subject: [PATCH] [!] use native Postgres connection parameters and ENVVARs Drop all custom connection parameters and move to the native Postgres handling, e.g. PGHOST, PGPORT, etc. Allow user to specify a whole connection string with `--constr` or as unnamed parameter. --- internal/config/cmdparser.go | 36 ++++++++++------------------ internal/pgengine/bootstrap.go | 26 ++++---------------- internal/pgengine/pgengine_test.go | 16 ++++--------- internal/scheduler/scheduler_test.go | 2 +- 4 files changed, 21 insertions(+), 59 deletions(-) diff --git a/internal/config/cmdparser.go b/internal/config/cmdparser.go index 29af520e..2ecc09dd 100644 --- a/internal/config/cmdparser.go +++ b/internal/config/cmdparser.go @@ -7,18 +7,6 @@ import ( flags "github.com/jessevdk/go-flags" ) -// ConnectionOpts specifies the database connection options -type ConnectionOpts struct { - Host string `short:"h" long:"host" description:"PostgreSQL host" default:"localhost" env:"PGTT_PGHOST"` - Port int `short:"p" long:"port" description:"PostgreSQL port" default:"5432" env:"PGTT_PGPORT"` - DBName string `short:"d" long:"dbname" description:"PostgreSQL database name" default:"timetable" env:"PGTT_PGDATABASE"` - User string `short:"u" long:"user" description:"PostgreSQL user" default:"scheduler" env:"PGTT_PGUSER"` - Password string `long:"password" description:"PostgreSQL user password" env:"PGTT_PGPASSWORD"` - SSLMode string `long:"sslmode" default:"disable" description:"Connection SSL mode" env:"PGTT_PGSSLMODE" choice:"disable" choice:"require"` - PgURL string `long:"pgurl" description:"PostgreSQL connection URL" env:"PGTT_URL"` - Timeout int `long:"timeout" description:"PostgreSQL connection timeout" env:"PGTT_TIMEOUT" default:"90"` -} - // LoggingOpts specifies the logging configuration type LoggingOpts struct { LogLevel string `long:"log-level" mapstructure:"log-level" description:"Verbosity level for stdout and log file" choice:"debug" choice:"info" choice:"error" default:"info"` @@ -54,16 +42,16 @@ type RestAPIOpts struct { // CmdOptions holds command line options passed type CmdOptions struct { - ClientName string `short:"c" long:"clientname" description:"Unique name for application instance" env:"PGTT_CLIENTNAME"` - Config string `long:"config" description:"YAML configuration file"` - Connection ConnectionOpts `group:"Connection" mapstructure:"Connection"` - Logging LoggingOpts `group:"Logging" mapstructure:"Logging"` - Start StartOpts `group:"Start" mapstructure:"Start"` - Resource ResourceOpts `group:"Resource" mapstructure:"Resource"` - RESTApi RestAPIOpts `group:"REST" mapstructure:"REST"` - NoProgramTasks bool `long:"no-program-tasks" mapstructure:"no-program-tasks" description:"Disable executing of PROGRAM tasks" env:"PGTT_NOPROGRAMTASKS"` - NoHelpMessage bool `long:"no-help" mapstructure:"no-help" hidden:"system use"` - Version bool `short:"v" long:"version" mapstructure:"version" description:"Output detailed version information" env:"PGTT_VERSION"` + ClientName string `short:"c" long:"clientname" description:"Unique name for application instance" env:"PGTT_CLIENTNAME"` + Config string `long:"config" description:"YAML configuration file"` + ConnStr string `long:"connstr" description:"Connection string" env:"PGTT_CONNSTR"` + Logging LoggingOpts `group:"Logging" mapstructure:"Logging"` + Start StartOpts `group:"Start" mapstructure:"Start"` + Resource ResourceOpts `group:"Resource" mapstructure:"Resource"` + RESTApi RestAPIOpts `group:"REST" mapstructure:"REST"` + NoProgramTasks bool `long:"no-program-tasks" mapstructure:"no-program-tasks" description:"Disable executing of PROGRAM tasks" env:"PGTT_NOPROGRAMTASKS"` + NoHelpMessage bool `long:"no-help" mapstructure:"no-help" hidden:"system use"` + Version bool `short:"v" long:"version" mapstructure:"version" description:"Output detailed version information" env:"PGTT_VERSION"` } // Verbose returns true if the debug log is enabled @@ -102,8 +90,8 @@ func Parse(writer io.Writer) (*flags.Parser, error) { } } //non-option arguments - if len(nonOptionArgs) > 0 && cmdOpts.Connection.PgURL == "" { - cmdOpts.Connection.PgURL = nonOptionArgs[0] + if len(nonOptionArgs) > 0 && cmdOpts.ConnStr == "" { + cmdOpts.ConnStr = nonOptionArgs[0] } return parser, nil } diff --git a/internal/pgengine/bootstrap.go b/internal/pgengine/bootstrap.go index ebc999d7..1f362a54 100644 --- a/internal/pgengine/bootstrap.go +++ b/internal/pgengine/bootstrap.go @@ -3,7 +3,6 @@ package pgengine import ( "context" "errors" - "fmt" "math/rand" "os" "strings" @@ -80,9 +79,7 @@ var sqlNames = []string{"Schema Init", "Cron Functions", "Tables and Views", "JS // New opens connection and creates schema func New(ctx context.Context, cmdOpts config.CmdOptions, logger log.LoggerHookerIface) (*PgEngine, error) { var ( - err error - connctx = ctx - conncancel context.CancelFunc + err error ) pge := &PgEngine{ l: logger, @@ -91,15 +88,10 @@ func New(ctx context.Context, cmdOpts config.CmdOptions, logger log.LoggerHooker chainSignalChan: make(chan ChainSignal, 64), } pge.l.WithField("sid", pge.Getsid()).Info("Starting new session... ") - if cmdOpts.Connection.Timeout > 0 { // Timeout less than 0 allows endless connection attempts - connctx, conncancel = context.WithTimeout(ctx, time.Duration(cmdOpts.Connection.Timeout)*time.Second) - defer conncancel() - } - config := pge.getPgxConnConfig() - if err = retry.Do(connctx, backoff, func(ctx context.Context) error { + if err = retry.Do(ctx, backoff, func(ctx context.Context) error { if pge.ConfigDb, err = pgxpool.NewWithConfig(ctx, config); err == nil { - err = pge.ConfigDb.Ping(connctx) + err = pge.ConfigDb.Ping(ctx) } if err != nil { pge.l.WithError(err).Error("Connection failed") @@ -140,17 +132,7 @@ func quoteIdent(s string) string { // getPgxConnConfig transforms standard connestion string to pgx specific one with func (pge *PgEngine) getPgxConnConfig() *pgxpool.Config { - var connstr string - if pge.Connection.PgURL != "" { - connstr = pge.Connection.PgURL - } else { - connstr = fmt.Sprintf("host='%s' port='%d' dbname='%s' sslmode='%s' user='%s'", - pge.Connection.Host, pge.Connection.Port, pge.Connection.DBName, pge.Connection.SSLMode, pge.Connection.User) - if pge.Connection.Password != "" { - connstr += fmt.Sprintf(" password='%s'", pge.Connection.Password) - } - } - connConfig, err := pgxpool.ParseConfig(connstr) + connConfig, err := pgxpool.ParseConfig(pge.ConnStr) if err != nil { pge.l.WithError(err).Error("Cannot parse connection string") return nil diff --git a/internal/pgengine/pgengine_test.go b/internal/pgengine/pgengine_test.go index 3a20aedb..2aa26640 100644 --- a/internal/pgengine/pgengine_test.go +++ b/internal/pgengine/pgengine_test.go @@ -20,7 +20,7 @@ import ( // this instance used for all engine tests var pge *pgengine.PgEngine -var cmdOpts *config.CmdOptions = config.NewCmdOptions("--clientname=pgengine_unit_test", "--password=somestrong") +var cmdOpts *config.CmdOptions = config.NewCmdOptions("--clientname=pgengine_unit_test", "--connstr=postgresql://scheduler:somestrong@localhost/timetable") // SetupTestCaseEx allows to configure the test case before execution func SetupTestCaseEx(t *testing.T, fc func(c *config.CmdOptions)) func(t *testing.T) { @@ -49,14 +49,6 @@ func SetupTestCase(t *testing.T) func(t *testing.T) { } } -// setupTestRenoteDBFunc used to connect to remote postgreSQL database -var setupTestRemoteDBFunc = func() (pgengine.PgxConnIface, error) { - c := cmdOpts.Connection - connstr := fmt.Sprintf("host='%s' port='%d' sslmode='%s' dbname='%s' user='%s' password='%s'", - c.Host, c.Port, c.SSLMode, c.DBName, c.User, c.Password) - return pge.GetRemoteDBConnection(context.Background(), connstr) -} - func TestInitAndTestConfigDBConnection(t *testing.T) { teardownTestCase := SetupTestCase(t) defer teardownTestCase(t) @@ -120,7 +112,7 @@ func TestInitAndTestConfigDBConnection(t *testing.T) { } func TestFailedConnect(t *testing.T) { - c := config.NewCmdOptions("-h", "fake", "-c", "pgengine_test") + c := config.NewCmdOptions("--connstr='host=fake'", "-c", "pgengine_test") ctx, cancel := context.WithTimeout(context.Background(), pgengine.WaitTime*2) defer cancel() _, err := pgengine.New(ctx, *c, log.Init(config.LoggingOpts{LogLevel: "error"})) @@ -191,12 +183,12 @@ func TestGetRemoteDBTransaction(t *testing.T) { teardownTestCase := SetupTestCase(t) defer teardownTestCase(t) ctx := context.Background() - remoteDb, err := setupTestRemoteDBFunc() + remoteDb, err := pge.GetRemoteDBConnection(context.Background(), cmdOpts.ConnStr) defer pge.FinalizeDBConnection(ctx, remoteDb) require.NoError(t, err, "remoteDB should be initialized") require.NotNil(t, remoteDb, "remoteDB should be initialized") - assert.NoError(t, pge.SetRole(ctx, remoteDb, pgtype.Text{String: cmdOpts.Connection.User, Valid: true}), + assert.NoError(t, pge.SetRole(ctx, remoteDb, pgtype.Text{String: "scheduler", Valid: true}), "Set Role failed") assert.NotPanics(t, func() { pge.ResetRole(ctx, remoteDb) }, "Reset Role failed") pge.FinalizeDBConnection(ctx, remoteDb) diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go index 072028b9..56b4fc52 100644 --- a/internal/scheduler/scheduler_test.go +++ b/internal/scheduler/scheduler_test.go @@ -17,7 +17,7 @@ var pge *pgengine.PgEngine // SetupTestCase used to connect and to initialize test PostgreSQL database func SetupTestCase(t *testing.T) func(t *testing.T) { - cmdOpts := config.NewCmdOptions("-c", "pgengine_unit_test", "--password=somestrong") + cmdOpts := config.NewCmdOptions("-c", "pgengine_unit_test", "--connstr=postgresql://scheduler:somestrong@localhost/timetable") t.Log("Setup test case") timeout := time.After(6 * time.Second) done := make(chan bool)