Adds proxy https readinessEndpoints support

[danehans]

Adds support for for TLS-based probes for validating a httpsProxy using one or more readinessEndpoints that contain a https scheme.

Requires #271

/assign @squeed @bparees

PTAL at this block and this block. All other changes are due to the #271 dependency.

[bparees]

last 2 commits (the ones that add the probes) lgtm

[knobunc]

/approve

[bparees]

i think this logic (and the comment above) are still incorrect, per my earlier comment.

If "trustedCA" is empty that means "proxyconfig.trustedca.name" is empty (in other words, the admin has not pointed us to a configmap of user provided CAs).

If that is the case:

1. there's no point in validating anything
2. we should be returning the system trust bundle (ie the trust bundle included in our image). Not the openshift-config-managed CM bundle.

There is never a reason to validate the openshift-config-managed CM bundle, as this is controller is itself responsible for producing that CM, so it better be valid for us to have produced it. Furthermore the purpose of the validation we are doing here is to ensure that the CA bundle can be used to talk to the proxy. That needs to happen before we copy any bundles into openshift-config-managed, because once it's in openshift-config-managed, it's going to get injected into the various consumer CMs. So validating openshift-config-managed isn't useful (it's too late).

[danehans]

If "trustedCA" is empty that means "proxyconfig.trustedca.name" is empty (in other words, the admin has not pointed us to a configmap of user provided CAs).

@bparees understood. I'm calling validateTrustedCA() instead of making multiple func calls to get the trusted-ca-bundle configmap, parse the data key into a []byte and then parse the []byte into a []*x509.Certificate that is used as a parameter for the validateReadinessEndpoint() func. Some of the funcs can be improved and renamed to remove the validation prefix, but I would prefer doing that in a follow-on PR at this point.

we should be returning the system trust bundle (ie the trust bundle included in our image). Not the openshift-config-managed CM bundle.

trusted-ca-bundle configmap contains the system trust bundle whether or not trustedCA is set. Even if I used the local system trust bundle, I would have to make multiple calls to read the local trust bundle file, parse the file into a []byte and then parse the []byte into a []*x509.Certificate that is used as a parameter for the validateReadinessEndpoint() func. validateTrustedCA() already does this, so I prefer to use the func for the time being and I can improve how the funcs are composed in a future PR.

I have updated the call to validateTrustedCA() to use "trusted-ca-bundle" and removed:

-       if len(trustedCA) == 0 {
-               trustedCA = names.TRUSTED_CA_BUNDLE_CONFIGMAP
-       }

[bparees]

this logic needs to validate that the set of CAs we are going to publish into trusted-ca-bundle, are sufficient to talk to the proxy in question.

So we cannot use the current set off CAs in trusted-ca-bundle to do that, we need to produce the new bundle (ie "just the system CAs" or "system CAs plus the new user provided CAs") and use that bundle to perform the validation(readiness checks)

[danehans]

we need to produce the new bundle (ie "just the system CAs" or "system CAs plus the new user provided CAs") and use that bundle to perform the validation(readiness checks)

@bparees I pushed a new commit that accomplishes this workflow. See this testing doc for details. When proxy.spec.trustedCA is updated, openshift-config-managed/trusted-ca-bundle configmap is updated before proxy config validation (i.e. running readinessEndpoints probes). openshift-config-managed/trusted-ca-bundle will contain the combined user/system bundle if proxy.spec.trustedCA is updated to a configmap with a different user-proivded bundle. openshift-config-managed/trusted-ca-bundle will contain only the system bundle if proxy.spec.trustedCA is empty. The controller will use this bundle for validating the httpsProxy using readinessEndpoints that contain a https scheme. If the https proxy's identity cert has been signed by a ca outside the system trust bundle and proxy.spec.trustedCA is empty, the readiness probe will fail.

[danehans]

/test e2e-aws-ovn-kubernetes

[openshift-ci-robot]

@danehans: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/e2e-aws-ovn-kubernetes	8fd3311	link	/test e2e-aws-ovn-kubernetes

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.

[bparees]

/lgtm

future refactor should:

1. only generate the proposed bundle a single time (and then pass it to Validate, instead of having Validate construct its own bundle) so that we are actually validating the same bundle we ultimately publish
2. combine/refactor the two codepaths for "did the proxyconfig change" and "did the configmap change" since both codepaths basically do the same thing, it's just duplicated code.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, danehans, knobunc

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 [knobunc]

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