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

fix: override tunnel details with user-provided settings #2707

Closed
wants to merge 9 commits into from

Conversation

chaospuppy
Copy link
Contributor

Description

Provides a means for users to override tunnel details when a target is specified

Related Issue

Fixes #2665

Checklist before merging

@chaospuppy chaospuppy requested review from dgershman and a team as code owners July 9, 2024 20:06
Copy link

netlify bot commented Jul 9, 2024

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit d84cc76
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/6691b7053f33d10008e76e92

@chaospuppy chaospuppy changed the title override tunnel details with user-provided settings fix: override tunnel details with user-provided settings Jul 9, 2024
Copy link

codecov bot commented Jul 9, 2024

Codecov Report

Attention: Patch coverage is 0% with 41 lines in your changes missing coverage. Please review.

Project coverage is 19.44%. Comparing base (a567687) to head (d84cc76).

Files Patch % Lines
src/pkg/cluster/tunnel.go 0.00% 27 Missing ⚠️
src/cmd/connect.go 0.00% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2707      +/-   ##
==========================================
- Coverage   19.44%   19.44%   -0.01%     
==========================================
  Files         171      171              
  Lines       12161    12163       +2     
==========================================
  Hits         2365     2365              
- Misses       9495     9497       +2     
  Partials      301      301              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@chaospuppy
Copy link
Contributor Author

I'll add tests for NewTargetTunnelInfo if the rest of this PR is acceptable

Copy link
Contributor

@AustinAbro321 AustinAbro321 left a comment

Choose a reason for hiding this comment

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

Thanks for making this, a few comments

src/pkg/cluster/tunnel.go Outdated Show resolved Hide resolved
src/config/lang/english.go Show resolved Hide resolved
@AustinAbro321 AustinAbro321 requested review from a team as code owners August 2, 2024 15:33
Copy link

codecov bot commented Aug 2, 2024

Codecov Report

Attention: Patch coverage is 0% with 41 lines in your changes missing coverage. Please review.

Files Patch % Lines
src/pkg/cluster/tunnel.go 0.00% 27 Missing ⚠️
src/cmd/connect.go 0.00% 14 Missing ⚠️
Files Coverage Δ
src/cmd/connect.go 0.00% <0.00%> (ø)
src/pkg/cluster/tunnel.go 11.35% <0.00%> (+0.12%) ⬆️

@AustinAbro321
Copy link
Contributor

@chaospuppy sorry just getting back to this. Since this PR was made we've started requiring DCO. Please follow the instructions under the details tab to get that fixed and we'll merge this PR.

@chaospuppy
Copy link
Contributor Author

Sorry for the delay @AustinAbro321, I was out on PTO tail end of last week. It looks like you may have merged main, do you still require DCO for my commits?

@schristoff
Copy link
Contributor

@chaospuppy We still do require DCO - I merged in main to just try and get those tests to pass (and see if it was a flake). Please feel encouraged to squash and push with everything signed :)

@chaospuppy
Copy link
Contributor Author

Closing this MR in favor of #2841, which was branched from a fork of the new Zarf repo and has been signed-off on.

@chaospuppy chaospuppy closed this Aug 5, 2024
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.

zarf connect <connect-name> --local-port does not work as expected
3 participants