Skip to content

Refactor the verify package to not have dependencies on CVO #279

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.

Sign up for GitHub

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

Merged
openshift-merge-robot merged 6 commits into from
Jan 17, 2020
Merged

Refactor the verify package to not have dependencies on CVO #279

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9bbf366
verify: Refactor the verify package to have common code
smarterclayton Dec 10, 2019
527b126
verify: Cache up to 64 recent successful validations
smarterclayton Dec 10, 2019
a82639c
vendor: Add fake clientset for testing
smarterclayton Dec 11, 2019
abd6802
verify: Allow the release verifier to load signatures abstractly
smarterclayton Dec 11, 2019
2ba18d8
verify: Periodically write valid signatures to an on cluster config map
smarterclayton Dec 11, 2019
1d13e88
DO NOT MERGE - Review comments, will be squashed
smarterclayton Dec 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 43 additions & 1 deletion Gopkg.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 30 additions & 1 deletion pkg/cvo/cvo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ import (

"github.com/blang/semver"
"github.com/google/uuid"
"github.com/pkg/errors"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait"
informerscorev1 "k8s.io/client-go/informers/core/v1"
Expand Down Expand Up @@ -246,7 +248,7 @@ func (optr *Operator) InitializeFromPayload(restConfig *rest.Config, burstRestCo
clientBuilder := &verifyClientBuilder{builder: optr.HTTPClient}

// attempt to load a verifier as defined in the payload
verifier, err := verify.LoadFromPayload(update, clientBuilder)
verifier, err := loadConfigMapVerifierDataFromUpdate(update, clientBuilder)
if err != nil {
return err
}
Expand Down Expand Up @@ -276,6 +278,33 @@ func (optr *Operator) InitializeFromPayload(restConfig *rest.Config, burstRestCo
return nil
}

// loadConfigMapVerifierDataFromUpdate fetches the first config map in the payload with the correct annotation.
// It returns an error if the data is not valid, or no verifier if no config map is found. See the verify
// package for more details on the algorithm for verification. If the annotation is set, a verifier or error
// is always returned.
func loadConfigMapVerifierDataFromUpdate(update *payload.Update, clientBuilder verify.ClientBuilder) (verify.Interface, error) {
configMapGVK := corev1.SchemeGroupVersion.WithKind("ConfigMap")
for _, manifest := range update.Manifests {
if manifest.GVK != configMapGVK {
continue
}
if _, ok := manifest.Obj.GetAnnotations()[verify.ReleaseAnnotationConfigMapVerifier]; !ok {
continue
}
src := fmt.Sprintf("the config map %s/%s", manifest.Obj.GetNamespace(), manifest.Obj.GetName())
data, _, err := unstructured.NestedStringMap(manifest.Obj.Object, "data")
if err != nil {
return nil, errors.Wrapf(err, "%s is not valid: %v", src, err)
}
verifier, err := verify.NewFromConfigMap(src, data, clientBuilder)
if err != nil {
return nil, err
}
return verifier, nil
}
return nil, nil
}

// Run runs the cluster version operator until stopCh is completed. Workers is ignored for now.
func (optr *Operator) Run(ctx context.Context, workers int) {
defer utilruntime.HandleCrash()
Expand Down
192 changes: 192 additions & 0 deletions pkg/cvo/cvo_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ package cvo

import (
"fmt"
"io/ioutil"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"reflect"
"strconv"
"testing"
Expand All @@ -17,6 +19,7 @@ import (
apiextclientv1 "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand All @@ -32,7 +35,10 @@ import (
configv1 "github.com/openshift/api/config/v1"
clientset "github.com/openshift/client-go/config/clientset/versioned"
"github.com/openshift/client-go/config/clientset/versioned/fake"

"github.com/openshift/cluster-version-operator/lib"
"github.com/openshift/cluster-version-operator/pkg/payload"
"github.com/openshift/cluster-version-operator/pkg/verify"
)

var (
Expand Down Expand Up @@ -3245,3 +3251,189 @@ func fakeClientsetWithUpdates(obj *configv1.ClusterVersion) *fake.Clientset {
})
return client
}

func Test_loadReleaseVerifierFromConfigMap(t *testing.T) {
redhatData, err := ioutil.ReadFile(filepath.Join("..", "verify", "testdata", "keyrings", "redhat.txt"))
if err != nil {
t.Fatal(err)
}

tests := []struct {
name string
update *payload.Update
want bool
wantErr bool

@wking wking Dec 11, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Neither want nor wantErr are very specific about what came back. Can we make wantErr a regexp or string matching the expected error message? And want something about the set of loaded keys?

wantVerifiers int
}{
{
name: "is a no-op when no objects are found",
update: &payload.Update{},
},
{
name: "requires data",
update: &payload.Update{
Manifests: []lib.Manifest{
{
GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
Obj: &unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"name": "release-verification",
"namespace": "openshift-config-managed",
"annotations": map[string]interface{}{
"release.openshift.io/verification-config-map": "",
},
},
},
},
},
},
},
wantErr: true,
},
{
name: "requires stores",
update: &payload.Update{
Manifests: []lib.Manifest{
{
GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
Obj: &unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"name": "verification",
"namespace": "openshift-config",
"annotations": map[string]interface{}{
"release.openshift.io/verification-config-map": "",
},
},
"data": map[string]interface{}{
"verifier-public-key-redhat": string(redhatData),
},
},
},
},
},
},
wantErr: true,
},
{
name: "requires verifiers",
update: &payload.Update{
Manifests: []lib.Manifest{
{
GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
Obj: &unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"name": "release-verification",
"namespace": "openshift-config-managed",
"annotations": map[string]interface{}{
"release.openshift.io/verification-config-map": "",
},
},
"data": map[string]interface{}{
"store-local": "file://../verify/testdata/signatures",
},
},
},
},
},
},
wantErr: true,
},
{
name: "loads valid configuration",
update: &payload.Update{
Manifests: []lib.Manifest{
{
GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
Obj: &unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"name": "release-verification",
"namespace": "openshift-config-managed",
"annotations": map[string]interface{}{
"release.openshift.io/verification-config-map": "",
},
},
"data": map[string]interface{}{
"verifier-public-key-redhat": string(redhatData),
"store-local": "file://../verify/testdata/signatures",
},
},
},
},
},
},
want: true,
wantVerifiers: 1,
},
{
name: "only the first valid configuration is used",
update: &payload.Update{
Manifests: []lib.Manifest{
{
GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
Obj: &unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"name": "release-verification",
"namespace": "openshift-config-managed",
"annotations": map[string]interface{}{
"release.openshift.io/verification-config-map": "",
},
},
"data": map[string]interface{}{
"verifier-public-key-redhat": string(redhatData),
"store-local": "\nfile://../verify/testdata/signatures\n",
},
},
},
},
{
GVK: schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"},
Obj: &unstructured.Unstructured{
Object: map[string]interface{}{
"metadata": map[string]interface{}{
"name": "release-verificatio-2n",
"namespace": "openshift-config-managed",
"annotations": map[string]interface{}{
"release.openshift.io/verification-config-map": "",
},
},
"data": map[string]interface{}{
"verifier-public-key-redhat": string(redhatData),
"verifier-public-key-redhat-2": string(redhatData),
"store-local": "file://../verify/testdata/signatures",
},
},
},
},
},
},
want: true,
wantVerifiers: 1,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := loadConfigMapVerifierDataFromUpdate(tt.update, verify.DefaultClient)
if (err != nil) != tt.wantErr {
t.Fatalf("loadReleaseVerifierFromPayload() error = %v, wantErr %v", err, tt.wantErr)
}
if (got != nil) != tt.want {
t.Fatal(got)
}
if err != nil {
return
}
if got == nil {
return
}
rv := got.(*verify.ReleaseVerifier)
if len(rv.Verifiers()) != tt.wantVerifiers {
t.Fatalf("unexpected release verifier: %#v", rv)
}
})
}
}
Loading