[release-4.3] Bug 1809253: Reload serving certs #160

marun · 2020-03-03T01:31:43Z

I attempted updating the 4.2 and 4.3 branches to kube 1.17, but given how far behind oauth-proxy seems to be (e.g. it doesn't know aboutk8s.io/api and still sources API types from client-go) it was much simpler to fork the dynamiccertificates code required to ensure cert reload. I've taken care to separate the forking from the changes required for the code to work. Hopefully this also makes for a happier patch manager.

/cc @deads2k @mfojtik @stlaz

Edit: Note that the forked code is contained to the dynamiccertificates path. If in future we decide to bump, it will be trivial to remove that path and rely on vendored code instead.

openshift-ci-robot · 2020-03-03T01:31:48Z

@marun: This pull request references Bugzilla bug 1809253, which is valid. The bug has been updated to refer to the pull request using the external bug tracker.

6 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.3.z) matches configured target release for branch (4.3.z)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
dependent bug Bugzilla bug 1801573 is in the state VERIFIED, which is one of the valid states (VERIFIED, RELEASE_PENDING, CLOSED (ERRATA))
dependent Bugzilla bug 1801573 targets the "4.4.0" release, matching the expected (4.4.0) release
bug has dependents

Details

In response to this:

[release-4.3] Bug 1809253: Reload serving certs

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.

marun · 2020-03-03T14:49:05Z

What's up with travis? The failure appears unrelated to the code, and I have no way to kick it.

openshift-ci-robot · 2020-03-03T15:04:26Z

@marun: This pull request references Bugzilla bug 1809253, which is valid.

6 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.3.z) matches configured target release for branch (4.3.z)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
dependent bug Bugzilla bug 1801573 is in the state VERIFIED, which is one of the valid states (VERIFIED, RELEASE_PENDING, CLOSED (ERRATA))
dependent Bugzilla bug 1801573 targets the "4.4.0" release, matching the expected (4.4.0) release
bug has dependents

Details

In response to this:

[release-4.3] Bug 1809253: Reload serving certs

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.

openshift-ci-robot · 2020-03-04T15:38:37Z

@marun: This pull request references Bugzilla bug 1809253, which is valid.

6 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.3.z) matches configured target release for branch (4.3.z)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)
dependent bug Bugzilla bug 1801573 is in the state VERIFIED, which is one of the valid states (VERIFIED, RELEASE_PENDING, CLOSED (ERRATA))
dependent Bugzilla bug 1801573 targets the "4.4.0" release, matching the expected (4.4.0) release
bug has dependents

Details

In response to this:

[release-4.3] Bug 1809253: Reload serving certs

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.

marun · 2020-03-06T20:57:22Z

@stlaz Travis passed, hooray!

stlaz · 2020-03-10T07:52:18Z

/lgtm

stlaz · 2020-03-17T16:57:51Z

/approve

mfojtik · 2020-03-18T14:01:16Z

/approve

mfojtik · 2020-03-18T14:21:29Z

/approve

sttts · 2020-03-18T14:27:20Z

/approve

openshift-ci-robot · 2020-03-18T14:27:40Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: marun, mfojtik, stlaz, sttts

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:

~~OWNERS~~ [sttts]

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

openshift-cherrypick-robot · 2020-03-18T16:36:54Z

@marun: once the present PR merges, I will cherry-pick it on top of release-4.2 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-4.2

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.

openshift-bot · 2020-03-18T17:00:20Z

/retest

Please review the full test history for this PR and help us cut down flakes.

