Skip to content

Move kubectl unbounded site add-machine to kubectl unbounded machine register#15

Merged
jveski merged 3 commits into
mainfrom
copilot/move-unbounded-site-add-machine
Apr 14, 2026
Merged

Move kubectl unbounded site add-machine to kubectl unbounded machine register#15
jveski merged 3 commits into
mainfrom
copilot/move-unbounded-site-add-machine

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 9, 2026 •
edited
Loading

Summary

Moves the site add-machine subcommand from the site command group to the machine command group as machine register. This better aligns the command hierarchy—machine lifecycle operations (register, reboot, reimage, manual-bootstrap) are now all under kubectl unbounded machine.

Changes

Go source

  • Renamed site_add_machine.go → machine_register.go
    • siteAddMachineHandler → machineRegisterHandler
    • siteAddMachineCommand() → machineRegisterCommand()
    • Cobra Use changed from "add-machine" to "register"
    • Cobra Short updated to "Register a machine to the site"
  • Renamed site_add_machine_test.go → machine_register_test.go
    • All test function names updated (TestSiteAddMachine… → TestMachineRegister…)
    • Assertion updated to expect "register" instead of "add-machine"
  • cmd_site.go: Removed siteAddMachineCommand() from site subcommands
  • cmd_machine.go: Added machineRegisterCommand() to machine subcommands

Documentation

  • README.md: Updated example command
  • docs/content/concepts/overview.md: Updated CLI reference
  • docs/content/guides/existing-cluster.md: Updated command examples
  • docs/content/guides/ssh.md: Updated all command examples and cross-references
  • docs/content/reference/cli.md: Moved section under new machine command group header, updated all examples
  • docs/content/reference/architecture.md: Updated command table and image alt text
  • docs/layouts/index.html: Updated landing page example
  • docs/static/img/architecture-ssh-provisioning.svg: Updated SVG text

Testing

  • go build ./cmd/kubectl-unbounded/... ✅
  • go test ./cmd/kubectl-unbounded/... ✅ (all tests pass)

@jveski
refactor: move site add-machine subcommand to machine create
1691d88 
Rename siteAddMachineHandler → machineCreateHandler and
siteAddMachineCommand → machineCreateCommand. Move the command
registration from cmd_site.go to cmd_machine.go. Update all
documentation, tests, and the architecture SVG.

Agent-Logs-Url: https://github.com/Azure/unbounded-kube/sessions/ab4d5170-33f1-48ad-b635-1980f8efb239

Co-authored-by: jveski <7576912+jveski@users.noreply.github.com>
Copilot AI requested a review from jveski April 9, 2026 21:16
@jveski jveski marked this pull request as ready for review April 9, 2026 21:17
@jveski jveski requested a review from a team April 9, 2026 21:17
@plombardi89
Copy link
Copy Markdown
Collaborator

plombardi89 commented Apr 9, 2026

Wonder if create is the right word for this? It sort of implies it will create a Machine behind the scenes but really this is more like register

@jveski
Copy link
Copy Markdown
Contributor

jveski commented Apr 9, 2026

Wonder if create is the right word for this? It sort of implies it will create a Machine behind the scenes but really this is more like register

IMO create generically implies that a k8s object will be created like kubectl create secret. But register is more specific so let's go with that.

@copilot Go ahead and make the change 🚀

@jwilder
Copy link
Copy Markdown
Contributor

jwilder commented Apr 9, 2026

Wonder if create is the right word for this? It sort of implies it will create a Machine behind the scenes but really this is more like register

Should also consider the removal side too.

create and delete?
register and unregister?
join and unjoin?

@plombardi89
Copy link
Copy Markdown
Collaborator

plombardi89 commented Apr 9, 2026

Wonder if create is the right word for this? It sort of implies it will create a Machine behind the scenes but really this is more like register

Should also consider the removal side too.

create and delete? register and unregister? join and unjoin?

Wonder if create is the right word for this? It sort of implies it will create a Machine behind the scenes but really this is more like register

Should also consider the removal side too.

create and delete? register and unregister? join and unjoin?

unregister if we're going with register ... I think worth remembering there's still kubectl delete machine and kubectl create machine if they drop down to the actual CR API. What unbounded provides is possible semantics and UX that can't be expressed via the usual CR API.

@jveski
refactor: rename machine create to machine register
a888391 
Per review feedback, `register` better describes the intent of the
command since it registers an existing machine rather than creating
one from scratch.

Agent-Logs-Url: https://github.com/Azure/unbounded-kube/sessions/bb29a5cd-f364-403c-8130-d925380eeb97

