-
Notifications
You must be signed in to change notification settings - Fork 33
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
Add basic CSI driver for the spiffe-helper #166
Draft
kfox1111
wants to merge
22
commits into
main
Choose a base branch
from
helper-csi
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 15 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
d16baa6
Add basic CSI driver for the spire-helper
kfox1111 10921ee
Remove extra comments
kfox1111 fb7ac8a
Fix tests
kfox1111 f3e9594
Update docs and examples
kfox1111 3a8e681
Merge branch 'main' into helper-csi
kfox1111 8cac62c
Merge branch 'main' into helper-csi
kfox1111 c5fcf4f
Merge branch 'main' into helper-csi
kfox1111 7dd4227
Merge branch 'main' into helper-csi
kfox1111 fcce05f
Merge branch 'main' into helper-csi
kfox1111 32edfb5
Merge branch 'main' into helper-csi
kfox1111 72fd539
Merge branch 'main' into helper-csi
kfox1111 f3ce5b2
Merge branch 'main' into helper-csi
kfox1111 ac54cb4
Merge branch 'main' into helper-csi
kfox1111 07641df
Merge branch 'main' into helper-csi
kfox1111 9cae837
Merge branch 'main' into helper-csi
kfox1111 ba13166
Update charts/spiffe-helper-csi-driver/values.yaml
kfox1111 57d9c75
Merge branch 'main' into helper-csi
kfox1111 85ac35d
Merge remaining bits from main
kfox1111 9510af9
Merge branch 'main' into helper-csi
kfox1111 702cb03
Merge branch 'main' into helper-csi
kfox1111 4aa44f6
Update charts/spiffe-helper-csi-driver/README.md
kfox1111 8708a23
Merge branch 'main' into helper-csi
kfox1111 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
# Patterns to ignore when building packages. | ||
# This supports shell glob matching, relative path matching, and | ||
# negation (prefixed with !). Only one pattern per line. | ||
.DS_Store | ||
# Common VCS dirs | ||
.git/ | ||
.gitignore | ||
.bzr/ | ||
.bzrignore | ||
.hg/ | ||
.hgignore | ||
.svn/ | ||
# Common backup files | ||
*.swp | ||
*.bak | ||
*.tmp | ||
*.orig | ||
*~ | ||
# Various IDEs | ||
.project | ||
.idea/ | ||
*.tmproj | ||
.vscode/ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
apiVersion: v2 | ||
name: spiffe-helper-csi-driver | ||
description: > | ||
A Helm chart for deploying the Spiffe Helper CSI Drier | ||
type: application | ||
version: 0.1.0 | ||
appVersion: "0.0.1" | ||
keywords: ["spiffe-helper-csi-driver"] | ||
home: https://github.com/spiffe/helm-charts/tree/main/charts/spire | ||
sources: | ||
- https://github.com/spiffe/helm-charts/tree/main/charts/spire | ||
icon: https://spiffe.io/img/logos/spire/icon/color/spire-icon-color.png | ||
maintainers: | ||
- name: marcofranssen | ||
email: [email protected] | ||
url: https://marcofranssen.nl | ||
- name: kfox1111 | ||
email: [email protected] | ||
- name: faisal-memon | ||
email: [email protected] | ||
- name: edwbuck | ||
email: [email protected] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
# spire-helper-csi-driver | ||
|
||
![Version: 0.1.0](https://img.shields.io/badge/Version-0.1.0-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: 1.7.2](https://img.shields.io/badge/AppVersion-1.7.2-informational?style=flat-square) | ||
|
||
A Helm chart to install the SPIFFE HELPER CSI Driver. | ||
kfox1111 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
**Homepage:** <https://github.com/spiffe/helm-charts/tree/main/charts/spire> | ||
|
||
## Maintainers | ||
|
||
| Name | Email | Url | | ||
| ---- | ------ | --- | | ||
| marcofranssen | <[email protected]> | <https://marcofranssen.nl> | | ||
| kfox1111 | <[email protected]> | | | ||
| faisal-memon | <[email protected]> | | | ||
| edwbuck | <[email protected]> | | | ||
|
||
## Source Code | ||
|
||
* <https://github.com/spiffe/helm-charts/tree/main/charts/spiffe-helpe-csi-driver> | ||
|
||
## Prereqs: | ||
|
||
Your cluster needs to have Kyverno installed. You can do that by running something like the following: | ||
|
||
``` | ||
helm upgrade --install --create-namespace kyverno kyverno -n kyverno --repo https://kyverno.github.io/kyverno/ --version 3.1.1 | ||
kfox1111 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
``` | ||
|
||
You also need SPIRE installed. You can do that by running something like the following for a non production test cluster: | ||
|
||
``` | ||
helm install -n spire-server spire-crds spire-crds --repo https://spiffe.github.io/helm-charts-hardened/ --create-namespace | ||
helm install -n spire-server spire spire --repo https://spiffe.github.io/helm-charts-hardened/ | ||
``` | ||
|
||
## Install Instructions | ||
``` | ||
helm install -n spire-server spiffe-helper-csi-driver charts/spiffe-helper-csi-driver | ||
``` | ||
|
||
## Example usage | ||
|
||
See the examples/good directory for different ways of using the driver. | ||
|
||
<!-- The parameters section is generated using helm-docs.sh and should not be edited by hand. --> | ||
|
||
## Parameters |
50 changes: 50 additions & 0 deletions
50
charts/spiffe-helper-csi-driver/examples/bad/test-pod.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
apiVersion: v1 | ||
kind: Pod | ||
metadata: | ||
name: bad-test | ||
spec: | ||
# shareProcessNamespace: true | ||
# hostPID: true | ||
containers: | ||
- name: nginx | ||
image: nginx | ||
command: | ||
- /bin/sh | ||
- -c | ||
- | | ||
echo $$$$ > $$SPIFFE_HELPER_PID_CERTS | ||
cat > /etc/nginx/conf.d/ssl.conf <<EOF | ||
server { | ||
listen 443 ssl; | ||
server_name localhost; | ||
ssl_certificate /certs/tls.crt; | ||
ssl_certificate_key /certs/tls.key; | ||
location / { | ||
root /usr/share/nginx/html; | ||
index index.html index.htm; | ||
} | ||
error_page 500 502 503 504 /50x.html; | ||
location = /50x.html { | ||
root /usr/share/nginx/html; | ||
} | ||
} | ||
EOF | ||
exec nginx -g "daemon off;" | ||
volumeMounts: | ||
- name: certs | ||
mountPath: /certs | ||
ports: | ||
- containerPort: 443 | ||
volumes: | ||
- name: certs | ||
csi: | ||
driver: helper.spiffe.io | ||
volumeAttributes: | ||
renewSignal: SIGHUP | ||
pidContainer: nginx | ||
|
||
addIntermediatesToBundle: "true" | ||
|
||
svidFileName: tls.crt | ||
svidKeyFileName: tls.key | ||
svidBundleFileName: ca.pem |
34 changes: 34 additions & 0 deletions
34
charts/spiffe-helper-csi-driver/examples/good/custom-sidecar-pod.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
# NOTE: | ||
# This example shows how you can use a custom image for your sidecar | ||
# enabling you to run specific commands whenever a certificate is updated. | ||
apiVersion: v1 | ||
kind: Pod | ||
metadata: | ||
name: custom-sidecar | ||
spec: | ||
containers: | ||
- name: busybox | ||
image: busybox | ||
command: | ||
- /bin/sh | ||
- -c | ||
- 'while true; do sleep 1000; done' | ||
volumeMounts: | ||
- name: certs | ||
mountPath: /certs | ||
- name: sidecar | ||
image: mysql | ||
env: | ||
- name: some-setting | ||
value: foo | ||
volumes: | ||
- name: certs | ||
csi: | ||
driver: helper.spiffe.io | ||
volumeAttributes: | ||
customSidecar: sidecar | ||
cmd: "bash" | ||
cmdArgs: "-c \"echo rolled. Could do a mysql cli command here.\"" | ||
svidBundleFileName: ca.pem | ||
svidFileName: tls.crt | ||
svidKeyFileName: tls.key |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
apiVersion: v1 | ||
kind: Pod | ||
metadata: | ||
name: nginx | ||
spec: | ||
shareProcessNamespace: true | ||
containers: | ||
- name: nginx | ||
image: nginx | ||
command: | ||
- /bin/sh | ||
- -c | ||
- | | ||
echo $$$$ > $$SPIFFE_HELPER_PID_CERTS | ||
cat > /etc/nginx/conf.d/ssl.conf <<EOF | ||
server { | ||
listen 443 ssl; | ||
server_name localhost; | ||
ssl_certificate /certs/tls.crt; | ||
ssl_certificate_key /certs/tls.key; | ||
location / { | ||
root /usr/share/nginx/html; | ||
index index.html index.htm; | ||
} | ||
error_page 500 502 503 504 /50x.html; | ||
location = /50x.html { | ||
root /usr/share/nginx/html; | ||
} | ||
} | ||
EOF | ||
exec nginx -g "daemon off;" | ||
volumeMounts: | ||
- name: certs | ||
mountPath: /certs | ||
ports: | ||
- containerPort: 443 | ||
volumes: | ||
- name: certs | ||
csi: | ||
driver: helper.spiffe.io | ||
volumeAttributes: | ||
renewSignal: SIGHUP | ||
pidContainer: nginx | ||
|
||
addIntermediatesToBundle: "true" | ||
|
||
svidFileName: tls.crt | ||
svidKeyFileName: tls.key | ||
svidBundleFileName: ca.pem |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not slip in a policy engine as an out-of-scope "extra" on a commit intended to add a CSI driver.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The whole hook is implemented as a kyverno mutating policy. Its needed to make it work at all.
Totally open to it needing its own golang based webhook implementation in the future, along with associated git repositories, a container, etc, to make it not depend on kyverno, but was looking at getting an implementation of the concept working in as minimal amount of time possible, and that lead to using an existing webhook engine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The expansion of the charts to include Kyverno is something that probably should be discussed with a majority of the maintainers. I understand that it's part of this implementation, but its inclusion should be done with consensus, and at least some commentary on what the alternatives might be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Completely agree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I 100% believe Kyverno is NOT the right solution in the long run.
The right place for it though is not clear. There are several potential places
I also don't think we should block users playing around with the concept and getting the api correct so that when we do identify a better way of implementing that api, we can switch out the kyverno based implementation with a better one without affecting users of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For what it is worth, in our own clusters we manage Kyverno ourself, meaning I probably need a way to disable it here, for the remainder I understand the reason why you are adding it now.
Probably it should be something managed in the controller itself to have the webhook implemented there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes use of Kyverno via a policy in the chart, but it doesn't install Kyvverno other then in the tests, to test the policy.
Longer term, it will be switched to kubernetes/kubernetes#127134 once it is a thing.