Add draft README for Kubernetes Workload Registrar tutorial by lucianozablocki · Pull Request #59 · spiffe/spire-tutorials

lucianozablocki · 2021-03-23T19:14:39Z

Signed-off-by: Luciano lucianozablocki@gmail.com

Signed-off-by: Luciano <lucianozablocki@gmail.com>

sanderson042

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

k8s/k8s-workload-registrar/README.md

sanderson042

Hi Luciano 👋 - here are some responses to your comments.

k8s/k8s-workload-registrar/README.md

sanderson042 · 2021-07-29T00:55:01Z

k8s/k8s-workload-registrar/README.md

+```
+
+# Webhook mode (default)
+


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.

sanderson042

Hi Luciano - we are close to finishing! (This iteration. 😄 )

k8s/k8s-workload-registrar/README.md

sanderson042 · 2021-07-30T22:40:23Z

k8s/k8s-workload-registrar/README.md

+```
+
+# Webhook mode (default)
+


👍 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 <lucianozablocki@gmail.com>

sanderson042

Thank you @lucianozablocki! This round of changes is approved 🚀

k8s/k8s-workload-registrar/README.md

lucianozablocki · 2021-08-02T23:47:06Z

Thank you @lucianozablocki! This round of changes is approved rocket

That's great! Thanks for your work on this.

evan2645

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!

evan2645 · 2021-08-26T20:21:50Z

k8s/k8s-workload-registrar/README.md

+```
+
+# Webhook mode (default)
+


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?

evan2645 · 2021-08-26T20:29:13Z

k8s/k8s-workload-registrar/mode-crd/k8s/not-annotated-workload.yaml

+      dnsPolicy: ClusterFirstWithHostNet
+      containers:
+        - name: example-workload
+          image: gcr.io/spiffe-io/spire-agent:0.12.0


latest release is 1.0.1 and on sept 2 there will be 1.0.2

Will update the versions used

evan2645 · 2021-08-26T20:30:52Z

k8s/k8s-workload-registrar/mode-crd/k8s/not-annotated-workload.yaml

+    spec:
+      hostPID: true
+      hostNetwork: true
+      dnsPolicy: ClusterFirstWithHostNet


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?

Yep, a little too much copy-paste in here.

evan2645 · 2021-08-27T17:05:14Z

k8s/k8s-workload-registrar/mode-webhook/scripts/deploy-scenario.sh

+else
+    docker cp "${PARENT_DIR}"/k8s/admctrl minikube:/var/lib/minikube/certs/
+    minikube stop
+    minikube start \


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.

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?

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?

lucianozablocki · 2021-09-01T22:12:26Z

wow thanks for this @lucianozablocki! so much yaml laughing

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!
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?

Haha, yaml will save us all!
In case someone is willing to learn about the default mode, I don't see it bad to include it in the final version. That said, I agree with you that it must be moved at the last part of it. If you agree with including it, I will place it as the last reviewed mode.

add draft readme for k8s workload registrar tutorial

72e8202

Signed-off-by: Luciano <lucianozablocki@gmail.com>

Andres-GC assigned lucianozablocki Mar 25, 2021

lucianozablocki added 3 commits April 14, 2021 15:43

checkpoint

d7e9a62

Signed-off-by: Luciano <lucianozablocki@gmail.com>

add tests and modify readme

5b97785

Signed-off-by: Luciano <lucianozablocki@gmail.com>

switch to minikube

efcb6c2

Signed-off-by: Luciano <lucianozablocki@gmail.com>

sanderson042 reviewed Jul 2, 2021

View reviewed changes

sanderson042 reviewed Jul 29, 2021

View reviewed changes

sanderson042 reviewed Jul 30, 2021

View reviewed changes

address PR comments

76a6368

Signed-off-by: Luciano <lucianozablocki@gmail.com>

sanderson042 previously approved these changes Aug 2, 2021

View reviewed changes

k8s/k8s-workload-registrar/README.md Outdated Show resolved Hide resolved

lucianozablocki marked this pull request as ready for review August 2, 2021 23:55

sanderson042 requested a review from evan2645 August 2, 2021 23:59

evan2645 reviewed Aug 27, 2021

View reviewed changes

sanderson042 mentioned this pull request Jan 10, 2022

Provide installation steps for the k8s-workload-registrar (+ webhook example in quick start) spiffe/spire#2659

Closed

MarcosDY dismissed sanderson042’s stale review via 76a6368 May 2, 2022 18:37

Conversation

lucianozablocki commented Mar 23, 2021

Uh oh!

sanderson042 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sanderson042 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanderson042 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sanderson042 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lucianozablocki commented Aug 2, 2021

Uh oh!

evan2645 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucianozablocki commented Sep 1, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants