Skip to content

fix!: drop the always-empty return value from Create - #51

Open
tas50 wants to merge 1 commit into
mainfrom
fix-create-returns-response
Open

fix!: drop the always-empty return value from Create#51
tas50 wants to merge 1 commit into
mainfrom
fix-create-returns-response

Conversation

@tas50

@tas50 tas50 commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Breaking change. Flagged fix!; happy to hold this one if you'd rather not spend the break.

Nodes.Create, Roles.Create and Environments.Create returned a *T decoded from the POST response body. The Chef Server answers those endpoints with {"uri":…}, not the created object — so the returned value was always a zero-valued struct behind a non-nil pointer.

The repo's own fixture says as much (roles_test.go:16):

srv.Handle("POST /organizations/o/roles",
    cinctest.Route{Status: 201, Body: `{"uri":"http://x/roles/db"}`})

and every call site in the repo already discarded the value. Confirmed against cinc-zero too: Nodes.Create returns &Node{} with every field empty.

The change

node, resp, err := c.Nodes.Create(ctx, n)   // before — node is always &Node{}
resp, err := c.Nodes.Create(ctx, n)         // after

Now matches Delete, which already returns (*Response, error).

Clients.Create deliberately keeps its *APIClient: chef_key genuinely is in that response, and it carries the generated private key a caller has exactly one chance to capture.

Migration

Mechanical — drop one return value at each call site:

- if _, _, err := c.Nodes.Create(ctx, n); err != nil {
+ if _, err := c.Nodes.Create(ctx, n); err != nil {

Test plan

  • TestNodesCreate_ReturnsOnlyResponseAndError — asserts the 201 comes back on *Response (did not compile before the change, which is the red for a signature change)
  • All in-repo callers updated: nodes_test.go, roles_test.go, environments_test.go, integration/integration_test.go
  • go vet ./... clean
  • go test ./... -race -count=2
  • cd integration && go test ./...
  • README Status table unchanged — no service, method, option or type is added, removed or renamed; the table does not carry signatures

@tas50
tas50 force-pushed the fix-create-returns-response branch 3 times, most recently from b9ebabd to 8a0cb33 Compare September 8, 2026 16:53
Nodes.Create, Roles.Create and Environments.Create returned a *T decoded
from the POST response. The Chef Server answers those with {"uri":...}, not
the created object, so the value was always a zero-valued struct behind a
non-nil pointer - the repo's own fixture (roles_test.go) returns exactly
that envelope and every caller discarded the result.

Return (*Response, error) instead, matching Delete. Clients.Create keeps its
*APIClient: chef_key really is in that response, and it carries the private
key a caller must capture.

BREAKING CHANGE: callers of Nodes.Create, Roles.Create and
Environments.Create drop one return value.

	node, resp, err := c.Nodes.Create(ctx, n)   // before
	resp, err := c.Nodes.Create(ctx, n)         // after

Signed-off-by: Tim Smith <tim@mondoo.com>
@tas50
tas50 force-pushed the fix-create-returns-response branch from 8a0cb33 to b2e1e75 Compare September 10, 2026 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant