USHIFT-2188: introduce microshift API Custom certs

[eslutsky]

No description provided.

[openshift-ci-robot]

@eslutsky: This pull request references USHIFT-2188 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.16.0" version, but no target version was set.

Details

In 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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[eslutsky]

/test all

[eslutsky]

/test all

[eslutsky]

/cc @benluddy

[eslutsky]

/cc @stlaz

[stlaz]

Seems fine from the auth side of things

[benluddy]

IIUC, cert and key files passed to --tls-sni-cert-key are watched and reloaded dynamically (via https://github.com/openshift/kubernetes/blob/352677df596e5dab4098898974e36df8bcd067e2/staging/src/k8s.io/apiserver/pkg/server/options/serving.go#L323). Is it worthwhile to perform the proposed validation once at startup?

OCP performs some validation before a configured cert and name reach that flag, but in that case the files being passed to the kube-apiserver process are operator-managed as opposed to being directly managed by the user, as in the Microshift case.

• https://github.com/openshift/kubernetes/blob/c3ad486c907cdf30ee97c9a7e7052a823a49c34b/openshift-kube-apiserver/admission/customresourcevalidation/apiserver/validate_apiserver.go#L54
• https://github.com/openshift/cluster-kube-apiserver-operator/blob/e6cba0a210714e935ac20c95873cc63e1175bb43/pkg/operator/configobservation/apiserver/observe_apiserver.go#L91

[dhellmann]

That validation to ensure the settings in the cert don't collide with the internal IPs of the cluster look valuable to avoid breaking the ability of pods to talk to the API server. Thanks for pointing those out, we had a hard time finding anything like that!

[eslutsky]

added table with reserved IP/DNS values which will be cause the certificate to be ignored.

[benluddy]

Do you plan to do anything to prevent the files at certPath/keyPath passing startup-time validation, being modified in such a way that they would fail startup-time validation, and then being dynamically reloaded at runtime? It's potentially surprising that changing namedCertificates in the config file has no effect until the next restart, but changes to the contents of files referenced in the config will be dynamically reloaded (unless I am misunderstanding how the --tls-sni-cert-key option works).

[dhellmann]

That does sound like a potentially bad hole that bypasses important validation.

[vrutkovs]

/lgtm

[openshift-ci]

@eslutsky: all tests passed!

Full PR test history. Your PR dashboard.

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.

[pmtk]

/lgtm

[jerpeter1]

@benluddy Have your questions/concerns been resolved?

[benluddy]

@benluddy Have your questions/concerns been resolved?

I added a follow-up question here https://github.com/openshift/enhancements/pull/1541/files#r1537894599 but it seems I am not allowed to mark its thread as unresolved. I'm happy leaving any (or no) resulting changes from that question to the MicroShift team's judgment since they know their users better than me.

/lgtm

[dhellmann]

@benluddy Have your questions/concerns been resolved?

I added a follow-up question here https://github.com/openshift/enhancements/pull/1541/files#r1537894599 but it seems I am not allowed to mark its thread as unresolved. I'm happy leaving any (or no) resulting changes from that question to the MicroShift team's judgment since they know their users better than me.

I unresolved the thread to make sure we don't lose track of it.

[jerpeter1]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerpeter1

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

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