[!] use native Postgres connection parameters and ENVVARs (#695)

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.

Author: pashagolub

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
 }

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

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)

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)