Skip to content

Commit

Permalink
Fix for multiple WithRemote options
Browse files Browse the repository at this point in the history
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
Signed-off-by: Warren Hodgkinson <[email protected]>
  • Loading branch information
warrenhodg committed Dec 19, 2024
1 parent fa805ab commit 6c0871d
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 1 deletion.
8 changes: 8 additions & 0 deletions pkg/oci/remote/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,14 @@ func WithRemoteOptions(opts ...remote.Option) Option {
}
}

// WithMoreRemoteOptions is a functional option for adding to the default
// remote options already specified
func WithMoreRemoteOptions(opts ...remote.Option) Option {
return func(o *options) {
o.ROpt = append(o.ROpt, opts...)
}
}

// WithTargetRepository is a functional option for overriding the default
// target repository hosting the signature and attestation tags.
func WithTargetRepository(repo name.Repository) Option {
Expand Down
68 changes: 67 additions & 1 deletion pkg/oci/remote/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package remote

import (
"context"
"errors"
"os"
"reflect"
Expand All @@ -42,6 +43,10 @@ func TestOptions(t *testing.T) {
// TODO(mattmoor): Incorporate user agent.
}

moreROpt := []remote.Option{
remote.WithContext(context.Background()),
}

tests := []struct {
name string
opts []Option
Expand Down Expand Up @@ -105,14 +110,24 @@ func TestOptions(t *testing.T) {
TargetRepository: repo,
ROpt: otherROpt,
},
}, {
name: "more remote options option",
opts: []Option{WithRemoteOptions(otherROpt...), WithMoreRemoteOptions(moreROpt...)},
want: &options{
SignatureSuffix: SignatureTagSuffix,
AttestationSuffix: AttestationTagSuffix,
SBOMSuffix: SBOMTagSuffix,
TargetRepository: repo,
ROpt: append(append([]remote.Option{}, otherROpt...), moreROpt...),
},
}}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
got := makeOptions(repo, test.opts...)
test.want.OriginalOptions = test.opts

if !reflect.DeepEqual(got, test.want) {
if !optionsEqual(got, test.want) {
t.Errorf("makeOptions() = %#v, wanted %#v", got, test.want)
}
})
Expand Down Expand Up @@ -170,3 +185,54 @@ func TestGetEnvTargetRepository(t *testing.T) {
})
}
}

// this is required due to the fact that reflect.DeepEquals reports
// two different slices of function points, with identical length and
// contents at each position as being different
func optionsEqual(o1, o2 *options) bool {
if (o1 == nil) != (o2 == nil) {
return false
}
if o1 == nil {
return true
}

if o1.AttestationSuffix != o2.AttestationSuffix {
return false
}
if o1.SignatureSuffix != o2.SignatureSuffix {
return false
}
if o1.SBOMSuffix != o2.SBOMSuffix {
return false
}
if o1.TagPrefix != o2.TagPrefix {
return false
}
if !slicesEqual(o1.ROpt, o2.ROpt) {
return false
}
if !slicesEqual(o1.NameOpts, o2.NameOpts) {
return false
}
if !slicesEqual(o1.OriginalOptions, o2.OriginalOptions) {
return false
}
return true
}

func slicesEqual[T any](o1, o2 []T) bool {
if len(o1) != len(o2) {
return false
}

for i := range o1 {
v1 := reflect.ValueOf(o1[i])
v2 := reflect.ValueOf(o2[i])
if v1 != v2 {
return false
}
}

return true
}

0 comments on commit 6c0871d

Please sign in to comment.