From cb85d034719c40cfb4cea302e6acdc202ae53c7b Mon Sep 17 00:00:00 2001 From: Pulak Kanti Bhowmick Date: Wed, 6 Nov 2024 01:10:06 +0600 Subject: [PATCH 1/8] detect cycle in cli custom commands Signed-off-by: Pulak Kanti Bhowmick --- cmd/cmd_utils.go | 64 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/cmd/cmd_utils.go b/cmd/cmd_utils.go index 2477679de..02cc8efea 100644 --- a/cmd/cmd_utils.go +++ b/cmd/cmd_utils.go @@ -33,6 +33,66 @@ func WithStackValidation(check bool) AtmosValidateOption { } } +func detectCycle(commands []schema.Command) bool { + // Build a command graph + graph := make(map[string][]string) + for _, cmd := range commands { + for _, step := range cmd.Steps { + // Add an edge from command to each command it depends on + graph[cmd.Name] = append(graph[cmd.Name], parseCommandName(step)) + } + } + + // To track visited nodes and detect cycles + visited := make(map[string]bool) + recStack := make(map[string]bool) + + // Run DFS for each command to detect cycles + for cmd := range graph { + if detectCycleUtil(cmd, graph, visited, recStack) { + return true // Cycle detected + } + } + return false // No cycle detected +} + +func detectCycleUtil(command string, graph map[string][]string, visited, recStack map[string]bool) bool { + // If the current command is in the recursion stack, there's a cycle + if recStack[command] { + return true + } + + // If already visited, no need to explore again + if visited[command] { + return false + } + + // Mark as visited and add to recursion stack + visited[command] = true + recStack[command] = true + + // Recurse for all dependencies + for _, dep := range graph[command] { + if detectCycleUtil(dep, graph, visited, recStack) { + return true + } + } + + // Remove from recursion stack before backtracking + recStack[command] = false + return false +} + +// Helper function to parse command name from the step +func parseCommandName(step string) string { + // Assuming the format "atmos " + parts := strings.Split(step, " ") + if len(parts) == 2 && parts[0] == "atmos" { + return parts[1] + } + return "" +} + // processCustomCommands processes and executes custom commands func processCustomCommands( cliConfig schema.CliConfiguration, @@ -47,6 +107,10 @@ func processCustomCommands( existingTopLevelCommands = getTopLevelCommands() } + if detectCycle(commands) { + return errors.New("cycle detected in custom cli commands") + } + for _, commandCfg := range commands { // Clone the 'commandCfg' struct into a local variable because of the automatic closure in the `Run` function of the Cobra command. // Cloning will make a closure over the local variable 'commandConfig' which is different in each iteration. From 91ad5e6cc2f7e6806b8ef5b6b02e753fdfe7226d Mon Sep 17 00:00:00 2001 From: Pulak Kanti Bhowmick Date: Wed, 6 Nov 2024 01:19:16 +0600 Subject: [PATCH 2/8] update command parsing Signed-off-by: Pulak Kanti Bhowmick --- cmd/cmd_utils.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/cmd/cmd_utils.go b/cmd/cmd_utils.go index 02cc8efea..11df9c9b2 100644 --- a/cmd/cmd_utils.go +++ b/cmd/cmd_utils.go @@ -85,10 +85,13 @@ func detectCycleUtil(command string, graph map[string][]string, visited, recStac // Helper function to parse command name from the step func parseCommandName(step string) string { - // Assuming the format "atmos " + // Split the step into parts parts := strings.Split(step, " ") - if len(parts) == 2 && parts[0] == "atmos" { - return parts[1] + + // Check if the command starts with "atmos" and has additional parts + if len(parts) > 1 && parts[0] == "atmos" { + // Return everything after "atmos" as a single string + return strings.Join(parts[1:], " ") } return "" } From 7517136c775cc82f05972df968e72c16331dbf86 Mon Sep 17 00:00:00 2001 From: Pulak Kanti Bhowmick Date: Sat, 23 Nov 2024 13:14:40 +0600 Subject: [PATCH 3/8] various update Signed-off-by: Pulak Kanti Bhowmick --- cmd/cmd_utils.go | 61 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 52 insertions(+), 9 deletions(-) diff --git a/cmd/cmd_utils.go b/cmd/cmd_utils.go index dd164b15b..51f08296b 100644 --- a/cmd/cmd_utils.go +++ b/cmd/cmd_utils.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path" + "strconv" "strings" "github.com/fatih/color" @@ -18,6 +19,11 @@ import ( u "github.com/cloudposse/atmos/pkg/utils" ) +const ( + AtmosCommandMaxDepthEnv = "ATMOS_COMMAND_MAX_DEPTH" + AtmosCommandDefaultDepth = 10 +) + // ValidateConfig holds configuration options for Atmos validation. // CheckStack determines whether stack configuration validation should be performed. type ValidateConfig struct { @@ -33,13 +39,32 @@ func WithStackValidation(check bool) AtmosValidateOption { } } +func getAtmosCommandMaxDepth() int { + maxDepth := os.Getenv(AtmosCommandMaxDepthEnv) + if maxDepth == "" { + return AtmosCommandDefaultDepth + } + if val, err := strconv.Atoi(maxDepth); err == nil { + return val + } + return AtmosCommandDefaultDepth +} + func detectCycle(commands []schema.Command) bool { + // Create a map of valid commands for quick lookup + validCommands := make(map[string]bool) + for _, cmd := range commands { + validCommands[cmd.Name] = true + } // Build a command graph graph := make(map[string][]string) for _, cmd := range commands { for _, step := range cmd.Steps { - // Add an edge from command to each command it depends on - graph[cmd.Name] = append(graph[cmd.Name], parseCommandName(step)) + cmdName := parseCommandName(step) + if cmdName != "" && !validCommands[cmdName] { + return true + } + graph[cmd.Name] = append(graph[cmd.Name], cmdName) } } @@ -47,16 +72,27 @@ func detectCycle(commands []schema.Command) bool { visited := make(map[string]bool) recStack := make(map[string]bool) - // Run DFS for each command to detect cycles + // Track the maximum recursion depth + maxDepth := getAtmosCommandMaxDepth() + + // Run DFS for each command to detect cycles and compute depth for cmd := range graph { - if detectCycleUtil(cmd, graph, visited, recStack) { + if detectCycleUtil(cmd, graph, visited, recStack, 0, &maxDepth) { return true // Cycle detected } } + + // Print or return the max depth if needed + fmt.Println("Maximum Recursion Depth:", maxDepth) return false // No cycle detected } -func detectCycleUtil(command string, graph map[string][]string, visited, recStack map[string]bool) bool { +func detectCycleUtil(command string, graph map[string][]string, visited, recStack map[string]bool, depth int, maxDepth *int) bool { + // Update the maximum recursion depth + if depth > *maxDepth { + *maxDepth = depth + } + // If the current command is in the recursion stack, there's a cycle if recStack[command] { return true @@ -73,7 +109,7 @@ func detectCycleUtil(command string, graph map[string][]string, visited, recStac // Recurse for all dependencies for _, dep := range graph[command] { - if detectCycleUtil(dep, graph, visited, recStack) { + if detectCycleUtil(dep, graph, visited, recStack, depth+1, maxDepth) { return true } } @@ -90,8 +126,15 @@ func parseCommandName(step string) string { // Check if the command starts with "atmos" and has additional parts if len(parts) > 1 && parts[0] == "atmos" { - // Return everything after "atmos" as a single string - return strings.Join(parts[1:], " ") + // Extract the actual command name, handling flags and arguments + cmdParts := []string{} + for _, part := range parts[1:] { + if strings.HasPrefix(part, "-") { + break + } + cmdParts = append(cmdParts, part) + } + return strings.Join(cmdParts, " ") } return "" } @@ -111,7 +154,7 @@ func processCustomCommands( } if detectCycle(commands) { - return errors.New("cycle detected in custom cli commands") + return fmt.Errorf("cycle detected in custom CLI commands - this could lead to infinite recursion") } for _, commandCfg := range commands { From 41dd88567667863ffc4f469aa0eb0719c24e1945 Mon Sep 17 00:00:00 2001 From: Pulak Kanti Bhowmick Date: Sun, 24 Nov 2024 09:52:02 +0600 Subject: [PATCH 4/8] allow command executation for some levels Signed-off-by: Pulak Kanti Bhowmick --- cmd/cmd_utils.go | 103 +++++------------------------------------------ 1 file changed, 11 insertions(+), 92 deletions(-) diff --git a/cmd/cmd_utils.go b/cmd/cmd_utils.go index 51f08296b..2ae20cee7 100644 --- a/cmd/cmd_utils.go +++ b/cmd/cmd_utils.go @@ -50,95 +50,6 @@ func getAtmosCommandMaxDepth() int { return AtmosCommandDefaultDepth } -func detectCycle(commands []schema.Command) bool { - // Create a map of valid commands for quick lookup - validCommands := make(map[string]bool) - for _, cmd := range commands { - validCommands[cmd.Name] = true - } - // Build a command graph - graph := make(map[string][]string) - for _, cmd := range commands { - for _, step := range cmd.Steps { - cmdName := parseCommandName(step) - if cmdName != "" && !validCommands[cmdName] { - return true - } - graph[cmd.Name] = append(graph[cmd.Name], cmdName) - } - } - - // To track visited nodes and detect cycles - visited := make(map[string]bool) - recStack := make(map[string]bool) - - // Track the maximum recursion depth - maxDepth := getAtmosCommandMaxDepth() - - // Run DFS for each command to detect cycles and compute depth - for cmd := range graph { - if detectCycleUtil(cmd, graph, visited, recStack, 0, &maxDepth) { - return true // Cycle detected - } - } - - // Print or return the max depth if needed - fmt.Println("Maximum Recursion Depth:", maxDepth) - return false // No cycle detected -} - -func detectCycleUtil(command string, graph map[string][]string, visited, recStack map[string]bool, depth int, maxDepth *int) bool { - // Update the maximum recursion depth - if depth > *maxDepth { - *maxDepth = depth - } - - // If the current command is in the recursion stack, there's a cycle - if recStack[command] { - return true - } - - // If already visited, no need to explore again - if visited[command] { - return false - } - - // Mark as visited and add to recursion stack - visited[command] = true - recStack[command] = true - - // Recurse for all dependencies - for _, dep := range graph[command] { - if detectCycleUtil(dep, graph, visited, recStack, depth+1, maxDepth) { - return true - } - } - - // Remove from recursion stack before backtracking - recStack[command] = false - return false -} - -// Helper function to parse command name from the step -func parseCommandName(step string) string { - // Split the step into parts - parts := strings.Split(step, " ") - - // Check if the command starts with "atmos" and has additional parts - if len(parts) > 1 && parts[0] == "atmos" { - // Extract the actual command name, handling flags and arguments - cmdParts := []string{} - for _, part := range parts[1:] { - if strings.HasPrefix(part, "-") { - break - } - cmdParts = append(cmdParts, part) - } - return strings.Join(cmdParts, " ") - } - return "" -} - // processCustomCommands processes and executes custom commands func processCustomCommands( cliConfig schema.CliConfiguration, @@ -153,9 +64,9 @@ func processCustomCommands( existingTopLevelCommands = getTopLevelCommands() } - if detectCycle(commands) { - return fmt.Errorf("cycle detected in custom CLI commands - this could lead to infinite recursion") - } + // Track the execution count for each command + visitCount := make(map[string]int) + maxIterations := getAtmosCommandMaxDepth() // Default to 10 or from environment for _, commandCfg := range commands { // Clone the 'commandCfg' struct into a local variable because of the automatic closure in the `Run` function of the Cobra command. @@ -177,6 +88,14 @@ func processCustomCommands( preCustomCommand(cmd, args, parentCommand, commandConfig) }, Run: func(cmd *cobra.Command, args []string) { + // Increment the visit count for the command + visitCount[commandConfig.Name]++ + if visitCount[commandConfig.Name] > maxIterations { + u.LogWarning(cliConfig, fmt.Sprintf("Command '%s' reached max iteration limit (%d). Skipping further execution.\n", commandConfig.Name, maxIterations)) + return + } + + // Execute the command if under the limit executeCustomCommand(cliConfig, cmd, args, parentCommand, commandConfig) }, } From 1420979115ac173a2d72d4c1424fb98ad7e8821c Mon Sep 17 00:00:00 2001 From: Pulak Kanti Bhowmick Date: Sun, 24 Nov 2024 11:52:31 +0600 Subject: [PATCH 5/8] Revert "allow command executation for some levels" This reverts commit 41dd88567667863ffc4f469aa0eb0719c24e1945. --- cmd/cmd_utils.go | 103 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 92 insertions(+), 11 deletions(-) diff --git a/cmd/cmd_utils.go b/cmd/cmd_utils.go index 2ae20cee7..51f08296b 100644 --- a/cmd/cmd_utils.go +++ b/cmd/cmd_utils.go @@ -50,6 +50,95 @@ func getAtmosCommandMaxDepth() int { return AtmosCommandDefaultDepth } +func detectCycle(commands []schema.Command) bool { + // Create a map of valid commands for quick lookup + validCommands := make(map[string]bool) + for _, cmd := range commands { + validCommands[cmd.Name] = true + } + // Build a command graph + graph := make(map[string][]string) + for _, cmd := range commands { + for _, step := range cmd.Steps { + cmdName := parseCommandName(step) + if cmdName != "" && !validCommands[cmdName] { + return true + } + graph[cmd.Name] = append(graph[cmd.Name], cmdName) + } + } + + // To track visited nodes and detect cycles + visited := make(map[string]bool) + recStack := make(map[string]bool) + + // Track the maximum recursion depth + maxDepth := getAtmosCommandMaxDepth() + + // Run DFS for each command to detect cycles and compute depth + for cmd := range graph { + if detectCycleUtil(cmd, graph, visited, recStack, 0, &maxDepth) { + return true // Cycle detected + } + } + + // Print or return the max depth if needed + fmt.Println("Maximum Recursion Depth:", maxDepth) + return false // No cycle detected +} + +func detectCycleUtil(command string, graph map[string][]string, visited, recStack map[string]bool, depth int, maxDepth *int) bool { + // Update the maximum recursion depth + if depth > *maxDepth { + *maxDepth = depth + } + + // If the current command is in the recursion stack, there's a cycle + if recStack[command] { + return true + } + + // If already visited, no need to explore again + if visited[command] { + return false + } + + // Mark as visited and add to recursion stack + visited[command] = true + recStack[command] = true + + // Recurse for all dependencies + for _, dep := range graph[command] { + if detectCycleUtil(dep, graph, visited, recStack, depth+1, maxDepth) { + return true + } + } + + // Remove from recursion stack before backtracking + recStack[command] = false + return false +} + +// Helper function to parse command name from the step +func parseCommandName(step string) string { + // Split the step into parts + parts := strings.Split(step, " ") + + // Check if the command starts with "atmos" and has additional parts + if len(parts) > 1 && parts[0] == "atmos" { + // Extract the actual command name, handling flags and arguments + cmdParts := []string{} + for _, part := range parts[1:] { + if strings.HasPrefix(part, "-") { + break + } + cmdParts = append(cmdParts, part) + } + return strings.Join(cmdParts, " ") + } + return "" +} + // processCustomCommands processes and executes custom commands func processCustomCommands( cliConfig schema.CliConfiguration, @@ -64,9 +153,9 @@ func processCustomCommands( existingTopLevelCommands = getTopLevelCommands() } - // Track the execution count for each command - visitCount := make(map[string]int) - maxIterations := getAtmosCommandMaxDepth() // Default to 10 or from environment + if detectCycle(commands) { + return fmt.Errorf("cycle detected in custom CLI commands - this could lead to infinite recursion") + } for _, commandCfg := range commands { // Clone the 'commandCfg' struct into a local variable because of the automatic closure in the `Run` function of the Cobra command. @@ -88,14 +177,6 @@ func processCustomCommands( preCustomCommand(cmd, args, parentCommand, commandConfig) }, Run: func(cmd *cobra.Command, args []string) { - // Increment the visit count for the command - visitCount[commandConfig.Name]++ - if visitCount[commandConfig.Name] > maxIterations { - u.LogWarning(cliConfig, fmt.Sprintf("Command '%s' reached max iteration limit (%d). Skipping further execution.\n", commandConfig.Name, maxIterations)) - return - } - - // Execute the command if under the limit executeCustomCommand(cliConfig, cmd, args, parentCommand, commandConfig) }, } From 3214acc59ee2fbab8854e19278d25eb7475ccc59 Mon Sep 17 00:00:00 2001 From: Pulak Kanti Bhowmick Date: Sun, 24 Nov 2024 12:03:00 +0600 Subject: [PATCH 6/8] take input from user while cycle detected Signed-off-by: Pulak Kanti Bhowmick --- cmd/cmd_utils.go | 44 +++++++++++++------------------------------- 1 file changed, 13 insertions(+), 31 deletions(-) diff --git a/cmd/cmd_utils.go b/cmd/cmd_utils.go index 51f08296b..44a5f8819 100644 --- a/cmd/cmd_utils.go +++ b/cmd/cmd_utils.go @@ -6,7 +6,6 @@ import ( "fmt" "os" "path" - "strconv" "strings" "github.com/fatih/color" @@ -19,11 +18,6 @@ import ( u "github.com/cloudposse/atmos/pkg/utils" ) -const ( - AtmosCommandMaxDepthEnv = "ATMOS_COMMAND_MAX_DEPTH" - AtmosCommandDefaultDepth = 10 -) - // ValidateConfig holds configuration options for Atmos validation. // CheckStack determines whether stack configuration validation should be performed. type ValidateConfig struct { @@ -39,17 +33,6 @@ func WithStackValidation(check bool) AtmosValidateOption { } } -func getAtmosCommandMaxDepth() int { - maxDepth := os.Getenv(AtmosCommandMaxDepthEnv) - if maxDepth == "" { - return AtmosCommandDefaultDepth - } - if val, err := strconv.Atoi(maxDepth); err == nil { - return val - } - return AtmosCommandDefaultDepth -} - func detectCycle(commands []schema.Command) bool { // Create a map of valid commands for quick lookup validCommands := make(map[string]bool) @@ -72,27 +55,17 @@ func detectCycle(commands []schema.Command) bool { visited := make(map[string]bool) recStack := make(map[string]bool) - // Track the maximum recursion depth - maxDepth := getAtmosCommandMaxDepth() - // Run DFS for each command to detect cycles and compute depth for cmd := range graph { - if detectCycleUtil(cmd, graph, visited, recStack, 0, &maxDepth) { + if detectCycleUtil(cmd, graph, visited, recStack) { return true // Cycle detected } } - // Print or return the max depth if needed - fmt.Println("Maximum Recursion Depth:", maxDepth) return false // No cycle detected } -func detectCycleUtil(command string, graph map[string][]string, visited, recStack map[string]bool, depth int, maxDepth *int) bool { - // Update the maximum recursion depth - if depth > *maxDepth { - *maxDepth = depth - } - +func detectCycleUtil(command string, graph map[string][]string, visited, recStack map[string]bool) bool { // If the current command is in the recursion stack, there's a cycle if recStack[command] { return true @@ -109,7 +82,7 @@ func detectCycleUtil(command string, graph map[string][]string, visited, recStac // Recurse for all dependencies for _, dep := range graph[command] { - if detectCycleUtil(dep, graph, visited, recStack, depth+1, maxDepth) { + if detectCycleUtil(dep, graph, visited, recStack) { return true } } @@ -154,7 +127,16 @@ func processCustomCommands( } if detectCycle(commands) { - return fmt.Errorf("cycle detected in custom CLI commands - this could lead to infinite recursion") + u.LogWarning(cliConfig, "cycle detected in custom CLI commands") + // Prompt the user for input + u.LogInfo(cliConfig, "Do you want to continue? (y/n):") + var userInput string + fmt.Scanln(&userInput) + + if strings.ToLower(userInput) != "y" { + return errors.New("execution stopped due to cycle detection") + } + u.LogWarning(cliConfig, "Continuing execution despite the detected cycle.") } for _, commandCfg := range commands { From 3498b40b7016f158dc9c0dc3766a8fb336e834e2 Mon Sep 17 00:00:00 2001 From: Pulak Kanti Bhowmick Date: Sun, 24 Nov 2024 12:13:41 +0600 Subject: [PATCH 7/8] add timeout for user input Signed-off-by: Pulak Kanti Bhowmick --- cmd/cmd_utils.go | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/cmd/cmd_utils.go b/cmd/cmd_utils.go index 44a5f8819..6b57cd49a 100644 --- a/cmd/cmd_utils.go +++ b/cmd/cmd_utils.go @@ -7,6 +7,7 @@ import ( "os" "path" "strings" + "time" "github.com/fatih/color" "github.com/spf13/cobra" @@ -130,13 +131,29 @@ func processCustomCommands( u.LogWarning(cliConfig, "cycle detected in custom CLI commands") // Prompt the user for input u.LogInfo(cliConfig, "Do you want to continue? (y/n):") - var userInput string - fmt.Scanln(&userInput) - if strings.ToLower(userInput) != "y" { - return errors.New("execution stopped due to cycle detection") + // Channels for user input and timeout + userInput := make(chan string) + timeout := time.After(30 * time.Second) + + // Goroutine to read user input + go func() { + var input string + fmt.Scanln(&input) + userInput <- strings.TrimSpace(strings.ToLower(input)) + }() + + // Use select to handle input or timeout + select { + case input := <-userInput: + if input == "y" { + u.LogWarning(cliConfig, "Continuing execution despite the detected cycle.") + } else { + return errors.New("execution stopped due to cycle detection") + } + case <-timeout: + u.LogWarning(cliConfig, "No response from the user. Automatically continuing execution.") } - u.LogWarning(cliConfig, "Continuing execution despite the detected cycle.") } for _, commandCfg := range commands { From 8e7904143f6b773c0ebb92a31772e30a4cc6ba94 Mon Sep 17 00:00:00 2001 From: Pulak Kanti Bhowmick Date: Sun, 24 Nov 2024 12:18:13 +0600 Subject: [PATCH 8/8] Revert "add timeout for user input" This reverts commit 3498b40b7016f158dc9c0dc3766a8fb336e834e2. --- cmd/cmd_utils.go | 27 +++++---------------------- 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/cmd/cmd_utils.go b/cmd/cmd_utils.go index 6b57cd49a..44a5f8819 100644 --- a/cmd/cmd_utils.go +++ b/cmd/cmd_utils.go @@ -7,7 +7,6 @@ import ( "os" "path" "strings" - "time" "github.com/fatih/color" "github.com/spf13/cobra" @@ -131,29 +130,13 @@ func processCustomCommands( u.LogWarning(cliConfig, "cycle detected in custom CLI commands") // Prompt the user for input u.LogInfo(cliConfig, "Do you want to continue? (y/n):") + var userInput string + fmt.Scanln(&userInput) - // Channels for user input and timeout - userInput := make(chan string) - timeout := time.After(30 * time.Second) - - // Goroutine to read user input - go func() { - var input string - fmt.Scanln(&input) - userInput <- strings.TrimSpace(strings.ToLower(input)) - }() - - // Use select to handle input or timeout - select { - case input := <-userInput: - if input == "y" { - u.LogWarning(cliConfig, "Continuing execution despite the detected cycle.") - } else { - return errors.New("execution stopped due to cycle detection") - } - case <-timeout: - u.LogWarning(cliConfig, "No response from the user. Automatically continuing execution.") + if strings.ToLower(userInput) != "y" { + return errors.New("execution stopped due to cycle detection") } + u.LogWarning(cliConfig, "Continuing execution despite the detected cycle.") } for _, commandCfg := range commands {