Fetching KubeVersion while rendering manifests #1
Conversation
WalkthroughA new helper function, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Main
participant K8sAPI as Kubernetes API Server
participant HelmInstall
User->>Main: Trigger manifest render or diff
Main->>K8sAPI: Query for server version (discoverKubeVersion)
K8sAPI-->>Main: Return version info
Main->>HelmInstall: Set KubeVersion field
Main->>HelmInstall: Proceed with render/diff action
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
🧹 Nitpick comments (1)
main.go (1)
327-336: Re-use the existing REST config instead of creating one more round-trip
realHelmDiff()creates a fresh REST config even though its caller already has one (and the function immediately uses it only for version discovery). This:
- Duplicates kube-config parsing work for every diff invocation.
- Makes the code path harder to unit-test because the function owns the config construction.
Consider passing the
*rest.Configdown from the caller (same asrenderManifestsalready does) and deleting the extrarestConfig()call to reduce overhead and keep the API uniform.
| if plain { | ||
| kubeVer, err := discoverKubeVersion(rc) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| inst.KubeVersion = &kubeVer | ||
| vers, err := discoverAPIVersions(rc) |
There was a problem hiding this comment.
*Guard against nil rest.Config before querying the cluster
renderManifests() assumes that rc is non-nil. When cmdShow is invoked with --plain and there is no valid KUBECONFIG, restConfig() returns (nil, err) but the error is ignored (rc, _ := restConfig()).
Calling discoverKubeVersion(rc) (or discoverAPIVersions(rc)) with a nil pointer will panic inside the REST client constructor.
-if plain {
- kubeVer, err := discoverKubeVersion(rc)
+if plain {
+ if rc == nil {
+ return "", fmt.Errorf("renderManifests: kubeconfig not found – cannot discover cluster version")
+ }
+ kubeVer, err := discoverKubeVersion(rc)Fail fast with a clear error instead of a runtime panic.
📝 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.
| if plain { | |
| kubeVer, err := discoverKubeVersion(rc) | |
| if err != nil { | |
| return "", err | |
| } | |
| inst.KubeVersion = &kubeVer | |
| vers, err := discoverAPIVersions(rc) | |
| if plain { | |
| if rc == nil { | |
| return "", fmt.Errorf("renderManifests: kubeconfig not found – cannot discover cluster version") | |
| } | |
| kubeVer, err := discoverKubeVersion(rc) | |
| if err != nil { | |
| return "", err | |
| } | |
| inst.KubeVersion = &kubeVer | |
| vers, err := discoverAPIVersions(rc) |
🤖 Prompt for AI Agents
In main.go around lines 240 to 246, the code calls discoverKubeVersion(rc) and
discoverAPIVersions(rc) without checking if rc (a *rest.Config) is nil, which
can cause a panic if restConfig() returned nil due to missing or invalid
KUBECONFIG. Modify the code to check if rc is nil immediately after calling
restConfig(), and if so, return a clear error indicating the missing or invalid
kubeconfig before proceeding to call any functions that use rc.
Signed-off-by: Andrei Kvapil <[email protected]>
Signed-off-by: Andrei Kvapil [email protected]
Summary by CodeRabbit