Add draft README for Kubernetes Workload Registrar tutorial #59
Conversation
Signed-off-by: Luciano <[email protected]>
Signed-off-by: Luciano <[email protected]>
Signed-off-by: Luciano <[email protected]>
Signed-off-by: Luciano <[email protected]>
There was a problem hiding this comment.
Hi @lucianozablocki 👋 ! Here are some editing suggestions for the text in this tutorial.
I'll be on vacation July 5-9, 2021 but will respond to your questions after that.
-Steve
| ``` | ||
|
|
||
| # Webhook mode (default) | ||
|
|
There was a problem hiding this comment.
Yes, I left part of this change out so I can see why you are confused. I am proposing that we demote the descriptions of the modes under a new heading. So currently we have this outline (sorry, some of these names may be old):
Configure SPIRE to use the Kubernetes Workload Registrar
Prerequisites
Common configuration
Webhook mode (default)
Pod deletion
Teardown
Reconcile mode
Pod deletion
Non-labeled pods
Teardown
CRD mode
Pod deletion
Non-annotated pods
SPIFFE ID CRD creation
SPIFFE ID CRD deletion
Teardown
I'm proposing that we demote the three different modes under a new heading called "Configure workload registrar modes" like this:
Configure SPIRE to use the Kubernetes Workload Registrar
Prerequisites
Common configuration
Configure workload registrar modes
Webhook mode (default)
Pod deletion
Teardown
Reconcile mode
Pod deletion
Non-labeled pods
Teardown
CRD mode
Pod deletion
Non-annotated pods
SPIFFE ID CRD creation
SPIFFE ID CRD deletion
Teardown
So then those new short sentences I added would be inserted under the start of each mode heading.
However, I don't feel too strongly about this, so we can leave the outline as is if you prefer.
| ``` | ||
|
|
||
| # Webhook mode (default) | ||
|
|
There was a problem hiding this comment.
👍 Yes, I think it's better than my version 😃 . I have one request, though. Please include at least one sentence between each heading. For example, under the new heading "Configure Webhook mode" have a sentence like the one I put before: "This section describes the older, default webhook mode of the Kubernetes Workload Registrar." Then put the new "Run the registrar in Webhook mode" heading.
Signed-off-by: Luciano <[email protected]>
There was a problem hiding this comment.
Thank you @lucianozablocki! This round of changes is approved 🚀
That's great! Thanks for your work on this. |
There was a problem hiding this comment.
wow thanks for this @lucianozablocki! so much yaml 😆
I left a couple comments/suggestions, curious to hear your thoughts. Can't wait to get this in as several people have been asking for it!
| ``` | ||
|
|
||
| # Webhook mode (default) | ||
|
|
There was a problem hiding this comment.
Since we're not currently recommending that anyone use the webhook mode, I wonder if it would be best to put this one last. Or should we exclude it altogether?
| dnsPolicy: ClusterFirstWithHostNet | ||
| containers: | ||
| - name: example-workload | ||
| image: gcr.io/spiffe-io/spire-agent:0.12.0 |
There was a problem hiding this comment.
latest release is 1.0.1 and on sept 2 there will be 1.0.2
There was a problem hiding this comment.
Will update the versions used
| spec: | ||
| hostPID: true | ||
| hostNetwork: true | ||
| dnsPolicy: ClusterFirstWithHostNet |
There was a problem hiding this comment.
Since this is just an example workload, can we remove these three lines? The agent won't actually be running and attesting things so I don't think it's needed?
There was a problem hiding this comment.
Yep, a little too much copy-paste in here.
| else | ||
| docker cp "${PARENT_DIR}"/k8s/admctrl minikube:/var/lib/minikube/certs/ | ||
| minikube stop | ||
| minikube start \ |
There was a problem hiding this comment.
Hmm ... I don't think that minikube is necessarily required for the k8s quickstart? Looking at the website, it seems like all you need is any kubernetes.
The quickstart tests appear to use minikube though, but it's ok because the minikube commands live only in test.sh... but in this case, we're telling folks to call this script from the readme so if they're not using minikube they may run into a problem.
There was a problem hiding this comment.
Thinking about this more, we should probably lean towards straight kubectl commands in the readme the same way that the quickstart does. To make this easier, we can combine the yaml's into a single file where it makes sense so there are fewer commands to run. What do you think?
There was a problem hiding this comment.
Even if all the necessary yaml's are combined into one, there's still the need for:
- start the containers in order (server, then agent, then workload). How can we ensure this specific order if only one yaml is applied?
- place the admission-control.yaml file into the k8s node. How can we do that without assuming a certain local kubernetes tool?
For instance, I see some bash scripts used in the envoy tutorials as well, so why don't use a schema like that on this tutorial?
Haha, yaml will save us all! |
Signed-off-by: Luciano [email protected]