Serve OpenAPI 3.0 spec at /openapi.v1.json#37038
Serve OpenAPI 3.0 spec at /openapi.v1.json#37038myers wants to merge 34 commits intogo-gitea:mainfrom
Conversation
|
Can you add more comments and document more details in the tool's code? For example: why it needs |
|
I think we should probably also evaluate using OAS3 for swagger-ui. Long-term I would like us to move completely to a OAS3-based solution, ideally OAS 3.1. |
There was a problem hiding this comment.
Pull request overview
Adds an OpenAPI 3.0 (OAS3) JSON spec endpoint generated at build-time by converting the existing Swagger 2.0 template, so API clients that require OAS3 can generate SDKs directly while keeping the existing Swagger v1 spec.
Changes:
- Serve an OAS3 spec at
/openapi.v1.jsonalongside the existing/swagger.v1.json. - Add a generator (
build/generate-openapi.go) to converttemplates/swagger/v1_json.tmpl→templates/swagger/v1_openapi3_json.tmpl. - Wire generation + consistency checks into
make generate-swagger/ backend checks and add required Go module dependencies.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| routers/web/web.go | Registers the new /openapi.v1.json route when Swagger is enabled. |
| routers/web/swagger_json.go | Adds the OpenAPI3Json handler to render the OAS3 JSON template. |
| build/generate-openapi.go | Implements Swagger2→OAS3 conversion and post-processing/enrichment steps. |
| build/openapi3-tools.go | Attempts to pin kin-openapi tool deps for generation. |
| templates/swagger/v1_openapi3_json.tmpl | Generated OAS3 JSON template served by the new route. |
| Makefile | Adds OpenAPI3 generation/check targets and hooks into existing checks. |
| go.mod / go.sum | Adds kin-openapi and related transitive deps. |
| .editorconfig | Adds formatting rules for the generated OAS3 template. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
IMO this should be one step. Displaying spec that's converted from generated one does not feel right to me. It's a step too much in the pipeline and I'd prefer to be closer to what we directly control. I'll try to get back to generating API directly from structs after I'll finish dealing with my package related PRs as I'm tied up there a bit. |
|
BTW I would make it just |
|
what about v4 which is supposedly in works? |
|
No idea about that , but I don't think anyone wants to maintain multiple versions of the api existing at the same time in the same repo, that's more like wishful thinking. |
|
But I guess keep the v1 for consistency with swagger and the unlikely event that there will be v2+. |
|
Regarding
|
SDK generators like progenitor (Rust) need named enum types to produce good code. Without them, you get duplicate anonymous string types on every field instead of a shared StateType enum. Deduplicating enums and giving them proper names in #/components/schemas/ is the main motivation for the converter. AST Walker: Walk Go source to find type StateType string declarations and extract the real type names. This would eliminate the map entirely and be correct by construction. But it's a significant scope increase — needs to parse Go source files, resolve which types are actually used in swagger-annotated structs, and handle edge cases. Worth doing eventually but feels like a separate PR. Or maybe better to focus efforts on building a better go annotations to OpenAPI 3 spec. My goal here was the shortest path to being able to use OpenAPI ecosystem tools. |
I understand the concern — a two-stage pipeline (annotations → Swagger 2.0 → OAS3) is more steps than ideal. But I think this PR is a useful incremental step:
That said, direct OAS3 generation would be strictly better long-term. |
e6f384f to
e9cd8cf
Compare
|
@silverwind @TheFox0x7 I made some changes. Could we move forward with this? If we don't want to move forward with this can you give me a path to get an openapi 3.x spec that you do like? |
Have you addressed reviews? |
I had missed some, thank you for the prompt. |
|
Reviewed with Claude Opus 4.7 and opencode; aggregated findings below. HighH1. Missing reserved username entry —
MediumM1.
M2. Walks response/request-body media-types but doesn't recurse into M3. Already discussed — author chose this over an AST walker as the shorter path. Worth documenting that this map must be updated when new enum types are added, otherwise new enums will silently get derived generic names. LowL1. Makefile prereq omits tools file —
L2. All other files in L3. Naming inconsistency — URL L4. When no Posted by Claude Opus 4.7 on behalf of @silverwind. |
|
Regarding Better designs, ranked:
|
Add a build-time conversion step that transforms the existing Swagger 2.0 spec into an OpenAPI 3.0 spec. The OAS3 spec is served alongside the existing Swagger 2.0 spec, enabling API clients that require OAS3 (progenitor, openapi-python-client, etc.) to generate code directly from Gitea's API. New files: - build/generate-openapi.go — converts swagger v1_json.tmpl to OAS3 - build/openapi3-tools.go — tool dependency for kin-openapi - routers/web/swagger_json.go — serves the OAS3 spec - templates/swagger/v1_openapi3_json.tmpl — generated OAS3 spec
- Add build/generate-openapi.go as Makefile prerequisite so changes to the converter trigger spec regeneration - Remove leftover `_ = key` no-op statement - Add top-of-file comment explaining what the converter does and why - Document why knownEnumTypes mapping is needed Co-Authored-By: Claude Opus 4 (claude-opus-4-6)
The enumGroups loop declared key but never used it; use _ instead so the generator compiles. Regenerate v1_openapi3_json.tmpl. Co-Authored-By: Claude (Opus)
Introduces build/openapi3gen as the testable home for OAS3 converter logic. Starts with EnumKey, the canonical value-set hash shared by the enum scanner and the converter. Co-Authored-By: Claude Opus 4.7 (claude-opus-4-7)
ScanSwaggerEnumTypes reads .go files under each directory, finds types documented with // swagger:enum TypeName, and returns a map from canonical value-set key to type name. Happy-path only; failure modes covered in follow-up commits. Co-Authored-By: Claude Opus 4.7 (claude-opus-4-7)
Task 8 promoted Permission fields to named string types (RepoWritePermission for input, AccessLevelName for output). Update integration tests that were building *string or comparing against raw string literals to use the new typed constants. Also fix latent runtime type mismatches in api_repo_collaborator_test.go and api_repo_teams_test.go where assert.Equal was comparing string literals against AccessLevelName fields (reflect.DeepEqual is type-sensitive so these would have failed at runtime). Co-Authored-By: Claude Sonnet 4.6 (claude-sonnet-4-6)
build/openapi3gen/convert.go is a normal Go package (not //go:build ignore like generate-openapi.go) and is subject to the depguard rule that forbids encoding/json in favor of modules/json. Co-Authored-By: Claude Opus 4.7 (claude-opus-4-7)
The new positive-assertion target should run in the same pipeline as the diff-based swagger-check and openapi3-check. Without this, the schema-name regression gate is runnable only manually. Co-Authored-By: Claude Opus 4.7 (claude-opus-4-7)
The kin-openapi dependency introduced by the OAS3 converter (and its transitive deps) needed license entries. Regenerated after merging upstream main, which pulled in tidy-check updates from other PRs. Co-Authored-By: Claude Opus 4.7 (claude-opus-4-7)
Replaces IIFE closures with Go 1.26's new(value) form, matching the idiom used for other *string fields in the same file. Co-Authored-By: Claude Opus 4.7 (claude-opus-4-7)
The converter itself aborts on any unmapped shared enum (via deriveEnumName's error path), and the existing openapi3-check idempotency target catches drift in the generated spec. The positive schema-name assertion was belt-and-suspenders with a hardcoded enum list that would drift as new types are added. Co-Authored-By: Claude Opus 4.7 (claude-opus-4-7)
4faceec to
b766a69
Compare
I pick this option. I ended up refactoring some other list of strings that should be enums. What do you think? EDIT: also addressed your CR findings. |
Addresses H1 from review: the /openapi.v1.json route is shadowable by a user registered with that name, since it's not in reservedUsernames. Mirrors the existing entry for swagger.v1.json. Test extended to cover both. Co-Authored-By: Claude Opus 4.7 (claude-opus-4-7)
Addresses M1 and L1 from review: - addDeprecatedFlags now matches "deprecated" as a leading marker rather than any substring, preventing future false-positives on phrases like "not deprecated". Current output is unchanged. - $(OPENAPI3_SPEC) rule now depends on build/openapi3-tools.go so changes to kin-openapi version pins trigger regeneration. Co-Authored-By: Claude Opus 4.7 (claude-opus-4-7)
Addresses L3 from review: the URL path /openapi.v1.json was inconsistent with the template filename v1_openapi3_json.tmpl and Makefile variables OPENAPI3_SPEC / generate-openapi3 etc. The "3" indicates OpenAPI version 3.0. Reserved username and integration test updated to match. Co-Authored-By: Claude Opus 4.7 (claude-opus-4-7)
|
Re-reviewed with Claude Opus 4.7 after latest commits. The rename to MediumM2. Walks only the top-level LowL2. All other AST walker (enumscan.go, new findings)A1. Single-pass ordering dependency —
A2. Grouped
A3. Blank-line-separated comment groups — cosmetic Only the All three AST issues are latent: they would silently miss a type, but Posted by Claude Opus 4.7 on behalf of @silverwind. |
|
AST approach is fine, a few more points above then I can approve. |
The exact-match reserved names openapi3.v1.json and swagger.v1.json are caught by the reserved list before the *.keys-style pattern list, so they produce name_reserved, not name_pattern_not_allowed. The previous "contains dot" heuristic conflated the two. Split the fixtures into reservedNames and patternNotAllowedNames, each with its own expected message key, so both paths are covered explicitly. Co-authored-by: Claude (Opus 4.7) <noreply@anthropic.com>
|
CI should pass now with the recent webkit removal from #37315. Is all feedback addressed? |
Top-level fixSchema walked only the root mediaType.Schema, leaving any nested type: file schemas as invalid OAS3. Walk Properties, Items, and AllOf/OneOf/AnyOf branches. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…yOf/not Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Previously collectEnumValues read enumTypes[ident.Name] at the moment a CONST decl was visited. If consts were placed before their annotated type — either within one file or across files sorted alphabetically — the values were silently dropped. Collect every annotated type first, then walk consts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
collectEnumType only looked at GenDecl.Doc, so annotations inside a grouped type(...) block were skipped. Read each TypeSpec.Doc as well, and note the blank-line caveat for future readers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Thanks for the careful re-review. Addressed M2/A1/A2/A3; declining L2 with reasoning below. M2 (nested file-schema recursion) — fixed. L2 (
What do you suggest? A1 (single-pass ordering dependency) — fixed. Two-pass scan: every annotated type is collected first across all files, then a second pass walks A2 (grouped A3 (blank-line comment-group caveat) — added a block comment on Drafted by Claude Opus 4.7 |
|
Resolved tests/integration/api_team_test.go: combined main's `apiTeam :=
DecodeJSON(t, resp, &api.Team{})` ergonomics with this branch's typed
api.AccessLevelName arguments to checkTeamResponse.
Regenerated templates/swagger/v1_openapi3_json.tmpl so the openapi3-check
backend check passes against the merged tree (picks up PreviousAttemptURL
on ActionRun and the new CreatePullReviewCommentReplyOptions schema).
Co-Authored-By: Claude <noreply@anthropic.com> (model: claude-opus-4-7)
Add a build-time conversion step that transforms the existing Swagger 2.0 spec into an OpenAPI 3.0 spec. The OAS3 spec is served alongside the existing Swagger 2.0 spec, enabling API clients that require OAS3 (progenitor, openapi-python-client, etc.) to generate code directly from Gitea's API.
New files:
This is not to be an answer to how gitea handles OAS3 long term, but a way to use what we have to move a step forward.
See https://github.com/myers/gt for how this can be used.
Written with Claude Code using Opus, and human reviewed.