OCPBUGS-55335: apiserver: remove the requirement for LoopbackClientConfig

[vrutkovs]

What type of PR is this?

/kind bug

What this PR does / why we need it:

This setting is verifying that apiserver always have a config to connect to itself via loopback connection. This config includes a certificate key pair generated on the fly and stored in memory.

This function is also used by library-go to create any controller (i.e. operator). The controllers don't need loopback connections to function, so it generates certificate keypairs and doesn't use them.

Removing the requirement for LoopbackClientConfig won't affect how apiservers functions (all of them have it created and that doesn't change) and it allows library-go controllers to avoid creating in-memory certificate keypair not included in TLS registry

[openshift-ci-robot]

@vrutkovs: This pull request references Jira Issue OCPBUGS-55335, which is invalid:

• expected the bug to target the "4.19.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

What type of PR is this?

/kind bug

What this PR does / why we need it:

This setting is verifying that apiserver always have a config to connect to itself via loopback connection. This config includes a certificate key pair generated on the fly and stored in memory.

This function is also used by library-go to create any controller (i.e. operator). The controllers don't need loopback connections to function, so it generates certificate keypairs and doesn't use them.

Removing the requirement for LoopbackClientConfig won't affect how apiservers functions (all of them have it created and that doesn't change) and it allows library-go controllers to avoid creating in-memory certificate keypair not included in TLS registry

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-robot]

@vrutkovs: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

• 119ef25|UPSTREAM: : OCPBUGS-55335: apiserver: remove the requirement for LoopbackClientConfig: does not specify an upstream backport in the commit message

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vrutkovs
Once this PR has been reviewed and has the lgtm label, please assign tkashem for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• DOWNSTREAM_OWNERS

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

[benluddy]

We don't have any operators that build against our Kubernetes fork, do we? If you wanted to pursue this approach I think you'd have to do it via https://github.com/openshift/kubernetes-apiserver/, but I don't think any of our operators should be using that, either.

[openshift-ci]

@vrutkovs: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-ovn-runc	119ef25	link	true	/test e2e-aws-ovn-runc
ci/prow/e2e-aws-ovn-cgroupsv2	119ef25	link	true	/test e2e-aws-ovn-cgroupsv2
ci/prow/okd-scos-e2e-aws-ovn	119ef25	link	false	/test okd-scos-e2e-aws-ovn
ci/prow/e2e-agnostic-ovn-cmd	119ef25	link	false	/test e2e-agnostic-ovn-cmd
ci/prow/e2e-aws-ovn-serial	119ef25	link	true	/test e2e-aws-ovn-serial

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

[vrutkovs]

We don't have any operators that build against our Kubernetes fork, do we? If you wanted to pursue this approach I think you'd have to do it via openshift/kubernetes-apiserver, but I don't think any of our operators should be using that, either.

This is used in library-go - https://github.com/openshift/library-go/blob/release-4.19/pkg/controller/controllercmd/builder.go#L326

And I think a replace is required then?

[benluddy]

We don't have any operators that build against our Kubernetes fork, do we? If you wanted to pursue this approach I think you'd have to do it via openshift/kubernetes-apiserver, but I don't think any of our operators should be using that, either.

This is used in library-go - https://github.com/openshift/library-go/blob/release-4.19/pkg/controller/controllercmd/builder.go#L326

And I think a replace is required then?

I don't think so. This fork already has a dependency on library-go.

The only established path we have for carrying patches to k8s.io/apiserver and consuming them from binaries other than the upstream Kubernetes components is to carry patches in github.com/openshift/kubernetes-apiserver... but is it worth carrying a patch there, plus replace directives in all openshift components, in order to address this issue?

[vrutkovs]

but is it worth carrying a patch there, plus replace directives in all openshift components, in order to address this issue?

I don't think the issue is that serious. The alternative might be a custom function in library-go which does everything that New() does except doesn't do LoopbackConfig check, wdyt?

New function is called here and the function itself is huge, its probably not worth it.

[vrutkovs]

Closing this as either we need to land this upstream first or use complicated series of carries and replaces

[openshift-ci-robot]

@vrutkovs: This pull request references Jira Issue OCPBUGS-55335. The bug has been updated to no longer refer to the pull request using the external bug tracker. All external bug links have been closed. The bug has been moved to the NEW state.

In response to this:

What type of PR is this?

/kind bug

What this PR does / why we need it:

This setting is verifying that apiserver always have a config to connect to itself via loopback connection. This config includes a certificate key pair generated on the fly and stored in memory.

This function is also used by library-go to create any controller (i.e. operator). The controllers don't need loopback connections to function, so it generates certificate keypairs and doesn't use them.

Removing the requirement for LoopbackClientConfig won't affect how apiservers functions (all of them have it created and that doesn't change) and it allows library-go controllers to avoid creating in-memory certificate keypair not included in TLS registry

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.