Skip to content

Commit d41d698

Browse files
committed
pkg/customsignaturestore: Implement signatureStores customization
The verifier Interface's AddStore has always been [1]: // AddStore adds additional stores for signature verification. func (v *releaseVerifier) AddStore(additionalStore store.Store) { v.store = &serial.Store{Stores: []store.Store{additionalStore, v.store}} } since it landed in 2020. We discussed then [2] whether we were comfortable with "always insert the new addition serially in front of the existing stores", and decided that it was good enough then (although we neglected to commit to that implementation in the Interface's Go docs). That history sets up my approach in this commit where I add support for ClusterVersion's new spec.signatureStores. The logic is: 1. Try and find the signature in the local ConfigMaps. * If we error, e.g. by failing to list ConfigMaps, fail out of the ConfigMap store [3], which will fail out of the serial store [4], which will fail verification [5] without further signature traversal. * If we find a valid signature, hooray, we're done [6]. * If we run out of ConfigMap entries to check without finding a valid signature, fall back to... 2. ClusterVersion's spec.signatureStores. * If this property is set to an empty array, fail out. Admins can configure this if they only want to support ConfigMap lookup. * If this property is set to a non-empty array, build a parallel store out of its current contents, and search the new store: * If we error, check in with the call-back function about whether the error should be fatal, e.g. [7]. But the verifier callback is very generous about allowing failures [5], so we'll keep going looking at any later signatures in that sigstore and at the other parallel sigstores. * If we find a valid signature, hooray, we're done [8,9]. * If we run out of sigstore entries to check without finding a valid signature, return an error, without wrapping with the callback, because we want to fail out [4,5] without further signature traversal. Admins who want to check default signature stores as well can configure them explicitly in signatureStores. * If this property is unset, fall back to... 3. The default signature stores [10]. * Same handling as the parallel store in spec.signatureStores, but with the default signature stores. We only fall through to the default stores when ConfigMap lookup is exhausted without finding a valid signature and signatureStores is unset, to support admins who would rather not have the cluster-version operator reach out to one or more of the default stores, whether or not they have local stores they would rather use. This allows admins to construct the set of stores they would like to use, while keeping the API surface down at a single property. Without the AddStore history, it would have been possible to construct simplified stores, like a ConfigMaps store alone in the case where signatureStores is an empty array. We could still pivot to that kind of approach with library-go changes, but for the initial implementation, I'm doing the above dance to work with AddStore as it stands. [1]: https://github.com/openshift/library-go/blame/08d73a9c798b5698d88efd52232127e443bef9b6/pkg/verify/verify.go#L247-L250 [2]: openshift/library-go#910 (comment) [3]: https://github.com/openshift/library-go/blob/08d73a9c798b5698d88efd52232127e443bef9b6/pkg/verify/store/configmap/configmap.go#L100 It's possible we want to wrap these failures in the callback, because the verifier callback is currently very generous about allowing failures and proceeding with subsequent retrieval attempts [5]. For example, it's possible that local ConfigMap listing fails while subsequent sigstore retrieval would succeed, and today the lack of callback-wrapping on that error means we are not making the subsequent sigstore requests. But in practice, I expect local ConfigMap listing fails so rarely that folks are not bothered by the current behavior. [4]: https://github.com/openshift/library-go/blob/08d73a9c798b5698d88efd52232127e443bef9b6/pkg/verify/store/serial/serial.go#L32-L38 [5]: https://github.com/openshift/library-go/blob/08d73a9c798b5698d88efd52232127e443bef9b6/pkg/verify/verify.go#L189-L215 [6]: https://github.com/openshift/library-go/blob/08d73a9c798b5698d88efd52232127e443bef9b6/pkg/verify/store/serial/serial.go#L24-L34 [7]: https://github.com/openshift/library-go/blob/08d73a9c798b5698d88efd52232127e443bef9b6/pkg/verify/store/sigstore/sigstore.go#L119-L121 [8]: https://github.com/openshift/library-go/blob/08d73a9c798b5698d88efd52232127e443bef9b6/pkg/verify/store/sigstore/sigstore.go#L134-L136 [9]: https://github.com/openshift/library-go/blob/08d73a9c798b5698d88efd52232127e443bef9b6/pkg/verify/store/parallel/parallel.go#L61-L66 [10]: https://github.com/openshift/cluster-update-keys/blob/804dfeceb7caa41a6212fb405bbcd3f3139b8a6f/manifests.rhel/0000_90_cluster-update-keys_configmap.yaml#L4-L5
1 parent 9bda820 commit d41d698

