Repository navigation
Conversation
Dependabot PR #720 updated oapi-codegen from 2.4.1 to 2.7.1, but it did not regenerate server.gen.go. Regenerate the file with the pinned version. The new generated code calls runtime functions that first appear in github.com/oapi-codegen/runtime v1.4.0. Update the runtime to v1.4.2, which also fixes a required-parameter regression from oapi-codegen 2.7.0. The generator renamed two Go fields. Variation.Id is now UnderscoreId and Project.LastSyncedFromSource is now UnderscoreLastSyncedFromSource. Update the hand-written handlers to use the new names. The JSON tags ("_id" and "_lastSyncedFromSource") do not change. Co-authored-by: rniebla <rniebla@launchdarkly.com>
oapi-codegen 2.7 accepts an empty body for an optional request body and sends a nil Body to the handler. PatchProject and PostAddProject read request.Body without a nil check, so an empty body caused a panic. Mark these request bodies as required in api.yaml. The generated code then returns the same 400 error for an empty body as before. Co-authored-by: rniebla <rniebla@launchdarkly.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requirements
This PR adds no new tests. The existing dev server tests pass, and a before/after test of the HTTP API is in the next section. I built and tested on linux/amd64 only.
Related issues
#720 updated oapi-codegen from 2.4.1 to 2.7.1. That PR did not regenerate
internal/dev_server/api/server.gen.go. Onmain,go generate ./...rewrites the file, and thengo buildfails.Describe the solution you've provided
This PR regenerates the file and fixes the build. It does not change the API behavior or the JSON wire format of the dev server.
server.gen.gowith the pinned oapi-codegen v2.7.1.github.com/oapi-codegen/runtimefrom v1.1.2 to v1.4.2. The new generated code calls functions that first appear in v1.4.0. The oapi-codegen v2.7.0 release notes also require v1.4.0 or later. v1.4.2 fixes a required-parameter regression from oapi-codegen v2.7.0.Variation.Idis nowUnderscoreId.Project.LastSyncedFromSourceis nowUnderscoreLastSyncedFromSource. The JSON tags_idand_lastSyncedFromSourcedo not change.patchProjectandpostAddProjectas required inapi.yaml. oapi-codegen 2.7 sends a nil body to the handler when an optional body is empty. Both handlers read the body without a nil check. Without this change, an empty body causes a nil pointer panic and an HTTP 500 response. With this change, the server returns the same 400 error as before. The UI and the CLI always send a JSON body to these endpoints.The generated file has other changes that do not change the wire format:
Valid()methods on the enum typeshttp.Method*constants in the routerStrictHandlerFuncandStrictMiddlewareFunctypesWire format evidence
I ran the same 48 requests against two binaries: one built from
mainand one built from this branch. Each binary used its own empty database. The requests cover each endpoint inapi.yaml, including error cases such as bad JSON, bad query parameters, conflicts, and missing projects. Eight requests called the LaunchDarkly API with a real project. These cover add project, sync (PATCH), and the environments endpoint.Result: 44 responses are byte-identical, including the status code,
Content-Type, andContent-Length. The other 4 responses are live project responses. Their parsed JSON is equal, but the key order inside some flag values is different. The same difference occurs when I run themainbinary two times. The flag values areldvalue.Valueobjects, and their key order comes from Go map order, which is random.I also built the first commit of this PR alone. On that build,
PATCH /dev/projects/{key}with an empty body returned HTTP 500 because of a nil pointer panic. The second commit restores the old 400 response.Comparison table (the live project key is shown as <live>)
For the backup endpoint, the test compares the status, the headers, the body length, and the first 16 bytes of the body.
Checks that I ran
go generate ./...two times: the second run makes no changes. This includescmd/resources/resource_cmds.goand the mocks.go mod tidy: no changes.go build ./...: passes.go test ./...: all 55 packages pass. IfLD_ACCESS_TOKENis set in the environment, 3 tests incmd/flags,cmd/setup, andcmd/whoamifail. They fail the same way onmain.golangci-lintv1.63.4: no issues.npm ci,npm run lint,npm test, andnpm run buildpass. The build does not changedist/. This PR does not change the UI.Describe alternatives you've considered
required: truechange keeps the old response.Additional context
CI does not run
go generate ./...and then check for a diff. That is why #720 merged with green CI. A drift check in CI can find this type of problem in the future.