diff --git a/cli/pkg/github/github.go b/cli/pkg/github/github.go index 0ce2ad8e..ad2ae09f 100644 --- a/cli/pkg/github/github.go +++ b/cli/pkg/github/github.go @@ -3,6 +3,10 @@ package github import ( "errors" "fmt" + "log/slog" + "os" + "strings" + "github.com/diggerhq/digger/cli/pkg/digger" "github.com/diggerhq/digger/cli/pkg/drift" github_models "github.com/diggerhq/digger/cli/pkg/github/models" @@ -21,9 +25,6 @@ import ( "github.com/google/go-github/v61/github" "github.com/samber/lo" "gopkg.in/yaml.v3" - "log/slog" - "os" - "strings" ) func initLogger() { @@ -145,13 +146,25 @@ func GitHubCI(lock core_locking.Lock, policyCheckerProvider core_policy.PolicyCh usage.ReportErrorAndExit(githubActor, "provide 'project' to run in 'manual' mode", 2) } - var projectConfig digger_config.Project - for _, projectConfig = range diggerConfig.Projects { - if projectConfig.Name == project { - break + projectConfig, projectFound := findProjectInConfig(diggerConfig.Projects, project) + + if !projectFound { + // Log available projects to help with debugging + var availableProjects []string + for _, p := range diggerConfig.Projects { + availableProjects = append(availableProjects, p.Name) } + slog.Error("Project not found in digger configuration", + "requestedProject", project, + "availableProjects", availableProjects) + usage.ReportErrorAndExit(githubActor, fmt.Sprintf("Project '%s' not found in digger configuration. Available projects: %v", project, availableProjects), 1) + } + + slog.Info("Found project configuration", "projectName", projectConfig.Name, "projectDir", projectConfig.Dir) + workflow, ok := diggerConfig.Workflows[projectConfig.Workflow] + if !ok { + usage.ReportErrorAndExit(githubActor, fmt.Sprintf("Workflow '%s' not found for project '%s'", projectConfig.Workflow, projectConfig.Name), 1) } - workflow := diggerConfig.Workflows[projectConfig.Workflow] stateEnvVars, commandEnvVars := digger_config.CollectTerraformEnvConfig(workflow.EnvVars, true) @@ -350,6 +363,16 @@ func GitHubCI(lock core_locking.Lock, policyCheckerProvider core_policy.PolicyCh usage.ReportErrorAndExit(githubActor, "Digger finished successfully", 0) } +// Helper function to search for a project in the configuration +func findProjectInConfig(projects []digger_config.Project, projectName string) (digger_config.Project, bool) { + for _, config := range projects { + if config.Name == projectName { + return config, true + } + } + return digger_config.Project{}, false +} + func logCommands(projectCommands []scheduler.Job) { logMessage := fmt.Sprintf("Following commands are going to be executed:\n") for _, pc := range projectCommands { diff --git a/cli/pkg/github/github_manual_mode_test.go b/cli/pkg/github/github_manual_mode_test.go new file mode 100644 index 00000000..1685e833 --- /dev/null +++ b/cli/pkg/github/github_manual_mode_test.go @@ -0,0 +1,207 @@ +package github + +import ( + "fmt" + "testing" + + "github.com/diggerhq/digger/libs/digger_config" + "github.com/stretchr/testify/assert" +) + +// Helper function to get available project names +func getAvailableProjectNames(projects []digger_config.Project) []string { + var availableProjects []string + for _, p := range projects { + availableProjects = append(availableProjects, p.Name) + } + return availableProjects +} + +func TestManualModeProjectValidation(t *testing.T) { + // Create a test digger config with some projects + diggerConfig := &digger_config.DiggerConfig{ + Projects: []digger_config.Project{ + { + Name: "project-a", + Dir: "./project-a", + }, + { + Name: "project-b", + Dir: "./project-b", + }, + { + Name: "project-c", + Dir: "./project-c", + }, + }, + } + + tests := []struct { + name string + requestedProject string + shouldFind bool + expectedProject string + }{ + { + name: "Valid project should be found", + requestedProject: "project-b", + shouldFind: true, + expectedProject: "project-b", + }, + { + name: "Non-existent project should not be found", + requestedProject: "non-existent-project", + shouldFind: false, + expectedProject: "", + }, + { + name: "Empty project name should not be found", + requestedProject: "", + shouldFind: false, + expectedProject: "", + }, + { + name: "Case sensitive project name should not be found", + requestedProject: "Project-A", + shouldFind: false, + expectedProject: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Use helper function for project search + projectConfig, projectFound := findProjectInConfig(diggerConfig.Projects, tt.requestedProject) + + if tt.shouldFind { + assert.True(t, projectFound, "Expected to find project %s", tt.requestedProject) + assert.Equal(t, tt.expectedProject, projectConfig.Name, "Expected project name to match") + assert.Equal(t, "./"+tt.expectedProject, projectConfig.Dir, "Expected project directory to match") + } else { + assert.False(t, projectFound, "Expected NOT to find project %s", tt.requestedProject) + // In the old buggy code, projectConfig would contain the last project from the loop + // With our fix, projectFound will be false and we should exit with an error + } + }) + } +} + +func TestManualModeProjectValidationWithOldBuggyLogic(t *testing.T) { + // This test demonstrates the old buggy behavior + diggerConfig := &digger_config.DiggerConfig{ + Projects: []digger_config.Project{ + { + Name: "project-a", + Dir: "./project-a", + }, + { + Name: "project-b", + Dir: "./project-b", + }, + { + Name: "dangerous-project", + Dir: "./dangerous-project", + }, + }, + } + + // Simulate the OLD buggy logic + requestedProject := "non-existent-project" + var projectConfig digger_config.Project + + // This is the OLD BUGGY CODE that would cause the issue + for _, projectConfig = range diggerConfig.Projects { + if projectConfig.Name == requestedProject { + break + } + } + + // In the old code, projectConfig would now contain "dangerous-project" + // (the last project in the loop) even though we requested "non-existent-project" + assert.Equal(t, "dangerous-project", projectConfig.Name, + "This demonstrates the old bug: projectConfig contains the last project from the loop") + assert.NotEqual(t, requestedProject, projectConfig.Name, + "This shows the dangerous behavior: we're using a different project than requested") +} + +func TestAvailableProjectsLogging(t *testing.T) { + // Test that we properly log available projects when a project is not found + diggerConfig := &digger_config.DiggerConfig{ + Projects: []digger_config.Project{ + {Name: "web-app"}, + {Name: "api-service"}, + {Name: "database"}, + }, + } + + requestedProject := "missing-project" + + // Use helper functions + _, projectFound := findProjectInConfig(diggerConfig.Projects, requestedProject) + availableProjects := getAvailableProjectNames(diggerConfig.Projects) + + assert.False(t, projectFound) + assert.Equal(t, []string{"web-app", "api-service", "database"}, availableProjects) +} + +// This test would actually call usage.ReportErrorAndExit in a real scenario +// but we can't easily test that without mocking the exit behavior +func TestProjectNotFoundErrorMessage(t *testing.T) { + diggerConfig := &digger_config.DiggerConfig{ + Projects: []digger_config.Project{ + {Name: "project-1"}, + {Name: "project-2"}, + }, + } + + requestedProject := "invalid-project" + + // Use helper functions + _, projectFound := findProjectInConfig(diggerConfig.Projects, requestedProject) + availableProjects := getAvailableProjectNames(diggerConfig.Projects) + + assert.False(t, projectFound) + + if !projectFound { + // This would normally call usage.ReportErrorAndExit + expectedErrorMsg := "Project 'invalid-project' not found in digger configuration. Available projects: [project-1 project-2]" + + // Verify the error message format using fmt.Sprintf for better maintainability + actualErrorMsg := fmt.Sprintf("Project '%s' not found in digger configuration. Available projects: %v", requestedProject, availableProjects) + assert.Equal(t, expectedErrorMsg, actualErrorMsg) + assert.Equal(t, []string{"project-1", "project-2"}, availableProjects) + } +} + +func TestWorkflowValidation(t *testing.T) { + // Test that we properly validate workflow existence + diggerConfig := &digger_config.DiggerConfig{ + Projects: []digger_config.Project{ + { + Name: "test-project", + Workflow: "custom-workflow", + }, + }, + Workflows: map[string]digger_config.Workflow{ + "default": { + Plan: &digger_config.Stage{Steps: []digger_config.Step{{Action: "init"}}}, + Apply: &digger_config.Stage{Steps: []digger_config.Step{{Action: "apply"}}}, + }, + }, + } + + // Test invalid workflow + project := diggerConfig.Projects[0] + _, workflowExists := diggerConfig.Workflows[project.Workflow] + assert.False(t, workflowExists, "custom-workflow should not exist") + + // Test that the error message is correctly formatted + expectedErrorMsg := fmt.Sprintf("Workflow '%s' not found for project '%s'", project.Workflow, project.Name) + actualErrorMsg := fmt.Sprintf("Workflow '%s' not found for project '%s'", project.Workflow, project.Name) + assert.Equal(t, expectedErrorMsg, actualErrorMsg) + + // Test workflow that exists + project.Workflow = "default" + _, workflowExists = diggerConfig.Workflows[project.Workflow] + assert.True(t, workflowExists, "default workflow should exist") +}