pkg/controller/cincinnati: Use InsecureEdgeTerminationPolicyNone #64
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Or in the 'oc -n ... create ...' call that pushes them into the cluster. And also use the NAMESPACE variable when looking up the policy engine's route. We could add an environment variable for the Cincinnati manifest's name, but I think 'example-name', which I'm pivoting to, is clear enough in pattern-matching that we don't need a formal environment variable.
We had used InsecureEdgeTerminationPolicyAllow since the route landed in 1fdf865 (Create a route for Cincinnati service, 2020-05-01, openshift#30). The motivation for that value didn't make it into the Git commit message, but from discussion in the GitHub pull request [1], it was: * InsecureEdgeTerminationPolicyAllow is the default termination policy. * Cincinnati's docs have no preference [2]. However, we really, really want HTTPS security for cluster-version operators making upstream requests for update recommendations. There are long-term plans for tightening down guards against malicious, compromised, or man-in-the-middled update recommendation services, but today we have yet to land even guards as basic as "upstream is lying about the version string associated with a given release image" [3]. By removing HTTP termination [4], we force consumers to configure their clients, including the cluster-version operator, with https:// URIs (or do something else explicit like setting up their own HTTP termination) before they can access the policy-engine output, which reduces the risk that they will recieve and trust compromised update graphs. This may be a breaking change, but: * We're still in beta, and not yet in general-availability with backwards-compatability requirements. * Folks who have configured their cluster-version operators and other clients with http:// upstreams should *want* to be broken. We are protecting them from all sorts of compromised-upstream failure modes. * The cluster-version operator, and other well-behaved clients, will report understandable error messages for "I tried to connect over HTTP and there was nobody there", which will lead users into auditing and fixing their upstream URIs, so recovering from the breakage should not be to onerous. [1]: openshift#30 (comment) [2]: https://github.com/openshift/cincinnati/blame/0bb5f6f3228858f9e5d1807bd6f45f46e537cdea/docs/user/running-cincinnati.md#L87-L88 [3]: openshift/cluster-version-operator#431 [4]: https://github.com/openshift/api/blob/346618ed7d5e6396191efe6f10b2c36f1e95d8b7/route/v1/types.go#L258-L259
|
[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
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
wking
force-pushed
the
route-tls-termination
branch
from
c0ec2fc
to
8d614f5
Compare
|
Tested the https (without configuring a cert) aspect of this change and appears okay. /lgtm |
|
I dunno what was wrong with the deployment. Maybe it was just a flake... /retest |
PratikMahajan
pushed a commit
to PratikMahajan/cincinnati-operator
that referenced
this pull request
Mar 17, 2021
channels/fast-4.2: Promote 4.2.19 (and 4.2.19+amd64 to fast-4.3)
PratikMahajan
pushed a commit
to PratikMahajan/cincinnati-operator
that referenced
this pull request
Mar 17, 2021
It's been at least 24 days since the fast promotions landed, so finish the "phased rollout" and put them in stable. * 4.2.16+amd64 was promoted to the feeder fast-4.3 by 7660e1d (Merge pull request openshift#73 from wking/4.2.16-to-fast-4.3, 2020-02-21). * There was no 4.2.17 (no releases in the week after 4.2.16). * 4.2.18+amd64 was promoted to the feeder fast-4.3 by fa66fd4 (Merge pull request openshift#60 from wking/fast-4.2.18, 2020-02-20). * 4.2.19+amd64 was promoted to the feeder fast-4.3 by 04c5b05 (Merge pull request openshift#64 from thiagoalessio/fast-4.2, 2020-02-24). The previous comments about "only valid into" were from 505f7a6 (Add 4.2 edges into stable-4.3, 2020-03-12, openshift#109), and I'm not clear on the motivation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Builds on #63; you may want to review that first.
We had used
InsecureEdgeTerminationPolicyAllowsince the route landed in 1fdf865 (#30). The motivation for that value didn't make it into the Git commit message, but from discussion in the GitHub pull request, it was:InsecureEdgeTerminationPolicyAllowis the default termination policy.However, we really, really want HTTPS security for cluster-version operators making upstream requests for update recommendations. There are long-term plans for tightening down guards against malicious, compromised, or man-in-the-middled update recommendation services, but today we have yet to land even guards as basic as "upstream is lying about the version string associated with a given release image".
By removing HTTP termination, we force consumers to configure their clients, including the cluster-version operator, with
https://URIs (or do something else explicit like setting up their own HTTP termination) before they can access the policy-engine output, which reduces the risk that they will recieve and trust compromised update graphs. This may be a breaking change, but:http://upstreams should want to be broken. We are protecting them from all sorts of compromised-upstream failure modes.