…1810036 The bugs were introduced by the [1] series, and fixed by the combination of [2,3]. This commit also tombstones affected releases to avoid further channel promotion. Details on the bug: * 4.5: Introduced by [1] (no PR?). Fixed by [2], service-ca-operator 74b5ce2 [4], which included library-go d9c73bb [5]. Also fixed by [3], oauth-proxy 3d0621e [6], which landed before the 4.4/4.5 split. * 4.4: Introduced by [1] (no PR?). Fixed by [7], service-ca-operator e5a04d6 [7], which included library-go 3c25293 [9]. $ oc adm release info --commits quay.io/openshift-release-dev/ocp-release:4.4.0-rc.0-x86_64 | grep service-ca-operator service-ca-operator https://github.com/openshift/service-ca-operator 094a9ad02dbe3bcb57d5fbad301cfcfcd48bd2ed $ oc adm release info --commits quay.io/openshift-release-dev/ocp-release:4.4.0-rc.1-x86_64 | grep service-ca-operator service-ca-operator https://github.com/openshift/service-ca-operator 094a9ad02dbe3bcb57d5fbad301cfcfcd48bd2ed $ git --no-pager log -2 --first-parent --oneline origin/release-4.4 e5a04d6a (origin/release-4.4) Merge pull request openshift#111 from marun/4.4-unique-ca-serial 094a9ad0 Merge pull request #95 from vareti/signer-ca-metrics So both RCs are affected. Also fixed by [3], oauth-proxy 3d0621e [6], which landed before the 4.4/4.5 split. $ oc adm release info --commits quay.io/openshift-release-dev/ocp-release:4.4.0-rc.0-x86_64 | grep oauth-proxy oauth-proxy https://github.com/openshift/oauth-proxy 3d0621eb72c9dd1c036505363032468a9016f381 $ oc adm release info --commits quay.io/openshift-release-dev/ocp-release:4.4.0-rc.1-x86_64 | grep oauth-proxy oauth-proxy https://github.com/openshift/oauth-proxy 3d0621eb72c9dd1c036505363032468a9016f381 So both RCs have OAuth fix, but neither has the service-ca-operator fix. * 4.3: Introduced by [10], service-ca-operator 8395d65 [11]. Fixed by [12], service-ca-operator dd7235b [13], which includes library-go 5844159 [14]. Fix has not been released yet. $ oc adm release info --commits quay.io/openshift-release-dev/ocp-release:4.3.3-x86_64 | grep service-ca-operator service-ca-operator https://github.com/openshift/service-ca-operator 774c394da334dec446703545d4baaf89611ccb9d $ oc adm release info --commits quay.io/openshift-release-dev/ocp-release:4.3.5-x86_64 | grep service-ca-operator service-ca-operator https://github.com/openshift/service-ca-operator 8395d65888b0a4249277989f18ee03f45383e409 So this was introduced in 4.3.5 (there was no 4.3.4). Fix also requires the OAuth proxy fix [15,16], which is still in flight. * 4.2: Introduced by [17], service-ca-operator 0324055 [18], which includes library-go 2cf86bb [19] and API 8ce0047 [20]. Fix in flight with [21,22]. [23] has already landed with library-go d58edcb. $ oc adm release info --commits quay.io/openshift-release-dev/ocp-release:4.2.21-x86_64 | grep service-ca-operator service-ca-operator https://github.com/openshift/service-ca-operator f6720573b9b63147436374e51e6fda44683b1e9f $ oc adm release info --commits quay.io/openshift-release-dev/ocp-release:4.2.22-x86_64 | grep service-ca-operator service-ca-operator https://github.com/openshift/service-ca-operator 0324055c3bad3a857dcf3471c024bf42c20d549e So this was introduced in 4.2.22. Fix also requires the OAuth proxy fix [24,25], which is still in flight. * 4.1: Backport stream introducing the bug is still ASSIGNED [26], so no 4.1 impact yet. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1774121 [2]: https://bugzilla.redhat.com/show_bug.cgi?id=1810036 [3]: https://bugzilla.redhat.com/show_bug.cgi?id=1801573 [4]: openshift/service-ca-operator#110 (comment) [5]: openshift/library-go#726 (comment) [6]: openshift/oauth-proxy#152 (comment) [7]: https://bugzilla.redhat.com/show_bug.cgi?id=1810418 [8]: openshift/service-ca-operator#111 (comment) [9]: openshift/library-go#728 (comment) [10]: https://bugzilla.redhat.com/show_bug.cgi?id=1788179 [11]: openshift/service-ca-operator#104 (comment) [12]: https://bugzilla.redhat.com/show_bug.cgi?id=1810420 [13]: openshift/service-ca-operator#112 (comment) [14]: openshift/library-go#729 (comment) [15]: https://bugzilla.redhat.com/show_bug.cgi?id=1809253 [16]: openshift/oauth-proxy#160 [17]: https://bugzilla.redhat.com/show_bug.cgi?id=1774156 [18]: openshift/service-ca-operator#105 (comment) [19]: openshift/library-go#684 (comment) [20]: openshift/api#577 (comment) [21]: https://bugzilla.redhat.com/show_bug.cgi?id=1810421 [22]: openshift/service-ca-operator#113 [23]: openshift/library-go#730 (comment) [24]: https://bugzilla.redhat.com/show_bug.cgi?id=1809258 [25]: openshift/oauth-proxy#164 [26]: https://bugzilla.redhat.com/show_bug.cgi?id=1774157

