Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented Jan 7, 2020

This abstracts signature retrieval, setting the stage for future pivots such as fetching signatures from config maps in the local cluster.

Spun off from these suggestions for #279. I've left the file implementation in place to minimize the diff (in 0cc3e41 I'd replaced it with an in-memory sig store). I've also left both the file and HTTP stores in the verify.go file to minimize the diff (in 0cc3e41 I'd moved the HTTP store into a subpackage to simplify verify.go).

@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 7, 2020
@wking wking force-pushed the signature-fetching-interface branch from 97452c4 to 496b98f Compare January 8, 2020 00:15
@wking
Copy link
Member Author

wking commented Jan 8, 2020

e2e-aws had:

fail [github.com/openshift/origin/test/extended/prometheus/prometheus_builds.go:163]: Expected
    <map[string]error | len:1>: {
        "ALERTS{alertname!~\"Watchdog|AlertmanagerReceiversNotConfigured\",alertstate=\"firing\"} >= 1": {
            s: "promQL query: ALERTS{alertname!~\"Watchdog|AlertmanagerReceiversNotConfigured\",alertstate=\"firing\"} >= 1 had reported incorrect results:\n[{\"metric\":{\"__name__\":\"ALERTS\",\"alertname\":\"FailingOperator\",\"alertstate\":\"firing\",\"endpoint\":\"https-metrics\",\"exported_namespace\":\"e2e-test-olm-23440-mnls8\",\"instance\":\"10.130.0.17:8081\",\"job\":\"olm-operator-metrics\",\"name\":\"etcdoperator.v0.9.4\",\"namespace\":\"openshift-operator-lifecycle-manager\",\"phase\":\"Failed\",\"pod\":\"olm-operator-684dcf9457-dzgjx\",\"reason\":\"NoOperatorGroup\",\"service\":\"olm-operator-metrics\",\"severity\":\"info\",\"version\":\"0.9.4\"},\"value\":[1578446043.22,\"1\"]}]",
        },
    }
to be empty

/retest

@jottofar
Copy link
Contributor

/cc @crawford PTAL

@openshift-ci-robot
Copy link
Contributor

@jottofar: GitHub didn't allow me to request PR reviews from the following users: PTAL.

Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @crawford PTAL

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.

@wking wking force-pushed the signature-fetching-interface branch from 496b98f to 2bd263b Compare January 14, 2020 21:26
@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 10, 2020
@wking wking force-pushed the signature-fetching-interface branch from 2bd263b to 7e02430 Compare June 16, 2020 20:06
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 16, 2020
@wking wking changed the title pkg/verify: Refactor signature fetching to use a Store interface pkg/verify: Refactor signature fetching to only use a Store interface Jun 16, 2020
@wking wking force-pushed the signature-fetching-interface branch 2 times, most recently from 24d774f to 921f3ea Compare June 16, 2020 20:30
@wking
Copy link
Member Author

wking commented Jun 16, 2020

Rebased around #279 with 2bd263b -> 921f3ea, laying the groundwork for using this to address rhbz#1840343 (parallel HTTP(S) signature fetching).

@wking wking force-pushed the signature-fetching-interface branch 2 times, most recently from d1724ce to 487d860 Compare June 16, 2020 20:47
This consolidates around abstract signature retrieval, removing the
split between stores and locations which we grew in 9bbf366
(verify: Refactor the verify package to have common code, 2019-12-09, openshift#279).
This will make it easier to make future changes like parallel HTTP(S)
signature retrieval retrieval [1].

[1]: https://bugzilla.redhat.com/show_bug.cgi?id=1840343
@wking wking force-pushed the signature-fetching-interface branch from 487d860 to 6044c9c Compare June 26, 2020 03:19
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wking

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@openshift-merge-robot openshift-merge-robot merged commit 6044c9c into openshift:master Jun 29, 2020
@wking wking deleted the signature-fetching-interface branch June 29, 2020 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants