Kong / Kong/volcano-cli

Follow-ups from PR #24 (localmode compose/db-auth)

Open
#29 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
2
Forks
1
Avg merge
13h 32m
Merged PRs (30d)
45

Description

Tracking two non-blocking items surfaced during review of #24 (approved & merged separately).

### 1. `composeTemplateEnvValue` regex is brittle to inline comments
`internal/localmode/compose_test.go` — the value-extraction regex ends in `"?\s*$`, which breaks if an inline comment is ever added to one of the three timing lines:

- **Quoted + comment** (`REDIS_TIMEOUT: "60s" # note`) → `FindStringSubmatch` returns nil → `require.Len(match, 2)` fails with a misleading *"missing env var"*.
- **Unquoted + comment** (`USAGE_SYNC: 30s # note`) → the comment is swallowed into the capture group → `time.ParseDuration` errors.

The runtime YAML/compose parser strips inline comments fine (verified with `yaml.v3`), so this is test-only — but the template already uses inline comments heavily, so it's an easy trap. Suggested fix:

```go
re := regexp.MustCompile(`(?m)^\s*` + regexp.QuoteMeta(key) + `:\s*"?([^"#\n]+?)"?\s*(?:#.*)?$`)
```

Ref: https://github.com/Kong/volcano-cli/pull/24#discussion_r3523452497

### 2. Confirm `info_test.go` DB-URL fixture matches the server contract (cross-repo)
`internal/localmode/info_test.go` now asserts a new `database_url` shape:

```
postgres://volcano_client_:vpg_local_secret@localhost:8002/app?sslmode=disable&application_name=volcano_full_access
```

This is a pure passthrough unit test (no CLI bug — `info.go` forwards the URL verbatim to `psql`), but the format isn't what the server currently emits. In `volcano-hosting`, `local info` builds the URL via `localmode.DatabaseConnectionString`, which still returns the old shape (`volcano:volcano@... & application_name=volcano_full_access:`).

The new format drops the `:` suffix from `application_name` and introduces a scoped role (`volcano_client_` + real password). The server's pgproxy currently identifies full_access/admin connections by parsing that suffix — `strings.CutPrefix(appName, "volcano_full_access:")` (`pooling_proxy.go:847`). Confirm the paired server change (a) emits exactly this URL from `local info` and (b) moves routing/auth onto the scoped role, otherwise admin connections lose their target-DB routing.

Ref: https://github.com/Kong/volcano-cli/pull/24#discussion_r3523462009

Contributor guide

Open the contributing guide

Research direction

Start with internal/localmode/compose_test.go and internal/localmode/info_test.go, then inspect the paired server contract at localmode.DatabaseConnectionString and pooling_proxy.go:847. Run the localmode tests to reproduce the inline-comment and database_url cases. Done means comment-tolerant extraction tests pass and the server emits the stated URL while preserving target-DB routing and scoped-role authentication.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, postgresql
Domain
cli, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.