Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌱 Prepare envtest certs for aggregation layer #1449

Closed

Conversation

timebertt
Copy link
Contributor

@timebertt timebertt commented Mar 24, 2021

Prepare API server certs in envtest for usage with the aggregation layer:

  • write CA cert of API server to a file for usage as --requestheader-client-ca-file and --client-ca-file
  • add ClientAuth key usage to API server cert, so it can be used by the API server to authenticate against an extension API server

With this change, tests are able to setup the API server's aggregation layer for registering APIServices in their test environment.

For example:

testEnv.ControlPlane.APIServer.Args = append(envtest.DefaultKubeAPIServerFlags,
	"--client-ca-file={{ .CertDir }}/apiserver-ca.crt",
	"--proxy-client-key-file={{ .CertDir }}/apiserver.key",
	"--proxy-client-cert-file={{ .CertDir }}/apiserver.crt",
	"--requestheader-client-ca-file={{ .CertDir }}/apiserver-ca.crt",
	"--requestheader-extra-headers-prefix=X-Remote-Extra-",
	"--requestheader-group-headers=X-Remote-Group",
	"--requestheader-username-headers=X-Remote-User",
)

These are the minimum required changes to make this scenario work.

Ref #1448

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 24, 2021
@k8s-ci-robot
Copy link
Contributor

Hi @timebertt. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: timebertt
To complete the pull request process, please assign apelisse after the PR has been reviewed.
You can assign the PR to them by writing /assign @apelisse in a comment when ready.

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

Needs approval from an approver in each of these files:

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

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 24, 2021
@timebertt
Copy link
Contributor Author

@alvaroaleman @DirectXMan12 can one of you check this PR?

@alvaroaleman
Copy link
Member

@DirectXMan12 is currently working on adding TLS support to envtest, maybe we should wait for that?

@timebertt
Copy link
Contributor Author

Hmm, probably makes sense.
@DirectXMan12 if you want we can then incorporate my changes into your larger PR.
Let me know if I can help you in any way. Is there already an issue for this?

@timebertt
Copy link
Contributor Author

/assign @DirectXMan12

@timebertt
Copy link
Contributor Author

friendly ping to @DirectXMan12

@timebertt
Copy link
Contributor Author

@alvaroaleman @DirectXMan12 how do we proceed here?
Should we just get this small change in for the time being?

@DirectXMan12
Copy link
Contributor

hey, I'm sorry for dropping this one, I'll figure out what to do with this shortly -- lost track of some PRs. Btw, if this happens again, please do ping me on slack -- I occasionally lose track of github notifications.

@timebertt
Copy link
Contributor Author

@DirectXMan12 Ok, will ping you on slack next time :)
Thanks for coming back to this PR. I'll have a look at #1486 in the meantime and check what is missing there for the aggregation layer use case.

@k8s-ci-robot
Copy link
Contributor

@timebertt: PR needs rebase.

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.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 27, 2021
@timebertt
Copy link
Contributor Author

