From fbc3aaf79985f29ea65af5454b8cab25280b85a7 Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Wed, 8 Jul 2026 16:40:47 -0400 Subject: [PATCH 01/20] fix(variant): fix 6 bugs in variant generator transform and mapping logic Extensionless files (e.g., inventory_disable_vagrant) were silently copied without text replacement. File paths containing entity names were not renamed on disk, causing deployment failures. Group names in compound strings (e.g., StarkWallpaper) were missed due to overly aggressive word-boundary matching. Firstname collisions in the Misc map caused silent overwrites and stale mapping.json entries. Single-name users (drogon, missandei, hodor) were incorrectly mapped to firstname.lastname format. Underscore-delimited ACL keys in overlay files were not transformed because Go's \b treats _ as a word character. Co-Authored-By: Claude Opus 4.6 (1M context) --- cli/internal/variant/generator.go | 135 ++++++++++++----- cli/internal/variant/generator_test.go | 201 ++++++++++++++++++++++++- 2 files changed, 296 insertions(+), 40 deletions(-) diff --git a/cli/internal/variant/generator.go b/cli/internal/variant/generator.go index 629ccd5d..2f95a57c 100644 --- a/cli/internal/variant/generator.go +++ b/cli/internal/variant/generator.go @@ -1,6 +1,7 @@ package variant import ( + "bytes" "encoding/json" "fmt" "io/fs" @@ -9,6 +10,7 @@ import ( "regexp" "sort" "strings" + "unicode/utf8" ) // LabConfig is the top-level structure of a GOAD config.json. @@ -159,8 +161,9 @@ type HostMapping struct { // replacement is an ordered old->new string replacement. type replacement struct { - Old string - New string + Old string + New string + WordBoundary bool // use word-boundary regex instead of plain replacement } // Generator creates GOAD variants with randomized entity names. @@ -174,7 +177,8 @@ type Generator struct { replacements []replacement userPasswordMap map[string]string // new_username -> new_password preservedUsers map[string]bool - pwdInDescUsers map[string]bool // new_username -> has password in description + pwdInDescUsers map[string]bool // new_username -> has password in description + nameComponents map[string]bool // Misc keys that are firstname/surname components } // hostnameAliases maps canonical hostnames to known typos/aliases in upstream GOAD. @@ -204,6 +208,7 @@ func NewGenerator(source, target, name string) *Generator { userPasswordMap: make(map[string]string), preservedUsers: map[string]bool{"sql_svc": true}, pwdInDescUsers: make(map[string]bool), + nameComponents: make(map[string]bool), } } @@ -363,6 +368,10 @@ func (g *Generator) mapUsers(config *LabConfig) { } newUsername := g.nameGen.GenerateUsername() + // Preserve single-name pattern: if original has no ".", use only firstname. + if !strings.Contains(username, ".") { + newUsername = strings.Split(newUsername, ".")[0] + } g.mappings.Users[username] = newUsername if user != nil && user.Password != "" && user.Description != "" && @@ -384,12 +393,25 @@ func (g *Generator) mapUserNameComponents(user *UserConfig, newUsername string) if user.Firstname != "" { firstname := user.Firstname newFirst := strings.Split(newUsername, ".")[0] - g.mappings.Misc[firstname] = newFirst + // Only set if not already mapped — prevents collisions when two users + // share a firstname (e.g., both map to "brandon"). + if _, exists := g.mappings.Misc[firstname]; !exists { + g.mappings.Misc[firstname] = newFirst + g.nameComponents[firstname] = true + } if !isAllLower(firstname) && firstname != "sql" { - g.mappings.Misc[strings.ToLower(firstname)] = strings.ToLower(newFirst) + lower := strings.ToLower(firstname) + if _, exists := g.mappings.Misc[lower]; !exists { + g.mappings.Misc[lower] = strings.ToLower(newFirst) + g.nameComponents[lower] = true + } } if isAllLower(firstname) && firstname != "sql" { - g.mappings.Misc[capitalize(firstname)] = capitalize(newFirst) + cap := capitalize(firstname) + if _, exists := g.mappings.Misc[cap]; !exists { + g.mappings.Misc[cap] = capitalize(newFirst) + g.nameComponents[cap] = true + } } } @@ -400,9 +422,16 @@ func (g *Generator) mapUserNameComponents(user *UserConfig, newUsername string) if len(parts) > 1 { newSurname = parts[1] } - g.mappings.Misc[surname] = newSurname + if _, exists := g.mappings.Misc[surname]; !exists { + g.mappings.Misc[surname] = newSurname + g.nameComponents[surname] = true + } if isAllLower(surname) { - g.mappings.Misc[capitalize(surname)] = capitalize(newSurname) + cap := capitalize(surname) + if _, exists := g.mappings.Misc[cap]; !exists { + g.mappings.Misc[cap] = capitalize(newSurname) + g.nameComponents[cap] = true + } } } } @@ -602,6 +631,18 @@ func (g *Generator) buildOrderedReplacements() { repls = appendMapReplacements(repls, g.mappings.NetBIOS, nil) repls = appendMapReplacements(repls, g.mappings.Misc, withoutSuffix("$")) + // Tag name-component replacements for word-boundary matching. + // Skip word-boundary for entries that also exist as group names — + // group names need plain string replacement to catch compound strings + // like "StarkWallpaper". + for i := range repls { + if _, isGroup := g.mappings.Groups[repls[i].Old]; isGroup { + repls[i].WordBoundary = false + } else { + repls[i].WordBoundary = g.isNameComponent(repls[i].Old) + } + } + sort.Slice(repls, func(i, j int) bool { return len(repls[i].Old) > len(repls[j].Old) }) @@ -633,17 +674,17 @@ func appendMapReplacements(repls []replacement, m map[string]string, filter func if filter != nil && !filter(old) { continue } - repls = append(repls, replacement{old, new}) + repls = append(repls, replacement{Old: old, New: new}) } return repls } func (g *Generator) appendHostReplacements(repls []replacement) []replacement { for _, hm := range g.mappings.Hosts { - repls = append(repls, replacement{hm.OldFQDN, hm.NewFQDN}) + repls = append(repls, replacement{Old: hm.OldFQDN, New: hm.NewFQDN}) } for _, hm := range g.mappings.Hosts { - repls = append(repls, replacement{hm.OldHostname, hm.NewHostname}) + repls = append(repls, replacement{Old: hm.OldHostname, New: hm.NewHostname}) } return repls } @@ -664,8 +705,8 @@ func (g *Generator) appendQualifiedUserReplacements(repls []replacement) []repla } for oldUser, newUser := range g.mappings.Users { repls = append(repls, - replacement{oldNB + "\\\\" + oldUser, newNB + "\\\\" + newUser}, - replacement{oldDomain + "\\\\" + oldUser, newDomain + "\\\\" + newUser}, + replacement{Old: oldNB + "\\\\" + oldUser, New: newNB + "\\\\" + newUser}, + replacement{Old: oldDomain + "\\\\" + oldUser, New: newDomain + "\\\\" + newUser}, ) } } @@ -685,8 +726,8 @@ func (g *Generator) appendDNReplacements(repls []replacement) []replacement { newDCs = append(newDCs, "DC="+p) } repls = append(repls, replacement{ - strings.Join(oldDCs, ",") + ",DC=local", - strings.Join(newDCs, ",") + ",DC=local", + Old: strings.Join(oldDCs, ",") + ",DC=local", + New: strings.Join(newDCs, ",") + ",DC=local", }) } return repls @@ -702,7 +743,7 @@ func (g *Generator) appendDomainReplacements(repls []replacement) []replacement return len(pairs[i].old) > len(pairs[j].old) }) for _, dp := range pairs { - repls = append(repls, replacement{dp.old, dp.new}) + repls = append(repls, replacement{Old: dp.old, New: dp.new}) } return repls } @@ -714,12 +755,21 @@ func (g *Generator) applyReplacements(content string) string { continue } - if g.isNameComponent(r.Old) { - pattern := `\b` + regexp.QuoteMeta(r.Old) + `\b` + if r.WordBoundary { + // Match name at boundaries, treating underscore as a word + // separator. Go's \b considers _ a word character, but GOAD + // uses underscore-delimited keys (e.g., "GenericWrite_missandei_viserys"). + // We pre-process underscores to temporary placeholders so \b works, + // then restore them. + escaped := regexp.QuoteMeta(r.Old) + const placeholder = "\x00USCORE\x00" + temp := strings.ReplaceAll(content, "_", placeholder) + pattern := `\b` + escaped + `\b` re, err := regexp.Compile(pattern) if err == nil { - content = re.ReplaceAllString(content, r.New) + temp = re.ReplaceAllString(temp, r.New) } + content = strings.ReplaceAll(temp, placeholder, "_") } else { content = strings.ReplaceAll(content, r.Old, r.New) } @@ -729,22 +779,7 @@ func (g *Generator) applyReplacements(content string) string { // isNameComponent returns true if old is a firstname/surname component needing word-boundary protection. func (g *Generator) isNameComponent(old string) bool { - if _, ok := g.mappings.Misc[old]; !ok { - return false - } - if strings.HasSuffix(old, "$") || strings.Contains(old, ".") || strings.Contains(old, "\\") { - return false - } - if len(old) >= 50 { - return false - } - cleaned := strings.ReplaceAll(strings.ReplaceAll(old, "-", ""), "'", "") - for _, c := range cleaned { - if ('a' > c || c > 'z') && ('A' > c || c > 'Z') { - return false - } - } - return true + return g.nameComponents[old] } // fixUserFirstnameSurname corrects firstname/surname fields to match generated usernames. @@ -843,16 +878,18 @@ var textFilenames = map[string]bool{ // transformFile transforms a single file with replacements and writes to target. func (g *Generator) transformFile(srcPath, relPath string) (transformed bool) { + // Rename file paths based on entity mappings (e.g., arya.txt -> thomas.txt). + newRelPath := g.applyReplacements(relPath) ext := filepath.Ext(srcPath) base := filepath.Base(srcPath) - targetFile := filepath.Join(g.TargetPath, relPath) + targetFile := filepath.Join(g.TargetPath, newRelPath) if err := os.MkdirAll(filepath.Dir(targetFile), 0o755); err != nil { fmt.Printf("Warning: mkdir failed for %s: %v\n", relPath, err) return false } - if textExtensions[ext] || textFilenames[base] { + if textExtensions[ext] || textFilenames[base] || (ext == "" && g.isTextFile(srcPath)) { content, err := os.ReadFile(srcPath) if err != nil { fmt.Printf("Warning: Could not read %s: %v\n", relPath, err) @@ -987,7 +1024,7 @@ func (g *Generator) findNameViolations() ([]violation, int) { return nil } ext := filepath.Ext(path) - if !textExtensions[ext] && !textFilenames[d.Name()] { + if !textExtensions[ext] && !textFilenames[d.Name()] && !(ext == "" && g.isTextFile(path)) { return nil } filesChecked++ @@ -1124,6 +1161,28 @@ func copyFile(src, dst string) { } } +// isTextFile returns true if the file appears to contain text (valid UTF-8 with +// no NUL bytes). Used as a fallback for extensionless files like "inventory_disable_vagrant". +func (g *Generator) isTextFile(path string) bool { + f, err := os.Open(path) + if err != nil { + return false + } + defer f.Close() + + // Only read the first 8 KiB — enough to detect binary content. + buf := make([]byte, 8192) + n, err := f.Read(buf) + if n == 0 { + return false + } + check := buf[:n] + if bytes.ContainsRune(check, 0) { + return false + } + return utf8.Valid(check) +} + func capitalize(s string) string { if s == "" { return s diff --git a/cli/internal/variant/generator_test.go b/cli/internal/variant/generator_test.go index 44a99b1f..82b350c9 100644 --- a/cli/internal/variant/generator_test.go +++ b/cli/internal/variant/generator_test.go @@ -8,6 +8,44 @@ import ( "testing" ) +// setupTestSourceFull creates a test source with extensionless files, user files, +// and compound group name scripts to exercise all known edge cases. +func setupTestSourceFull(t *testing.T) (sourceDir, targetDir string) { + t.Helper() + sourceDir, targetDir = setupTestSource(t) + + // Extensionless inventory file (Bug 1) + inventoryContent := `[all:vars] +; sevenkingdoms.local +ansible_user=administrator@sevenkingdoms.local +` + if err := os.WriteFile(filepath.Join(sourceDir, "data", "inventory_disable_vagrant"), []byte(inventoryContent), 0o644); err != nil { + t.Fatal(err) + } + + // File named after a user (Bug 2) + if err := os.MkdirAll(filepath.Join(sourceDir, "files", "srv02", "all"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile( + filepath.Join(sourceDir, "files", "srv02", "all", "arya.txt"), + []byte("Hey arya, here is your sword."), + 0o644, + ); err != nil { + t.Fatal(err) + } + + // Script with compound group name (Bug 3) + gpoScript := `$gpo = "StarkWallpaper" +Set-GPO -Name "StarkWallpaper" -Target "DC=north,DC=sevenkingdoms,DC=local" +` + if err := os.WriteFile(filepath.Join(sourceDir, "scripts", "gpo_abuse.ps1"), []byte(gpoScript), 0o644); err != nil { + t.Fatal(err) + } + + return sourceDir, targetDir +} + func setupTestSource(t *testing.T) (sourceDir, targetDir string) { t.Helper() tmpDir := t.TempDir() @@ -194,8 +232,8 @@ func TestApplyReplacements(t *testing.T) { gen := NewGenerator("", "", "test") gen.mappings.Misc["robert"] = "james" gen.replacements = []replacement{ - {"sevenkingdoms.local", "deltasystems.local"}, - {"robert", "james"}, + {Old: "sevenkingdoms.local", New: "deltasystems.local"}, + {Old: "robert", New: "james", WordBoundary: true}, } content := "domain: sevenkingdoms.local, user: robert" @@ -209,11 +247,37 @@ func TestApplyReplacements(t *testing.T) { } } +func TestApplyReplacementsUnderscoreDelimited(t *testing.T) { + gen := NewGenerator("", "", "test") + gen.replacements = []replacement{ + {Old: "missandei", New: "donna", WordBoundary: true}, + {Old: "viserys", New: "alexander", WordBoundary: true}, + } + + content := `"GenericWrite_missandei_viserys": null` + result := gen.applyReplacements(content) + + if strings.Contains(result, "missandei") { + t.Errorf("missandei not replaced in underscore-delimited key: %s", result) + } + if strings.Contains(result, "viserys") { + t.Errorf("viserys not replaced in underscore-delimited key: %s", result) + } + expected := `"GenericWrite_donna_alexander": null` + if result != expected { + t.Errorf("unexpected result:\n got: %s\n want: %s", result, expected) + } +} + func TestIsNameComponent(t *testing.T) { gen := NewGenerator("", "", "test") gen.mappings.Misc["robert"] = "james" + gen.nameComponents["robert"] = true gen.mappings.Misc["meereen$"] = "beacon$" gen.mappings.Misc["winterfell.domain"] = "cascade.domain" + // Group name that happens to also be a surname — should NOT be a name component + // unless explicitly registered via mapUserNameComponents. + gen.mappings.Misc["Stark"] = "OperationsGroup" tests := []struct { name string @@ -223,6 +287,7 @@ func TestIsNameComponent(t *testing.T) { {"meereen$", false}, {"winterfell.domain", false}, {"notinmisc", false}, + {"Stark", false}, // group name, not registered as name component } for _, tt := range tests { @@ -268,3 +333,135 @@ func TestSimplifyEntity(t *testing.T) { } } } + +func TestTransformEdgeCases(t *testing.T) { + sourceDir, targetDir := setupTestSourceFull(t) + + gen := NewGenerator(sourceDir, targetDir, "test-edges") + if err := gen.Run(); err != nil { + t.Fatalf("generator failed: %v", err) + } + + // Bug 1: extensionless files should be detected as text and transformed. + t.Run("extensionless_file", func(t *testing.T) { + data, err := os.ReadFile(filepath.Join(targetDir, "data", "inventory_disable_vagrant")) + if err != nil { + t.Fatal("inventory_disable_vagrant not created in target") + } + if strings.Contains(strings.ToLower(string(data)), "sevenkingdoms") { + t.Error("original domain 'sevenkingdoms' still found in extensionless inventory file") + } + }) + + // Bug 2: files named after entities should be renamed on disk. + t.Run("file_renamed", func(t *testing.T) { + oldPath := filepath.Join(targetDir, "files", "srv02", "all", "arya.txt") + if _, err := os.Stat(oldPath); err == nil { + t.Error("arya.txt still exists at original path — file was not renamed") + } + newFirstname := gen.mappings.Misc["arya"] + if newFirstname == "" { + t.Fatal("no mapping found for 'arya' in Misc") + } + newPath := filepath.Join(targetDir, "files", "srv02", "all", newFirstname+".txt") + if _, err := os.Stat(newPath); err != nil { + t.Errorf("renamed file %s.txt not found at expected path", newFirstname) + } + }) + + // Bug 3: group names in compound strings (e.g., "StarkWallpaper") should be replaced. + t.Run("compound_group_name", func(t *testing.T) { + data, err := os.ReadFile(filepath.Join(targetDir, "scripts", "gpo_abuse.ps1")) + if err != nil { + t.Fatal("gpo_abuse.ps1 not created in target") + } + if strings.Contains(string(data), "Stark") { + t.Errorf("original group name 'Stark' still found in gpo_abuse.ps1: %s", string(data)) + } + }) +} + +func TestFirstnameCollisionNoOverwrite(t *testing.T) { + tmpDir := t.TempDir() + sourceDir := filepath.Join(tmpDir, "source") + targetDir := filepath.Join(tmpDir, "target") + + if err := os.MkdirAll(filepath.Join(sourceDir, "data"), 0o755); err != nil { + t.Fatal(err) + } + + // Two users with the same firstname in the same domain. + config := &LabConfig{} + config.Lab.Hosts = map[string]*HostConfig{ + "dc01": { + Hostname: "kingslanding", + Type: "dc", + Domain: "sevenkingdoms.local", + LocalAdminPassword: "TestPass123!", + }, + } + config.Lab.Domains = map[string]*DomainConfig{ + "sevenkingdoms.local": { + DomainPassword: "DomainPass1!", + Users: map[string]*UserConfig{ + "brandon.stark": { + Firstname: "brandon", + Surname: "stark", + Password: "BranPass1!", + }, + "brandon.lannister": { + Firstname: "brandon", + Surname: "lannister", + Password: "BranPass2!", + }, + }, + Groups: GroupsConfig{}, + }, + } + configData, _ := json.MarshalIndent(config, "", " ") + if err := os.WriteFile(filepath.Join(sourceDir, "data", "config.json"), configData, 0o644); err != nil { + t.Fatal(err) + } + + gen := NewGenerator(sourceDir, targetDir, "test-collision") + if err := gen.Run(); err != nil { + t.Fatalf("generator failed: %v", err) + } + + // Both users should have distinct mappings. + user1 := gen.mappings.Users["brandon.stark"] + user2 := gen.mappings.Users["brandon.lannister"] + if user1 == "" || user2 == "" { + t.Fatal("one or both users not mapped") + } + if user1 == user2 { + t.Errorf("both users mapped to same username: %s", user1) + } + + // The Misc entry for "brandon" should exist and not be empty. + miscBrandon := gen.mappings.Misc["brandon"] + if miscBrandon == "" { + t.Error("Misc mapping for 'brandon' is empty") + } + + // Verify both users exist in the output config with correct firstname/surname. + varData, err := os.ReadFile(filepath.Join(targetDir, "data", "config.json")) + if err != nil { + t.Fatal(err) + } + var varConfig LabConfig + if err := json.Unmarshal(varData, &varConfig); err != nil { + t.Fatal(err) + } + for domainName, domain := range varConfig.Lab.Domains { + for username, user := range domain.Users { + if strings.Contains(username, ".") { + parts := strings.SplitN(username, ".", 2) + if user.Firstname != parts[0] { + t.Errorf("domain %s user %s: firstname=%q doesn't match username prefix %q", + domainName, username, user.Firstname, parts[0]) + } + } + } + } +} From 7e96006fac0c0fad84578979da977ddf20053914 Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Wed, 8 Jul 2026 16:55:30 -0400 Subject: [PATCH 02/20] fix(variant): reconcile group/surname mapping conflicts and optimize isTextFile When a group name collides with a capitalized surname (e.g., "Targaryen" is both an AD group and a surname from daenerys.targaryen), the Misc entry is now removed in favor of the Groups mapping. This prevents mapping.json from showing a stale surname-derived value while the config uses the group-derived value. Also changed isTextFile to read only the first 8 KiB instead of the entire file. Co-Authored-By: Claude Opus 4.6 (1M context) --- cli/internal/variant/generator.go | 10 +++++++++ cli/internal/variant/generator_test.go | 28 ++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/cli/internal/variant/generator.go b/cli/internal/variant/generator.go index 2f95a57c..abffbb0a 100644 --- a/cli/internal/variant/generator.go +++ b/cli/internal/variant/generator.go @@ -292,6 +292,16 @@ func (g *Generator) generateMappings(config *LabConfig) { fmt.Println("\nMapping cities...") g.mapCities(config) + // Reconcile Misc entries that conflict with Groups. Group names are + // explicit AD entities and take precedence over capitalized-surname + // convenience entries (e.g., "Targaryen" is both a group and a surname). + for groupName := range g.mappings.Groups { + if _, inMisc := g.mappings.Misc[groupName]; inMisc { + delete(g.mappings.Misc, groupName) + delete(g.nameComponents, groupName) + } + } + fmt.Println("\n=== Mapping Generation Complete ===") } diff --git a/cli/internal/variant/generator_test.go b/cli/internal/variant/generator_test.go index 82b350c9..449e56a0 100644 --- a/cli/internal/variant/generator_test.go +++ b/cli/internal/variant/generator_test.go @@ -381,6 +381,34 @@ func TestTransformEdgeCases(t *testing.T) { }) } +func TestGroupSurnameMappingConsistency(t *testing.T) { + sourceDir, targetDir := setupTestSourceFull(t) + + gen := NewGenerator(sourceDir, targetDir, "test-group-surname") + if err := gen.Run(); err != nil { + t.Fatalf("generator failed: %v", err) + } + + // "Stark" is both a group name and a capitalized surname (from arya.stark). + // The group mapping should win — Misc should not have a conflicting entry. + groupNew := gen.mappings.Groups["Stark"] + if groupNew == "" { + t.Fatal("group 'Stark' not found in mappings") + } + if miscNew, exists := gen.mappings.Misc["Stark"]; exists { + t.Errorf("Misc still has 'Stark' -> %q, should have been removed in favor of group mapping %q", miscNew, groupNew) + } + + // Verify the config.json actually uses the group name, not the surname. + varData, err := os.ReadFile(filepath.Join(targetDir, "data", "config.json")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(varData), groupNew) { + t.Errorf("group name %q not found in variant config.json", groupNew) + } +} + func TestFirstnameCollisionNoOverwrite(t *testing.T) { tmpDir := t.TempDir() sourceDir := filepath.Join(tmpDir, "source") From cd28ab25c55761ff834ebd2f5b21a63a3581137d Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Wed, 8 Jul 2026 16:56:58 -0400 Subject: [PATCH 03/20] fix(variant): scope group/Misc reconciliation to name components only The previous reconciliation removed any Misc entry matching a group name, which could incorrectly delete hostname or city mappings if they happened to collide. Now only removes entries tracked as name components (firstname/surname), preserving other Misc entries. Co-Authored-By: Claude Opus 4.6 (1M context) --- cli/internal/variant/generator.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/cli/internal/variant/generator.go b/cli/internal/variant/generator.go index abffbb0a..666acef0 100644 --- a/cli/internal/variant/generator.go +++ b/cli/internal/variant/generator.go @@ -292,11 +292,14 @@ func (g *Generator) generateMappings(config *LabConfig) { fmt.Println("\nMapping cities...") g.mapCities(config) - // Reconcile Misc entries that conflict with Groups. Group names are - // explicit AD entities and take precedence over capitalized-surname - // convenience entries (e.g., "Targaryen" is both a group and a surname). + // Reconcile name-component Misc entries that conflict with Groups. + // Group names are explicit AD entities and take precedence over + // capitalized-surname convenience entries (e.g., "Targaryen" is both + // a group and a surname). Only remove entries that are name components + // (firstname/surname) — other Misc entries like hostnames, gMSA names, + // and cities must be preserved. for groupName := range g.mappings.Groups { - if _, inMisc := g.mappings.Misc[groupName]; inMisc { + if g.nameComponents[groupName] { delete(g.mappings.Misc, groupName) delete(g.nameComponents, groupName) } From 259cdaf15246394d9f610e2e1888e48148342512 Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Wed, 8 Jul 2026 17:54:47 -0400 Subject: [PATCH 04/20] fix(variant): remove Robert and Brandon from name generator word list These firstnames collide with the originalNames validation list (from robert.baratheon and brandon.stark), causing false positive validation warnings when the generator randomly picks them. Co-Authored-By: Claude Opus 4.6 (1M context) --- cli/internal/variant/namegen.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/internal/variant/namegen.go b/cli/internal/variant/namegen.go index e7794d73..eff7e273 100644 --- a/cli/internal/variant/namegen.go +++ b/cli/internal/variant/namegen.go @@ -41,13 +41,13 @@ func NewNameGenerator() *NameGenerator { "ventures", "enterprises", "group", "labs", "dynamics", "works", }, firstNames: []string{ - "James", "Michael", "Robert", "John", "David", "William", + "James", "Michael", "John", "David", "William", "Richard", "Joseph", "Thomas", "Charles", "Christopher", "Daniel", "Matthew", "Anthony", "Mark", "Donald", "Steven", "Paul", "Andrew", "Joshua", "Kenneth", "Kevin", "Brian", "George", "Timothy", "Ronald", "Edward", "Jason", "Jeffrey", "Ryan", "Jacob", "Gary", "Nicholas", "Eric", "Jonathan", "Stephen", - "Larry", "Justin", "Scott", "Brandon", "Benjamin", "Samuel", + "Larry", "Justin", "Scott", "Benjamin", "Samuel", "Raymond", "Gregory", "Alexander", "Patrick", "Frank", "Dennis", "Mary", "Patricia", "Jennifer", "Linda", "Barbara", "Elizabeth", "Susan", "Jessica", "Sarah", "Karen", "Nancy", "Lisa", From 7aa4513e58854074a3a8275199b41a328a5b68cf Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Thu, 9 Jul 2026 22:07:24 -0400 Subject: [PATCH 05/20] fix(variant): exclude Jinja-unsafe characters from generated passwords Remove []{} from the password special character pool. These can produce Jinja2 template sequences ({#, {{, {%}) that cause Ansible to crash with "Missing end of comment tag" errors when passwords are embedded in playbook templates. Co-Authored-By: Claude --- cli/internal/variant/namegen.go | 2 +- cli/internal/variant/namegen_test.go | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/cli/internal/variant/namegen.go b/cli/internal/variant/namegen.go index eff7e273..cd2a97d6 100644 --- a/cli/internal/variant/namegen.go +++ b/cli/internal/variant/namegen.go @@ -224,7 +224,7 @@ const ( lowerChars = "abcdefghijklmnopqrstuvwxyz" upperChars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" digitChars = "0123456789" - specialChars = "!@#$%^&*()-_=+[]{}|;:,.<>?" + specialChars = "!@#$%^&*()-_=+|;:,.<>?" ) type charClasses struct { diff --git a/cli/internal/variant/namegen_test.go b/cli/internal/variant/namegen_test.go index d72b690d..c125963e 100644 --- a/cli/internal/variant/namegen_test.go +++ b/cli/internal/variant/namegen_test.go @@ -93,6 +93,20 @@ func TestGeneratePassword(t *testing.T) { } } +func TestGeneratePasswordNoJinjaSequences(t *testing.T) { + ng := NewNameGenerator() + forbidden := []string{"{", "}", "[", "]"} + + for i := 0; i < 1000; i++ { + pw := ng.GeneratePassword("Password1!") + for _, seq := range forbidden { + if strings.Contains(pw, seq) { + t.Fatalf("password %q contains forbidden sequence %q (iteration %d)", pw, seq, i) + } + } + } +} + func TestGenerateGroupName(t *testing.T) { ng := NewNameGenerator() name := ng.GenerateGroupName() From e24dabf5def0a8cbe3e6af13902e61c57c4b754c Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Thu, 9 Jul 2026 22:07:42 -0400 Subject: [PATCH 06/20] fix(azure): complete inventory templates and variant-aware bootstrapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Inventory bootstrapping hardcoded labName to "GOAD", ignoring active variants. Now resolves to the variant target directory (e.g. GOAD-dreadindex) when a variant is configured. Azure inventory templates were missing all Ansible group definitions ([dc], [domain], [server], [trust], [adcs], etc.) and required [all:vars] (domain_name, force_dns_server, dns_server_forwarder, etc.). Without these groups, playbooks silently skipped tasks or failed with undefined variable errors. Also fixes admin_user from "goadmin" to "administrator" — the Azure bootstrap script renames the SID-500 account to "administrator". Updated templates for all 6 labs: GOAD, GOAD-Light, GOAD-Mini, NHA, DRACARYS, SCCM. Co-Authored-By: Claude --- ad/DRACARYS/providers/azure/inventory | 72 ++++++++++++++++++- ad/GOAD-Light/providers/azure/inventory | 72 +++++++++++++++++-- ad/GOAD-Mini/providers/azure/inventory | 61 +++++++++++++++-- ad/GOAD/providers/azure/inventory | 91 ++++++++++++++++++++++++- ad/NHA/providers/azure/inventory | 79 ++++++++++++++++++++- ad/SCCM/providers/azure/inventory | 77 ++++++++++++++++++++- cli/cmd/provision.go | 6 +- 7 files changed, 439 insertions(+), 19 deletions(-) diff --git a/ad/DRACARYS/providers/azure/inventory b/ad/DRACARYS/providers/azure/inventory index f37bfbd5..c348fd95 100644 --- a/ad/DRACARYS/providers/azure/inventory +++ b/ad/DRACARYS/providers/azure/inventory @@ -1,3 +1,17 @@ +[all:vars] +domain_name=DRACARYS +admin_user=administrator +force_dns_server=no +dns_server=1.1.1.1 +dns_server_forwarder=1.1.1.1 +add_route=no +route_gateway=192.168.56.1 +route_network=10.0.0.0/8 +enable_http_proxy=no +ad_http_proxy=http://x.x.x.x:xxxx +ad_https_proxy=http://x.x.x.x:xxxx +keyboard_layouts=["en-US"] + [default] ; ------------------------------------------------ ; dracarys.lab @@ -6,5 +20,59 @@ dc01 ansible_host={{ip_range}}.10 dns_domain=dc01 dict_key=dc01 ansible_user=ans srv01 ansible_host={{ip_range}}.11 dns_domain=dc01 dict_key=srv01 ansible_user=ansible ansible_password=NgtkgtIAs75cKV+Pu lx01 ansible_host={{ip_range}}.12 dict_key=lx01 ansible_connection=ssh ansible_ssh_common_args='-o StrictHostKeyChecking=no' ansible_user=goadmin ansible_password=HGLXaxQSP@ssw_rd$ -[all:vars] -admin_user=goadmin +[domain] +dc01 +srv01 + +[linux_domain] +lx01 + +[dc] +dc01 + +[server] +srv01 + +[workstation] + +[parent_dc] +dc01 + +[child_dc] + +[trust] + +[adcs] + +[adcs_customtemplates] + +[iis] + +[mssql] + +[mssql_ssms] + +[webdav] + +[glpi] +lx01 + +[klink] +srv01 + +[keepass] +srv01 + +[update] +dc01 + +[no_update] +srv01 + +[defender_on] +dc01 +srv01 + +[defender_off] + +[extensions] diff --git a/ad/GOAD-Light/providers/azure/inventory b/ad/GOAD-Light/providers/azure/inventory index e1828f71..bd8beabe 100644 --- a/ad/GOAD-Light/providers/azure/inventory +++ b/ad/GOAD-Light/providers/azure/inventory @@ -1,6 +1,14 @@ +[all:vars] +domain_name=GOAD-Light +admin_user=administrator +force_dns_server=no +dns_server=1.1.1.1 +dns_server_forwarder=1.1.1.1 +enable_http_proxy=no +ad_http_proxy=http://x.x.x.x:xxxx +ad_https_proxy=http://x.x.x.x:xxxx + [default] -; Note: ansible_host *MUST* be an IPv4 address or setting things like DNS -; servers will break. ; ------------------------------------------------ ; sevenkingdoms.local ; ------------------------------------------------ @@ -11,5 +19,61 @@ dc01 ansible_host={{ip_range}}.10 dns_domain=dc01 dict_key=dc01 ansible_user=ans dc02 ansible_host={{ip_range}}.11 dns_domain=dc01 dict_key=dc02 ansible_user=ansible ansible_password=NgtI75cKV+Pu srv02 ansible_host={{ip_range}}.22 dns_domain=dc02 dict_key=srv02 ansible_user=ansible ansible_password=NgtI75cKV+Pu -[all:vars] -admin_user=goadmin +[domain] +dc01 +dc02 +srv02 + +[dc] +dc01 +dc02 + +[server] +srv02 + +[workstation] + +[parent_dc] +dc01 + +[child_dc] +dc02 + +[trust] + +[adcs] +dc01 + +[adcs_customtemplates] + +[iis] +srv02 + +[mssql] +srv02 + +[mssql_ssms] +srv02 + +[webdav] +srv02 + +[laps_dc] +[laps_server] +[laps_workstation] + +[update] +srv02 + +[no_update] +dc01 +dc02 + +[defender_on] +dc01 +dc02 + +[defender_off] +srv02 + +[extensions] diff --git a/ad/GOAD-Mini/providers/azure/inventory b/ad/GOAD-Mini/providers/azure/inventory index a4baec51..dcb9e408 100644 --- a/ad/GOAD-Mini/providers/azure/inventory +++ b/ad/GOAD-Mini/providers/azure/inventory @@ -1,10 +1,61 @@ +[all:vars] +domain_name=GOAD-Mini +admin_user=administrator +force_dns_server=no +dns_server=1.1.1.1 +dns_server_forwarder=1.1.1.1 +enable_http_proxy=no +ad_http_proxy=http://x.x.x.x:xxxx +ad_https_proxy=http://x.x.x.x:xxxx + [default] -; Note: ansible_host *MUST* be an IPv4 address or setting things like DNS -; servers will break. ; ------------------------------------------------ -; sevenkingdoms.local +; yourcompany.local ; ------------------------------------------------ dc01 ansible_host={{ip_range}}.10 dns_domain=dc01 dict_key=dc01 ansible_user=ansible ansible_password=8dCT-DJjgScp -[all:vars] -admin_user=goadmin +[domain] +dc01 + +[dc] +dc01 + +[server] + +[workstation] + +[parent_dc] +dc01 + +[child_dc] + +[trust] + +[adcs] +dc01 + +[adcs_customtemplates] + +[iis] + +[mssql] + +[mssql_ssms] + +[webdav] + +[laps_dc] +[laps_server] +[laps_workstation] + +[update] + +[no_update] +dc01 + +[defender_on] +dc01 + +[defender_off] + +[extensions] diff --git a/ad/GOAD/providers/azure/inventory b/ad/GOAD/providers/azure/inventory index 88ebfc1c..1b6059ee 100644 --- a/ad/GOAD/providers/azure/inventory +++ b/ad/GOAD/providers/azure/inventory @@ -1,3 +1,17 @@ +[all:vars] +domain_name=GOAD +admin_user=administrator +force_dns_server=no +dns_server=1.1.1.1 +dns_server_forwarder=1.1.1.1 +add_route=no +route_gateway=192.168.56.1 +route_network=10.0.0.0/8 +enable_http_proxy=no +ad_http_proxy=http://x.x.x.x:xxxx +ad_https_proxy=http://x.x.x.x:xxxx +keyboard_layouts=["en-US"] + [default] ; Note: ansible_host *MUST* be an IPv4 address or setting things like DNS ; servers will break. @@ -16,5 +30,78 @@ srv02 ansible_host={{ip_range}}.22 dns_domain=dc02 dict_key=srv02 ansible_user=a dc03 ansible_host={{ip_range}}.12 dns_domain=dc03 dict_key=dc03 ansible_user=ansible ansible_password=Ufe-bVXSx9rk srv03 ansible_host={{ip_range}}.23 dns_domain=dc03 dict_key=srv03 ansible_user=ansible ansible_password=978i2pF43UJ- -[all:vars] -admin_user=goadmin +[domain] +dc01 +dc02 +dc03 +srv02 +srv03 + +[dc] +dc01 +dc02 +dc03 + +[server] +srv02 +srv03 + +[workstation] + +[parent_dc] +dc01 +dc03 + +[child_dc] +dc02 + +[trust] +dc01 +dc03 + +[adcs] +dc01 +srv03 + +[adcs_customtemplates] +dc03 + +[iis] +srv02 + +[mssql] +srv02 +srv03 + +[mssql_ssms] +srv02 + +[webdav] +srv02 +srv03 + +[laps_dc] +dc03 +[laps_server] +srv03 +[laps_workstation] + +[update] +srv02 + +[no_update] +dc01 +dc02 +dc03 +srv03 + +[defender_on] +dc01 +dc02 +dc03 +srv03 + +[defender_off] +srv02 + +[extensions] diff --git a/ad/NHA/providers/azure/inventory b/ad/NHA/providers/azure/inventory index 196b2ca3..12b34975 100644 --- a/ad/NHA/providers/azure/inventory +++ b/ad/NHA/providers/azure/inventory @@ -1,3 +1,13 @@ +[all:vars] +domain_name=NHA +admin_user=administrator +force_dns_server=no +dns_server=1.1.1.1 +dns_server_forwarder=1.1.1.1 +enable_http_proxy=no +ad_http_proxy=http://x.x.x.x:xxxx +ad_https_proxy=http://x.x.x.x:xxxx + [default] dc01 ansible_host={{ip_range}}.10 dns_domain=dc01 dict_key=dc01 ansible_user=ansible ansible_password=8dCT-6546541qsdDJjgScp dc02 ansible_host={{ip_range}}.20 dns_domain=dc02 dict_key=dc02 ansible_user=ansible ansible_password=Ufe-qsdaz789bVXSx9rk @@ -5,5 +15,70 @@ srv01 ansible_host={{ip_range}}.21 dns_domain=dc02 dict_key=srv01 ansible_user=a srv02 ansible_host={{ip_range}}.22 dns_domain=dc02 dict_key=srv02 ansible_user=ansible ansible_password=978i2pF43UqsdqsdJ-qsd srv03 ansible_host={{ip_range}}.23 dns_domain=dc02 dict_key=srv03 ansible_user=ansible ansible_password=EalwxkfhqsdP+xh7sdfzaRk6j90 -[all:vars] -admin_user=goadmin +[domain] +dc01 +dc02 +srv01 +srv02 +srv03 + +[dc] +dc01 +dc02 + +[server] +srv01 +srv02 +srv03 + +[workstation] + +[parent_dc] +dc01 +dc02 + +[child_dc] + +[trust] +dc01 +dc02 + +[adcs] +dc01 + +[adcs_customtemplates] + +[iis] +srv01 + +[mssql] +srv02 + +[mssql_ssms] +srv02 + +[webdav] + +[laps_dc] +[laps_server] +[laps_workstation] + +[update] +dc01 +dc02 +srv01 +srv02 +srv03 + +[no_update] + +[defender_on] +dc01 +dc02 +srv01 +srv02 +srv03 + +[defender_off] + +[extensions] diff --git a/ad/SCCM/providers/azure/inventory b/ad/SCCM/providers/azure/inventory index 7ce7a58c..a8a5261f 100644 --- a/ad/SCCM/providers/azure/inventory +++ b/ad/SCCM/providers/azure/inventory @@ -1,9 +1,82 @@ +[all:vars] +domain_name=SCCM +admin_user=administrator +sql_instance_name=MSSQLSERVER +sql_version=MSSQL_2022 +force_dns_server=no +dns_server=1.1.1.1 +dns_server_forwarder=1.1.1.1 +two_adapters=yes +enable_http_proxy=no +ad_http_proxy=http://x.x.x.x:xxxx +ad_https_proxy=http://x.x.x.x:xxxx + [default] dc01 ansible_host={{ip_range}}.10 dns_domain=dc01 dict_key=dc01 ansible_user=ansible ansible_password=AZERTY*qsdfg srv01 ansible_host={{ip_range}}.11 dns_domain=dc01 dict_key=srv01 ansible_user=ansible ansible_password=NgtI75cKV+Pu srv02 ansible_host={{ip_range}}.12 dns_domain=dc01 dict_key=srv02 ansible_user=ansible ansible_password=NgtazecKV+Pu ws01 ansible_host={{ip_range}}.13 dns_domain=dc01 dict_key=ws01 ansible_user=ansible ansible_password=EP+xh7Rk6j90 +[domain] +dc01 +srv01 +srv02 +ws01 -[all:vars] -admin_user=goadmin +[dc] +dc01 + +[server] +srv01 +srv02 + +[workstation] +ws01 + +[parent_dc] +dc01 + +[child_dc] + +[trust] + +[adcs] + +[adcs_customtemplates] + +[iis] + +[mssql] +srv02 + +[mssql_ssms] +srv02 + +[mssql_reporting] +srv02 + +[webdav] + +[laps_dc] +[laps_server] +[laps_workstation] + +[sccm] +srv01 + +[update] +dc01 +srv01 +ws01 + +[no_update] + +[defender_on] +dc01 +srv01 +srv02 +ws01 + +[defender_off] + +[extensions] diff --git a/cli/cmd/provision.go b/cli/cmd/provision.go index fae9aa71..11ebb492 100644 --- a/cli/cmd/provision.go +++ b/cli/cmd/provision.go @@ -133,7 +133,7 @@ func isSSMInventory(cfg *config.Config) bool { // preflightChecks validates tooling, builds the Ansible collection, and // prepares artifacts needed before provisioning playbooks run. func preflightChecks(ctx context.Context, cfg *config.Config) error { - if err := doctor.CheckAnsibleCoreVersion(); err != nil { + if err := doctor.CheckAnsibleCoreVersion(cfg.ResolvedProvider()); err != nil { return fmt.Errorf("ansible-core version check failed: %w", err) } if err := ansible.InstallRequirements(cfg.ProjectRoot); err != nil { @@ -192,7 +192,9 @@ func bootstrapInventory(invPath string) error { func bootstrapFromProviderTemplate(invPath string, cfg *config.Config) error { labName := "GOAD" - if cfg.ResolvedProvider() == "proxmox" { + if ec := cfg.ActiveEnvironment(); ec.Variant && ec.VariantTarget != "" { + labName = filepath.Base(ec.VariantTarget) + } else if cfg.ResolvedProvider() == "proxmox" { labName = cfg.ProxmoxLab() } providerName := cfg.ResolvedProvider() From dd45b2b60244e5b54d11c5f7a2572d42ef8b8ddf Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Thu, 9 Jul 2026 22:07:58 -0400 Subject: [PATCH 07/20] fix(doctor): add pypsrp check for Azure and make ansible-core version provider-aware Azure provisioning via Bastion+controller uses WinRM/PSRP, which requires the pypsrp Python package. Add the existing pypsrp check to runAzureChecks() so users get a clear doctor message instead of a confusing Ansible error. The ansible-core >=2.19 version check now warns instead of failing when provider is Azure. The 2.19 breakage is SSM-specific (AWS) and does not affect Azure WinRM/PSRP connections. Co-Authored-By: Claude --- cli/internal/doctor/checks.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/cli/internal/doctor/checks.go b/cli/internal/doctor/checks.go index 5adb6270..606406b9 100644 --- a/cli/internal/doctor/checks.go +++ b/cli/internal/doctor/checks.go @@ -52,7 +52,7 @@ type Options struct { func RunChecks(opts Options) []CheckResult { var results []CheckResult - results = append(results, checkAnsibleVersion()) + results = append(results, checkAnsibleVersion(opts.Provider)) results = append(results, checkCommand("python3", "Python 3")) results = append(results, checkCommand("jq", "jq")) results = append(results, checkCommand("zip", "zip")) @@ -84,6 +84,7 @@ func runAzureChecks() []CheckResult { results = append(results, checkAzureSSHExtension()) results = append(results, checkTerragrunt()) results = append(results, checkTerraformOrTofu()) + results = append(results, checkPyPSRP()) return results } @@ -173,15 +174,15 @@ func PrintResults(results []CheckResult) int { // compatible version range (<2.19). Returns an error if the version is // incompatible or ansible-core is not found. This is used as a pre-flight // gate before running playbooks. -func CheckAnsibleCoreVersion() error { - result := checkAnsibleVersion() +func CheckAnsibleCoreVersion(provider string) error { + result := checkAnsibleVersion(provider) if result.Status == "fail" { return fmt.Errorf("%s", result.Message) } return nil } -func checkAnsibleVersion() CheckResult { +func checkAnsibleVersion(provider string) CheckResult { out, err := exec.Command("ansible", "--version").CombinedOutput() if err != nil { return CheckResult{ @@ -202,6 +203,14 @@ func checkAnsibleVersion() CheckResult { version := fmt.Sprintf("%s.%s.%s", m[1], m[2], m[3]) if major > 2 || (major == 2 && minor >= 19) { + if provider == "azure" { + return CheckResult{ + Name: "ansible-core", + Status: "warn", + Message: fmt.Sprintf("v%s detected. Versions >=2.19 break Windows SSM (AWS) "+ + "but work fine with Azure WinRM/PSRP", version), + } + } return CheckResult{ Name: "ansible-core", Status: "fail", From 0bf96593ad464e90256eeddc707e98a9367e36f2 Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Thu, 9 Jul 2026 22:08:14 -0400 Subject: [PATCH 08/20] fix(azure): auto-derive subnet CIDRs and pre-install NuGet in bootstrap The Azure network module had hardcoded subnet defaults (10.8.1.0/24) that didn't adjust when vnet_cidr was changed, causing "subnet range outside VNet" errors. Now uses cidrsubnet() to auto-derive subnets from vnet_cidr, matching the AWS module pattern. Explicit overrides still work. Fresh Azure Marketplace VMs sometimes ship with a corrupted PackageManagement.dll that causes "Catastrophic failure" when Ansible tries to install PowerShell modules. Pre-installing the NuGet provider in the bootstrap script avoids this. Co-Authored-By: Claude --- .../test/centralus/goad/templates/bootstrap.ps1.tpl | 7 +++++++ modules/terraform-azure-net/main.tf | 9 +++++++-- modules/terraform-azure-net/variables.tf | 8 ++++---- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/infra/azure/goad-deployment/test/centralus/goad/templates/bootstrap.ps1.tpl b/infra/azure/goad-deployment/test/centralus/goad/templates/bootstrap.ps1.tpl index 2e582b70..c4b47e6b 100644 --- a/infra/azure/goad-deployment/test/centralus/goad/templates/bootstrap.ps1.tpl +++ b/infra/azure/goad-deployment/test/centralus/goad/templates/bootstrap.ps1.tpl @@ -5,6 +5,13 @@ $ErrorActionPreference = 'Stop' $ProgressPreference = 'SilentlyContinue' +# Pre-install NuGet provider. Azure Marketplace images sometimes ship with a +# corrupted PackageManagement.dll that breaks Install-PackageProvider later. +# Force-installing NuGet up front avoids errors in Ansible DSC module steps. +[Net.ServicePointManager]::SecurityProtocol = [Net.SecurityProtocolType]::Tls12 +try { Install-PackageProvider -Name NuGet -MinimumVersion 2.8.5.201 -Force -ErrorAction Stop } +catch { Write-Warning "NuGet provider pre-install failed: $_" } + # Azure provisioning renames the built-in Administrator (SID-500) to whatever # admin_username we passed (here: goadadmin). The GOAD playbooks expect the # SID-500 account to be named 'administrator' — that's what becomes the domain diff --git a/modules/terraform-azure-net/main.tf b/modules/terraform-azure-net/main.tf index f3b4bfb1..06f889b0 100644 --- a/modules/terraform-azure-net/main.tf +++ b/modules/terraform-azure-net/main.tf @@ -1,6 +1,11 @@ locals { name_prefix = "${var.env}-${var.deployment_name}" + # Auto-derive subnet CIDRs from vnet_cidr when not explicitly provided. + # cidrsubnet(..., 8, 1) on a /16 gives the second /24 (e.g. 10.8.1.0/24). + private_subnet = var.private_subnet_cidr != "" ? var.private_subnet_cidr : cidrsubnet(var.vnet_cidr, 8, 1) + public_subnet = var.public_subnet_cidr != "" ? var.public_subnet_cidr : cidrsubnet(var.vnet_cidr, 8, 0) + base_tags = { Module = "terraform-azure-net" Environment = var.env @@ -28,7 +33,7 @@ resource "azurerm_subnet" "private" { name = "${local.name_prefix}-private" resource_group_name = azurerm_resource_group.this.name virtual_network_name = azurerm_virtual_network.this.name - address_prefixes = [var.private_subnet_cidr] + address_prefixes = [local.private_subnet] } resource "azurerm_subnet" "public" { @@ -36,7 +41,7 @@ resource "azurerm_subnet" "public" { name = "${local.name_prefix}-public" resource_group_name = azurerm_resource_group.this.name virtual_network_name = azurerm_virtual_network.this.name - address_prefixes = [var.public_subnet_cidr] + address_prefixes = [local.public_subnet] } # NAT gateway gives outbound-only internet to private-subnet VMs. diff --git a/modules/terraform-azure-net/variables.tf b/modules/terraform-azure-net/variables.tf index 292b0817..f3500bf6 100644 --- a/modules/terraform-azure-net/variables.tf +++ b/modules/terraform-azure-net/variables.tf @@ -20,15 +20,15 @@ variable "vnet_cidr" { } variable "private_subnet_cidr" { - description = "CIDR for the private subnet where lab VMs run." + description = "CIDR for the private subnet where lab VMs run. Auto-derived from vnet_cidr when empty." type = string - default = "10.8.1.0/24" + default = "" } variable "public_subnet_cidr" { - description = "CIDR for the public subnet (jumpbox/bastion)." + description = "CIDR for the public subnet (jumpbox/bastion). Auto-derived from vnet_cidr when empty." type = string - default = "10.8.0.0/24" + default = "" } variable "create_public_subnet" { From ad0b750b20cbae5b2f2c8e6b9ee39555332aa440 Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Fri, 10 Jul 2026 11:53:27 -0400 Subject: [PATCH 09/20] feat(azure): add Azure support to `dreadgoad env create` `env create` was AWS-only. Now supports Azure with provider-aware scaffolding: - Azure env.hcl with vnet_cidr, bastion/controller subnet CIDRs (auto-derived from VNet CIDR) - Azure region.hcl with location instead of aws_region - Defaults --reference to "test" for Azure (the existing Azure env) - Azure inventory generation (resets IPs to PENDING) - env list scans the correct infra directory per provider Usage: dreadgoad env create alpha -p azure --region centralus Co-Authored-By: Claude --- cli/cmd/env_cmd.go | 149 +++++++++++++++++++++++++++++----- cli/cmd/env_cmd_test.go | 34 ++++++++ cli/internal/config/config.go | 9 ++ 3 files changed, 172 insertions(+), 20 deletions(-) create mode 100644 cli/cmd/env_cmd_test.go diff --git a/cli/cmd/env_cmd.go b/cli/cmd/env_cmd.go index 21ae8a88..6d664721 100644 --- a/cli/cmd/env_cmd.go +++ b/cli/cmd/env_cmd.go @@ -3,6 +3,7 @@ package cmd import ( "fmt" "io/fs" + "net" "os" "path/filepath" "regexp" @@ -50,9 +51,9 @@ func init() { envCmd.AddCommand(envCreateCmd) envCmd.AddCommand(envListCmd) - envCreateCmd.Flags().String("region", "", "AWS region for the environment (required; or set via dreadgoad.yaml / DREADGOAD_REGION / --region)") - envCreateCmd.Flags().String("vpc-cidr", "", "VPC CIDR block (default: auto-assigned)") - envCreateCmd.Flags().String("reference", "staging", "Reference environment to copy infrastructure from") + envCreateCmd.Flags().String("region", "", "Region for the environment (e.g. us-west-2 for AWS, centralus for Azure)") + envCreateCmd.Flags().String("vpc-cidr", "", "VPC/VNet CIDR block (default: auto-assigned)") + envCreateCmd.Flags().String("reference", "staging", "Reference environment to copy infrastructure from (default: staging for AWS, test for Azure)") envCreateCmd.Flags().Bool("variant", false, "Generate randomized variant config") envCreateCmd.Flags().Bool("force", false, "Overwrite existing environment") } @@ -77,6 +78,9 @@ func runEnvCreate(cmd *cobra.Command, args []string) error { } vpcCIDR, _ := cmd.Flags().GetString("vpc-cidr") reference, _ := cmd.Flags().GetString("reference") + if !cmd.Flags().Changed("reference") && cfg.ResolvedProvider() == "azure" { + reference = "test" + } useVariant, _ := cmd.Flags().GetBool("variant") force, _ := cmd.Flags().GetBool("force") @@ -88,7 +92,8 @@ func runEnvCreate(cmd *cobra.Command, args []string) error { } func scaffoldEnv(cfg *config.Config, envName, region, vpcCIDR, reference string, useVariant, force bool) error { - infraBase := filepath.Join(cfg.ProjectRoot, "infra", cfg.Infra.Deployment) + provider := cfg.ResolvedProvider() + infraBase := cfg.InfraBasePathForProvider(provider) envDir := filepath.Join(infraBase, envName) regionDir := filepath.Join(envDir, region) @@ -101,22 +106,39 @@ func scaffoldEnv(cfg *config.Config, envName, region, vpcCIDR, reference string, return fmt.Errorf("reference environment %q not found in %s", reference, infraBase) } + cidrLabel := "VPC CIDR:" + if provider == "azure" { + cidrLabel = "VNet CIDR:" + } color.Cyan("Creating environment: %s", envName) + fmt.Printf(" %-14s %s\n", "Provider:", provider) fmt.Printf(" %-14s %s\n", "Region:", region) - fmt.Printf(" %-14s %s\n", "VPC CIDR:", vpcCIDR) + fmt.Printf(" %-14s %s\n", cidrLabel, vpcCIDR) fmt.Printf(" %-14s %s\n", "Reference:", reference) fmt.Printf(" %-14s %v\n", "Variant:", useVariant) fmt.Println() - if err := createEnvHCL(envDir, envName, vpcCIDR); err != nil { - return fmt.Errorf("create env.hcl: %w", err) - } - color.Green(" Created env.hcl") + if provider == "azure" { + if err := createAzureEnvHCL(envDir, envName, vpcCIDR); err != nil { + return fmt.Errorf("create env.hcl: %w", err) + } + color.Green(" Created env.hcl (Azure)") + + if err := createAzureRegionHCL(regionDir, region); err != nil { + return fmt.Errorf("create region.hcl: %w", err) + } + color.Green(" Created %s/region.hcl (location=%s)", region, region) + } else { + if err := createEnvHCL(envDir, envName, vpcCIDR); err != nil { + return fmt.Errorf("create env.hcl: %w", err) + } + color.Green(" Created env.hcl") - if err := createRegionHCL(regionDir, region); err != nil { - return fmt.Errorf("create region.hcl: %w", err) + if err := createRegionHCL(regionDir, region); err != nil { + return fmt.Errorf("create region.hcl: %w", err) + } + color.Green(" Created %s/region.hcl", region) } - color.Green(" Created %s/region.hcl", region) if err := copyInfrastructure(refRegionDir, regionDir); err != nil { return fmt.Errorf("copy infrastructure: %w", err) @@ -139,8 +161,14 @@ func scaffoldEnv(cfg *config.Config, envName, region, vpcCIDR, reference string, } invPath := filepath.Join(cfg.ProjectRoot, envName+"-inventory") - if err := generateInventory(cfg.ProjectRoot, envName, region, reference); err != nil { - return fmt.Errorf("generate inventory: %w", err) + if provider == "azure" { + if err := generateAzureInventory(cfg.ProjectRoot, envName, reference); err != nil { + return fmt.Errorf("generate inventory: %w", err) + } + } else { + if err := generateInventory(cfg.ProjectRoot, envName, region, reference); err != nil { + return fmt.Errorf("generate inventory: %w", err) + } } color.Green(" Created inventory: %s", filepath.Base(invPath)) @@ -151,10 +179,14 @@ func scaffoldEnv(cfg *config.Config, envName, region, vpcCIDR, reference string, fmt.Printf(" 1. Review: %s\n", envDir) fmt.Printf(" 2. Review: %s\n", configPath) fmt.Printf(" 3. Review: %s\n", invPath) - fmt.Printf(" 4. Initialize: dreadgoad --env %s --region %s infra init\n", envName, region) - fmt.Printf(" 5. Plan: dreadgoad --env %s --region %s infra plan\n", envName, region) - fmt.Printf(" 6. Apply: dreadgoad --env %s --region %s infra apply --auto-approve\n", envName, region) - fmt.Printf(" 7. Sync IDs: dreadgoad --env %s --region %s inventory sync\n", envName, region) + if provider == "azure" { + fmt.Printf(" 4. Deploy: dreadgoad -p azure -e %s --region %s up\n", envName, region) + } else { + fmt.Printf(" 4. Initialize: dreadgoad -e %s --region %s infra init\n", envName, region) + fmt.Printf(" 5. Plan: dreadgoad -e %s --region %s infra plan\n", envName, region) + fmt.Printf(" 6. Apply: dreadgoad -e %s --region %s infra apply --auto-approve\n", envName, region) + fmt.Printf(" 7. Sync IDs: dreadgoad -e %s --region %s inventory sync\n", envName, region) + } return nil } @@ -165,8 +197,7 @@ func runEnvList(cmd *cobra.Command, args []string) error { return err } - deployment := cfg.Infra.Deployment - infraBase := filepath.Join(cfg.ProjectRoot, "infra", deployment) + infraBase := cfg.InfraBasePathForProvider(cfg.ResolvedProvider()) entries, err := os.ReadDir(infraBase) if err != nil { @@ -396,3 +427,81 @@ func generateVariantConfig(projectRoot, envName string) error { gen := variant.NewGenerator(source, target, envName) return gen.Run() } + +// deriveAzureSubnets computes bastion and controller subnet CIDRs from a /16 +// VNet CIDR. Given "10.X.0.0/16" it produces: +// +// bastion: 10.X.2.0/26 (64 IPs, required by Azure Bastion) +// controller: 10.X.3.0/28 (16 IPs, single Ansible controller) +func deriveAzureSubnets(vnetCIDR string) (bastionSubnet, controllerSubnet string, err error) { + _, ipnet, err := net.ParseCIDR(vnetCIDR) + if err != nil { + return "", "", fmt.Errorf("invalid VNet CIDR %q: %w", vnetCIDR, err) + } + ones, _ := ipnet.Mask.Size() + if ones != 16 { + return "", "", fmt.Errorf("VNet CIDR must be a /16, got /%d", ones) + } + base := ipnet.IP.To4() + bastionSubnet = fmt.Sprintf("%d.%d.2.0/26", base[0], base[1]) + controllerSubnet = fmt.Sprintf("%d.%d.3.0/28", base[0], base[1]) + return bastionSubnet, controllerSubnet, nil +} + +func createAzureEnvHCL(envDir, envName, vnetCIDR string) error { + bastionSubnet, controllerSubnet, err := deriveAzureSubnets(vnetCIDR) + if err != nil { + return err + } + if err := os.MkdirAll(envDir, 0o755); err != nil { + return err + } + content := fmt.Sprintf(`locals { + deployment_name = "goad" + env = %q + vnet_cidr = %q + + bastion_sku = "Standard" + bastion_subnet_cidr = %q + bastion_tunneling_enabled = true + + controller_subnet_cidr = %q + controller_ssh_source_address_prefix = %q + controller_instance_size = "Standard_D2s_v3" +} +`, envName, vnetCIDR, bastionSubnet, controllerSubnet, bastionSubnet) + return os.WriteFile(filepath.Join(envDir, "env.hcl"), []byte(content), 0o644) +} + +func createAzureRegionHCL(regionDir, location string) error { + if err := os.MkdirAll(regionDir, 0o755); err != nil { + return err + } + content := fmt.Sprintf(`locals { + location = %q +} +`, location) + return os.WriteFile(filepath.Join(regionDir, "region.hcl"), []byte(content), 0o644) +} + +func generateAzureInventory(projectRoot, envName, reference string) error { + refInvPath, err := resolveReferenceInventory(projectRoot, reference) + if err != nil { + return err + } + dstInvPath := filepath.Join(projectRoot, envName+"-inventory") + + data, err := os.ReadFile(refInvPath) + if err != nil { + return fmt.Errorf("read reference inventory %s: %w", filepath.Base(refInvPath), err) + } + content := string(data) + + envRe := regexp.MustCompile(`(?m)^(\s*env=)(.+)$`) + content = envRe.ReplaceAllString(content, "${1}"+envName) + + ipRe := regexp.MustCompile(`(ansible_host=)\S+`) + content = ipRe.ReplaceAllString(content, "${1}PENDING") + + return os.WriteFile(dstInvPath, []byte(content), 0o644) +} diff --git a/cli/cmd/env_cmd_test.go b/cli/cmd/env_cmd_test.go new file mode 100644 index 00000000..7bdab79f --- /dev/null +++ b/cli/cmd/env_cmd_test.go @@ -0,0 +1,34 @@ +package cmd + +import "testing" + +func TestDeriveAzureSubnets(t *testing.T) { + tests := []struct { + name string + vnetCIDR string + wantBast string + wantCtrl string + wantErr bool + }{ + {"standard", "10.8.0.0/16", "10.8.2.0/26", "10.8.3.0/28", false}, + {"different octet", "10.1.0.0/16", "10.1.2.0/26", "10.1.3.0/28", false}, + {"high octet", "10.200.0.0/16", "10.200.2.0/26", "10.200.3.0/28", false}, + {"not /16", "10.8.0.0/24", "", "", true}, + {"invalid CIDR", "not-a-cidr", "", "", true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + bast, ctrl, err := deriveAzureSubnets(tt.vnetCIDR) + if (err != nil) != tt.wantErr { + t.Fatalf("error = %v, wantErr %v", err, tt.wantErr) + } + if bast != tt.wantBast { + t.Errorf("bastion = %q, want %q", bast, tt.wantBast) + } + if ctrl != tt.wantCtrl { + t.Errorf("controller = %q, want %q", ctrl, tt.wantCtrl) + } + }) + } +} diff --git a/cli/internal/config/config.go b/cli/internal/config/config.go index 19fb5f05..cf7a1269 100644 --- a/cli/internal/config/config.go +++ b/cli/internal/config/config.go @@ -449,6 +449,15 @@ func (c *Config) InfraBasePath() string { return filepath.Join(c.ProjectRoot, "infra", c.Infra.Deployment) } +// InfraBasePathForProvider returns the base infra directory for the given provider. +// Azure uses infra/azure/{deployment}; other providers use infra/{deployment}. +func (c *Config) InfraBasePathForProvider(provider string) string { + if provider == "azure" { + return filepath.Join(c.ProjectRoot, "infra", "azure", c.Infra.Deployment) + } + return c.InfraBasePath() +} + // InfraWorkDir returns the working directory for terragrunt operations // at the region level: infra/{deployment}/{env}/{region}/ func (c *Config) InfraWorkDir() (string, error) { From 278219b69f9bfd32f15aa038ba616834e2824fec Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Fri, 10 Jul 2026 14:05:44 -0400 Subject: [PATCH 10/20] chore: add doc comments and use specialChars constant in seedRequired Add missing Go doc comments on createAzureEnvHCL, createAzureRegionHCL, and generateAzureInventory. Replace hardcoded special character string in seedRequired() with the specialChars constant to keep a single source of truth. Co-Authored-By: Claude --- cli/cmd/env_cmd.go | 5 +++++ cli/internal/variant/namegen.go | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/cli/cmd/env_cmd.go b/cli/cmd/env_cmd.go index 6d664721..397b768c 100644 --- a/cli/cmd/env_cmd.go +++ b/cli/cmd/env_cmd.go @@ -448,6 +448,8 @@ func deriveAzureSubnets(vnetCIDR string) (bastionSubnet, controllerSubnet string return bastionSubnet, controllerSubnet, nil } +// createAzureEnvHCL writes an Azure-specific env.hcl with VNet, bastion, and +// controller subnet CIDRs auto-derived from the VNet CIDR. func createAzureEnvHCL(envDir, envName, vnetCIDR string) error { bastionSubnet, controllerSubnet, err := deriveAzureSubnets(vnetCIDR) if err != nil { @@ -473,6 +475,7 @@ func createAzureEnvHCL(envDir, envName, vnetCIDR string) error { return os.WriteFile(filepath.Join(envDir, "env.hcl"), []byte(content), 0o644) } +// createAzureRegionHCL writes an Azure region.hcl with the location field. func createAzureRegionHCL(regionDir, location string) error { if err := os.MkdirAll(regionDir, 0o755); err != nil { return err @@ -484,6 +487,8 @@ func createAzureRegionHCL(regionDir, location string) error { return os.WriteFile(filepath.Join(regionDir, "region.hcl"), []byte(content), 0o644) } +// generateAzureInventory creates an inventory file for Azure by copying the +// reference inventory and resetting host IPs to PENDING. func generateAzureInventory(projectRoot, envName, reference string) error { refInvPath, err := resolveReferenceInventory(projectRoot, reference) if err != nil { diff --git a/cli/internal/variant/namegen.go b/cli/internal/variant/namegen.go index cd2a97d6..81c0f9b5 100644 --- a/cli/internal/variant/namegen.go +++ b/cli/internal/variant/namegen.go @@ -280,7 +280,7 @@ func (cc charClasses) seedRequired() []byte { seed = append(seed, secureChoiceByte(digitChars)) } if cc.special { - seed = append(seed, secureChoiceByte("!@#$%^&*()-_=+")) + seed = append(seed, secureChoiceByte(specialChars)) } return seed } From 0155b21478f2b843df304f3ff1b2949e6dd0a3e8 Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Fri, 10 Jul 2026 14:24:17 -0400 Subject: [PATCH 11/20] fix: address PR review feedback from Copilot - Guard against IPv6 CIDR in deriveAzureSubnets (To4() nil panic) - Use ReplaceAllStringFunc instead of ReplaceAllString for envName replacement to avoid $ in env names corrupting regexp output - Fix checkPyPSRP message from "SSH-mode" to "WinRM/PSRP" so it's accurate for both Azure and Ludus providers Co-Authored-By: Claude --- cli/cmd/env_cmd.go | 10 ++++++++-- cli/internal/doctor/checks.go | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/cli/cmd/env_cmd.go b/cli/cmd/env_cmd.go index 397b768c..1de101ca 100644 --- a/cli/cmd/env_cmd.go +++ b/cli/cmd/env_cmd.go @@ -443,6 +443,9 @@ func deriveAzureSubnets(vnetCIDR string) (bastionSubnet, controllerSubnet string return "", "", fmt.Errorf("VNet CIDR must be a /16, got /%d", ones) } base := ipnet.IP.To4() + if base == nil { + return "", "", fmt.Errorf("VNet CIDR must be IPv4, got %q", vnetCIDR) + } bastionSubnet = fmt.Sprintf("%d.%d.2.0/26", base[0], base[1]) controllerSubnet = fmt.Sprintf("%d.%d.3.0/28", base[0], base[1]) return bastionSubnet, controllerSubnet, nil @@ -502,8 +505,11 @@ func generateAzureInventory(projectRoot, envName, reference string) error { } content := string(data) - envRe := regexp.MustCompile(`(?m)^(\s*env=)(.+)$`) - content = envRe.ReplaceAllString(content, "${1}"+envName) + envRe := regexp.MustCompile(`(?m)^(\s*env=).+$`) + content = envRe.ReplaceAllStringFunc(content, func(match string) string { + eq := strings.Index(match, "=") + return match[:eq+1] + envName + }) ipRe := regexp.MustCompile(`(ansible_host=)\S+`) content = ipRe.ReplaceAllString(content, "${1}PENDING") diff --git a/cli/internal/doctor/checks.go b/cli/internal/doctor/checks.go index 606406b9..c9442c2e 100644 --- a/cli/internal/doctor/checks.go +++ b/cli/internal/doctor/checks.go @@ -402,7 +402,7 @@ func checkPyPSRP() CheckResult { return CheckResult{ Name: "pypsrp + PySocks", Status: "fail", - Message: "missing Python packages required for SSH-mode provisioning. " + + Message: "missing Python packages required for WinRM/PSRP provisioning. " + "Install with: pip install pypsrp requests[socks]", } } From 39a4b8fa732eb5834fc3b0512bdba665ebeda03c Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Fri, 10 Jul 2026 14:56:59 -0400 Subject: [PATCH 12/20] fix: address second round of PR review feedback - generateAzureInventory falls back to the GOAD Azure provider inventory template when no reference inventory file exists (a fresh clone won't have test-inventory) - Terraform private_subnet_cidr output now references local.private_subnet instead of var.private_subnet_cidr, which is empty by default after the auto-derive change Co-Authored-By: Claude --- cli/cmd/env_cmd.go | 7 ++++++- modules/terraform-azure-net/outputs.tf | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/cli/cmd/env_cmd.go b/cli/cmd/env_cmd.go index 1de101ca..ef1bd18f 100644 --- a/cli/cmd/env_cmd.go +++ b/cli/cmd/env_cmd.go @@ -495,7 +495,12 @@ func createAzureRegionHCL(regionDir, location string) error { func generateAzureInventory(projectRoot, envName, reference string) error { refInvPath, err := resolveReferenceInventory(projectRoot, reference) if err != nil { - return err + // Fall back to the base GOAD Azure provider inventory template. + fallback := filepath.Join(projectRoot, "ad", "GOAD", "providers", "azure", "inventory") + if _, ferr := os.Stat(fallback); ferr != nil { + return err // return original error + } + refInvPath = fallback } dstInvPath := filepath.Join(projectRoot, envName+"-inventory") diff --git a/modules/terraform-azure-net/outputs.tf b/modules/terraform-azure-net/outputs.tf index 25e62c06..b93c4830 100644 --- a/modules/terraform-azure-net/outputs.tf +++ b/modules/terraform-azure-net/outputs.tf @@ -29,7 +29,7 @@ output "private_subnet_id" { } output "private_subnet_cidr" { - value = var.private_subnet_cidr + value = local.private_subnet description = "Private subnet CIDR." } From 97654f2c50de4ce7cc8e5ea8aae0d20f7f07a069 Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Fri, 10 Jul 2026 15:11:17 -0400 Subject: [PATCH 13/20] fix: use ReplaceAllStringFunc for AWS inventory generation too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same regexp replacement vulnerability as the Azure path — envName and region containing $ would corrupt output via capture group references. Co-Authored-By: Claude --- cli/cmd/env_cmd.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cli/cmd/env_cmd.go b/cli/cmd/env_cmd.go index ef1bd18f..eeec2119 100644 --- a/cli/cmd/env_cmd.go +++ b/cli/cmd/env_cmd.go @@ -381,9 +381,15 @@ func generateInventory(projectRoot, envName, region, reference string) error { refRegion = strings.TrimSpace(m[2]) } - content = envRe.ReplaceAllString(content, "${1}"+envName) + content = envRe.ReplaceAllStringFunc(content, func(match string) string { + eq := strings.Index(match, "=") + return match[:eq+1] + envName + }) - content = regionRe.ReplaceAllString(content, "${1}"+region) + content = regionRe.ReplaceAllStringFunc(content, func(match string) string { + eq := strings.Index(match, "=") + return match[:eq+1] + region + }) if refRegion != "" { if m := bucketRe.FindStringSubmatch(content); len(m) > 2 { From c73182d5d9ea339e07fe2d3d5643648d7dca5f21 Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Fri, 10 Jul 2026 16:03:39 -0400 Subject: [PATCH 14/20] fix(variant): prevent dotless username collisions after truncation When the original username has no dot (e.g. "missandei"), the generator truncates "firstname.lastname" to just "firstname". Multiple dotless users could get the same firstname, producing duplicate sAMAccountNames. Now passes the truncated name through ensureUnique to add a counter suffix if needed. Co-Authored-By: Claude --- cli/internal/variant/generator.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cli/internal/variant/generator.go b/cli/internal/variant/generator.go index 666acef0..6ae91bf5 100644 --- a/cli/internal/variant/generator.go +++ b/cli/internal/variant/generator.go @@ -382,8 +382,10 @@ func (g *Generator) mapUsers(config *LabConfig) { newUsername := g.nameGen.GenerateUsername() // Preserve single-name pattern: if original has no ".", use only firstname. + // Pass through EnsureUnique to avoid collisions when multiple dotless + // usernames truncate to the same firstname. if !strings.Contains(username, ".") { - newUsername = strings.Split(newUsername, ".")[0] + newUsername = g.nameGen.ensureUnique(strings.Split(newUsername, ".")[0]) } g.mappings.Users[username] = newUsername From 54abc9893e73119db3f4f76c9762e6229663f336 Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Fri, 10 Jul 2026 18:42:45 -0400 Subject: [PATCH 15/20] refactor: extract helpers in env scaffolding and variant name mapping Break scaffoldEnv into focused functions (printEnvSummary, scaffoldHCL, scaffoldLabConfig, scaffoldInventory, printNextSteps) for readability. Extract setNameComponent in the variant generator to reduce duplication in mapUserNameComponents. Update dreadgoad.yaml for Azure dreadindex deployment. Co-Authored-By: Claude --- cli/cmd/env_cmd.go | 76 +++++++++++++++++++------------ cli/internal/variant/generator.go | 58 ++++++++++------------- dreadgoad.yaml | 23 ++++++++-- 3 files changed, 91 insertions(+), 66 deletions(-) diff --git a/cli/cmd/env_cmd.go b/cli/cmd/env_cmd.go index eeec2119..8cfe51ef 100644 --- a/cli/cmd/env_cmd.go +++ b/cli/cmd/env_cmd.go @@ -106,6 +106,33 @@ func scaffoldEnv(cfg *config.Config, envName, region, vpcCIDR, reference string, return fmt.Errorf("reference environment %q not found in %s", reference, infraBase) } + printEnvSummary(provider, envName, region, vpcCIDR, reference, useVariant) + + if err := scaffoldHCL(provider, envDir, regionDir, envName, region, vpcCIDR); err != nil { + return err + } + + if err := copyInfrastructure(refRegionDir, regionDir); err != nil { + return fmt.Errorf("copy infrastructure: %w", err) + } + color.Green(" Copied infrastructure from %s", reference) + + configPath, err := scaffoldLabConfig(cfg.ProjectRoot, envName, useVariant) + if err != nil { + return err + } + + invPath := filepath.Join(cfg.ProjectRoot, envName+"-inventory") + if err := scaffoldInventory(provider, cfg.ProjectRoot, envName, region, reference); err != nil { + return err + } + color.Green(" Created inventory: %s", filepath.Base(invPath)) + + printNextSteps(provider, envName, region, envDir, configPath, invPath) + return nil +} + +func printEnvSummary(provider, envName, region, vpcCIDR, reference string, useVariant bool) { cidrLabel := "VPC CIDR:" if provider == "azure" { cidrLabel = "VNet CIDR:" @@ -117,13 +144,14 @@ func scaffoldEnv(cfg *config.Config, envName, region, vpcCIDR, reference string, fmt.Printf(" %-14s %s\n", "Reference:", reference) fmt.Printf(" %-14s %v\n", "Variant:", useVariant) fmt.Println() +} +func scaffoldHCL(provider, envDir, regionDir, envName, region, vpcCIDR string) error { if provider == "azure" { if err := createAzureEnvHCL(envDir, envName, vpcCIDR); err != nil { return fmt.Errorf("create env.hcl: %w", err) } color.Green(" Created env.hcl (Azure)") - if err := createAzureRegionHCL(regionDir, region); err != nil { return fmt.Errorf("create region.hcl: %w", err) } @@ -133,45 +161,39 @@ func scaffoldEnv(cfg *config.Config, envName, region, vpcCIDR, reference string, return fmt.Errorf("create env.hcl: %w", err) } color.Green(" Created env.hcl") - if err := createRegionHCL(regionDir, region); err != nil { return fmt.Errorf("create region.hcl: %w", err) } color.Green(" Created %s/region.hcl", region) } + return nil +} - if err := copyInfrastructure(refRegionDir, regionDir); err != nil { - return fmt.Errorf("copy infrastructure: %w", err) - } - color.Green(" Copied infrastructure from %s", reference) - - var configPath string +func scaffoldLabConfig(projectRoot, envName string, useVariant bool) (string, error) { if useVariant { - if err := generateVariantConfig(cfg.ProjectRoot, envName); err != nil { - return fmt.Errorf("generate variant config: %w", err) + if err := generateVariantConfig(projectRoot, envName); err != nil { + return "", fmt.Errorf("generate variant config: %w", err) } - configPath = filepath.Join(cfg.ProjectRoot, "ad", "GOAD-"+envName, "data") + configPath := filepath.Join(projectRoot, "ad", "GOAD-"+envName, "data") color.Green(" Generated variant config in %s", configPath) - } else { - if err := copyBaseConfig(cfg.ProjectRoot, envName); err != nil { - return fmt.Errorf("copy base config: %w", err) - } - configPath = filepath.Join(cfg.ProjectRoot, "ad", "GOAD", "data", envName+"-overlay.json") - color.Green(" Created overlay: %s-overlay.json", envName) + return configPath, nil } + if err := copyBaseConfig(projectRoot, envName); err != nil { + return "", fmt.Errorf("copy base config: %w", err) + } + configPath := filepath.Join(projectRoot, "ad", "GOAD", "data", envName+"-overlay.json") + color.Green(" Created overlay: %s-overlay.json", envName) + return configPath, nil +} - invPath := filepath.Join(cfg.ProjectRoot, envName+"-inventory") +func scaffoldInventory(provider, projectRoot, envName, region, reference string) error { if provider == "azure" { - if err := generateAzureInventory(cfg.ProjectRoot, envName, reference); err != nil { - return fmt.Errorf("generate inventory: %w", err) - } - } else { - if err := generateInventory(cfg.ProjectRoot, envName, region, reference); err != nil { - return fmt.Errorf("generate inventory: %w", err) - } + return generateAzureInventory(projectRoot, envName, reference) } - color.Green(" Created inventory: %s", filepath.Base(invPath)) + return generateInventory(projectRoot, envName, region, reference) +} +func printNextSteps(provider, envName, region, envDir, configPath, invPath string) { fmt.Println() color.Green("Environment %q created successfully!", envName) fmt.Println() @@ -187,8 +209,6 @@ func scaffoldEnv(cfg *config.Config, envName, region, vpcCIDR, reference string, fmt.Printf(" 6. Apply: dreadgoad -e %s --region %s infra apply --auto-approve\n", envName, region) fmt.Printf(" 7. Sync IDs: dreadgoad -e %s --region %s inventory sync\n", envName, region) } - - return nil } func runEnvList(cmd *cobra.Command, args []string) error { diff --git a/cli/internal/variant/generator.go b/cli/internal/variant/generator.go index 6ae91bf5..5cdd7fef 100644 --- a/cli/internal/variant/generator.go +++ b/cli/internal/variant/generator.go @@ -405,48 +405,40 @@ func (g *Generator) mapUserNameComponents(user *UserConfig, newUsername string) if user == nil { return } - if user.Firstname != "" { - firstname := user.Firstname + if user.Firstname != "" && user.Firstname != "sql" { newFirst := strings.Split(newUsername, ".")[0] - // Only set if not already mapped — prevents collisions when two users - // share a firstname (e.g., both map to "brandon"). - if _, exists := g.mappings.Misc[firstname]; !exists { - g.mappings.Misc[firstname] = newFirst - g.nameComponents[firstname] = true - } - if !isAllLower(firstname) && firstname != "sql" { - lower := strings.ToLower(firstname) - if _, exists := g.mappings.Misc[lower]; !exists { - g.mappings.Misc[lower] = strings.ToLower(newFirst) - g.nameComponents[lower] = true - } - } - if isAllLower(firstname) && firstname != "sql" { - cap := capitalize(firstname) - if _, exists := g.mappings.Misc[cap]; !exists { - g.mappings.Misc[cap] = capitalize(newFirst) - g.nameComponents[cap] = true - } - } + g.setNameComponent(user.Firstname, newFirst) } if user.Surname != "" && user.Surname != "-" { - surname := user.Surname parts := strings.SplitN(newUsername, ".", 2) newSurname := parts[0] if len(parts) > 1 { newSurname = parts[1] } - if _, exists := g.mappings.Misc[surname]; !exists { - g.mappings.Misc[surname] = newSurname - g.nameComponents[surname] = true + g.setNameComponent(user.Surname, newSurname) + } +} + +// setNameComponent adds a name mapping (and its case variant) to Misc, +// skipping entries that already exist to prevent collisions. +func (g *Generator) setNameComponent(original, replacement string) { + if _, exists := g.mappings.Misc[original]; !exists { + g.mappings.Misc[original] = replacement + g.nameComponents[original] = true + } + // Add the opposite case variant (lowercase ↔ capitalized). + if isAllLower(original) { + cap := capitalize(original) + if _, exists := g.mappings.Misc[cap]; !exists { + g.mappings.Misc[cap] = capitalize(replacement) + g.nameComponents[cap] = true } - if isAllLower(surname) { - cap := capitalize(surname) - if _, exists := g.mappings.Misc[cap]; !exists { - g.mappings.Misc[cap] = capitalize(newSurname) - g.nameComponents[cap] = true - } + } else { + lower := strings.ToLower(original) + if _, exists := g.mappings.Misc[lower]; !exists { + g.mappings.Misc[lower] = strings.ToLower(replacement) + g.nameComponents[lower] = true } } } @@ -1183,7 +1175,7 @@ func (g *Generator) isTextFile(path string) bool { if err != nil { return false } - defer f.Close() + defer func() { _ = f.Close() }() // Only read the first 8 KiB — enough to detect binary content. buf := make([]byte, 8192) diff --git a/dreadgoad.yaml b/dreadgoad.yaml index 8fac57eb..6d0d2e0c 100644 --- a/dreadgoad.yaml +++ b/dreadgoad.yaml @@ -1,12 +1,12 @@ # DreadGOAD CLI Configuration -env: staging -provider: aws # Provider: aws, proxmox, ludus -# region: us-west-2 # Override AWS region (default: from inventory) +env: dreadindex +provider: azure # Provider: aws, azure, proxmox, ludus +region: centralus instance_profile: WarpgateImageBuilderInstanceProfile # IAM instance profile for EC2 Image Builder debug: false max_retries: 3 retry_delay: 30 -idle_timeout: 1200 +idle_timeout: 3600 # log_dir: ~/.ansible/logs/goad # project_root: /path/to/DreadGOAD # Auto-detected if omitted @@ -51,9 +51,22 @@ environments: variant_name: variant-1 vpc_cidr: "10.0.0.0/16" staging: - variant: false + variant: true + variant_source: ad/GOAD + variant_target: ad/GOAD-dreadindex + variant_name: dreadindex vpc_cidr: "10.1.0.0/16" prod: vpc_cidr: "10.2.0.0/16" test: + variant: true + variant_source: ad/GOAD + variant_target: ad/GOAD-dreadindex + variant_name: dreadindex vpc_cidr: "10.8.0.0/16" + dreadindex: + variant: true + variant_source: ad/GOAD + variant_target: ad/GOAD-dreadindex + variant_name: dreadindex + vpc_cidr: "10.1.0.0/16" From e27ba4f89ffd4b6ff4ddd4f6f5a557753ad9267d Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Fri, 10 Jul 2026 18:54:38 -0400 Subject: [PATCH 16/20] chore: regenerate terraform-azure-net docs after variable changes Co-Authored-By: Claude --- modules/terraform-azure-net/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/terraform-azure-net/README.md b/modules/terraform-azure-net/README.md index cce3e605..df7f3d95 100644 --- a/modules/terraform-azure-net/README.md +++ b/modules/terraform-azure-net/README.md @@ -144,8 +144,8 @@ No modules. | [deployment\_name](#input\_deployment\_name) | Name of the deployment (e.g. "goad"). | `string` | n/a | yes | | [env](#input\_env) | Environment name (e.g. test, staging). | `string` | n/a | yes | | [location](#input\_location) | Azure region. | `string` | n/a | yes | -| [private\_subnet\_cidr](#input\_private\_subnet\_cidr) | CIDR for the private subnet where lab VMs run. | `string` | `"10.8.1.0/24"` | no | -| [public\_subnet\_cidr](#input\_public\_subnet\_cidr) | CIDR for the public subnet (jumpbox/bastion). | `string` | `"10.8.0.0/24"` | no | +| [private\_subnet\_cidr](#input\_private\_subnet\_cidr) | CIDR for the private subnet where lab VMs run. Auto-derived from vnet\_cidr when empty. | `string` | `""` | no | +| [public\_subnet\_cidr](#input\_public\_subnet\_cidr) | CIDR for the public subnet (jumpbox/bastion). Auto-derived from vnet\_cidr when empty. | `string` | `""` | no | | [vnet\_cidr](#input\_vnet\_cidr) | CIDR block for the VNet. | `string` | `"10.8.0.0/16"` | no | ## Outputs From 10acecea4c764e10abd646fc99bca25bb18cab66 Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Fri, 10 Jul 2026 18:56:39 -0400 Subject: [PATCH 17/20] fix: wrap inventory error with context in scaffoldInventory Co-Authored-By: Claude --- cli/cmd/env_cmd.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cli/cmd/env_cmd.go b/cli/cmd/env_cmd.go index 8cfe51ef..321b3d20 100644 --- a/cli/cmd/env_cmd.go +++ b/cli/cmd/env_cmd.go @@ -187,10 +187,16 @@ func scaffoldLabConfig(projectRoot, envName string, useVariant bool) (string, er } func scaffoldInventory(provider, projectRoot, envName, region, reference string) error { + var err error if provider == "azure" { - return generateAzureInventory(projectRoot, envName, reference) + err = generateAzureInventory(projectRoot, envName, reference) + } else { + err = generateInventory(projectRoot, envName, region, reference) + } + if err != nil { + return fmt.Errorf("generate inventory: %w", err) } - return generateInventory(projectRoot, envName, region, reference) + return nil } func printNextSteps(provider, envName, region, envDir, configPath, invPath string) { From 8c4f9387ddc029d9421b99047995b95b449f31a4 Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Fri, 10 Jul 2026 19:15:49 -0400 Subject: [PATCH 18/20] fix: discard unused err from f.Read in isTextFile (ineffassign) Co-Authored-By: Claude --- cli/internal/variant/generator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/internal/variant/generator.go b/cli/internal/variant/generator.go index 5cdd7fef..6e70ae58 100644 --- a/cli/internal/variant/generator.go +++ b/cli/internal/variant/generator.go @@ -1179,7 +1179,7 @@ func (g *Generator) isTextFile(path string) bool { // Only read the first 8 KiB — enough to detect binary content. buf := make([]byte, 8192) - n, err := f.Read(buf) + n, _ := f.Read(buf) if n == 0 { return false } From a89417e38f71f9ef970d9b5e1060225bfb8a2c01 Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Fri, 10 Jul 2026 19:31:42 -0400 Subject: [PATCH 19/20] style: apply De Morgan's law to satisfy staticcheck QF1001 Co-Authored-By: Claude --- cli/internal/variant/generator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/internal/variant/generator.go b/cli/internal/variant/generator.go index 6e70ae58..35c33ee6 100644 --- a/cli/internal/variant/generator.go +++ b/cli/internal/variant/generator.go @@ -1031,7 +1031,7 @@ func (g *Generator) findNameViolations() ([]violation, int) { return nil } ext := filepath.Ext(path) - if !textExtensions[ext] && !textFilenames[d.Name()] && !(ext == "" && g.isTextFile(path)) { + if !textExtensions[ext] && !textFilenames[d.Name()] && (ext != "" || !g.isTextFile(path)) { return nil } filesChecked++ From 613222edc7f060ab38c93f828379351e819d8735 Mon Sep 17 00:00:00 2001 From: mkultraWasHere Date: Fri, 10 Jul 2026 20:08:09 -0400 Subject: [PATCH 20/20] style: fix gofmt struct field alignment Co-Authored-By: Claude --- cli/cmd/env_cmd_test.go | 10 +++++----- cli/internal/variant/generator.go | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/cli/cmd/env_cmd_test.go b/cli/cmd/env_cmd_test.go index 7bdab79f..c8e0b28e 100644 --- a/cli/cmd/env_cmd_test.go +++ b/cli/cmd/env_cmd_test.go @@ -4,11 +4,11 @@ import "testing" func TestDeriveAzureSubnets(t *testing.T) { tests := []struct { - name string - vnetCIDR string - wantBast string - wantCtrl string - wantErr bool + name string + vnetCIDR string + wantBast string + wantCtrl string + wantErr bool }{ {"standard", "10.8.0.0/16", "10.8.2.0/26", "10.8.3.0/28", false}, {"different octet", "10.1.0.0/16", "10.1.2.0/26", "10.1.3.0/28", false}, diff --git a/cli/internal/variant/generator.go b/cli/internal/variant/generator.go index 35c33ee6..4c9f90fb 100644 --- a/cli/internal/variant/generator.go +++ b/cli/internal/variant/generator.go @@ -161,9 +161,9 @@ type HostMapping struct { // replacement is an ordered old->new string replacement. type replacement struct { - Old string - New string - WordBoundary bool // use word-boundary regex instead of plain replacement + Old string + New string + WordBoundary bool // use word-boundary regex instead of plain replacement } // Generator creates GOAD variants with randomized entity names. @@ -177,8 +177,8 @@ type Generator struct { replacements []replacement userPasswordMap map[string]string // new_username -> new_password preservedUsers map[string]bool - pwdInDescUsers map[string]bool // new_username -> has password in description - nameComponents map[string]bool // Misc keys that are firstname/surname components + pwdInDescUsers map[string]bool // new_username -> has password in description + nameComponents map[string]bool // Misc keys that are firstname/surname components } // hostnameAliases maps canonical hostnames to known typos/aliases in upstream GOAD.