refactor(controlplane,dataplane): endpoints sqlc migration

[jirevwe]

No description provided.

[convoy-engineering]

@cursoragent bugbot review

[convoy-engineering]

@cursoragent bugbot review

[cursor]

Here is my updated review of PR #2608 after the latest commit (ebb0a41e):

---

Updated Review: endpoints sqlc migration (post-refactor commit)

The author addressed several findings from the prior review in commit ebb0a41e ("Refactor endpoints helpers and repository to simplify processing and remove unused code"), removing ~310 lines. Here's a summary of what was fixed and what remains.

Previously raised items -- now resolved:

• Item 1 (marshalAuthFields swallowing errors): Fixed. marshalAuthFields now returns an error as the fifth return value and wraps marshal failures with descriptive messages. Callers in CreateEndpoint and UpdateEndpoint properly check and propagate it.
• Item 2 (secretsToJSON swallowing errors): Fixed. secretsToJSON now returns ([]byte, error) and callers handle the error.
• Item 4 (UpdateEndpointStatus/UpdateEndpointSecrets returning unused rows): Fixed. Both queries changed from :one with RETURNING to :execresult. UpdateEndpointStatus was further simplified -- it no longer fetches the encryption key at all since it's just setting a status column. UpdateEndpointSecrets still correctly uses the encryption key for the pgp_sym_encrypt call. The rowToEndpoint type switch also lost two cases (UpdateEndpointStatusRow and UpdateEndpointSecretsRow), going from 9 to 7 cases.
• Item 7 (log.Fatal in constructor): Changed to panic with a descriptive message. This is marginally better since it's recoverable via recover(), though the practical difference is small.

Remaining items (non-blocking):

• ContentType typed as interface{} (queries.sql.go:154, 1132): Still present in the generated CreateEndpointParams and UpdateEndpointParams. This is a sqlc codegen limitation when the column appears in certain SQL expression contexts. The runtime value passed is always a string, so it's safe, but it loses compile-time type checking. A @content_type::text cast in the SQL might persuade sqlc to generate string, but this is minor.
• Service instantiated per request: Still present in handlers (endpointsvc.New(h.A.Logger, h.A.DB) on each HTTP request). Not a performance concern but a style nit vs. storing a singleton on the Handler struct.
• Missing OAuth2/BasicAuth round-trip tests: Test coverage still only covers API key auth end-to-end. Adding create+fetch tests for OAuth2 and BasicAuth auth types would be valuable.
• Repetitive type switch (helpers.go:56-160): Still 7 near-identical cases. Expected with sqlc-generated per-query row types.

Overall assessment: The migration is in good shape. The most important data-integrity issues (silent error swallowing) and the performance concern (unnecessary decryption in update queries) have been addressed. The remaining items are all non-blocking and can be addressed as follow-ups.

  

[subomi]

LGTM. Left just one nit comment.