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

Cert rotation for e2e chart #145

Merged

Conversation

jsturtevant
Copy link
Contributor

@jsturtevant jsturtevant commented Mar 13, 2024

Follow up to #141 to add e2e tests

closes #135

This adds e2e tests for cert rotation using Cert-manager. The general process is the same when using the manual process but I an not going to add those since this tests the functionally of the rotation in the webhook which will be the same no matter how the cert is created/rotated.

This does add root-ca for cert-manager deployment, so that intermediate certs can be signed and issued, while the old one can still be validated. In a real deployment it would be be advised to use a CA issuer for you PKI infrastructure but this enables us to boot strap it here.

Read more about this here https://cert-manager.io/docs/configuration/selfsigned/ and here https://cert-manager.io/docs/usage/certificate/#issuance-behavior-rotation-of-the-private-key

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. sig/windows Categorizes an issue or PR as relevant to SIG Windows. labels Mar 13, 2024
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 13, 2024
@jsturtevant jsturtevant force-pushed the cert-rotation-e2e-chart branch from ba0310c to 2dd1e8a Compare March 14, 2024 21:30
@jsturtevant jsturtevant marked this pull request as ready for review March 14, 2024 23:29
@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 Mar 14, 2024
@jsturtevant
Copy link
Contributor Author

/assign @marosset @aravindhp @ycheng-kareo

@k8s-ci-robot
Copy link
Contributor

@jsturtevant: GitHub didn't allow me to assign the following users: ycheng-kareo.

Note that only kubernetes-sigs members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @marosset @aravindhp @ycheng-kareo

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.

Signed-off-by: James Sturtevant <jsturtevant@gmail.com>
Signed-off-by: James Sturtevant <jsturtevant@gmail.com>
Signed-off-by: James Sturtevant <jsturtevant@gmail.com>
Signed-off-by: James Sturtevant <jsturtevant@gmail.com>
@jsturtevant jsturtevant force-pushed the cert-rotation-e2e-chart branch from 55b69e4 to b301883 Compare March 15, 2024 18:36
Signed-off-by: James Sturtevant <jsturtevant@gmail.com>
@jsturtevant jsturtevant force-pushed the cert-rotation-e2e-chart branch from b301883 to 0ab2972 Compare March 15, 2024 20:35
@jsturtevant
Copy link
Contributor Author

/cc @ycheng-kareo
This has a change to the loading which let the updates be seen to the webhook that you will likely run into. When the secret propagated the flags we were checking were not being updated. I've updated it to take any change to the file and reload.

@k8s-ci-robot
Copy link
Contributor

@jsturtevant: GitHub didn't allow me to request PR reviews from the following users: ycheng-kareo.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @ycheng-kareo
This has a change to the loading which let the updates be seen to the webhook that you will likely run into. When the secret propagated the flags we were checking were not being updated. I've updated it to take any change to the file and reload.

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.

@ycheng-kareo
Copy link
Contributor

/cc @ycheng-kareo This has a change to the loading which let the updates be seen to the webhook that you will likely run into. When the secret propagated the flags we were checking were not being updated. I've updated it to take any change to the file and reload.

thanks @jsturtevant! very helpful to see the code changes you made

@jsturtevant jsturtevant requested a review from aravindhp March 19, 2024 14:48
Copy link
Contributor

@marosset marosset left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jsturtevant, marosset

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:
  • OWNERS [jsturtevant,marosset]

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 merged commit f4d9b49 into kubernetes-sigs:master Mar 21, 2024
11 checks passed
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. 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. sig/windows Categorizes an issue or PR as relevant to SIG Windows. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Certificate renewal requires a restart of the gmsa pods
5 participants