Fix kamaji make update #598
Conversation
dosubot
bot
added
the
size:M
WalkthroughThis pull request introduces changes to the Kamaji package, focusing on version updates and configuration improvements. The modifications span multiple files in the Kamaji charts and Dockerfile, including updates to versioning information, dependency requirements, and configuration handling. The changes primarily affect how DataStore configurations are managed, with enhanced flexibility in setting default values and more precise validation rules for Tenant Control Plane configurations. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
packages/system/kamaji/charts/kamaji/templates/controller.yaml (1)
Invalid Go version
golang:1.23specified in DockerfileThe Dockerfile at
packages/system/kamaji/images/kamaji/DockerfilespecifiesFROM golang:1.23, but Go 1.23 is not yet released and doesn't exist. As of January 2025, the latest stable Go version is 1.21.x.🔗 Analysis chain
Line range hint
1-1: Verify golang version in Dockerfile.The PR description mentions updating to
golang:1.23, but this seems incorrect as Go 1.23 is not yet released (latest stable is 1.21.x as of January 2025).Please run this script to verify the Go version:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if golang:1.23 is valid and get latest available versions echo "Checking available golang versions..." curl -s https://go.dev/dl/?mode=json | jq -r '.[].version' | head -n 5 echo -e "\nChecking Dockerfile..." rg "FROM golang:" "packages/system/kamaji/Dockerfile"Length of output: 367
Script:
#!/bin/bash # Find Dockerfile and verify golang versions echo "Searching for Dockerfile..." fd Dockerfile echo -e "\nChecking available golang versions from Docker Hub..." curl -s "https://hub.docker.com/v2/repositories/library/golang/tags?page_size=10" | jq -r '.results[].name' | grep -E '^[0-9]+\.[0-9]+(\.[0-9]+)?$' || echo "No matching versions found"Length of output: 1641
Script:
#!/bin/bash # Check Dockerfile content and golang versions echo "Reading Dockerfile content..." cat "packages/system/kamaji/images/kamaji/Dockerfile" echo -e "\nChecking available golang versions..." curl -s "https://registry.hub.docker.com/v2/repositories/library/golang/tags?page=1&page_size=5&ordering=last_updated" | jq -r '.results[].name'Length of output: 1432
🧹 Nitpick comments (1)
packages/system/kamaji/charts/kamaji/README.md (1)
25-25: Fix markdown lint issueThe bare URL in the repository table should be properly formatted.
-| https://clastix.github.io/charts | kamaji-etcd | >=0.8.1 | +| <https://clastix.github.io/charts> | kamaji-etcd | >=0.8.1 |🧰 Tools
🪛 Markdownlint (0.37.0)
25-25: null
Bare URL used(MD034, no-bare-urls)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/system/kamaji/charts/kamaji/Chart.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
packages/system/kamaji/Makefile(1 hunks)packages/system/kamaji/charts/kamaji/Chart.yaml(2 hunks)packages/system/kamaji/charts/kamaji/README.md(3 hunks)packages/system/kamaji/charts/kamaji/crds/kamaji.clastix.io_tenantcontrolplanes.yaml(3 hunks)packages/system/kamaji/charts/kamaji/templates/controller.yaml(1 hunks)packages/system/kamaji/charts/kamaji/values.yaml(1 hunks)packages/system/kamaji/images/kamaji/Dockerfile(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/system/kamaji/charts/kamaji/values.yaml
🧰 Additional context used
🪛 Markdownlint (0.37.0)
packages/system/kamaji/charts/kamaji/README.md
25-25: null
Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (7)
packages/system/kamaji/charts/kamaji/crds/kamaji.clastix.io_tenantcontrolplanes.yaml (2)
6415-6431: LGTM! Improved DataStore configuration with better safety.The changes enhance the DataStore configuration by:
- Clarifying default value inheritance behavior
- Adding the DataStoreSchema field with proper immutability validation
- Preventing accidental data loss by disallowing unsetting of DataStore fields
Line range hint
6554-6620: LGTM! Enhanced NetworkProfile with robust validation rules.The changes strengthen the NetworkProfile configuration by:
- Adding immutable clusterDomain field with proper validation
- Adding immutable loadBalancerClass field
- Adding loadBalancerSourceRanges for better security control
- Implementing cross-field validations to ensure LoadBalancer settings are used correctly
packages/system/kamaji/charts/kamaji/templates/controller.yaml (1)
36-38: LGTM! The conditional handling of defaultDatastoreName looks good.The change improves flexibility by making the
--datastoreargument optional while maintaining backward compatibility.Please ensure that:
- This change is documented in the
values.yamlfile- The default value in
values.yamlaligns with this new behavior- Any existing documentation is updated to reflect that
defaultDatastoreNameis now optionalRun this script to verify the documentation:
✅ Verification successful
Documentation and configuration are properly aligned with the new behavior ✅
The
defaultDatastoreNameis:
- Correctly documented in README.md with "If specified" clause
- Properly configured in values.yaml with default value "default"
- Implementation matches both documentation and configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check values.yaml and documentation for defaultDatastoreName echo "Checking values.yaml for defaultDatastoreName..." rg -A 5 "defaultDatastoreName:" "packages/system/kamaji/charts/kamaji/values.yaml" echo -e "\nChecking documentation..." rg -A 5 "defaultDatastoreName" "packages/system/kamaji/charts/kamaji/README.md"Length of output: 982
packages/system/kamaji/Makefile (1)
12-12: LGTM! Improved version update mechanismThe change correctly updates the
ARG VERSIONline instead of theFROMline, which aligns better with the Dockerfile structure.packages/system/kamaji/charts/kamaji/Chart.yaml (2)
2-2: Verify version reset strategyThe version and appVersion have been reset to
0.0.0. This is an unusual change that requires clarification:
- Was this intentional?
- Does this align with the project's versioning strategy?
Please confirm if this is part of a broader versioning strategy change.
Also applies to: 20-20
24-24: Verify kamaji-etcd compatibilityThe kamaji-etcd dependency version has been updated to
>=0.8.1.✅ Verification successful
Version compatibility verified successfully
The kamaji-etcd chart version
>=0.8.1is available in the repository. Compatible versions found: 0.8.1, 0.9.0, and 0.9.1.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify kamaji-etcd chart version compatibility curl -s https://clastix.github.io/charts/index.yaml | \ yq e '.entries."kamaji-etcd"[] | select(.version >= "0.8.1") | .version' -Length of output: 223
Script:
#!/bin/bash # Fetch the chart repository index and search for kamaji-etcd versions curl -s https://clastix.github.io/charts/index.yaml | grep -A 2 "name: kamaji-etcd$"Length of output: 1660
Script:
#!/bin/bash # Fetch the chart repository index and extract version numbers curl -s https://clastix.github.io/charts/index.yaml | grep -A 5 "name: kamaji-etcd$" | grep "version: "Length of output: 105
Script:
#!/bin/bash # Save the index.yaml content to examine the structure curl -s https://clastix.github.io/charts/index.yaml > /tmp/index.yaml head -n 50 /tmp/index.yamlLength of output: 1860
Script:
#!/bin/bash # Extract kamaji-etcd section and find versions curl -s https://clastix.github.io/charts/index.yaml | \ sed -n '/^ kamaji-etcd:/,/^ [a-z]/p' | \ grep "version: " | \ sort -VLength of output: 484
packages/system/kamaji/charts/kamaji/README.md (1)
73-73: LGTM! Improved configuration documentationThe description for
defaultDatastoreNamehas been enhanced to better explain its behavior with unassigned DataStore instances.
| @@ -1,7 +1,7 @@ | |||
| # Build the manager binary | |||
| FROM golang:1.22 as builder | |||
| FROM golang:1.23 as builder | |||
There was a problem hiding this comment.
Critical: Invalid Go version specified
The specified Go version 1.23 does not exist yet. The latest stable version is 1.22.
Apply this diff to fix the version:
-FROM golang:1.23 as builder
+FROM golang:1.22 as builder📝 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.
| FROM golang:1.23 as builder | |
| FROM golang:1.22 as builder |
|
|
||
| ARG VERSION=edge-24.9.2 | ||
| ARG VERSION=edge-24.12.1 |
There was a problem hiding this comment.
💡 Codebase verification
Invalid Kamaji version specified
The version edge-24.12.1 appears to be invalid:
- This version tag does not exist in the Kamaji repository
- The repository uses semantic versioning (latest: v1.0.0) without any "edge-" prefixes
- Please verify and update to a valid version tag from https://github.com/clastix/kamaji/tags
🔗 Analysis chain
Verify Kamaji version compatibility
The version has been updated to edge-24.12.1. Let's verify this version exists and is compatible.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the specified version exists in the Kamaji repository
gh api repos/clastix/kamaji/tags --jq '.[].name' | grep -q "edge-24.12.1" && \
echo "Version edge-24.12.1 exists" || \
echo "Version edge-24.12.1 not found"
Length of output: 144
Script:
#!/bin/bash
# List the 10 most recent tags from the Kamaji repository
gh api repos/clastix/kamaji/tags --jq '.[0:10] | .[] | .name'
Length of output: 133
Fix makefile update target for kamaji
Fix Dockerfile for kamaji (golang:1.23 as builder)
kamaji Chart updated
Summary by CodeRabbit
Release Notes
New Features
Improvements
edge-24.12.1Dependency Updates
kamaji-etcddependency to version>=0.8.1Version Changes
0.0.0