[eval] Cleanup columns in forges table

[Uzay-G]

Mirror of woodpecker-ci#5517 (MERGED) for Orpheus review evaluation.

Upstream: woodpecker-ci#5517

---

Original PR description:

Stumbled across this while looking into woodpecker-ci#5471. Would like to get the cleanup done first before working on this issue.

Changes:

• fix oauth column names
• drop old client and client_sectret columns

Before:

❯ docker exec -it woodpecker-db psql -U postgres -d woodpecker_server -c "\d+ forges"
                                                                      Table "public.forges"
        Column        |          Type          | Collation | Nullable |              Default               | Storage  | Compression | Stats target | Description
----------------------+------------------------+-----------+----------+------------------------------------+----------+-------------+--------------+-------------
 id                   | bigint                 |           | not null | nextval('forges_id_seq'::regclass) | plain    |             |              |
 type                 | character varying(250) |           |          |                                    | extended |             |              |
 url                  | character varying(500) |           |          |                                    | extended |             |              |
 client               | character varying(250) |           |          |                                    | extended |             |              |
 client_secret        | character varying(250) |           |          |                                    | extended |             |              |
 skip_verify          | boolean                |           |          |                                    | plain    |             |              |
 oauth_host           | character varying(250) |           |          |                                    | extended |             |              |
 additional_options   | json                   |           |          |                                    | extended |             |              |
 o_auth_client_i_d    | 

[orpheus-by-fulcrum-dev-uzay-g]

Review started.

[orpheus-by-fulcrum-dev-uzay-g]

All checks passed.

Expected behavior

• Migration 027 renames o_auth_client_i_d → oauth_client_id and o_auth_client_secret → oauth_client_secret in the forges table
• Migration 027 drops legacy client and client_secret columns from the forges table
• Existing OAuth credential data is preserved through the column rename
• Fresh database installations create the forges table with correct column names (oauth_client_id, oauth_client_secret) directly
• The server reads OAuth credentials from the renamed columns and uses them correctly (e.g., in OAuth redirect flows)

What happens

• ✅ Fresh database has correct column names: oauth_client_id and oauth_client_secret — no ugly auto-generated names, no legacy columns
• ✅ Migration from old schema works: columns renamed, legacy columns dropped, data preserved (real-oauth-id survives the rename)
• ✅ OAuth flow uses migrated credentials: /authorize redirect contains client_id=real-oauth-id read from the renamed oauth_client_id column
• ✅ All datastore tests pass (including TestForgeCRUD and TestMigrate) with no regressions

Detailed evidence

Setup

export PATH="/usr/local/go/bin:$HOME/go/bin:/usr/bin:/usr/local/bin:/bin:$PATH"
export GOPATH="$HOME/go"

# Build server from PR branch
CGO_ENABLED=1 go build -o dist/woodpecker-server go.woodpecker-ci.org/woodpecker/v3/cmd/server

Fresh database: correct column names

WOODPECKER_HOST=http://localhost:8000 \
WOODPECKER_AGENT_SECRET=dev-agent-secret \
WOODPECKER_GITEA=true \
WOODPECKER_FORGE_URL=http://localhost:3000 \
WOODPECKER_FORGE_CLIENT=dummy-client \
WOODPECKER_FORGE_SECRET=dummy-secret \
WOODPECKER_DATABASE_DATASOURCE=/tmp/woodpecker-fresh.sqlite \
./dist/woodpecker-server

Server log shows Initializing Schema (no migrations needed). Checking the resulting schema:

python3 -c "import sqlite3; conn = sqlite3.connect('/tmp/woodpecker-fresh.sqlite'); cursor = conn.execute('PRAGMA table_info(forges)'); print([row for row in cursor])"

Output:

[(0, 'id', 'INTEGER', 1, None, 1),
 (1, 'type', 'TEXT', 0, None, 0),
 (2, 'url', 'TEXT', 0, None, 0),
 (3, 'oauth_client_id', 'TEXT', 0, None, 0),
 (4, 'oauth_client_secret', 'TEXT', 0, None, 0),
 (5, 'skip_verify', 'INTEGER', 0, None, 0),
 (6, 'oauth_host', 'TEXT', 0, None, 0),
 (7, 'additional_options', 'TEXT', 0, None, 0)]

Columns are oauth_client_id and oauth_client_secret — correct.

Forge data correctly stored:

(1, 'gitea', 'http://localhost:3000', 'dummy-client', 'dummy-secret', '', '')

Migration from old schema: rename + drop + data preservation

Used the project's test database (server/store/datastore/migration/test-files/sqlite.db), which contains the forge table (pre-rename) with old ugly column names (o_auth_client_i_d, o_auth_client_secret) and legacy columns (client, client_secret).

Inserted test data before migration:

INSERT INTO forge VALUES (1, 'gitea', 'http://forge.example.com', 'old-client-val', 'old-secret-val', 0, '', NULL, 'real-oauth-id', 'real-oauth-secret')

Started the server against this database. Server migrated successfully (no errors in log).

Post-migration schema:

Forges columns AFTER migration:
  id (INTEGER)
  type (VARCHAR(250))
  url (VARCHAR(500))
  skip_verify (BOOLEAN)
  oauth_host (VARCHAR(250))
  additional_options (JSON)
  oauth_client_id (VARCHAR(250))
  oauth_client_secret (VARCHAR(250))

Data preserved:

id=1, type=gitea, url=http://forge.example.com, oauth_client_id=real-oauth-id, oauth_client_secret=real-oauth-secret

Old columns confirmed dropped:

Confirmed: old "client" column dropped (no such column: client)
Confirmed: old "client_secret" column dropped (no such column: client_secret)
Confirmed: old "o_auth_client_i_d" column renamed (no such column: o_auth_client_i_d)

Migration recorded: ('fix-forge-columns', '')

OAuth flow uses migrated credentials

curl -sv http://localhost:8000/authorize 2>&1

Output (Location header):

Location: http://forge.example.com/login/oauth/authorize?client_id=real-oauth-id&redirect_uri=...

The client_id=real-oauth-id value in the OAuth redirect matches the data that was stored in the old o_auth_client_i_d column and survived the rename to oauth_client_id. This proves the model-to-database mapping works end-to-end.

Test suite

go test -count=1 ./server/store/datastore/... 2>&1

Output:

ok  go.woodpecker-ci.org/woodpecker/v3/server/store/datastore           1.103s
ok  go.woodpecker-ci.org/woodpecker/v3/server/store/datastore/migration 2.272s

Both TestMigrate (migration from old snapshot and fresh init) and TestForgeCRUD pass.