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

csiaddons: add support for TLS #172

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

bipuladh
Copy link

@bipuladh bipuladh commented Nov 13, 2024

Description

This PR introduces a controller to approve CSR requests intended for Ceph CSI TLS communications.

Context

To have a secure gRPC communication between the CSI add-ons sidecar and CSI manager we need to create certificates that can be verified by a CA. We use the k8s CSR resource to sign our certificates. Related to: csi-addons/kubernetes-csi-addons#692

Is the change backward compatible?

No

Are there concerns around backward compatibility?

We will move ahead with disabled by default approach so that it doesn't introduce breakages.

Provide any external context for the change, if any.

For example:

  • Kubernetes links that explain why the change is required
  • Ceph-CSI spec related changes/catch-up that necessitates this patch
  • golang related practices that necessitates this change

Checklist:

  • Commit Message Formatting: Commit titles and messages follow
    guidelines in the developer
    guide
    .
  • Reviewed the developer guide on Submitting a Pull
    Request
  • Pending release
    notes

    updated with breaking and/or notable changes for the next major release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

@bipuladh bipuladh changed the title [WIP] csiaddons: add support for TLS csiaddons: add support for TLS Nov 13, 2024
Adds required permissions

Signed-off-by: Bipul Adhikari <[email protected]>
@@ -0,0 +1,132 @@
/*
Copy link
Author

Choose a reason for hiding this comment

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

As discussed with the team the CSI operator should not be responsible for generation of certificates. ODF operators can generate these certificates.

@leelavg
Copy link
Contributor

leelavg commented Nov 20, 2024

pls excuse I partly understood what the code is doing but don't get the "why". Why should csi-op behave like a trimmed down CertificateAuthority? A CA has much more functionalities like rotation, revocation to name a few in addition to a lot of options in x509 cert.

btw, I was OOO during some of csi-op meetings, pls do let me know if this PR premise was already discussed based on #172 (comment) and this PR is being reworked/removed. thanks!

Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in two weeks if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants