Skip to content

feat: Use Port IDs on Grant, Revoke SSH#397

Merged
drewmalin merged 6 commits into
mainfrom
dm/testfix
May 21, 2026
Merged

feat: Use Port IDs on Grant, Revoke SSH#397
drewmalin merged 6 commits into
mainfrom
dm/testfix

Conversation

@drewmalin
Copy link
Copy Markdown
Contributor

@drewmalin drewmalin commented May 19, 2026

Companion to https://github.com/brevdev/dev-plane/pull/2182

Port IDs must be explicitly used and persisted going forward, as SSH-type ports are simply TCP.

@drewmalin drewmalin changed the title fix: update ssh port test feat: Use Port IDs on Grant, Revoke SSH May 20, 2026
t.Vprint("")

// Check if the node already has an SSH port allocated (e.g. for another linux user)
port, err := existingSSHPort(ctx, deps, tokenProvider, reg)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a thing anymore. We now need to ask either: do you want to open a port, or do you want to use an existing port?

Comment on lines -109 to +110
t.Vprintf(" %s\n", t.Yellow(fmt.Sprintf("Warning: could not check for existing ports: %v", err)))
return fmt.Errorf("enable SSH failed: %w", err)
Copy link
Copy Markdown
Contributor Author

@drewmalin drewmalin May 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of these errors just ¯\(ツ)/¯ 'd before, so tightening them to immediately fail.

Comment on lines -148 to -150
if p.GetProtocol() == nodev1.PortProtocol_PORT_PROTOCOL_SSH {
return p.GetPortNumber(), nil
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Core to this PR: this will now always return false, so there is no point in doing any of this checking for the SSH protocol.

Comment on lines -181 to +287
return fmt.Errorf("invalid SSH port %d: port must be between 1 and 65535", port)
return "", fmt.Errorf("invalid port %d: port must be between 1 and 65535", port)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling it "SSH" here (and elsewhere) isn't exactly wrong, but making consistent with the rest of the system.

Comment on lines -334 to +447
taggedKey := pubKey + " " + BrevKeyTag(brevUserID)
taggedLine := pubKey + " " + DevplaneAuthorizedKeysComment(portID, brevUserID)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honor the devplane comment/tag here. Today we can leave keys dangling in authorized_keys.

Comment thread go.mod
Comment on lines +6 to +7
buf.build/gen/go/brevdev/devplane/connectrpc/go v1.19.2-20260520183101-9f4cb67aff2c.1
buf.build/gen/go/brevdev/devplane/protocolbuffers/go v1.36.11-20260520183101-9f4cb67aff2c.1
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread Makefile
@@ -346,11 +346,11 @@ develop-with-nix:
nix develop .

.PHONY: update-devplane-deps
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was failing to find the appropriate connect modules before.

@drewmalin drewmalin marked this pull request as ready for review May 20, 2026 19:56
@drewmalin drewmalin requested a review from a team as a code owner May 20, 2026 19:56
@drewmalin drewmalin merged commit f87411f into main May 21, 2026
9 checks passed
@drewmalin drewmalin deleted the dm/testfix branch May 21, 2026 23:07
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.

2 participants