From 7da687089394db1d45fae73279dbc9205d5c7fc8 Mon Sep 17 00:00:00 2001 From: Hayden B <8418760+haydentherapper@users.noreply.github.com> Date: Thu, 17 Apr 2025 20:26:29 -0400 Subject: [PATCH] Select highest API version with multiple SigningConfig services Right now, when selecting a service or set of services, the SigningConfig will select the first match given a validity period and API version. If the client supports two API versions concurrently (e.g. Rekor v1 and Rekor v2) and the distributed signing config has entries for multiple API versions (which it will for Rekor), then the client may select an older API version depending on the sorting of the services. This change sorts the supported API versions to select the highest API version first. Fixes #453 Signed-off-by: Hayden B <8418760+haydentherapper@users.noreply.github.com> --- pkg/root/signing_config.go | 78 +++++++++++++------- pkg/root/signing_config_test.go | 123 +++++++++++++++++++++++++------- 2 files changed, 153 insertions(+), 48 deletions(-) diff --git a/pkg/root/signing_config.go b/pkg/root/signing_config.go index 495afb03..1490e145 100644 --- a/pkg/root/signing_config.go +++ b/pkg/root/signing_config.go @@ -47,15 +47,27 @@ type ServiceConfiguration struct { } // SelectService returns which service endpoint should be used based on supported API versions -// and current time. It will select the first service that matches the criteria. Services should -// be sorted from newest to oldest validity period start time, to minimize how far clients -// need to search to find a matching service. +// and current time. It will select the first service with the highest API version that matches +// the criteria. Services should be sorted from newest to oldest validity period start time, to +// minimize how far clients need to search to find a matching service. func SelectService(services []Service, supportedAPIVersions []uint32, currentTime time.Time) (string, error) { - for _, s := range services { - if slices.Contains(supportedAPIVersions, s.MajorAPIVersion) && s.ValidAtTime(currentTime) { - return s.URL, nil + if len(supportedAPIVersions) == 0 { + return "", fmt.Errorf("no supported API versions") + } + + sortedVersions := make([]uint32, len(supportedAPIVersions)) + copy(sortedVersions, supportedAPIVersions) + slices.Sort(sortedVersions) + slices.Reverse(sortedVersions) + + for _, version := range sortedVersions { + for _, s := range services { + if version == s.MajorAPIVersion && s.ValidAtTime(currentTime) { + return s.URL, nil + } } } + return "", fmt.Errorf("no matching service found for API versions %v and current time %v", supportedAPIVersions, currentTime) } @@ -63,31 +75,49 @@ func SelectService(services []Service, supportedAPIVersions []uint32, currentTim // and current time. It will use the configuration's selector to pick a set of services. // ALL will return all service endpoints, ANY will return a random endpoint, and // EXACT will return a random selection of a specified number of endpoints. +// It will select services from the highest supported API versions and will not select +// services from different API versions. func SelectServices(services []Service, config ServiceConfiguration, supportedAPIVersions []uint32, currentTime time.Time) ([]string, error) { - var urls []string + if len(supportedAPIVersions) == 0 { + return nil, fmt.Errorf("no supported API versions") + } + + urlsByVersion := make(map[uint32][]string) for _, s := range services { if slices.Contains(supportedAPIVersions, s.MajorAPIVersion) && s.ValidAtTime(currentTime) { - urls = append(urls, s.URL) + urlsByVersion[s.MajorAPIVersion] = append(urlsByVersion[s.MajorAPIVersion], s.URL) } } - if len(urls) == 0 { - return nil, fmt.Errorf("no matching services found for API versions %v and current time %v", supportedAPIVersions, currentTime) - } - switch config.Selector { - case prototrustroot.ServiceSelector_ALL: - return urls, nil - case prototrustroot.ServiceSelector_ANY: - i := rand.Intn(len(urls)) // #nosec G404 - return []string{urls[i]}, nil - case prototrustroot.ServiceSelector_EXACT: - matchedUrls, err := selectExact(urls, config.Count) - if err != nil { - return nil, err + + sortedVersions := make([]uint32, len(supportedAPIVersions)) + copy(sortedVersions, supportedAPIVersions) + slices.Sort(sortedVersions) + slices.Reverse(sortedVersions) + + // Select services from the highest supported API version + for _, version := range sortedVersions { + urls, ok := urlsByVersion[version] + if !ok { + continue + } + switch config.Selector { + case prototrustroot.ServiceSelector_ALL: + return urls, nil + case prototrustroot.ServiceSelector_ANY: + i := rand.Intn(len(urls)) // #nosec G404 + return []string{urls[i]}, nil + case prototrustroot.ServiceSelector_EXACT: + matchedUrls, err := selectExact(urls, config.Count) + if err != nil { + return nil, err + } + return matchedUrls, nil + default: + return nil, fmt.Errorf("invalid service selector") } - return matchedUrls, nil - default: - return nil, fmt.Errorf("invalid service selector") } + + return nil, fmt.Errorf("no matching services found for API versions %v and current time %v", supportedAPIVersions, currentTime) } func selectExact[T any](slice []T, count uint32) ([]T, error) { diff --git a/pkg/root/signing_config_test.go b/pkg/root/signing_config_test.go index 0c6913f5..a339d67a 100644 --- a/pkg/root/signing_config_test.go +++ b/pkg/root/signing_config_test.go @@ -86,7 +86,7 @@ func TestSelectService(t *testing.T) { tests := []struct { name string services []Service - supportedVersions uint32 + supportedVersions []uint32 currentTime time.Time expectedURL string expectedErr bool @@ -95,7 +95,7 @@ func TestSelectService(t *testing.T) { { name: "single matching service", services: services, - supportedVersions: 1, + supportedVersions: []uint32{1}, currentTime: now, expectedURL: "url1", expectedErr: false, @@ -103,7 +103,7 @@ func TestSelectService(t *testing.T) { { name: "multiple matching service, first selected", services: services, - supportedVersions: 2, + supportedVersions: []uint32{2}, currentTime: now, expectedURL: "url2", expectedErr: false, @@ -111,7 +111,7 @@ func TestSelectService(t *testing.T) { { name: "no matching version", services: services, - supportedVersions: 3, + supportedVersions: []uint32{3}, currentTime: now, expectedErr: true, expectedErrMessage: "no matching service found for API versions [3] and current time", @@ -119,7 +119,7 @@ func TestSelectService(t *testing.T) { { name: "valid with no end time", services: services, - supportedVersions: 2, + supportedVersions: []uint32{2}, currentTime: farFuture, expectedURL: "url_no_end", expectedErr: false, @@ -127,7 +127,7 @@ func TestSelectService(t *testing.T) { { name: "no matching service at all", services: []Service{}, - supportedVersions: 1, + supportedVersions: []uint32{1}, currentTime: now, expectedErr: true, expectedErrMessage: "no matching service found for API versions [1] and current time", @@ -135,16 +135,43 @@ func TestSelectService(t *testing.T) { { name: "first service selected when multiple match", services: services, - supportedVersions: 2, + supportedVersions: []uint32{2}, currentTime: now.Add(time.Minute), // In the validity period of url3_new expectedURL: "url2", expectedErr: false, }, + { + name: "match to highest API version with multiple supported versions", + services: services, + supportedVersions: []uint32{1, 2}, + currentTime: now, + expectedURL: "url2", + expectedErr: false, + }, + { + name: "match to highest API version with multiple supported versions, lower API version", + services: []Service{{URL: "url1", + MajorAPIVersion: 1, + ValidityPeriodStart: past, + ValidityPeriodEnd: future}}, + supportedVersions: []uint32{2, 1}, + currentTime: now, + expectedURL: "url1", + expectedErr: false, + }, + { + name: "no supported versions", + services: services, + supportedVersions: []uint32{}, + currentTime: now, + expectedErr: true, + expectedErrMessage: "no supported API versions", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - url, err := SelectService(tt.services, []uint32{tt.supportedVersions}, tt.currentTime) + url, err := SelectService(tt.services, tt.supportedVersions, tt.currentTime) if (err != nil) != tt.expectedErr { t.Errorf("SelectService() error = %v, expectedErr %v", err, tt.expectedErr) return @@ -190,7 +217,7 @@ func TestSelectServices(t *testing.T) { name string services []Service config ServiceConfiguration - supportedVersions uint32 + supportedVersions []uint32 currentTime time.Time expectedURLs []string possibleURLs [][]string @@ -203,7 +230,7 @@ func TestSelectServices(t *testing.T) { config: ServiceConfiguration{ Selector: prototrustroot.ServiceSelector_ALL, }, - supportedVersions: 2, + supportedVersions: []uint32{2}, currentTime: now, expectedURLs: []string{"url2", "url3"}, expectedErr: false, @@ -214,7 +241,7 @@ func TestSelectServices(t *testing.T) { config: ServiceConfiguration{ Selector: prototrustroot.ServiceSelector_ALL, }, - supportedVersions: 1, + supportedVersions: []uint32{1}, currentTime: now, expectedURLs: []string{"url1"}, expectedErr: false, @@ -225,7 +252,7 @@ func TestSelectServices(t *testing.T) { config: ServiceConfiguration{ Selector: prototrustroot.ServiceSelector_ALL, }, - supportedVersions: 3, + supportedVersions: []uint32{3}, currentTime: now, expectedErr: true, expectedErrMessage: "no matching services found for API versions [3] and current time", @@ -236,7 +263,7 @@ func TestSelectServices(t *testing.T) { config: ServiceConfiguration{ Selector: prototrustroot.ServiceSelector_ANY, }, - supportedVersions: 2, + supportedVersions: []uint32{2}, currentTime: now, possibleURLs: [][]string{{"url2"}, {"url3"}}, expectedErr: false, @@ -247,7 +274,7 @@ func TestSelectServices(t *testing.T) { config: ServiceConfiguration{ Selector: prototrustroot.ServiceSelector_ANY, }, - supportedVersions: 1, + supportedVersions: []uint32{1}, currentTime: now, expectedURLs: []string{"url1"}, expectedErr: false, @@ -258,7 +285,7 @@ func TestSelectServices(t *testing.T) { config: ServiceConfiguration{ Selector: prototrustroot.ServiceSelector_ANY, }, - supportedVersions: 3, + supportedVersions: []uint32{3}, currentTime: now, expectedErr: true, expectedErrMessage: "no matching services found for API versions [3] and current time", @@ -270,7 +297,7 @@ func TestSelectServices(t *testing.T) { Selector: prototrustroot.ServiceSelector_EXACT, Count: 1, }, - supportedVersions: 2, + supportedVersions: []uint32{2}, currentTime: now, possibleURLs: [][]string{{"url2"}, {"url3"}}, expectedErr: false, @@ -282,7 +309,7 @@ func TestSelectServices(t *testing.T) { Selector: prototrustroot.ServiceSelector_EXACT, Count: 2, }, - supportedVersions: 2, + supportedVersions: []uint32{2}, currentTime: now, possibleURLs: [][]string{{"url2", "url3"}, {"url3", "url2"}}, expectedErr: false, @@ -294,7 +321,7 @@ func TestSelectServices(t *testing.T) { Selector: prototrustroot.ServiceSelector_EXACT, Count: 1, }, - supportedVersions: 1, + supportedVersions: []uint32{1}, currentTime: now, expectedURLs: []string{"url1"}, expectedErr: false, @@ -306,7 +333,7 @@ func TestSelectServices(t *testing.T) { Selector: prototrustroot.ServiceSelector_EXACT, Count: 0, }, - supportedVersions: 2, + supportedVersions: []uint32{2}, currentTime: now, expectedErr: true, expectedErrMessage: "service selector count must be greater than 0", @@ -318,7 +345,19 @@ func TestSelectServices(t *testing.T) { Selector: prototrustroot.ServiceSelector_EXACT, Count: 3, }, - supportedVersions: 2, + supportedVersions: []uint32{2}, + currentTime: now, + expectedErr: true, + expectedErrMessage: "service selector count 3 must be less than or equal to the slice length 2", + }, + { + name: "EXACT selector, count greater than matches, multiple supported versions", + services: services, + config: ServiceConfiguration{ + Selector: prototrustroot.ServiceSelector_EXACT, + Count: 3, + }, + supportedVersions: []uint32{1, 2}, currentTime: now, expectedErr: true, expectedErrMessage: "service selector count 3 must be less than or equal to the slice length 2", @@ -330,7 +369,7 @@ func TestSelectServices(t *testing.T) { Selector: prototrustroot.ServiceSelector_EXACT, Count: 100, }, - supportedVersions: 2, + supportedVersions: []uint32{2}, currentTime: now, expectedErr: true, expectedErrMessage: "service selector count 100 must be less than or equal to the slice length 2", @@ -342,7 +381,7 @@ func TestSelectServices(t *testing.T) { Selector: prototrustroot.ServiceSelector_EXACT, Count: 1, }, - supportedVersions: 3, + supportedVersions: []uint32{3}, currentTime: now, expectedErr: true, expectedErrMessage: "no matching services found for API versions [3] and current time", @@ -353,16 +392,52 @@ func TestSelectServices(t *testing.T) { config: ServiceConfiguration{ Selector: 99, // Invalid }, - supportedVersions: 2, + supportedVersions: []uint32{2}, currentTime: now, expectedErr: true, expectedErrMessage: "invalid service selector", }, + { + name: "match to highest API version with multiple supported versions", + services: services, + config: ServiceConfiguration{ + Selector: prototrustroot.ServiceSelector_ALL, + }, + supportedVersions: []uint32{1, 2}, + currentTime: now, + expectedURLs: []string{"url2", "url3"}, + expectedErr: false, + }, + { + name: "match to highest API version with multiple supported versions, lower API version", + services: []Service{{URL: "url1", + MajorAPIVersion: 1, + ValidityPeriodStart: past, + ValidityPeriodEnd: future}}, + config: ServiceConfiguration{ + Selector: prototrustroot.ServiceSelector_ALL, + }, + supportedVersions: []uint32{2, 1}, + currentTime: now, + expectedURLs: []string{"url1"}, + expectedErr: false, + }, + { + name: "no supported versions", + services: services, + config: ServiceConfiguration{ + Selector: prototrustroot.ServiceSelector_ALL, + }, + supportedVersions: []uint32{}, + currentTime: now, + expectedErr: true, + expectedErrMessage: "no supported API versions", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - urls, err := SelectServices(tt.services, tt.config, []uint32{tt.supportedVersions}, tt.currentTime) + urls, err := SelectServices(tt.services, tt.config, tt.supportedVersions, tt.currentTime) if (err != nil) != tt.expectedErr { t.Errorf("SelectServices() error = %v, expectedErr %v", err, tt.expectedErr) return