Ensure ingress generation follows HTTP/hostname contract and split manifest generation into plan/render/post-render stages

cmd/devenv/generate.go

@@ -31,6 +31,7 @@ var (
 	configDir string // Input directory for developer configs
 	dryRun    bool
 	allDevs   bool
+	noCleanup bool
 )
 
 var generateCmd = &cobra.Command{
@@ -75,6 +76,7 @@ func init() {
 	generateCmd.Flags().StringVar(&configDir, "config-dir", "./developers", "Directory containing developer configuration files")
 	generateCmd.Flags().BoolVar(&dryRun, "dry-run", false, "Show what would be generated without creating files")
 	generateCmd.Flags().BoolVar(&allDevs, "all-developers", false, "Generate manifests for all developers")
+	generateCmd.Flags().BoolVar(&noCleanup, "no-cleanup", false, "Preserve files from previous runs instead of deleting prior generated manifests before rendering")
 
 }
 
@@ -266,30 +268,52 @@ func generateSingleDeveloper(developerName string) {
 }
 
 func generateSystemManifests(cfg *config.BaseConfig, outputDir string) error {
-	// Create template renderer
-	renderer := templates.NewSystemRenderer(outputDir)
+	if !noCleanup {
+		if err := cleanupTemplateOutputs(outputDir, templates.SystemCleanupScope()); err != nil {
+			return fmt.Errorf("failed to clean output directory: %w", err)
+		}
+	}
 
-	// Render all main templates
-	if err := renderer.RenderAll(cfg); err != nil {
+	templateNames := templates.BuildSystemRenderPlan()
+	renderer := templates.NewSystemRenderer(outputDir, cfg, templateNames)
+
+	if err := renderer.RenderAll(); err != nil {
 		return fmt.Errorf("failed to render templates: %w", err)
 	}
 
 	fmt.Printf("🎉 Successfully generated system manifests\n")
-
 	return nil
 }
 
 // generateDeveloperManifests creates Kubernetes manifests for a developer
 func generateDeveloperManifests(cfg *config.DevEnvConfig, outputDir string) error {
-	// Create template renderer
-	renderer := templates.NewDevRenderer(outputDir)
+	if !noCleanup {
+		if err := cleanupTemplateOutputs(outputDir, templates.DevCleanupScope()); err != nil {
+			return fmt.Errorf("failed to clean output directory: %w", err)
+		}
+	}
+
+	templateNames, err := templates.BuildDevRenderPlan(cfg)
+	if err != nil {
+		return fmt.Errorf("failed to build render plan: %w", err)
+	}
+	renderer := templates.NewDevRenderer(outputDir, cfg, templateNames)
 
-	// Render all main templates
-	if err := renderer.RenderAll(cfg); err != nil {
+	if err := renderer.RenderAll(); err != nil {
 		return fmt.Errorf("failed to render templates: %w", err)
 	}
 
 	fmt.Printf("🎉 Successfully generated manifests for %s\n", cfg.Name)
+	return nil
+}
+
+func cleanupTemplateOutputs(outputDir string, templateNames []string) error {
+	for _, templateName := range templateNames {
+		outputPath := filepath.Join(outputDir, templateName+".yaml")
+		if err := os.Remove(outputPath); err != nil && !os.IsNotExist(err) {
+			return err
+		}
+	}
 
 	return nil
 }

internal/config/types.go

@@ -222,6 +222,21 @@ func (c *DevEnvConfig) NodePort() int {
 	return c.SSHPort
 }
 
+// HasHTTPPort reports whether HTTP exposure is configured.
+func (c *DevEnvConfig) HasHTTPPort() bool {
+	return c.HTTPPort != 0
+}
+
+// HasHostName reports whether a non-empty ingress hostname is configured.
+func (c *DevEnvConfig) HasHostName() bool {
+	return strings.TrimSpace(c.HostName) != ""
+}
+
+// ShouldRenderIngress reports whether ingress can be safely rendered.
+func (c *DevEnvConfig) ShouldRenderIngress() bool {
+	return c.HasHTTPPort() && c.HasHostName()
+}
+
 // VolumeMounts returns the configured volume mount specifications.
 // Returns the slice of VolumeMount configurations for binding local directories
 // into the developer environment container.

internal/config/validation.go

@@ -250,6 +250,22 @@ func ValidateDevEnvConfig(config *DevEnvConfig) error {
 		return fmt.Errorf("gpu must be >= 0")
 	}
 
+	if config.HasHTTPPort() && !config.HasHostName() {
+		return fmt.Errorf("hostName is required when httpPort is set")
+	}
+
+	if config.EnableAuth && config.SkipAuth {
+		return fmt.Errorf("enableAuth and skipAuth cannot both be true")
+	}
+
+	if config.EnableAuth && strings.TrimSpace(config.AuthURL) == "" {
+		return fmt.Errorf("authURL is required when enableAuth is true")
+	}
+
+	if config.EnableAuth && strings.TrimSpace(config.AuthSignIn) == "" {
+		return fmt.Errorf("authSignIn is required when enableAuth is true")
+	}
+
 	return nil
 }
 

internal/config/validation_test.go

@@ -347,3 +347,73 @@ func TestValidateDevEnvConfig_VolumeMountPaths(t *testing.T) {
 		assert.Contains(t, err.Error(), "ContainerPath")
 	})
 }
