feat: add --postgres-database-url-file flag for file-based DB credentials

[MatteoMori8]

Summary

• Add --postgres-database-url-file CLI flag (auto-mapped to POSTGRES_DATABASE_URL_FILE env var) to read the PostgreSQL connection URL from a file
• File contents take precedence over --postgres-database-url when set
• Update Helm chart with database.postgres.urlFile value and conditional ConfigMap entry

[Copilot]

Pull request overview

Adds support for supplying the Postgres connection URL via a file, enabling secret-file based deployments (e.g., Kubernetes Secrets) while keeping existing direct-URL configuration.

Changes:

• Add --postgres-database-url-file flag / POSTGRES_DATABASE_URL_FILE env var support and resolve URL from file with precedence over --postgres-database-url.
• Add Helm values (database.postgres.urlFile) and conditionally emit POSTGRES_DATABASE_URL_FILE in the controller ConfigMap.
• Add unit tests covering the new flag/env var and file-reading behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
helm/kagent/values.yaml	Adds database.postgres.urlFile value (defaults empty) with precedence comment.
helm/kagent/templates/controller-configmap.yaml	Emits POSTGRES_DATABASE_URL_FILE when urlFile is set, otherwise falls back to POSTGRES_DATABASE_URL.
go/pkg/app/app.go	Adds config field + flag and resolves DB URL from file during startup.
go/pkg/app/app_test.go	Adds tests for URL-file behavior and the new flag/env var mapping.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[EItanya]

Changes make sense to me. Any particular reason this is a file and not just an env var, isn't it just a single string

[MatteoMori8]

Changes make sense to me. Any particular reason this is a file and not just an env var, isn't it just a single string

Hey @EItanya 👋

Our infrastructure uses a secrets management solution that injects credentials as files mounted into pods rather than environment variables. Without this change I cannot get the Postgres credentials I need to make kAgent happy

[EItanya]

I understand why you implemented it this way for brevity, but I'm worried that this sort of change is hard to track overtime without regression test. Rather, if you also add the config to the actual manager then we can more easily test these changes in-situ. What do you think?

[MatteoMori8]

Hey @EItanya

Not sure if I fully grasp your point unfortunately but, I personally see the value in resolving the file in Start() ( it looks like a very similar pattern as what LoadFromEnv does, no? ). My idea was to resolve the inputs first and then send "clean" data to the internal database layer.

That said, if you'd prefer it in NewManager I can add a URLFile field to PostgresConfig and resolve it there with a small helper. Happy either way!

[EItanya]

I do prefer the Url vs UrlFile being resolved in the manager. Imagine later down the road some code changes in the manager, or the start function, we would not know that this feature is broken until either we have a specific e2e test for it, or you report another bug. If the logic is in the manager then we can have a more accurate unit test to reflect it, and then a unit test to check that the app level config flag is properly added to the db options. Does that make sense?

[MatteoMori8]

@EItanya - I think I am seeing what you mean. Something like the last change?

[EItanya]

I understand why you're removing this as there are now secrets, but I think we need redaction rather than removing this entirely, this log line is incredibly useful to know what the effective config is

[MatteoMori8]

@EItanya - Fair enough! I have reverted the change and I will have a look at sorting out hiding sensitive info in another PR, if it helps

[EItanya]

@EItanya - I think I am seeing what you mean. Something like the last change?

Yes this is what I meant :)