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

📖 Document changes to BYO certificates #9585

Merged
merged 1 commit into from
Mar 6, 2024

Conversation

lentzi90
Copy link
Contributor

@lentzi90 lentzi90 commented Oct 20, 2023

What this PR does / why we need it:

This documents a change in requirements for user provided certificates

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #9568

@k8s-ci-robot
Copy link
Contributor

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

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 20, 2023
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 20, 2023
@k8s-ci-robot k8s-ci-robot added do-not-merge/needs-area PR is missing an area label size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 20, 2023
- Introduced function `CollectInfrastructureLogs` at the `ClusterLogCollector` interface in `test/framework/cluster_proxy.go` to allow collecting infrastructure related logs during tests.
- A `GetTypedConfigOwner` function has been added to the `sigs.k8s.io./cluster-api/bootstrap/util` package. It is equivalent to `GetConfigOwner` except that it uses the cached typed client instead of the uncached unstructured client, so `GetTypedConfigOwner` is expected to be more performant.
- `ClusterToObjectsMapper` in `sigs.k8s.io./cluster-api/util` has been deprecated, please use `ClusterToTypedObjectsMapper` instead.
- The generated `kubeconfig` by the Control Plane providers must be labelled with the key-value pair `cluster.x-k8s.io/cluster-name=${CLUSTER_NAME}`.
This is required for the CAPI managers caches to store and retrieve them for the required operations.
This is required for the CAPI managers caches to store and retrieve them for the required operations.
- When using custom certificates, the certificates must be labeled with the key-value pair `cluster.x-k8s.io/cluster-name=${CLUSTER_NAME}`.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: The ${CLUSTER_NAME} is not really required. As long as the key exists, it will work, but perhaps it is better to be consistent throughout the docs with the name also?

Comment on lines 269 to 280
Cache: cache.Options{
// Namespaces: watchNamespaces,
SyncPeriod: &syncPeriod,
ByObject: map[client.Object]cache.ByObject{
// Note: Only Secrets with the cluster name label are cached.
// The default client of the manager won't use the cache for secrets at all (see Client.Cache.DisableFor).
// The cached secrets will only be used by the secretCachingClient we create below.
&corev1.Secret{}: {
Label: clusterSecretCacheSelector,
},
},
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems ok, cc @sbueringer for a double check

Copy link
Contributor

@killianmuldoon killianmuldoon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/area documentation

@k8s-ci-robot k8s-ci-robot added area/documentation Issues or PRs related to documentation and removed do-not-merge/needs-area PR is missing an area label labels Oct 20, 2023
@lentzi90
Copy link
Contributor Author

/test pull-cluster-api-verify-main

@lentzi90 lentzi90 changed the title WIP: 📖 Document changes to BYO certificates and add tests 📖 Document changes to BYO certificates and add tests Nov 3, 2023
@lentzi90 lentzi90 marked this pull request as ready for review November 3, 2023 10:30
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 3, 2023
g.Expect(kcp.Status.Replicas).To(BeEquivalentTo(1))
g.Expect(conditions.IsFalse(kcp, controlplanev1.AvailableCondition)).To(BeTrue())

k, err := kubeconfig.FromSecret(ctx, env, util.ObjectKey(cluster))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Should we check if the KubeConfig is actually using the custom CA?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the huge delay...
I think it would be a good addition to check the CA, but I'm not sure how to do it. Could you give me some hint?
Do we check the default CA in any place so I could replicate that?
@fabriziopandini

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 28, 2024
@lentzi90 lentzi90 changed the title 📖 Document changes to BYO certificates and add tests 📖 Document changes to BYO certificates Feb 28, 2024
@lentzi90
Copy link
Contributor Author

I have removed the tests for now as I don't have time to work on them. Can we merge the documentation as is?

Copy link
Member

@chrischdi chrischdi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

I guess this should get cherry-picked?

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 29, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 9dec8433547f9afe2ee7cb1a97a56552ca0abe98

@lentzi90
Copy link
Contributor Author

Yes please!

@chrischdi
Copy link
Member

/cherry-pick release-1.6

@k8s-infra-cherrypick-robot

@chrischdi: once the present PR merges, I will cherry-pick it on top of release-1.6 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1.6

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.

Copy link
Member

@fabriziopandini fabriziopandini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for documenting this!

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini

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

The pull request process is described 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 approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 6, 2024
@fabriziopandini
Copy link
Member

/cherry-pick release-1-6

@k8s-infra-cherrypick-robot

@fabriziopandini: once the present PR merges, I will cherry-pick it on top of release-1-6 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1-6

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.

@fabriziopandini
Copy link
Member

/cherry-pick release-1-5

@k8s-infra-cherrypick-robot

@fabriziopandini: once the present PR merges, I will cherry-pick it on top of release-1-5 in a new PR and assign it to you.

In response to this:

/cherry-pick release-1-5

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 merged commit 5e3d813 into kubernetes-sigs:main Mar 6, 2024
31 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.7 milestone Mar 6, 2024
@k8s-infra-cherrypick-robot

@chrischdi: new pull request created: #10230

In response to this:

/cherry-pick release-1.6

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-infra-cherrypick-robot

@fabriziopandini: cannot checkout release-1-6: error checking out "release-1-6": exit status 1 error: pathspec 'release-1-6' did not match any file(s) known to git

In response to this:

/cherry-pick release-1-6

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-infra-cherrypick-robot

@fabriziopandini: cannot checkout release-1-5: error checking out "release-1-5": exit status 1 error: pathspec 'release-1-5' did not match any file(s) known to git

In response to this:

/cherry-pick release-1-5

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.

@fabriziopandini
Copy link
Member

/cherry-pick release-1.5

@k8s-infra-cherrypick-robot

@fabriziopandini: new pull request created: #10234

In response to this:

/cherry-pick release-1.5

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/documentation Issues or PRs related to documentation cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test and document bring-your-own-cert use cases for KCP
6 participants