Bug 1771564: pkg/asset/manifests/proxy: Link trustedCA for transparent proxies #2658
Conversation
openshift-ci-robot
added
the
size/XS
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
wking
changed the title
openshift-ci-robot
added
the
bugzilla/valid-bug
|
@wking: This pull request references Bugzilla bug 1771564, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
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. |
|
/hold
transparent and configuration are weird together.
i don't like us hacking this, because this is only true for components that think they have to talk using proxy and there isn't a requirement that every one needs to do that yet. |
openshift-ci-robot
added
the
do-not-merge/hold
Can you elaborate on this?
Everyone that might need to go through a proxy is already required to respect the proxy's And I think my "I think we want to adjust the installer as we go to put the configured trust bundle in the appropriate place to get the cluster components to pick it up, and at the moment the proxy config is that place." is sound. If you think "proxy config is not good enough", then we probably want installer logic to fatally error or at least warn if |
|
/retest The CI-cluster thing that lead to all the |
|
I don't think we should do this. To me the Installer is doing what it should be. Other components should be responsible for populating the CA throughout the cluster whether or not a proxy is configured. To be discussed during group-g arch call on Thursday. |
|
The installer should always propogate CA bundles to the components that need them. Whether or not this is a valid long term solution, we do need a short term solution. |
|
/retest Still stuck on the same images job. Maybe the failure was cached? If so, cache should have expired by now. |
Folks may want to configure additional CAs even in the absence of an explicitly-configured proxy (e.g. because they are using a transparent man-in-the-middle proxy). With this commit, we set the proxy config to point at the trusted CA bundle regardless of whether the httpProxy or httpsProxy properties were set. This also opens us up to folks who want to set additional CAs for other purposes (e.g. connecting to a local registry) by leaning on the network-operator's injection tooling and the proxy config's cluster-scoped trustedCA property. That's cheating a bit, and for things like registry access there may be more appropriate operator-level configs that we could be setting. But the installer-level additionalTrustBundle isn't documented as proxy-specific, so I think we want to adjust the installer as we go to put the configured trust bundle in the appropriate place to get the cluster components to pick it up, and at the moment the proxy config is that place.
wking
force-pushed
the
transparent-proxies
branch
from
5e17408 to
a55be4c
Compare
|
Better way to bump tests, I just rebased onto master with 5e174080c -> a55be4c :p. |
|
/test e2e-openstack test failed due to infra failure |
|
@wking: The following tests failed, say
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. DetailsInstructions 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. |
|
/test e2e-openstack |
|
If we land this (and maybe even if we don't?), we'll want to also update logic like this CVO code that is currently only consuming |
|
closing in favor of concensus from openshift/enhancements#115 /close |
|
@abhinavdahiya: Closed this PR. DetailsIn response to this:
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. |
5 participants
Folks may want to configure additional CAs even in the absence of an explicitly-configured proxy (e.g. because they are using a transparent man-in-the-middle proxy). With this commit, we set the proxy config to point at the trusted CA bundle regardless of whether the httpProxy or httpsProxy properties were set.
This also opens us up to folks who want to set additional CAs for other purposes (e.g. connecting to a local registry) by leaning on the network-operator's injection tooling and the proxy config's cluster-scoped
trustedCAproperty. That's cheating a bit, and for things like registry access there may be more appropriate operator-level configs that we could be setting. But the installer-leveladditionalTrustBundleisn't documented as proxy-specific, so I think we want to adjust the installer as we go to put the configured trust bundle in the appropriate place to get the cluster components to pick it up, and at the moment the proxy config is that place.