File tree

3 files changed

+131
-3
lines changed

3 files changed

+131
-3
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
// Package customsignaturestore implements a signature store as configured by ClusterVersion.
2+
package customsignaturestore
3+
4+
import (
5+
"context"
6+
"errors"
7+
"fmt"
8+
"net/url"
9+
"strings"
10+
"sync"
11+
12+
configv1listers "github.com/openshift/client-go/config/listers/config/v1"
13+
"github.com/openshift/library-go/pkg/verify/store"
14+
"github.com/openshift/library-go/pkg/verify/store/parallel"
15+
"github.com/openshift/library-go/pkg/verify/store/sigstore"
16+
)
17+
18+
type Store struct {
19+
// Name is the name of the ClusterVersion object that configures this store.
20+
Name string
21+
22+
// Lister allows the store to fetch the current ClusterVersion configuration.
23+
Lister configv1listers.ClusterVersionLister
24+
25+
// HTTPClient is called once for each Signatures call to ensure
26+
// requests are made with the currently-recommended parameters.
27+
HTTPClient sigstore.HTTPClient
28+
29+
// lock allows the store to be locked while mutating or accessing internal state.
30+
lock sync.Mutex
31+
32+
// customURIs tracks the most-recently retrieved ClusterVersion configuration.
33+
customURIs []*url.URL
34+
}
35+
36+
// Signatures fetches signatures for the provided digest.
37+
func (s *Store) Signatures(ctx context.Context, name string, digest string, fn store.Callback) error {
38+
uris, err := s.refreshConfiguration(ctx)
39+
if err != nil {
40+
return err
41+
}
42+
43+
if uris == nil {
44+
return nil
45+
}
46+
47+
if len(uris) == 0 {
48+
return errors.New("ClusterVersion spec.signatureStores is an empty array. Unset signatureStores entirely if you want to to enable the default signature stores.")
49+
}
50+
51+
allDone := false
52+
53+
wrapper := func(ctx context.Context, signature []byte, errIn error) (done bool, err error) {
54+
done, err = fn(ctx, signature, errIn)
55+
if done {
56+
allDone = true
57+
}
58+
return done, err
59+
}
60+
61+
stores := make([]store.Store, 0, len(uris))
62+
for i := range uris {
63+
uri := *uris[i]
64+
stores = append(stores, &sigstore.Store{
65+
URI: &uri,
66+
HTTPClient: s.HTTPClient,
67+
})
68+
}
69+
store := &parallel.Store{Stores: stores}
70+
if err := store.Signatures(ctx, name, digest, wrapper); err != nil || allDone {
71+
return err
72+
}
73+
return errors.New("ClusterVersion spec.signatureStores exhausted without finding a valid signature.")
74+
}
75+
76+
func (s *Store) refreshConfiguration(ctx context.Context) ([]*url.URL, error) {
77+
var uris []*url.URL
78+
config, err := s.Lister.Get(s.Name)
79+
if err != nil {
80+
return uris, err
81+
}
82+
83+
if config.Spec.SignatureStores != nil {
84+
uris = make([]*url.URL, 0, len(config.Spec.SignatureStores))
85+
for _, store := range config.Spec.SignatureStores {
86+
uri, err := url.Parse(store.URL)
87+
if err != nil {
88+
return uris, err
89+
}
90+
91+
uris = append(uris, uri)
92+
}
93+
}
94+
95+
s.lock.Lock()
96+
defer s.lock.Unlock()
97+
s.customURIs = uris
98+
return uris, err
99+
}
100+
101+
// String returns a description of where this store finds
102+
// signatures.
103+
func (s *Store) String() string {
104+
s.lock.Lock()
105+
defer s.lock.Unlock()
106+
107+
if s.customURIs == nil {
108+
return "ClusterVersion signatureStores unset, falling back to default stores"
109+
} else if len(s.customURIs) == 0 {
110+
return "0 ClusterVersion signatureStores"
111+
}
112+
uris := make([]string, 0, len(s.customURIs))
113+
for _, uri := range s.customURIs {
114+
uris = append(uris, uri.String())
115+
}
116+
return fmt.Sprintf("ClusterVersion signatureStores: %s", strings.Join(uris, ", "))
117+
}

pkg/cvo/cvo.go

