Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion tests/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"go.opentelemetry.io/otel/trace"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/flanksource/commons/logger"
"github.com/flanksource/duty/context"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand Down Expand Up @@ -40,7 +41,8 @@ var _ = Describe("Context", func() {

Expect(c.GetObjectMeta().Name).To(Equal("test"))
Expect(c.IsDebug()).To(BeTrue())
Expect(c.IsTrace()).To(BeFalse())

Expect(c.IsTrace()).To(Equal(logger.IsTraceEnabled()))

Expect(c.GetName()).To(Equal("test"))
Expect(c.GetNamespace()).To(Equal("default"))
Expand Down
14 changes: 10 additions & 4 deletions tests/e2e/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,14 @@ func TestE2E(t *testing.T) {
ginkgo.RunSpecs(t, "E2E Suite")
}

var _ = ginkgo.BeforeSuite(func() {
DefaultContext = setup.BeforeSuiteFn()
})
var setupOpts = setup.SetupOpts{DummyData: true}

var _ = ginkgo.AfterSuite(setup.AfterSuiteFn)
var _ = ginkgo.SynchronizedBeforeSuite(
func() []byte { return setup.SetupTemplate(setupOpts) },
func(data []byte) { DefaultContext = setup.SetupNode(data, setupOpts) },
)

var _ = ginkgo.SynchronizedAfterSuite(
setup.SynchronizedAfterSuiteAllNodes,
setup.SynchronizedAfterSuiteNode1,
)
2 changes: 1 addition & 1 deletion tests/setup/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ func SetupDB(dbName string, args ...interface{}) (context.Context, error) {
return context.Context{}, fmt.Errorf("cannot create %s: %v", dbName, err)
}

shutdown.AddHookWithPriority("remote postgres", shutdown.PriorityCritical, func() {
shutdown.AddHookWithPriority("remove postgres db", shutdown.PriorityCritical, func() {
if err := execPostgres(postgresDBUrl, fmt.Sprintf("DROP DATABASE %s (FORCE)", dbName)); err != nil {
logger.Errorf("execPostgres: %v", err)
}
Expand Down
241 changes: 241 additions & 0 deletions tests/setup/template.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,241 @@
package setup

import (
"encoding/json"
"fmt"
"os"
"strconv"
"strings"

embeddedPG "github.com/fergusstrange/embedded-postgres"
"github.com/flanksource/commons/logger"
"github.com/flanksource/commons/properties"
"github.com/flanksource/duty"
"github.com/flanksource/duty/context"
dutyKubernetes "github.com/flanksource/duty/kubernetes"
"github.com/flanksource/duty/shutdown"
"github.com/flanksource/duty/telemetry"
"github.com/flanksource/duty/tests/fixtures/dummy"
"github.com/onsi/ginkgo/v2"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"
)

type SetupOpts struct {
DummyData bool
}

type templateInfo struct {
AdminURL string `json:"admin_url"`
TemplateDB string `json:"template_db"`
Port int `json:"port"`
}

func (t templateInfo) Marshal() []byte {
data, err := json.Marshal(t)
if err != nil {
panic(fmt.Sprintf("failed to marshal templateInfo: %v", err))
}
return data
}

func unmarshalTemplateInfo(data []byte) templateInfo {
var info templateInfo
if err := json.Unmarshal(data, &info); err != nil {
panic(fmt.Sprintf("failed to unmarshal templateInfo: %v", err))
}
return info
}

var (
adminURL string
nodeDBName string
)

func SetupTemplate(opts SetupOpts) []byte {
if err := properties.LoadFile(findFileInPath("test.properties", 2)); err != nil {
logger.Errorf("Failed to load test properties: %v", err)
}

defer telemetry.InitTracer()

var port int
if val, ok := os.LookupEnv(TEST_DB_PORT); ok {
parsed, err := strconv.ParseInt(val, 10, 32)
if err != nil {
panic(fmt.Sprintf("failed to parse TEST_DB_PORT: %v", err))
}
port = int(parsed)
} else {
port = duty.FreePort()
}

templateDB := "duty_test_template"

url := os.Getenv(DUTY_DB_URL)
if url != "" && !recreateDatabase {
// DUTY_DB_CREATE=false: use direct connection, no template
PgUrl = url
return templateInfo{AdminURL: url, TemplateDB: "", Port: port}.Marshal()
}

adminConn, err := ensurePostgres(port)
if err != nil {
panic(fmt.Sprintf("failed to start postgres: %v", err))
}
adminURL = adminConn

// Always recreate — dummy data uses uuid.New() so a cached template has stale UUIDs
_ = execPostgres(adminConn, fmt.Sprintf("ALTER DATABASE %s WITH is_template = false", templateDB))
_ = execPostgres(adminConn, fmt.Sprintf(
"SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '%s' AND pid <> pg_backend_pid()", templateDB))
_ = execPostgres(adminConn, fmt.Sprintf("DROP DATABASE IF EXISTS %s (FORCE)", templateDB))

if err := execPostgres(adminConn, fmt.Sprintf("CREATE DATABASE %s", templateDB)); err != nil {
panic(fmt.Sprintf("failed to create template db: %v", err))
}

templateURL := strings.Replace(adminConn, "/postgres", "/"+templateDB, 1)
if !strings.Contains(adminConn, "/postgres") {
templateURL = fmt.Sprintf("postgres://postgres:postgres@localhost:%d/%s?sslmode=disable", port, templateDB)
}
Comment on lines +99 to +102

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Parse the Postgres DSN instead of replacing "/postgres" as raw text.

strings.Replace(..., "/postgres", ..., 1) rewrites the first /postgres inside postgres://postgres:..., so the username becomes duty_test_template / duty_test_nodeN while the path stays /postgres. The auth failure reported at Line 111 is this exact corruption, and the same logic will break node connections too.

🐛 Suggested fix
-	templateURL := strings.Replace(adminConn, "/postgres", "/"+templateDB, 1)
-	if !strings.Contains(adminConn, "/postgres") {
-		templateURL = fmt.Sprintf("postgres://postgres:postgres@localhost:%d/%s?sslmode=disable", port, templateDB)
-	}
+	templateURL := withDatabase(adminConn, templateDB)
-	if strings.Contains(adminURL, "/postgres") {
-		PgUrl = strings.Replace(adminURL, "/postgres", "/"+nodeDBName, 1)
-	} else {
-		PgUrl = fmt.Sprintf("postgres://postgres:postgres@localhost:%d/%s?sslmode=disable", info.Port, nodeDBName)
-	}
+	PgUrl = withDatabase(adminURL, nodeDBName)
// add: neturl "net/url"
func withDatabase(rawURL, dbName string) string {
	u, err := neturl.Parse(rawURL)
	if err != nil {
		panic(fmt.Sprintf("invalid postgres url %q: %v", rawURL, err))
	}
	u.Path = "/" + dbName
	return u.String()
}

Also applies to: 172-176

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/setup/template.go` around lines 99 - 102, Replace the fragile
strings.Replace approach that mangles the DSN with a proper URL parse-and-modify
helper: add a function (e.g., withDatabase(rawURL, dbName string)) that uses
net/url.Parse to parse adminConn, sets u.Path = "/" + dbName, handles parse
errors (panic or return error per test setup style) and returns u.String(); then
use withDatabase(adminConn, templateDB) when computing templateURL (and
similarly replace the other occurrences noted around lines 172-176) instead of
doing strings.Replace so the username/host/query parts remain intact.


dbOptions := []duty.StartOption{duty.DisablePostgrest, duty.RunMigrations, duty.WithUrl(templateURL)}
if !disableRLS {
dbOptions = append(dbOptions, duty.EnableRLS)
}

ctx, stop, err := duty.Start(templateDB, dbOptions...)
if err != nil {
panic(fmt.Sprintf("failed to start duty for template: %v", err))
}

if err := ctx.DB().Exec("SET TIME ZONE 'UTC'").Error; err != nil {
panic(fmt.Sprintf("failed to set timezone: %v", err))
}

if opts.DummyData {
dummyData = dummy.GetStaticDummyData(ctx.DB())
if err := dummyData.Delete(ctx.DB()); err != nil {
logger.Errorf(err.Error())
}
if err := dummyData.Populate(ctx); err != nil {
panic(fmt.Sprintf("failed to populate dummy data: %v", err))
}
logger.Infof("Created dummy data in template (%d checks)", len(dummyData.Checks))
}

// Close all connections so the DB can be used as a template
stop()
_ = execPostgres(adminConn, fmt.Sprintf(
"SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '%s' AND pid <> pg_backend_pid()", templateDB))

if err := execPostgres(adminConn, fmt.Sprintf("ALTER DATABASE %s WITH is_template = true", templateDB)); err != nil {
panic(fmt.Sprintf("failed to mark template db: %v", err))
}

return templateInfo{AdminURL: adminConn, TemplateDB: templateDB, Port: port}.Marshal()
}

func SetupNode(data []byte, opts SetupOpts) context.Context {
info := unmarshalTemplateInfo(data)

if info.TemplateDB == "" {
// Direct connection mode (DUTY_DB_CREATE=false)
PgUrl = info.AdminURL
ctx, _, err := duty.Start("direct", duty.ClientOnly, duty.WithUrl(PgUrl))
if err != nil {
panic(fmt.Sprintf("failed to connect to db: %v", err))
}
return setupNodeContext(ctx, "direct")
}

adminURL = info.AdminURL
nodeDBName = fmt.Sprintf("duty_test_node%d", ginkgo.GinkgoParallelProcess())

// Drop and clone from template
_ = execPostgres(adminURL, fmt.Sprintf("DROP DATABASE IF EXISTS %s (FORCE)", nodeDBName))

// Terminate any lingering connections to the template before cloning
_ = execPostgres(adminURL, fmt.Sprintf(
"SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '%s' AND pid <> pg_backend_pid()", info.TemplateDB))

// Unmark template temporarily for cloning (some pg versions need this)
_ = execPostgres(adminURL, fmt.Sprintf("ALTER DATABASE %s WITH is_template = false", info.TemplateDB))
if err := execPostgres(adminURL, fmt.Sprintf("CREATE DATABASE %s TEMPLATE %s", nodeDBName, info.TemplateDB)); err != nil {
panic(fmt.Sprintf("failed to clone template: %v", err))
}
_ = execPostgres(adminURL, fmt.Sprintf("ALTER DATABASE %s WITH is_template = true", info.TemplateDB))

// Build node connection URL
if strings.Contains(adminURL, "/postgres") {
PgUrl = strings.Replace(adminURL, "/postgres", "/"+nodeDBName, 1)
} else {
PgUrl = fmt.Sprintf("postgres://postgres:postgres@localhost:%d/%s?sslmode=disable", info.Port, nodeDBName)
}

// Skip migrations — the clone is byte-for-byte identical to the template
ctx, _, err := duty.Start(nodeDBName, duty.ClientOnly, duty.WithUrl(PgUrl))
if err != nil {
panic(fmt.Sprintf("failed to connect to node db: %v", err))
}

return setupNodeContext(ctx, nodeDBName)
}

func setupNodeContext(ctx context.Context, dbName string) context.Context {
if err := ctx.DB().Exec("SET TIME ZONE 'UTC'").Error; err != nil {
panic(fmt.Sprintf("failed to set timezone: %v", err))
}

ctx = ctx.WithValue("db_name", dbName).WithValue("db_url", PgUrl)

clientset := fake.NewClientset(&v1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{Name: "test-cm", Namespace: "default"},
Data: map[string]string{"foo": "bar"},
}, &v1.Secret{
ObjectMeta: metav1.ObjectMeta{Name: "test-secret", Namespace: "default"},
Data: map[string][]byte{"foo": []byte("secret")},
})

return ctx.WithLocalKubernetes(dutyKubernetes.NewKubeClient(logger.GetLogger("k8s"), clientset, nil))
}

func SynchronizedAfterSuiteAllNodes() {
if nodeDBName != "" && adminURL != "" {
if err := execPostgres(adminURL, fmt.Sprintf("DROP DATABASE IF EXISTS %s (FORCE)", nodeDBName)); err != nil {
logger.Errorf("failed to drop node db: %v", err)
}
}
}

func SynchronizedAfterSuiteNode1() {
shutdown.Shutdown()
}


func ensurePostgres(port int) (string, error) {
url := os.Getenv(DUTY_DB_URL)
if url != "" {
postgresDBUrl = url
return url, nil
}

if postgresServer == nil {
config, _ := GetEmbeddedPGConfig("postgres", port)

if v, ok := os.LookupEnv(DUTY_DB_DATA_DIR); ok {
config = config.DataPath(v)
}

postgresServer = embeddedPG.NewDatabase(config)
logger.Infof("starting embedded postgres on port %d", port)
if err := postgresServer.Start(); err != nil {
return "", err
}
logger.Infof("Started postgres on port %d", port)
}
Comment on lines +225 to +238

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Re-add the embedded Postgres shutdown hook in the new setup path.

ensurePostgres() starts postgresServer, but this code path no longer registers the "embedded pg" shutdown hook that tests/setup/common.go used. SynchronizedAfterSuiteNode1() only calls shutdown.Shutdown(), so the child Postgres process never gets stopped here and can keep the port/data dir locked across suites.

🧹 Suggested fix
 	if postgresServer == nil {
 		config, _ := GetEmbeddedPGConfig("postgres", port)

 		if v, ok := os.LookupEnv(DUTY_DB_DATA_DIR); ok {
 			config = config.DataPath(v)
 		}

 		postgresServer = embeddedPG.NewDatabase(config)
 		logger.Infof("starting embedded postgres on port %d", port)
 		if err := postgresServer.Start(); err != nil {
 			return "", err
 		}
 		logger.Infof("Started postgres on port %d", port)
+		shutdown.AddHookWithPriority("embedded pg", shutdown.PriorityCritical, func() {
+			if err := postgresServer.Stop(); err != nil {
+				logger.Errorf("postgresServer.Stop: %v", err)
+			}
+		})
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if postgresServer == nil {
config, _ := GetEmbeddedPGConfig("postgres", port)
if v, ok := os.LookupEnv(DUTY_DB_DATA_DIR); ok {
config = config.DataPath(v)
}
postgresServer = embeddedPG.NewDatabase(config)
logger.Infof("starting embedded postgres on port %d", port)
if err := postgresServer.Start(); err != nil {
return "", err
}
logger.Infof("Started postgres on port %d", port)
}
if postgresServer == nil {
config, _ := GetEmbeddedPGConfig("postgres", port)
if v, ok := os.LookupEnv(DUTY_DB_DATA_DIR); ok {
config = config.DataPath(v)
}
postgresServer = embeddedPG.NewDatabase(config)
logger.Infof("starting embedded postgres on port %d", port)
if err := postgresServer.Start(); err != nil {
return "", err
}
logger.Infof("Started postgres on port %d", port)
shutdown.AddHookWithPriority("embedded pg", shutdown.PriorityCritical, func() {
if err := postgresServer.Stop(); err != nil {
logger.Errorf("postgresServer.Stop: %v", err)
}
})
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/setup/template.go` around lines 225 - 238, ensurePostgres() starts
postgresServer via embeddedPG.NewDatabase(...) but doesn't register the shutdown
hook, so the child Postgres process isn't stopped by
SynchronizedAfterSuiteNode1() calling shutdown.Shutdown(); add a shutdown
registration for postgresServer (e.g. shutdown.Register("embedded pg", func()
error { return postgresServer.Stop() })) right after postgresServer.Start() (or
immediately after creating postgresServer) so the embedded Postgres is properly
stopped during shutdown; reference ensurePostgres(), postgresServer,
embeddedPG.NewDatabase, SynchronizedAfterSuiteNode1(), and shutdown.Shutdown()
when making the change.


return fmt.Sprintf("postgres://postgres:postgres@localhost:%d/postgres?sslmode=disable", port), nil
}
14 changes: 10 additions & 4 deletions tests/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,14 @@ func TestDuty(t *testing.T) {
ginkgo.RunSpecs(t, "Duty Suite")
}

var _ = ginkgo.BeforeSuite(func() {
DefaultContext = setup.BeforeSuiteFn()
})
var setupOpts = setup.SetupOpts{DummyData: true}

var _ = ginkgo.AfterSuite(setup.AfterSuiteFn)
var _ = ginkgo.SynchronizedBeforeSuite(
func() []byte { return setup.SetupTemplate(setupOpts) },
func(data []byte) { DefaultContext = setup.SetupNode(data, setupOpts) },
)

var _ = ginkgo.SynchronizedAfterSuite(
setup.SynchronizedAfterSuiteAllNodes,
setup.SynchronizedAfterSuiteNode1,
)
Loading