ci: add shared gitHub shared workflows for PR checks, tag release#360
ci: add shared gitHub shared workflows for PR checks, tag release#360
Conversation
anket-cd
requested review from
a team,
anmolnagpal and
clouddrove-ci
as code owners
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Plan function in internal/terraform/plan.go to simplify change detection by utilizing the boolean return value from tf.Plan instead of manually parsing output buffers. This change removes significant string manipulation and JSON filtering logic. Feedback was provided regarding a potential silent failure path when os.Stat fails on a plan file and a minor typo in a log message.
| if out != "" { | ||
| // We saved a plan file, examine it to determine if there are changes | ||
| plan, err := tf.ShowPlanFile(context.Background(), out) | ||
| if err == nil && plan != nil { | ||
| hasChanges = len(plan.ResourceChanges) > 0 | ||
| } else { | ||
| // Fallback: check if the plan file has content | ||
| if fileInfo, err := os.Stat(out); err == nil && fileInfo.Size() > 0 { | ||
| // Plan file exists and has content, assume there are changes | ||
| hasChanges = true | ||
| } | ||
| } | ||
| } else { | ||
| // No plan file saved, we need to check the output | ||
| // The easiest way is to check if the output contains "Plan:" | ||
| outputStr := outputBuffer.String() | ||
| if strings.Contains(outputStr, "Plan:") && !strings.Contains(outputStr, "Plan: 0 to add") { | ||
| hasChanges = true | ||
| } | ||
| } | ||
|
|
||
| // Handle based on whether changes were detected | ||
| if hasChanges { | ||
| if out != "" { | ||
| Success("\nTerraform plan saved to: %s", out) | ||
| // Check if plan file was created and has content | ||
| if fileInfo, err := os.Stat(out); err == nil && fileInfo.Size() > 0 { | ||
| Success("Terraform plan saved to: %s", out) | ||
| Info("To apply this plan, run: smurf stf apply %s", out) | ||
| } else { | ||
| Success("\nTerraform plan executed successfully. Review the changes above before applying.") | ||
| } | ||
| } else { | ||
| Success("\nNo changes. Your infrastructure matches the configuration.") | ||
|
|
||
| if out != "" { | ||
| // If we saved a plan file but there are no changes, it's still saved but empty | ||
| Info("Note: Plan file was saved even though no changes detected: %s", out) | ||
| } else if !hasChanges { | ||
| Success("No changes; the infrastructure is up to date with the current configuration.") | ||
| // Clean up empty plan file | ||
| if fileInfo, err := os.Stat(out); err == nil && fileInfo.Size() == 0 { | ||
| os.Remove(out) | ||
| Info("Removed empty plan file: %s", out) | ||
| } | ||
| os.Remove(out) | ||
| Info(" Removed empty plan file: %s", out) | ||
| } | ||
| } else if !hasChanges { | ||
| Success("No changes; the infrastructure is up to date with the current configuration.") | ||
| } else { | ||
| Success("Terraform plan executed successfully. Review the changes above before applying.") | ||
| } |
There was a problem hiding this comment.
The current logic for success messages contains a silent failure path: if out is set and hasChanges is true, but os.Stat(out) fails, no feedback is given to the user. Additionally, there is a typo (leading space) in the log message on line 134. Refactoring to handle the !hasChanges case first simplifies the logic and ensures all scenarios are covered correctly.
|
1 participant
This PR introduces shared GitHub Actions workflows to standardize CI/CD processes across repositories.
Changes Included