…1810036 The bugs were introduced by the [1] series, and fixed by the combination of [2,3]. This commit also tombstones affected releases to avoid further channel promotion. Details on the bug: * 4.5: Introduced by [1] (no linked PR, so not sure exactly when it was introduced). Fixed by [2], service-ca-operator 74b5ce2 [4], which included library-go d9c73bb [5]. Also fixed by [3], oauth-proxy 3d0621e [6], which landed before the 4.4/4.5 split. * 4.4: Introduced by [1] (no linked PR, so not sure exactly when it was introduced). Fixed by [7], service-ca-operator e5a04d6 [7], which included library-go 3c25293 [9]. $ oc adm release info --commits quay.io/openshift-release-dev/ocp-release:4.4.0-rc.0-x86_64 | grep service-ca-operator service-ca-operator https://github.com/openshift/service-ca-operator 094a9ad02dbe3bcb57d5fbad301cfcfcd48bd2ed $ oc adm release info --commits quay.io/openshift-release-dev/ocp-release:4.4.0-rc.1-x86_64 | grep service-ca-operator service-ca-operator https://github.com/openshift/service-ca-operator 094a9ad02dbe3bcb57d5fbad301cfcfcd48bd2ed $ git --no-pager log -2 --first-parent --oneline origin/release-4.4 e5a04d6a (origin/release-4.4) Merge pull request openshift#111 from marun/4.4-unique-ca-serial 094a9ad0 Merge pull request #95 from vareti/signer-ca-metrics So both RCs are affected. Also fixed by [3], oauth-proxy 3d0621e [6], which landed before the 4.4/4.5 split. $ oc adm release info --commits quay.io/openshift-release-dev/ocp-release:4.4.0-rc.0-x86_64 | grep oauth-proxy oauth-proxy https://github.com/openshift/oauth-proxy 3d0621eb72c9dd1c036505363032468a9016f381 $ oc adm release info --commits quay.io/openshift-release-dev/ocp-release:4.4.0-rc.1-x86_64 | grep oauth-proxy oauth-proxy https://github.com/openshift/oauth-proxy 3d0621eb72c9dd1c036505363032468a9016f381 So both RCs have OAuth fix, but neither has the service-ca-operator fix. * 4.3: Introduced by [10], service-ca-operator 8395d65 [11]. Fixed by [12], service-ca-operator dd7235b [13], which includes library-go 5844159 [14]. Fix has not been released yet. $ oc adm release info --commits quay.io/openshift-release-dev/ocp-release:4.3.3-x86_64 | grep service-ca-operator service-ca-operator https://github.com/openshift/service-ca-operator 774c394da334dec446703545d4baaf89611ccb9d $ oc adm release info --commits quay.io/openshift-release-dev/ocp-release:4.3.5-x86_64 | grep service-ca-operator service-ca-operator https://github.com/openshift/service-ca-operator 8395d65888b0a4249277989f18ee03f45383e409 So this was introduced in 4.3.5 (there was no 4.3.4). Fix also requires the OAuth proxy fix [15,16], which is still in flight. * 4.2: Introduced by [17], service-ca-operator 0324055 [18], which includes library-go 2cf86bb [19] and API 8ce0047 [20]. Fix in flight with [21,22]. [23] has already landed with library-go d58edcb. $ oc adm release info --commits quay.io/openshift-release-dev/ocp-release:4.2.21-x86_64 | grep service-ca-operator service-ca-operator https://github.com/openshift/service-ca-operator f6720573b9b63147436374e51e6fda44683b1e9f $ oc adm release info --commits quay.io/openshift-release-dev/ocp-release:4.2.22-x86_64 | grep service-ca-operator service-ca-operator https://github.com/openshift/service-ca-operator 0324055c3bad3a857dcf3471c024bf42c20d549e So this was introduced in 4.2.22. Fix also requires the OAuth proxy fix [24,25], which is still in flight. * 4.1: Backport stream introducing the bug is still ASSIGNED [26], so no 4.1 impact yet. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1774121 [2]: https://bugzilla.redhat.com/show_bug.cgi?id=1810036 [3]: https://bugzilla.redhat.com/show_bug.cgi?id=1801573 [4]: openshift/service-ca-operator#110 (comment) [5]: openshift/library-go#726 (comment) [6]: openshift/oauth-proxy#152 (comment) [7]: https://bugzilla.redhat.com/show_bug.cgi?id=1810418 [8]: openshift/service-ca-operator#111 (comment) [9]: openshift/library-go#728 (comment) [10]: https://bugzilla.redhat.com/show_bug.cgi?id=1788179 [11]: openshift/service-ca-operator#104 (comment) [12]: https://bugzilla.redhat.com/show_bug.cgi?id=1810420 [13]: openshift/service-ca-operator#112 (comment) [14]: openshift/library-go#729 (comment) [15]: https://bugzilla.redhat.com/show_bug.cgi?id=1809253 [16]: openshift/oauth-proxy#160 [17]: https://bugzilla.redhat.com/show_bug.cgi?id=1774156 [18]: openshift/service-ca-operator#105 (comment) [19]: openshift/library-go#684 (comment) [20]: openshift/api#577 (comment) [21]: https://bugzilla.redhat.com/show_bug.cgi?id=1810421 [22]: openshift/service-ca-operator#113 [23]: openshift/library-go#730 (comment) [24]: https://bugzilla.redhat.com/show_bug.cgi?id=1809258 [25]: openshift/oauth-proxy#164 [26]: https://bugzilla.redhat.com/show_bug.cgi?id=1774157

