Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
98bdd54
fix(startup): make user/group provisioning idempotent by using getent…
spapa013 Mar 4, 2026
8200e63
fix(startup): defensively set linuxbrew ownership before Homebrew ins…
spapa013 Mar 5, 2026
82b8ebd
fix(startup): ensure .vscode-server exists before chown to avoid star…
spapa013 Mar 5, 2026
e9c9dba
fix(config): require pythonBinPath to be absolute via explicit valida…
spapa013 Mar 5, 2026
b6edc5d
chore(startup): harden account and ownership commands with consistent…
spapa013 Mar 5, 2026
5691071
fix(startup): bootstrap /opt/venv on demand when default pythonBinPat…
spapa013 Mar 5, 2026
cc146b7
fix(startup): recursively repair persisted home ownership before user…
spapa013 Mar 5, 2026
db29fd8
fix: update golden template internal/scripts
spapa013 Mar 5, 2026
4ca67d0
Merge pull request #24 from spapa013/spapadop/plt-859-stabilize-deven…
spapa013 Mar 6, 2026
f24cbf3
fix(config): validate volume mount paths syntactically instead of fil…
spapa013 Mar 5, 2026
bd8eecf
Merge pull request #25 from spapa013/spapadop/plt-881-volumemount-pat…
spapa013 Mar 6, 2026
bd48064
fix(templates): :bug: set namespace on all namespaced manifests and u…
spapa013 Mar 5, 2026
49d81ae
Merge pull request #26 from spapa013/spapadop/plt-862-fix-namespace-u…
spapa013 Mar 6, 2026
af9b0dd
fix(templates): :bug: add headless devenv-{{.Name}} governing Service…
spapa013 Mar 5, 2026
5cdaefe
Merge pull request #27 from spapa013/spapadop/plt-849-statefulset-ser…
spapa013 Mar 6, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions internal/config/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type BaseConfig struct {
InstallHomebrew bool `yaml:"installHomebrew,omitempty"`
ClearLocalPackages bool `yaml:"clearLocalPackages,omitempty"`
ClearVSCodeCache bool `yaml:"clearVSCodeCache,omitempty"`
PythonBinPath string `yaml:"pythonBinPath,omitempty" validate:"omitempty,min=1,filepath"`
PythonBinPath string `yaml:"pythonBinPath,omitempty" validate:"omitempty,min=1"`
HostName string `yaml:"hostName,omitempty" validate:"omitempty,min=1,hostname"`
EnableAuth bool `yaml:"enableAuth,omitempty"`
AuthURL string `yaml:"authURL,omitempty" validate:"omitempty,min=1,url"`
Expand Down Expand Up @@ -94,8 +94,8 @@ type ResourceConfig struct {
// VolumeMount represents a volume mount configuration
type VolumeMount struct {
Name string `yaml:"name" validate:"required,min=1,max=63,alphanum"`
LocalPath string `yaml:"localPath" validate:"required,min=1,filepath"`
ContainerPath string `yaml:"containerPath" validate:"required,min=1,filepath"`
LocalPath string `yaml:"localPath" validate:"required,mount_path"`
ContainerPath string `yaml:"containerPath" validate:"required,mount_path"`
}

// RefreshConfig represents auto-refresh settings
Expand Down
50 changes: 50 additions & 0 deletions internal/config/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package config
import (
"fmt"
"math"
"path"
"regexp"
"strconv"
"strings"
Expand Down Expand Up @@ -56,6 +57,9 @@ func init() {
if err := validate.RegisterValidation("k8s_memory", validateKubernetesMemory); err != nil {
panic(fmt.Errorf("register validator k8s_memory: %w", err))
}
if err := validate.RegisterValidation("mount_path", validateMountPath); err != nil {
panic(fmt.Errorf("register validator mount_path: %w", err))
}
validate.RegisterStructValidation(validateGitRepo, GitRepo{})
}

Expand Down Expand Up @@ -187,12 +191,42 @@ func validateKubernetesMemory(fl validator.FieldLevel) bool {
}
}

// validateMountPath implements the "mount_path" tag.
// It validates mount paths using syntax only (no filesystem checks).
func validateMountPath(fl validator.FieldLevel) bool {
p, ok := fl.Field().Interface().(string)
if !ok {
return false
}

p = strings.TrimSpace(p)
if p == "" {
return false
}

// Kubernetes-style mount paths are absolute, slash-separated paths.
if !path.IsAbs(p) {
return false
}

// Reject NUL bytes; otherwise rely on lexical cleaning only.
if strings.ContainsRune(p, '\x00') {
return false
}

clean := path.Clean(p)
return clean != "" && clean != "."
}

// ValidateDevEnvConfig runs tag-based validation and then applies
// additional semantic checks that are easier to express in code.
func ValidateDevEnvConfig(config *DevEnvConfig) error {
if err := validate.Struct(config); err != nil {
return formatValidationError(err)
}
if err := validatePythonBinPathAbsolute(config.PythonBinPath); err != nil {
return err
}

// Require ≥1 SSH public key with valid format.
sshKeys, err := config.GetSSHKeys()
Expand Down Expand Up @@ -225,6 +259,20 @@ func ValidateBaseConfig(config *BaseConfig) error {
if err := validate.Struct(config); err != nil {
return formatValidationError(err)
}
if err := validatePythonBinPathAbsolute(config.PythonBinPath); err != nil {
return err
}
return nil
}

func validatePythonBinPathAbsolute(p string) error {
p = strings.TrimSpace(p)
if p == "" {
return nil
}
if !path.IsAbs(p) {
return fmt.Errorf("pythonBinPath must be an absolute path, got %q", p)
}
return nil
}

Expand Down Expand Up @@ -264,6 +312,8 @@ func formatFieldError(fieldError validator.FieldError) string {
return fmt.Sprintf("'%s' must be a valid URL, got '%v'", fieldName, value)
case "filepath":
return fmt.Sprintf("'%s' must be a valid file path, got '%v'", fieldName, value)
case "mount_path":
return fmt.Sprintf("'%s' must be a valid absolute mount path, got '%v'", fieldName, value)
case "cron":
return fmt.Sprintf("'%s' must be a valid cron expression, got '%v'", fieldName, value)

Expand Down
111 changes: 109 additions & 2 deletions internal/config/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,118 @@ func TestValidateDevEnvConfig_ResourcesNonNegative(t *testing.T) {
}

//
// --- ValidateBaseConfig: no tag failures by default -------------------------
// --- ValidateBaseConfig ------------------------------------------------------
//

func TestValidateBaseConfig_Smoke(t *testing.T) {
// With no validation tags on BaseConfig itself, this should succeed.
// Zero-value BaseConfig should still pass baseline validation.
var bc BaseConfig
require.NoError(t, ValidateBaseConfig(&bc))
}

func TestValidateBaseConfig_DefaultsPass(t *testing.T) {
bc := NewBaseConfigWithDefaults()
require.NoError(t, ValidateBaseConfig(&bc))
}

func TestValidateBaseConfig_PythonBinPathMustBeAbsolute(t *testing.T) {
ok := &BaseConfig{PythonBinPath: "/opt/venv/bin"}
require.NoError(t, ValidateBaseConfig(ok))

bad := &BaseConfig{PythonBinPath: "usr/bin"}
err := ValidateBaseConfig(bad)
require.Error(t, err)
assert.Contains(t, err.Error(), "pythonBinPath")
assert.Contains(t, err.Error(), "absolute path")
}

func TestValidateDevEnvConfig_PythonBinPathMustBeAbsolute(t *testing.T) {
ok := &DevEnvConfig{
Name: "alice",
BaseConfig: BaseConfig{
PythonBinPath: "/opt/venv/bin",
SSHPublicKey: "ssh-ed25519 AAAAB3NzaC1lZDI1NTE5AAAA user@host",
},
}
require.NoError(t, ValidateDevEnvConfig(ok))

bad := &DevEnvConfig{
Name: "alice",
BaseConfig: BaseConfig{
PythonBinPath: "opt/venv/bin",
SSHPublicKey: "ssh-ed25519 AAAAB3NzaC1lZDI1NTE5AAAA user@host",
},
}
err := ValidateDevEnvConfig(bad)
require.Error(t, err)
assert.Contains(t, err.Error(), "pythonBinPath")
assert.Contains(t, err.Error(), "absolute path")
}

func TestValidator_MountPath(t *testing.T) {
type S struct {
Path string `validate:"mount_path"`
}

cases := []struct {
name string
val string
ok bool
}{
{name: "root mount dir", val: "/mnt", ok: true},
{name: "mount subpath", val: "/mnt/data", ok: true},
{name: "root", val: "/", ok: true},
{name: "empty", val: "", ok: false},
{name: "whitespace", val: " ", ok: false},
{name: "relative path", val: "mnt/data", ok: false},
}

for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
err := validate.Struct(&S{Path: tc.val})
if tc.ok {
require.NoError(t, err)
} else {
require.Error(t, err)
}
})
}
}

func TestValidateDevEnvConfig_VolumeMountPaths(t *testing.T) {
newCfg := func(localPath, containerPath string) *DevEnvConfig {
return &DevEnvConfig{
Name: "alice",
BaseConfig: BaseConfig{
SSHPublicKey: "ssh-ed25519 AAAAB3NzaC1lZDI1NTE5AAAA user@host",
Volumes: []VolumeMount{
{
Name: "mnt",
LocalPath: localPath,
ContainerPath: containerPath,
},
},
},
}
}

t.Run("accepts root directory mounts", func(t *testing.T) {
require.NoError(t, ValidateDevEnvConfig(newCfg("/mnt", "/mnt")))
})

t.Run("accepts mount subpaths", func(t *testing.T) {
require.NoError(t, ValidateDevEnvConfig(newCfg("/mnt/data", "/mnt/data")))
})

t.Run("rejects empty localPath", func(t *testing.T) {
err := ValidateDevEnvConfig(newCfg("", "/mnt"))
require.Error(t, err)
assert.Contains(t, err.Error(), "LocalPath")
})

t.Run("rejects empty containerPath", func(t *testing.T) {
err := ValidateDevEnvConfig(newCfg("/mnt", ""))
require.Error(t, err)
assert.Contains(t, err.Error(), "ContainerPath")
})
}
6 changes: 4 additions & 2 deletions internal/templates/renderer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func TestRenderTemplate(t *testing.T) {
},
UID: 2000,
Image: "ubuntu:22.04",
Namespace: "devenv-test",
Packages: config.PackageConfig{
Python: []string{"numpy", "pandas"},
APT: []string{"vim", "curl"},
Expand Down Expand Up @@ -57,7 +58,7 @@ func TestRenderTemplate(t *testing.T) {
},
}

templates := []string{"statefulset", "service", "env-vars", "startup-scripts"}
templates := []string{"statefulset", "service", "env-vars", "startup-scripts", "ingress"}

for _, templateName := range templates {
t.Run(templateName, func(t *testing.T) {
Expand Down Expand Up @@ -109,6 +110,7 @@ func TestRenderAll(t *testing.T) {
Name: "minimal",
BaseConfig: config.BaseConfig{
SSHPublicKey: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQC7... minimal@example.com",
Namespace: "devenv-test",
},
SSHPort: 30002,
}
Expand All @@ -121,7 +123,7 @@ func TestRenderAll(t *testing.T) {
require.NoError(t, err, "RenderAll should not return error")

// Verify all expected files were created
expectedFiles := []string{"statefulset.yaml", "service.yaml", "env-vars.yaml", "startup-scripts.yaml"}
expectedFiles := []string{"statefulset.yaml", "service.yaml", "env-vars.yaml", "startup-scripts.yaml", "ingress.yaml"}

for _, filename := range expectedFiles {
filePath := filepath.Join(tempDir, filename)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ apiVersion: v1
kind: ConfigMap
metadata:
name: env-vars-{{.Name}}
namespace: {{.Namespace}}
labels:
app: devenv-{{.Name}}
data:
Expand Down
3 changes: 2 additions & 1 deletion internal/templates/template_files/dev/manifests/ingress.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: devenv-ingress-{{.Name}}
namespace: {{.Namespace}}
annotations:
nginx.ingress.kubernetes.io/force-ssl-redirect: "true"
cert-manager.io/cluster-issuer: "letsencrypt"
Expand All @@ -28,4 +29,4 @@ spec:
tls:
- hosts:
- "*.{{.HostName}}"
secretName: http-{{.Name}}-tls
secretName: http-{{.Name}}-tls
22 changes: 21 additions & 1 deletion internal/templates/template_files/dev/manifests/service.tmpl
Original file line number Diff line number Diff line change
@@ -1,7 +1,26 @@
apiVersion: v1
kind: Service
metadata:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this extra SSH port for?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the reply I got when I asked ChatGPT at the time:

Great question. Practical implications are:

  1. With the port (22) on the headless governing Service:
  • Service is clearly valid/standard and reliably accepted by Kubernetes API.
  • EndpointSlices include a named port (ssh), so SRV-based discovery can work.
  • You get no extra external exposure by itself, because it is still headless (clusterIP: None) and not NodePort/LoadBalancer.
  1. Without any port on that Service:
  • You still get the main StatefulSet DNS identity behavior (pod hostnames) in many setups.
  • But Service definitions without ports are less standard and can be rejected depending on API validation/version/config.
  • You lose explicit service-port metadata (and SRV records tied to named ports).
  1. Bottom line:
  • The port is mostly low-risk structural correctness and compatibility.
  • It does not replace or affect external SSH access; that still comes from devenv-ssh-{{.Name}} NodePort service.

name: devenv-{{.Name}}
namespace: {{.Namespace}}
labels:
app: devenv-{{.Name}}
service: governing
spec:
clusterIP: None
selector:
app: devenv-{{.Name}}
ports:
- name: ssh
port: 22
targetPort: 22
protocol: TCP
---
apiVersion: v1
kind: Service
metadata:
name: devenv-ssh-{{.Name}}
namespace: {{.Namespace}}
labels:
app: devenv-{{.Name}}
service: ssh
Expand All @@ -21,6 +40,7 @@ apiVersion: v1
kind: Service
metadata:
name: devenv-http-{{.Name}}
namespace: {{.Namespace}}
labels:
app: devenv-{{.Name}}
service: http
Expand All @@ -33,4 +53,4 @@ spec:
port: {{.HTTPPort}}
targetPort: {{.HTTPPort}}
protocol: TCP
{{- end}}
{{- end}}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ apiVersion: v1
kind: ConfigMap
metadata:
name: startup-scripts-{{.Name}}
namespace: {{.Namespace}}
labels:
app: devenv-{{.Name}}
data:
Expand All @@ -19,4 +20,4 @@ data:

# User setup script
setup.sh: |
{{getTemplatedScript "user-setup.sh" . | indent 4}}
{{getTemplatedScript "user-setup.sh" . | indent 4}}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ apiVersion: apps/v1
kind: StatefulSet
metadata:
name: devenv-{{.Name}}
namespace: {{.Namespace}}
labels:
app: devenv-{{.Name}}
component: devenv
Expand Down Expand Up @@ -127,4 +128,4 @@ spec:
hostPath:
path: {{.LocalPath}}
type: DirectoryOrCreate
{{- end}}
{{- end}}
Loading