|
| 1 | +# Config Migration and Interactive Init Design |
| 2 | + |
| 3 | +## Problem |
| 4 | + |
| 5 | +1. Users with existing `.okdev.yaml` files containing deprecated fields (e.g., `spec.workspace`) get hard errors with no automated fix path. |
| 6 | +2. `okdev init` is non-interactive and uses hardcoded `fmt.Sprintf` templates that are not customizable. |
| 7 | + |
| 8 | +## Decisions |
| 9 | + |
| 10 | +| Question | Decision | |
| 11 | +|----------|----------| |
| 12 | +| Migration trigger | Explicit `okdev migrate` command | |
| 13 | +| Migration behavior | Best-effort with YAML comment annotations for ambiguous parts | |
| 14 | +| Migration scope | Content-only (no file location changes) | |
| 15 | +| Init interactivity | Interactive by default, `--yes` to skip | |
| 16 | +| Template system | User-provided via path/URL, built-in ones by name | |
| 17 | +| Overall approach | Unified migration registry + Go `text/template` engine | |
| 18 | + |
| 19 | +--- |
| 20 | + |
| 21 | +## 1. Migration System |
| 22 | + |
| 23 | +### `okdev migrate` Command |
| 24 | + |
| 25 | +``` |
| 26 | +okdev migrate [flags] |
| 27 | + -c, --config <path> Config file to migrate (uses standard discovery if omitted) |
| 28 | + --dry-run Print migrated config to stdout without writing |
| 29 | + --backup Save original as .okdev.yaml.bak before overwriting (default: true) |
| 30 | +``` |
| 31 | + |
| 32 | +### Migration Registry |
| 33 | + |
| 34 | +A chain of named migration functions, each targeting a specific transformation: |
| 35 | + |
| 36 | +```go |
| 37 | +type Migration struct { |
| 38 | + Name string |
| 39 | + Description string |
| 40 | + Applies func(node *yaml.Node) bool |
| 41 | + Transform func(node *yaml.Node) ([]string, error) // returns warnings |
| 42 | +} |
| 43 | +``` |
| 44 | + |
| 45 | +Migrations are registered in order and run sequentially. Each migration: |
| 46 | + |
| 47 | +1. Checks if it applies (e.g., does `spec.workspace` exist?) |
| 48 | +2. Transforms the YAML node tree |
| 49 | +3. Returns warnings for anything that couldn't be auto-resolved |
| 50 | + |
| 51 | +### YAML Node Manipulation |
| 52 | + |
| 53 | +Use `gopkg.in/yaml.v3` node API for the migrate command specifically. This preserves: |
| 54 | + |
| 55 | +- User comments |
| 56 | +- Key ordering |
| 57 | +- Formatting/indentation |
| 58 | + |
| 59 | +The rest of the codebase continues using struct-based parsing via `sigs.k8s.io/yaml`. |
| 60 | + |
| 61 | +**Round-trip safety:** Since `sigs.k8s.io/yaml` (used for loading) and `gopkg.in/yaml.v3` (used for migration) have subtly different serialization behaviors (whitespace, quoting, null handling), the migrated output must be integration-tested to confirm it round-trips cleanly through `sigs.k8s.io/yaml` unmarshaling. Migration tests should load the migrated YAML back through the standard `config.Load()` path and validate the result. |
| 62 | + |
| 63 | +### Idempotency |
| 64 | + |
| 65 | +All migrations must be idempotent. Running `okdev migrate` twice on the same file must produce the same output. This is enforced by the `Applies()` check -- once a migration has transformed the config, `Applies()` should return false on subsequent runs. No version tracking marker is needed; the structure of the YAML itself is the version indicator. |
| 66 | + |
| 67 | +### Ambiguity Handling |
| 68 | + |
| 69 | +When a migration can't fully resolve a field, it: |
| 70 | + |
| 71 | +1. Inserts a YAML comment at the relevant location (e.g., `# TODO: review storageClassName`) |
| 72 | +2. Adds the warning to the summary printed after migration |
| 73 | + |
| 74 | +Example output: |
| 75 | + |
| 76 | +``` |
| 77 | +okdev migrate |
| 78 | + workspace-to-volumes: migrated spec.workspace to spec.volumes + podTemplate |
| 79 | + Warning: Review storageClassName in spec.volumes[0] -- was previously inferred |
| 80 | + Warning: Check volumeMount path matches your workflow |
| 81 | +
|
| 82 | +Wrote migrated config to .okdev.yaml (backup: .okdev.yaml.bak) |
| 83 | +``` |
| 84 | + |
| 85 | +### Current Migrations |
| 86 | + |
| 87 | +One migration to start: |
| 88 | + |
| 89 | +- **workspace-to-volumes**: Transforms `spec.workspace` into `spec.volumes` + `spec.podTemplate.spec.containers[*].volumeMounts`. |
| 90 | + |
| 91 | + Expected `workspace` sub-keys: |
| 92 | + - `mountPath` (string) → becomes `volumeMounts[0].mountPath` |
| 93 | + - `pvc.claimName` (string) → becomes `volumes[0].persistentVolumeClaim.claimName` |
| 94 | + - `pvc.size` (string) → becomes `volumes[0].persistentVolumeClaim.resources.requests.storage` |
| 95 | + - `pvc.storageClassName` (string) → becomes `volumes[0].persistentVolumeClaim.storageClassName` |
| 96 | + |
| 97 | + Since `pvc` is `map[string]string`, unexpected keys are preserved as YAML comments with a warning (e.g., `# TODO: unknown workspace.pvc key "foo" = "bar" -- review manually`). |
| 98 | + |
| 99 | +New migrations are added by appending to the registry as the schema evolves. |
| 100 | + |
| 101 | +--- |
| 102 | + |
| 103 | +## 2. Template System Redesign |
| 104 | + |
| 105 | +### Template Resolution |
| 106 | + |
| 107 | +`--template` accepts three forms: |
| 108 | + |
| 109 | +1. **Built-in name**: `--template basic` resolves to embedded template |
| 110 | +2. **Local path**: `--template ./my-template.yaml` reads from disk |
| 111 | +3. **URL**: `--template https://example.com/template.yaml` fetches remotely |
| 112 | + |
| 113 | +Resolution order: if the value contains a path separator (`/`) or file extension (`.yaml`, `.yml`, `.tmpl`), check as file path first; otherwise check if it's a built-in name. If neither matches, treat as URL. This avoids the footgun where a local file named `basic` is shadowed by the built-in. |
| 114 | + |
| 115 | +### Template Format |
| 116 | + |
| 117 | +Templates are standard `.okdev.yaml` files with Go `text/template` variables: |
| 118 | + |
| 119 | +```yaml |
| 120 | +apiVersion: okdev.io/v1alpha1 |
| 121 | +kind: DevEnvironment |
| 122 | +metadata: |
| 123 | + name: {{ .Name }} |
| 124 | +spec: |
| 125 | + namespace: {{ .Namespace | default "default" }} |
| 126 | + sidecar: |
| 127 | + image: {{ .SidecarImage }} |
| 128 | + session: |
| 129 | + defaultNameTemplate: '{{`{{ .Repo }}-{{ .Branch }}-{{ .User }}`}}' |
| 130 | + sync: |
| 131 | + engine: syncthing |
| 132 | + paths: |
| 133 | + - "{{ .SyncLocal }}:{{ .SyncRemote }}" |
| 134 | + ssh: |
| 135 | + user: {{ .SSHUser | default "root" }} |
| 136 | + {{- if .Ports }} |
| 137 | + ports: |
| 138 | + {{- range .Ports }} |
| 139 | + - name: {{ .Name }} |
| 140 | + local: {{ .Local }} |
| 141 | + remote: {{ .Remote }} |
| 142 | + {{- end }} |
| 143 | + {{- end }} |
| 144 | +``` |
| 145 | + |
| 146 | +**Template escaping:** The existing `session.defaultNameTemplate` field uses `{{ .Repo }}`, `{{ .Branch }}`, `{{ .User }}` syntax that is resolved at session runtime, not at init time. In `.yaml.tmpl` files, these must be escaped using Go's backtick-raw syntax: `` {{` + "`" + `{{ .Repo }}` + "`" + `}} `` so they pass through `text/template` rendering as literal strings. |
| 147 | + |
| 148 | +### Built-in Templates |
| 149 | + |
| 150 | +The current three templates (`basic`, `gpu`, `llm-stack`) are converted from hardcoded `fmt.Sprintf` strings to embedded `.yaml.tmpl` files under `internal/config/templates/`, using `//go:embed` directives. This makes them readable examples for users authoring custom templates. |
| 151 | + |
| 152 | +### Template Variables |
| 153 | + |
| 154 | +```go |
| 155 | +type TemplateVars struct { |
| 156 | + // Common (all templates) |
| 157 | + Name string // metadata.name (default: repo basename) |
| 158 | + Namespace string // default: "default" |
| 159 | + SidecarImage string // default: version-derived |
| 160 | + SyncLocal string // default: "." |
| 161 | + SyncRemote string // default: "/workspace" |
| 162 | + SSHUser string // default: "root" |
| 163 | + Ports []PortVar |
| 164 | + |
| 165 | + // GPU template |
| 166 | + BaseImage string // podTemplate container image (default: "nvidia/cuda:12.4.1-devel-ubuntu22.04") |
| 167 | + GPUCount string // nvidia.com/gpu resource limit (default: "1") |
| 168 | + |
| 169 | + // Session |
| 170 | + TTLHours int // session.ttlHours (default: 0, meaning no TTL) |
| 171 | +} |
| 172 | +``` |
| 173 | + |
| 174 | +Template-specific variables are only prompted when the selected template references them. Built-in templates define which variables they use. For user-provided templates, unused variables are rendered as their zero values -- the template author controls what variables appear via standard Go template syntax. |
| 175 | + |
| 176 | +The prompt system does not auto-discover variables from custom templates. Custom template authors are expected to render the template with `--yes` (defaults) or provide values via flags. Interactive prompts are limited to the known `TemplateVars` fields. |
| 177 | + |
| 178 | +--- |
| 179 | + |
| 180 | +## 3. Interactive `okdev init` Flow |
| 181 | + |
| 182 | +### Default Behavior (Interactive) |
| 183 | + |
| 184 | +When the user runs `okdev init` without `--yes`: |
| 185 | + |
| 186 | +``` |
| 187 | +$ okdev init |
| 188 | +
|
| 189 | +? Template: (basic) [basic / gpu / llm-stack / path or URL] |
| 190 | +? Environment name: (my-repo) |
| 191 | +? Namespace: (default) |
| 192 | +? Sidecar image: (ghcr.io/acmore/okdev:v0.2.1) |
| 193 | +? Sync local path: (.) |
| 194 | +? Sync remote path: (/workspace) |
| 195 | +? SSH user: (root) |
| 196 | +? Add port forwards? (y/N) |
| 197 | +
|
| 198 | +Wrote .okdev.yaml |
| 199 | +``` |
| 200 | + |
| 201 | +Each prompt shows the default in parentheses. Pressing Enter accepts the default. The prompts are derived from the template's variables -- a custom template with different variables would produce different prompts. |
| 202 | + |
| 203 | +Template is asked first. The selected template determines which follow-up questions are asked (e.g., a GPU template might ask about GPU count). |
| 204 | + |
| 205 | +### Non-Interactive Mode |
| 206 | + |
| 207 | +`okdev init --yes` skips all prompts and uses defaults. Equivalent to the current behavior. |
| 208 | + |
| 209 | +Values can also be passed as flags to override specific defaults without prompts: |
| 210 | + |
| 211 | +``` |
| 212 | +okdev init --yes --namespace staging --name my-env |
| 213 | +``` |
| 214 | + |
| 215 | +### Flag Summary |
| 216 | + |
| 217 | +``` |
| 218 | +okdev init [flags] |
| 219 | + -c, --config <path> Output path (default: .okdev.yaml) |
| 220 | + --template <name|path> Template to use (default: basic) |
| 221 | + --force Overwrite existing config |
| 222 | + --yes Non-interactive, accept all defaults |
| 223 | + --name <string> Environment name |
| 224 | + --namespace <string> Namespace |
| 225 | +``` |
| 226 | + |
| 227 | +--- |
| 228 | + |
| 229 | +## 4. Integration |
| 230 | + |
| 231 | +### Separation of Concerns |
| 232 | + |
| 233 | +- `okdev migrate` transforms existing configs. Never prompts for new values; only restructures what's there. |
| 234 | +- `okdev init` creates new configs. Interactive prompts, template rendering. |
| 235 | +- No overlap. |
| 236 | + |
| 237 | +### Validation Integration |
| 238 | + |
| 239 | +The existing `Validate()` error message for deprecated fields is updated to suggest running `okdev migrate`: |
| 240 | + |
| 241 | +``` |
| 242 | +Error: spec.workspace is no longer supported. |
| 243 | +Use spec.volumes (k8s Volume) and podTemplate.spec.containers[*].volumeMounts instead, |
| 244 | +or run "okdev migrate" to automatically update your config. |
| 245 | +``` |
| 246 | + |
| 247 | +The existing manual-fix guidance is preserved, with the `okdev migrate` suggestion appended. |
| 248 | + |
| 249 | +Additionally, `loadConfigAndNamespace()` in `internal/cli/common.go` (the shared config-loading path for all commands like `up`, `ssh`, `sync`, `ports`) detects migration-eligible validation errors and prints a visible hint to stderr: |
| 250 | + |
| 251 | +``` |
| 252 | +Hint: run "okdev migrate" to automatically fix this. |
| 253 | +``` |
| 254 | + |
| 255 | +This uses a `MigrationEligibleError` sentinel type in the config package so the CLI layer can distinguish migration-fixable errors from other validation failures. |
| 256 | + |
| 257 | +### Package Layout |
| 258 | + |
| 259 | +``` |
| 260 | +internal/config/ |
| 261 | + config.go # Structs, defaults, validation (existing) |
| 262 | + loader.go # Load/discovery (existing) |
| 263 | + migrate.go # Migration registry + migrate logic (new) |
| 264 | + migrate_test.go # (new) |
| 265 | + template.go # TemplateVars, rendering, resolution (rewritten) |
| 266 | + template_test.go # (rewritten) |
| 267 | + templates/ |
| 268 | + basic.yaml.tmpl # //go:embed |
| 269 | + gpu.yaml.tmpl # //go:embed |
| 270 | + llm-stack.yaml.tmpl # //go:embed |
| 271 | +
|
| 272 | +internal/cli/ |
| 273 | + init.go # Updated to use new template + prompt system |
| 274 | + prompt.go # Interactive prompts for init (new) |
| 275 | + prompt_test.go # (new) |
| 276 | + migrate.go # New subcommand wiring (new) |
| 277 | +``` |
| 278 | + |
| 279 | +Prompt logic lives in `internal/cli/` (not `internal/config/`) to keep the config package free of terminal/tty dependencies. |
| 280 | + |
| 281 | +### Dependencies |
| 282 | + |
| 283 | +- Interactive prompts: `github.com/AlecAivazis/survey/v2` or `github.com/charmbracelet/huh` |
| 284 | +- YAML node manipulation: `gopkg.in/yaml.v3` (already available) |
| 285 | +- URL template fetching: HTTP GET with 30s timeout, fail on non-200, no caching, TLS verification enabled |
0 commit comments