build-container-helm-chart-flux-demo by g-bgg · Pull Request #41

build-container-helm-chart-flux-demo by g-bgg · Pull Request #41 · controlplaneio/netassert

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff
A few thoughts & suggestions

Also should we have a workflow that ensures the chart and/or the demo chart apply properly?
Can be in a future PR

Comment on lines +38 to +40

		SnifferContainerImage: "docker.io/controlplane/netassertv2-packet-sniffer:v1.1.7",
		SnifferContainerPrefix: "netassertv2-sniffer",
		ScannerContainerImage: "docker.io/controlplane/netassertv2-l4-client:latest",
		ScannerContainerImage: "docker.io/controlplane/netassertv2-l4-client:v1.0.6",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're committing this can we have a simple bash script or something to update this?
Also we could use digest sha

		spec:
		initContainers:
		- name: "sleepy"
		image: busybox:1.36

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably best to use a full image path. I recall seeing a post about CRI-O requiring full path soon but I can't find the link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also if these images are defined in helm values then there's only 1 place to update them.

		spec:
		containers:
		- name: busybox
		image: busybox

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for this image

Comment on lines +71 to +73

		command:
		- sleep
		- "360000"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could probably follow the same pattern as above sleepy

		spec:
		containers:
		- name: webserver
		image: nginx:latest

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image path


		---

		### 1.2 Create a Kind Cluster

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

	### 1.2 Create a Kind Cluster
	### 1.2 Create a KinD Cluster

		- name: Push Helm chart to GHCR
		run: \|
		CLEAN_VERSION=$(echo "$RELEASE_VERSION" \| sed 's/^v//')
		helm push "./netassert-${CLEAN_VERSION}.tgz" oci://ghcr.io/${{ github.repository_owner }}/charts No newline at end of file

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing newline EOF

		attestations: write
		id-token: write
		steps:
		- name: Checkout repository

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can some spacing be added between each step for readability please

		env:
		GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

		packagerelease:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

	packagerelease:
	helm-release:

	packagerelease:
	container-and-helm:

		build:
		runs-on: ubuntu-latest
		needs: lint
		#needs: lint

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we re-enable linting here or bring it back in a separate workflow?