open-cluster-management-io / registration

hub / spoke registration controllers
Apache License 2.0
42 stars 58 forks source link

client certificate expiration seconds must greater or qual to 3600 #320

Closed youhangwang closed 1 year ago

youhangwang commented 1 year ago

In PR: https://github.com/open-cluster-management-io/registration/pull/312, a option --client-cert-expiration-seconds was exposed so that a user can specify a expiration seconds for the client cert. And we use the minimum value of expiration time in CSR - 10min to check if the expiration time is validated.

However, for the clientCertificateController in registration, 600 is too short:

  1. clientCertificateController to check if cert is expired every 5min.
  2. kubeclient resync the client cert every 5min.
  3. clientCertificateController will refresh the cert if the cert has less than a random percentage range from 20% to 25% of its life remaining.

so the minimum expiration second would be (5 * 60)/(1/5) = 1500s for clientCertificateController. otherwise, the kube client will use a expired client cert to send request to the remote cluster.

Considering to add some margin. 1h(3600s) could be a good choice.

openshift-ci[bot] commented 1 year ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: youhangwang Once this PR has been reviewed and has the lgtm label, please assign deads2k for approval. For more information see the Kubernetes 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: - **[OWNERS](https://github.com/open-cluster-management-io/registration/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
skeeey commented 1 year ago

@youhangwang

currently, we are merging our components to a single repo (https://github.com/open-cluster-management-io/registration#note-do-not-edit-any-file-in-this-repo), this repo will not accept any new code change, please open new pr in the ocm repo

thanks a lot

youhangwang commented 1 year ago

@skeeey oh, thanks, create a new PR in https://github.com/open-cluster-management-io/ocm/pull/149