Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fix functionality #22

Merged
merged 7 commits into from
Nov 3, 2024
Merged

Add fix functionality #22

merged 7 commits into from
Nov 3, 2024

Conversation

RoseSecurity
Copy link
Owner

@RoseSecurity RoseSecurity commented Nov 2, 2024

Why

  • Add the ability to fix and recommend enhancements

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced a fix command for diagnosing configuration errors in Terraform and OpenTofu files.
    • Added a recommend command for personalized recommendations to improve efficiency, security, and performance of configurations.
  • Improvements

    • Updated command structure to transition from flag-based to command-based interactions.
    • Enhanced versioning logic to trigger updates based on the latest release.
  • Bug Fixes

    • Removed obsolete initialization functions, streamlining command registration.
  • Documentation

    • Updated command descriptions for clarity and guidance.
  • Style

    • Renamed internal functions for improved accessibility and clarity.

Copy link

coderabbitai bot commented Nov 2, 2024

Warning

Rate limit exceeded

@RoseSecurity has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 33 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between f8fbd35 and bedf334.

Walkthrough

This pull request introduces several significant changes to the command-line interface (CLI) of the application. It removes the init function from cmd/docs.go, altering how commands are registered. A new command, fix, is added in cmd/fix.go for diagnosing configuration errors, while recommendCmd is introduced in cmd/recommend.go for providing recommendations on Terraform and OpenTofu configurations. The root command's structure is reorganized in cmd/root.go, and the version number is updated in cmd/version.go. Additionally, various internal functions are modified to accommodate these changes.

Changes

File Change Summary
cmd/docs.go Removed init function, affecting automatic registration of docsCmd.
cmd/fix.go Added fixCmd command for diagnosing configuration errors with various flags; introduced Diagnose function.
cmd/recommend.go Added recommendCmd command for analyzing configurations and providing recommendations; introduced Analyze function.
cmd/root.go Updated root command structure, removed previous flags, added docsCmd, recommendCmd, and fixCmd; changed Run function from runAnalyzer to Banner.
cmd/version.go Updated Version from "0.2.0" to "1.0.0"; modified logic in versionCmd to check for newer releases.
internal/analyzer.go Modified Run and printDiff functions to replace model parameter with tool.
internal/llm.go Updated GetRecommendations function to change prompt parameter to tool and adjusted output formatting.
internal/output.go Renamed prettyPrint to PrettyPrint, changing its visibility to exported.

Possibly related PRs

  • Add Fancy TUI #19: The changes in the main PR involve the removal of the init function from cmd/docs.go, which is related to the restructuring of command handling in cmd/root.go. Both PRs focus on command management within the CLI, indicating a connection in how commands are registered and executed.

🐰 In the land of code where rabbits play,
New commands hop in to brighten the day!
With fix and recommend, we’re ready to cheer,
Diagnosing and guiding, there's nothing to fear.
So let’s celebrate changes, both big and small,
In our CLI garden, we’ll flourish for all! 🌼✨


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Signed-off-by: RoseSecurity <72598486+RoseSecurity@users.noreply.github.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

🧹 Outside diff range and nitpick comments (7)
internal/output.go (2)

Line range hint 12-45: Improve error handling to avoid program termination.

Using log.Fatal in a library function is too aggressive as it terminates the program immediately. Instead, return errors to allow callers to handle them appropriately.

Apply this diff:

