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: set tls config while retaining other fields from default http transport #4007

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

nianyush
Copy link
Contributor

@nianyush nianyush commented Jan 12, 2025

Signed-off-by: Nianyu Shen xiaoyu9964@gmail.com

Summary

original issue #3269
Proxy from environment variable is not respected while setting insecure true.
At

opts = append(opts, remote.WithTransport(&http.Transport{TLSClientConfig: tlsConfig}))
, its initializing an empty transport with only insecure set but those default behaviors from default transport is also gone. In this case, proxy value from envs is removed. The fix is to clone a default transport object and override the tlsconfig inside it.

Release Note

Documentation

…ansport

Signed-off-by: Nianyu Shen <xiaoyu9964@gmail.com>
Copy link
Contributor

@haydentherapper haydentherapper left a comment

Choose a reason for hiding this comment

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

Thank you!

@haydentherapper haydentherapper enabled auto-merge (squash) January 13, 2025 03:31
Copy link

codecov bot commented Jan 13, 2025

Codecov Report

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

Project coverage is 36.50%. Comparing base (2ef6022) to head (aa61fea).
Report is 276 commits behind head on main.

Files with missing lines Patch % Lines
cmd/cosign/cli/options/registry.go 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4007      +/-   ##
==========================================
- Coverage   40.10%   36.50%   -3.60%     
==========================================
  Files         155      209      +54     
  Lines       10044    13361    +3317     
==========================================
+ Hits         4028     4878     +850     
- Misses       5530     7862    +2332     
- Partials      486      621     +135     

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

@haydentherapper haydentherapper merged commit d01988e into sigstore:main Jan 13, 2025
22 of 23 checks passed
@nianyush nianyush deleted the 3269-nianyu branch January 24, 2025 16:28
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