openshift-bot · 2020-03-18T19:09:01Z

/retest

Please review the full test history for this PR and help us cut down flakes.

marun · 2020-03-18T20:21:34Z

/retest

openshift-bot · 2020-03-18T22:49:35Z

/retest

Please review the full test history for this PR and help us cut down flakes.

vikaslaad · 2020-03-18T23:08:24Z

/retest

Add dynamiccertificates from kube 1.17

f86e99a

openshift-ci-robot requested review from deads2k, mfojtik and stlaz March 3, 2020 01:31

openshift-ci-robot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 3, 2020

marun added 2 commits March 2, 2020 17:41

Fix forked dynamiccertificates code to work with old dep state

d9734b8

Add dep github.com/golang/groupcache (required by event recorder)

9d9b136

marun force-pushed the 4.3-reload-serving-cert branch from 11b045c to b2d9064 Compare March 3, 2020 01:42

Reload serving certs

658cafa

marun force-pushed the 4.3-reload-serving-cert branch from b2d9064 to 658cafa Compare March 3, 2020 01:42

openshift-ci-robot assigned stlaz Mar 10, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 10, 2020

mfojtik added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Mar 18, 2020

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 18, 2020

mrunalp added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Mar 18, 2020

marun mentioned this pull request Mar 18, 2020

[release-4.2] Bug 1809258: Reload serving certs #164

Merged

openshift-merge-robot merged commit d5df318 into openshift:release-4.3 Mar 19, 2020

[release-4.3] Bug 1809253: Reload serving certs #160

[release-4.3] Bug 1809253: Reload serving certs #160

Uh oh!

Conversation

marun commented Mar 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci-robot commented Mar 3, 2020

Uh oh!

marun commented Mar 3, 2020

Uh oh!

openshift-ci-robot commented Mar 3, 2020

Uh oh!

openshift-ci-robot commented Mar 4, 2020

Uh oh!

marun commented Mar 6, 2020

Uh oh!

stlaz commented Mar 10, 2020

Uh oh!

stlaz commented Mar 17, 2020

Uh oh!

mfojtik commented Mar 18, 2020

Uh oh!

mfojtik commented Mar 18, 2020

Uh oh!

sttts commented Mar 18, 2020

Uh oh!

openshift-ci-robot commented Mar 18, 2020

Uh oh!

openshift-cherrypick-robot commented Mar 18, 2020

Uh oh!

openshift-bot commented Mar 18, 2020

Uh oh!

openshift-bot commented Mar 18, 2020

Uh oh!

marun commented Mar 18, 2020

Uh oh!

openshift-bot commented Mar 18, 2020

Uh oh!

vikaslaad commented Mar 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

marun commented Mar 3, 2020 •

edited

Loading