Skip to content

Refactor k8s deployment and helm files - #733

Open
mkl262 wants to merge 2 commits into
k8snetworkplumbingwg:masterfrom
mkl262:master
Open

Refactor k8s deployment and helm files#733
mkl262 wants to merge 2 commits into
k8snetworkplumbingwg:masterfrom
mkl262:master

Conversation

@mkl262

@mkl262 mkl262 commented Jul 18, 2026

Copy link
Copy Markdown

What this PR does / why we need it:

  • Consolidate all Kubernetes deployment resources into a single deploy/directory with a clearer layout:

    • deploy/crds/ — CRD manifests (moved from doc/crds/)
    • deploy/manifests/ — raw manifests split from the monolithic daemonset-install.yaml into rbac, daemonset, reconciler, and node-slice-controller files
    • deploy/charts/whereabouts/ — Helm chart (moved from deployment/whereabouts-chart/, renamed to match chart name)
  • Lint All yaml files

  • Restructure Helm chart templates to mirror the raw manifests, add cross-reference comments between manifest and chart files, and apply

  • Update README, CLAUDE.md, and hack scripts (e2e setup, codegen,chart release) to reference the new paths.

Which issue(s) this PR fixes:

Special notes for your reviewer (optional):
!IMPORTANT! changes to selector labels will cause direct upgrade to fail, requiring deleting deployments and daemonset crate by helm/ manifests.

Summary by CodeRabbit

  • New Features
    • Added Helm/Kubernetes resources for the Whereabouts DaemonSet, reconciler, and optional node-slice controller.
    • Introduced new CustomResourceDefinitions for IPPool, NodeSlicePool, and OverlappingRangeIPReservation.
    • Expanded Helm configuration for image, namespace, resources, scheduling, and reconciliation/cron behavior.
  • Documentation
    • Updated installation guidance to use the new manifest set and corrected manifest references.
  • Chores
    • Added repository-wide YAML linting support and normalized CI workflow/YAML formatting.
    • Updated Helm chart metadata/version to 0.9.4.

@mkl262
mkl262 requested review from dougbtv and maiqueb as code owners July 18, 2026 19:59
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The change relocates Kubernetes manifests and Helm chart resources into deploy/, updates installation and release scripts, adds CRDs and controller resources, introduces YAML linting, and normalizes workflow and manifest formatting.

Changes

Deployment and packaging migration