+12-2
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
configlistersv1 "github.com/openshift/client-go/config/listers/config/v1"
3434
"github.com/openshift/library-go/pkg/manifest"
3535
"github.com/openshift/library-go/pkg/verify"
36+
"github.com/openshift/library-go/pkg/verify/store"
3637
"github.com/openshift/library-go/pkg/verify/store/configmap"
3738
"github.com/openshift/library-go/pkg/verify/store/sigstore"
3839

@@ -41,6 +42,7 @@ import (
4142
"github.com/openshift/cluster-version-operator/lib/validation"
4243
"github.com/openshift/cluster-version-operator/pkg/clusterconditions"
4344
"github.com/openshift/cluster-version-operator/pkg/clusterconditions/standard"
45+
"github.com/openshift/cluster-version-operator/pkg/customsignaturestore"
4446
cvointernal "github.com/openshift/cluster-version-operator/pkg/cvo/internal"
4547
"github.com/openshift/cluster-version-operator/pkg/cvo/internal/dynamicclient"
4648
"github.com/openshift/cluster-version-operator/pkg/internal"
@@ -292,8 +294,14 @@ func (optr *Operator) InitializeFromPayload(ctx context.Context, restConfig *res
292294
return fmt.Errorf("unable to create a configuration client: %v", err)
293295
}
294296

297+
customSignatureStore := &customsignaturestore.Store{
298+
Lister: optr.cvLister,
299+
Name: optr.name,
300+
HTTPClient: httpClientConstructor.HTTPClient,
301+
}
302+
295303
// attempt to load a verifier as defined in the payload
296-
verifier, signatureStore, err := loadConfigMapVerifierDataFromUpdate(update, httpClientConstructor.HTTPClient, configClient)
304+
verifier, signatureStore, err := loadConfigMapVerifierDataFromUpdate(update, httpClientConstructor.HTTPClient, configClient, customSignatureStore)
297305
if err != nil {
298306
return err
299307
}
@@ -360,7 +368,7 @@ func (optr *Operator) ownerReferenceModifier(object metav1.Object) {
360368
// It returns an error if the data is not valid, or no verifier if no config map is found. See the verify
361369
// package for more details on the algorithm for verification. If the annotation is set, a verifier or error
362370
// is always returned.
363-
func loadConfigMapVerifierDataFromUpdate(update *payload.Update, clientBuilder sigstore.HTTPClient, configMapClient coreclientsetv1.ConfigMapsGetter) (verify.Interface, *verify.StorePersister, error) {
371+
func loadConfigMapVerifierDataFromUpdate(update *payload.Update, clientBuilder sigstore.HTTPClient, configMapClient coreclientsetv1.ConfigMapsGetter, customSignatureStore store.Store) (verify.Interface, *verify.StorePersister, error) {
364372
verifier, err := verify.NewFromManifests(update.Manifests, clientBuilder)
365373
if err != nil {
366374
return nil, nil, err
@@ -369,6 +377,8 @@ func loadConfigMapVerifierDataFromUpdate(update *payload.Update, clientBuilder s
369377
return nil, nil, nil
370378
}
371379

380+
verifier.AddStore(customSignatureStore)
381+
372382
// allow the verifier to consult the cluster for signature data, and also configure
373383
// a process that writes signatures back to that store
374384
signatureStore := configmap.NewStore(configMapClient, nil)

pkg/cvo/cvo_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import (
4242

4343
"github.com/openshift/cluster-version-operator/pkg/payload"
4444
"github.com/openshift/library-go/pkg/manifest"
45+
"github.com/openshift/library-go/pkg/verify/store/serial"
4546
"github.com/openshift/library-go/pkg/verify/store/sigstore"
4647
)
4748

@@ -4203,7 +4204,7 @@ func Test_loadReleaseVerifierFromConfigMap(t *testing.T) {
42034204
}
42044205
t.Run(tt.name, func(t *testing.T) {
42054206
f := kfake.NewSimpleClientset()
4206-
got, store, err := loadConfigMapVerifierDataFromUpdate(tt.update, sigstore.DefaultClient, f.CoreV1())
4207+
got, store, err := loadConfigMapVerifierDataFromUpdate(tt.update, sigstore.DefaultClient, f.CoreV1(), &serial.Store{})
42074208
if err == nil {
42084209
if tt.expectedError != "" {
42094210
t.Fatalf("loadConfigMapVerifierDataFromUpdate succeeded when we expected error \"%s\"", tt.expectedError)

0 commit comments

Comments
 (0)