Add custom SSH port support for dynamic and static nodepools#2026
Merged
Add custom SSH port support for dynamic and static nodepools#2026
Conversation
added 10 commits
March 28, 2026 20:39
…hen creating cluster
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds support for custom SSH ports on both static and dynamic nodepools throughout the Claudie platform, defaulting to port 22 when unspecified. Changes include API definitions, protobuf schemas, Ansible inventory templates, and service implementations. Changes
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
proto/spec/nodepool.proto (1)
111-112: Consider documenting the acceptedsshPortrange next to the new proto fields.You already document the
0 -> 22fallback; adding expected bounds (e.g.,1-65535) here would reduce ambiguity across downstream consumers.Also applies to: 144-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@proto/spec/nodepool.proto` around lines 111 - 112, Update the proto field comments to explicitly document the valid port range for sshPort and the other similar fields (e.g., the field at lines 144-145) by stating that 0 falls back to 22 and valid values are 1-65535 (or 0 for fallback), e.g., "SSH port for the nodes in this node pool. Default 0 means port 22; valid range 1-65535 (0 => 22)". Apply the same wording pattern to the other proto field referenced so downstream consumers have unambiguous bounds.services/terraformer/internal/worker/service/internal/cluster-builder/cluster_builder.go (1)
308-318: Consider using a map for deduplication.The
slices.Containscheck inside the loop is O(n²). While nodepool counts are typically small, a map-based approach would be more idiomatic for collecting unique values.♻️ Optional refactor using a map
func sshPorts(pools []*spec.NodePool) []int32 { - var ports []int32 + seen := make(map[int32]struct{}) for _, np := range pools { port := nodepools.SSHPort(np) - if !slices.Contains(ports, port) { - ports = append(ports, port) - } + seen[port] = struct{}{} + } + ports := make([]int32, 0, len(seen)) + for p := range seen { + ports = append(ports, p) } return ports }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/terraformer/internal/worker/service/internal/cluster-builder/cluster_builder.go` around lines 308 - 318, The sshPorts function currently uses slices.Contains inside a loop causing O(n²) behavior; change it to use a seen map (e.g., map[int32]struct{}) to deduplicate SSH ports: iterate pools, get port via nodepools.SSHPort(np), check seen[port], if not present set seen[port]=struct{}{} and append port to ports slice (this preserves first-seen order and is O(n)). Update the sshPorts function to use this map-based deduplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/api/crd/inputmanifest/v1beta1/inputmanifest_types.go`:
- Around line 126-128: The SshPort field lacks CRD validation so out-of-range
port values can be accepted; add kubebuilder validation markers to the SshPort
field in inputmanifest_types.go (the SshPort int32 `json:"sshPort,omitempty"`
declaration) by adding +kubebuilder:validation:Minimum=1 and
+kubebuilder:validation:Maximum=65535 above the field (preserving the existing
+optional comment) so the generated CRD enforces valid TCP port bounds.
In `@internal/api/manifest/manifest.go`:
- Around line 239-241: The SshPort field currently uses validate:"omitempty"
which permits out-of-range integers; update the struct tag on SshPort to
constrain values to the valid TCP port range (1–65535) by replacing or extending
the validate tag (e.g., validate:"omitempty,min=1,max=65535" or validator
equivalent such as gte/lte) on the SshPort int32 field in manifest.go so
negative values and values >65535 are rejected while preserving yaml/json tags.
In `@internal/api/manifest/utils.go`:
- Line 405: When building the stored spec where SshPort is set (the struct
literal that assigns SshPort: nodePool.SshPort), normalize the value so a zero
is converted to the documented static default 22 before persisting; update the
assignment to use a normalized value (e.g., computedPort := nodePool.SshPort; if
computedPort == 0 { computedPort = 22 }) and then assign SshPort: computedPort
so downstream consumers never see 0.
In
`@services/kube-eleven/internal/worker/service/internal/kube-eleven/kube_eleven.go`:
- Around line 202-203: The code is passing raw
nodepool.GetDynamicNodePool().SshPort (which is 0 when unset) into the template,
producing invalid SSH ports; normalize the value first by reading SshPort from
nodepool.GetDynamicNodePool(), set it to 22 when it is 0, and then pass that
normalized variable into the struct/template (do this for the SshPort assignment
around the SshPort: ... occurrence and the other occurrence at lines 213-214).
Ensure you reference and replace direct uses of
nodepool.GetDynamicNodePool().SshPort with the normalized local variable before
writing template data.
In `@services/manager/internal/service/rolling_update.go`:
- Line 62: The code unconditionally sets inner.SshPort =
nodepools.ClaudieSSHPort during the rolling-update clone which forces port 22522
for legacy pools; change the assignment to only apply when the cloned nodepool's
SSH port is unset/zero (i.e., if inner.SshPort == 0) so existing ports (like 22)
are preserved during clone; locate the assignment to inner.SshPort and replace
it with a conditional that keeps the original value unless it's missing, using
the symbols inner.SshPort and nodepools.ClaudieSSHPort.
---
Nitpick comments:
In `@proto/spec/nodepool.proto`:
- Around line 111-112: Update the proto field comments to explicitly document
the valid port range for sshPort and the other similar fields (e.g., the field
at lines 144-145) by stating that 0 falls back to 22 and valid values are
1-65535 (or 0 for fallback), e.g., "SSH port for the nodes in this node pool.
Default 0 means port 22; valid range 1-65535 (0 => 22)". Apply the same wording
pattern to the other proto field referenced so downstream consumers have
unambiguous bounds.
In
`@services/terraformer/internal/worker/service/internal/cluster-builder/cluster_builder.go`:
- Around line 308-318: The sshPorts function currently uses slices.Contains
inside a loop causing O(n²) behavior; change it to use a seen map (e.g.,
map[int32]struct{}) to deduplicate SSH ports: iterate pools, get port via
nodepools.SSHPort(np), check seen[port], if not present set
seen[port]=struct{}{} and append port to ports slice (this preserves first-seen
order and is O(n)). Update the sshPorts function to use this map-based
deduplication.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1fda5339-715b-43f2-859f-d456fce61db3
⛔ Files ignored due to path filters (1)
proto/pb/spec/nodepool.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (23)
docs/input-manifest/api-reference.mddocs/input-manifest/example.mddocs/input-manifest/providers/on-prem.mdinternal/api/crd/inputmanifest/v1beta1/inputmanifest_types.gointernal/api/manifest/manifest.gointernal/api/manifest/utils.gointernal/nodepools/nodepools.gomanifests/claudie/crd/claudie.io_inputmanifests.yamlproto/spec/nodepool.protoservices/ansibler/templates/all-node-inventory.goiniservices/ansibler/templates/k8s-inventory.goiniservices/ansibler/templates/lb-inventory.goiniservices/ansibler/templates/proxy-envs.goiniservices/claudie-operator/pkg/controller/controler_helpers.goservices/kube-eleven/internal/worker/service/internal/kube-eleven/kube_eleven.goservices/kube-eleven/internal/worker/service/internal/kube-eleven/types.goservices/kube-eleven/templates/kubeone.tplservices/manager/internal/service/existing_state.goservices/manager/internal/service/healthcheck.goservices/manager/internal/service/rolling_update.goservices/terraformer/internal/worker/service/internal/cluster-builder/cluster_builder.goservices/terraformer/internal/worker/service/internal/templates/structures.goservices/testing-framework/utils.go
…. Removed unnecessary code
…Not needed as terrafrom has static port in template
…parately in dynamic and static nodepools
m-brando
added
the
test-set-ordinary
Merged
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.
sshPort fieldin the manifestSummary by CodeRabbit
New Features
Documentation
sshPortfield for node pool specifications.