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 for multiple WithRemote options #3982

Merged
merged 1 commit into from
Dec 19, 2024

Conversation

warrenhodg
Copy link
Contributor

Summary

When specifying multiple sets of remote options using WithRemoteOptions, only the last set of options are used. The earlier ones are discarded. This is a problem for example, when an initial set of WithRemoteOptions are generated by command-line flags, and then additional ones need to be specified during application execution. Doing so would discard the options set by the command-line flags.

To preserve backwards compatibility, where the WithRemoteOptions function overrides the defaults, an additional function WithMoreRemoteOptions has been added, which appends the options to any existing ones.

Resolves: sigstore/cosign/#3981

Release Note

Function WithMoreRemoteOptions has been added that allows one to append additional remote options to those
already specified.

Documentation

None

When specifying multiple sets of remote options using
WithRemoteOptions, only the last set of options are used.
The earlier ones are discarded. This is a problem for example, when
an initial set of WithRemoteOptions are generated by command-line
flags, and then additional ones need to be specified during
application execution. Doing so would discard the options set
by the command-line flags.

To preserve backwards compatibility, where the WithRemoteOptions
function overrides the defaults, an additional function
WithMoreRemoteOptions has been added, which appends the options
to any existing ones.

Resolves: sigstore/cosign/sigstore#3981
Signed-off-by: Warren Hodgkinson <[email protected]>
Copy link

codecov bot commented Dec 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 36.48%. Comparing base (2ef6022) to head (6c0871d).
Report is 260 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3982      +/-   ##
==========================================
- Coverage   40.10%   36.48%   -3.63%     
==========================================
  Files         155      209      +54     
  Lines       10044    13316    +3272     
==========================================
+ Hits         4028     4858     +830     
- Misses       5530     7839    +2309     
- Partials      486      619     +133     

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

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 merged commit ce5acdf into sigstore:main Dec 19, 2024
23 checks passed
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