-func PrettyPrint(markdownContent string) {
+func PrettyPrint(markdownContent string) error {
     if strings.TrimSpace(markdownContent) == "" {
-        log.Fatal("markdown content cannot be empty")
+        return fmt.Errorf("markdown content cannot be empty")
     }
     // ... styling code ...
     renderer, err := glamour.NewTermRenderer(glamour.WithAutoStyle())
     if err != nil {
-        log.Fatal(err)
+        return fmt.Errorf("failed to create renderer: %w", err)
     }
 
     renderedContent, err := renderer.Render(markdownContent)
     if err != nil {
-        log.Fatal(err)
+        return fmt.Errorf("failed to render content: %w", err)
     }
     // ... output code ...
     fmt.Println(output)
+    return nil

Line range hint 14-26: Extract styles and constants for better maintainability.

The styles and colors are hardcoded within the function. Consider extracting them to package-level variables or a dedicated styling package for better reusability and maintenance.

Consider this approach:

+var (
+    // Colors
+    headerColor = "#FF79C6"
+    commentColor = "#8BE9FD"
+    backgroundColor = "#282A36"
+
+    // Styles
+    defaultHeaderStyle = lipgloss.NewStyle().
+        Bold(true).
+        Foreground(lipgloss.Color(headerColor)).
+        Background(lipgloss.Color(backgroundColor)).
+        Padding(1, 2).
+        BorderStyle(lipgloss.NormalBorder()).
+        BorderForeground(lipgloss.Color(headerColor)).
+        Align(lipgloss.Center).
+        Width(75)
+
+    defaultCommentStyle = lipgloss.NewStyle().
+        Foreground(lipgloss.Color(commentColor)).
+        Italic(true).
+        Margin(1, 2)
+)
+
 func PrettyPrint(markdownContent string) {
-    // Create a style for headers
-    headerStyle := lipgloss.NewStyle().
-        Bold(true).
-        Foreground(lipgloss.Color("#FF79C6")).
-        Background(lipgloss.Color("#282A36")).
-        Padding(1, 2).
-        BorderStyle(lipgloss.NormalBorder()).
-        BorderForeground(lipgloss.Color("#FF79C6")).
-        Align(lipgloss.Center).
-        Width(75)
-
-    // Create a style for recommendations
-    commentStyle := lipgloss.NewStyle().
-        Foreground(lipgloss.Color("#8BE9FD")).
-        Italic(true).
-        Margin(1, 2)
+    headerStyle := defaultHeaderStyle
+    commentStyle := defaultCommentStyle
cmd/version.go (1)

Line range hint 63-66: Fix inconsistent project naming.

The function updateTerramaid refers to "Terramaid" in the message, but the project name appears to be "Kuzco" based on the repository URL and command name.

Apply this fix:

func updateTerramaid(latestVersion string) {
	c1 := color.New(color.FgCyan)
-	c1.Println(fmt.Sprintf("\nYour version of Terramaid is out of date. The latest version is %s\n\n", latestVersion))
+	c1.Println(fmt.Sprintf("\nYour version of Kuzco is out of date. The latest version is %s\n\n", latestVersion))
}
internal/analyzer.go (1)

44-44: Consider using a configuration struct to improve maintainability

The function now has 6 parameters, which could make it harder to maintain and modify in the future. Consider introducing a configuration struct to encapsulate these parameters.

Here's a suggested refactoring:

+type DiffConfig struct {
+    Resources []Resource
+    Schema    ProviderSchema
+    Model     string
+    Tool      string
+    Prompt    string
+    Addr      string
+}
+
-func printDiff(resources []Resource, schema ProviderSchema, model, tool, prompt, addr string) error {
+func printDiff(config DiffConfig) error {
cmd/fix.go (1)

12-18: Enhance command documentation for clarity and completeness.

The command documentation could be improved to:

  1. Mention OpenTofu support in the Long description
  2. Include examples with different flag combinations
 var fixCmd = &cobra.Command{
 	Use:     "fix",
 	Short:   "Diagnose configuration errors",
-	Long:    `This command analyzes and diagnoses Terraform configuration errors`,
+	Long:    `This command analyzes and diagnoses configuration errors in Terraform and OpenTofu files, providing detailed recommendations for fixes.`,
-	Example: "kuzco fix -f path/to/config.tf -t terraform",
+	Example: `  # Analyze Terraform configuration
+  kuzco fix -f path/to/config.tf -t terraform
+
+  # Analyze OpenTofu configuration with custom model
+  kuzco fix -f path/to/config.tf -t opentofu -m codellama`,
 	Run:     Diagnose,
 }
internal/llm.go (2)

47-47: Consider adding input validation for the tool parameter.

While the title casing is implemented correctly, consider adding validation to ensure the tool parameter is not empty and contains valid characters.

 func GetRecommendations(resourceType string, unusedAttrs []string, model string, tool string, prompt string, addr string) (string, error) {
+    if tool == "" {
+        return "", fmt.Errorf("tool parameter cannot be empty")
+    }
     tool = cases.Title(language.English, cases.NoLower).String(tool)

Line range hint 50-77: Consider refactoring to reduce prompt template duplication.

The example output section is duplicated in both prompt templates. Consider extracting common parts into constants or a template string to improve maintainability.

+const exampleOutputTemplate = `
Example output:
resource "type" "name" {
  # Enables feature X for improved security
  attribute1 = value1
  
  # Optimizes performance by setting Y
  attribute2 = value2
}`

 func GetRecommendations(resourceType string, unusedAttrs []string, model string, tool string, prompt string, addr string) (string, error) {
     tool = cases.Title(language.English, cases.NoLower).String(tool)
     var formattedPrompt string
     if prompt != "" {
-        formattedPrompt = fmt.Sprintf(`Unused attributes for '%s' resource '%s': %v
-
-For each attribute that should be enabled:
-1. Recommend it as Terraform code
-2. Add a brief comment explaining its purpose
-3. Format as a resource block with comments above uncommented parameters
-
-Example output:
-resource "type" "name" {
-  # Enables feature X for improved security
-  attribute1 = value1
-  
-  # Optimizes performance by setting Y
-  attribute2 = value2
-}`, tool, resourceType, unusedAttrs)
+        formattedPrompt = fmt.Sprintf("Unused attributes for '%s' resource '%s': %v\n\n"+
+            "For each attribute that should be enabled:\n"+
+            "1. Recommend it as Terraform code\n"+
+            "2. Add a brief comment explaining its purpose\n"+
+            "3. Format as a resource block with comments above uncommented parameters\n\n"+
+            "%s", tool, resourceType, unusedAttrs, exampleOutputTemplate)
     } else {
-        formattedPrompt = fmt.Sprintf(`Unused attributes for '%s' resource '%s': %v
-
-'%s'
-
-Example output:
-resource "type" "name" {
-  # Enables feature X for improved security
-  attribute1 = value1
-  
-  # Optimizes performance by setting Y
-  attribute2 = value2
-}`, tool, resourceType, unusedAttrs, prompt)
+        formattedPrompt = fmt.Sprintf("Unused attributes for '%s' resource '%s': %v\n\n"+
+            "'%s'\n\n"+
+            "%s", tool, resourceType, unusedAttrs, prompt, exampleOutputTemplate)
     }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 9bb0918 and f8fbd35.

📒 Files selected for processing (8)
  • cmd/docs.go (0 hunks)
  • cmd/fix.go (1 hunks)
  • cmd/recommend.go (1 hunks)
  • cmd/root.go (2 hunks)
  • cmd/version.go (2 hunks)
  • internal/analyzer.go (1 hunks)
  • internal/llm.go (4 hunks)
  • internal/output.go (1 hunks)
💤 Files with no reviewable changes (1)
  • cmd/docs.go
🔇 Additional comments (13)
internal/output.go (1)

Line range hint 29-33: Consider proper resource cleanup for the renderer.

The renderer might hold system resources. Consider implementing proper cleanup if the glamour.TermRenderer has a Close or similar cleanup method.

Let's check if the renderer has cleanup methods:

cmd/version.go (1)

14-15: Verify version management process.

The comment suggests this is a placeholder for builds, but there's no clear indication of how this value gets replaced during the build process.

Let's check for build-related files and version management:

✅ Verification successful

Version placeholder is correctly managed through build process

The version value is properly managed through the build system. The Makefile shows that the version is injected during build time using Go's -ldflags flag:

-ldflags "-X 'github.com/RoseSecurity/kuzco/cmd.Version=local'"

This is a standard practice in Go applications where the placeholder version in the source code gets replaced with the actual version during compilation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for build configuration files and version management
# Test 1: Look for build scripts or configuration
rg -g '!{.git,vendor}/*' -l 'Version.*=.*[0-9]+\.[0-9]+\.[0-9]+' 
# Test 2: Check for Makefile or similar build tools
fd -g 'Makefile*|*.mk|goreleaser.yml|.goreleaser.yml'

Length of output: 137


Script:

#!/bin/bash
# Let's check for any GitHub Actions workflows or build scripts
fd -e yml -e yaml -p '.github/workflows'

# Look for any Go build tags or ldflags usage that might inject version
rg -g '!{.git,vendor}/*' -i 'ldflags.*version'

# Check for any version-related build commands or scripts
fd -e sh -e bash
rg -g '!{.git,vendor}/*' -i 'build.*version|version.*build'

Length of output: 534

cmd/recommend.go (2)

1-9: LGTM: Clean package structure and imports.

The package name and imports are appropriate and well-organized.


18-24: ⚠️ Potential issue

Add missing variable declarations and input validation.

The flag initialization has several issues:

  1. Global variables used by flags are not declared.
  2. The tool flag needs validation for allowed values.
  3. The default model value might be incorrect.
  4. The addr flag needs URL validation.

Add these declarations at the package level:

+var (
+	filePath string
+	tool     string
+	model    string
+	prompt   string
+	addr     string
+)

Add validation in the init() function:

 func init() {
 	recommendCmd.Flags().StringVarP(&filePath, "file", "f", "", "Path to the Terraform and OpenTofu file (required)")
 	recommendCmd.Flags().StringVarP(&tool, "tool", "t", "terraform", "Specifies the configuration tooling for configurations. Valid values include: `terraform` and `opentofu`")
 	recommendCmd.Flags().StringVarP(&model, "model", "m", "llama3.2", "LLM model to use for generating recommendations")
 	recommendCmd.Flags().StringVarP(&prompt, "prompt", "p", "", "User prompt for guiding the response format of the LLM model")
 	recommendCmd.Flags().StringVarP(&addr, "address", "a", "http://localhost:11434", "IP Address and port to use for the LLM model (ex: http://localhost:11434)")
+
+	// Mark required flags
+	recommendCmd.MarkFlagRequired("file")
+
+	// Add validation
+	recommendCmd.RegisterFlagCompletionFunc("tool", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
+		return []string{"terraform", "opentofu"}, cobra.ShellCompDirectiveNoFileComp
+	})
 }

Also, verify if "llama3.2" is the correct default model version.

internal/analyzer.go (3)

37-37: LGTM: Parameter update is consistent

The modification correctly passes the tool parameter to printDiff, maintaining consistency with the function signature changes.


52-54: LGTM: GetRecommendations parameter update is consistent

The modification correctly passes the tool parameter to GetRecommendations, maintaining consistency with the broader changes in the codebase.


56-56: Verify the intention to export PrettyPrint

The capitalization of PrettyPrint makes it an exported function. Please verify if this change in visibility is intentional and necessary.

✅ Verification successful

Let me check how PrettyPrint is used in these files to verify its usage pattern.


Export of PrettyPrint is intentional and correctly used

The capitalization of PrettyPrint is intentional and necessary as it's defined in internal/output.go and used by cmd/fix.go. Since the cmd package needs to access this function from the internal package, it must be exported. The usage follows Go's package design principles correctly.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if PrettyPrint is used outside the internal package
rg -l "PrettyPrint" | grep -v "internal/analyzer.go"

Length of output: 80


Script:

#!/bin/bash
# Check the context of PrettyPrint usage in both files
echo "=== cmd/fix.go ==="
rg -C 3 "PrettyPrint" cmd/fix.go

echo -e "\n=== internal/output.go ==="
rg -C 3 "PrettyPrint" internal/output.go

Length of output: 424

cmd/fix.go (1)

1-10: LGTM: Imports are well-organized and necessary.

internal/llm.go (2)

12-13: LGTM: Appropriate imports for text case handling.

The added imports from golang.org/x/text are appropriate for implementing the title case functionality.


46-46: Verify the impact of function signature change.

The function signature has been modified to accept both tool and prompt parameters. While this provides more flexibility, we should ensure all existing callers have been updated accordingly.

✅ Verification successful

Since I found the only call to GetRecommendations in internal/analyzer.go and it's already updated with all the required parameters (model, tool, prompt, addr), I can generate the final response.

Function signature change has been properly propagated

The only caller of GetRecommendations in internal/analyzer.go has been correctly updated to match the new signature, passing all required parameters including both tool and prompt.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for all calls to GetRecommendations to verify they've been updated
ast-grep --pattern 'GetRecommendations($$$)'

Length of output: 172

cmd/root.go (3)

31-32: Ensure recommendCmd and fixCmd are properly defined

The commands recommendCmd and fixCmd are being added to rootCmd. Verify that these commands are properly defined and imported to avoid undefined reference errors.

Run the following script to check if recommendCmd and fixCmd are defined:

#!/bin/bash
# Description: Search for the definitions of `recommendCmd` and `fixCmd` in the codebase.

# Test: Search for `var recommendCmd` and `var fixCmd` definitions. Expect: At least one match for each.
rg '^var recommendCmd' --type go
rg '^var fixCmd' --type go

29-29: ⚠️ Potential issue

Potentially undefined docsCmd added to rootCmd

The command docsCmd is being added to rootCmd, but according to the AI summary, docsCmd has been removed from cmd/docs.go. This will result in a compilation error due to an undefined reference.

Run the following script to check if docsCmd is defined in the codebase:

#!/bin/bash
# Description: Search for the definition of `docsCmd` in the codebase.

# Test: Search for `var docsCmd` definitions. Expect: No matches if it has been removed.
rg '^var docsCmd' --type go

If docsCmd is no longer defined, you should remove it from the init function.

Proposed Fix:

 func init() {
-	rootCmd.AddCommand(docsCmd)
 	rootCmd.AddCommand(versionCmd)
 	rootCmd.AddCommand(recommendCmd)
 	rootCmd.AddCommand(fixCmd)
 }

62-63: Ensure banner is properly initialized before printing

The variable banner is printed using fmt.Println(banner). Verify that banner has been correctly initialized without errors from the ascii.Render function.

Run the following script to check if banner is properly set before usage:

✅ Verification successful

Let me verify the error handling by checking the complete context around this code.


banner is properly initialized and error-handled

The code shows correct initialization and error handling:

  • banner is initialized using ascii.Render
  • Error is properly checked and handled with appropriate error message and exit
  • Only after successful initialization, banner is printed
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for proper initialization of `banner` variable.

# Test: Search for all assignments to `banner`. Expect: Correct initialization before use.
rg 'banner, err := ascii.Render' --type go -A 2

Length of output: 209


Script:

#!/bin/bash
# Get more context around the banner initialization and usage
rg 'banner, err := ascii.Render' --type go -B 5 -A 10

Length of output: 606

@@ -9,7 +9,7 @@ import (
"github.com/charmbracelet/lipgloss"
)

func prettyPrint(markdownContent string) {
func PrettyPrint(markdownContent string) {
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add documentation and input validation for the exported function.

Since PrettyPrint is now part of the public API, it should have proper documentation and input validation.

Apply this diff:

+// PrettyPrint formats and displays markdown content with styled headers and recommendations.
+// It applies custom styling using lipgloss and renders markdown using glamour.
+// The output includes a header, the rendered markdown content, and a footer.
 func PrettyPrint(markdownContent string) {
+    if strings.TrimSpace(markdownContent) == "" {
+        log.Fatal("markdown content cannot be empty")
+    }
📝 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
func PrettyPrint(markdownContent string) {
// PrettyPrint formats and displays markdown content with styled headers and recommendations.
// It applies custom styling using lipgloss and renders markdown using glamour.
// The output includes a header, the rendered markdown content, and a footer.
func PrettyPrint(markdownContent string) {
if strings.TrimSpace(markdownContent) == "" {
log.Fatal("markdown content cannot be empty")
}
```
Note: The suggestion assumes that the `strings` package is already imported in the file. If it's not, an additional import statement would be needed at the top of the file:
```go
import "strings"

cmd/version.go Outdated
Comment on lines 31 to 33
currentRelease := strings.TrimPrefix(Version, "v")
if latestRelease != currentRelease {
if latestRelease > currentRelease {
updateTerramaid(latestRelease)
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Use proper semantic version comparison.

String comparison (>) may not correctly handle semantic versioning. For example, "2.0.0" > "10.0.0" would return true because of string comparison.

Consider using a semantic version comparison library. Here's a suggested implementation:

-			if latestRelease > currentRelease {
+			if semver.Compare(latestRelease, currentRelease) > 0 {

Add this import:

import "golang.org/x/mod/semver"

Comment on lines +11 to +16
var recommendCmd = &cobra.Command{
Use: "recommend",
Short: "Intelligently analyze your Terraform and OpenTofu configurations",
Long: `Intelligently analyze your Terraform and OpenTofu configurations to receive personalized recommendations for boosting efficiency, security, and performance.`,
Run: Analyze,
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance command definition with required flags and examples.

Consider these improvements:

  1. Mark required flags using MarkFlagRequired() in the init() function.
  2. Add usage examples using the Example field.

Apply this enhancement:

 var recommendCmd = &cobra.Command{
 	Use:   "recommend",
 	Short: "Intelligently analyze your Terraform and OpenTofu configurations",
 	Long:  `Intelligently analyze your Terraform and OpenTofu configurations to receive personalized recommendations for boosting efficiency, security, and performance.`,
+	Example: `  # Analyze a Terraform file
+  kuzco recommend -f main.tf
+
+  # Analyze an OpenTofu file with a custom model
+  kuzco recommend -f config.tf -t opentofu -m llama2`,
 	Run:   Analyze,
 }

Committable suggestion skipped: line range outside the PR's diff.

cmd/recommend.go Outdated
Comment on lines 26 to 38
func Analyze(cmd *cobra.Command, args []string) {
// Validate that the specified model exists in Ollama
if err := internal.ValidateModel(model, addr); err != nil {
fmt.Fprintf(os.Stderr, "Model validation error: %v\n", err)
os.Exit(1)
}

// Proceed with the main logic if all required flags are set
if err := internal.Run(filePath, tool, model, prompt, addr); err != nil {
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
os.Exit(1)
}
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Enhance error handling and add input validation.

The command execution could be improved with:

  1. Validation of the tool flag value.
  2. More descriptive error messages.
  3. Proper cleanup in case of errors.

Apply these improvements:

 func Analyze(cmd *cobra.Command, args []string) {
+	// Validate tool
+	if tool != "terraform" && tool != "opentofu" {
+		fmt.Fprintf(os.Stderr, "Error: Invalid tool %q. Valid values are: terraform, opentofu\n", tool)
+		os.Exit(1)
+	}
+
 	// Validate that the specified model exists in Ollama
 	if err := internal.ValidateModel(model, addr); err != nil {
-		fmt.Fprintf(os.Stderr, "Model validation error: %v\n", err)
+		fmt.Fprintf(os.Stderr, "Error: Model %q not found in Ollama at %s\n", model, addr)
+		fmt.Fprintf(os.Stderr, "Please ensure the model is installed using: ollama pull %s\n", model)
 		os.Exit(1)
 	}
 
 	// Proceed with the main logic if all required flags are set
 	if err := internal.Run(filePath, tool, model, prompt, addr); err != nil {
-		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
+		fmt.Fprintf(os.Stderr, "Error analyzing %s configuration: %v\n", tool, err)
+		fmt.Fprintf(os.Stderr, "Please ensure the file exists and contains valid configuration\n")
 		os.Exit(1)
 	}
 }
📝 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
func Analyze(cmd *cobra.Command, args []string) {
// Validate that the specified model exists in Ollama
if err := internal.ValidateModel(model, addr); err != nil {
fmt.Fprintf(os.Stderr, "Model validation error: %v\n", err)
os.Exit(1)
}
// Proceed with the main logic if all required flags are set
if err := internal.Run(filePath, tool, model, prompt, addr); err != nil {
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
os.Exit(1)
}
}
func Analyze(cmd *cobra.Command, args []string) {
// Validate tool
if tool != "terraform" && tool != "opentofu" {
fmt.Fprintf(os.Stderr, "Error: Invalid tool %q. Valid values are: terraform, opentofu\n", tool)
os.Exit(1)
}
// Validate that the specified model exists in Ollama
if err := internal.ValidateModel(model, addr); err != nil {
fmt.Fprintf(os.Stderr, "Error: Model %q not found in Ollama at %s\n", model, addr)
fmt.Fprintf(os.Stderr, "Please ensure the model is installed using: ollama pull %s\n", model)
os.Exit(1)
}
// Proceed with the main logic if all required flags are set
if err := internal.Run(filePath, tool, model, prompt, addr); err != nil {
fmt.Fprintf(os.Stderr, "Error analyzing %s configuration: %v\n", tool, err)
fmt.Fprintf(os.Stderr, "Please ensure the file exists and contains valid configuration\n")
os.Exit(1)
}
}

cmd/fix.go Outdated
Comment on lines 46 to 68
// Generate a formatted prompt for the recommendations function
formattedPrompt := fmt.Sprintf(`Error detected in configuration file '%s':

Error Details:
%%v

Resolution Steps:
1. Identify the attribute(s) causing the error.
2. Refer to the Terraform or OpenTofu documentation for valid syntax and attribute usage for this resource type.
3. Correct the invalid attribute(s) or remove them if they are unnecessary.
4. Reformat the corrected resource block if needed.

Example Corrected Configuration:
resource "type" "name" {
# Explanation of attribute1's purpose
attribute1 = "value1"

# Optional comment for attribute2
attribute2 = "value2"
}

Please review and update the configuration file as outlined above to resolve the issue.`, filePath)

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Review and fix the prompt template.

The prompt template contains a %%v placeholder that's never used. Additionally, the example configuration is quite generic and could be more specific to Terraform/OpenTofu resources.

-	formattedPrompt := fmt.Sprintf(`Error detected in configuration file '%s':
-
-Error Details:
-%%v
-
-Resolution Steps:
+	formattedPrompt := fmt.Sprintf(`Analyze the following configuration file '%s' for potential errors and provide:

+1. Detailed error analysis
+2. Resolution steps with references to official documentation
+3. Corrected configuration example specific to the resource type
+4. Best practices and security considerations

+Configuration Analysis Guidelines:
 1. Identify the attribute(s) causing the error.
 2. Refer to the Terraform or OpenTofu documentation for valid syntax and attribute usage for this resource type.
 3. Correct the invalid attribute(s) or remove them if they are unnecessary.
 4. Reformat the corrected resource block if needed.

-Example Corrected Configuration:
-resource "type" "name" {
-  # Explanation of attribute1's purpose
-  attribute1 = "value1"
-  
-  # Optional comment for attribute2
-  attribute2 = "value2"
-}
-
-Please review and update the configuration file as outlined above to resolve the issue.`, filePath)
+Please provide a detailed analysis following these guidelines.`, filePath)
📝 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
// Generate a formatted prompt for the recommendations function
formattedPrompt := fmt.Sprintf(`Error detected in configuration file '%s':
Error Details:
%%v
Resolution Steps:
1. Identify the attribute(s) causing the error.
2. Refer to the Terraform or OpenTofu documentation for valid syntax and attribute usage for this resource type.
3. Correct the invalid attribute(s) or remove them if they are unnecessary.
4. Reformat the corrected resource block if needed.
Example Corrected Configuration:
resource "type" "name" {
# Explanation of attribute1's purpose
attribute1 = "value1"
# Optional comment for attribute2
attribute2 = "value2"
}
Please review and update the configuration file as outlined above to resolve the issue.`, filePath)
// Generate a formatted prompt for the recommendations function
formattedPrompt := fmt.Sprintf(`Analyze the following configuration file '%s' for potential errors and provide:
1. Detailed error analysis
2. Resolution steps with references to official documentation
3. Corrected configuration example specific to the resource type
4. Best practices and security considerations
Configuration Analysis Guidelines:
1. Identify the attribute(s) causing the error.
2. Refer to the Terraform or OpenTofu documentation for valid syntax and attribute usage for this resource type.
3. Correct the invalid attribute(s) or remove them if they are unnecessary.
4. Reformat the corrected resource block if needed.
Please provide a detailed analysis following these guidelines.`, filePath)

Comment on lines +27 to +44
func Diagnose(cmd *cobra.Command, args []string) {
// Ensure file path is provided
if filePath == "" {
fmt.Fprintf(os.Stderr, "Error: file path is required. Use the -f flag to specify the configuration file.\n")
os.Exit(1)
}

// Validate that the specified model exists in Ollama
if err := internal.ValidateModel(model, addr); err != nil {
fmt.Fprintf(os.Stderr, "Model validation error: %v\n", err)
os.Exit(1)
}

// Read the configuration file content
config, err := os.ReadFile(filePath)
if err != nil {
u.LogErrorAndExit(err)
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add validation for empty configuration files.

The function reads the file but doesn't validate if the content is empty.

 	config, err := os.ReadFile(filePath)
 	if err != nil {
 		u.LogErrorAndExit(err)
 	}
+	if len(config) == 0 {
+		fmt.Fprintf(os.Stderr, "Error: configuration file is empty\n")
+		os.Exit(1)
+	}
📝 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
func Diagnose(cmd *cobra.Command, args []string) {
// Ensure file path is provided
if filePath == "" {
fmt.Fprintf(os.Stderr, "Error: file path is required. Use the -f flag to specify the configuration file.\n")
os.Exit(1)
}
// Validate that the specified model exists in Ollama
if err := internal.ValidateModel(model, addr); err != nil {
fmt.Fprintf(os.Stderr, "Model validation error: %v\n", err)
os.Exit(1)
}
// Read the configuration file content
config, err := os.ReadFile(filePath)
if err != nil {
u.LogErrorAndExit(err)
}
func Diagnose(cmd *cobra.Command, args []string) {
// Ensure file path is provided
if filePath == "" {
fmt.Fprintf(os.Stderr, "Error: file path is required. Use the -f flag to specify the configuration file.\n")
os.Exit(1)
}
// Validate that the specified model exists in Ollama
if err := internal.ValidateModel(model, addr); err != nil {
fmt.Fprintf(os.Stderr, "Model validation error: %v\n", err)
os.Exit(1)
}
// Read the configuration file content
config, err := os.ReadFile(filePath)
if err != nil {
u.LogErrorAndExit(err)
}
if len(config) == 0 {
fmt.Fprintf(os.Stderr, "Error: configuration file is empty\n")
os.Exit(1)
}

Comment on lines +69 to +76
// Pass the prompt and file content to GetRecommendations
recommendations, err := internal.GetRecommendations(string(config), nil, model, tool, formattedPrompt, addr)
if err != nil {
u.LogErrorAndExit(err)
}

internal.PrettyPrint(recommendations)
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add context handling and improve error handling.

Several improvements needed:

  1. Add context with timeout for long-running operations
  2. Remove the nil argument being passed to GetRecommendations
  3. Ensure consistent error output handling
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer cancel()
+
 	// Pass the prompt and file content to GetRecommendations
-	recommendations, err := internal.GetRecommendations(string(config), nil, model, tool, formattedPrompt, addr)
+	recommendations, err := internal.GetRecommendations(ctx, string(config), model, tool, formattedPrompt, addr)
 	if err != nil {
 		u.LogErrorAndExit(err)
 	}
 
-	internal.PrettyPrint(recommendations)
+	if err := internal.PrettyPrint(recommendations); err != nil {
+		u.LogErrorAndExit(err)
+	}

Don't forget to add the required imports:

 import (
+	"context"
 	"fmt"
 	"os"
+	"time"
 
 	"github.com/RoseSecurity/kuzco/internal"
 	u "github.com/RoseSecurity/kuzco/pkg/utils"
 	"github.com/spf13/cobra"
 )
📝 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
// Pass the prompt and file content to GetRecommendations
recommendations, err := internal.GetRecommendations(string(config), nil, model, tool, formattedPrompt, addr)
if err != nil {
u.LogErrorAndExit(err)
}
internal.PrettyPrint(recommendations)
}
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
// Pass the prompt and file content to GetRecommendations
recommendations, err := internal.GetRecommendations(ctx, string(config), model, tool, formattedPrompt, addr)
if err != nil {
u.LogErrorAndExit(err)
}
if err := internal.PrettyPrint(recommendations); err != nil {
u.LogErrorAndExit(err)
}
}

Comment on lines +20 to +25
func init() {
fixCmd.Flags().StringVarP(&filePath, "file", "f", "", "Path to the Terraform and OpenTofu file (required)")
fixCmd.Flags().StringVarP(&tool, "tool", "t", "terraform", "Specifies the configuration tooling for configurations. Valid values include: `terraform` and `opentofu`")
fixCmd.Flags().StringVarP(&model, "model", "m", "llama3.2", "LLM model to use for generating recommendations")
fixCmd.Flags().StringVarP(&addr, "address", "a", "http://localhost:11434", "IP Address and port to use for the LLM model (ex: http://localhost:11434)")
}
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Add required flag marking and tool validation

The review comment is correct. Based on the codebase analysis:

  1. The file flag is required as shown in usage but not marked using MarkFlagRequired anywhere
  2. The tool flag accepts specific values (terraform and opentofu) but lacks validation
  3. The variables are defined in cmd/root.go and shared across commands

The suggested changes to add flag requirement marking and input validation are necessary to improve CLI robustness.

🔗 Analysis chain

Add required flag marking and input validation.

  1. The file flag is required but not marked as required using MarkFlagRequired
  2. The tool flag accepts specific values but lacks validation
 func init() {
 	fixCmd.Flags().StringVarP(&filePath, "file", "f", "", "Path to the Terraform and OpenTofu file (required)")
 	fixCmd.Flags().StringVarP(&tool, "tool", "t", "terraform", "Specifies the configuration tooling for configurations. Valid values include: `terraform` and `opentofu`")
 	fixCmd.Flags().StringVarP(&model, "model", "m", "llama3.2", "LLM model to use for generating recommendations")
 	fixCmd.Flags().StringVarP(&addr, "address", "a", "http://localhost:11434", "IP Address and port to use for the LLM model (ex: http://localhost:11434)")
+	
+	// Mark required flags
+	fixCmd.MarkFlagRequired("file")
+	
+	// Add validation for tool flag
+	fixCmd.RegisterFlagCompletionFunc("tool", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
+		return []string{"terraform", "opentofu"}, cobra.ShellCompDirectiveNoFileComp
+	})
 }
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify if the global variables are defined elsewhere
rg -A 1 "var \b(filePath|tool|model|addr)\b" cmd/

Length of output: 51


Script:

#!/bin/bash
# Let's try a different approach to find variable declarations
ast-grep --pattern 'var $_ string'

# Also search for these variables in the entire cmd directory
rg "(filePath|tool|model|addr)" cmd/

Length of output: 2466


Script:

#!/bin/bash
# Let's check if there's any existing validation for the tool flag
rg -A 5 "ValidateTool|tool.*valid|validateTool" cmd/ internal/

# Also check if there's any existing flag requirement marking
rg "MarkFlagRequired" cmd/

Length of output: 91

cmd/root.go Outdated
Comment on lines 54 to 59
options.FontColor = []figlet4go.Color{
color, // Magenta
}
banner, err := ascii.Render("Kuzco")
if err != nil {
fmt.Fprintf(os.Stderr, "Error generating ASCII banner: %v\n", err)
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Undefined variables and missing imports

  • The variable options is used but not defined in this context.
  • The variable color is used but not defined.
  • The package figlet4go is referenced (figlet4go.Color), but it's not imported.
  • The ascii package is used (ascii.Render("Kuzco")), but it is not imported.

These issues will lead to compilation errors.

Proposed Fix:

  • Import the necessary packages.
  • Define or initialize the options and color variables.

Add the required imports:

 import (
 	"fmt"
 	"os"

 	"github.com/RoseSecurity/kuzco/internal"
 	tuiUtils "github.com/RoseSecurity/kuzco/internal/tui/utils"
 	u "github.com/RoseSecurity/kuzco/pkg/utils"
 	"github.com/spf13/cobra"
+	"github.com/common-nighthawk/go-figure" // For ASCII art rendering
+	"github.com/mbndr/figlet4go"
 )

Define and initialize options and color:

+var ascii = figlet4go.NewAsciiRender()

+options := figlet4go.NewRenderOptions()
+color := []figlet4go.Color{
+	figlet4go.ColorMagenta,
+}
+options.FontColor = color

Adjust the ascii.Render call to include options:

-banner, err := ascii.Render("Kuzco")
+banner, err := ascii.RenderOpts("Kuzco", options)

Committable suggestion skipped: line range outside the PR's diff.

cmd/root.go Outdated
Comment on lines 24 to 25
Long: `Intelligently analyze your Terraform and OpenTofu configurations to receive personalized recommendations and fixes for boosting efficiency, security, and performance.`,
Run: Banner,
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

⚠️ Potential issue

Undefined function Banner will cause compilation error

The verification confirms that the Banner function is not defined in the codebase:

  • No function definition found in any Go files
  • Not available in any of the imported packages
  • Not defined in the TUI utilities

The Run field needs to be assigned a valid function to prevent compilation errors.

🔗 Analysis chain

Undefined function Banner assigned to Run

The Run field of rootCmd is set to Banner, but there is no definition of a Banner function in this file or imported packages. This will cause a compilation error.

Run the following script to check if the Banner function is defined in the codebase:

If the Banner function is not defined, consider defining it or changing the Run field back to runAnalyzer.

Proposed Fix:

 var rootCmd = &cobra.Command{
 	Use:   "kuzco",
 	Short: "Intelligently analyze your Terraform and OpenTofu configurations",
 	Long:  `Intelligently analyze your Terraform and OpenTofu configurations to receive personalized recommendations and fixes for boosting efficiency, security, and performance.`,
-	Run:   Banner,
+	Run:   runAnalyzer,
 }

Let me search for the Banner function with a different approach to ensure we haven't missed any definitions.


Let me check one more thing to see if the Banner function might be defined in any of the imported packages, particularly in the TUI utils since it seems related to display functionality.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for the definition of the `Banner` function in the codebase.

# Test: Search for `func Banner` definitions. Expect: At least one match.
rg '^func Banner' --type go

Length of output: 155


Script:

#!/bin/bash
# Search for Banner function with more context and different patterns
rg "Banner" --type go -A 3

# Also search for any imports that might contain Banner
rg "import \(" --type go -A 10

Length of output: 5524


Script:

#!/bin/bash
# Check the content of internal/tui/utils/utils.go for any Banner-related functions
cat internal/tui/utils/utils.go

Length of output: 406

@RoseSecurity RoseSecurity merged commit 7f665a4 into main Nov 3, 2024
2 checks passed
@RoseSecurity RoseSecurity deleted the add-fix-functionality branch November 3, 2024 00:06
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