+
+func TestValidateDevEnvConfig_IngressDependencies(t *testing.T) {
+	newCfg := func() *DevEnvConfig {
+		return &DevEnvConfig{
+			Name: "alice",
+			BaseConfig: BaseConfig{
+				SSHPublicKey: "ssh-ed25519 AAAAB3NzaC1lZDI1NTE5AAAA user@host",
+			},
+		}
+	}
+
+	t.Run("requires hostName when httpPort is set", func(t *testing.T) {
+		cfg := newCfg()
+		cfg.HTTPPort = 8080
+
+		err := ValidateDevEnvConfig(cfg)
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "hostName is required when httpPort is set")
+	})
+
+	t.Run("allows httpPort with hostName", func(t *testing.T) {
+		cfg := newCfg()
+		cfg.HTTPPort = 8080
+		cfg.HostName = "devenv.example.com"
+
+		require.NoError(t, ValidateDevEnvConfig(cfg))
+	})
+
+	t.Run("requires authURL when enableAuth is true", func(t *testing.T) {
+		cfg := newCfg()
+		cfg.EnableAuth = true
+		cfg.AuthSignIn = "https://auth.example.com/start"
+
+		err := ValidateDevEnvConfig(cfg)
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "authURL is required when enableAuth is true")
+	})
+
+	t.Run("requires authSignIn when enableAuth is true", func(t *testing.T) {
+		cfg := newCfg()
+		cfg.EnableAuth = true
+		cfg.AuthURL = "https://auth.example.com/auth"
+
+		err := ValidateDevEnvConfig(cfg)
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "authSignIn is required when enableAuth is true")
+	})
+
+	t.Run("allows enableAuth with auth URLs", func(t *testing.T) {
+		cfg := newCfg()
+		cfg.EnableAuth = true
+		cfg.AuthURL = "https://auth.example.com/auth"
+		cfg.AuthSignIn = "https://auth.example.com/start"
+
+		require.NoError(t, ValidateDevEnvConfig(cfg))
+	})
+
+	t.Run("rejects enableAuth with skipAuth", func(t *testing.T) {
+		cfg := newCfg()
+		cfg.EnableAuth = true
+		cfg.SkipAuth = true
+		cfg.AuthURL = "https://auth.example.com/auth"
+		cfg.AuthSignIn = "https://auth.example.com/start"
+
+		err := ValidateDevEnvConfig(cfg)
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "enableAuth and skipAuth cannot both be true")
+	})
+
+}

internal/templates/plan.go

