Create reusable package to store and verify signatures for use by cvo…

[jottofar]

… and oc

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jottofar
To complete the pull request process, please assign deads2k
You can assign the PR to them by writing /assign @deads2k in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci-robot]

@jottofar: The following tests failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/prow/unit	fc4a27e	link	/test unit
ci/prow/verify	fc4a27e	link	/test verify
ci/prow/verify-deps	fc4a27e	link	/test verify-deps

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[wking]

Comparing with the CVO's tip:

$ git --no-pager -C cluster-version-operator log -1 --oneline
268d89b (HEAD -> master, origin/release-4.5, origin/release-4.4, origin/master, origin/HEAD) Merge pull request #306 from smarterclayton/fix_panic
$ git --no-pager -C library-go log -1 --oneline
fc4a27e5a (HEAD, origin/pr/671) Create reusable package to store and verify signatures for use by cvo and oc
$ diff -ru cluster-version-operator/pkg/verify library-go/pkg/verify
Only in library-go/pkg/verify: manifest.go
Only in library-go/pkg/verify: .manifest.go.un~
Only in library-go/pkg/verify: manifest_test.go
Only in library-go/pkg/verify: .manifest_test.go.un~
Only in library-go/pkg/verify: signature.go
Only in library-go/pkg/verify: .signature.go.un~
diff -ru cluster-version-operator/pkg/verify/verifyconfigmap/store.go library-go/pkg/verify/verifyconfigmap/store.go
--- cluster-version-operator/pkg/verify/verifyconfigmap/store.go	2020-01-21 12:29:56.761124609 -0800
+++ library-go/pkg/verify/verifyconfigmap/store.go	2020-01-21 12:29:36.817899331 -0800
@@ -41,7 +41,6 @@
 	return &Store{
 		client: client,
 		ns:     "openshift-config-managed",
-		limiter: limiter,
 	}
 }
 
diff -ru cluster-version-operator/pkg/verify/verify.go library-go/pkg/verify/verify.go
--- cluster-version-operator/pkg/verify/verify.go	2020-01-21 12:29:56.760124598 -0800
+++ library-go/pkg/verify/verify.go	2020-01-21 12:29:36.817899331 -0800
@@ -106,10 +106,8 @@
 // WithStores copies the provided verifier and adds any provided stores to the list.
 func (v *ReleaseVerifier) WithStores(stores ...SignatureStore) *ReleaseVerifier {
 	return &ReleaseVerifier{
-		verifiers:     v.verifiers,
-		locations:     v.locations,
-		clientBuilder: v.clientBuilder,
-
+		verifiers:      v.verifiers,
+		locations:      v.locations,
 		stores:         append(append(make([]SignatureStore, 0, len(v.stores)+len(stores)), v.stores...), stores...),
 		signatureCache: v.Signatures(),
 	}

Things like .manifest.go.un~ look like local editor files that you don't want to actually commit. And looks like you need to pick up openshift/cluster-version-operator#303 and openshift/cluster-version-operator#306. Can you talk more about manifest.go and signature.go, which don't seem to be in the CVO?

[jottofar]

Things like .manifest.go.un~ look like local editor files that you don't want to actually commit.

My mistake. Yes, I need to remove these backup related files.

And looks like you need to pick up openshift/cluster-version-operator#303 and openshift/cluster-version-operator#306.

Yep, will do.

Can you talk more about manifest.go and signature.go, which don't seem to be in the CVO?

Again, I'm trying to keep this as simple as possible but of course don't want to do anything that just doesn't make sense or causes undue heartache down the road.

signature.go:

The new method created in cvo.go by PR 279 and currently called is loadConfigMapVerifierDataFromUpdate. I created this new file to contain that top-level method, essentially the api method. At the highest level, and from a pseudo OO view, seems to me this new library-go blob were creating is a 'signature' but not cleanly of course. That's why I proposed calling the top-level package signature rather than verify.

manifest.go:

loadConfigMapVerifierDataFromUpdate in cvo takes in *payload.Update but only uses 'Manifests []lib.Manifest' from it so I changed the input paramater in my signatures.LoadConfigMapVerifierDataFromUpdate to just take in []Manifest directly. Manifest is currently defined in cluster-version-operator/lib so I moved the definition into here. If cvo is refactored to use this stuff it would pickup Manifest from here as well.

[sttts]

what are those .un~ files above?

[jottofar]

My mistake. Yes, I need to remove these backup related files. I'm putting this on hold until I have other significant changes pushed as well. Thanks for reviewing.

[jottofar]

/hold