Skip to content

Commit b6189a0

Browse files
feat: add validation for file extensions in apply command (#434)
Co-authored-by: William Chen <william_w_chen@trendmicro.com>
1 parent e06ed06 commit b6189a0

2 files changed

Lines changed: 184 additions & 0 deletions

File tree

cmd/apply/apply.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"database/sql"
77
"fmt"
88
"os"
9+
"path/filepath"
910
"strings"
1011

1112
planCmd "github.com/pgplex/pgschema/cmd/plan"
@@ -264,13 +265,36 @@ func ApplyMigration(config *ApplyConfig, provider postgres.DesiredStateProvider)
264265
return nil
265266
}
266267

268+
// validateFileExtension checks that --file and --plan flags have the expected file extensions.
269+
// Returns an actionable error if a likely flag mix-up is detected.
270+
func validateFileExtension(file, planFile string) error {
271+
if file != "" {
272+
ext := strings.ToLower(filepath.Ext(file))
273+
if ext == ".json" {
274+
return fmt.Errorf("--file expects a SQL schema file, but got a JSON file (%s). Did you mean to use --plan instead?", filepath.Base(file))
275+
}
276+
}
277+
if planFile != "" {
278+
ext := strings.ToLower(filepath.Ext(planFile))
279+
if ext == ".sql" {
280+
return fmt.Errorf("--plan expects a JSON plan file, but got a SQL file (%s). Did you mean to use --file instead?", filepath.Base(planFile))
281+
}
282+
}
283+
return nil
284+
}
285+
267286
// RunApply executes the apply command logic. Exported for testing.
268287
func RunApply(cmd *cobra.Command, args []string) error {
269288
// Validate that either --file or --plan is provided
270289
if applyFile == "" && applyPlan == "" {
271290
return fmt.Errorf("either --file or --plan must be specified")
272291
}
273292

293+
// Validate file extensions to catch flag mix-ups early
294+
if err := validateFileExtension(applyFile, applyPlan); err != nil {
295+
return err
296+
}
297+
274298
// Derive final password: use provided password or check environment variable
275299
finalPassword := applyPassword
276300
if finalPassword == "" {

cmd/apply/apply_test.go

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,166 @@ func TestApplyCommandFileError(t *testing.T) {
497497
}
498498
}
499499

500+
func TestApplyFileExtensionValidation(t *testing.T) {
501+
// Save original values
502+
origDB := applyDB
503+
origUser := applyUser
504+
origFile := applyFile
505+
origPlan := applyPlan
506+
defer func() {
507+
applyDB = origDB
508+
applyUser = origUser
509+
applyFile = origFile
510+
applyPlan = origPlan
511+
}()
512+
513+
t.Run("file with json extension suggests plan", func(t *testing.T) {
514+
applyDB = "testdb"
515+
applyUser = "testuser"
516+
applyFile = "plan.json"
517+
applyPlan = ""
518+
519+
err := RunApply(nil, []string{})
520+
if err == nil {
521+
t.Fatal("Expected error when --file has .json extension")
522+
}
523+
if !strings.Contains(err.Error(), "--file expects a SQL schema file") {
524+
t.Errorf("Expected SQL schema file error, got: %v", err)
525+
}
526+
if !strings.Contains(err.Error(), "--plan instead") {
527+
t.Errorf("Expected suggestion to use --plan, got: %v", err)
528+
}
529+
})
530+
531+
t.Run("file with JSON extension case insensitive", func(t *testing.T) {
532+
applyDB = "testdb"
533+
applyUser = "testuser"
534+
applyFile = "plan.JSON"
535+
applyPlan = ""
536+
537+
err := RunApply(nil, []string{})
538+
if err == nil {
539+
t.Fatal("Expected error when --file has .JSON extension")
540+
}
541+
if !strings.Contains(err.Error(), "--file expects a SQL schema file") {
542+
t.Errorf("Expected SQL schema file error, got: %v", err)
543+
}
544+
})
545+
546+
t.Run("plan with sql extension suggests file", func(t *testing.T) {
547+
applyDB = "testdb"
548+
applyUser = "testuser"
549+
applyFile = ""
550+
applyPlan = "schema.sql"
551+
552+
err := RunApply(nil, []string{})
553+
if err == nil {
554+
t.Fatal("Expected error when --plan has .sql extension")
555+
}
556+
if !strings.Contains(err.Error(), "--plan expects a JSON plan file") {
557+
t.Errorf("Expected JSON plan file error, got: %v", err)
558+
}
559+
if !strings.Contains(err.Error(), "--file instead") {
560+
t.Errorf("Expected suggestion to use --file, got: %v", err)
561+
}
562+
})
563+
564+
t.Run("plan with SQL extension case insensitive", func(t *testing.T) {
565+
applyDB = "testdb"
566+
applyUser = "testuser"
567+
applyFile = ""
568+
applyPlan = "schema.SQL"
569+
570+
err := RunApply(nil, []string{})
571+
if err == nil {
572+
t.Fatal("Expected error when --plan has .SQL extension")
573+
}
574+
if !strings.Contains(err.Error(), "--plan expects a JSON plan file") {
575+
t.Errorf("Expected JSON plan file error, got: %v", err)
576+
}
577+
})
578+
579+
t.Run("file with sql extension no error", func(t *testing.T) {
580+
tmpDir := t.TempDir()
581+
schemaPath := filepath.Join(tmpDir, "schema.sql")
582+
if err := os.WriteFile(schemaPath, []byte("CREATE TABLE test (id INT);"), 0644); err != nil {
583+
t.Fatalf("Failed to write schema file: %v", err)
584+
}
585+
586+
applyDB = "testdb"
587+
applyUser = "testuser"
588+
applyFile = schemaPath
589+
applyPlan = ""
590+
591+
err := RunApply(nil, []string{})
592+
// Should fail on something other than extension validation
593+
if err != nil && strings.Contains(err.Error(), "expects a") {
594+
t.Errorf("Should not get extension validation error for .sql file: %v", err)
595+
}
596+
})
597+
598+
t.Run("plan with json extension no error", func(t *testing.T) {
599+
tmpDir := t.TempDir()
600+
planPath := filepath.Join(tmpDir, "plan.json")
601+
planJSON := fmt.Sprintf(`{"version":"1.0.0","pgschema_version":"%s","created_at":"2024-01-01T00:00:00Z","transaction":true,"summary":{"total":0,"add":0,"change":0,"destroy":0,"by_type":{}},"diffs":[]}`, version.App())
602+
if err := os.WriteFile(planPath, []byte(planJSON), 0644); err != nil {
603+
t.Fatalf("Failed to write plan file: %v", err)
604+
}
605+
606+
applyDB = "testdb"
607+
applyUser = "testuser"
608+
applyFile = ""
609+
applyPlan = planPath
610+
611+
err := RunApply(nil, []string{})
612+
// Should not get extension validation error
613+
if err != nil && strings.Contains(err.Error(), "expects a") {
614+
t.Errorf("Should not get extension validation error for .json plan: %v", err)
615+
}
616+
})
617+
}
618+
619+
func TestValidateFileExtension(t *testing.T) {
620+
tests := []struct {
621+
name string
622+
file string
623+
plan string
624+
wantErr bool
625+
errMsg string
626+
}{
627+
{"file with json", "plan.json", "", true, "--file expects a SQL schema file"},
628+
{"file with JSON uppercase", "plan.JSON", "", true, "--file expects a SQL schema file"},
629+
{"file with Json mixed case", "plan.Json", "", true, "--file expects a SQL schema file"},
630+
{"plan with sql", "", "schema.sql", true, "--plan expects a JSON plan file"},
631+
{"plan with SQL uppercase", "", "schema.SQL", true, "--plan expects a JSON plan file"},
632+
{"file with sql is ok", "schema.sql", "", false, ""},
633+
{"plan with json is ok", "", "plan.json", false, ""},
634+
{"both empty is ok", "", "", false, ""},
635+
{"file with no extension is ok", "schema", "", false, ""},
636+
{"plan with no extension is ok", "", "plan", false, ""},
637+
{"file with path and json", "/tmp/plans/plan.json", "", true, "--file expects a SQL schema file"},
638+
{"plan with path and sql", "", "/tmp/schemas/schema.sql", true, "--plan expects a JSON plan file"},
639+
}
640+
641+
for _, tt := range tests {
642+
t.Run(tt.name, func(t *testing.T) {
643+
err := validateFileExtension(tt.file, tt.plan)
644+
if tt.wantErr {
645+
if err == nil {
646+
t.Fatal("Expected error but got nil")
647+
}
648+
if !strings.Contains(err.Error(), tt.errMsg) {
649+
t.Errorf("Expected error containing %q, got: %v", tt.errMsg, err)
650+
}
651+
} else {
652+
if err != nil {
653+
t.Errorf("Expected no error, got: %v", err)
654+
}
655+
}
656+
})
657+
}
658+
}
659+
500660
func TestApplyCommand_PlanDatabaseFlags(t *testing.T) {
501661
flags := ApplyCmd.Flags()
502662

0 commit comments

Comments
 (0)