@@ -0,0 +1,33 @@
+package templates
+
+import "github.com/nauticalab/devenv-engine/internal/config"
+
+var devTemplates = []string{"statefulset", "service", "env-vars", "startup-scripts", "ingress"}
+
+var systemTemplates = []string{"namespace"}
+
+// BuildDevRenderPlan computes the template set from config before rendering.
+func BuildDevRenderPlan(cfg *config.DevEnvConfig) ([]string, error) {
+	templateNames := make([]string, 0, len(devTemplates))
+	for _, templateName := range devTemplates {
+		if templateName == "ingress" && !cfg.ShouldRenderIngress() {
+			continue
+		}
+		templateNames = append(templateNames, templateName)
+	}
+
+	return templateNames, nil
+}
+
+// BuildSystemRenderPlan computes the template set for system-level manifests.
+func BuildSystemRenderPlan() []string {
+	return append([]string{}, systemTemplates...)
+}
+
+func DevCleanupScope() []string {
+	return append([]string{}, devTemplates...)
+}
+
+func SystemCleanupScope() []string {
+	return append([]string{}, systemTemplates...)
+}

internal/templates/plan_test.go

@@ -0,0 +1,80 @@
+package templates
+
+import (
+	"testing"
+
+	"github.com/nauticalab/devenv-engine/internal/config"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestBuildDevRenderPlan(t *testing.T) {
+	t.Run("excludes ingress when HTTP port is unset", func(t *testing.T) {
+		cfg := &config.DevEnvConfig{}
+
+		templateNames, err := BuildDevRenderPlan(cfg)
+		require.NoError(t, err)
+
+		assert.Equal(t, expectedDevTemplateNames(false), templateNames)
+	})
+
+	t.Run("includes ingress when HTTP port is set", func(t *testing.T) {
+		cfg := &config.DevEnvConfig{HTTPPort: 8080, BaseConfig: config.BaseConfig{HostName: "devenv.example.com"}}
+
+		templateNames, err := BuildDevRenderPlan(cfg)
+		require.NoError(t, err)
+
+		assert.Equal(t, expectedDevTemplateNames(true), templateNames)
+	})
+
+	t.Run("excludes ingress when hostName is missing", func(t *testing.T) {
+		cfg := &config.DevEnvConfig{HTTPPort: 8080}
+
+		templateNames, err := BuildDevRenderPlan(cfg)
+		require.NoError(t, err)
+
+		assert.Equal(t, expectedDevTemplateNames(false), templateNames)
+	})
+}
+
+func TestBuildDevRenderPlan_Contract(t *testing.T) {
+	t.Run("http disabled", func(t *testing.T) {
+		templateNames, err := BuildDevRenderPlan(&config.DevEnvConfig{})
+		require.NoError(t, err)
+		assert.Equal(t, []string{"statefulset", "service", "env-vars", "startup-scripts"}, templateNames)
+	})
+
+	t.Run("http enabled", func(t *testing.T) {
+		templateNames, err := BuildDevRenderPlan(&config.DevEnvConfig{HTTPPort: 8080, BaseConfig: config.BaseConfig{HostName: "devenv.example.com"}})
+		require.NoError(t, err)
+		assert.Equal(t, []string{"statefulset", "service", "env-vars", "startup-scripts", "ingress"}, templateNames)
+	})
+}
+
+func TestBuildSystemRenderPlan(t *testing.T) {
+	templateNames := BuildSystemRenderPlan()
+
+	assert.Equal(t, copyTemplateNames(systemTemplates), templateNames)
+}
+
+func TestBuildSystemRenderPlan_Contract(t *testing.T) {
+	templateNames := BuildSystemRenderPlan()
+	assert.Equal(t, []string{"namespace"}, templateNames)
+}
+
+func TestTemplateScopes(t *testing.T) {
+	assert.Equal(t, []string{"statefulset", "service", "env-vars", "startup-scripts", "ingress"}, DevCleanupScope())
+	assert.Equal(t, []string{"namespace"}, SystemCleanupScope())
+}
+
+func expectedDevTemplateNames(includeOptional bool) []string {
+	templateNames := []string{"statefulset", "service", "env-vars", "startup-scripts"}
+	if includeOptional {
+		templateNames = append(templateNames, "ingress")
+	}
+	return templateNames
+}
+
+func copyTemplateNames(templateNames []string) []string {
+	return append([]string{}, templateNames...)
+}

internal/templates/renderer.go

@@ -12,11 +12,6 @@ import (
 	"github.com/nauticalab/devenv-engine/internal/config"
 )
 
-var devTemplatesToRender = []string{"statefulset", "service", "env-vars",
-	"startup-scripts", "ingress"}
-
-var systemTemplatesToRender = []string{"namespace"}
-
 // Embed all devTemplates and scripts at compile time
 //
 //go:embed template_files
@@ -27,22 +22,26 @@ type Renderer[T config.BaseConfig | config.DevEnvConfig] struct {
 	outputDir       string
 	templateRoot    string
 	targetTemplates []string
+	config          *T
 }
 
-// NewRenderer creates a new template renderer
-func NewDevRenderer(outputDir string) *Renderer[config.DevEnvConfig] {
-	return NewRendererWithFS[config.DevEnvConfig](outputDir, "template_files/dev", devTemplatesToRender)
+// NewDevRenderer is a convenience wrapper for dev-specific render tests and
+// direct callers.
+func NewDevRenderer(outputDir string, cfg *config.DevEnvConfig, templateNames []string) *Renderer[config.DevEnvConfig] {
+	return NewRenderer[config.DevEnvConfig](outputDir, "template_files/dev", templateNames, cfg)
 }
 
-func NewSystemRenderer(outputDir string) *Renderer[config.BaseConfig] {
-	return NewRendererWithFS[config.BaseConfig](outputDir, "template_files/system", systemTemplatesToRender)
+// NewSystemRenderer is a convenience wrapper for system-specific direct callers.
+func NewSystemRenderer(outputDir string, cfg *config.BaseConfig, templateNames []string) *Renderer[config.BaseConfig] {
+	return NewRenderer[config.BaseConfig](outputDir, "template_files/system", templateNames, cfg)
 }
 
-func NewRendererWithFS[T config.BaseConfig | config.DevEnvConfig](outputDir string, templateRoot string, targetTemplates []string) *Renderer[T] {
+func NewRenderer[T config.BaseConfig | config.DevEnvConfig](outputDir string, templateRoot string, targetTemplates []string, cfg *T) *Renderer[T] {
 	return &Renderer[T]{
 		outputDir:       outputDir,
 		templateRoot:    templateRoot,
 		targetTemplates: targetTemplates,
+		config:          cfg,
 	}
 }
 
@@ -85,7 +84,7 @@ func templateFuncs(templateRoot string) template.FuncMap {
 	}
 }
 
-func (r *Renderer[T]) RenderTemplate(templateName string, config *T) error {
+func (r *Renderer[T]) RenderTemplate(templateName string) error {
 	// Get the template content from embedded files
 	templateContent, err := templates.ReadFile(filepath.Join(r.templateRoot, fmt.Sprintf("manifests/%s.tmpl", templateName)))
 	if err != nil {
@@ -115,19 +114,20 @@ func (r *Renderer[T]) RenderTemplate(templateName string, config *T) error {
 	defer outputFile.Close()
 
 	// Execute template with DevEnvConfig - simple and clean!
-	if err := tmpl.Execute(outputFile, config); err != nil {
+	if err := tmpl.Execute(outputFile, r.config); err != nil {
 		return fmt.Errorf("failed to render template %s: %w", templateName, err)
 	}
 
 	fmt.Printf("✅ Generated %s\n", outputPath)
 	return nil
 }
 
-func (r *Renderer[T]) RenderAll(config *T) error {
+func (r *Renderer[T]) RenderAll() error {
 	for _, templateName := range r.targetTemplates {
-		if err := r.RenderTemplate(templateName, config); err != nil {
+		if err := r.RenderTemplate(templateName); err != nil {
 			return fmt.Errorf("failed to render template %s: %w", templateName, err)
 		}
 	}
+
 	return nil
 }