Layer / File(s) Summary
Helm chart resources
deploy/charts/whereabouts/*
Updates chart metadata, values, helpers, CRDs, RBAC, DaemonSet, reconciler, node-slice controller, and ConfigMap templates.
Standalone Kubernetes manifests
deploy/crds/*, deploy/manifests/*
Adds CRDs, RBAC, DaemonSet, reconciler, node-slice controller, and cron configuration manifests.
Installation and release wiring
hack/e2e-setup-kind-cluster.sh, hack/generate-code.sh, hack/release/*, README.md, CLAUDE.md
Redirects generation, Kind setup, chart packaging, chart updates, and documentation to the new deployment paths.
Workflow configuration normalization
.github/dependabot.yml, .github/workflows/*
Normalizes YAML structure and formatting while preserving workflow behavior.
YAML linting and formatting support
.yamllint.yml, Makefile, hack/cni-install.yml, yamls/*
Adds yamllint configuration and target, and reformats additional YAML files.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested reviewers: dougbtv, maiqueb

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately summarizes the main change: refactoring Kubernetes deployment and Helm files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
hack/release/chart-update.sh (1)

48-51: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Fix duplicated image registry prefix.

The chart templates (e.g., daemonset.yaml and deployments) now explicitly render the image using print .Values.image.registry "/" .Values.image.repository. Since .image.registry is already defined as ghcr.io in values.yaml, injecting ghcr.io/ directly into the repository field here will result in a malformed image URL like ghcr.io/ghcr.io/.../whereabouts:tag.

To correctly override the image repository while avoiding duplication, strip the registry from the repository injection in this script.

🐛 Proposed fix to avoid duplicating `ghcr.io/`
 # whereabouts image:
 WHEREABOUTS_REPO=${GITHUB_REPO_OWNER} # this is used to allow to release whereabouts from forks
-$YQ_CMD -i ".image.repository = \"ghcr.io/${WHEREABOUTS_REPO}/whereabouts\"" ${HELM_VALUES}
+$YQ_CMD -i ".image.repository = \"${WHEREABOUTS_REPO}/whereabouts\"" ${HELM_VALUES}
 $YQ_CMD -i ".image.tag = \"${WHEREABOUTS_TAG}\"" ${HELM_VALUES}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/release/chart-update.sh` around lines 48 - 51, Update the image
repository assignment in the release script so it contains only the GitHub owner
and repository path, without the hardcoded ghcr.io/ prefix. Keep the existing
WHEREABOUTS_REPO and WHEREABOUTS_TAG substitutions intact, allowing the chart’s
image.registry value to supply the registry exactly once.
🧹 Nitpick comments (3)
.github/workflows/binaries-upload-release.yml (1)

20-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Replace archived GitHub Action.

The actions/upload-release-asset action has been archived by GitHub and is no longer maintained. Consider replacing it with the official GitHub CLI (gh release upload) or a maintained community alternative like softprops/action-gh-release to ensure ongoing compatibility and security.

♻️ Proposed alternative using `gh` CLI
-      - name: Upload whereabouts binary
-        uses: actions/upload-release-asset@e8f9f06c4b078e705bd2ea027f0926603fc9b4d5 # v1
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-        with:
-          upload_url: ${{ github.event.release.upload_url }}
-          asset_path: ./bin/whereabouts
-          asset_name: whereabouts-${{ matrix.arch }}
-          asset_content_type: application/octet-stream
+      - name: Upload whereabouts binary
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          gh release upload ${{ github.event.release.tag_name }} ./bin/whereabouts#whereabouts-${{ matrix.arch }}

(Note: Requires checking out the repo and potentially adjusting the release tag reference.)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/binaries-upload-release.yml around lines 20 - 27, Replace
the archived actions/upload-release-asset step with a maintained release-upload
implementation, preferably softprops/action-gh-release or the GitHub CLI gh
release upload. Preserve the existing matrix asset path, architecture-based
asset name, release association, and GITHUB_TOKEN authentication.
hack/e2e-setup-kind-cluster.sh (1)

110-112: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the redundant loop for CRDs.

This loop attempts to inject an imagePullPolicy: Never directive into the CRD manifests and apply them again. However, CRDs do not contain an image: field, making the sed command a no-op. Furthermore, the CRDs have already been applied earlier in the script (lines 101-103), rendering this entire block redundant.

✂️ Proposed fix to remove the redundant loop
-for file in "whereabouts.cni.cncf.io_ippools.yaml" "whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml" "whereabouts.cni.cncf.io_nodeslicepools.yaml"; do
-  sed '/        image:/a\        imagePullPolicy: Never' "$ROOT/deploy/crds/$file" | retry kubectl apply -f -
-done
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/e2e-setup-kind-cluster.sh` around lines 110 - 112, Remove the loop
iterating over the three CRD manifest filenames after the earlier CRD
application in the setup script. Delete the associated sed and retry kubectl
apply commands, leaving the existing initial CRD application unchanged.
deploy/manifests/rbac.yaml (1)

2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Update stale Helm chart paths in comments. The comments still refer to the old deployment/whereabouts-chart path instead of the newly consolidated deploy/charts/whereabouts directory.

  • deploy/manifests/rbac.yaml#L2-L2: Update the path to reference deploy/charts/whereabouts/templates/....
  • deploy/manifests/deployment-reconciler.yaml#L2-L2: Update the path to reference deploy/charts/whereabouts/templates/deployment-reconciler.yaml.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@deploy/manifests/rbac.yaml` at line 2, Update the stale chart-path comments
in deploy/manifests/rbac.yaml lines 2-2 and
deploy/manifests/deployment-reconciler.yaml lines 2-2 to reference
deploy/charts/whereabouts/templates/... instead of deployment/whereabouts-chart;
use the specific corresponding template paths, including
deployment-reconciler.yaml in the latter comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/test.yml:
- Around line 52-58: Update the coverage upload step’s flag-name under “Send
coverage” to remove the undefined matrix.go reference, or replace it with a
property defined by the job’s strategy.matrix; preserve a valid, non-empty flag
name.

In `@deploy/charts/whereabouts/templates/NOTES.txt`:
- Line 5: Update the pod selector in the NOTES template to use the standard
app.kubernetes.io/name label produced by whereabouts.selectorLabels, replacing
the obsolete app=whereabouts selector while preserving the existing namespace
and command behavior.

In `@deploy/manifests/deployment-node-slice-controller.yaml`:
- Around line 45-65: Remove the cni-bin, cni-net-dir, and
cron-scheduler-configmap entries from the node-slice-controller deployment’s
volumeMounts and volumes sections. Keep the controller container and its
Kubernetes API configuration unchanged, leaving only mounts and volumes required
by this controller.

In `@hack/generate-code.sh`:
- Line 5: Update the controller-gen invocation in generate-code.sh to emit CRDs
only to deploy/crds, then copy the generated YAML files into
deploy/charts/whereabouts/crds/ so both destinations are populated.

In `@README.md`:
- Around line 46-52: Update the README manifest application command to remove
the duplicate rbac.yaml and daemonset.yaml entries, and add the
whereabouts.cni.cncf.io_nodeslicepools.yaml CRD alongside the other CRD
manifests so Fast IPAM is supported.

---

Outside diff comments:
In `@hack/release/chart-update.sh`:
- Around line 48-51: Update the image repository assignment in the release
script so it contains only the GitHub owner and repository path, without the
hardcoded ghcr.io/ prefix. Keep the existing WHEREABOUTS_REPO and
WHEREABOUTS_TAG substitutions intact, allowing the chart’s image.registry value
to supply the registry exactly once.

---

Nitpick comments:
In @.github/workflows/binaries-upload-release.yml:
- Around line 20-27: Replace the archived actions/upload-release-asset step with
a maintained release-upload implementation, preferably
softprops/action-gh-release or the GitHub CLI gh release upload. Preserve the
existing matrix asset path, architecture-based asset name, release association,
and GITHUB_TOKEN authentication.

In `@deploy/manifests/rbac.yaml`:
- Line 2: Update the stale chart-path comments in deploy/manifests/rbac.yaml
lines 2-2 and deploy/manifests/deployment-reconciler.yaml lines 2-2 to reference
deploy/charts/whereabouts/templates/... instead of deployment/whereabouts-chart;
use the specific corresponding template paths, including
deployment-reconciler.yaml in the latter comment.

In `@hack/e2e-setup-kind-cluster.sh`:
- Around line 110-112: Remove the loop iterating over the three CRD manifest
filenames after the earlier CRD application in the setup script. Delete the
associated sed and retry kubectl apply commands, leaving the existing initial
CRD application unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e88fa5c6-c8e1-4ea7-8392-281f5ae4e795

📥 Commits

Reviewing files that changed from the base of the PR and between 8cb4e28 and f583915.

📒 Files selected for processing (52)
  • .github/dependabot.yml
  • .github/workflows/binaries-upload-release.yml
  • .github/workflows/build.yml
  • .github/workflows/chart-push-release.yml
  • .github/workflows/codeql.yml
  • .github/workflows/stale.yml
  • .github/workflows/test.yml
  • .yamllint.yml
  • CLAUDE.md
  • Makefile
  • README.md
  • deploy/charts/whereabouts/Chart.yaml
  • deploy/charts/whereabouts/crds/whereabouts.cni.cncf.io_ippools.yaml
  • deploy/charts/whereabouts/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml
  • deploy/charts/whereabouts/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • deploy/charts/whereabouts/templates/NOTES.txt
  • deploy/charts/whereabouts/templates/_helpers.tpl
  • deploy/charts/whereabouts/templates/clusterRole.yaml
  • deploy/charts/whereabouts/templates/clusterRoleBinding.yaml
  • deploy/charts/whereabouts/templates/configmap.yaml
  • deploy/charts/whereabouts/templates/daemonset.yaml
  • deploy/charts/whereabouts/templates/deployment-node-slice-controller.yaml
  • deploy/charts/whereabouts/templates/deployment-reconciler.yaml
  • deploy/charts/whereabouts/templates/serviceaccount.yaml
  • deploy/charts/whereabouts/values.yaml
  • deploy/crds/whereabouts.cni.cncf.io_ippools.yaml
  • deploy/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml
  • deploy/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • deploy/manifests/daemonset.yaml
  • deploy/manifests/deployment-node-slice-controller.yaml
  • deploy/manifests/deployment-reconciler.yaml
  • deploy/manifests/rbac.yaml
  • deployment/whereabouts-chart/crds/whereabouts.cni.cncf.io_ippools.yaml
  • deployment/whereabouts-chart/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml
  • deployment/whereabouts-chart/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • deployment/whereabouts-chart/templates/cluster_role.yaml
  • deployment/whereabouts-chart/templates/configmap.yaml
  • deployment/whereabouts-chart/templates/node-slice-controller.yaml
  • deployment/whereabouts-chart/templates/reconciler.yaml
  • doc/crds/daemonset-install.yaml
  • doc/crds/node-slice-controller.yaml
  • doc/crds/reconciler-deployment.yaml
  • doc/crds/whereabouts.cni.cncf.io_ippools.yaml
  • doc/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml
  • doc/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • hack/cni-install.yml
  • hack/e2e-setup-kind-cluster.sh
  • hack/generate-code.sh
  • hack/release/chart-push.sh
  • hack/release/chart-update.sh
  • yamls/scaleTestDeployment.yaml
  • yamls/whereaboutsScaleNAD.yaml
💤 Files with no reviewable changes (13)
  • deployment/whereabouts-chart/templates/cluster_role.yaml
  • doc/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • doc/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml
  • doc/crds/whereabouts.cni.cncf.io_ippools.yaml
  • deployment/whereabouts-chart/crds/whereabouts.cni.cncf.io_ippools.yaml
  • deployment/whereabouts-chart/templates/configmap.yaml
  • doc/crds/node-slice-controller.yaml
  • deployment/whereabouts-chart/templates/reconciler.yaml
  • deployment/whereabouts-chart/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • doc/crds/reconciler-deployment.yaml
  • deployment/whereabouts-chart/templates/node-slice-controller.yaml
  • doc/crds/daemonset-install.yaml
  • deployment/whereabouts-chart/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
hack/release/chart-update.sh (1)

48-51: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Fix duplicated image registry prefix.

The chart templates (e.g., daemonset.yaml and deployments) now explicitly render the image using print .Values.image.registry "/" .Values.image.repository. Since .image.registry is already defined as ghcr.io in values.yaml, injecting ghcr.io/ directly into the repository field here will result in a malformed image URL like ghcr.io/ghcr.io/.../whereabouts:tag.

To correctly override the image repository while avoiding duplication, strip the registry from the repository injection in this script.

🐛 Proposed fix to avoid duplicating `ghcr.io/`
 # whereabouts image:
 WHEREABOUTS_REPO=${GITHUB_REPO_OWNER} # this is used to allow to release whereabouts from forks
-$YQ_CMD -i ".image.repository = \"ghcr.io/${WHEREABOUTS_REPO}/whereabouts\"" ${HELM_VALUES}
+$YQ_CMD -i ".image.repository = \"${WHEREABOUTS_REPO}/whereabouts\"" ${HELM_VALUES}
 $YQ_CMD -i ".image.tag = \"${WHEREABOUTS_TAG}\"" ${HELM_VALUES}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/release/chart-update.sh` around lines 48 - 51, Update the image
repository assignment in the release script so it contains only the GitHub owner
and repository path, without the hardcoded ghcr.io/ prefix. Keep the existing
WHEREABOUTS_REPO and WHEREABOUTS_TAG substitutions intact, allowing the chart’s
image.registry value to supply the registry exactly once.
🧹 Nitpick comments (3)
.github/workflows/binaries-upload-release.yml (1)

20-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Replace archived GitHub Action.

The actions/upload-release-asset action has been archived by GitHub and is no longer maintained. Consider replacing it with the official GitHub CLI (gh release upload) or a maintained community alternative like softprops/action-gh-release to ensure ongoing compatibility and security.

♻️ Proposed alternative using `gh` CLI
-      - name: Upload whereabouts binary
-        uses: actions/upload-release-asset@e8f9f06c4b078e705bd2ea027f0926603fc9b4d5 # v1
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-        with:
-          upload_url: ${{ github.event.release.upload_url }}
-          asset_path: ./bin/whereabouts
-          asset_name: whereabouts-${{ matrix.arch }}
-          asset_content_type: application/octet-stream
+      - name: Upload whereabouts binary
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          gh release upload ${{ github.event.release.tag_name }} ./bin/whereabouts#whereabouts-${{ matrix.arch }}

(Note: Requires checking out the repo and potentially adjusting the release tag reference.)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/binaries-upload-release.yml around lines 20 - 27, Replace
the archived actions/upload-release-asset step with a maintained release-upload
implementation, preferably softprops/action-gh-release or the GitHub CLI gh
release upload. Preserve the existing matrix asset path, architecture-based
asset name, release association, and GITHUB_TOKEN authentication.
hack/e2e-setup-kind-cluster.sh (1)

110-112: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the redundant loop for CRDs.

This loop attempts to inject an imagePullPolicy: Never directive into the CRD manifests and apply them again. However, CRDs do not contain an image: field, making the sed command a no-op. Furthermore, the CRDs have already been applied earlier in the script (lines 101-103), rendering this entire block redundant.

✂️ Proposed fix to remove the redundant loop
-for file in "whereabouts.cni.cncf.io_ippools.yaml" "whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml" "whereabouts.cni.cncf.io_nodeslicepools.yaml"; do
-  sed '/        image:/a\        imagePullPolicy: Never' "$ROOT/deploy/crds/$file" | retry kubectl apply -f -
-done
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/e2e-setup-kind-cluster.sh` around lines 110 - 112, Remove the loop
iterating over the three CRD manifest filenames after the earlier CRD
application in the setup script. Delete the associated sed and retry kubectl
apply commands, leaving the existing initial CRD application unchanged.
deploy/manifests/rbac.yaml (1)

2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Update stale Helm chart paths in comments. The comments still refer to the old deployment/whereabouts-chart path instead of the newly consolidated deploy/charts/whereabouts directory.

  • deploy/manifests/rbac.yaml#L2-L2: Update the path to reference deploy/charts/whereabouts/templates/....
  • deploy/manifests/deployment-reconciler.yaml#L2-L2: Update the path to reference deploy/charts/whereabouts/templates/deployment-reconciler.yaml.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@deploy/manifests/rbac.yaml` at line 2, Update the stale chart-path comments
in deploy/manifests/rbac.yaml lines 2-2 and
deploy/manifests/deployment-reconciler.yaml lines 2-2 to reference
deploy/charts/whereabouts/templates/... instead of deployment/whereabouts-chart;
use the specific corresponding template paths, including
deployment-reconciler.yaml in the latter comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/test.yml:
- Around line 52-58: Update the coverage upload step’s flag-name under “Send
coverage” to remove the undefined matrix.go reference, or replace it with a
property defined by the job’s strategy.matrix; preserve a valid, non-empty flag
name.

In `@deploy/charts/whereabouts/templates/NOTES.txt`:
- Line 5: Update the pod selector in the NOTES template to use the standard
app.kubernetes.io/name label produced by whereabouts.selectorLabels, replacing
the obsolete app=whereabouts selector while preserving the existing namespace
and command behavior.

In `@deploy/manifests/deployment-node-slice-controller.yaml`:
- Around line 45-65: Remove the cni-bin, cni-net-dir, and
cron-scheduler-configmap entries from the node-slice-controller deployment’s
volumeMounts and volumes sections. Keep the controller container and its
Kubernetes API configuration unchanged, leaving only mounts and volumes required
by this controller.

In `@hack/generate-code.sh`:
- Line 5: Update the controller-gen invocation in generate-code.sh to emit CRDs
only to deploy/crds, then copy the generated YAML files into
deploy/charts/whereabouts/crds/ so both destinations are populated.

In `@README.md`:
- Around line 46-52: Update the README manifest application command to remove
the duplicate rbac.yaml and daemonset.yaml entries, and add the
whereabouts.cni.cncf.io_nodeslicepools.yaml CRD alongside the other CRD
manifests so Fast IPAM is supported.

---

Outside diff comments:
In `@hack/release/chart-update.sh`:
- Around line 48-51: Update the image repository assignment in the release
script so it contains only the GitHub owner and repository path, without the
hardcoded ghcr.io/ prefix. Keep the existing WHEREABOUTS_REPO and
WHEREABOUTS_TAG substitutions intact, allowing the chart’s image.registry value
to supply the registry exactly once.

---

Nitpick comments:
In @.github/workflows/binaries-upload-release.yml:
- Around line 20-27: Replace the archived actions/upload-release-asset step with
a maintained release-upload implementation, preferably
softprops/action-gh-release or the GitHub CLI gh release upload. Preserve the
existing matrix asset path, architecture-based asset name, release association,
and GITHUB_TOKEN authentication.

In `@deploy/manifests/rbac.yaml`:
- Line 2: Update the stale chart-path comments in deploy/manifests/rbac.yaml
lines 2-2 and deploy/manifests/deployment-reconciler.yaml lines 2-2 to reference
deploy/charts/whereabouts/templates/... instead of deployment/whereabouts-chart;
use the specific corresponding template paths, including
deployment-reconciler.yaml in the latter comment.

In `@hack/e2e-setup-kind-cluster.sh`:
- Around line 110-112: Remove the loop iterating over the three CRD manifest
filenames after the earlier CRD application in the setup script. Delete the
associated sed and retry kubectl apply commands, leaving the existing initial
CRD application unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e88fa5c6-c8e1-4ea7-8392-281f5ae4e795

📥 Commits

Reviewing files that changed from the base of the PR and between 8cb4e28 and f583915.

📒 Files selected for processing (52)
  • .github/dependabot.yml
  • .github/workflows/binaries-upload-release.yml
  • .github/workflows/build.yml
  • .github/workflows/chart-push-release.yml
  • .github/workflows/codeql.yml
  • .github/workflows/stale.yml
  • .github/workflows/test.yml
  • .yamllint.yml
  • CLAUDE.md
  • Makefile
  • README.md
  • deploy/charts/whereabouts/Chart.yaml
  • deploy/charts/whereabouts/crds/whereabouts.cni.cncf.io_ippools.yaml
  • deploy/charts/whereabouts/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml
  • deploy/charts/whereabouts/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • deploy/charts/whereabouts/templates/NOTES.txt
  • deploy/charts/whereabouts/templates/_helpers.tpl
  • deploy/charts/whereabouts/templates/clusterRole.yaml
  • deploy/charts/whereabouts/templates/clusterRoleBinding.yaml
  • deploy/charts/whereabouts/templates/configmap.yaml
  • deploy/charts/whereabouts/templates/daemonset.yaml
  • deploy/charts/whereabouts/templates/deployment-node-slice-controller.yaml
  • deploy/charts/whereabouts/templates/deployment-reconciler.yaml
  • deploy/charts/whereabouts/templates/serviceaccount.yaml
  • deploy/charts/whereabouts/values.yaml
  • deploy/crds/whereabouts.cni.cncf.io_ippools.yaml
  • deploy/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml
  • deploy/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • deploy/manifests/daemonset.yaml
  • deploy/manifests/deployment-node-slice-controller.yaml
  • deploy/manifests/deployment-reconciler.yaml
  • deploy/manifests/rbac.yaml
  • deployment/whereabouts-chart/crds/whereabouts.cni.cncf.io_ippools.yaml
  • deployment/whereabouts-chart/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml
  • deployment/whereabouts-chart/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • deployment/whereabouts-chart/templates/cluster_role.yaml
  • deployment/whereabouts-chart/templates/configmap.yaml
  • deployment/whereabouts-chart/templates/node-slice-controller.yaml
  • deployment/whereabouts-chart/templates/reconciler.yaml
  • doc/crds/daemonset-install.yaml
  • doc/crds/node-slice-controller.yaml
  • doc/crds/reconciler-deployment.yaml
  • doc/crds/whereabouts.cni.cncf.io_ippools.yaml
  • doc/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml
  • doc/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • hack/cni-install.yml
  • hack/e2e-setup-kind-cluster.sh
  • hack/generate-code.sh
  • hack/release/chart-push.sh
  • hack/release/chart-update.sh
  • yamls/scaleTestDeployment.yaml
  • yamls/whereaboutsScaleNAD.yaml
💤 Files with no reviewable changes (13)
  • deployment/whereabouts-chart/templates/cluster_role.yaml
  • doc/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • doc/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml
  • doc/crds/whereabouts.cni.cncf.io_ippools.yaml
  • deployment/whereabouts-chart/crds/whereabouts.cni.cncf.io_ippools.yaml
  • deployment/whereabouts-chart/templates/configmap.yaml
  • doc/crds/node-slice-controller.yaml
  • deployment/whereabouts-chart/templates/reconciler.yaml
  • deployment/whereabouts-chart/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • doc/crds/reconciler-deployment.yaml
  • deployment/whereabouts-chart/templates/node-slice-controller.yaml
  • doc/crds/daemonset-install.yaml
  • deployment/whereabouts-chart/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml
🛑 Comments failed to post (5)
.github/workflows/test.yml (1)

52-58: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Undefined matrix property matrix.go.

The expression ${{ matrix.go }} references a matrix property that does not exist. The strategy.matrix defined for this job only contains os: [ubuntu-latest]. This will resolve to an empty string, causing the flag name to be rendered as Go-.

You should remove the -${{ matrix.go }} suffix or replace it with a valid variable.

🐛 Proposed fix
       - name: Send coverage
         uses: shogo82148/actions-goveralls@9606dbc5ac5cf888a0e9ef901515c3cd516a2790 # v1
         with:
           path-to-profile: coverage.out
-          flag-name: Go-${{ matrix.go }}
+          flag-name: Go
         env:
           KUBEBUILDER_ASSETS: "$(pwd)/bin"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

      - name: Send coverage
        uses: shogo82148/actions-goveralls@9606dbc5ac5cf888a0e9ef901515c3cd516a2790 # v1
        with:
          path-to-profile: coverage.out
          flag-name: Go
        env:
          KUBEBUILDER_ASSETS: "$(pwd)/bin"
🧰 Tools
🪛 actionlint (1.7.12)

[error] 56-56: property "go" is not defined in object type {os: string}

(expression)

🪛 zizmor (1.26.1)

[warning] 13-58: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block

(excessive-permissions)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/test.yml around lines 52 - 58, Update the coverage upload
step’s flag-name under “Send coverage” to remove the undefined matrix.go
reference, or replace it with a property defined by the job’s strategy.matrix;
preserve a valid, non-empty flag name.

Source: Linters/SAST tools

deploy/charts/whereabouts/templates/NOTES.txt (1)

5-5: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Update the label selector to match the new selector labels.

The app: whereabouts label was removed from whereabouts.selectorLabels in _helpers.tpl. This kubectl command will return no pods because they now use the standard app.kubernetes.io/name labels.

🐛 Proposed fix
-kubectl get pods -n {{ .Release.Namespace }} -l app=whereabouts
+kubectl get pods -n {{ .Release.Namespace }} -l app.kubernetes.io/name={{ include "whereabouts.name" . }}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

kubectl get pods -n {{ .Release.Namespace }} -l app.kubernetes.io/name={{ include "whereabouts.name" . }}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@deploy/charts/whereabouts/templates/NOTES.txt` at line 5, Update the pod
selector in the NOTES template to use the standard app.kubernetes.io/name label
produced by whereabouts.selectorLabels, replacing the obsolete app=whereabouts
selector while preserving the existing namespace and command behavior.
deploy/manifests/deployment-node-slice-controller.yaml (1)

45-65: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove unnecessary volumes and mounts for the node-slice controller.

The node-slice-controller runs as a standard Kubernetes controller and only communicates with the Kubernetes API to manage NodeSlicePool resources. It does not interact with host CNI directories (/opt/cni/bin, /etc/cni/net.d) nor does it require the reconciler's cron-schedule ConfigMap. These mounts appear to have been copied over from the daemonset and reconciler manifests.

♻️ Proposed fix to remove unused volumes
-          volumeMounts:
-            - mountPath: /host/opt/cni/bin
-              name: cni-bin
-            - mountPath: /host/etc/cni/net.d
-              name: cni-net-dir
-            - mountPath: /cron-schedule
-              name: cron-scheduler-configmap
-      volumes:
-        - name: cni-bin
-          hostPath:
-            path: /opt/cni/bin
-        - name: cni-net-dir
-          hostPath:
-            path: /etc/cni/net.d
-        - name: cron-scheduler-configmap
-          configMap:
-            name: whereabouts-config
-            items:
-              - key: cron-expression
-                path: config
-            defaultMode: 0744
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.


🧰 Tools
🪛 Checkov (3.3.8)

[medium] 3-65: Containers should not run with allowPrivilegeEscalation

(CKV_K8S_20)


[medium] 3-65: Minimize the admission of root containers

(CKV_K8S_23)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@deploy/manifests/deployment-node-slice-controller.yaml` around lines 45 - 65,
Remove the cni-bin, cni-net-dir, and cron-scheduler-configmap entries from the
node-slice-controller deployment’s volumeMounts and volumes sections. Keep the
controller container and its Kubernetes API configuration unchanged, leaving
only mounts and volumes required by this controller.
hack/generate-code.sh (1)

5-5: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Test if controller-gen populated both directories
./hack/generate-code.sh
if [ -z "$(ls -A deploy/crds 2>/dev/null)" ]; then
    echo "deploy/crds is empty! The flag was overridden."
else
    echo "Both locations are populated."
fi

Repository: k8snetworkplumbingwg/whereabouts

Length of output: 497


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'hack/generate-code.sh:\n'
cat -n hack/generate-code.sh | sed -n '1,40p'

printf '\nExisting crd directories/files:\n'
find deploy -maxdepth 4 \( -path 'deploy/crds' -o -path 'deploy/charts/whereabouts/crds' \) -type f 2>/dev/null | sort | sed -n '1,120p'

Repository: k8snetworkplumbingwg/whereabouts

Length of output: 472


🌐 Web query:

controller-gen repeated output:crd:artifacts:config marker multiple destinations semantics

💡 Result:

In controller-gen, the output:crd:artifacts:config marker is used to define the directory where generated CRD configuration artifacts (YAML files) should be written [1][2]. When this marker is repeated or provided with multiple values, controller-gen does not support "multiple destinations" for a single artifact type in the sense of duplicating the output to several directories simultaneously. Instead, the final behavior is determined by the last argument provided in the command line or configuration [3][4][5]. Controller-gen processes these options sequentially, and subsequent definitions typically override previous ones. If your intent is to send different types of artifacts to different locations, note the following: 1. Rule Specificity: You can use distinct output rules for different generator types. For example, you can specify output:crd:artifacts:config=path/a and output:rbac:artifacts:config=path/b in the same command to route CRDs and RBAC roles to different folders [3][5]. 2. Output Rules: - output:crd:artifacts:config=path: Explicitly directs non-code CRD configuration artifacts to the specified directory [1][6]. - output:dir=path: An alternative, more general output rule that forces all generated artifacts (regardless of type or association) into the specified directory [3][4][5]. In summary, providing multiple output:crd:artifacts:config markers for the same artifact type will result in only the final path being respected. To handle multiple destinations for different artifact types, use the specific output rules associated with each generator (e.g., crd, rbac, webhook) rather than repeating the same configuration marker [3][5].

Citations:


hack/generate-code.sh:5 writes CRDs to only one directory

controller-gen treats repeated output:crd:artifacts:config markers as one destination, so the later path overrides deploy/crds. Generate once to deploy/crds and copy the YAMLs into deploy/charts/whereabouts/crds/.

🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 5-5: Double quote to prevent globbing and word splitting.

(SC2086)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/generate-code.sh` at line 5, Update the controller-gen invocation in
generate-code.sh to emit CRDs only to deploy/crds, then copy the generated YAML
files into deploy/charts/whereabouts/crds/ so both destinations are populated.
README.md (1)

46-52: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Fix duplicated manifests and include the missing CRD.

The list of files to apply contains duplicates for rbac.yaml and daemonset.yaml. Additionally, the new whereabouts.cni.cncf.io_nodeslicepools.yaml CRD is missing from the command, which is required for the Fast IPAM feature.

🛠️ Proposed fix
 kubectl apply \
     -f deploy/manifests/rbac.yaml \
     -f deploy/manifests/daemonset.yaml \
     -f deploy/crds/whereabouts.cni.cncf.io_ippools.yaml \
     -f deploy/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml \
-    -f deploy/manifests/rbac.yaml \
-    -f deploy/manifests/daemonset.yaml \
+    -f deploy/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml \
     -f deploy/manifests/deployment-reconciler.yaml
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    -f deploy/manifests/rbac.yaml \
    -f deploy/manifests/daemonset.yaml \
    -f deploy/crds/whereabouts.cni.cncf.io_ippools.yaml \
    -f deploy/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml \
    -f deploy/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml \
    -f deploy/manifests/deployment-reconciler.yaml
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` around lines 46 - 52, Update the README manifest application
command to remove the duplicate rbac.yaml and daemonset.yaml entries, and add
the whereabouts.cni.cncf.io_nodeslicepools.yaml CRD alongside the other CRD
manifests so Fast IPAM is supported.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (2)
hack/release/chart-update.sh (1)

50-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Quote variables to prevent word splitting.

It is recommended to double-quote ${HELM_VALUES} to prevent potential issues with globbing and word splitting if the path contains spaces.

♻️ Proposed refactor
-$YQ_CMD -i ".image.registry = \"ghcr.io\"" ${HELM_VALUES}
-$YQ_CMD -i ".image.repository = \"${WHEREABOUTS_REPO}/whereabouts\"" ${HELM_VALUES}
+$YQ_CMD -i ".image.registry = \"ghcr.io\"" "${HELM_VALUES}"
+$YQ_CMD -i ".image.repository = \"${WHEREABOUTS_REPO}/whereabouts\"" "${HELM_VALUES}"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@hack/release/chart-update.sh` around lines 50 - 51, Quote the HELM_VALUES
argument in both YQ_CMD invocations so the values-file path is passed as a
single shell argument. Update only the commands assigning image.registry and
image.repository, preserving their existing expressions and behavior.

Source: Linters/SAST tools

.github/workflows/binaries-upload-release.yml (1)

19-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Migrate away from archived actions/upload-release-asset.

The actions/upload-release-asset repository is archived and no longer maintained. Consider migrating to softprops/action-gh-release or using gh release upload via the GitHub CLI in a run step to ensure continued support and security updates.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/binaries-upload-release.yml around lines 19 - 20, Replace
the archived actions/upload-release-asset step in the release workflow with a
maintained release-upload mechanism, preferably softprops/action-gh-release or
gh release upload. Preserve the existing whereabouts binary asset path, release
association, and upload behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/test.yml:
- Line 56: Update the workflow step using the `flag-name` key to stop
referencing the undefined `matrix.go` value, or add a corresponding `go` entry
to the job matrix; ensure the resulting flag name is populated consistently with
the matrix configuration.
- Around line 3-5: Remove the trailing single quote from the markdown glob under
paths-ignore so the pattern correctly matches all .md files; leave the other
ignore pattern unchanged.

In `@deploy/manifests/deployment-node-slice-controller.yaml`:
- Line 7: Update the deployment namespace for the node slice controller so it
matches the namespace used by the Fast IPAM NetworkAttachmentDefinitions; verify
whether kube-system is correct, and parameterize the namespace instead of
hardcoding it when the NADs use another namespace.

---

Nitpick comments:
In @.github/workflows/binaries-upload-release.yml:
- Around line 19-20: Replace the archived actions/upload-release-asset step in
the release workflow with a maintained release-upload mechanism, preferably
softprops/action-gh-release or gh release upload. Preserve the existing
whereabouts binary asset path, release association, and upload behavior.

In `@hack/release/chart-update.sh`:
- Around line 50-51: Quote the HELM_VALUES argument in both YQ_CMD invocations
so the values-file path is passed as a single shell argument. Update only the
commands assigning image.registry and image.repository, preserving their
existing expressions and behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 97dcf7b1-991c-424f-b623-0f44b22d3335

📥 Commits

Reviewing files that changed from the base of the PR and between f583915 and f6ebb90.

📒 Files selected for processing (35)
  • .github/dependabot.yml
  • .github/workflows/binaries-upload-release.yml
  • .github/workflows/build.yml
  • .github/workflows/chart-push-release.yml
  • .github/workflows/codeql.yml
  • .github/workflows/stale.yml
  • .github/workflows/test.yml
  • .yamllint.yml
  • Makefile
  • README.md
  • deploy/charts/whereabouts/Chart.yaml
  • deploy/charts/whereabouts/crds/whereabouts.cni.cncf.io_ippools.yaml
  • deploy/charts/whereabouts/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml
  • deploy/charts/whereabouts/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • deploy/charts/whereabouts/templates/_helpers.tpl
  • deploy/charts/whereabouts/templates/clusterRole.yaml
  • deploy/charts/whereabouts/templates/clusterRoleBinding.yaml
  • deploy/charts/whereabouts/templates/configmap.yaml
  • deploy/charts/whereabouts/templates/daemonset.yaml
  • deploy/charts/whereabouts/templates/deployment-node-slice-controller.yaml
  • deploy/charts/whereabouts/templates/deployment-reconciler.yaml
  • deploy/charts/whereabouts/templates/serviceaccount.yaml
  • deploy/charts/whereabouts/values.yaml
  • deploy/crds/whereabouts.cni.cncf.io_ippools.yaml
  • deploy/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml
  • deploy/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • deploy/manifests/daemonset.yaml
  • deploy/manifests/deployment-node-slice-controller.yaml
  • deploy/manifests/deployment-reconciler.yaml
  • deploy/manifests/rbac.yaml
  • hack/cni-install.yml
  • hack/release/chart-push.sh
  • hack/release/chart-update.sh
  • yamls/scaleTestDeployment.yaml
  • yamls/whereaboutsScaleNAD.yaml
🚧 Files skipped from review as they are similar to previous changes (24)
  • yamls/whereaboutsScaleNAD.yaml
  • deploy/charts/whereabouts/templates/serviceaccount.yaml
  • .github/workflows/codeql.yml
  • deploy/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • deploy/charts/whereabouts/Chart.yaml
  • deploy/crds/whereabouts.cni.cncf.io_ippools.yaml
  • deploy/charts/whereabouts/crds/whereabouts.cni.cncf.io_overlappingrangeipreservations.yaml
  • deploy/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml
  • deploy/charts/whereabouts/templates/clusterRole.yaml
  • deploy/charts/whereabouts/templates/configmap.yaml
  • deploy/charts/whereabouts/crds/whereabouts.cni.cncf.io_ippools.yaml
  • .yamllint.yml
  • .github/workflows/stale.yml
  • deploy/manifests/rbac.yaml
  • README.md
  • deploy/charts/whereabouts/crds/whereabouts.cni.cncf.io_nodeslicepools.yaml
  • deploy/charts/whereabouts/templates/deployment-reconciler.yaml
  • Makefile
  • deploy/charts/whereabouts/templates/deployment-node-slice-controller.yaml
  • .github/workflows/chart-push-release.yml
  • deploy/charts/whereabouts/templates/clusterRoleBinding.yaml
  • deploy/charts/whereabouts/templates/_helpers.tpl
  • deploy/charts/whereabouts/templates/daemonset.yaml
  • deploy/charts/whereabouts/values.yaml

Comment on lines 3 to 5
paths-ignore:
- "**/*.md'"
- "doc/*.{json,png,svg}"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Fix typo in paths-ignore pattern.