Co-authored-by: jveski <7576912+jveski@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 9, 2026

Wonder if create is the right word for this? It sort of implies it will create a Machine behind the scenes but really this is more like register

IMO create generically implies that a k8s obje...

Done in a888391 — renamed create → register across all Go source, tests, and documentation.

Copilot AI changed the title Move kubectl unbounded site add-machine to kubectl unbounded machine create Move kubectl unbounded site add-machine to kubectl unbounded machine register Apr 9, 2026
@jveski
Copy link
Copy Markdown
Contributor

jveski commented Apr 10, 2026

@copilot rebase please

@jveski
Merge remote-tracking branch 'origin/main' into copilot/move-unbounde…
514f4dc 
…d-site-add-machine

# Conflicts:
#	README.md

Co-authored-by: jveski <7576912+jveski@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 10, 2026

@copilot rebase please

Merged main into the branch in 514f4dc. Resolved the README.md conflict from the upstream rewrite — the new README doesn't reference site add-machine so no additional updates were needed there. Build and tests pass.

@jveski jveski merged commit 47c9fba into main Apr 14, 2026
7 checks passed
@jveski jveski deleted the copilot/move-unbounded-site-add-machine branch April 14, 2026 16:43
plombardi89 added a commit that referenced this pull request May 14, 2026
@plombardi89
orca/origin: clamp List max-results before int32 cast (CodeQL #14, #15)
daa4582 
GHAS / CodeQL flagged two "Incorrect conversion between integer
types" findings on the awss3 and azureblob origin adapters: the
List call sites cast a host-int maxResults to int32 without an
upper-bound check, so an untrusted caller passing
?max-keys=99999999999 would silently overflow MaxKeys/MaxResults
to a non-deterministic (often negative) int32 value before it
reached the SDK. The server-side parse (server.go:704-711) only
guards v > 0; the > int32 max case slipped through.

Per-adapter clamp instead of a server-side ceiling so each backend
keeps its own documented max:
- awss3.clampMaxKeys: 0..1000 (S3 ListObjectsV2 server-side cap)
- azureblob.clampMaxResults: 0..5000 (Azure ListBlobs server-side cap)

A server-wide cap would have artificially limited Azure callers to
the smaller of the two ceilings; clamping per-adapter keeps the
backends' native quotas reachable while making the int32 conversion
locally provable. Negative inputs collapse to 0 (which the
backends interpret as "use default") so the symmetric overflow
window is also closed.

Tests: TestClampMaxKeys and TestClampMaxResults table-drive every
boundary (zero, small positive, exact cap, cap+1, math.MaxInt32,
math.MaxInt, -1, math.MinInt). Both run as fast unit tests with
no SDK or network seam needed.

Resolves https://github.com/Azure/unbounded/security/code-scanning/14
Resolves https://github.com/Azure/unbounded/security/code-scanning/15
plombardi89 added a commit that referenced this pull request May 14, 2026
@plombardi89
orca/origin: clamp List max-results before int32 cast (CodeQL #14, #15)
f756a1e 
GHAS / CodeQL flagged two "Incorrect conversion between integer
types" findings on the awss3 and azureblob origin adapters: the
List call sites cast a host-int maxResults to int32 without an
upper-bound check, so an untrusted caller passing
?max-keys=99999999999 would silently overflow MaxKeys/MaxResults
to a non-deterministic (often negative) int32 value before it
reached the SDK. The server-side parse (server.go:704-711) only
guards v > 0; the > int32 max case slipped through.

Per-adapter clamp instead of a server-side ceiling so each backend
keeps its own documented max:
- awss3.clampMaxKeys: 0..1000 (S3 ListObjectsV2 server-side cap)
- azureblob.clampMaxResults: 0..5000 (Azure ListBlobs server-side cap)

A server-wide cap would have artificially limited Azure callers to
the smaller of the two ceilings; clamping per-adapter keeps the
backends' native quotas reachable while making the int32 conversion
locally provable. Negative inputs collapse to 0 (which the
backends interpret as "use default") so the symmetric overflow
window is also closed.

Tests: TestClampMaxKeys and TestClampMaxResults table-drive every
boundary (zero, small positive, exact cap, cap+1, math.MaxInt32,
math.MaxInt, -1, math.MinInt). Both run as fast unit tests with
no SDK or network seam needed.

Resolves https://github.com/Azure/unbounded/security/code-scanning/14
Resolves https://github.com/Azure/unbounded/security/code-scanning/15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jwilder jwilder jwilder approved these changes

@jveski jveski Awaiting requested review from jveski

Assignees

@jveski jveski

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@plombardi89 @jveski @jwilder