cmd/k8s-operator: generate static kube manifests from the Helm chart. #10436

irbekrm · 2023-12-01T12:11:09Z

Generate static kube manifests for the Tailscale operator from the Helm chart to ensure a single source of truth.

The workflow for adding new functionality for the manifests will now be:

add the changes to ./cmd/k8s-operator/deploy/charts
run go generate tailscale.com/cmd/k8s-operator to ensure the change is propagated to the static manifests
commit all changes

This PR also adds a CI test to ensure that the static manifests are up to date with regards to the chart.

I have tested the generated manifest by setting the client id and client secret fields of the Secret data and applying it to a cluster.

irbekrm · 2023-12-01T12:14:33Z

cmd/k8s-operator/generate/main.go

+	finalString, _ := strings.CutSuffix(final.String(), "---\n")
+	if err := os.WriteFile(filepath.Join(repoRoot, "cmd/k8s-operator/deploy/manifests/operator.yaml"), []byte(finalString), 0664); err != nil {
+		log.Fatalf("error writing new file: %v", err)
+	}


I was looking for some way how to validate that the generated manifest is a valid Kubernetes manifest, however it looks like kubectl apply --dry-run still requires a running cluster and marshaling the yamls into kube types in Go would be an overkill so I think this is fine for now.

Would a sensible alternative be running something like kubectl-validate or kubeconform as a Github action when operator.yaml gets changed?

I was considering adding a validating kubectl plugin like this, yes. But I was a bit reluctant to add two more tools (kubectl and the validating plugin) that are kube-specific to ./tools (more dependencies, running another third party tool in our test env). We will though need kubectl anyway for the e2e tests. Perhaps, we could first get the kubectl in in a separate PR to make it more explicit in case folks have objections to adding more tools and then look at whether we want to add more plugins. I am not particularly concerned about generating invalid yaml right now- we are not planning to make big changes to the manifests and unlike the helm templates, the static manifests are quite readable so should be easy to evaluate a change with 👀

This is done to ensure that there is a single source of truth for the operator kube manifests. Also adds linux node selector to the static manifests as this was added as a default to the Helm chart. Static manifests can now be generated by running go generate tailscale.com/cmd/k8s-operator. Updates #9222 Signed-off-by: Irbe Krumina <irbe@tailscale.com>

Also ensure that the check is only ran when files in ./cmd/k8s-operator or the test have changed. Signed-off-by: Irbe Krumina <irbe@tailscale.com>

irbekrm · 2023-12-01T12:45:28Z

cmd/k8s-operator/deploy/manifests/operator.yaml

+          value: "false"
+        - name: PROXY_FIREWALL_MODE
+          value: auto
+        image: tailscale/k8s-operator:unstable


Probably at some point we will want to hardcode this to 'latest' tag instead, but not in this PR.

And/or we might publish the manifest at a specific tag with operator and proxy tags set accordingly to Github releases

irbekrm · 2023-12-01T12:47:20Z

cmd/k8s-operator/deploy/manifests/operator.yaml

  name: tailscale
 ---
 apiVersion: v1
-kind: ServiceAccount


Changes to this file are:

resource definitions being swapped around due to the order of the Helm templates

things actually being brought up to date from changes in the Helm templates:

node selector set to 'linux'

image pull policy set to 'Always'

.github/workflows/kubemanifests.yaml

knyar · 2023-12-03T19:00:51Z

cmd/k8s-operator/deploy/README.md

@@ -0,0 +1,12 @@
+# Tailscale Kubernetes operator deployment manifests
+
+./cmd/k8s-operator/deploy contain various Tailscale Kubernetes operator deployment manifests.


Any thoughts on whether cmd/k8s-operator is the best place for these? It might not be too late to move them to a top-level directory.

It might not be too late to move them to a top-level directory

I would first publish them to a Github releases and tell people to consume it from there, after that we can move them around in the repo as we see fit.

But I want to get the charts work in first.

knyar · 2023-12-03T23:23:52Z

cmd/k8s-operator/generate/main.go

+	cmd := exec.Command("./tool/helm", "template", "operator", "./cmd/k8s-operator/deploy/chart",
+		"--namespace=tailscale")
+	cmd.Dir = repoRoot
+	out := bytes.NewBuffer([]byte{})


Is there a reason to create a buffer this way as opposed to declaring a variable, e.g. var out bytes.Buffer?

It is *bytes.Buffer that implements io.Writer and var out *bytes.Buffer would get initialized to nil so would need some value assigned to it anyway, so I think out := bytes.NewBuffer([]byte{}) is preferable here
I need more coffee. Yeah could do var out bytes.Buffer

cmd/k8s-operator/generate/main.go

knyar · 2023-12-04T00:01:49Z

cmd/k8s-operator/generate/main.go

+	finalString, _ := strings.CutSuffix(final.String(), "---\n")
+	if err := os.WriteFile(filepath.Join(repoRoot, "cmd/k8s-operator/deploy/manifests/operator.yaml"), []byte(finalString), 0664); err != nil {
+		log.Fatalf("error writing new file: %v", err)
+	}


Would a sensible alternative be running something like kubectl-validate or kubeconform as a Github action when operator.yaml gets changed?

Signed-off-by: Irbe Krumina <irbe@tailscale.com>

irbekrm · 2023-12-04T10:06:29Z

cmd/k8s-operator/deploy/manifests/operator.yaml

 metadata:
-  name: operator
-  namespace: tailscale
+    name: operator


gopkg.in/yaml.v3 has opinions wrt tabs vs spaces

irbekrm changed the title ~~cmd/k8s-operator: generate static kube manifests from Helm charts.~~ cmd/k8s-operator: generate static kube manifests from the Helm chart. Dec 1, 2023

irbekrm commented Dec 1, 2023

View reviewed changes

irbekrm force-pushed the irbekrm/generatestaticmanifests branch from be5024e to 31a0775 Compare December 1, 2023 12:23

irbekrm added 2 commits December 1, 2023 12:35

.github/workflows: check that static kube manifests are up to date.

ec9faa5

Also ensure that the check is only ran when files in ./cmd/k8s-operator or the test have changed. Signed-off-by: Irbe Krumina <irbe@tailscale.com>

irbekrm force-pushed the irbekrm/generatestaticmanifests branch from 31a0775 to ec9faa5 Compare December 1, 2023 12:36

irbekrm commented Dec 1, 2023

View reviewed changes

irbekrm requested review from knyar and maisem December 1, 2023 12:53

irbekrm mentioned this pull request Dec 1, 2023

cmd/k8s-operator: operator can create subnetrouter #9505

Merged

knyar approved these changes Dec 4, 2023

View reviewed changes

code review feedback

4df8d3f

Signed-off-by: Irbe Krumina <irbe@tailscale.com>

irbekrm commented Dec 4, 2023

View reviewed changes

irbekrm merged commit 49fd0a6 into main Dec 4, 2023

irbekrm deleted the irbekrm/generatestaticmanifests branch December 4, 2023 10:18

		@@ -0,0 +1,12 @@
		# Tailscale Kubernetes operator deployment manifests

		./cmd/k8s-operator/deploy contain various Tailscale Kubernetes operator deployment manifests.

cmd/k8s-operator: generate static kube manifests from the Helm chart. #10436

cmd/k8s-operator: generate static kube manifests from the Helm chart. #10436

Uh oh!

Conversation

irbekrm commented Dec 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

irbekrm Dec 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

irbekrm Dec 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

irbekrm commented Dec 1, 2023 •

edited

Loading

irbekrm Dec 4, 2023 •

edited

Loading

irbekrm Dec 4, 2023 •

edited

Loading