There is a trailing single quote in the "**/*.md'" pattern which will prevent it from matching markdown files correctly.

🐛 Proposed fix
   pull_request:
     paths-ignore:
-      - "**/*.md'"
+      - "**/*.md"
       - "doc/*.{json,png,svg}"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
paths-ignore:
- "**/*.md'"
- "doc/*.{json,png,svg}"
paths-ignore:
- "**/*.md"
- "doc/*.{json,png,svg}"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/test.yml around lines 3 - 5, Remove the trailing single
quote from the markdown glob under paths-ignore so the pattern correctly matches
all .md files; leave the other ignore pattern unchanged.

uses: shogo82148/actions-goveralls@9606dbc5ac5cf888a0e9ef901515c3cd516a2790 # v1
with:
path-to-profile: coverage.out
flag-name: Go-${{ matrix.go }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Undefined matrix variable.

The matrix defined for this job only contains os, so ${{ matrix.go }} is undefined. This will result in an empty value (e.g., Go-). Consider either adding go to the matrix or removing the matrix reference here.

🧰 Tools
🪛 actionlint (1.7.12)

[error] 56-56: property "go" is not defined in object type {os: string}

(expression)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/test.yml at line 56, Update the workflow step using the
`flag-name` key to stop referencing the undefined `matrix.go` value, or add a
corresponding `go` entry to the job matrix; ensure the resulting flag name is
populated consistently with the matrix configuration.

Source: Linters/SAST tools

kind: Deployment
metadata:
name: whereabouts-node-slice-controller
namespace: kube-system

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Ensure namespace aligns with NetworkAttachmentDefinitions for Fast IPAM.

The namespace is currently hardcoded to kube-system. As per coding guidelines, the node slice controller must run in the same namespace as the NetworkAttachmentDefinitions when Fast IPAM is enabled. Please verify that kube-system is the correct namespace for your NADs, or parameterize this value if it needs to match another namespace.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@deploy/manifests/deployment-node-slice-controller.yaml` at line 7, Update the
deployment namespace for the node slice controller so it matches the namespace
used by the Fast IPAM NetworkAttachmentDefinitions; verify whether kube-system
is correct, and parameterize the namespace instead of hardcoding it when the
NADs use another namespace.

Source: Coding guidelines

mkl262 added 2 commits August 21, 2026 12:48
Consolidate all Kubernetes deployment resources into a single deploy/
directory with a clearer layout:

- deploy/crds/ — CRD manifests (moved from doc/crds/)
- deploy/manifests/ — raw manifests split from the monolithic
  daemonset-install.yaml into rbac, daemonset, reconciler, and
  node-slice-controller files
- deploy/charts/whereabouts/ — Helm chart (moved from
  deployment/whereabouts-chart/, renamed to match chart name)

Restructure Helm chart templates to mirror the raw manifests, add
cross-reference comments between manifest and chart files, and apply

Update README, CLAUDE.md, and hack scripts (e2e setup, codegen,
chart release) to reference the new paths.
!IMPORTANT! changes to selector labels will cause direct upgrade to fail, requiring deleting deployments and daemonset crate by helm/ manifests.

change chart name to whereabouts

add and run yamllint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant