Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unbound ssh option for older clients #1721

Merged
merged 3 commits into from
Nov 13, 2023
Merged

Conversation

hellt
Copy link
Member

@hellt hellt commented Nov 13, 2023

fix #1719

Comment on lines 74 to 82

// if we couldn't parse the ssh version we assume we can't use unbound option
// or if the version is lower than 8.9
// and the node has the PubkeyAuthentication set to unbound
// we set it to empty string since it is not supported by the SSH client
if (sshVersion == "" || semver.Compare(sshVersion, "8.9") < 0) && nodeData.SSHConfig.PubkeyAuthentication == types.PubkeyAuthValueUnbound {
nodeData.SSHConfig.PubkeyAuthentication = ""
}

Copy link
Member Author

Choose a reason for hiding this comment

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

@steiler I am going to merge this one so that people who upgrade won't have to wait long for this crit fix, but if you find a better way to handle this (like grepping ssh version once and storing it centrally) then we can revisit this in a subsequent PR

Copy link

codecov bot commented Nov 13, 2023

Codecov Report

Merging #1721 (7635509) into main (3b3cbef) will increase coverage by 0.05%.
The diff coverage is 81.81%.

❗ Current head 7635509 differs from pull request most recent head 29009f7. Consider uploading reports for the commit 29009f7 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1721      +/-   ##
==========================================
+ Coverage   51.51%   51.57%   +0.05%     
==========================================
  Files         142      143       +1     
  Lines       13776    13807      +31     
==========================================
+ Hits         7097     7121      +24     
- Misses       5891     5896       +5     
- Partials      788      790       +2     
Files Coverage Δ
clab/sshconfig.go 67.24% <80.00%> (+2.79%) ⬆️
utils/ssh.go 83.33% <83.33%> (ø)

... and 2 files with indirect coverage changes

@hellt hellt merged commit 19dbf13 into main Nov 13, 2023
18 of 20 checks passed
@hellt hellt deleted the unbound-when-supported branch November 13, 2023 17:48
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.

Use unbound ssh option only for ssh clients supporting it.
1 participant