Currently working on upgrading to controller-runtime@v0.9.0 (gardener/gardener#4174). Will figure out what to do with this PR during that.

timebertt added a commit to timebertt/gardener that referenced this pull request Jun 16, 2021
- Make use of the new Users concept in envtest to provision a dedicated
  user for gardener-apiserver and a valid kubeconfig

- Make use of the new way to configure API server args to easily configure
  kube-aggregator flags

- Also generate certs for aggregation layer on our own instead of reusing
  the API server ca/certs (which is semantically correct), which allows us
  to drop our fork including kubernetes-sigs/controller-runtime#1449
timebertt added a commit to gardener/gardener that referenced this pull request Jun 16, 2021
* Upgrade to k/*@v0.21.1 in go.mod

* [automated] make revendor for k/* dependencies

This deletes pkg/mock/client-go/kubernetes/mocks.go to resolve the
following deadlock: make revendor fails because of some dependencies
of the file and make generate fails because of missing revendoring.
File will be generated again in the next commit.

* [automated] make generate for k/* dependencies

* Upgrade to c-r@v0.9.0 in go.mod

* [automated] make revendor for c-r dependency

`make revendor` results in `hack/setup-envtest.sh` being broken, so reset
the file after running `make revendor`. Adaption to breaking changes
in the upstream file will be done in a later commit.

* manager.NewClientBuilder was removed in favor of cluster.DefaultNewClient

ref kubernetes-sigs/controller-runtime#1409

* client.*MergeFrom* now take client.Object instead of runtime.Object

ref kubernetes-sigs/controller-runtime#1395

* [automated] make generate for c-r dependency

* Adapt to changes in labels.NewRequirement

ref kubernetes/kubernetes#97538

* Adapt to new setup-envtest tool

Makes use of the new setup-envtest tool (kubernetes-sigs/controller-runtime#1488)
in hack/setup-envtest.sh instead of vendoring hack/setup-envtest.sh and fetching
binaries with that.

* [automated] make revendor for setup-envtest tool

* Adapt pkg/envtest to upstream changes

- Make use of the new Users concept in envtest to provision a dedicated
  user for gardener-apiserver and a valid kubeconfig

- Make use of the new way to configure API server args to easily configure
  kube-aggregator flags

- Also generate certs for aggregation layer on our own instead of reusing
  the API server ca/certs (which is semantically correct), which allows us
  to drop our fork including kubernetes-sigs/controller-runtime#1449

* Styling nits
@timebertt
Copy link
Contributor Author

timebertt commented Jun 17, 2021

After upgrading to controller-runtime@v0.9.0, I adapted our configuration of the aggregation layer to #1541 and decided to not reuse the client/serving certs and generate dedicated front-proxy CA/cert/key instead (which is also cleaner) – reusing the client/serving certs was more a quick and dirty solution actually. With that, this PR is not required anymore, so
/close

You can find the updated example for how to configure the aggregation layer in envtest here.

@k8s-ci-robot
Copy link
Contributor

@timebertt: Closed this PR.

In response to this:

After upgrading to controller-runtime@v0.9.0, I adapted our configuration of the aggregation layer to #1541 and decided to not reuse the client certs and generate dedicated front-proxy CA/cert/key instead (which is also cleaner) – reusing the client certs was more a quick and dirty solution actually. With that, this PR is not required anymore, so
/close

You can find the updated example for how to configure the aggregation layer in envtest here.

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.

@timebertt timebertt deleted the enh/envtest-certs branch June 17, 2021 08:39
krgostev pushed a commit to krgostev/gardener that referenced this pull request Apr 21, 2022
* Upgrade to k/*@v0.21.1 in go.mod

* [automated] make revendor for k/* dependencies

This deletes pkg/mock/client-go/kubernetes/mocks.go to resolve the
following deadlock: make revendor fails because of some dependencies
of the file and make generate fails because of missing revendoring.
File will be generated again in the next commit.

* [automated] make generate for k/* dependencies

* Upgrade to c-r@v0.9.0 in go.mod

* [automated] make revendor for c-r dependency

`make revendor` results in `hack/setup-envtest.sh` being broken, so reset
the file after running `make revendor`. Adaption to breaking changes
in the upstream file will be done in a later commit.

* manager.NewClientBuilder was removed in favor of cluster.DefaultNewClient

ref kubernetes-sigs/controller-runtime#1409

* client.*MergeFrom* now take client.Object instead of runtime.Object

ref kubernetes-sigs/controller-runtime#1395

* [automated] make generate for c-r dependency

* Adapt to changes in labels.NewRequirement

ref kubernetes/kubernetes#97538

* Adapt to new setup-envtest tool

Makes use of the new setup-envtest tool (kubernetes-sigs/controller-runtime#1488)
in hack/setup-envtest.sh instead of vendoring hack/setup-envtest.sh and fetching
binaries with that.

* [automated] make revendor for setup-envtest tool

* Adapt pkg/envtest to upstream changes

- Make use of the new Users concept in envtest to provision a dedicated
  user for gardener-apiserver and a valid kubeconfig

- Make use of the new way to configure API server args to easily configure
  kube-aggregator flags

- Also generate certs for aggregation layer on our own instead of reusing
  the API server ca/certs (which is semantically correct), which allows us
  to drop our fork including kubernetes-sigs/controller-runtime#1449

* Styling nits
krgostev pushed a commit to krgostev/gardener that referenced this pull request Jul 5, 2022
* Upgrade to k/*@v0.21.1 in go.mod

* [automated] make revendor for k/* dependencies

This deletes pkg/mock/client-go/kubernetes/mocks.go to resolve the
following deadlock: make revendor fails because of some dependencies
of the file and make generate fails because of missing revendoring.
File will be generated again in the next commit.

* [automated] make generate for k/* dependencies

* Upgrade to c-r@v0.9.0 in go.mod

* [automated] make revendor for c-r dependency

`make revendor` results in `hack/setup-envtest.sh` being broken, so reset
the file after running `make revendor`. Adaption to breaking changes
in the upstream file will be done in a later commit.

* manager.NewClientBuilder was removed in favor of cluster.DefaultNewClient

ref kubernetes-sigs/controller-runtime#1409

* client.*MergeFrom* now take client.Object instead of runtime.Object

ref kubernetes-sigs/controller-runtime#1395

* [automated] make generate for c-r dependency

* Adapt to changes in labels.NewRequirement

ref kubernetes/kubernetes#97538

* Adapt to new setup-envtest tool

Makes use of the new setup-envtest tool (kubernetes-sigs/controller-runtime#1488)
in hack/setup-envtest.sh instead of vendoring hack/setup-envtest.sh and fetching
binaries with that.

* [automated] make revendor for setup-envtest tool

* Adapt pkg/envtest to upstream changes

- Make use of the new Users concept in envtest to provision a dedicated
  user for gardener-apiserver and a valid kubeconfig

- Make use of the new way to configure API server args to easily configure
  kube-aggregator flags

- Also generate certs for aggregation layer on our own instead of reusing
  the API server ca/certs (which is semantically correct), which allows us
  to drop our fork including kubernetes-sigs/controller-runtime#1449

* Styling nits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants