spiffe / spiffe-csi

Container Storage Interface components for SPIFFE
Apache License 2.0
53 stars 21 forks source link

Add PVC for spire-server in spiffe-csi example #135

Closed abe-hpe closed 5 months ago

abe-hpe commented 11 months ago

Added a PVC to the spire-server deployment in order to persist the spire server data store (especially node and workload registrations) across pod restarts. This would fail if the target cluster does not have a default storage class, and we could handle that with some logic in the deploy-spire-and-csi-driver.sh script, but in the majority of cases, storage will be available, and certainly the spire-server needs persistent storage in any real-world use case. So I think it's fair to expect a PVC creation to succeed, for this example.

azdagron commented 11 months ago

The examples in the this repository should be focused on how to utilize the SPIFFE CSI Driver to provide the Workload API socket (e.g. from SPIRE Agent) into a Workload Pod. They aren't intended as a jumping off point for installing SPIRE into a K8s cluster. There are other projects much better suited for that (e.g. the helm repos)

In that spirit, I feel that this change might be introducing more complexity that might divert attention away from the purpose of the example. I'd rather keep the SPIRE server deployment in the example bare-bones and focus instead on the SPIFFE CSI Driver integration with the SPIRE agent.

If we're still worried about it, I think what might be helpful is a big comment on the SPIRE server yaml that says something to the effect of "This is not a real-world deployment of the SPIRE Server, but rather just enough to get a basic instance up and running to demonstrate the functionality offered by the SPIFFE CSI Driver. For a more in-depth example of a SPIRE deployment in Kubernetes, see [blah], [blah], and [blah]."

abe-hpe commented 11 months ago

Thanks for checking it, @edwbuck and @azdagron . I take your points and it definitely wasn't my intention to turn this example into something that people might try and use as a production SPIRE deployment. But when using the example for testing (and frequently restarting spire-server) I found it helpful to have /run/spire/data on a PV so that I didn't have to recreate the workload registrations each time. It also brings this example into line with the non-CSI stateful set example at https://github.com/spiffe/spire-examples/tree/main/examples/k8s/simple_sat, which has persistent storage for spire-server.

A detailed comment in the spire-server yaml to explain all this and warn about having multiple replicas of spire-server using the same (or different!) data stores sounds like a good idea. I'll update this PR shortly to include that.

edwbuck commented 11 months ago

Thanks for checking it, @edwbuck and @azdagron . I take your points and it definitely wasn't my intention to turn this example into something that people might try and use as a production SPIRE deployment. But when using the example for testing (and frequently restarting spire-server) I found it helpful to have /run/spire/data on a PV so that I didn't have to recreate the workload registrations each time. It also brings this example into line with the non-CSI stateful set example at https://github.com/spiffe/spire-examples/tree/main/examples/k8s/simple_sat, which has persistent storage for spire-server.

A detailed comment in the spire-server yaml to explain all this and warn about having multiple replicas of spire-server using the same (or different!) data stores sounds like a good idea. I'll update this PR shortly to include that.

Would there be an advantage to installing a quick postgresql database for your environment? It matches the SPIRE recommendations much better than using SQLite. I can provide the commands and assistance if you desire.

azdagron commented 5 months ago

I'm going to close out this PR for now. If you want to re-open and add the comment we discussed, please feel free to do so.