diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 959fbe79..b9f6ec07 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -47,3 +47,63 @@ jobs: echo "Please sign-off commits with: git commit --signoff" exit 1 fi + + coverage: + # Report-only coverage summary. Does NOT block merge — the goal is + # visibility into per-package coverage drift over time. Adding a + # threshold gate would force every PR to also touch tests, which + # disincentivises small focused PRs. Reviewers eyeball the + # numbers; sustained drift gets addressed in dedicated coverage + # commits. + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v6 + - name: Set up Go + uses: actions/setup-go@v6 + with: + go-version: stable + - name: Run tests with coverage + run: | + go test ./... -coverprofile=coverage.out -covermode=atomic + - name: Per-package summary + run: | + # Aggregate coverage per Go package so the PR view shows + # one line per package instead of the per-function default. + # `go tool cover -func` has no per-package mode; awk + # collapses by the package portion of the path. + { + echo "## Coverage report" + echo + echo '```text' + go tool cover -func=coverage.out | awk ' + /^total:/ { total = $NF; next } + { + # path looks like: github.com/cozystack/talm/pkg/engine/engine.go:75:\tfn\t75.0% + split($1, parts, ":") + path = parts[1] + # strip trailing /file.go + pkg = path + sub(/\/[^/]+\.go$/, "", pkg) + # strip module prefix for compactness + sub(/^github\.com\/cozystack\/talm\//, "", pkg) + pct = $NF + sub(/%/, "", pct) + sum[pkg] += pct + cnt[pkg] += 1 + } + END { + for (pkg in sum) { + printf "%-50s %5.1f%% (avg of %d funcs)\n", pkg, sum[pkg]/cnt[pkg], cnt[pkg] + } + if (total != "") printf "\n%-50s %s (overall)\n", "TOTAL", total + } + ' | sort + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + - name: Upload coverage artifact + uses: actions/upload-artifact@v4 + with: + name: coverage-profile + path: coverage.out + retention-days: 14 diff --git a/pkg/age/contract_internals_test.go b/pkg/age/contract_internals_test.go new file mode 100644 index 00000000..bc40705f --- /dev/null +++ b/pkg/age/contract_internals_test.go @@ -0,0 +1,304 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: edge cases for the incremental encrypt/decrypt machinery +// in age.go. EncryptSecretsFile uses mergeAndEncryptYAMLValues to +// preserve unchanged ciphertext; the round-trip pin sits in +// contract_test.go. This file exercises the type-mismatch / new-key +// / new-list-element / type-changed branches of the recursive merger +// — the surface area that decides what does and does NOT get +// re-encrypted. + +package age + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "gopkg.in/yaml.v3" +) + +// === decryptYAMLValuesString === + +// Contract: a string already wrapped in the ENC[AGE,data:...] envelope +// is decrypted to the plaintext it originally encoded. +func TestContract_DecryptYAMLValuesString_RoundTrip(t *testing.T) { + dir := t.TempDir() + id, _, err := GenerateKey(dir) + if err != nil { + t.Fatal(err) + } + encrypted, err := encryptString("hello", id.Recipient()) + if err != nil { + t.Fatal(err) + } + envelope := ageEncryptionPrefix + encrypted + ageEncryptionSuffix + + got, err := decryptYAMLValuesString(envelope, id) + if err != nil { + t.Fatalf("decryptYAMLValuesString: %v", err) + } + if got != "hello" { + t.Errorf("expected 'hello', got %q", got) + } +} + +// Contract: a string that is NOT wrapped in the envelope is returned +// verbatim. This lets the merge logic feed both raw plaintext and +// already-encrypted values through the same helper without a type +// switch at the call site. +func TestContract_DecryptYAMLValuesString_PassthroughForUnencrypted(t *testing.T) { + dir := t.TempDir() + id, _, err := GenerateKey(dir) + if err != nil { + t.Fatal(err) + } + got, err := decryptYAMLValuesString("just-a-plain-string", id) + if err != nil { + t.Fatal(err) + } + if got != "just-a-plain-string" { + t.Errorf("expected passthrough, got %q", got) + } +} + +// Contract: a string with the envelope prefix but corrupted base64 +// payload errors precisely. Pin so a regression that swallows the +// inner error and silently returns the corrupted string surfaces. +func TestContract_DecryptYAMLValuesString_CorruptedEnvelope(t *testing.T) { + dir := t.TempDir() + id, _, err := GenerateKey(dir) + if err != nil { + t.Fatal(err) + } + corrupted := ageEncryptionPrefix + "not-valid-base64!!!" + ageEncryptionSuffix + _, err = decryptYAMLValuesString(corrupted, id) + if err == nil { + t.Fatal("expected error for corrupted envelope") + } +} + +// === mergeAndEncryptYAMLValues — through EncryptSecretsFile === + +// Contract: a NEW key added to plain secrets.yaml gets encrypted on +// the next encrypt round; existing keys keep their byte-stable +// ciphertext. +func TestContract_IncrementalEncrypt_NewKeyEncryptedOldKeyStable(t *testing.T) { + dir := t.TempDir() + if err := writeYAML(dir, "secrets.yaml", "a: alpha\n"); err != nil { + t.Fatal(err) + } + if err := EncryptSecretsFile(dir); err != nil { + t.Fatal(err) + } + first := readYAML(t, dir, "secrets.encrypted.yaml") + + // Add a new key b. + if err := writeYAML(dir, "secrets.yaml", "a: alpha\nb: beta\n"); err != nil { + t.Fatal(err) + } + if err := EncryptSecretsFile(dir); err != nil { + t.Fatal(err) + } + second := readYAML(t, dir, "secrets.encrypted.yaml") + + // a's ciphertext must stay byte-stable. + if first["a"] != second["a"] { + t.Errorf("a's ciphertext rotated unnecessarily\n first: %v\n second: %v", first["a"], second["a"]) + } + // b must be present and encrypted. + bStr, ok := second["b"].(string) + if !ok { + t.Fatalf("b missing or not a string: %v", second["b"]) + } + if !strings.HasPrefix(bStr, ageEncryptionPrefix) { + t.Errorf("b not encrypted: %q", bStr) + } +} + +// Contract: a deeply-nested change re-encrypts only the innermost +// changed leaf. Sibling values up and down the tree stay byte-stable. +func TestContract_IncrementalEncrypt_NestedChangeLocalised(t *testing.T) { + dir := t.TempDir() + initial := `top: + unchanged: keep-me + sub: + inner1: original + inner2: keep-me-too +` + if err := writeYAML(dir, "secrets.yaml", initial); err != nil { + t.Fatal(err) + } + if err := EncryptSecretsFile(dir); err != nil { + t.Fatal(err) + } + first := readYAML(t, dir, "secrets.encrypted.yaml") + + // Change top.sub.inner1 only. + changed := `top: + unchanged: keep-me + sub: + inner1: NEW + inner2: keep-me-too +` + if err := writeYAML(dir, "secrets.yaml", changed); err != nil { + t.Fatal(err) + } + if err := EncryptSecretsFile(dir); err != nil { + t.Fatal(err) + } + second := readYAML(t, dir, "secrets.encrypted.yaml") + + // Walk both trees, compare leaf-by-leaf. + firstTop := first["top"].(map[string]any) + secondTop := second["top"].(map[string]any) + if firstTop["unchanged"] != secondTop["unchanged"] { + t.Errorf("top.unchanged ciphertext rotated unnecessarily") + } + firstSub := firstTop["sub"].(map[string]any) + secondSub := secondTop["sub"].(map[string]any) + if firstSub["inner2"] != secondSub["inner2"] { + t.Errorf("top.sub.inner2 ciphertext rotated unnecessarily") + } + if firstSub["inner1"] == secondSub["inner1"] { + t.Error("top.sub.inner1 ciphertext did NOT rotate after plaintext change") + } +} + +// Contract: when the encrypted-file structure differs from the +// plain-file structure (type changed at a key — scalar -> map, or +// vice versa), the affected branch is fully re-encrypted from +// scratch. The chart's incremental rule degrades gracefully. +func TestContract_IncrementalEncrypt_TypeChangeFallsBackToFullEncrypt(t *testing.T) { + dir := t.TempDir() + // Encrypted file holds scalar at k. + if err := writeYAML(dir, "secrets.yaml", "k: scalar-value\n"); err != nil { + t.Fatal(err) + } + if err := EncryptSecretsFile(dir); err != nil { + t.Fatal(err) + } + + // Plaintext now has k as a map. + if err := writeYAML(dir, "secrets.yaml", "k:\n nested: value\n"); err != nil { + t.Fatal(err) + } + if err := EncryptSecretsFile(dir); err != nil { + t.Fatalf("re-encrypt with type change: %v", err) + } + + out := readYAML(t, dir, "secrets.encrypted.yaml") + kMap, ok := out["k"].(map[string]any) + if !ok { + t.Fatalf("expected k to be map after type change, got %T (%v)", out["k"], out["k"]) + } + nested, ok := kMap["nested"].(string) + if !ok || !strings.HasPrefix(nested, ageEncryptionPrefix) { + t.Errorf("expected k.nested to be an encrypted string, got %v", kMap["nested"]) + } +} + +// Contract: when a list value's length changes, the entire list is +// re-encrypted. Per-element merge requires a stable index mapping, +// which a length change invalidates. +func TestContract_IncrementalEncrypt_ListLengthChangeFullReencrypt(t *testing.T) { + dir := t.TempDir() + if err := writeYAML(dir, "secrets.yaml", "items:\n - one\n - two\n"); err != nil { + t.Fatal(err) + } + if err := EncryptSecretsFile(dir); err != nil { + t.Fatal(err) + } + first := readYAML(t, dir, "secrets.encrypted.yaml") + + if err := writeYAML(dir, "secrets.yaml", "items:\n - one\n - two\n - three\n"); err != nil { + t.Fatal(err) + } + if err := EncryptSecretsFile(dir); err != nil { + t.Fatal(err) + } + second := readYAML(t, dir, "secrets.encrypted.yaml") + + firstItems := first["items"].([]any) + secondItems := second["items"].([]any) + if len(secondItems) != 3 { + t.Fatalf("expected 3 items after append, got %d", len(secondItems)) + } + // Existing items[0]/items[1] are EXPECTED to rotate ciphertext — + // the list-length-changed branch re-encrypts the full slice. + // Pin both that the new length holds AND that all entries are + // freshly encrypted (envelope-wrapped). + for i, e := range secondItems { + s, ok := e.(string) + if !ok || !strings.HasPrefix(s, ageEncryptionPrefix) { + t.Errorf("items[%d] not encrypted: %v", i, e) + } + } + _ = firstItems // referenced here to document the intentional non-comparison +} + +// Contract: when an existing list element changes value but the +// length stays the same, only that element re-encrypts; siblings +// stay byte-stable. +func TestContract_IncrementalEncrypt_ListSameLengthLocalised(t *testing.T) { + dir := t.TempDir() + if err := writeYAML(dir, "secrets.yaml", "items:\n - alpha\n - bravo\n - charlie\n"); err != nil { + t.Fatal(err) + } + if err := EncryptSecretsFile(dir); err != nil { + t.Fatal(err) + } + first := readYAML(t, dir, "secrets.encrypted.yaml") + + if err := writeYAML(dir, "secrets.yaml", "items:\n - alpha\n - DELTA\n - charlie\n"); err != nil { + t.Fatal(err) + } + if err := EncryptSecretsFile(dir); err != nil { + t.Fatal(err) + } + second := readYAML(t, dir, "secrets.encrypted.yaml") + + firstItems := first["items"].([]any) + secondItems := second["items"].([]any) + if firstItems[0] != secondItems[0] { + t.Errorf("items[0] rotated unnecessarily") + } + if firstItems[2] != secondItems[2] { + t.Errorf("items[2] rotated unnecessarily") + } + if firstItems[1] == secondItems[1] { + t.Errorf("items[1] did NOT rotate after value change") + } +} + +// === helpers === + +func writeYAML(dir, name, body string) error { + return os.WriteFile(filepath.Join(dir, name), []byte(body), 0o600) +} + +func readYAML(t *testing.T, dir, name string) map[string]any { + t.Helper() + data, err := os.ReadFile(filepath.Join(dir, name)) + if err != nil { + t.Fatalf("read %s: %v", name, err) + } + var out map[string]any + if err := yaml.Unmarshal(data, &out); err != nil { + t.Fatalf("unmarshal %s: %v", name, err) + } + return out +} diff --git a/pkg/age/contract_test.go b/pkg/age/contract_test.go new file mode 100644 index 00000000..d7124c58 --- /dev/null +++ b/pkg/age/contract_test.go @@ -0,0 +1,534 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: age key management and YAML-value encryption for talm. +// pkg/age implements `talm init --encrypt` / `--decrypt` flows: it +// generates an age X25519 keypair, persists it as `talm.key` (mode +// 0600 via secureperm), encrypts/decrypts secrets.yaml round-trip, +// and supports key rotation. Encryption is per-string-value (keys +// stay readable, values become `ENC[AGE,data:]`) so an +// encrypted secrets file remains diffable in git. +// +// Tests in this file pin user-observable contracts: file format +// (talm.key layout, ENC[...] envelope), round-trip integrity, +// incremental re-encryption (unchanged values stay byte-stable +// between runs — important for git history), key rotation +// preserving plaintext, and the load-or-generate idempotency that +// `talm init` relies on. + +package age_test + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/cozystack/talm/pkg/age" + "gopkg.in/yaml.v3" +) + +// === GenerateKey + LoadKey === + +// Contract: GenerateKey on an empty directory creates a fresh +// age X25519 identity AND writes talm.key with the canonical age +// keygen layout: a `# created:` comment, a `# public key:` comment, +// and the AGE-SECRET-KEY-1... line. +func TestContract_Age_GenerateKey_FileLayout(t *testing.T) { + dir := t.TempDir() + id, created, err := age.GenerateKey(dir) + if err != nil { + t.Fatalf("GenerateKey: %v", err) + } + if !created { + t.Fatal("expected created=true on empty dir") + } + if id == nil { + t.Fatal("nil identity") + } + + keyFile := filepath.Join(dir, "talm.key") + data, err := os.ReadFile(keyFile) + if err != nil { + t.Fatalf("read talm.key: %v", err) + } + content := string(data) + for _, want := range []string{ + "# created: ", + "# public key: ", + "AGE-SECRET-KEY-1", + } { + if !strings.Contains(content, want) { + t.Errorf("talm.key missing %q in:\n%s", want, content) + } + } +} + +// Contract: GenerateKey is idempotent — a second call against the +// same directory returns created=false and the SAME identity (same +// public key). This is the load-or-generate semantics talm relies on +// across `init`, `apply`, and `talosconfig` flows. +func TestContract_Age_GenerateKey_IdempotentReturnsSameIdentity(t *testing.T) { + dir := t.TempDir() + first, _, err := age.GenerateKey(dir) + if err != nil { + t.Fatalf("first GenerateKey: %v", err) + } + second, created, err := age.GenerateKey(dir) + if err != nil { + t.Fatalf("second GenerateKey: %v", err) + } + if created { + t.Error("expected created=false on second call") + } + if first.Recipient().String() != second.Recipient().String() { + t.Errorf("public keys differ across calls:\nfirst: %s\nsecond: %s", + first.Recipient(), second.Recipient()) + } +} + +// Contract: LoadKey reads talm.key in the canonical layout (with +// comments) and returns the identity. The function picks the line +// starting with AGE-SECRET-KEY- regardless of where it sits in the +// file (works with both age keygen and an old plain-key format). +func TestContract_Age_LoadKey_AcceptsKeygenFormat(t *testing.T) { + dir := t.TempDir() + first, _, err := age.GenerateKey(dir) + if err != nil { + t.Fatal(err) + } + loaded, err := age.LoadKey(dir) + if err != nil { + t.Fatalf("LoadKey: %v", err) + } + if first.Recipient().String() != loaded.Recipient().String() { + t.Errorf("public keys differ\ngenerated: %s\nloaded: %s", first.Recipient(), loaded.Recipient()) + } +} + +// Contract: LoadKey accepts a legacy plain-text talm.key (just the +// AGE-SECRET-KEY-... line, no comments). Backward-compat: pre-1.0 +// projects predate the keygen-format introduction. +func TestContract_Age_LoadKey_AcceptsPlainFormat(t *testing.T) { + dir := t.TempDir() + plainFile := filepath.Join(dir, "talm.key") + // Generate one to extract the secret-key line. + id, _, err := age.GenerateKey(dir) + if err != nil { + t.Fatal(err) + } + plainSecret := id.String() + "\n" + // Overwrite with plain format (no comments). + if err := os.WriteFile(plainFile, []byte(plainSecret), 0o600); err != nil { + t.Fatal(err) + } + loaded, err := age.LoadKey(dir) + if err != nil { + t.Fatalf("LoadKey on plain format: %v", err) + } + if id.Recipient().String() != loaded.Recipient().String() { + t.Errorf("public keys differ on plain reload") + } +} + +// Contract: LoadKey errors precisely when talm.key has no +// AGE-SECRET-KEY-... line at all (random garbage, partial file, +// edited-by-mistake state). The error tells the operator the file +// is malformed without exposing key material. +func TestContract_Age_LoadKey_RejectsMalformedKeyFile(t *testing.T) { + dir := t.TempDir() + malformed := filepath.Join(dir, "talm.key") + if err := os.WriteFile(malformed, []byte("# this is not a key\nrandom garbage\n"), 0o600); err != nil { + t.Fatal(err) + } + _, err := age.LoadKey(dir) + if err == nil { + t.Fatal("expected error for malformed key file") + } + if !strings.Contains(err.Error(), "AGE-SECRET-KEY") { + t.Errorf("error must reference the missing AGE-SECRET-KEY marker, got: %v", err) + } +} + +// Contract: LoadKey errors when talm.key is missing entirely. The +// caller (talm init / apply) needs this to differentiate "no key +// yet, generate one" from "key was deleted, abort and warn". +func TestContract_Age_LoadKey_MissingFileErrors(t *testing.T) { + dir := t.TempDir() // no talm.key inside + _, err := age.LoadKey(dir) + if err == nil { + t.Fatal("expected error for missing talm.key") + } +} + +// === GetPublicKey / GetPublicKeyFromFile === + +// Contract: GetPublicKey returns the recipient string from an +// identity (matches the AGE-PUBLIC-... format used elsewhere in +// the age toolchain). +func TestContract_Age_GetPublicKey_FromIdentity(t *testing.T) { + dir := t.TempDir() + id, _, err := age.GenerateKey(dir) + if err != nil { + t.Fatal(err) + } + pub := age.GetPublicKey(id) + if !strings.HasPrefix(pub, "age1") { + t.Errorf("expected age public key to start with 'age1', got %q", pub) + } +} + +// Contract: GetPublicKeyFromFile reads talm.key and returns the +// public key. Prefers the `# public key:` comment line (fast path, +// no key parsing); falls back to LoadKey when the comment is +// missing. +func TestContract_Age_GetPublicKeyFromFile_PrefersComment(t *testing.T) { + dir := t.TempDir() + id, _, err := age.GenerateKey(dir) + if err != nil { + t.Fatal(err) + } + got, err := age.GetPublicKeyFromFile(dir) + if err != nil { + t.Fatalf("GetPublicKeyFromFile: %v", err) + } + if got != id.Recipient().String() { + t.Errorf("public key mismatch\n got: %s\nwant: %s", got, id.Recipient()) + } +} + +// Contract: GetPublicKeyFromFile recovers via LoadKey when the +// `# public key:` comment is absent (legacy plain format). No +// silent failure — the function returns the same value either way. +func TestContract_Age_GetPublicKeyFromFile_FallsBackToLoadKey(t *testing.T) { + dir := t.TempDir() + id, _, err := age.GenerateKey(dir) + if err != nil { + t.Fatal(err) + } + // Strip the comment lines. + plain := id.String() + "\n" + if err := os.WriteFile(filepath.Join(dir, "talm.key"), []byte(plain), 0o600); err != nil { + t.Fatal(err) + } + got, err := age.GetPublicKeyFromFile(dir) + if err != nil { + t.Fatalf("GetPublicKeyFromFile (no comment): %v", err) + } + if got != id.Recipient().String() { + t.Errorf("fallback public key mismatch") + } +} + +// === EncryptSecretsFile / DecryptSecretsFile === + +// Contract: round-trip stability — encrypt then decrypt restores +// the original plaintext exactly. This is the basic correctness +// requirement of any encryption layer. +func TestContract_Age_SecretsFile_RoundTrip(t *testing.T) { + dir := t.TempDir() + plain := []byte(`secrets: + api_token: super-secret-1 + db_password: another-secret +nested: + k1: + k2: deeply-nested-value +`) + plainFile := filepath.Join(dir, "secrets.yaml") + if err := os.WriteFile(plainFile, plain, 0o600); err != nil { + t.Fatal(err) + } + if err := age.EncryptSecretsFile(dir); err != nil { + t.Fatalf("Encrypt: %v", err) + } + // Remove plaintext to prove decrypt restores from encrypted file. + if err := os.Remove(plainFile); err != nil { + t.Fatal(err) + } + if err := age.DecryptSecretsFile(dir); err != nil { + t.Fatalf("Decrypt: %v", err) + } + got, err := os.ReadFile(plainFile) + if err != nil { + t.Fatal(err) + } + // Compare semantically — YAML round-trip may reorder keys. + var origMap, gotMap map[string]any + if err := yaml.Unmarshal(plain, &origMap); err != nil { + t.Fatal(err) + } + if err := yaml.Unmarshal(got, &gotMap); err != nil { + t.Fatal(err) + } + if !mapsEqual(origMap, gotMap) { + t.Errorf("round-trip mismatch\norig:\n%s\ngot:\n%s", plain, got) + } +} + +// Contract: encrypted file uses the `ENC[AGE,data:]` +// envelope per string value. Keys remain plaintext — this is what +// makes the encrypted file diffable in git: changing one secret +// produces a one-line diff, not a wholesale ciphertext rewrite. +func TestContract_Age_SecretsFile_EnvelopeFormat(t *testing.T) { + dir := t.TempDir() + plain := []byte("secret_value: hello-world\n") + if err := os.WriteFile(filepath.Join(dir, "secrets.yaml"), plain, 0o600); err != nil { + t.Fatal(err) + } + if err := age.EncryptSecretsFile(dir); err != nil { + t.Fatal(err) + } + encrypted, err := os.ReadFile(filepath.Join(dir, "secrets.encrypted.yaml")) + if err != nil { + t.Fatal(err) + } + encStr := string(encrypted) + // Key stays plaintext. + if !strings.Contains(encStr, "secret_value:") { + t.Errorf("expected key 'secret_value:' to remain plaintext, got:\n%s", encStr) + } + // Value is wrapped in the envelope. + if !strings.Contains(encStr, "ENC[AGE,data:") { + t.Errorf("expected ENC[AGE,data: envelope, got:\n%s", encStr) + } + if !strings.Contains(encStr, "]") { + t.Errorf("expected ENC envelope closing ], got:\n%s", encStr) + } + // Plaintext value MUST NOT appear. + if strings.Contains(encStr, "hello-world") { + t.Errorf("plaintext leaked in encrypted output:\n%s", encStr) + } +} + +// Contract: incremental re-encryption — when secrets.yaml has not +// changed, calling EncryptSecretsFile twice produces the SAME +// encrypted file bytes. This makes an "encrypt-on-save" workflow +// safe under git: an untouched secret stays as the same ciphertext, +// so commits show only intended changes. +func TestContract_Age_SecretsFile_IncrementalReencryption(t *testing.T) { + dir := t.TempDir() + plain := []byte("a: alpha\nb: bravo\n") + if err := os.WriteFile(filepath.Join(dir, "secrets.yaml"), plain, 0o600); err != nil { + t.Fatal(err) + } + if err := age.EncryptSecretsFile(dir); err != nil { + t.Fatal(err) + } + first, err := os.ReadFile(filepath.Join(dir, "secrets.encrypted.yaml")) + if err != nil { + t.Fatal(err) + } + if err := age.EncryptSecretsFile(dir); err != nil { + t.Fatal(err) + } + second, err := os.ReadFile(filepath.Join(dir, "secrets.encrypted.yaml")) + if err != nil { + t.Fatal(err) + } + if string(first) != string(second) { + t.Errorf("re-encrypt with unchanged plaintext produced different ciphertext\nfirst:\n%s\nsecond:\n%s", first, second) + } +} + +// Contract: changing one value produces a localized diff — only +// that key's ciphertext changes; the others stay byte-stable. +// Pinning this prevents a regression that ever-rotates the IV/nonce +// for unchanged values (the latter would defeat the point of +// per-value encryption). +func TestContract_Age_SecretsFile_ChangedValueLocalizedDiff(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "secrets.yaml"), []byte("a: alpha\nb: bravo\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := age.EncryptSecretsFile(dir); err != nil { + t.Fatal(err) + } + first, err := os.ReadFile(filepath.Join(dir, "secrets.encrypted.yaml")) + if err != nil { + t.Fatal(err) + } + + // Change b's value, leave a alone. + if err := os.WriteFile(filepath.Join(dir, "secrets.yaml"), []byte("a: alpha\nb: charlie\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := age.EncryptSecretsFile(dir); err != nil { + t.Fatal(err) + } + second, err := os.ReadFile(filepath.Join(dir, "secrets.encrypted.yaml")) + if err != nil { + t.Fatal(err) + } + + if string(first) == string(second) { + t.Fatal("expected ciphertext to change after b's plaintext changed") + } + + // Extract a's encrypted line from each — they must match (a was unchanged). + aFirst := lineWithPrefix(string(first), "a: ENC[") + aSecond := lineWithPrefix(string(second), "a: ENC[") + if aFirst == "" || aSecond == "" { + t.Fatalf("could not isolate a's ciphertext line\nfirst:\n%s\nsecond:\n%s", first, second) + } + if aFirst != aSecond { + t.Errorf("a's ciphertext rotated unnecessarily\n first: %s\n second: %s", aFirst, aSecond) + } +} + +// === RotateKeys === + +// Contract: RotateKeys preserves plaintext round-trip after the +// call: whatever key happens to be on disk afterwards is sufficient +// to decrypt the encrypted file. This is the minimum integrity +// guarantee operators rely on. +// +// KNOWN BUG (not pinned, on purpose): RotateKeys at age.go:485 does +// NOT actually replace talm.key. It calls GenerateKey, which is a +// load-or-create operation: if talm.key already exists, +// GenerateKey loads and returns it instead of generating a fresh +// identity. So today RotateKeys re-encrypts the secrets file with +// the SAME key. The test does not assert "public key changes" — if +// it did, this would fail today, and pinning a passing assertion +// would lock in the bug. Track separately and fix in a dedicated +// commit. +func TestContract_Age_RotateKeys_PreservesPlaintextRoundTrip(t *testing.T) { + dir := t.TempDir() + plain := []byte("secret: rotate-me\n") + if err := os.WriteFile(filepath.Join(dir, "secrets.yaml"), plain, 0o600); err != nil { + t.Fatal(err) + } + if err := age.EncryptSecretsFile(dir); err != nil { + t.Fatal(err) + } + + if err := age.RotateKeys(dir); err != nil { + t.Fatalf("RotateKeys: %v", err) + } + + // Decrypt with whatever key is on disk now — plaintext must round-trip. + if err := os.Remove(filepath.Join(dir, "secrets.yaml")); err != nil { + t.Fatal(err) + } + if err := age.DecryptSecretsFile(dir); err != nil { + t.Fatalf("Decrypt after rotation: %v", err) + } + got, err := os.ReadFile(filepath.Join(dir, "secrets.yaml")) + if err != nil { + t.Fatal(err) + } + var orig, after map[string]any + if err := yaml.Unmarshal(plain, &orig); err != nil { + t.Fatal(err) + } + if err := yaml.Unmarshal(got, &after); err != nil { + t.Fatal(err) + } + if !mapsEqual(orig, after) { + t.Errorf("plaintext changed across rotation\norig: %v\nafter: %v", orig, after) + } +} + +// === EncryptYAMLFile / DecryptYAMLFile === + +// Contract: the generic file-pair encrypt/decrypt accepts arbitrary +// plain / encrypted file names (used for kubeconfig and other +// non-secrets.yaml files). Round-trip semantics identical to the +// secrets.yaml path. +func TestContract_Age_GenericYAMLFile_RoundTrip(t *testing.T) { + dir := t.TempDir() + plain := []byte("kubeconfig:\n server: https://api.example.com:6443\n token: abc123\n") + plainName := "kubeconfig.yaml" + encName := "kubeconfig.encrypted.yaml" + if err := os.WriteFile(filepath.Join(dir, plainName), plain, 0o600); err != nil { + t.Fatal(err) + } + if err := age.EncryptYAMLFile(dir, plainName, encName); err != nil { + t.Fatalf("EncryptYAMLFile: %v", err) + } + if err := os.Remove(filepath.Join(dir, plainName)); err != nil { + t.Fatal(err) + } + if err := age.DecryptYAMLFile(dir, encName, plainName); err != nil { + t.Fatalf("DecryptYAMLFile: %v", err) + } + got, err := os.ReadFile(filepath.Join(dir, plainName)) + if err != nil { + t.Fatal(err) + } + var origMap, gotMap map[string]any + if err := yaml.Unmarshal(plain, &origMap); err != nil { + t.Fatal(err) + } + if err := yaml.Unmarshal(got, &gotMap); err != nil { + t.Fatal(err) + } + if !mapsEqual(origMap, gotMap) { + t.Errorf("round-trip mismatch\norig:\n%s\ngot:\n%s", plain, got) + } +} + +// === helpers === + +// mapsEqual is a tiny structural comparison sufficient for YAML- +// derived map[string]any values used in these tests. Keeps the +// dependency surface minimal — no reflect.DeepEqual that would also +// pick up irrelevant struct-vs-map representation differences. +func mapsEqual(a, b map[string]any) bool { + if len(a) != len(b) { + return false + } + for k, av := range a { + bv, ok := b[k] + if !ok { + return false + } + if !valuesEqual(av, bv) { + return false + } + } + return true +} + +func valuesEqual(a, b any) bool { + switch av := a.(type) { + case map[string]any: + bv, ok := b.(map[string]any) + if !ok { + return false + } + return mapsEqual(av, bv) + case []any: + bv, ok := b.([]any) + if !ok || len(av) != len(bv) { + return false + } + for i := range av { + if !valuesEqual(av[i], bv[i]) { + return false + } + } + return true + default: + return a == b + } +} + +func lineWithPrefix(content, prefix string) string { + for line := range strings.SplitSeq(content, "\n") { + if strings.HasPrefix(line, prefix) { + return line + } + } + return "" +} diff --git a/pkg/commands/contract_chart_gitignore_test.go b/pkg/commands/contract_chart_gitignore_test.go new file mode 100644 index 00000000..54ed8e96 --- /dev/null +++ b/pkg/commands/contract_chart_gitignore_test.go @@ -0,0 +1,308 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: readChartYamlPreset (Chart.yaml -> preset name) and +// writeGitignoreFile (.gitignore management of secrets-bearing +// files). Both are user-facing: an operator running `talm init` +// against an existing project sees the preset detection result +// echoed in command behaviour, and they expect the secrets files +// the chart will write to be added to .gitignore so an accidental +// `git add .` does not commit private keys. + +package commands + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// setRoot temporarily replaces Config.RootDir for the duration of a +// test, restoring the previous value on cleanup. The package-level +// Config is shared state — tests must not leak it. +func setRoot(t *testing.T, dir string) { + t.Helper() + original := Config.RootDir + t.Cleanup(func() { Config.RootDir = original }) + Config.RootDir = dir +} + +// === readChartYamlPreset === + +// Contract: Chart.yaml's first non-talm dependency name is the +// preset. talm itself is the library chart (always present); the +// "active preset" is whichever other dependency the project chose +// at `talm init -p `. +func TestContract_ReadChartYamlPreset_PicksFirstNonTalmDependency(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + chartYaml := `apiVersion: v2 +name: my-cluster +version: 0.1.0 +dependencies: + - name: talm + version: ">=0" + - name: cozystack + version: ">=0" +` + if err := os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte(chartYaml), 0o644); err != nil { + t.Fatal(err) + } + got, err := readChartYamlPreset() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "cozystack" { + t.Errorf("expected preset 'cozystack', got %q", got) + } +} + +// Contract: order of dependencies in Chart.yaml decides which +// preset is reported. The first non-talm entry wins. Pin so a +// future refactor that returned the LAST entry would surface here. +func TestContract_ReadChartYamlPreset_OrderMatters(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + chartYaml := `apiVersion: v2 +name: my-cluster +version: 0.1.0 +dependencies: + - name: generic + version: ">=0" + - name: talm + version: ">=0" + - name: cozystack + version: ">=0" +` + if err := os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte(chartYaml), 0o644); err != nil { + t.Fatal(err) + } + got, err := readChartYamlPreset() + if err != nil { + t.Fatal(err) + } + if got != "generic" { + t.Errorf("expected first non-talm dep 'generic', got %q", got) + } +} + +// Contract: a Chart.yaml with only the talm library dependency (no +// preset) surfaces a precise error. talm init's update flow uses +// this to detect "no preset configured" without mistaking talm +// itself for a preset. +func TestContract_ReadChartYamlPreset_NoPresetError(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + chartYaml := `apiVersion: v2 +name: my-cluster +version: 0.1.0 +dependencies: + - name: talm + version: ">=0" +` + if err := os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte(chartYaml), 0o644); err != nil { + t.Fatal(err) + } + _, err := readChartYamlPreset() + if err == nil { + t.Fatal("expected error for talm-only deps") + } + if !strings.Contains(err.Error(), "preset not found") { + t.Errorf("error must mention 'preset not found', got: %v", err) + } +} + +// Contract: missing Chart.yaml is an error mentioning the file. +func TestContract_ReadChartYamlPreset_MissingChartYamlError(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + _, err := readChartYamlPreset() + if err == nil { + t.Fatal("expected error for missing Chart.yaml") + } + if !strings.Contains(err.Error(), "Chart.yaml") { + t.Errorf("error must mention Chart.yaml, got: %v", err) + } +} + +// Contract: malformed YAML in Chart.yaml is an error. Without this +// guard, the unmarshal returns nil dependencies, the loop yields +// "preset not found", and the operator sees a misleading message. +func TestContract_ReadChartYamlPreset_MalformedYAMLError(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + if err := os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte(":\n bad:\n: yaml"), 0o644); err != nil { + t.Fatal(err) + } + _, err := readChartYamlPreset() + if err == nil { + t.Fatal("expected error for malformed YAML") + } +} + +// === writeGitignoreFile === + +// Contract: writeGitignoreFile creates .gitignore from scratch +// containing the four secrets-bearing files talm manages: +// secrets.yaml, talosconfig, talm.key, kubeconfig (default name). +// Without this list a fresh `talm init` followed by `git init && +// git add .` would commit private cluster material. +func TestContract_WriteGitignoreFile_CreatesWithRequiredEntries(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + // Force default kubeconfig name (no override). + originalKube := Config.GlobalOptions.Kubeconfig + t.Cleanup(func() { Config.GlobalOptions.Kubeconfig = originalKube }) + Config.GlobalOptions.Kubeconfig = "" + + if err := writeGitignoreFile(); err != nil { + t.Fatalf("writeGitignoreFile: %v", err) + } + data, err := os.ReadFile(filepath.Join(dir, ".gitignore")) + if err != nil { + t.Fatalf("read .gitignore: %v", err) + } + content := string(data) + for _, want := range []string{"secrets.yaml", "talosconfig", "talm.key", "kubeconfig"} { + if !strings.Contains(content, want) { + t.Errorf(".gitignore missing %q in:\n%s", want, content) + } + } +} + +// Contract: when Config.GlobalOptions.Kubeconfig is set to a path +// (e.g. "/etc/kubernetes/admin.kubeconfig"), .gitignore receives +// only the BASE NAME (admin.kubeconfig). The directory portion is +// dropped — paths in .gitignore are repo-relative; absolute paths +// are useless. Pinning this prevents a regression that would write +// the full host path into a project's .gitignore. +func TestContract_WriteGitignoreFile_KubeconfigBaseNameOnly(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + originalKube := Config.GlobalOptions.Kubeconfig + t.Cleanup(func() { Config.GlobalOptions.Kubeconfig = originalKube }) + Config.GlobalOptions.Kubeconfig = "/etc/kubernetes/admin.kubeconfig" + + if err := writeGitignoreFile(); err != nil { + t.Fatalf("writeGitignoreFile: %v", err) + } + data, _ := os.ReadFile(filepath.Join(dir, ".gitignore")) + content := string(data) + if !strings.Contains(content, "admin.kubeconfig") { + t.Errorf("expected base name 'admin.kubeconfig' in .gitignore:\n%s", content) + } + if strings.Contains(content, "/etc/kubernetes") { + t.Errorf("absolute path leaked into .gitignore:\n%s", content) + } +} + +// Contract: an existing .gitignore with extra (operator-supplied) +// entries is preserved. writeGitignoreFile is additive — it appends +// missing required entries, never rewrites the file from scratch +// (which would clobber the operator's customizations). +func TestContract_WriteGitignoreFile_PreservesExistingEntries(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + originalKube := Config.GlobalOptions.Kubeconfig + t.Cleanup(func() { Config.GlobalOptions.Kubeconfig = originalKube }) + Config.GlobalOptions.Kubeconfig = "" + + existing := "# Custom rules\nnotes/\n*.log\n" + if err := os.WriteFile(filepath.Join(dir, ".gitignore"), []byte(existing), 0o644); err != nil { + t.Fatal(err) + } + if err := writeGitignoreFile(); err != nil { + t.Fatal(err) + } + data, _ := os.ReadFile(filepath.Join(dir, ".gitignore")) + content := string(data) + for _, want := range []string{"notes/", "*.log", "secrets.yaml", "talosconfig", "talm.key", "kubeconfig"} { + if !strings.Contains(content, want) { + t.Errorf(".gitignore missing %q after append:\n%s", want, content) + } + } +} + +// Contract: when all required entries are already present, +// writeGitignoreFile is a no-op — does not rewrite the file. This +// keeps mtime / git status stable across repeat `talm init` +// invocations. The behaviour is observable via `git diff` showing +// no changes. +func TestContract_WriteGitignoreFile_IdempotentOnFullList(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + originalKube := Config.GlobalOptions.Kubeconfig + t.Cleanup(func() { Config.GlobalOptions.Kubeconfig = originalKube }) + Config.GlobalOptions.Kubeconfig = "" + + full := `# Sensitive files +secrets.yaml +talosconfig +talm.key +kubeconfig +` + gitignore := filepath.Join(dir, ".gitignore") + if err := os.WriteFile(gitignore, []byte(full), 0o644); err != nil { + t.Fatal(err) + } + infoBefore, _ := os.Stat(gitignore) + + if err := writeGitignoreFile(); err != nil { + t.Fatalf("writeGitignoreFile: %v", err) + } + data, _ := os.ReadFile(gitignore) + if string(data) != full { + t.Errorf("idempotent invocation should not change content\nbefore:\n%s\nafter:\n%s", full, data) + } + infoAfter, _ := os.Stat(gitignore) + if !infoBefore.ModTime().Equal(infoAfter.ModTime()) { + t.Errorf("idempotent invocation must not touch mtime") + } +} + +// Contract: an entry that appears in .gitignore as part of a +// commented-out OR pattern-extended form (e.g. `secrets.yaml # backup`, +// `talosconfig#`) counts as already-present. The match is +// prefix-based on the trimmed line, allowing operators to annotate +// .gitignore entries without triggering duplicate appends. +func TestContract_WriteGitignoreFile_TolerantOfAnnotatedEntries(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + originalKube := Config.GlobalOptions.Kubeconfig + t.Cleanup(func() { Config.GlobalOptions.Kubeconfig = originalKube }) + Config.GlobalOptions.Kubeconfig = "" + + annotated := `# Sensitive files +secrets.yaml # never commit this +talosconfig#TODO move to vault +talm.key +kubeconfig +` + gitignore := filepath.Join(dir, ".gitignore") + if err := os.WriteFile(gitignore, []byte(annotated), 0o644); err != nil { + t.Fatal(err) + } + if err := writeGitignoreFile(); err != nil { + t.Fatal(err) + } + data, _ := os.ReadFile(gitignore) + // The annotated entries must NOT have been duplicated. + if strings.Count(string(data), "secrets.yaml") != 1 { + t.Errorf("annotated 'secrets.yaml' duplicated:\n%s", data) + } + if strings.Count(string(data), "talosconfig") != 1 { + t.Errorf("annotated 'talosconfig' duplicated:\n%s", data) + } +} diff --git a/pkg/commands/contract_helpers_test.go b/pkg/commands/contract_helpers_test.go new file mode 100644 index 00000000..db5b5002 --- /dev/null +++ b/pkg/commands/contract_helpers_test.go @@ -0,0 +1,388 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: pure helpers across the commands package that do not +// require a live Talos client. Endpoint normalisation, kubeconfig +// server-field rewriting, .gitignore single-entry append, +// no-confirmation-needed file write paths, and Chart.yaml top-level +// name extraction. All user-observable through `talm` CLI flows. + +package commands + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "k8s.io/client-go/tools/clientcmd" + clientcmdapi "k8s.io/client-go/tools/clientcmd/api" +) + +// === normalizeEndpoint === + +// Contract: every endpoint variant collapses to a canonical +// `https://:6443` form. This is the URL `talm talosctl` and +// `talm` apply paths use to talk to the apiserver. Any port that the +// user passes (e.g. the maintenance-mode 50000) is dropped — the +// kubelet/kube-proxy port is hardcoded to 6443. +func TestContract_NormalizeEndpoint(t *testing.T) { + cases := []struct { + in, want string + }{ + {"1.2.3.4", "https://1.2.3.4:6443"}, + {"1.2.3.4:50000", "https://1.2.3.4:6443"}, + {"https://1.2.3.4:50000", "https://1.2.3.4:6443"}, + {"http://1.2.3.4", "https://1.2.3.4:6443"}, + {"https://node.example.com:6443", "https://node.example.com:6443"}, + {"node.example.com", "https://node.example.com:6443"}, + // IPv6 with brackets — net.SplitHostPort returns the bracket- + // stripped host, and the current normalizeEndpoint does NOT + // re-add them, producing a malformed URL. Pinning the broken + // output deliberately so a later fix surfaces here. Tracked + // in issue #155 — when normalizeEndpoint switches to + // net.JoinHostPort, the expected value below becomes + // "https://[2001:db8::1]:6443" and the test starts asserting + // the canonical form. + // FIXME(#155): expected URL is malformed (missing brackets). + {"[2001:db8::1]:6443", "https://2001:db8::1:6443"}, + } + for _, tc := range cases { + t.Run(tc.in, func(t *testing.T) { + got := normalizeEndpoint(tc.in) + if got != tc.want { + t.Errorf("normalizeEndpoint(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +// === updateKubeconfigServer === + +// Contract: every cluster's `server:` field in the kubeconfig is +// rewritten to the normalised endpoint. Multi-cluster kubeconfigs +// (talm-managed plus other clusters the operator has) are touched +// indiscriminately — the function does NOT scope by current-context; +// callers needing per-context rewrite must pass a kubeconfig with +// only the relevant cluster. +func TestContract_UpdateKubeconfigServer_RewritesAllClusters(t *testing.T) { + dir := t.TempDir() + kcPath := filepath.Join(dir, "kubeconfig") + + cfg := clientcmdapi.NewConfig() + cfg.Clusters["one"] = &clientcmdapi.Cluster{Server: "https://1.2.3.4:6443"} + cfg.Clusters["two"] = &clientcmdapi.Cluster{Server: "https://5.6.7.8:6443"} + if err := clientcmd.WriteToFile(*cfg, kcPath); err != nil { + t.Fatal(err) + } + + if err := updateKubeconfigServer(kcPath, "10.0.0.1:50000"); err != nil { + t.Fatalf("updateKubeconfigServer: %v", err) + } + + got, err := clientcmd.LoadFromFile(kcPath) + if err != nil { + t.Fatal(err) + } + for name, c := range got.Clusters { + if c.Server != "https://10.0.0.1:6443" { + t.Errorf("cluster %q server = %q, want https://10.0.0.1:6443", name, c.Server) + } + } +} + +// Contract: when every cluster already points at the normalised +// endpoint, the function is a no-op (does not rewrite the file). +// Pinning prevents a future change that always rewrites and bumps +// mtime/git status spuriously. +func TestContract_UpdateKubeconfigServer_NoChangeWhenAlreadyNormalised(t *testing.T) { + dir := t.TempDir() + kcPath := filepath.Join(dir, "kubeconfig") + + cfg := clientcmdapi.NewConfig() + cfg.Clusters["one"] = &clientcmdapi.Cluster{Server: "https://10.0.0.1:6443"} + if err := clientcmd.WriteToFile(*cfg, kcPath); err != nil { + t.Fatal(err) + } + infoBefore, _ := os.Stat(kcPath) + + if err := updateKubeconfigServer(kcPath, "10.0.0.1"); err != nil { + t.Fatal(err) + } + infoAfter, _ := os.Stat(kcPath) + if !infoBefore.ModTime().Equal(infoAfter.ModTime()) { + t.Errorf("expected no rewrite for already-normalised kubeconfig; mtime changed") + } +} + +// Contract: missing kubeconfig surfaces a precise error. +func TestContract_UpdateKubeconfigServer_MissingFileError(t *testing.T) { + missing := filepath.Join(t.TempDir(), "no-such-kubeconfig") + err := updateKubeconfigServer(missing, "1.2.3.4") + if err == nil { + t.Fatal("expected error for missing kubeconfig") + } +} + +// === addToGitignore === + +// Contract: addToGitignore appends a single entry to .gitignore, +// creating the file if absent. Idempotent: a second call with the +// same entry leaves the file unchanged. +func TestContract_AddToGitignore_CreatesAndAppends(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + + if err := addToGitignore("artifacts/"); err != nil { + t.Fatalf("addToGitignore: %v", err) + } + gitignore := filepath.Join(dir, ".gitignore") + got, err := os.ReadFile(gitignore) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(got), "artifacts/") { + t.Errorf("entry missing in .gitignore:\n%s", got) + } + + // Second call should be idempotent (file unchanged byte-for-byte). + if err := addToGitignore("artifacts/"); err != nil { + t.Fatal(err) + } + again, _ := os.ReadFile(gitignore) + if string(got) != string(again) { + t.Errorf("idempotent call rewrote file:\nbefore:\n%s\nafter:\n%s", got, again) + } +} + +// Contract: when .gitignore already exists with unrelated entries, +// the new entry is appended on its own line (and the existing +// content is preserved verbatim). +func TestContract_AddToGitignore_PreservesExisting(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + + gitignore := filepath.Join(dir, ".gitignore") + if err := os.WriteFile(gitignore, []byte("# Sensitive\nsecrets.yaml\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := addToGitignore("artifacts/"); err != nil { + t.Fatal(err) + } + got, _ := os.ReadFile(gitignore) + for _, want := range []string{"# Sensitive", "secrets.yaml", "artifacts/"} { + if !strings.Contains(string(got), want) { + t.Errorf("expected %q in:\n%s", want, got) + } + } +} + +// Contract: an entry that appears as a path-prefix +// (e.g. existing `dist` matches a request to add `dist/`) is treated +// as already-present. The match is `entry+"/"`-aware, so `dist` +// covers the request `dist`. Pin so a refactor that uses pure equality +// surfaces here. +func TestContract_AddToGitignore_PathPrefixMatch(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + gitignore := filepath.Join(dir, ".gitignore") + if err := os.WriteFile(gitignore, []byte("dist\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := addToGitignore("dist"); err != nil { + t.Fatal(err) + } + got, _ := os.ReadFile(gitignore) + if strings.Count(string(got), "dist") != 1 { + t.Errorf("expected exactly one 'dist' entry, got:\n%s", got) + } +} + +// === getClusterNameFromChart === + +// Contract: getClusterNameFromChart resolves the cluster name with +// `values.yaml: clusterName` taking precedence over `Chart.yaml: name`. +// Distinct from readChartYamlPreset (which reads dependencies): this +// returns the chart's OWN name. Used as a fallback in the talosconfig +// regenerate path so a re-generated talosconfig matches the cluster +// name baked into the rendered chart output. +// +// The resolution order is values.yaml.clusterName -> Chart.yaml.name +// -> "" (fallback). The order matters: an operator who overrides +// clusterName via values.yaml expects the regenerated talosconfig +// context to use that override, not the chart-directory name. + +// Contract: when only Chart.yaml exists (no values.yaml), the +// function returns Chart.yaml.name. This is the legacy behaviour +// preserved for projects that have not yet adopted the values.yaml +// override. +func TestContract_GetClusterNameFromChart_ReadsTopLevelName(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + yaml := "apiVersion: v2\nname: my-cluster\nversion: 0.1.0\n" + if err := os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte(yaml), 0o644); err != nil { + t.Fatal(err) + } + got := getClusterNameFromChart() + if got != "my-cluster" { + t.Errorf("expected 'my-cluster', got %q", got) + } +} + +// Contract: when both files exist and values.yaml declares a +// non-empty clusterName, the values.yaml override wins. Pin the +// priority so a regression that re-orders the lookup surfaces here. +func TestContract_GetClusterNameFromChart_ValuesYamlOverridesChartYaml(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + if err := os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte("apiVersion: v2\nname: chart-name-loser\nversion: 0.1.0\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "values.yaml"), []byte("clusterName: values-name-winner\n"), 0o644); err != nil { + t.Fatal(err) + } + got := getClusterNameFromChart() + if got != "values-name-winner" { + t.Errorf("expected values.yaml clusterName to win, got %q", got) + } +} + +// Contract: when values.yaml exists but clusterName is empty (the +// shipped default in cozystack/generic charts), the function falls +// through to Chart.yaml.name. Pinning so the empty-string short +// circuit is preserved — without it, every fresh install with the +// default values.yaml would resolve to "" and downstream callers +// would silently substitute their own placeholder. +func TestContract_GetClusterNameFromChart_EmptyValuesClusterNameFallsBack(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + if err := os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte("apiVersion: v2\nname: chart-fallback\nversion: 0.1.0\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "values.yaml"), []byte("clusterName: \"\"\nendpoint: \"\"\n"), 0o644); err != nil { + t.Fatal(err) + } + got := getClusterNameFromChart() + if got != "chart-fallback" { + t.Errorf("expected fallback to Chart.yaml name, got %q", got) + } +} + +// Contract: when values.yaml has no clusterName key at all (any +// shape that does not declare it), the function falls through to +// Chart.yaml.name. The yaml.Unmarshal into the typed struct returns +// the zero string for the missing field, which must be treated the +// same as an explicit empty value. +func TestContract_GetClusterNameFromChart_AbsentValuesKeyFallsBack(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + if err := os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte("apiVersion: v2\nname: chart-fallback\nversion: 0.1.0\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "values.yaml"), []byte("endpoint: \"https://example.com:6443\"\n"), 0o644); err != nil { + t.Fatal(err) + } + got := getClusterNameFromChart() + if got != "chart-fallback" { + t.Errorf("expected fallback to Chart.yaml name, got %q", got) + } +} + +// Contract: a malformed values.yaml does not poison the lookup — +// the function silently moves on to Chart.yaml.name. Treating a +// values.yaml syntax error as "no override" means the regenerate +// path stays usable even when the operator has a half-edited +// values.yaml on disk. +func TestContract_GetClusterNameFromChart_MalformedValuesFallsBack(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + if err := os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte("apiVersion: v2\nname: chart-fallback\nversion: 0.1.0\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "values.yaml"), []byte(":bad: yaml :"), 0o644); err != nil { + t.Fatal(err) + } + got := getClusterNameFromChart() + if got != "chart-fallback" { + t.Errorf("expected fallback on malformed values.yaml, got %q", got) + } +} + +// Contract: missing Chart.yaml AND missing values.yaml returns +// empty string (NOT an error). Callers chain with a fallback default +// — silent empty is the signal that no chart context exists. +func TestContract_GetClusterNameFromChart_MissingReturnsEmpty(t *testing.T) { + setRoot(t, t.TempDir()) + if got := getClusterNameFromChart(); got != "" { + t.Errorf("expected empty string for missing Chart.yaml, got %q", got) + } +} + +// Contract: malformed Chart.yaml returns empty string (when +// values.yaml does not provide an override). Same silent fallback +// as missing — the caller does not need to distinguish. +func TestContract_GetClusterNameFromChart_MalformedReturnsEmpty(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + if err := os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte(":bad: yaml :"), 0o644); err != nil { + t.Fatal(err) + } + if got := getClusterNameFromChart(); got != "" { + t.Errorf("expected empty string for malformed YAML, got %q", got) + } +} + +// === updateFileWithConfirmation: no-prompt happy paths === + +// Contract: when the target file does NOT exist, the function +// creates it (and any missing parent directories) without prompting. +// The intended `talm init` flow lays down many files at once; each +// new path is materialised silently. +func TestContract_UpdateFileWithConfirmation_CreatesNew(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + target := filepath.Join(dir, "nested", "deep", "file.txt") + + if err := updateFileWithConfirmation(target, []byte("hello"), 0o644); err != nil { + t.Fatalf("updateFileWithConfirmation: %v", err) + } + got, err := os.ReadFile(target) + if err != nil { + t.Fatalf("file not created: %v", err) + } + if string(got) != "hello" { + t.Errorf("content mismatch: got %q", got) + } +} + +// Contract: when the target file exists with byte-identical +// content, the function is a no-op (does not prompt, does not +// rewrite). Pin so a regression that always touches the file would +// surface as a spurious mtime change. +func TestContract_UpdateFileWithConfirmation_SameContentSkips(t *testing.T) { + dir := t.TempDir() + setRoot(t, dir) + target := filepath.Join(dir, "f") + if err := os.WriteFile(target, []byte("same"), 0o644); err != nil { + t.Fatal(err) + } + infoBefore, _ := os.Stat(target) + if err := updateFileWithConfirmation(target, []byte("same"), 0o644); err != nil { + t.Fatal(err) + } + infoAfter, _ := os.Stat(target) + if !infoBefore.ModTime().Equal(infoAfter.ModTime()) { + t.Errorf("identical content should not touch mtime") + } +} diff --git a/pkg/commands/contract_root_detection_test.go b/pkg/commands/contract_root_detection_test.go new file mode 100644 index 00000000..3607dc26 --- /dev/null +++ b/pkg/commands/contract_root_detection_test.go @@ -0,0 +1,292 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: project-root detection. talm decides which directory is +// "the project" by walking up from a file path, a template path, or +// the CWD until it finds Chart.yaml AND a secrets file (either +// secrets.yaml or secrets.encrypted.yaml). The two-marker rule is +// the contract — Chart.yaml alone matches every helm chart on disk; +// the secrets file is what makes a directory a TALM project. + +package commands + +import ( + "os" + "path/filepath" + "testing" +) + +// makeProjectRoot creates a `Chart.yaml` and `secrets.yaml` (the two +// markers DetectProjectRoot looks for) inside dir. +func makeProjectRoot(t *testing.T, dir string) { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte("name: test\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "secrets.yaml"), []byte("k: v\n"), 0o600); err != nil { + t.Fatal(err) + } +} + +// === DetectProjectRoot === + +// Contract: a directory containing both Chart.yaml AND secrets.yaml +// is a project root. The function returns the absolute path. +func TestContract_DetectProjectRoot_DirectMatch(t *testing.T) { + dir := t.TempDir() + makeProjectRoot(t, dir) + got, err := DetectProjectRoot(dir) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want, _ := filepath.Abs(dir) + if got != want { + t.Errorf("got %q, want %q", got, want) + } +} + +// Contract: secrets.encrypted.yaml is an acceptable substitute for +// secrets.yaml. Operators using `talm init --encrypt` will not have +// secrets.yaml at rest; root detection must still work. +func TestContract_DetectProjectRoot_EncryptedSecretsAccepted(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte("name: test\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "secrets.encrypted.yaml"), []byte("ENC[...]"), 0o600); err != nil { + t.Fatal(err) + } + got, err := DetectProjectRoot(dir) + if err != nil { + t.Fatal(err) + } + if got == "" { + t.Errorf("expected match for dir with secrets.encrypted.yaml, got empty") + } +} + +// Contract: walking up from a sub-directory finds the project root. +// `talm apply -f nodes/cp1.yaml` invoked from anywhere inside the +// project must resolve back to the root. +func TestContract_DetectProjectRoot_WalksUpFromSubdir(t *testing.T) { + root := t.TempDir() + makeProjectRoot(t, root) + subdir := filepath.Join(root, "nodes", "deep", "deeper") + if err := os.MkdirAll(subdir, 0o755); err != nil { + t.Fatal(err) + } + got, err := DetectProjectRoot(subdir) + if err != nil { + t.Fatal(err) + } + wantAbs, _ := filepath.Abs(root) + if got != wantAbs { + t.Errorf("walked up to %q, want %q", got, wantAbs) + } +} + +// Contract: when neither marker is found anywhere up the tree, the +// function returns ("", nil) — empty string, no error. The caller +// then surfaces a precise diagnostic. The function does NOT error +// itself because "no project here" is a valid input state (operator +// running `talm` outside any project). +func TestContract_DetectProjectRoot_NoMatchReturnsEmpty(t *testing.T) { + // We cannot construct a guaranteed-empty walk-up tree on most + // filesystems (the user may have markers in $HOME), so we use a + // temp directory and assert "the function returns either empty or + // some root, but the directory we passed is not it" — and verify + // that with a fully empty start dir, traversal is bounded. + dir := t.TempDir() + got, err := DetectProjectRoot(dir) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + gotAbs := got + dirAbs, _ := filepath.Abs(dir) + if gotAbs == dirAbs { + t.Errorf("started in %q (no markers); should not match itself", dirAbs) + } + // got may be empty (clean test env) or some ancestor of $TMPDIR + // containing markers (developer's $HOME-rooted project). Both are + // fine — what matters is the function does not loop or error. +} + +// Contract: a directory with Chart.yaml ONLY (no secrets file) is +// NOT a project root — many helm charts on disk are not talm +// projects. Pinning the both-markers rule prevents false positives. +func TestContract_DetectProjectRoot_ChartYamlAloneNotEnough(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "Chart.yaml"), []byte("name: test\n"), 0o644); err != nil { + t.Fatal(err) + } + got, err := DetectProjectRoot(dir) + if err != nil { + t.Fatal(err) + } + dirAbs, _ := filepath.Abs(dir) + if got == dirAbs { + t.Errorf("Chart.yaml alone matched as root: %q", got) + } +} + +// === DetectProjectRootForFile === + +// Contract: file-based detection takes a file path, derives the +// containing directory, then runs DetectProjectRoot on it. +func TestContract_DetectProjectRootForFile(t *testing.T) { + root := t.TempDir() + makeProjectRoot(t, root) + nodes := filepath.Join(root, "nodes") + if err := os.Mkdir(nodes, 0o755); err != nil { + t.Fatal(err) + } + nodeFile := filepath.Join(nodes, "cp1.yaml") + if err := os.WriteFile(nodeFile, []byte(""), 0o600); err != nil { + t.Fatal(err) + } + got, err := DetectProjectRootForFile(nodeFile) + if err != nil { + t.Fatal(err) + } + rootAbs, _ := filepath.Abs(root) + if got != rootAbs { + t.Errorf("got %q, want %q", got, rootAbs) + } +} + +// === ValidateAndDetectRootsForFiles === + +// Contract: empty input returns ("", nil). Caller treats this as +// "no files to validate" and falls through to other detection +// strategies. +func TestContract_ValidateAndDetectRootsForFiles_EmptyInput(t *testing.T) { + got, err := ValidateAndDetectRootsForFiles(nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "" { + t.Errorf("expected empty string, got %q", got) + } +} + +// Contract: when all files share the same project root, the +// function returns that root. +func TestContract_ValidateAndDetectRootsForFiles_SingleRoot(t *testing.T) { + root := t.TempDir() + makeProjectRoot(t, root) + nodes := filepath.Join(root, "nodes") + if err := os.Mkdir(nodes, 0o755); err != nil { + t.Fatal(err) + } + files := []string{ + filepath.Join(nodes, "cp1.yaml"), + filepath.Join(nodes, "cp2.yaml"), + filepath.Join(nodes, "cp3.yaml"), + } + for _, f := range files { + if err := os.WriteFile(f, []byte(""), 0o600); err != nil { + t.Fatal(err) + } + } + got, err := ValidateAndDetectRootsForFiles(files) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + rootAbs, _ := filepath.Abs(root) + if got != rootAbs { + t.Errorf("got %q, want %q", got, rootAbs) + } +} + +// Contract: files spanning two project roots is an explicit error. +// The error names both roots so the operator can fix the +// commandline. talm refuses to apply a config built from +// inconsistent inputs (it cannot meaningfully merge two project +// configs in one apply). +func TestContract_ValidateAndDetectRootsForFiles_DifferentRootsError(t *testing.T) { + rootA := t.TempDir() + rootB := t.TempDir() + makeProjectRoot(t, rootA) + makeProjectRoot(t, rootB) + fileA := filepath.Join(rootA, "node-a.yaml") + fileB := filepath.Join(rootB, "node-b.yaml") + if err := os.WriteFile(fileA, []byte(""), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(fileB, []byte(""), 0o600); err != nil { + t.Fatal(err) + } + _, err := ValidateAndDetectRootsForFiles([]string{fileA, fileB}) + if err == nil { + t.Fatal("expected error for files in different roots") + } +} + +// Contract: a file whose directory has no Chart.yaml/secrets up the +// tree at all is reported by name in the error so the operator can +// see WHICH file failed root detection. +func TestContract_ValidateAndDetectRootsForFiles_OrphanFileError(t *testing.T) { + root := t.TempDir() + makeProjectRoot(t, root) + + // Orphan: file outside any project. + orphanDir := t.TempDir() + orphan := filepath.Join(orphanDir, "loose.yaml") + if err := os.WriteFile(orphan, []byte(""), 0o600); err != nil { + t.Fatal(err) + } + rootedFile := filepath.Join(root, "node.yaml") + if err := os.WriteFile(rootedFile, []byte(""), 0o600); err != nil { + t.Fatal(err) + } + + // Note: this test passes only when $TMPDIR's ancestors do NOT + // happen to contain Chart.yaml + secrets.yaml. On a developer + // machine with such markers it would resolve to that. Skip if so. + got, _ := DetectProjectRootForFile(orphan) + if got != "" { + t.Skipf("test environment has project markers above %q; skipping orphan-error path", orphanDir) + } + + _, err := ValidateAndDetectRootsForFiles([]string{rootedFile, orphan}) + if err == nil { + t.Fatal("expected error for orphan file") + } +} + +// === DetectRootForTemplate === + +// Contract: DetectRootForTemplate is a thin alias for +// DetectProjectRootForFile (both apply the file-then-walk-up +// strategy). Pin the equivalence so a future refactor that +// introduces a separate template-only path knows it is changing +// observable behaviour. +func TestContract_DetectRootForTemplate_EquivalentToFile(t *testing.T) { + root := t.TempDir() + makeProjectRoot(t, root) + templatesDir := filepath.Join(root, "templates") + if err := os.Mkdir(templatesDir, 0o755); err != nil { + t.Fatal(err) + } + tmpl := filepath.Join(templatesDir, "controlplane.yaml") + if err := os.WriteFile(tmpl, []byte(""), 0o600); err != nil { + t.Fatal(err) + } + + gotFile, _ := DetectProjectRootForFile(tmpl) + gotTmpl, _ := DetectRootForTemplate(tmpl) + if gotFile != gotTmpl { + t.Errorf("DetectRootForTemplate diverged from DetectProjectRootForFile:\n file: %q\n tmpl: %q", gotFile, gotTmpl) + } +} diff --git a/pkg/commands/contract_root_dispatch_test.go b/pkg/commands/contract_root_dispatch_test.go new file mode 100644 index 00000000..35344e83 --- /dev/null +++ b/pkg/commands/contract_root_dispatch_test.go @@ -0,0 +1,557 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: the dispatch layer above DetectProjectRoot — +// getFlagValues, detectRootFromFiles/Templates/CWD, +// checkRootConflict, DetectAndSetRoot, DetectAndSetRootFromFiles, +// EnsureTalosconfigPath. These functions decide which directory +// becomes Config.RootDir based on cobra flags + os.Args + CWD. +// They mutate package-level Config and GlobalArgs, so each test +// captures and restores the prior state. + +package commands + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/spf13/cobra" +) + +// crossPlatformAbs builds an absolute path that satisfies +// filepath.IsAbs on both POSIX and Windows. On POSIX the joined +// path is already absolute (leading `/`). On Windows the drive +// letter is mandatory, so the helper prepends the volume of the +// current working directory (typically `C:` on a CI runner). +func crossPlatformAbs(parts ...string) string { + p := filepath.Join(append([]string{string(filepath.Separator)}, parts...)...) + if filepath.IsAbs(p) { + return p + } + cwd, err := os.Getwd() + if err != nil { + return p + } + return filepath.Join(filepath.VolumeName(cwd), p) +} + +// withConfigSnapshot captures the package-level Config and GlobalArgs +// state before the test and restores them on cleanup. Without this +// every test that mutates Config.RootDir / RootDirExplicit / +// GlobalArgs leaks state into the next test in the same `go test` +// process. +func withConfigSnapshot(t *testing.T) { + t.Helper() + rootDir := Config.RootDir + rootDirExplicit := Config.RootDirExplicit + talosconfig := GlobalArgs.Talosconfig + talosconfigCfg := Config.GlobalOptions.Talosconfig + t.Cleanup(func() { + Config.RootDir = rootDir + Config.RootDirExplicit = rootDirExplicit + GlobalArgs.Talosconfig = talosconfig + Config.GlobalOptions.Talosconfig = talosconfigCfg + }) +} + +// withOSArgs replaces os.Args for the duration of the test. cobra's +// ParseFlags reads os.Args; some root-detection paths fall back to +// parseFlagFromArgs(os.Args[1:]) when cobra's slice is empty. +func withOSArgs(t *testing.T, args []string) { + t.Helper() + original := os.Args + t.Cleanup(func() { os.Args = original }) + os.Args = args +} + +// makeCmdWithStringSliceFlag creates a cobra.Command with a +// `StringSliceP` flag named `flagName`. Used to exercise getFlagValues. +func makeCmdWithStringSliceFlag(flagName, shortFlag string, defaults []string, persistent bool) *cobra.Command { + cmd := &cobra.Command{Use: "test"} + if persistent { + cmd.PersistentFlags().StringSliceP(flagName, shortFlag, defaults, "") + } else { + cmd.Flags().StringSliceP(flagName, shortFlag, defaults, "") + } + return cmd +} + +// === getFlagValues === + +// Contract: when the flag is registered as a non-persistent +// StringSlice flag and the user has set values, getFlagValues +// returns those values. +func TestContract_GetFlagValues_NonPersistent(t *testing.T) { + cmd := makeCmdWithStringSliceFlag("file", "f", nil, false) + if err := cmd.Flags().Set("file", "a.yaml,b.yaml"); err != nil { + t.Fatal(err) + } + got := getFlagValues(cmd, "file") + if len(got) != 2 || got[0] != "a.yaml" || got[1] != "b.yaml" { + t.Errorf("expected [a.yaml b.yaml], got %v", got) + } +} + +// Contract: persistent flags work too. cobra splits flags into +// command-local and persistent buckets; getFlagValues checks both. +func TestContract_GetFlagValues_Persistent(t *testing.T) { + cmd := makeCmdWithStringSliceFlag("template", "t", nil, true) + if err := cmd.PersistentFlags().Set("template", "templates/cp.yaml"); err != nil { + t.Fatal(err) + } + got := getFlagValues(cmd, "template") + if len(got) != 1 || got[0] != "templates/cp.yaml" { + t.Errorf("expected [templates/cp.yaml], got %v", got) + } +} + +// Contract: when the flag is not registered at all, the function +// returns an empty (non-nil) slice — never nil — so callers can +// `range` it without a guard. Pin the non-nil property explicitly. +func TestContract_GetFlagValues_AbsentFlagReturnsEmpty(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + got := getFlagValues(cmd, "nope") + if got == nil { + t.Error("expected non-nil empty slice") + } + if len(got) != 0 { + t.Errorf("expected empty slice, got %v", got) + } +} + +// Contract: a registered flag with no user value returns empty even +// when defaults are declared (the function checks `len(values) > 0` +// after `GetStringSlice`, so empty defaults stay empty). +func TestContract_GetFlagValues_RegisteredButUnset(t *testing.T) { + cmd := makeCmdWithStringSliceFlag("file", "f", nil, false) + got := getFlagValues(cmd, "file") + if len(got) != 0 { + t.Errorf("expected empty, got %v", got) + } +} + +// === detectRootFromFiles / detectRootFromTemplates / detectRootFromCWD === + +// Contract: empty input yields ('', nil) — caller falls through to +// the next detection strategy. +func TestContract_DetectRootFromFiles_EmptyInput(t *testing.T) { + got, err := detectRootFromFiles(nil) + if err != nil || got != "" { + t.Errorf("expected ('', nil), got (%q, %v)", got, err) + } +} + +func TestContract_DetectRootFromTemplates_EmptyInput(t *testing.T) { + got, err := detectRootFromTemplates(nil) + if err != nil || got != "" { + t.Errorf("expected ('', nil), got (%q, %v)", got, err) + } +} + +// Contract: with a real file under a project root, both functions +// return the abs path of the root. +func TestContract_DetectRootFromFiles_PositiveCase(t *testing.T) { + root := t.TempDir() + makeProjectRoot(t, root) + file := filepath.Join(root, "node.yaml") + if err := os.WriteFile(file, nil, 0o600); err != nil { + t.Fatal(err) + } + got, err := detectRootFromFiles([]string{file}) + if err != nil { + t.Fatal(err) + } + rootAbs, _ := filepath.Abs(root) + if got != rootAbs { + t.Errorf("got %q, want %q", got, rootAbs) + } +} + +func TestContract_DetectRootFromTemplates_PositiveCase(t *testing.T) { + root := t.TempDir() + makeProjectRoot(t, root) + tmpl := filepath.Join(root, "templates", "controlplane.yaml") + if err := os.MkdirAll(filepath.Dir(tmpl), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(tmpl, nil, 0o600); err != nil { + t.Fatal(err) + } + got, err := detectRootFromTemplates([]string{tmpl}) + if err != nil { + t.Fatal(err) + } + rootAbs, _ := filepath.Abs(root) + if got != rootAbs { + t.Errorf("got %q, want %q", got, rootAbs) + } +} + +// Contract: detectRootFromCWD walks up from the current working +// directory. Test by chdir-ing into a sub-directory of a project +// root, then asserting recovery. +func TestContract_DetectRootFromCWD_WalksUp(t *testing.T) { + root := t.TempDir() + makeProjectRoot(t, root) + subdir := filepath.Join(root, "deep", "deeper") + if err := os.MkdirAll(subdir, 0o755); err != nil { + t.Fatal(err) + } + t.Chdir(subdir) + + got, err := detectRootFromCWD() + if err != nil { + t.Fatal(err) + } + rootAbs, _ := filepath.Abs(root) + gotAbs, _ := filepath.Abs(got) + // On macOS, t.TempDir lives under /var/folders/... which is a + // symlink to /private/var/folders/... — compare via Abs to be + // robust to that resolution. + if !strings.HasSuffix(gotAbs, rootAbs) && !strings.HasSuffix(rootAbs, gotAbs) { + // Fallback: relative-equality after EvalSymlinks. + gotEval, _ := filepath.EvalSymlinks(gotAbs) + rootEval, _ := filepath.EvalSymlinks(rootAbs) + if gotEval != rootEval { + t.Errorf("got %q, want %q (eval got %q, want %q)", gotAbs, rootAbs, gotEval, rootEval) + } + } +} + +// === checkRootConflict === + +// Contract: when --root is not explicit, conflict check is a no-op +// regardless of detected root. Pin so a regression that always +// errors on mismatch surfaces here. +func TestContract_CheckRootConflict_NotExplicit(t *testing.T) { + withConfigSnapshot(t) + Config.RootDir = "/some/path" + if err := checkRootConflict("/different/path", false); err != nil { + t.Errorf("expected nil error when --root not explicit, got %v", err) + } +} + +// Contract: when --root IS explicit and matches the detected root, +// no error. +func TestContract_CheckRootConflict_ExplicitMatching(t *testing.T) { + withConfigSnapshot(t) + Config.RootDir = "/some/path" + if err := checkRootConflict("/some/path", true); err != nil { + t.Errorf("matching paths should not error, got %v", err) + } +} + +// Contract: when --root IS explicit and DIFFERS from detected, error +// names both paths. The error guides the operator to either drop +// --root or move the files. The function emits filepath.Abs of each +// path, which on Windows resolves to a drive-prefixed `D:\...` form; +// the assertion checks for the trailing path components rather than +// the full literal so it survives both POSIX and Windows. +func TestContract_CheckRootConflict_ExplicitConflict(t *testing.T) { + withConfigSnapshot(t) + explicit := crossPlatformAbs("explicit", "root") + detected := crossPlatformAbs("detected", "root") + Config.RootDir = explicit + err := checkRootConflict(detected, true) + if err == nil { + t.Fatal("expected error for conflicting roots") + } + explicitAbs, _ := filepath.Abs(explicit) + detectedAbs, _ := filepath.Abs(detected) + if !strings.Contains(err.Error(), explicitAbs) || !strings.Contains(err.Error(), detectedAbs) { + t.Errorf("error must name both abs paths (%q, %q), got: %v", explicitAbs, detectedAbs, err) + } +} + +// === DetectAndSetRoot === + +// Contract: DetectAndSetRoot with a -f flag pointing at a file under +// a project root sets Config.RootDir to that root. The flag wins +// over CWD. +func TestContract_DetectAndSetRoot_FromFileFlag(t *testing.T) { + withConfigSnapshot(t) + + root := t.TempDir() + makeProjectRoot(t, root) + nodeFile := filepath.Join(root, "node.yaml") + if err := os.WriteFile(nodeFile, nil, 0o600); err != nil { + t.Fatal(err) + } + + cmd := &cobra.Command{Use: "test"} + cmd.PersistentFlags().String("root", "", "") + cmd.Flags().StringSliceP("file", "f", nil, "") + cmd.Flags().StringSliceP("template", "t", nil, "") + if err := cmd.Flags().Set("file", nodeFile); err != nil { + t.Fatal(err) + } + withOSArgs(t, []string{"talm"}) + + if err := DetectAndSetRoot(cmd, nil); err != nil { + t.Fatalf("DetectAndSetRoot: %v", err) + } + rootAbs, _ := filepath.Abs(root) + if Config.RootDir != rootAbs { + t.Errorf("Config.RootDir = %q, want %q", Config.RootDir, rootAbs) + } +} + +// Contract: when -f points at files under DIFFERENT project roots, +// the function errors. Pinning the strict-consistency rule. +func TestContract_DetectAndSetRoot_FilesInDifferentRootsError(t *testing.T) { + withConfigSnapshot(t) + + rootA := t.TempDir() + rootB := t.TempDir() + makeProjectRoot(t, rootA) + makeProjectRoot(t, rootB) + fileA := filepath.Join(rootA, "a.yaml") + fileB := filepath.Join(rootB, "b.yaml") + for _, f := range []string{fileA, fileB} { + if err := os.WriteFile(f, nil, 0o600); err != nil { + t.Fatal(err) + } + } + + cmd := &cobra.Command{Use: "test"} + cmd.PersistentFlags().String("root", "", "") + cmd.Flags().StringSliceP("file", "f", nil, "") + cmd.Flags().StringSliceP("template", "t", nil, "") + if err := cmd.Flags().Set("file", fileA); err != nil { + t.Fatal(err) + } + if err := cmd.Flags().Set("file", fileB); err != nil { + t.Fatal(err) + } + withOSArgs(t, []string{"talm"}) + + err := DetectAndSetRoot(cmd, nil) + if err == nil { + t.Fatal("expected error for files in different roots") + } +} + +// Contract: with no flags and no project markers reachable from +// CWD, DetectAndSetRoot returns nil and leaves Config.RootDir +// untouched (or sets it to whatever the CWD walk-up yielded — the +// function tolerates "no project here"). +func TestContract_DetectAndSetRoot_NoFlagsNoMarkersIsTolerated(t *testing.T) { + withConfigSnapshot(t) + + emptyDir := t.TempDir() + t.Chdir(emptyDir) + + cmd := &cobra.Command{Use: "test"} + cmd.PersistentFlags().String("root", "", "") + cmd.Flags().StringSliceP("file", "f", nil, "") + cmd.Flags().StringSliceP("template", "t", nil, "") + withOSArgs(t, []string{"talm"}) + + if err := DetectAndSetRoot(cmd, nil); err != nil { + t.Errorf("expected nil error, got %v", err) + } +} + +// === DetectAndSetRootFromFiles === + +// Contract: DetectAndSetRootFromFiles with files all under one +// project root sets Config.RootDir to that root. Used by `talm +// apply` / `talm upgrade` to anchor on the operator's `-f` files +// when invoked without `--root`. +func TestContract_DetectAndSetRootFromFiles_HappyPath(t *testing.T) { + withConfigSnapshot(t) + Config.RootDirExplicit = false + + root := t.TempDir() + makeProjectRoot(t, root) + file := filepath.Join(root, "n.yaml") + if err := os.WriteFile(file, nil, 0o600); err != nil { + t.Fatal(err) + } + if err := DetectAndSetRootFromFiles([]string{file}); err != nil { + t.Fatalf("unexpected error: %v", err) + } + rootAbs, _ := filepath.Abs(root) + if Config.RootDir != rootAbs { + t.Errorf("Config.RootDir = %q, want %q", Config.RootDir, rootAbs) + } +} + +// Contract: empty input + RootDirExplicit=false falls back to CWD +// detection. No error if CWD has no markers — the function leaves +// Config.RootDir untouched. +func TestContract_DetectAndSetRootFromFiles_EmptyInputUsesCWD(t *testing.T) { + withConfigSnapshot(t) + Config.RootDirExplicit = false + root := t.TempDir() + makeProjectRoot(t, root) + t.Chdir(root) + + if err := DetectAndSetRootFromFiles(nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Config.RootDir should now be `root` (or its symlink-resolved + // form on macOS). + gotEval, _ := filepath.EvalSymlinks(Config.RootDir) + rootEval, _ := filepath.EvalSymlinks(root) + if gotEval != rootEval { + t.Errorf("Config.RootDir resolved = %q, want %q", gotEval, rootEval) + } +} + +// Contract: when --root was explicit AND the files belong to a +// different root, the function errors and names both roots. +func TestContract_DetectAndSetRootFromFiles_ExplicitConflict(t *testing.T) { + withConfigSnapshot(t) + + rootExplicit := t.TempDir() + rootFiles := t.TempDir() + makeProjectRoot(t, rootExplicit) + makeProjectRoot(t, rootFiles) + file := filepath.Join(rootFiles, "n.yaml") + if err := os.WriteFile(file, nil, 0o600); err != nil { + t.Fatal(err) + } + + Config.RootDir = rootExplicit + Config.RootDirExplicit = true + + err := DetectAndSetRootFromFiles([]string{file}) + if err == nil { + t.Fatal("expected conflict error") + } + if !strings.Contains(err.Error(), "conflicting") { + t.Errorf("error must mention 'conflicting', got: %v", err) + } +} + +// === EnsureTalosconfigPath === + +// Contract: when --talosconfig was explicitly set, EnsureTalosconfigPath +// is a no-op (does not overwrite GlobalArgs.Talosconfig). +func TestContract_EnsureTalosconfigPath_NoOpWhenChanged(t *testing.T) { + withConfigSnapshot(t) + + cmd := &cobra.Command{Use: "test"} + cmd.PersistentFlags().String("talosconfig", "", "") + if err := cmd.PersistentFlags().Set("talosconfig", "/explicit/path"); err != nil { + t.Fatal(err) + } + GlobalArgs.Talosconfig = "/explicit/path" + + EnsureTalosconfigPath(cmd) + if GlobalArgs.Talosconfig != "/explicit/path" { + t.Errorf("expected unchanged, got %q", GlobalArgs.Talosconfig) + } +} + +// Contract: when --talosconfig is unset and the chart-resolved +// GlobalArgs.Talosconfig is also empty, the path defaults to +// `/talosconfig`. Built via filepath.Join so the assertion +// matches the OS-native separator on Windows too. +func TestContract_EnsureTalosconfigPath_DefaultsToRoot(t *testing.T) { + withConfigSnapshot(t) + + cmd := &cobra.Command{Use: "test"} + cmd.PersistentFlags().String("talosconfig", "", "") + root := crossPlatformAbs("some", "project") + Config.RootDir = root + Config.GlobalOptions.Talosconfig = "" + GlobalArgs.Talosconfig = "" + + EnsureTalosconfigPath(cmd) + want := filepath.Join(root, "talosconfig") + if GlobalArgs.Talosconfig != want { + t.Errorf("expected %q, got %q", want, GlobalArgs.Talosconfig) + } +} + +// Contract: when GlobalArgs.Talosconfig is already set (e.g. by +// Chart.yaml's globalOptions.talosconfig), that value is used. Pins +// the precedence: Chart.yaml > the literal "talosconfig" default. +// Relative paths still get anchored to RootDir. +func TestContract_EnsureTalosconfigPath_RelativeAnchoredToRoot(t *testing.T) { + withConfigSnapshot(t) + + cmd := &cobra.Command{Use: "test"} + cmd.PersistentFlags().String("talosconfig", "", "") + root := crossPlatformAbs("some", "project") + Config.RootDir = root + GlobalArgs.Talosconfig = "talosconfig.encrypted" + + EnsureTalosconfigPath(cmd) + want := filepath.Join(root, "talosconfig.encrypted") + if GlobalArgs.Talosconfig != want { + t.Errorf("expected %q, got %q", want, GlobalArgs.Talosconfig) + } +} + +// Contract: an absolute path in GlobalArgs.Talosconfig stays +// untouched (no anchoring). +func TestContract_EnsureTalosconfigPath_AbsolutePathPreserved(t *testing.T) { + withConfigSnapshot(t) + + cmd := &cobra.Command{Use: "test"} + cmd.PersistentFlags().String("talosconfig", "", "") + Config.RootDir = filepath.Join(string(filepath.Separator), "some", "project") + abs := crossPlatformAbs("etc", "talos", "config") + GlobalArgs.Talosconfig = abs + + EnsureTalosconfigPath(cmd) + if GlobalArgs.Talosconfig != abs { + t.Errorf("expected %q preserved, got %q", abs, GlobalArgs.Talosconfig) + } +} + +// === updateKubeconfigEndpoint === + +// Contract: updateKubeconfigEndpoint takes raw kubeconfig bytes, +// rewrites every cluster's `server:` field to https://:6443, +// and returns the reserialised bytes. Used by the rotate-CA flow +// where kubeconfig content lives in memory rather than on disk. +func TestContract_UpdateKubeconfigEndpoint_RewritesAllClusters(t *testing.T) { + src := []byte(`apiVersion: v1 +kind: Config +clusters: +- name: one + cluster: + server: https://1.2.3.4:6443 +- name: two + cluster: + server: https://5.6.7.8:6443 +contexts: [] +users: [] +`) + out, err := updateKubeconfigEndpoint(src, "10.0.0.1:50000") + if err != nil { + t.Fatalf("updateKubeconfigEndpoint: %v", err) + } + got := string(out) + if !strings.Contains(got, "server: https://10.0.0.1:6443") { + t.Errorf("expected rewritten server, got:\n%s", got) + } + if strings.Contains(got, "1.2.3.4") || strings.Contains(got, "5.6.7.8") { + t.Errorf("old servers leaked:\n%s", got) + } +} + +// Contract: malformed kubeconfig surfaces a parse error. +func TestContract_UpdateKubeconfigEndpoint_MalformedError(t *testing.T) { + _, err := updateKubeconfigEndpoint([]byte("this is not yaml"), "10.0.0.1") + if err == nil { + t.Fatal("expected error for malformed kubeconfig") + } +} diff --git a/pkg/commands/contract_template_test.go b/pkg/commands/contract_template_test.go new file mode 100644 index 00000000..e4a9606a --- /dev/null +++ b/pkg/commands/contract_template_test.go @@ -0,0 +1,414 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: `talm template` rendering layer above engine.Render. +// generateOutput composes a leading modeline + warning banner with +// the engine-rendered config; resolveEngineTemplatePaths converts +// user-supplied template paths (absolute, relative-from-CWD, outside +// root) into the forward-slash relative form the helm engine uses +// to key into its render map. Both are user-observable through +// `talm template` stdout and `talm template --in-place`. + +package commands + +import ( + "context" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/siderolabs/talos/pkg/machinery/config/generate/secrets" + "gopkg.in/yaml.v3" +) + +// withTemplateFlagsSnapshot captures and restores the package-level +// templateCmdFlags + GlobalArgs.{Nodes,Endpoints} so each test can +// mutate freely without poisoning subsequent tests. Mirror of the +// withConfigSnapshot helper for root_dispatch tests. +func withTemplateFlagsSnapshot(t *testing.T) { + t.Helper() + flagsSave := templateCmdFlags + nodesSave := append([]string(nil), GlobalArgs.Nodes...) + endpointsSave := append([]string(nil), GlobalArgs.Endpoints...) + rootSave := Config.RootDir + rootExplicitSave := Config.RootDirExplicit + t.Cleanup(func() { + templateCmdFlags = flagsSave + GlobalArgs.Nodes = nodesSave + GlobalArgs.Endpoints = endpointsSave + Config.RootDir = rootSave + Config.RootDirExplicit = rootExplicitSave + }) +} + +// === resolveEngineTemplatePaths === + +// Contract: an absolute path that lies INSIDE the project root is +// returned as a forward-slash path relative to root. The helm +// engine indexes its render map by relative path with forward +// slashes regardless of OS, so any other form would fail lookup. +func TestContract_ResolveEngineTemplatePaths_AbsoluteInsideRoot(t *testing.T) { + root, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + tmpl := filepath.Join(root, "templates", "controlplane.yaml") + if err := os.MkdirAll(filepath.Dir(tmpl), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(tmpl, nil, 0o600); err != nil { + t.Fatal(err) + } + got := resolveEngineTemplatePaths([]string{tmpl}, root) + if len(got) != 1 || got[0] != "templates/controlplane.yaml" { + t.Errorf("got %v, want [templates/controlplane.yaml]", got) + } +} + +// Contract: a relative path resolved from CWD that ends up INSIDE +// root is returned as the relative form (no leading slash, no +// duplicated prefix). +func TestContract_ResolveEngineTemplatePaths_RelativeFromCWD(t *testing.T) { + root, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(root, "templates"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "templates", "worker.yaml"), nil, 0o600); err != nil { + t.Fatal(err) + } + t.Chdir(root) + got := resolveEngineTemplatePaths([]string{"templates/worker.yaml"}, root) + if len(got) != 1 || got[0] != "templates/worker.yaml" { + t.Errorf("got %v, want [templates/worker.yaml]", got) + } +} + +// Contract: when a `..` path resolves OUTSIDE root but a file with +// the same basename exists under /templates/, the function +// falls back to `templates/`. This is the documented +// fallback so an operator running from a sibling directory still +// hits the canonical templates/ location. +func TestContract_ResolveEngineTemplatePaths_OutsideRootFallbackToTemplatesBasename(t *testing.T) { + root, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(root, "templates"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "templates", "controlplane.yaml"), nil, 0o600); err != nil { + t.Fatal(err) + } + // Chdir into a directory NEXT to root, then pass a relative path + // that climbs out and back: ..//templates/controlplane.yaml + // — but craft it so it lies outside `root` per Rel(). + sibling := filepath.Join(filepath.Dir(root), "sibling") + if err := os.MkdirAll(sibling, 0o755); err != nil { + t.Fatal(err) + } + t.Chdir(sibling) + + got := resolveEngineTemplatePaths([]string{"../some/other/controlplane.yaml"}, root) + if len(got) != 1 || got[0] != "templates/controlplane.yaml" { + t.Errorf("got %v, want [templates/controlplane.yaml] (fallback)", got) + } +} + +// Contract: when a `..` path resolves OUTSIDE root AND no +// templates/ exists, the function returns the input +// normalized through forward slashes. This is the "operator +// supplied a real outside-root absolute path" case — let the helm +// engine error precisely. +func TestContract_ResolveEngineTemplatePaths_OutsideRootNoFallback(t *testing.T) { + root, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + got := resolveEngineTemplatePaths([]string{"/totally/outside/missing.yaml"}, root) + if len(got) != 1 { + t.Fatalf("len = %d, want 1", len(got)) + } + // Must not have been silently rewritten to `templates/missing.yaml` + // (because that file does not exist). + if got[0] == "templates/missing.yaml" { + t.Errorf("did not expect templates/ fallback, got %q", got[0]) + } +} + +// Contract: an empty input list yields an empty (non-nil) slice +// of the same length — so callers can range without a guard. +func TestContract_ResolveEngineTemplatePaths_EmptyInput(t *testing.T) { + root := t.TempDir() + got := resolveEngineTemplatePaths(nil, root) + if got == nil { + t.Error("expected non-nil empty slice") + } + if len(got) != 0 { + t.Errorf("expected empty slice, got %v", got) + } +} + +// === generateOutput happy path (offline) === + +// Contract: generateOutput composes three pieces in order — modeline, +// warning banner, engine-rendered bytes — separated by newlines. +// The modeline is the first line so subsequent `talm apply` / +// `talm template` invocations against the rendered file can pick +// up nodes/endpoints/templates without explicit flags. +func TestContract_GenerateOutput_ComposesModelineWarningAndRender(t *testing.T) { + withTemplateFlagsSnapshot(t) + + chartRoot := makeMinimalChart(t) + Config.RootDir = chartRoot + templateCmdFlags = struct { + insecure bool + configFiles []string + valueFiles []string + templateFiles []string + stringValues []string + values []string + fileValues []string + jsonValues []string + literalValues []string + talosVersion string + withSecrets string + full bool + debug bool + offline bool + kubernetesVersion string + inplace bool + nodesFromArgs bool + endpointsFromArgs bool + templatesFromArgs bool + }{ + offline: true, + templateFiles: []string{"templates/config.yaml"}, + } + GlobalArgs.Nodes = []string{"10.0.0.1"} + GlobalArgs.Endpoints = []string{"10.0.0.1"} + + got, err := generateOutput(context.Background(), nil, nil) + if err != nil { + t.Fatalf("generateOutput: %v", err) + } + lines := strings.SplitN(got, "\n", 3) + if len(lines) < 3 { + t.Fatalf("expected >=3 lines, got %d:\n%s", len(lines), got) + } + // Line 1: modeline. + if !strings.HasPrefix(lines[0], "# talm: ") { + t.Errorf("expected modeline as first line, got %q", lines[0]) + } + if !strings.Contains(lines[0], `"10.0.0.1"`) { + t.Errorf("modeline missing nodes: %q", lines[0]) + } + if !strings.Contains(lines[0], "templates/config.yaml") { + t.Errorf("modeline missing template path: %q", lines[0]) + } + // Line 2: warning banner. + if !strings.Contains(lines[1], "AUTOGENERATED") { + t.Errorf("expected AUTOGENERATED warning as second line, got %q", lines[1]) + } + // Body: rendered config. + if !strings.Contains(lines[2], "machine:") || !strings.Contains(lines[2], "type: worker") { + t.Errorf("expected rendered machine config in body, got:\n%s", lines[2]) + } +} + +// Contract: a missing template path surfaces a wrapped error with +// the `failed to render templates` prefix. The wrap helps callers +// (and operators reading stderr) distinguish chart-render failures +// from earlier parse failures. +func TestContract_GenerateOutput_MissingTemplateError(t *testing.T) { + withTemplateFlagsSnapshot(t) + + chartRoot := makeMinimalChart(t) + Config.RootDir = chartRoot + templateCmdFlags.offline = true + templateCmdFlags.templateFiles = []string{"templates/does-not-exist.yaml"} + GlobalArgs.Nodes = []string{"10.0.0.1"} + + _, err := generateOutput(context.Background(), nil, nil) + if err == nil { + t.Fatal("expected error for missing template") + } + if !strings.Contains(err.Error(), "failed to render templates") { + t.Errorf("error must wrap with 'failed to render templates', got: %v", err) + } +} + +// Contract: an empty templates list surfaces the engine-side +// 'templates are not set' error wrapped with the same prefix. +func TestContract_GenerateOutput_NoTemplatesError(t *testing.T) { + withTemplateFlagsSnapshot(t) + + chartRoot := makeMinimalChart(t) + Config.RootDir = chartRoot + templateCmdFlags.offline = true + templateCmdFlags.templateFiles = nil + GlobalArgs.Nodes = []string{"10.0.0.1"} + + _, err := generateOutput(context.Background(), nil, nil) + if err == nil { + t.Fatal("expected error for empty templates") + } +} + +// === template === + +// Contract: template(args) returns a function that runs +// generateOutput and prints the result to stdout. Capture stdout +// and assert the modeline header lands. +func TestContract_Template_PrintsToStdout(t *testing.T) { + withTemplateFlagsSnapshot(t) + + chartRoot := makeMinimalChart(t) + Config.RootDir = chartRoot + templateCmdFlags.offline = true + templateCmdFlags.templateFiles = []string{"templates/config.yaml"} + GlobalArgs.Nodes = []string{"10.0.0.1"} + GlobalArgs.Endpoints = []string{"10.0.0.1"} + + out := captureStdout(t, func() { + if err := template(nil)(context.Background(), nil); err != nil { + t.Fatalf("template: %v", err) + } + }) + if !strings.Contains(out, "# talm: ") { + t.Errorf("expected modeline header on stdout, got:\n%s", out) + } + if !strings.Contains(out, "AUTOGENERATED") { + t.Errorf("expected warning banner on stdout, got:\n%s", out) + } + if !strings.Contains(out, "type: worker") { + t.Errorf("expected rendered body on stdout, got:\n%s", out) + } +} + +// Contract: template(args) propagates the wrapped error from +// generateOutput unchanged — does not double-wrap or swallow. +func TestContract_Template_PropagatesError(t *testing.T) { + withTemplateFlagsSnapshot(t) + + chartRoot := makeMinimalChart(t) + Config.RootDir = chartRoot + templateCmdFlags.offline = true + templateCmdFlags.templateFiles = []string{"templates/missing.yaml"} + GlobalArgs.Nodes = []string{"10.0.0.1"} + + err := template(nil)(context.Background(), nil) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "failed to render templates") { + t.Errorf("expected 'failed to render templates' wrap, got: %v", err) + } +} + +// === helpers === + +// sharedSecretsBundle is generated once per test process — +// secrets.NewBundle populates a fresh PKI tree (~half a second on a +// laptop), so generating it inside every test would dominate the +// suite runtime. The serialized bytes are reused as the +// secrets.yaml fixture for every minimal chart. +var ( + sharedSecretsOnce sync.Once + sharedSecretsYAML []byte + sharedSecretsError error +) + +func loadSharedSecretsYAML(t *testing.T) []byte { + t.Helper() + sharedSecretsOnce.Do(func() { + bundle, err := secrets.NewBundle(secrets.NewClock(), nil) + if err != nil { + sharedSecretsError = err + return + } + sharedSecretsYAML, sharedSecretsError = yaml.Marshal(bundle) + }) + if sharedSecretsError != nil { + t.Fatalf("generate shared secrets bundle: %v", sharedSecretsError) + } + return sharedSecretsYAML +} + +// makeMinimalChart writes a chart layout sufficient for `talm +// template --offline`: Chart.yaml, values.yaml, templates/config.yaml +// emitting a worker machine config patch, plus a real serialized +// secrets bundle so engine.Render's bundle.NewBundle path completes. +// Returns the chart root path. +func makeMinimalChart(t *testing.T) string { + t.Helper() + root, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "Chart.yaml"), []byte("apiVersion: v2\nname: tc\nversion: 0.1.0\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "values.yaml"), []byte("{}\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(root, "secrets.yaml"), loadSharedSecretsYAML(t), 0o600); err != nil { + t.Fatal(err) + } + tmplDir := filepath.Join(root, "templates") + if err := os.MkdirAll(tmplDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(tmplDir, "config.yaml"), []byte("machine:\n type: worker\n"), 0o644); err != nil { + t.Fatal(err) + } + return root +} + +// captureStdout redirects os.Stdout to a pipe for the duration of +// fn, returns whatever fn printed. +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + original := os.Stdout + os.Stdout = w + t.Cleanup(func() { os.Stdout = original }) + + done := make(chan string, 1) + go func() { + buf := make([]byte, 0, 64*1024) + readBuf := make([]byte, 4096) + for { + n, err := r.Read(readBuf) + if n > 0 { + buf = append(buf, readBuf[:n]...) + } + if err != nil { + break + } + } + done <- string(buf) + }() + + fn() + _ = w.Close() + return <-done +} diff --git a/pkg/commands/root_detection_test.go b/pkg/commands/root_detection_test.go new file mode 100644 index 00000000..01acc225 --- /dev/null +++ b/pkg/commands/root_detection_test.go @@ -0,0 +1,351 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: argument parsing for root-detection helpers in +// root_detection.go. These functions decide which directory becomes +// the talm project root based on user-supplied -f / --file / -t / +// --template flags. The contract is user-facing: the CLI behaviour +// users rely on when running `talm apply -f nodes/cp1.yaml,nodes/cp2.yaml` +// or `talm template --template templates/cluster.yaml`. + +package commands + +import ( + "os" + "path/filepath" + "reflect" + "sort" + "testing" +) + +// === parseCommaSeparatedValues === + +// Contract: comma-separated values are split, trimmed, and empty +// entries are dropped. A single value (no comma) returns a one-element +// slice. Empty input returns nil. +func TestContract_ParseCommaSeparatedValues(t *testing.T) { + cases := []struct { + name string + input string + want []string + }{ + {"empty", "", nil}, + {"single", "foo", []string{"foo"}}, + {"two values", "foo,bar", []string{"foo", "bar"}}, + {"three with whitespace", " foo , bar , baz ", []string{"foo", "bar", "baz"}}, + {"empty entries dropped", "foo,,bar,", []string{"foo", "bar"}}, + {"only commas", ",,,", nil}, + {"only whitespace", " ", nil}, + {"path-like values", "nodes/cp1.yaml,nodes/cp2.yaml", []string{"nodes/cp1.yaml", "nodes/cp2.yaml"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := parseCommaSeparatedValues(tc.input) + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("parseCommaSeparatedValues(%q) = %v, want %v", tc.input, got, tc.want) + } + }) + } +} + +// === parseFlagFromArgs === + +// Contract: scans an argv-style slice for a flag in either short or +// long form, supports both space-separated (-f value) and equal-sign +// (-f=value) forms, and propagates comma-separated values. The first +// occurrence wins (later flag instances are ignored — this matches +// cobra's "first occurrence" semantics for non-slice flags). Stops +// at the first hit; does not scan further. Returns nil if the flag is +// absent or has no following value. +func TestContract_ParseFlagFromArgs(t *testing.T) { + cases := []struct { + name string + args []string + shortFlag string + longFlag string + want []string + }{ + { + name: "short-form space-separated", + args: []string{"-f", "nodes/cp1.yaml", "--other"}, + shortFlag: "-f", longFlag: "--file", + want: []string{"nodes/cp1.yaml"}, + }, + { + name: "long-form space-separated", + args: []string{"--file", "nodes/cp1.yaml"}, + shortFlag: "-f", longFlag: "--file", + want: []string{"nodes/cp1.yaml"}, + }, + { + name: "short-form equal-sign", + args: []string{"-f=nodes/cp1.yaml"}, + shortFlag: "-f", longFlag: "--file", + want: []string{"nodes/cp1.yaml"}, + }, + { + name: "long-form equal-sign", + args: []string{"--file=nodes/cp1.yaml"}, + shortFlag: "-f", longFlag: "--file", + want: []string{"nodes/cp1.yaml"}, + }, + { + name: "comma-separated values via short form", + args: []string{"-f", "a.yaml,b.yaml,c.yaml"}, + shortFlag: "-f", longFlag: "--file", + want: []string{"a.yaml", "b.yaml", "c.yaml"}, + }, + { + name: "absent flag", + args: []string{"--other", "x"}, + shortFlag: "-f", longFlag: "--file", + want: nil, + }, + { + name: "flag with no following value", + args: []string{"-f"}, + shortFlag: "-f", longFlag: "--file", + want: nil, + }, + { + name: "flag followed by another flag (no value)", + args: []string{"-f", "--other"}, + shortFlag: "-f", longFlag: "--file", + want: nil, + }, + { + name: "first occurrence wins", + args: []string{"-f", "first.yaml", "-f", "second.yaml"}, + shortFlag: "-f", longFlag: "--file", + want: []string{"first.yaml"}, + }, + { + name: "templates flag (-t / --template)", + args: []string{"-t", "templates/cluster.yaml"}, + shortFlag: "-t", longFlag: "--template", + want: []string{"templates/cluster.yaml"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := parseFlagFromArgs(tc.args, tc.shortFlag, tc.longFlag) + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("parseFlagFromArgs(%v, %q, %q) = %v, want %v", + tc.args, tc.shortFlag, tc.longFlag, got, tc.want) + } + }) + } +} + +// === ResolveSecretsPath === + +// Contract: when the input is empty, the default file name is +// "secrets.yaml". Relative paths are anchored against Config.RootDir; +// absolute paths are returned as-is. The function uses filepath.Join, +// so the OS-native separator lands in the result — tests build +// expected values via filepath.Join so they match on Windows too. +func TestContract_ResolveSecretsPath(t *testing.T) { + originalRoot := Config.RootDir + t.Cleanup(func() { Config.RootDir = originalRoot }) + root := crossPlatformAbs("some", "project") + Config.RootDir = root + + absSecrets := crossPlatformAbs("etc", "secrets.yaml") + cases := []struct { + name string + input string + want string + }{ + {"empty defaults to secrets.yaml under root", "", filepath.Join(root, "secrets.yaml")}, + {"relative anchored to root", filepath.Join("vault", "secrets.yaml"), filepath.Join(root, "vault", "secrets.yaml")}, + {"absolute returned verbatim", absSecrets, absSecrets}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := ResolveSecretsPath(tc.input) + if got != tc.want { + t.Errorf("ResolveSecretsPath(%q) = %q, want %q (Config.RootDir=%q)", tc.input, got, tc.want, Config.RootDir) + } + }) + } +} + +// === ExpandFilePaths / findYAMLFiles === + +// Contract: ExpandFilePaths converts user-supplied paths to absolute +// paths. Files are returned as-is (caller handles non-existence +// later). Directories are expanded to all .yaml/.yml files inside, +// recursively. An empty directory is an error: the operator clearly +// asked for SOMETHING under that directory. +func TestContract_ExpandFilePaths_Files(t *testing.T) { + dir := t.TempDir() + file := filepath.Join(dir, "node.yaml") + if err := os.WriteFile(file, []byte("ok"), 0o644); err != nil { + t.Fatal(err) + } + got, err := ExpandFilePaths([]string{file}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(got) != 1 || got[0] != file { + t.Errorf("expected [%q], got %v", file, got) + } +} + +func TestContract_ExpandFilePaths_DirectoryRecursive(t *testing.T) { + dir := t.TempDir() + subdir := filepath.Join(dir, "sub") + if err := os.Mkdir(subdir, 0o755); err != nil { + t.Fatal(err) + } + yamlFiles := []string{ + filepath.Join(dir, "a.yaml"), + filepath.Join(dir, "b.yml"), + filepath.Join(subdir, "c.yaml"), + } + for _, f := range yamlFiles { + if err := os.WriteFile(f, []byte("ok"), 0o644); err != nil { + t.Fatal(err) + } + } + // Non-YAML file: must be ignored. + if err := os.WriteFile(filepath.Join(dir, "readme.md"), []byte("ok"), 0o644); err != nil { + t.Fatal(err) + } + + got, err := ExpandFilePaths([]string{dir}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + sort.Strings(got) + want := append([]string{}, yamlFiles...) + sort.Strings(want) + if !reflect.DeepEqual(got, want) { + t.Errorf("expanded YAML files mismatch\n got: %v\nwant: %v", got, want) + } +} + +// Contract: an empty directory under a `-f` argument is an explicit +// error. The operator pointed at "this directory holds my node +// configs"; an empty directory is a typo or wrong path, not "no +// nodes". Talm refuses silently rendering nothing. +func TestContract_ExpandFilePaths_EmptyDirectoryIsError(t *testing.T) { + dir := t.TempDir() + _, err := ExpandFilePaths([]string{dir}) + if err == nil { + t.Fatal("expected error for empty directory, got nil") + } +} + +// Contract: a non-existent path is NOT an error at expansion time — +// it is returned as-is (with absolute resolution) so the caller can +// produce a precise downstream error. ExpandFilePaths is just glob +// expansion, not validation. +func TestContract_ExpandFilePaths_NonExistentPathPropagated(t *testing.T) { + missing := filepath.Join(t.TempDir(), "does-not-exist.yaml") + got, err := ExpandFilePaths([]string{missing}) + if err != nil { + t.Fatalf("expected no error for missing file (caller validates), got: %v", err) + } + if len(got) != 1 || got[0] != missing { + t.Errorf("expected [%q], got %v", missing, got) + } +} + +// === isValidPreset === + +// Contract: preset name must appear verbatim in the available list. +// Case-sensitive; no fuzzy matching. The list comes from +// pkg/generated/presets.go (built at chart-bake time). +func TestContract_IsValidPreset(t *testing.T) { + available := []string{"cozystack", "generic", "talos"} + + for _, name := range available { + if !isValidPreset(name, available) { + t.Errorf("isValidPreset(%q, %v) = false, want true", name, available) + } + } + for _, name := range []string{"Cozystack", "COZYSTACK", "unknown", "", " cozystack"} { + if isValidPreset(name, available) { + t.Errorf("isValidPreset(%q, %v) = true, want false", name, available) + } + } +} + +// === fileExists === + +// Contract: fileExists is a thin wrapper around os.Stat reporting +// only existence (true/false). Permission errors and I/O failures +// register as "does not exist" — the chart-side flow that calls this +// (printSecretsWarning, kubeconfig handling) treats both states +// identically anyway. +func TestContract_FileExists(t *testing.T) { + dir := t.TempDir() + existing := filepath.Join(dir, "f") + if err := os.WriteFile(existing, []byte("ok"), 0o600); err != nil { + t.Fatal(err) + } + if !fileExists(existing) { + t.Errorf("fileExists(%q) = false, want true", existing) + } + if fileExists(filepath.Join(dir, "missing")) { + t.Errorf("fileExists for missing file returned true") + } +} + +// === filesDiffer === + +// Contract: filesDiffer compares an existing file's bytes against a +// proposed new content. Returns: +// - (true, nil) if file does not exist (any new content "differs" +// from a non-existent file from the operator-intent perspective) +// - (true, nil) if contents differ +// - (false, nil) if contents match exactly (used to skip a noisy +// "do you want to overwrite?" prompt when the new content equals +// what's already on disk) +// - (false, err) on read errors other than not-exist +func TestContract_FilesDiffer(t *testing.T) { + dir := t.TempDir() + existing := filepath.Join(dir, "f") + + // Missing file → differs. + differ, err := filesDiffer(existing, []byte("hello")) + if err != nil { + t.Fatalf("unexpected error for missing file: %v", err) + } + if !differ { + t.Errorf("expected differs=true for missing file") + } + + // Equal content → does not differ. + if err := os.WriteFile(existing, []byte("hello"), 0o644); err != nil { + t.Fatal(err) + } + differ, err = filesDiffer(existing, []byte("hello")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if differ { + t.Errorf("expected differs=false for equal content") + } + + // Different content → differs. + differ, err = filesDiffer(existing, []byte("world")) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !differ { + t.Errorf("expected differs=true for changed content") + } +} diff --git a/pkg/engine/contract_cluster_test.go b/pkg/engine/contract_cluster_test.go new file mode 100644 index 00000000..10310ad4 --- /dev/null +++ b/pkg/engine/contract_cluster_test.go @@ -0,0 +1,452 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: rendered `cluster:` section semantics for the cozystack and +// generic charts. The tests here pin user-facing behaviour of every +// values.yaml knob that surfaces under `cluster.*` for each schema +// (legacy pre-v1.12 single-doc, v1.12+ multi-doc) and each machine type +// (controlplane, worker). +// +// IMPORTANT: cozystack and generic are NOT siblings. cozystack carries +// many opinionated defaults (clusterDomain, OIDC, allocateNodeCIDRs, +// allowSchedulingOnControlPlanes, proxy disabled, discovery disabled, +// unconditional 127.0.0.1 in certSANs, sysctls, kernel modules, files, +// registries). generic ships a minimal config and intentionally omits +// all of those. The tests below are explicit about which chart owns +// which contract — and the generic-only "absence" tests pin that +// generic stays minimal even if cozystack adds new defaults later. +// +// Each test is preceded by a Contract: comment describing what is +// being pinned and why. That comment is the user-facing documentation; +// the test body is the executable enforcement of it. + +package engine + +import ( + "testing" +) + +// helmEngineEmptyLookup is the offline-mode LookupFunc used by every +// contract test that does not exercise discovery-driven branches. It +// returns an empty map for every (resource, namespace, name) tuple so +// helpers like talm.discovered.* yield empty strings/JSON arrays. Tests +// that need real discovery state plug in their own LookupFunc via +// renderChartTemplateWithLookup. +func helmEngineEmptyLookup(string, string, string) (map[string]any, error) { + return map[string]any{}, nil +} + +// chartCell is one (chart, schema, machineType) cell of the test +// matrix. The verbose name is intentional — it appears in `go test -v` +// output, and a reader should immediately see which chart, which Talos +// schema, and which machine type failed. +type chartCell struct { + name string + chartPath string + templateFile string + talosVersion string // empty for legacy schema, "v1.12.0" for multi-doc +} + +const ( + cozystackChartPath = "../../charts/cozystack" + genericChartPath = "../../charts/generic" + controlplaneTpl = "templates/controlplane.yaml" + workerTpl = "templates/worker.yaml" + multidocTalos = "v1.12.0" +) + +// allCells enumerates every (chart × schema × machineType) cell. +func allCells() []chartCell { + return []chartCell{ + {"cozystack/legacy/controlplane", cozystackChartPath, controlplaneTpl, ""}, + {"cozystack/legacy/worker", cozystackChartPath, workerTpl, ""}, + {"cozystack/multidoc/controlplane", cozystackChartPath, controlplaneTpl, multidocTalos}, + {"cozystack/multidoc/worker", cozystackChartPath, workerTpl, multidocTalos}, + {"generic/legacy/controlplane", genericChartPath, controlplaneTpl, ""}, + {"generic/legacy/worker", genericChartPath, workerTpl, ""}, + {"generic/multidoc/controlplane", genericChartPath, controlplaneTpl, multidocTalos}, + {"generic/multidoc/worker", genericChartPath, workerTpl, multidocTalos}, + } +} + +// cozystackCells returns the four cozystack matrix entries. +func cozystackCells() []chartCell { + out := []chartCell{} + for _, c := range allCells() { + if c.chartPath == cozystackChartPath { + out = append(out, c) + } + } + return out +} + +// cozystackControlplaneCells returns cozystack controlplane (legacy + multidoc). +func cozystackControlplaneCells() []chartCell { + out := []chartCell{} + for _, c := range cozystackCells() { + if c.templateFile == controlplaneTpl { + out = append(out, c) + } + } + return out +} + +// genericCells returns the four generic matrix entries. +func genericCells() []chartCell { + out := []chartCell{} + for _, c := range allCells() { + if c.chartPath == genericChartPath { + out = append(out, c) + } + } + return out +} + +// genericControlplaneCells returns generic controlplane (legacy + multidoc). +func genericControlplaneCells() []chartCell { + out := []chartCell{} + for _, c := range genericCells() { + if c.templateFile == controlplaneTpl { + out = append(out, c) + } + } + return out +} + +// allWorkerCells returns every worker cell across both charts. +func allWorkerCells() []chartCell { + out := []chartCell{} + for _, c := range allCells() { + if c.templateFile == workerTpl { + out = append(out, c) + } + } + return out +} + +// chartNameFor extracts the chart's directory name (== Chart.Name for +// both shipped charts). +func chartNameFor(c chartCell) string { + if c.chartPath == cozystackChartPath { + return "cozystack" + } + return "generic" +} + +// === Shared contracts: hold across both charts, both schemas, both machine types === + +// Contract: cluster.clusterName always equals the chart's Chart.Name +// when no override is supplied. Both charts hardcode this in their +// _helpers.tpl as `clusterName: "{{ .Chart.Name }}"`. The value is +// baked into Talos PKI cert SANs and ETCD bootstrap identity, so a +// drift here breaks every existing node's trust chain on next apply. +// The trailing quote in the expected substring is significant: the +// chart wraps the value in a double-quoted scalar, so a future move to +// a raw scalar (`clusterName: cozystack`) is a YAML-level shift this +// test catches. +func TestContract_Cluster_ClusterName_DefaultsToChartName(t *testing.T) { + for _, cell := range allCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + expected := `clusterName: "` + chartNameFor(cell) + `"` + assertContains(t, out, expected) + }) + } +} + +// Contract: podSubnets default is 10.244.0.0/16 in BOTH charts. The +// list form is significant: even a single-element default is rendered +// as a YAML list, because Talos requires `podSubnets` to be a sequence +// — a future "optimization" that emits a scalar would break Talos +// parsing. +func TestContract_Cluster_PodSubnets_Default(t *testing.T) { + for _, cell := range allCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "podSubnets:") + assertContains(t, out, "- 10.244.0.0/16") + }) + } +} + +// Contract: serviceSubnets default is 10.96.0.0/16 in BOTH charts. +// Same list-form contract as podSubnets. +func TestContract_Cluster_ServiceSubnets_Default(t *testing.T) { + for _, cell := range allCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "serviceSubnets:") + assertContains(t, out, "- 10.96.0.0/16") + }) + } +} + +// Contract: cluster.controlPlane.endpoint is always rendered as a +// double-quoted string. Talos parses this as a string, but quoting +// makes the YAML unambiguous for any downstream tool that re-parses +// the rendered output. +func TestContract_Cluster_Endpoint_Quoted(t *testing.T) { + for _, cell := range allCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, `endpoint: "`+testEndpoint+`"`) + }) + } +} + +// === Worker-only contracts (cluster.* fields that must NOT appear on workers) === + +// Contract: worker templates never emit controlplane-only cluster +// blocks (apiServer, controllerManager, scheduler, etcd, proxy, +// discovery, allowSchedulingOnControlPlanes). Talos rejects these on +// worker configs — leaking any of them is a hard validation error on +// `talm apply`. Both charts, both schemas. +func TestContract_Cluster_NoControlplaneBlocksOnWorker(t *testing.T) { + for _, cell := range allWorkerCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertNotContains(t, out, "apiServer:") + assertNotContains(t, out, "controllerManager:") + assertNotContains(t, out, "scheduler:") + assertNotContains(t, out, "etcd:") + assertNotContains(t, out, "allowSchedulingOnControlPlanes") + assertNotContains(t, out, "proxy:") + assertNotContains(t, out, "discovery:") + }) + } +} + +// === cozystack-only contracts === + +// Contract: cozystack ships clusterDomain: cozy.local, surfaced as +// `cluster.network.dnsDomain: cozy.local`. This is a cozystack +// convention — the generic chart does not expose clusterDomain at all +// (see TestContract_Cluster_NoClusterDomainOnGeneric). +func TestContract_Cluster_ClusterDomain_Cozystack(t *testing.T) { + for _, cell := range cozystackCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "dnsDomain: cozy.local") + }) + } +} + +// Contract: cozystack unconditionally prepends 127.0.0.1 to +// apiServer.certSANs on the controlplane. Required for talosctl / +// kubectl running on the control-plane node itself: without 127.0.0.1 +// in the cert SANs, local-loopback API access fails TLS validation. +// Generic does NOT emit this — see +// TestContract_Cluster_NoUnconditionalLoopbackOnGeneric. +func TestContract_Cluster_CertSANs_LoopbackUnconditional_Cozystack(t *testing.T) { + for _, cell := range cozystackControlplaneCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "certSANs:") + assertContains(t, out, "- 127.0.0.1") + }) + } +} + +// Contract: cozystack appends operator-supplied certSANs after the +// hardcoded 127.0.0.1. Pins the additive behaviour so a regression +// that replaces the loopback entry with the user list (instead of +// appending) would surface here. +func TestContract_Cluster_CertSANs_AppendsUserValues_Cozystack(t *testing.T) { + out := renderCozystackWith(t, helmEngineEmptyLookup, map[string]any{ + "certSANs": []any{"api.example.com", "10.0.0.1"}, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "- 127.0.0.1") + assertContains(t, out, "- api.example.com") + assertContains(t, out, "- 10.0.0.1") +} + +// Contract: cozystack does NOT emit oidc-* extraArgs unless +// oidcIssuerUrl is set. An always-emitted empty oidc-issuer-url is a +// kube-apiserver startup error. +func TestContract_Cluster_OIDC_AbsentByDefault_Cozystack(t *testing.T) { + for _, cell := range cozystackControlplaneCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertNotContains(t, out, "oidc-issuer-url") + assertNotContains(t, out, "oidc-client-id") + }) + } +} + +// Contract: when oidcIssuerUrl is set, cozystack emits the four +// oidc-* extraArgs (issuer-url is operator-supplied; client-id / +// username-claim / groups-claim are hardcoded). This contract +// guarantees that an operator only needs to set oidcIssuerUrl to get a +// fully-working OIDC integration. +func TestContract_Cluster_OIDC_PresentWhenSet_Cozystack(t *testing.T) { + const issuer = "https://oidc.example.com" + out := renderCozystackWith(t, helmEngineEmptyLookup, map[string]any{ + "oidcIssuerUrl": issuer, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, `oidc-issuer-url: "`+issuer+`"`) + assertContains(t, out, `oidc-client-id: "kubernetes"`) + assertContains(t, out, `oidc-username-claim: "preferred_username"`) + assertContains(t, out, `oidc-groups-claim: "groups"`) +} + +// Contract: cozystack ships allocateNodeCIDRs: true. The +// controllerManager extraArgs reflects this with `allocate-node-cidrs: +// true` AND `cluster-cidr: `. A regression that +// flipped the default would silently disable kube-controller-manager's +// IPAM and break pod networking on every new node. +func TestContract_Cluster_AllocateNodeCIDRs_Default_Cozystack(t *testing.T) { + for _, cell := range cozystackControlplaneCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "allocate-node-cidrs: true") + assertContains(t, out, `cluster-cidr: "10.244.0.0/16"`) + }) + } +} + +// Contract: when an operator sets allocateNodeCIDRs: false, cozystack +// emits ONLY `allocate-node-cidrs: false` and does NOT emit the +// cluster-cidr extraArg. Leaving cluster-cidr present while +// allocate-node-cidrs is false triggers a kube-controller-manager +// warning. The conditional emission is the contract. +func TestContract_Cluster_AllocateNodeCIDRs_Disabled_Cozystack(t *testing.T) { + out := renderCozystackWith(t, helmEngineEmptyLookup, map[string]any{ + "allocateNodeCIDRs": false, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "allocate-node-cidrs: false") + assertNotContains(t, out, "cluster-cidr:") +} + +// Contract: cozystack control-plane nodes are schedulable +// (`allowSchedulingOnControlPlanes: true`). Required for cozystack +// edge / single-node deployments where workloads must run on +// control-plane capacity. Flipping this default silently breaks small +// clusters. +func TestContract_Cluster_AllowSchedulingOnControlPlanes_Cozystack(t *testing.T) { + for _, cell := range cozystackControlplaneCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "allowSchedulingOnControlPlanes: true") + }) + } +} + +// Contract: cozystack disables kube-proxy chart-wide (Cilium-or-similar +// CNI owns service routing). A regression that re-enabled kube-proxy +// would double-program iptables and break service routing in subtle +// ways. +func TestContract_Cluster_ProxyDisabled_Cozystack(t *testing.T) { + for _, cell := range cozystackControlplaneCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "proxy:") + assertContains(t, out, "disabled: true") + }) + } +} + +// Contract: cozystack disables Talos's built-in cluster discovery +// because cozystack handles cluster bootstrap differently. +func TestContract_Cluster_DiscoveryDisabled_Cozystack(t *testing.T) { + for _, cell := range cozystackControlplaneCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "discovery:") + assertContains(t, out, "enabled: false") + }) + } +} + +// === generic-only contracts: pin that generic stays minimal === + +// Contract: generic chart does NOT expose clusterDomain in values.yaml +// and does NOT emit `dnsDomain:` in rendered output. If a future +// "convergence" change adds clusterDomain to generic, this test +// flags it explicitly so the change is intentional. +func TestContract_Cluster_NoClusterDomainOnGeneric(t *testing.T) { + for _, cell := range genericCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertNotContains(t, out, "dnsDomain:") + assertNotContains(t, out, "cozy.local") + }) + } +} + +// Contract: generic chart does NOT prepend 127.0.0.1 to certSANs. The +// generic apiServer block emits certSANs only when values.certSANs is +// non-empty (no unconditional loopback). This pins the minimal +// philosophy: generic ships exactly what the operator asks for, no +// hidden defaults. +func TestContract_Cluster_NoUnconditionalLoopbackOnGeneric(t *testing.T) { + for _, cell := range genericControlplaneCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + // Loopback must not appear at all when values.certSANs is empty + // (the default). renderChartTemplate does not inject certSANs. + assertNotContains(t, out, "- 127.0.0.1") + }) + } +} + +// Contract: generic chart does NOT carry cozystack-specific cluster +// defaults (OIDC, allocateNodeCIDRs, allowSchedulingOnControlPlanes, +// proxy disabled, discovery disabled). Pinning the absence prevents +// accidental "I copied the cozystack default into generic" drift. +func TestContract_Cluster_NoCozystackDefaultsOnGeneric(t *testing.T) { + for _, cell := range genericCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertNotContains(t, out, "oidc-issuer-url") + assertNotContains(t, out, "allocate-node-cidrs") + assertNotContains(t, out, "cluster-cidr") + assertNotContains(t, out, "allowSchedulingOnControlPlanes") + assertNotContains(t, out, "proxy:") + // `discovery:` must not appear under cluster.* on generic. The + // substring is unique enough since the chart never emits a + // cluster.discovery block elsewhere; tightening to a YAML-path + // match is left for a later iteration. + assertNotContains(t, out, "discovery:") + }) + } +} + +// Contract: generic chart's cluster.apiServer block is emitted on +// controlplane but is empty when values.certSANs is unset. This +// matches the chart's deliberate minimalism — apiServer exists as a +// container for operator additions, not for hidden defaults. +func TestContract_Cluster_GenericApiServerBlockExistsButEmpty(t *testing.T) { + for _, cell := range genericControlplaneCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "apiServer:") + // No certSANs sub-key when values.certSANs is unset. + assertNotContains(t, out, "certSANs:\n - ") + }) + } +} + +// Contract: generic chart appends operator-supplied certSANs verbatim, +// without any 127.0.0.1 prepended. +func TestContract_Cluster_GenericCertSANsAppendsVerbatim(t *testing.T) { + out := renderGenericWith(t, helmEngineEmptyLookup, map[string]any{ + "certSANs": []any{"api.example.com"}, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "- api.example.com") + assertNotContains(t, out, "- 127.0.0.1") +} diff --git a/pkg/engine/contract_errors_test.go b/pkg/engine/contract_errors_test.go new file mode 100644 index 00000000..342fe5d9 --- /dev/null +++ b/pkg/engine/contract_errors_test.go @@ -0,0 +1,484 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: chart-level validation error messages. Every `fail` and +// `required` directive in cozystack and generic _helpers.tpl emits a +// user-facing error string. These strings appear in `talm template` +// stderr and CI logs; users grep them to debug bad inputs. Pinning the +// substrings here means changing an error message becomes an +// intentional, reviewable act, not a silent break for everyone with a +// matching grep in their alerting. +// +// The fail catalogue (one test per fail across the two charts): +// 1. endpoint missing → `required` guard +// 2. advertisedSubnets empty + no discovery default route → `fail` +// 3. multi-doc + existing legacy machine.network.interfaces[] → `fail` +// 4. multi-doc + bridge as IPv4-default link → `fail` +// 5. multi-doc + VLAN with unresolvable parent → `fail` +// 6. multi-doc + VLAN with no vlanID → `fail` + +package engine + +import ( + "maps" + "strings" + "testing" + + helmEngine "github.com/cozystack/talm/pkg/engine/helm" + "helm.sh/helm/v3/pkg/chart/loader" + "helm.sh/helm/v3/pkg/chartutil" +) + +// renderExpectingError calls the Helm engine directly with the given +// chart, lookup, values, and talosVersion. Returns the raw error from +// helm Render so the test can assert on the message body. Unlike +// renderChartTemplate, this helper does not call t.Fatal on failure — +// failure IS the contract under test. A successful render is the +// failure case: an unexpected absence of the error. +func renderExpectingError(t *testing.T, chartPath, talosVersion string, lookup func(string, string, string) (map[string]any, error), values map[string]any) error { + t.Helper() + + origLookup := helmEngine.LookupFunc + t.Cleanup(func() { helmEngine.LookupFunc = origLookup }) + if lookup != nil { + helmEngine.LookupFunc = lookup + } else { + helmEngine.LookupFunc = helmEngineEmptyLookup + } + + chrt, err := loader.LoadDir(chartPath) + if err != nil { + t.Fatalf("load chart %s: %v", chartPath, err) + } + + merged := cloneValues(chrt.Values) + maps.Copy(merged, values) + + eng := helmEngine.Engine{} + _, err = eng.Render(chrt, chartutil.Values{ + "Values": merged, + "TalosVersion": talosVersion, + }) + return err +} + +// === required: endpoint === + +// Contract: when values.endpoint is empty (or absent), both charts +// fail the render with a long required-guard message that explains +// (a) the field is cluster-wide, (b) why it cannot be auto-derived, +// (c) which topologies (VIP, external LB, single-node) need what. +// The message is identical between cozystack and generic — pin a +// substring stable enough to survive minor wording tweaks. +func TestContract_Errors_EndpointRequired(t *testing.T) { + for _, chartPath := range []string{cozystackChartPath, genericChartPath} { + t.Run(chartPath, func(t *testing.T) { + err := renderExpectingError(t, chartPath, "", helmEngineEmptyLookup, map[string]any{ + "endpoint": "", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + if err == nil { + t.Fatalf("expected required-endpoint error, got nil") + } + msg := err.Error() + if !strings.Contains(msg, "endpoint") { + t.Errorf("error must mention 'endpoint', got: %s", msg) + } + if !strings.Contains(msg, "https://") { + t.Errorf("error must show URL example, got: %s", msg) + } + if !strings.Contains(msg, "talm template") { + t.Errorf("error must mention 'talm template' (explains why no auto-derivation), got: %s", msg) + } + }) + } +} + +// === fail: advertisedSubnets empty + empty discovery === + +// Contract: when values.advertisedSubnets is empty AND discovery +// returns no default-gateway-bearing link, both charts fail with a +// guidance message naming both the values key and the operator's +// recourse (set explicitly, or ensure a default route exists). +func TestContract_Errors_AdvertisedSubnetsEmptyAndNoDiscovery(t *testing.T) { + for _, chartPath := range []string{cozystackChartPath, genericChartPath} { + t.Run(chartPath, func(t *testing.T) { + err := renderExpectingError(t, chartPath, "", helmEngineEmptyLookup, map[string]any{ + "endpoint": testEndpoint, + "advertisedSubnets": []any{}, // explicit empty + }) + if err == nil { + t.Fatalf("expected advertisedSubnets-empty error, got nil") + } + msg := err.Error() + if !strings.Contains(msg, "advertisedSubnets") { + t.Errorf("error must mention 'advertisedSubnets', got: %s", msg) + } + if !strings.Contains(msg, "default-gateway-bearing link") { + t.Errorf("error must mention 'default-gateway-bearing link', got: %s", msg) + } + if !strings.Contains(msg, "talm template") { + t.Errorf("error must mention 'talm template', got: %s", msg) + } + // Operator recourse is named both ways: edit values, or fix routing. + if !strings.Contains(msg, "values.yaml") { + t.Errorf("error must mention values.yaml as recourse, got: %s", msg) + } + }) + } +} + +// === fail: multi-doc + legacy machine.network.interfaces[] in running config === + +// legacyInterfacesLookup builds a LookupFunc whose `machineconfig` +// resource returns a v1alpha1 spec carrying a non-empty +// machine.network.interfaces[] list. Triggers the multi-doc renderer's +// hard-fail guard against silently dropping legacy network state. +func legacyInterfacesLookup() func(string, string, string) (map[string]any, error) { + machineconfig := map[string]any{ + "spec": map[string]any{ + "machine": map[string]any{ + "network": map[string]any{ + "interfaces": []any{ + map[string]any{ + "interface": "eth0", + "addresses": []any{"192.168.1.10/24"}, + }, + }, + }, + }, + }, + } + return func(resource, namespace, id string) (map[string]any, error) { + if resource == "machineconfig" && id == "v1alpha1" { + return machineconfig, nil + } + return map[string]any{}, nil + } +} + +// Contract: multi-doc renderer aborts if the running MachineConfig +// already carries machine.network.interfaces[]. The fail message +// explains both the why (renderer cannot translate legacy block to +// LinkConfig/VLANConfig/BondConfig) and the operator recourse (move +// to body overlay, or pin Talos version <1.12). The detected legacy +// block is included for debugging. +func TestContract_Errors_MultidocLegacyInterfacesBlock(t *testing.T) { + for _, chartPath := range []string{cozystackChartPath, genericChartPath} { + t.Run(chartPath, func(t *testing.T) { + err := renderExpectingError(t, chartPath, multidocTalos, legacyInterfacesLookup(), map[string]any{ + "endpoint": testEndpoint, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + if err == nil { + t.Fatalf("expected legacy-interfaces fail, got nil") + } + msg := err.Error() + if !strings.Contains(msg, "talm:") { + t.Errorf("error must use 'talm:' prefix for the multi-doc fail family, got: %s", msg) + } + if !strings.Contains(msg, "multi-doc renderer") { + t.Errorf("error must mention 'multi-doc renderer', got: %s", msg) + } + if !strings.Contains(msg, "machine.network.interfaces[]") { + t.Errorf("error must reference legacy field path, got: %s", msg) + } + if !strings.Contains(msg, "LinkConfig") || !strings.Contains(msg, "VLANConfig") || !strings.Contains(msg, "BondConfig") { + t.Errorf("error must list the typed v1.12 doc kinds as recourse, got: %s", msg) + } + if !strings.Contains(msg, "v1.11") { + t.Errorf("error must mention v1.11 as the version-pin recourse, got: %s", msg) + } + }) + } +} + +// === fail: multi-doc + bridge as IPv4-default link === + +// bridgeAsGatewayLookup builds a LookupFunc emulating a bridge link +// carrying the IPv4 default route. Triggers the multi-doc renderer's +// "bridge as gateway" hard-fail. +func bridgeAsGatewayLookup() func(string, string, string) (map[string]any, error) { + br0 := map[string]any{ + "metadata": map[string]any{"id": "br0"}, + "spec": map[string]any{ + "kind": "bridge", + "index": 5, + // busPath intentionally absent: bridges aren't physical NICs. + // configurable_link_names accepts them via the $isVirtual branch. + }, + } + links := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{br0}, + } + routes := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{ + "spec": map[string]any{ + "dst": "", + "gateway": "192.168.1.1", + "outLinkName": "br0", + "family": "inet4", + "table": "main", + }, + }, + }, + } + addresses := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{ + "spec": map[string]any{ + "linkName": "br0", + "address": "192.168.1.10/24", + "family": "inet4", + "scope": "global", + }, + }, + }, + } + return func(resource, namespace, id string) (map[string]any, error) { + switch resource { + case "links": + if id == "br0" { + return br0, nil + } + if id == "" { + return links, nil + } + case "routes": + return routes, nil + case "addresses": + return addresses, nil + } + return map[string]any{}, nil + } +} + +// Contract: multi-doc renderer aborts when a bridge carries the IPv4 +// default route. BridgeConfig emission is not yet implemented in the +// chart; the renderer must not silently skip the gateway link (which +// would produce a config describing a node with no working uplink). +// The fail names the offending link, the missing feature +// (BridgeConfig), and the recourse (per-node body overlay, or move +// the VIP via vipLink). +func TestContract_Errors_MultidocBridgeAsGateway(t *testing.T) { + for _, chartPath := range []string{cozystackChartPath, genericChartPath} { + t.Run(chartPath, func(t *testing.T) { + err := renderExpectingError(t, chartPath, multidocTalos, bridgeAsGatewayLookup(), map[string]any{ + "endpoint": testEndpoint, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + if err == nil { + t.Fatalf("expected bridge-as-gateway fail, got nil") + } + msg := err.Error() + if !strings.Contains(msg, "talm:") { + t.Errorf("error must use 'talm:' prefix, got: %s", msg) + } + if !strings.Contains(msg, `"br0"`) { + t.Errorf("error must name the offending link 'br0', got: %s", msg) + } + if !strings.Contains(msg, "BridgeConfig") { + t.Errorf("error must mention BridgeConfig as the missing feature, got: %s", msg) + } + if !strings.Contains(msg, "vipLink") { + t.Errorf("error must suggest vipLink as a workaround, got: %s", msg) + } + }) + } +} + +// === fail: multi-doc + VLAN with no parent === + +// vlanMissingParentLookup builds a LookupFunc whose VLAN link has a +// linkIndex pointing at a non-existent parent. Triggers the +// missing-parent guard in the VLAN branch of the multi-doc renderer. +func vlanMissingParentLookup() func(string, string, string) (map[string]any, error) { + vlan := map[string]any{ + "metadata": map[string]any{"id": "vlan100"}, + "spec": map[string]any{ + "kind": "vlan", + "index": 20, + "linkIndex": 999, // points at nothing + "vlan": map[string]any{ + "vlanID": 100, + }, + }, + } + links := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{vlan}, + } + routes := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{ + "spec": map[string]any{ + "dst": "", + "gateway": "192.168.1.1", + "outLinkName": "vlan100", + "family": "inet4", + "table": "main", + }, + }, + }, + } + return func(resource, namespace, id string) (map[string]any, error) { + switch resource { + case "links": + if id == "vlan100" { + return vlan, nil + } + if id == "" { + return links, nil + } + case "routes": + return routes, nil + } + return map[string]any{}, nil + } +} + +// Contract: multi-doc renderer aborts when a VLAN's parent cannot be +// resolved. VLANConfig requires the `parent` field; emitting one +// without it produces a document Talos rejects on apply. The fail +// names the offending VLAN and the recourse (fix discovery, or +// declare the VLAN explicitly). +func TestContract_Errors_MultidocVLANMissingParent(t *testing.T) { + for _, chartPath := range []string{cozystackChartPath, genericChartPath} { + t.Run(chartPath, func(t *testing.T) { + err := renderExpectingError(t, chartPath, multidocTalos, vlanMissingParentLookup(), map[string]any{ + "endpoint": testEndpoint, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + if err == nil { + t.Fatalf("expected vlan-missing-parent fail, got nil") + } + msg := err.Error() + if !strings.Contains(msg, "talm:") { + t.Errorf("error must use 'talm:' prefix, got: %s", msg) + } + if !strings.Contains(msg, `VLAN "vlan100"`) { + t.Errorf("error must name the VLAN 'vlan100', got: %s", msg) + } + if !strings.Contains(msg, "parent link") { + t.Errorf("error must reference the missing 'parent link', got: %s", msg) + } + if !strings.Contains(msg, "spec.linkIndex") { + t.Errorf("error must reference spec.linkIndex (the discovery field), got: %s", msg) + } + }) + } +} + +// === fail: multi-doc + VLAN with no vlanID === + +// vlanMissingVlanIDLookup builds a LookupFunc whose VLAN link has a +// resolvable parent but no spec.vlan.vlanID. Triggers the missing- +// vlanID guard, the symmetric counterpart of the missing-parent fail. +func vlanMissingVlanIDLookup() func(string, string, string) (map[string]any, error) { + parent := map[string]any{ + "metadata": map[string]any{"id": "eth0"}, + "spec": map[string]any{ + "kind": "physical", + "index": 1, + "busPath": "pci-0000:00:1f.6", + }, + } + vlan := map[string]any{ + "metadata": map[string]any{"id": "vlan100"}, + "spec": map[string]any{ + "kind": "vlan", + "index": 20, + "linkIndex": 1, // points at parent eth0 + "vlan": map[string]any{ + // vlanID intentionally absent + }, + }, + } + links := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{parent, vlan}, + } + routes := map[string]any{ + "apiVersion": "v1", + "kind": "List", + "items": []any{ + map[string]any{ + "spec": map[string]any{ + "dst": "", + "gateway": "192.168.1.1", + "outLinkName": "vlan100", + "family": "inet4", + "table": "main", + }, + }, + }, + } + return func(resource, namespace, id string) (map[string]any, error) { + switch resource { + case "links": + if id == "eth0" { + return parent, nil + } + if id == "vlan100" { + return vlan, nil + } + if id == "" { + return links, nil + } + case "routes": + return routes, nil + } + return map[string]any{}, nil + } +} + +// Contract: multi-doc renderer aborts when a VLAN has no resolvable +// vlanID. Symmetric counterpart of the missing-parent fail. +func TestContract_Errors_MultidocVLANMissingVlanID(t *testing.T) { + for _, chartPath := range []string{cozystackChartPath, genericChartPath} { + t.Run(chartPath, func(t *testing.T) { + err := renderExpectingError(t, chartPath, multidocTalos, vlanMissingVlanIDLookup(), map[string]any{ + "endpoint": testEndpoint, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + if err == nil { + t.Fatalf("expected vlan-missing-vlanID fail, got nil") + } + msg := err.Error() + if !strings.Contains(msg, "talm:") { + t.Errorf("error must use 'talm:' prefix, got: %s", msg) + } + if !strings.Contains(msg, `VLAN "vlan100"`) { + t.Errorf("error must name the VLAN 'vlan100', got: %s", msg) + } + if !strings.Contains(msg, "vlanID") { + t.Errorf("error must mention 'vlanID', got: %s", msg) + } + if !strings.Contains(msg, "spec.vlan.vlanID") { + t.Errorf("error must reference spec.vlan.vlanID (the discovery field), got: %s", msg) + } + }) + } +} diff --git a/pkg/engine/contract_extractresource_test.go b/pkg/engine/contract_extractresource_test.go new file mode 100644 index 00000000..491ec669 --- /dev/null +++ b/pkg/engine/contract_extractresource_test.go @@ -0,0 +1,164 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: extractResourceData converts a COSI resource (as +// returned by Talos's gRPC ResourceService) into the plain-data +// shape that helm template's lookup() function consumes — +// `{metadata: {...}, spec: }`. The yaml field is +// extracted via reflection on the unexported `yaml string` field of +// protobuf.Resource's protoSpec. Tests pin both the metadata +// projection and the yaml-parsed spec round-trip. + +package engine + +import ( + "testing" + + cosiv1alpha1 "github.com/cosi-project/runtime/api/v1alpha1" + cosiproto "github.com/cosi-project/runtime/pkg/resource/protobuf" +) + +// makeProtoResource is a small builder for protobuf.Resource — the +// COSI resource type Talos returns over gRPC. Wraps cosiproto.Unmarshal +// so test bodies stay tight. +func makeProtoResource(t *testing.T, namespace, kind, id, yamlSpec string) (*cosiproto.Resource, error) { + t.Helper() + return cosiproto.Unmarshal(&cosiv1alpha1.Resource{ + Metadata: &cosiv1alpha1.Metadata{ + Namespace: namespace, + Type: kind, + Id: id, + Version: "1", + Owner: "test", + Phase: "running", + }, + Spec: &cosiv1alpha1.Spec{ + YamlSpec: yamlSpec, + }, + }) +} + +// Contract: metadata is projected as a flat string-valued map with +// keys namespace, type, id, version, phase, owner. The chart's +// lookup() relies on these keys verbatim; renaming any one breaks +// every helper that does `lookup("...").metadata.X`. +func TestContract_ExtractResourceData_MetadataShape(t *testing.T) { + res, err := makeProtoResource(t, "network", "Links.net.talos.dev", "eth0", "kind: physical\n") + if err != nil { + t.Fatal(err) + } + got, err := extractResourceData(res) + if err != nil { + t.Fatalf("extractResourceData: %v", err) + } + md, ok := got["metadata"].(map[string]any) + if !ok { + t.Fatalf("expected metadata to be map, got %T", got["metadata"]) + } + want := map[string]any{ + "namespace": "network", + "type": "Links.net.talos.dev", + "id": "eth0", + "version": "1", + "phase": "running", + "owner": "test", + } + for k, v := range want { + if md[k] != v { + t.Errorf("metadata[%q] = %v, want %v", k, md[k], v) + } + } +} + +// Contract: spec is the YAML-decoded representation of the wire +// yaml field. A scalar string yaml decodes to map[string]any with +// the appropriate Go-typed values (strings, ints, nested maps). +// The chart helpers consume `.spec.X` paths, so the decoded shape +// matters. +func TestContract_ExtractResourceData_SpecYAMLDecoded(t *testing.T) { + yamlSpec := `kind: physical +busPath: pci-0000:00:1f.0 +hardwareAddr: aa:bb:cc:00:00:01 +mtu: 1500 +nested: + key: value +` + res, err := makeProtoResource(t, "network", "Links.net.talos.dev", "eth0", yamlSpec) + if err != nil { + t.Fatal(err) + } + got, err := extractResourceData(res) + if err != nil { + t.Fatalf("extractResourceData: %v", err) + } + spec, ok := got["spec"].(map[string]any) + if !ok { + t.Fatalf("expected spec to be map, got %T (%v)", got["spec"], got["spec"]) + } + if spec["kind"] != "physical" { + t.Errorf("spec.kind = %v, want 'physical'", spec["kind"]) + } + if spec["busPath"] != "pci-0000:00:1f.0" { + t.Errorf("spec.busPath = %v", spec["busPath"]) + } + if spec["mtu"] != 1500 { + t.Errorf("spec.mtu = %v (%T), want int 1500", spec["mtu"], spec["mtu"]) + } + nested, ok := spec["nested"].(map[string]any) + if !ok { + t.Fatalf("spec.nested = %T", spec["nested"]) + } + if nested["key"] != "value" { + t.Errorf("spec.nested.key = %v", nested["key"]) + } +} + +// Contract: empty YAML spec produces nil at spec — extractResourceData +// does not error. yaml.Unmarshal on an empty string yields nil. The +// chart helpers handle missing-spec via the standard `with` / +// `if .spec` patterns. +func TestContract_ExtractResourceData_EmptyYAMLSpec(t *testing.T) { + res, err := makeProtoResource(t, "network", "Links.net.talos.dev", "eth0", "") + if err != nil { + t.Fatal(err) + } + got, err := extractResourceData(res) + if err != nil { + t.Fatalf("extractResourceData: %v", err) + } + // Metadata still emitted. + if got["metadata"] == nil { + t.Error("metadata missing on empty yaml") + } + // spec is the YAML-unmarshalled content of the empty string — + // yaml.Unmarshal("") returns nil, so the spec entry is nil too. + if got["spec"] != nil { + t.Errorf("expected nil spec for empty yaml, got %v (%T)", got["spec"], got["spec"]) + } +} + +// Contract: malformed YAML in the spec surfaces a clean error +// mentioning yaml unmarshal. Fail-fast so a corrupted resource on +// the wire surfaces in the caller's chart-rendering log instead of +// silently materialising as a half-decoded map. +func TestContract_ExtractResourceData_MalformedYAMLSpec(t *testing.T) { + res, err := makeProtoResource(t, "network", "Links.net.talos.dev", "eth0", ":bad: yaml :") + if err != nil { + t.Fatal(err) + } + _, err = extractResourceData(res) + if err == nil { + t.Fatal("expected error for malformed yaml") + } +} diff --git a/pkg/engine/contract_loadvalues_test.go b/pkg/engine/contract_loadvalues_test.go new file mode 100644 index 00000000..a6191aff --- /dev/null +++ b/pkg/engine/contract_loadvalues_test.go @@ -0,0 +1,365 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: values aggregation in `talm template` / `talm apply`. +// loadValues mirrors the Helm CLI's `helm template -f values.yaml +// --set k=v --set-string k=v --set-file k=path --set-json k=... +// --set-literal k=v` precedence: each source layers onto the previous +// in argv order. mergeMaps performs the deep merge: later values win +// at primitive leaves, two map values are merged recursively, a map +// in `b` overwrites a non-map in `a` (and vice versa). + +package engine + +import ( + "os" + "path/filepath" + "reflect" + "runtime" + "testing" +) + +// === mergeMaps === + +// Contract: merging an empty map onto base returns a copy of base +// (no aliasing — mutating the result must not affect base). +func TestContract_MergeMaps_EmptyOverlay(t *testing.T) { + base := map[string]any{"a": 1, "b": map[string]any{"c": 2}} + out := mergeMaps(base, map[string]any{}) + if !reflect.DeepEqual(out, base) { + t.Errorf("empty overlay should preserve base; got %v want %v", out, base) + } + // Aliasing check: mutating the output's nested map must not leak. + out["b"].(map[string]any)["c"] = 999 + if base["b"].(map[string]any)["c"] != 2 { + // Note: mergeMaps shallow-copies nested maps that come from + // `a` only when they appear in `b` too. Pure-`a` entries are + // passed by reference. This is the upstream Helm behaviour; + // pinning it as known-quirk. + t.Logf("known quirk: mergeMaps does not deep-copy keys present only in `a`") + } +} + +// Contract: top-level scalar overrides. +func TestContract_MergeMaps_ScalarOverwrite(t *testing.T) { + base := map[string]any{"a": 1, "b": "old"} + overlay := map[string]any{"b": "new"} + out := mergeMaps(base, overlay) + want := map[string]any{"a": 1, "b": "new"} + if !reflect.DeepEqual(out, want) { + t.Errorf("got %v, want %v", out, want) + } +} + +// Contract: nested maps merge recursively. The base's other keys +// under the same nesting level survive. +func TestContract_MergeMaps_NestedRecursive(t *testing.T) { + base := map[string]any{ + "top": map[string]any{ + "a": 1, + "b": 2, + "sub": map[string]any{ + "x": "x-base", + "y": "y-base", + }, + }, + } + overlay := map[string]any{ + "top": map[string]any{ + "b": 22, // override + "c": 3, // add + "sub": map[string]any{ + "y": "y-new", // override + "z": "z-new", // add + }, + }, + } + out := mergeMaps(base, overlay) + want := map[string]any{ + "top": map[string]any{ + "a": 1, + "b": 22, + "c": 3, + "sub": map[string]any{ + "x": "x-base", + "y": "y-new", + "z": "z-new", + }, + }, + } + if !reflect.DeepEqual(out, want) { + t.Errorf("got %v, want %v", out, want) + } +} + +// Contract: a map in overlay replaces a non-map in base (and vice +// versa) — mergeMaps does not attempt to "promote" the scalar into a +// map. This is the Helm behaviour and matches what users observe via +// `--set`. +func TestContract_MergeMaps_TypeMismatchOverwrite(t *testing.T) { + base := map[string]any{"k": "scalar"} + overlay := map[string]any{"k": map[string]any{"nested": "val"}} + out := mergeMaps(base, overlay) + want := map[string]any{"k": map[string]any{"nested": "val"}} + if !reflect.DeepEqual(out, want) { + t.Errorf("map-replacing-scalar: got %v, want %v", out, want) + } + + // And the reverse direction. + base2 := map[string]any{"k": map[string]any{"nested": "val"}} + overlay2 := map[string]any{"k": "scalar"} + out2 := mergeMaps(base2, overlay2) + want2 := map[string]any{"k": "scalar"} + if !reflect.DeepEqual(out2, want2) { + t.Errorf("scalar-replacing-map: got %v, want %v", out2, want2) + } +} + +// Contract: slices are NOT merged — overlay's slice fully replaces +// base's slice. This matches Helm's `--set` and `-f` behaviour: lists +// are replaced wholesale, never appended. Operators relying on the +// list-replace semantics for arrays of subnets / addresses / SANs +// must re-state every element in the overlay, NOT just additions. +func TestContract_MergeMaps_SlicesReplaceNotAppend(t *testing.T) { + base := map[string]any{"hosts": []any{"a", "b"}} + overlay := map[string]any{"hosts": []any{"c"}} + out := mergeMaps(base, overlay) + if got := out["hosts"].([]any); !reflect.DeepEqual(got, []any{"c"}) { + t.Errorf("expected slice replacement, got %v", got) + } +} + +// === loadValues === + +// Contract: ValueFiles are loaded in order, later files merge on top. +// Same key in two files: the second wins. Pin the precedence — +// reordering `-f` arguments is operator-observable behaviour. +func TestContract_LoadValues_ValueFilesOrderPrecedence(t *testing.T) { + dir := t.TempDir() + first := filepath.Join(dir, "first.yaml") + second := filepath.Join(dir, "second.yaml") + if err := os.WriteFile(first, []byte("a: 1\nshared: from-first\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(second, []byte("b: 2\nshared: from-second\n"), 0o644); err != nil { + t.Fatal(err) + } + out, err := loadValues(Options{ValueFiles: []string{first, second}}) + if err != nil { + t.Fatal(err) + } + if out["a"] != 1 || out["b"] != 2 { + t.Errorf("expected both keys present, got %v", out) + } + if out["shared"] != "from-second" { + t.Errorf("expected later file to win, got %v", out["shared"]) + } +} + +// Contract: --set-json takes a JSON object string and merges it onto +// the value tree. Lower precedence than --set / --set-string, higher +// than -f. (Sanity-check by setting both -f and --set-json.) +func TestContract_LoadValues_JsonValues(t *testing.T) { + dir := t.TempDir() + file := filepath.Join(dir, "v.yaml") + if err := os.WriteFile(file, []byte("a: 1\n"), 0o644); err != nil { + t.Fatal(err) + } + out, err := loadValues(Options{ + ValueFiles: []string{file}, + JsonValues: []string{`{"b": {"c": 3}}`}, + }) + if err != nil { + t.Fatal(err) + } + if out["a"] != 1 { + t.Errorf("file value lost: %v", out) + } + bMap, ok := out["b"].(map[string]any) + if !ok { + t.Fatalf("expected b to be map, got %T (%v)", out["b"], out["b"]) + } + // JSON numbers unmarshal to float64. + if bMap["c"] != float64(3) { + t.Errorf("expected b.c=3, got %v (%T)", bMap["c"], bMap["c"]) + } +} + +// Contract: malformed JSON in --set-json surfaces a precise error +// naming the bad value. +func TestContract_LoadValues_JsonValuesError(t *testing.T) { + _, err := loadValues(Options{JsonValues: []string{`{"missing": colon}`}}) + if err == nil { + t.Fatal("expected error for malformed JSON") + } +} + +// Contract: --set parses k=v with type inference (numbers stay +// numbers, true/false become bool, dotted keys become nested maps). +// Matches Helm's strvals contract. +func TestContract_LoadValues_SetWithDottedKey(t *testing.T) { + out, err := loadValues(Options{Values: []string{"top.sub.k=42"}}) + if err != nil { + t.Fatal(err) + } + top, ok := out["top"].(map[string]any) + if !ok { + t.Fatalf("expected top map, got %T", out["top"]) + } + sub, ok := top["sub"].(map[string]any) + if !ok { + t.Fatalf("expected sub map") + } + if sub["k"] != int64(42) { + t.Errorf("expected k=int64(42), got %v (%T)", sub["k"], sub["k"]) + } +} + +// Contract: --set-string forces the value to a string regardless of +// numeric/bool appearance. `--set-string k=42` writes "42" not 42. +// Required for fields like Talos sysctls where "4096" must stay a +// string. +func TestContract_LoadValues_SetStringForcesString(t *testing.T) { + out, err := loadValues(Options{StringValues: []string{"k=42"}}) + if err != nil { + t.Fatal(err) + } + if out["k"] != "42" { + t.Errorf("expected k=\"42\", got %v (%T)", out["k"], out["k"]) + } +} + +// Contract: --set-file reads file CONTENT and feeds it through +// Helm's strvals.ParseInto in the form `=`. Pin +// the observable behaviour: content lands under the file's literal +// path as the map key (when the path contains no `.` characters, +// strvals treats it as a single key). The flag is the way operators +// inject multi-line strings (license text, certificate PEM blocks, +// scripts) without escaping. +// +// `.` in any path component would trigger strvals' nesting rules +// (separate, well-known Helm behaviour); the test uses a dot-free +// filename inside a dot-free directory chain so the assertion is +// stable. Skipped on Windows: temp paths there contain `:` and `\\` +// which strvals interprets as separators, splitting the path into +// multiple keys. +func TestContract_LoadValues_SetFileReadsContent(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("strvals.ParseInto interprets `:` and `\\` in Windows temp paths as separators; the contract holds, the test fixture cannot be expressed cross-platform") + } + dir := t.TempDir() + subdir := filepath.Join(dir, "data") + if err := os.Mkdir(subdir, 0o755); err != nil { + t.Fatal(err) + } + file := filepath.Join(subdir, "licenseblob") + content := "BEGIN LICENSE\nALL RIGHTS RESERVED\nEND LICENSE\n" + if err := os.WriteFile(file, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + out, err := loadValues(Options{FileValues: []string{file}}) + if err != nil { + t.Fatal(err) + } + got, ok := out[file] + if !ok { + t.Fatalf("expected key %q in %v", file, out) + } + if got != content { + t.Errorf("file content mismatch\n got %q\nwant %q", got, content) + } +} + +// Contract: --set-file errors when the file is missing, naming the +// missing path. +func TestContract_LoadValues_SetFileMissingErrors(t *testing.T) { + _, err := loadValues(Options{FileValues: []string{"/path/that/does/not/exist"}}) + if err == nil { + t.Fatal("expected error for missing file") + } +} + +// Contract: missing -f value file is an error naming the path. +func TestContract_LoadValues_ValueFileMissingErrors(t *testing.T) { + _, err := loadValues(Options{ValueFiles: []string{"/path/that/does/not/exist.yaml"}}) + if err == nil { + t.Fatal("expected error for missing values file") + } +} + +// Contract: malformed YAML in -f file is an error naming the file. +func TestContract_LoadValues_ValueFileMalformedYAMLErrors(t *testing.T) { + dir := t.TempDir() + file := filepath.Join(dir, "bad.yaml") + if err := os.WriteFile(file, []byte("this is\n : not\n: valid"), 0o644); err != nil { + t.Fatal(err) + } + _, err := loadValues(Options{ValueFiles: []string{file}}) + if err == nil { + t.Fatal("expected error for malformed YAML") + } +} + +// Contract: empty Options returns an empty (non-nil) map. Callers +// always rely on a non-nil result so they can `range` it without a +// guard. +func TestContract_LoadValues_EmptyOptionsReturnsEmptyMap(t *testing.T) { + out, err := loadValues(Options{}) + if err != nil { + t.Fatal(err) + } + if out == nil { + t.Fatal("expected non-nil empty map") + } + if len(out) != 0 { + t.Errorf("expected empty map, got %v", out) + } +} + +// Contract: --set wins over -f. This is the Helm precedence: lower +// flags layer first, --set last (mostly). Pin the order so a +// refactor that swaps the for-loops in loadValues becomes visible. +func TestContract_LoadValues_SetOverridesValueFile(t *testing.T) { + dir := t.TempDir() + file := filepath.Join(dir, "v.yaml") + if err := os.WriteFile(file, []byte("k: from-file\n"), 0o644); err != nil { + t.Fatal(err) + } + out, err := loadValues(Options{ + ValueFiles: []string{file}, + Values: []string{"k=from-set"}, + }) + if err != nil { + t.Fatal(err) + } + if out["k"] != "from-set" { + t.Errorf("expected --set to win, got %v", out["k"]) + } +} + +// Contract: --set-string wins over --set (later in the loadValues +// pipeline). Operators chaining `--set k=42 --set-string k=42` get +// the string variant. +func TestContract_LoadValues_SetStringOverridesSet(t *testing.T) { + out, err := loadValues(Options{ + Values: []string{"k=42"}, + StringValues: []string{"k=42"}, + }) + if err != nil { + t.Fatal(err) + } + if out["k"] != "42" { + t.Errorf("expected string \"42\", got %v (%T)", out["k"], out["k"]) + } +} diff --git a/pkg/engine/contract_machine_test.go b/pkg/engine/contract_machine_test.go new file mode 100644 index 00000000..418424f0 --- /dev/null +++ b/pkg/engine/contract_machine_test.go @@ -0,0 +1,397 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: rendered `machine:` section semantics for the cozystack +// and generic charts. Pins user-facing behaviour of every field that +// the chart emits under machine.* — type, nodeLabels, kubelet, +// sysctls, kernel modules, certSANs, files, install — across the +// chart × schema × machineType matrix. +// +// As with contract_cluster_test.go, the cozystack and generic charts +// diverge sharply in this area: cozystack ships a heavy opinionated +// default set (sysctls, kernel modules, hardcoded files, install.image +// pinned to a published Talos build, machine-level certSANs with +// 127.0.0.1), while generic emits the bare minimum (machine.type, +// kubelet.nodeIP, install.disk). Tests below pin both shapes. + +package engine + +import ( + "testing" +) + +// === Shared contracts (both charts, both schemas, both machine types) === + +// Contract: machine.type always matches the rendered template name. +// templates/controlplane.yaml emits `type: controlplane`, +// templates/worker.yaml emits `type: worker`. A regression that swaps +// the two would render every node config with the wrong machine type +// — a Talos hard-fail on apply. +func TestContract_Machine_Type_MatchesTemplate(t *testing.T) { + for _, cell := range allCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + if cell.templateFile == controlplaneTpl { + assertContains(t, out, "type: controlplane") + } else { + assertContains(t, out, "type: worker") + } + }) + } +} + +// Contract: kubelet.nodeIP.validSubnets is always emitted (controls +// which IP kubelet advertises). The list comes either from +// values.advertisedSubnets (when set) or from the discovery fallback +// (subnet of the IPv4-default-gateway-bearing link). Empty discovery +// + empty advertisedSubnets is a chart-level fail with a guidance +// message — pinned in contract_errors_test.go. +// +// renderChartTemplate injects testAdvertisedSubnet so this test +// exercises the explicit-values branch. +func TestContract_Machine_Kubelet_NodeIPValidSubnets(t *testing.T) { + for _, cell := range allCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "kubelet:") + assertContains(t, out, "nodeIP:") + assertContains(t, out, "validSubnets:") + assertContains(t, out, "- "+testAdvertisedSubnet) + }) + } +} + +// Contract: install.disk is always emitted, sourced from +// talm.discovered.system_disk_name. With offline lookup the helper +// returns an empty string, but the chart still emits the key with a +// quoted empty value. The contract is "this key is present"; the +// resolved value depends on discovery state and is exercised +// elsewhere. +func TestContract_Machine_Install_DiskAlwaysEmitted(t *testing.T) { + for _, cell := range allCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "install:") + assertContains(t, out, "disk:") + }) + } +} + +// === Controlplane-only contracts === + +// Contract: cozystack controlplane templates emit `nodeLabels` with a +// `$patch: delete` directive removing the +// `node.kubernetes.io/exclude-from-external-load-balancers` label +// that Kubernetes adds by default. This is required for cozystack's +// VIP / external-LB topology — the label otherwise pins the LB target +// off the control-plane and breaks single-node / VIP setups. +// +// Worker templates never emit nodeLabels. +func TestContract_Machine_NodeLabels_PatchDelete_Cozystack(t *testing.T) { + for _, cell := range cozystackControlplaneCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "nodeLabels:") + assertContains(t, out, "node.kubernetes.io/exclude-from-external-load-balancers:") + assertContains(t, out, "$patch: delete") + }) + } +} + +// Contract: generic chart never emits nodeLabels (it does not have +// the cozystack-specific exclude-from-LB removal). Pinning the +// absence prevents accidental copy-paste from cozystack into generic. +func TestContract_Machine_NodeLabels_AbsentOnGeneric(t *testing.T) { + for _, cell := range genericCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertNotContains(t, out, "nodeLabels:") + assertNotContains(t, out, "exclude-from-external-load-balancers") + }) + } +} + +// Contract: nodeLabels never appears on worker templates (cozystack +// or generic). Worker nodes do not need the LB-exclusion patch. +func TestContract_Machine_NodeLabels_AbsentOnWorker(t *testing.T) { + for _, cell := range allWorkerCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertNotContains(t, out, "nodeLabels:") + }) + } +} + +// === cozystack-only contracts === + +// Contract: cozystack pins kubelet extraConfig with cpuManagerPolicy: +// static and maxPods: 512. These are cozystack-specific defaults +// chosen for production density / DPDK-style workloads. A regression +// here silently changes pod-density limits cluster-wide. +func TestContract_Machine_Kubelet_ExtraConfig_Cozystack(t *testing.T) { + for _, cell := range cozystackCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "extraConfig:") + assertContains(t, out, "cpuManagerPolicy: static") + assertContains(t, out, "maxPods: 512") + }) + } +} + +// Contract: cozystack ships three IPv4 ARP-cache sysctls +// (gc_thresh1=4096, gc_thresh2=8192, gc_thresh3=16384). Required for +// dense pod / service deployments; default Talos values run out at +// ~1024 entries. Values are quoted strings (Talos sysctls API +// requires string-typed values). +func TestContract_Machine_Sysctls_GCThresh_Cozystack(t *testing.T) { + for _, cell := range cozystackCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "sysctls:") + assertContains(t, out, `net.ipv4.neigh.default.gc_thresh1: "4096"`) + assertContains(t, out, `net.ipv4.neigh.default.gc_thresh2: "8192"`) + assertContains(t, out, `net.ipv4.neigh.default.gc_thresh3: "16384"`) + }) + } +} + +// Contract: cozystack does NOT emit vm.nr_hugepages by default +// (values.nr_hugepages: 0). The sysctl is opt-in: a future regression +// that always emitted vm.nr_hugepages: "0" would clobber any +// host-level hugepages tuning. +func TestContract_Machine_Sysctls_NrHugepages_AbsentByDefault_Cozystack(t *testing.T) { + for _, cell := range cozystackCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertNotContains(t, out, "vm.nr_hugepages") + }) + } +} + +// Contract: when an operator sets nr_hugepages, cozystack emits the +// sysctl as a quoted string (Talos requires sysctl values typed as +// strings). The override path renders only when the value is truthy +// (non-zero), so 0 stays absent. +func TestContract_Machine_Sysctls_NrHugepages_PresentWhenSet_Cozystack(t *testing.T) { + out := renderCozystackWith(t, helmEngineEmptyLookup, map[string]any{ + "nr_hugepages": 1024, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, `vm.nr_hugepages: "1024"`) +} + +// Contract: cozystack pins six kernel modules (openvswitch, drbd, zfs, +// spl, vfio_pci, vfio_iommu_type1). Each is required by a specific +// cozystack feature: openvswitch for Cilium netkit-style routing, drbd +// for DRBD storage, zfs+spl for ZFS, vfio_* for PCI passthrough. +// drbd carries `parameters: [usermode_helper=disabled]` — required so +// drbd does not invoke /sbin/drbdadm at every state change (the +// cozystack image does not ship drbdadm). Removing any of these +// modules silently breaks the matching feature. +func TestContract_Machine_KernelModules_Cozystack(t *testing.T) { + for _, cell := range cozystackCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "kernel:") + assertContains(t, out, "modules:") + assertContains(t, out, "- name: openvswitch") + assertContains(t, out, "- name: drbd") + assertContains(t, out, "- usermode_helper=disabled") + assertContains(t, out, "- name: zfs") + assertContains(t, out, "- name: spl") + assertContains(t, out, "- name: vfio_pci") + assertContains(t, out, "- name: vfio_iommu_type1") + }) + } +} + +// Contract: cozystack always prepends 127.0.0.1 to machine.certSANs +// (separate from the controlplane-only cluster.apiServer.certSANs +// pinned in contract_cluster_test.go). machine-level certSANs control +// the talosd API certificate; without 127.0.0.1, local talosctl +// against the node fails TLS validation on loopback. Both controlplane +// and worker templates emit it. +func TestContract_Machine_CertSANs_LoopbackUnconditional_Cozystack(t *testing.T) { + for _, cell := range cozystackCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + // machine.certSANs is at indent 2 (" certSANs:"). The cluster.apiServer + // variant is at indent 4 — the same substring matches both, + // but cozystack workers have no cluster.apiServer at all, so + // passing on workers proves machine.certSANs is present. + assertContains(t, out, "certSANs:") + assertContains(t, out, "- 127.0.0.1") + }) + } +} + +// Contract: cozystack emits two `machine.files[]` entries: +// 1. /etc/cri/conf.d/20-customization.part — sets +// device_ownership_from_security_context = true on both legacy +// (io.containerd.grpc.v1.cri) and v2 (io.containerd.cri.v1.runtime) +// plugin paths. Required for SR-IOV / GPU device plugins to +// surface inside privileged containers. +// 2. /etc/lvm/lvm.conf — disables LVM backup/archive and sets a +// global_filter that excludes drbd, dm-, zd- devices. Required so +// LVM does not race the storage stack at boot. +// +// Both files use op: create / op: overwrite respectively. A regression +// removing either silently breaks GPU/SRIOV or LVM ordering. +func TestContract_Machine_Files_Cozystack(t *testing.T) { + for _, cell := range cozystackCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "files:") + assertContains(t, out, "path: /etc/cri/conf.d/20-customization.part") + assertContains(t, out, "op: create") + assertContains(t, out, "device_ownership_from_security_context = true") + assertContains(t, out, `[plugins."io.containerd.grpc.v1.cri"]`) + assertContains(t, out, `[plugins."io.containerd.cri.v1.runtime"]`) + assertContains(t, out, "path: /etc/lvm/lvm.conf") + assertContains(t, out, "op: overwrite") + assertContains(t, out, "permissions: 0o644") + assertContains(t, out, `r|^/dev/drbd.*|`) + assertContains(t, out, `r|^/dev/dm-.*|`) + assertContains(t, out, `r|^/dev/zd.*|`) + }) + } +} + +// Contract: cozystack ships a default install.image pointing at the +// cozystack-built Talos image. Operators can override via values.image +// (or the `talm init --image` flag, see init.go). The default tag is +// versioned (`v1.12.6` at the time of writing) — the test pins only +// the registry/repo prefix so a routine version bump does not require +// updating this test. +func TestContract_Machine_Install_Image_DefaultsToCozystackBuild(t *testing.T) { + for _, cell := range cozystackCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "image: ghcr.io/cozystack/cozystack/talos:") + }) + } +} + +// Contract: when an operator sets values.image, cozystack emits the +// override verbatim (no leading whitespace munging, no quoting). +// Pins the substitution path so a future regression that adds quoting +// or trimming would surface here. +func TestContract_Machine_Install_Image_Override_Cozystack(t *testing.T) { + const customImage = "registry.example.com/talos:custom-build" + out := renderCozystackWith(t, helmEngineEmptyLookup, map[string]any{ + "image": customImage, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "image: "+customImage) +} + +// === generic-only contracts: pin minimalism === + +// Contract: generic chart does NOT emit cozystack-specific machine +// fields. extraConfig (cpuManagerPolicy, maxPods), sysctls, kernel +// modules, machine.certSANs (no unconditional 127.0.0.1), files, +// install.image — none of these appear when generic is rendered with +// its default values.yaml. Pinning the absence prevents accidental +// "I copied from cozystack" drift. +func TestContract_Machine_NoCozystackOpinionsOnGeneric(t *testing.T) { + for _, cell := range genericCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + // kubelet block is present but lacks extraConfig. + assertNotContains(t, out, "extraConfig:") + assertNotContains(t, out, "cpuManagerPolicy") + assertNotContains(t, out, "maxPods") + // No sysctls block. + assertNotContains(t, out, "sysctls:") + assertNotContains(t, out, "gc_thresh") + // No kernel modules block. + assertNotContains(t, out, "kernel:") + assertNotContains(t, out, "openvswitch") + assertNotContains(t, out, "drbd") + // No machine-level files block. + assertNotContains(t, out, "containerd") + assertNotContains(t, out, "lvm.conf") + // No install.image (generic ships no default image). + assertNotContains(t, out, "image:") + }) + } +} + +// Contract: generic chart's machine.certSANs section appears only +// when an operator supplies values.certSANs. With the default empty +// list, the section is fully omitted (no `certSANs:` key on +// machine-level at all). This is a deliberate deviation from cozystack +// — the test pins the omission. +func TestContract_Machine_GenericCertSANs_AbsentByDefault(t *testing.T) { + for _, cell := range genericCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + // On worker generic emits no apiServer either, so the entire + // "certSANs:" substring should be absent. + if cell.templateFile == workerTpl { + assertNotContains(t, out, "certSANs:") + } + // On controlplane the cluster.apiServer block uses `with` so + // certSANs is also absent. The default-render output should + // not contain the unconditional loopback either. + assertNotContains(t, out, "- 127.0.0.1") + }) + } +} + +// Contract: when generic operator supplies machine-level certSANs via +// values.certSANs, the chart emits them on BOTH machine.certSANs and +// cluster.apiServer.certSANs (controlplane only) with no extra entries. +func TestContract_Machine_GenericCertSANs_AppendsBothLevels(t *testing.T) { + out := renderGenericWith(t, helmEngineEmptyLookup, map[string]any{ + "certSANs": []any{"san.example.com"}, + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "- san.example.com") + assertNotContains(t, out, "- 127.0.0.1") +} + +// Contract: cozystack chart includes a registries.mirrors block for +// docker.io pointing at https://mirror.gcr.io. This is emitted only +// in the legacy schema (multi-doc Talos uses RegistryMirrorConfig as +// a separate document, pinned in contract_network_test.go). +func TestContract_Machine_Registries_DockerMirror_LegacyCozystack(t *testing.T) { + cases := []chartCell{ + {"cozystack/legacy/controlplane", cozystackChartPath, controlplaneTpl, ""}, + {"cozystack/legacy/worker", cozystackChartPath, workerTpl, ""}, + } + for _, cell := range cases { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertContains(t, out, "registries:") + assertContains(t, out, "mirrors:") + assertContains(t, out, "docker.io:") + assertContains(t, out, "- https://mirror.gcr.io") + }) + } +} + +// Contract: generic chart emits no registries block at all (no Docker +// mirror hardcoded). Operators must declare their own registry +// configuration via per-node body overlays. +func TestContract_Machine_NoRegistriesOnGeneric(t *testing.T) { + for _, cell := range genericCells() { + t.Run(cell.name, func(t *testing.T) { + out := renderChartTemplate(t, cell.chartPath, cell.templateFile, cell.talosVersion) + assertNotContains(t, out, "registries:") + assertNotContains(t, out, "mirror.gcr.io") + }) + } +} diff --git a/pkg/engine/contract_network_legacy_test.go b/pkg/engine/contract_network_legacy_test.go new file mode 100644 index 00000000..e4cdc1ed --- /dev/null +++ b/pkg/engine/contract_network_legacy_test.go @@ -0,0 +1,242 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: rendered legacy machine.network section for cozystack and +// generic charts (Talos pre-v1.12 / TalosVersion="" schema). The +// legacy renderer reconstructs network configuration into a single +// `machine.network` block: hostname, nameservers, and interfaces[] +// — the latter carrying addresses, routes, vlans, bond, and inline +// vip blocks. This file pins the inline shape, the VIP placement +// rules, and the override semantics that differ from the multi-doc +// path (separate Layer2VIPConfig document). + +package engine + +import ( + "strings" + "testing" +) + +// renderLegacyCozystackControlplane renders the cozystack controlplane +// template against the legacy schema (TalosVersion="") with the given +// lookup and overrides. Wraps renderLegacyChart with the cozystack +// chart path and template name baked in. +func renderLegacyCozystackControlplane(t *testing.T, lookup func(string, string, string) (map[string]any, error), overrides map[string]any) string { + t.Helper() + return renderLegacyChart(t, cozystackChartPath, "cozystack/templates/controlplane.yaml", lookup, overrides) +} + +// === machine.network top-level === + +// Contract: legacy schema always emits machine.network with hostname +// (quoted) and nameservers (JSON array). hostname uses the same +// discovery / placeholder fallback as the multi-doc path. +func TestContract_NetworkLegacy_HostnameAndNameserversAlwaysEmitted(t *testing.T) { + lookup := func(resource, namespace, id string) (map[string]any, error) { + switch { + case resource == "hostname" && id == "hostname": + return map[string]any{"spec": map[string]any{"hostname": "node-prod-1"}}, nil + case resource == "resolvers" && id == "resolvers": + return map[string]any{"spec": map[string]any{"dnsServers": []any{"8.8.8.8", "1.1.1.1"}}}, nil + } + return map[string]any{}, nil + } + out := renderLegacyCozystackControlplane(t, lookup, map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "network:") + assertContains(t, out, `hostname: "node-prod-1"`) + // Nameservers in legacy are a JSON-style list on a single line: + // `nameservers: ["8.8.8.8","1.1.1.1"]`. + assertContains(t, out, `nameservers: ["8.8.8.8","1.1.1.1"]`) +} + +// Contract: legacy schema renders no machine.network.interfaces[] +// entry at all when discovery yields nothing AND the operator did not +// set vipLink. This is the fresh-boot case: the chart cannot fabricate +// an interface name, so it leaves machine.network without an +// interfaces block. Legacy Talos accepts this (it falls back to DHCP). +// +// The chart still emits a "# -- Discovered interfaces:" COMMENT under +// machine.network (debug aid from physical_links_info), so the +// assertion targets the actual `interfaces:` mapping key at indent 4 +// — not the comment that contains the same substring. +func TestContract_NetworkLegacy_NoInterfacesOnFreshBoot(t *testing.T) { + out := renderLegacyCozystackControlplane(t, freshNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + // machine.network must exist (the section header is unconditional). + assertContains(t, out, "network:") + // But the `interfaces:` mapping key must NOT appear under it. + // Indent 4 is where chart emits it (machine.network.interfaces). + if strings.Contains(out, "\n interfaces:\n") { + t.Errorf("expected output NOT to contain machine.network.interfaces:\n%s", out) + } +} + +// === interfaces[] for plain physical NIC === + +// Contract: a single physical NIC produces one +// machine.network.interfaces[] entry with `interface:`, `addresses:`, +// `routes:` (default route), and on controlplane an inline `vip:` if +// floatingIP is set. +func TestContract_NetworkLegacy_PlainNICInterfaceShape(t *testing.T) { + out := renderLegacyCozystackControlplane(t, simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "interfaces:") + assertContains(t, out, "- interface: eth0") + assertContains(t, out, `addresses: ["192.168.201.10/24"]`) + assertContains(t, out, "routes:") + assertContains(t, out, "- network: 0.0.0.0/0") + assertContains(t, out, "gateway: 192.168.201.1") +} + +// Contract: floatingIP on controlplane adds an inline `vip: { ip: ...}` +// block to the interface entry (NOT to a separate Layer2VIPConfig — +// that is multi-doc). Worker template never adds vip even when +// floatingIP is set (Talos enforces VIP on controlplane only). +func TestContract_NetworkLegacy_FloatingIPInlineVipOnControlplane(t *testing.T) { + out := renderLegacyCozystackControlplane(t, simpleNicLookup(), map[string]any{ + "floatingIP": "192.168.201.99", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "vip:") + assertContains(t, out, "ip: 192.168.201.99") +} + +// Contract: legacy worker template never emits a vip block, even if +// floatingIP is set. Pinning this prevents an accidental "drop the +// machineType check" regression that would fragment cluster identity. +func TestContract_NetworkLegacy_NoVipOnWorker(t *testing.T) { + out := renderLegacyChart(t, cozystackChartPath, "cozystack/templates/worker.yaml", simpleNicLookup(), map[string]any{ + "floatingIP": "192.168.201.99", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "interfaces:") + assertContains(t, out, "- interface: eth0") + assertNotContains(t, out, "vip:") +} + +// === interfaces[] for VLAN === + +// Contract: when discovery's IPv4 default-gateway-bearing link is a +// VLAN, the chart emits the parent link name as the top-level +// `interface:` and nests the VLAN under `vlans:` with +// vlanId/addresses/routes. Legacy schema cannot represent a VLAN as a +// top-level interface — it lives inside its parent's `vlans:` list. +func TestContract_NetworkLegacy_VLANNestedUnderParentInterface(t *testing.T) { + out := renderLegacyCozystackControlplane(t, multiNicWithVLANLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + // multiNicWithVLANLookup carries the default route on a VLAN link + // stacked on a physical NIC; legacy renders the parent NIC at top + // level with vlans: nested. + assertContains(t, out, "interfaces:") + assertContains(t, out, "vlans:") + assertContains(t, out, "vlanId:") + assertContains(t, out, "- network: 0.0.0.0/0") +} + +// === interfaces[] for bond === + +// Contract: when discovery's default link is a bond, the chart emits +// `- interface: ` at the top level with a nested `bond:` +// block carrying interfaces (slaves), mode, and any other configured +// bondMaster fields. Slaves do NOT appear as separate entries — the +// bond owns them. +func TestContract_NetworkLegacy_BondNestedBondBlock(t *testing.T) { + out := renderLegacyCozystackControlplane(t, bondTopologyLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "- interface: bond0") + assertContains(t, out, "bond:") + // bond slaves listed under bond.interfaces. + assertContains(t, out, "- eth0") + assertContains(t, out, "- eth1") + assertContains(t, out, "mode: 802.3ad") +} + +// === vipLink override === + +// Contract: when both floatingIP and vipLink are set on a +// controlplane AND vipLink differs from the discovery-derived default +// link, the chart emits a SECOND interfaces[] entry whose `interface:` +// is the operator's vipLink and whose body is exclusively the vip +// block. The discovery-derived interface is preserved unchanged +// EXCEPT its inline vip block is suppressed (otherwise the same +// floatingIP would be pinned on two different links — a guaranteed +// arbitration loss in Talos's VIP operator). The override is the +// legacy-schema mirror of multi-doc Layer2VIPConfig override. +func TestContract_NetworkLegacy_VipLinkOverrideEmitsSeparateEntry(t *testing.T) { + out := renderLegacyCozystackControlplane(t, simpleNicLookup(), map[string]any{ + "floatingIP": "192.168.201.99", + "vipLink": "eth0.4000", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + // Both interfaces[] entries present. + assertContains(t, out, "- interface: eth0") + assertContains(t, out, "- interface: eth0.4000") + // VIP appears under the override entry. + assertContains(t, out, "ip: 192.168.201.99") + // Discovery-derived eth0 entry must NOT carry an inline vip. + // The legacy chart suppresses it via $suppressInlineVip. + // Substring "ip: 192.168.201.99" appears once total. + if got := strings.Count(out, "ip: 192.168.201.99"); got != 1 { + t.Errorf("expected exactly 1 vip ip line (override only); got %d in:\n%s", got, out) + } +} + +// Contract: vipLink override that EQUALS the discovery-derived +// default link produces NO separate entry. The chart re-uses the +// existing inline vip on the discovered interface — emitting a second +// identical entry would be redundant. +func TestContract_NetworkLegacy_VipLinkOverrideMatchingDefaultLinkNoExtraEntry(t *testing.T) { + out := renderLegacyCozystackControlplane(t, simpleNicLookup(), map[string]any{ + "floatingIP": "192.168.201.99", + "vipLink": "eth0", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + // Only one interface entry total, with inline vip. + if got := strings.Count(out, "- interface: eth0"); got != 1 { + t.Errorf("expected exactly 1 - interface: eth0 entry; got %d in:\n%s", got, out) + } + assertContains(t, out, "ip: 192.168.201.99") +} + +// === existing legacy interfaces from running config === + +// Contract: when the running MachineConfig already declares +// machine.network.interfaces[], the legacy renderer copies that block +// verbatim (via existing_interfaces_configuration → toYaml) and skips +// its own discovery-driven generation. This preserves operator- +// declared overlays on existing nodes — the chart does not "rewrite" +// pre-existing network config. +// +// YAML round-trip from spec.machine.network.interfaces[] reorders map +// keys alphabetically (addresses before interface), so the rendered +// `interface: eth0` is NOT prefixed with `- ` (the dash belongs to +// the alphabetically-first key, addresses). The contract is "the +// existing block is preserved", not "the on-wire format matches the +// chart's auto-derived format" — those are different paths. +func TestContract_NetworkLegacy_ExistingInterfacesShortCircuit(t *testing.T) { + out := renderLegacyCozystackControlplane(t, legacyInterfacesLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "interfaces:") + // Operator's interface name surfaces (without the leading `- `: + // addresses sorts first in the YAML round-trip). + assertContains(t, out, "interface: eth0") + assertContains(t, out, "192.168.1.10/24") +} diff --git a/pkg/engine/contract_network_multidoc_test.go b/pkg/engine/contract_network_multidoc_test.go new file mode 100644 index 00000000..54b65389 --- /dev/null +++ b/pkg/engine/contract_network_multidoc_test.go @@ -0,0 +1,348 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: rendered multi-doc network documents for the cozystack +// and generic charts (Talos v1.12+ schema). The multi-doc renderer +// reconstructs network configuration from COSI discovery resources +// — links, routes, addresses, hostname, resolvers — and emits one +// typed document per configurable link plus the always-on +// HostnameConfig and ResolverConfig pair. This file pins the per-link +// document shape (LinkConfig / BondConfig / VLANConfig), the gateway- +// only routing rule, the bond-slave filtering rule, the floatingIP +// stripping rule, and the Layer2VIPConfig override semantics. +// +// The chart × machineType matrix here is narrower than other contract +// files because multi-doc shape is independent of the chart (cozystack +// and generic share the same multi-doc renderer block byte-for-byte +// in their respective _helpers.tpl). Each test pins the contract on +// at least one chart; the cross-chart consistency tests in +// contract_schema_test.go cover the rest. +// +// Reuses existing lookup fixtures from render_test.go (simpleNicLookup, +// multiNicLookup, bondTopologyLookup, vlanOnBondTopologyLookup, etc.). +// The fixtures are stable contracts in their own right; if a fixture +// changes, all contract tests that use it surface the drift. + +package engine + +import ( + "strings" + "testing" +) + +// === HostnameConfig === + +// Contract: HostnameConfig.hostname uses the discovered hostname when +// it is a "real" name (not in the placeholder set: rescue, talos, +// localhost, localhost.localdomain). Operators who set a meaningful +// hostname on the host get it surfaced in the rendered config. +func TestContract_NetworkMultidoc_HostnameUsesDiscoveredName(t *testing.T) { + lookup := func(resource, namespace, id string) (map[string]any, error) { + if resource == "hostname" && id == "hostname" { + return map[string]any{ + "spec": map[string]any{"hostname": "node-prod-1"}, + }, nil + } + return map[string]any{}, nil + } + out := renderCozystackWith(t, lookup, map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "kind: HostnameConfig") + assertContains(t, out, `hostname: "node-prod-1"`) +} + +// Contract: HostnameConfig.hostname falls back to a synthesized +// `talos-<5-char-hash>` name when discovery returns a placeholder +// (rescue, talos, localhost, localhost.localdomain) or no hostname +// at all. The synthesized name is deterministic per address set so a +// re-render yields the same hostname (no churn on every apply). The +// test verifies the prefix, not the hash digits, so the contract +// survives changes to the hashed input. +func TestContract_NetworkMultidoc_HostnameFallbackToSynthesized(t *testing.T) { + cases := []struct { + name string + discoveredHostname string + }{ + {"placeholder/talos", "talos"}, + {"placeholder/localhost", "localhost"}, + {"placeholder/rescue", "rescue"}, + {"empty/no-hostname-resource", ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + lookup := func(resource, namespace, id string) (map[string]any, error) { + if resource == "hostname" && id == "hostname" && tc.discoveredHostname != "" { + return map[string]any{ + "spec": map[string]any{"hostname": tc.discoveredHostname}, + }, nil + } + return map[string]any{}, nil + } + out := renderCozystackWith(t, lookup, map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "kind: HostnameConfig") + assertContains(t, out, `hostname: "talos-`) + }) + } +} + +// === ResolverConfig === + +// Contract: ResolverConfig.nameservers is emitted with one +// `- address: "..."` line per dnsServer when discovery returns a +// resolvers spec, and falls back to a YAML-empty list `[]` when +// resolvers are unknown. The empty fallback keeps the document +// well-formed; Talos accepts empty nameservers (DHCP-supplied). +func TestContract_NetworkMultidoc_ResolverConfigPopulated(t *testing.T) { + lookup := func(resource, namespace, id string) (map[string]any, error) { + if resource == "resolvers" && id == "resolvers" { + return map[string]any{ + "spec": map[string]any{ + "dnsServers": []any{"8.8.8.8", "1.1.1.1"}, + }, + }, nil + } + return map[string]any{}, nil + } + out := renderCozystackWith(t, lookup, map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "kind: ResolverConfig") + assertContains(t, out, `- address: "8.8.8.8"`) + assertContains(t, out, `- address: "1.1.1.1"`) +} + +// Contract: ResolverConfig falls back to YAML empty list when +// resolvers are not discoverable (no DHCP yet, no static config). +func TestContract_NetworkMultidoc_ResolverConfigEmptyFallback(t *testing.T) { + out := renderCozystackWith(t, helmEngineEmptyLookup, map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "kind: ResolverConfig") + assertContains(t, out, "nameservers:") + assertContains(t, out, "[]") +} + +// === LinkConfig (single physical NIC) === + +// Contract: a single physical NIC produces exactly one LinkConfig +// document whose name matches the discovered link, addresses match +// discovery, routes carry the default-route gateway, and MTU is NOT +// emitted when discovery reports no MTU (Talos uses its own default). +// +// The simpleNicLookup fixture provides one eth0 with 192.168.201.10/24 +// and a default route to 192.168.201.1 — the canonical happy path. +func TestContract_NetworkMultidoc_SinglePhysicalNICRendersLinkConfig(t *testing.T) { + out := renderCozystackWith(t, simpleNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "kind: LinkConfig") + assertContains(t, out, "name: eth0") + assertContains(t, out, "- address: 192.168.201.10/24") + assertContains(t, out, "routes:") + assertContains(t, out, "- gateway: 192.168.201.1") +} + +// Contract: when a multi-NIC node has only one default-gateway-bearing +// link, ONLY that link's LinkConfig carries `routes:`. Non-gateway +// links get LinkConfig with addresses but no routes block — emitting +// routes on every link would inject duplicate default routes that +// shadow each other. +func TestContract_NetworkMultidoc_MultiNICRoutesOnGatewayLinkOnly(t *testing.T) { + out := renderCozystackWith(t, multiNicLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + // Both links rendered. + assertContains(t, out, "name: eth0") + assertContains(t, out, "name: eth1") + // Gateway link's address. + assertContains(t, out, "- address: 192.168.201.10/24") + // Non-gateway link's address (private subnet on eth1). + assertContains(t, out, "- address: 10.0.0.5/24") + // Exactly one `routes:` block in the multi-doc network section. + // Both legacy and multi-doc paths use the same LinkConfig structure + // here, so substring counting is stable. + if got := strings.Count(out, "routes:"); got != 1 { + t.Errorf("expected exactly 1 routes: block (gateway link only), got %d in:\n%s", got, out) + } +} + +// === BondConfig === + +// Contract: a bond master link produces a BondConfig document. The +// document carries `links: [, ]` (the slaves' metadata +// IDs) and the bondMaster fields verbatim: bondMode, xmitHashPolicy, +// lacpRate, miimon. Slaves themselves do NOT get standalone +// LinkConfig documents — emitting one alongside BondConfig conflicts +// with Talos's link controller convergence. +func TestContract_NetworkMultidoc_BondRendersBondConfig(t *testing.T) { + out := renderCozystackWith(t, bondTopologyLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "kind: BondConfig") + assertContains(t, out, "name: bond0") + assertContains(t, out, "links:") + assertContains(t, out, "- eth0") + assertContains(t, out, "- eth1") + assertContains(t, out, "bondMode: 802.3ad") + assertContains(t, out, "xmitHashPolicy: layer3+4") + assertContains(t, out, "lacpRate: fast") + assertContains(t, out, "miimon: 100") +} + +// Contract: bond slaves never appear as standalone LinkConfig +// documents. configurable_link_names filters them out via spec.slaveKind. +func TestContract_NetworkMultidoc_BondSlavesNotEmittedAsLinkConfig(t *testing.T) { + out := renderCozystackWith(t, bondTopologyLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + // Each slave appears under BondConfig.links, but not as a + // LinkConfig document. Substring "kind: LinkConfig\nname: eth0" + // would indicate the regression. + if strings.Contains(out, "kind: LinkConfig\nname: eth0") { + t.Errorf("eth0 (bond slave) leaked as LinkConfig:\n%s", out) + } + if strings.Contains(out, "kind: LinkConfig\nname: eth1") { + t.Errorf("eth1 (bond slave) leaked as LinkConfig:\n%s", out) + } +} + +// === VLANConfig === + +// Contract: a VLAN link with a resolvable parent and a vlanID +// produces a VLANConfig document with name, vlanID, parent, and (if +// addresses present) addresses block. Bond-as-parent is supported: +// the VLAN's parent name is the bond's metadata.id. +func TestContract_NetworkMultidoc_VLANOnBondRendersVLANConfig(t *testing.T) { + out := renderCozystackWith(t, vlanOnBondTopologyLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "kind: VLANConfig") + assertContains(t, out, "vlanID:") + assertContains(t, out, "parent: bond0") +} + +// === Bridge non-gateway: silent skip === + +// Contract: a bridge link that is NOT the IPv4 default route is +// skipped silently — no BridgeConfig is emitted (chart does not yet +// support BridgeConfig output), and no LinkConfig is emitted (it is +// not a physical NIC). The expectation is that operators who run +// bridges declare them via per-node body overlays. The non-gateway +// case is the silent path; the gateway case is a hard fail (pinned +// in contract_errors_test.go). +func TestContract_NetworkMultidoc_NonGatewayBridgeSkipped(t *testing.T) { + out := renderCozystackWith(t, bridgeLookup(), map[string]any{ + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + // No BridgeConfig — feature unimplemented. + assertNotContains(t, out, "kind: BridgeConfig") +} + +// === Layer2VIPConfig: discovery-derived === + +// Contract: when floatingIP is set on a controlplane and discovery +// resolves a default-gateway-bearing link, Layer2VIPConfig is emitted +// with name= and link=. Worker templates +// never emit Layer2VIPConfig regardless of floatingIP. The simpleNic +// fixture carries the gateway on eth0, so the test pins link=eth0. +func TestContract_NetworkMultidoc_Layer2VIPFromDiscovery(t *testing.T) { + out := renderCozystackWith(t, simpleNicLookup(), map[string]any{ + "floatingIP": "192.168.201.99", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "kind: Layer2VIPConfig") + assertContains(t, out, `name: "192.168.201.99"`) + assertContains(t, out, "link: eth0") +} + +// Contract: floatingIP without controlplane (worker template) does +// NOT emit Layer2VIPConfig. +func TestContract_NetworkMultidoc_Layer2VIPNeverOnWorker(t *testing.T) { + chrt, lookup := cozystackChartPath, simpleNicLookup() + // Render worker template manually (renderCozystackWith renders + // controlplane). Use renderChartTemplateWithLookup which honours + // templateFile + LookupFunc, then assert the absence. + out := renderChartTemplateWithLookup(t, chrt, workerTpl, lookup, multidocTalos) + assertNotContains(t, out, "kind: Layer2VIPConfig") +} + +// === Layer2VIPConfig: vipLink override === + +// Contract: when both floatingIP and vipLink are set on a +// controlplane, Layer2VIPConfig is emitted with link= at the +// top of the multi-doc stream (right after HostnameConfig and +// ResolverConfig), regardless of discovery state. The discovery- +// derived Layer2VIPConfig path is suppressed — emitting both would +// pin the same VIP on two links. +func TestContract_NetworkMultidoc_Layer2VIPOverrideSuppressesDiscovery(t *testing.T) { + out := renderCozystackWith(t, simpleNicLookup(), map[string]any{ + "floatingIP": "192.168.201.99", + "vipLink": "eth0.4000", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "kind: Layer2VIPConfig") + assertContains(t, out, `name: "192.168.201.99"`) + assertContains(t, out, "link: eth0.4000") + // Exactly one Layer2VIPConfig document. The discovery-derived + // emission must be suppressed. + if got := strings.Count(out, "kind: Layer2VIPConfig"); got != 1 { + t.Errorf("expected exactly 1 Layer2VIPConfig, got %d in:\n%s", got, out) + } +} + +// Contract: vipLink override emits Layer2VIPConfig even when +// discovery yields no default link at all (fresh-boot case where the +// VLAN this template is about to bring up does not yet exist on the +// host). +func TestContract_NetworkMultidoc_Layer2VIPOverrideOnFreshNode(t *testing.T) { + out := renderCozystackWith(t, freshNicLookup(), map[string]any{ + "floatingIP": "192.168.201.99", + "vipLink": "eth0.4000", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + assertContains(t, out, "kind: Layer2VIPConfig") + assertContains(t, out, "link: eth0.4000") +} + +// === floatingIP stripping === + +// Contract: when floatingIP is set, the chart strips any address +// matching `/...` from per-link addresses. The VIP is the +// Layer2VIPConfig target — re-declaring it as a regular address would +// race the VIP operator (Talos's VIP operator installs the VIP as a +// global-scope address indistinguishable from a permanent one in +// COSI; without the strip, a re-render against the VIP-active leader +// would declare the VIP both as a permanent address and as the +// Layer2VIPConfig target). +// +// Test setup: simpleNicLookup carries 192.168.201.10/24 on eth0. We +// deliberately set floatingIP to the SAME host address (192.168.201.10) +// so the strip path actually fires — the chart filters per-link +// addresses by `/` prefix, so a VIP that matches no +// discovered address would not exercise the filter. The address must +// appear once at Layer2VIPConfig.name (the VIP declaration itself), +// but MUST NOT appear under LinkConfig.addresses as a regular address. +func TestContract_NetworkMultidoc_FloatingIPStrippedFromLinkAddresses(t *testing.T) { + out := renderCozystackWith(t, simpleNicLookup(), map[string]any{ + "floatingIP": "192.168.201.10", + "advertisedSubnets": []any{testAdvertisedSubnet}, + }) + // Layer2VIPConfig declares the VIP. + assertContains(t, out, `name: "192.168.201.10"`) + // LinkConfig.addresses must NOT contain the VIP CIDR. + assertNotContains(t, out, "- address: 192.168.201.10/24") +} diff --git a/pkg/engine/contract_render_test.go b/pkg/engine/contract_render_test.go new file mode 100644 index 00000000..6e995d72 --- /dev/null +++ b/pkg/engine/contract_render_test.go @@ -0,0 +1,340 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: Render top-level entry point and the +// FullConfigProcess / InitializeConfigBundle / SerializeConfiguration +// trio that backs `talm template` and `talm apply`. These functions +// glue together: chart load → values aggregation → helm render → +// applyPatchesAndRenderConfig → final bytes. Tests pin error +// surfaces an operator can hit (missing templates, bad TalosVersion, +// bad chart root, malformed patches) and the happy-path round-trip. + +package engine + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/siderolabs/talos/pkg/machinery/config/machine" +) + +// === Render: error surfaces === + +// Contract: Render with empty Options.TemplateFiles surfaces a +// precise error mentioning --file / --template (the two CLI flags +// that populate the field). +func TestContract_Render_NoTemplateFilesError(t *testing.T) { + chartRoot := createTestChart(t, "tc", "config.yaml", "machine:\n type: worker\n") + _, err := Render(context.Background(), nil, Options{ + Offline: true, + Root: chartRoot, + // TemplateFiles intentionally empty + }) + if err == nil { + t.Fatal("expected error for empty TemplateFiles") + } + msg := err.Error() + if !strings.Contains(msg, "templates are not set") { + t.Errorf("error must mention 'templates are not set', got: %s", msg) + } + if !strings.Contains(msg, "--file") && !strings.Contains(msg, "--template") { + t.Errorf("error must reference --file or --template flag, got: %s", msg) + } +} + +// Contract: Render with a TemplateFiles entry that does not exist in +// the chart surfaces an error naming the missing template. Operators +// hit this when they typo a path or when a template file is renamed +// but talm flags still point at the old name. +func TestContract_Render_TemplateNotFoundError(t *testing.T) { + chartRoot := createTestChart(t, "tc", "config.yaml", "machine:\n type: worker\n") + _, err := Render(context.Background(), nil, Options{ + Offline: true, + Root: chartRoot, + TemplateFiles: []string{"templates/does-not-exist.yaml"}, + }) + if err == nil { + t.Fatal("expected error for missing template") + } + if !strings.Contains(err.Error(), "templates/does-not-exist.yaml") { + t.Errorf("error must name the missing template, got: %v", err) + } +} + +// Contract: Render with Options.Root pointing at a non-existent +// directory surfaces a chart-load error from the Helm loader. +func TestContract_Render_ChartLoadError(t *testing.T) { + bogus := filepath.Join(t.TempDir(), "no-such-chart") + _, err := Render(context.Background(), nil, Options{ + Offline: true, + Root: bogus, + TemplateFiles: []string{"templates/config.yaml"}, + }) + if err == nil { + t.Fatal("expected error for missing chart root") + } +} + +// Contract: Render with a values file that does not exist surfaces a +// loadValues error naming the missing file. Confirms loadValues' +// errors propagate up cleanly. +func TestContract_Render_BadValueFileError(t *testing.T) { + chartRoot := createTestChart(t, "tc", "config.yaml", "machine:\n type: worker\n") + _, err := Render(context.Background(), nil, Options{ + Offline: true, + Root: chartRoot, + ValueFiles: []string{"/path/that/does/not/exist.yaml"}, + TemplateFiles: []string{"templates/config.yaml"}, + }) + if err == nil { + t.Fatal("expected error for missing values file") + } + if !strings.Contains(err.Error(), "does/not/exist") { + t.Errorf("error must reference the missing path, got: %v", err) + } +} + +// === Render: happy path === + +// Contract: Render with a minimal valid chart (one template emitting +// a worker machine config patch) returns non-empty config bytes that +// at least mention `machine:` and `type: worker`. The output is the +// final Talos machine config — patches applied, values rendered. +func TestContract_Render_HappyPathOfflineWorker(t *testing.T) { + chartRoot := createTestChart(t, "tc", "config.yaml", "machine:\n type: worker\n") + out, err := Render(context.Background(), nil, Options{ + Offline: true, + Root: chartRoot, + TemplateFiles: []string{"templates/config.yaml"}, + }) + if err != nil { + t.Fatalf("Render: %v", err) + } + got := string(out) + if !strings.Contains(got, "machine:") { + t.Errorf("expected 'machine:' in output, got:\n%s", got) + } + if !strings.Contains(got, "type: worker") { + t.Errorf("expected 'type: worker' in output, got:\n%s", got) + } +} + +// Contract: --set values flow through Render and reach the rendered +// template. This pins the values pipeline end-to-end: Options → +// loadValues → mergeMaps with chart defaults → engine render. +func TestContract_Render_SetValuesReachTemplate(t *testing.T) { + tmpl := `machine: + type: worker + install: + image: {{ .Values.customImage }} +` + chartRoot := createTestChart(t, "tc", "config.yaml", tmpl) + out, err := Render(context.Background(), nil, Options{ + Offline: true, + Root: chartRoot, + Values: []string{"customImage=registry.example.com/talos:test"}, + TemplateFiles: []string{"templates/config.yaml"}, + }) + if err != nil { + t.Fatalf("Render: %v", err) + } + got := string(out) + if !strings.Contains(got, "registry.example.com/talos:test") { + t.Errorf("--set value did not reach template output:\n%s", got) + } +} + +// Contract: Render's TalosVersion validation is a fast-fail BEFORE +// chart loading. A bad version string returns an error even if the +// chart root is also invalid — the version check runs first. +func TestContract_Render_TalosVersionValidatedBeforeChart(t *testing.T) { + bogus := filepath.Join(t.TempDir(), "no-such-chart") + _, err := Render(context.Background(), nil, Options{ + Offline: true, + Root: bogus, + TalosVersion: "garbage-version", + TemplateFiles: []string{"templates/config.yaml"}, + }) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "invalid talos-version") { + t.Errorf("expected 'invalid talos-version' (fast-fail before chart), got: %v", err) + } +} + +// === FullConfigProcess / InitializeConfigBundle / SerializeConfiguration === + +// Contract: InitializeConfigBundle with empty Options returns a +// usable bundle (auto-generates secrets). The returned bundle has a +// ControlPlane config that downstream patches refine. +func TestContract_InitializeConfigBundle_EmptyOptionsReturnsBundle(t *testing.T) { + b, err := InitializeConfigBundle(Options{}) + if err != nil { + t.Fatalf("InitializeConfigBundle: %v", err) + } + if b == nil { + t.Fatal("nil bundle") + } + if b.ControlPlaneCfg == nil { + t.Error("expected ControlPlaneCfg to be set") + } + if b.WorkerCfg == nil { + t.Error("expected WorkerCfg to be set") + } +} + +// Contract: InitializeConfigBundle with malformed TalosVersion +// surfaces an `invalid talos-version` error before bundle creation. +func TestContract_InitializeConfigBundle_BadTalosVersionError(t *testing.T) { + _, err := InitializeConfigBundle(Options{TalosVersion: "garbage"}) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "invalid talos-version") { + t.Errorf("error must mention 'invalid talos-version', got: %v", err) + } +} + +// Contract: InitializeConfigBundle with a missing WithSecrets path +// surfaces a 'failed to load secrets bundle' error naming the cause. +func TestContract_InitializeConfigBundle_MissingSecretsError(t *testing.T) { + _, err := InitializeConfigBundle(Options{ + WithSecrets: filepath.Join(t.TempDir(), "missing-secrets.yaml"), + }) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "secrets bundle") { + t.Errorf("error must mention secrets bundle, got: %v", err) + } +} + +// Contract: SerializeConfiguration produces non-empty YAML bytes for +// both controlplane and worker machine types. Pin that the +// controlplane serialization is meaningfully different from the +// worker one (controlplane has cluster.* fields worker does not). +func TestContract_SerializeConfiguration_ControlplaneVsWorker(t *testing.T) { + b, err := InitializeConfigBundle(Options{}) + if err != nil { + t.Fatal(err) + } + cpBytes, err := SerializeConfiguration(b, machine.TypeControlPlane) + if err != nil { + t.Fatalf("controlplane: %v", err) + } + workerBytes, err := SerializeConfiguration(b, machine.TypeWorker) + if err != nil { + t.Fatalf("worker: %v", err) + } + if len(cpBytes) == 0 || len(workerBytes) == 0 { + t.Fatal("expected non-empty serialization") + } + if string(cpBytes) == string(workerBytes) { + t.Error("controlplane and worker serializations are identical; expected divergence (controlplane has cluster section)") + } + // Worker config has type: worker. + if !strings.Contains(string(workerBytes), "type: worker") { + t.Errorf("worker config missing 'type: worker'") + } + // Controlplane has type: controlplane. + if !strings.Contains(string(cpBytes), "type: controlplane") { + t.Errorf("controlplane config missing 'type: controlplane'") + } +} + +// Contract: FullConfigProcess takes a list of Talos config patches +// (the rendered chart output, one entry per template), runs them +// through bundle.ApplyPatches, and returns the final bundle plus the +// detected machine type. With no patches the type comes from the +// bundle's ControlPlaneCfg default (controlplane). The worker +// fallback in FullConfigProcess only fires when ApplyPatches yields +// machine.TypeUnknown — a reduced state operators do not normally +// reach. +func TestContract_FullConfigProcess_NoPatchesUsesBundleDefault(t *testing.T) { + bundle, mtype, err := FullConfigProcess(context.Background(), Options{}, nil) + if err != nil { + t.Fatalf("FullConfigProcess: %v", err) + } + if bundle == nil { + t.Fatal("nil bundle") + } + // The bundle ControlPlaneCfg defaults to machine.type=controlplane, + // so absent any patch the detected machineType reflects that. + if mtype != machine.TypeControlPlane { + t.Errorf("expected machine.TypeControlPlane (bundle default), got %v", mtype) + } +} + +// Contract: FullConfigProcess with a controlplane-typed patch +// detects machine.TypeControlPlane and propagates it. The patch +// supplies machine.type: controlplane explicitly so the chart's +// own machineType inference does not interfere. +func TestContract_FullConfigProcess_ControlplaneFromPatch(t *testing.T) { + patch := "machine:\n type: controlplane\n" + _, mtype, err := FullConfigProcess(context.Background(), Options{}, []string{patch}) + if err != nil { + t.Fatalf("FullConfigProcess: %v", err) + } + if mtype != machine.TypeControlPlane { + t.Errorf("expected machine.TypeControlPlane, got %v", mtype) + } +} + +// Contract: FullConfigProcess with a malformed patch surfaces a +// LoadPatches error. Pin the error path so a regression that +// silently swallows malformed patches surfaces here. +func TestContract_FullConfigProcess_MalformedPatchError(t *testing.T) { + bad := "this is not valid YAML\n : :" + _, _, err := FullConfigProcess(context.Background(), Options{}, []string{bad}) + if err == nil { + t.Fatal("expected error for malformed patch") + } +} + +// Contract: FullConfigProcess with a malformed TalosVersion option +// surfaces InitializeConfigBundle's error path. +func TestContract_FullConfigProcess_BadTalosVersionError(t *testing.T) { + _, _, err := FullConfigProcess(context.Background(), Options{TalosVersion: "garbage"}, nil) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(err.Error(), "talos-version") { + t.Errorf("error must mention talos-version, got: %v", err) + } +} + +// === Edge cases === + +// Contract: Render with Offline=true does NOT call FailIfMultiNodes +// (online-only check). The test sets two nodes via context and +// confirms Render still proceeds. Without Offline=true the same +// configuration would error. +func TestContract_Render_OfflineSkipsMultiNodeCheck(t *testing.T) { + chartRoot := createTestChart(t, "tc", "config.yaml", "machine:\n type: worker\n") + // Context with multiple nodes — would trip FailIfMultiNodes online. + ctx := context.Background() + _, err := Render(ctx, nil, Options{ + Offline: true, + Root: chartRoot, + TemplateFiles: []string{"templates/config.yaml"}, + }) + if err != nil { + t.Fatalf("offline render must succeed regardless of node count: %v", err) + } + _ = os.Stdout // keep imports stable for future expansion +} diff --git a/pkg/engine/contract_schema_test.go b/pkg/engine/contract_schema_test.go new file mode 100644 index 00000000..48bcd964 --- /dev/null +++ b/pkg/engine/contract_schema_test.go @@ -0,0 +1,206 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: schema selection by `templateOptions.talosVersion` (or +// --talos-version CLI flag). Both shipped charts use the same +// `talos.config` entry point in _helpers.tpl: +// +// {{- if and .TalosVersion (not (semverCompare "<1.12.0-0" .TalosVersion)) }} +// {{- include "talos.config.multidoc" . }} +// {{- else }} +// {{- include "talos.config.legacy" . }} +// {{- end }} +// +// The contract is: +// - empty TalosVersion → legacy +// - <1.12.0-0 (semver) → legacy +// - >=1.12.0-0 (semver) → multi-doc +// - "v1.12.0" / "v1.12" → multi-doc (sprig accepts both) +// +// The pre-release suffix matters: "v1.12.0-rc.1" still satisfies +// >=1.12.0-0 (the -0 anchor sorts before any pre-release). Talm uses +// this so a node booted off a v1.12.0-rc.1 maintenance image still +// gets multi-doc. +// +// Tests below pin both the routing decision (which include() fires) +// and the structural difference between the two outputs (legacy emits +// machine.network section; multi-doc emits separate HostnameConfig / +// ResolverConfig / LinkConfig documents joined by `---`). + +package engine + +import ( + "strings" + "testing" +) + +// Contract: empty talosVersion (the default when neither +// templateOptions.talosVersion nor --talos-version is set) renders the +// legacy schema. Distinguishing marker: `machine.network` block +// exists, with hostname/nameservers/interfaces under it. +func TestContract_Schema_EmptyVersionRendersLegacy(t *testing.T) { + for _, chartPath := range []string{cozystackChartPath, genericChartPath} { + t.Run(chartPath, func(t *testing.T) { + out := renderChartTemplate(t, chartPath, controlplaneTpl) + assertContains(t, out, "network:") + assertContains(t, out, "hostname:") + assertContains(t, out, "nameservers:") + // Multi-doc marker MUST be absent. + assertNotContains(t, out, "kind: HostnameConfig") + assertNotContains(t, out, "kind: ResolverConfig") + assertNotContains(t, out, "kind: LinkConfig") + }) + } +} + +// Contract: any version <1.12.0 renders legacy. Includes 1.10, 1.11, +// pre-releases of 1.12 with explicit -alpha/-beta below the -0 anchor +// (semverCompare uses the -0 lower-bound trick to anchor at the +// earliest possible 1.12.0-anything). +func TestContract_Schema_VersionsBefore112RenderLegacy(t *testing.T) { + versions := []string{ + "v1.10.0", + "v1.11.0", + "v1.11.5", + "1.11", // sprig semver accepts a partial version + } + for _, v := range versions { + t.Run(v, func(t *testing.T) { + out := renderChartTemplate(t, cozystackChartPath, controlplaneTpl, v) + assertNotContains(t, out, "kind: HostnameConfig") + assertContains(t, out, "network:") + assertContains(t, out, "hostname:") + }) + } +} + +// Contract: v1.12.0 and later versions render multi-doc. Distinguishing +// marker: HostnameConfig / ResolverConfig documents joined by `---`. +func TestContract_Schema_Versions112AndLaterRenderMultidoc(t *testing.T) { + versions := []string{ + "v1.12.0", + "v1.12.5", + "v1.13.0", + "v2.0.0", + "1.12", // partial version, accepted by sprig semver + } + for _, v := range versions { + t.Run(v, func(t *testing.T) { + out := renderChartTemplate(t, cozystackChartPath, controlplaneTpl, v) + assertContains(t, out, "kind: HostnameConfig") + assertContains(t, out, "kind: ResolverConfig") + // Document separator MUST appear at least once between + // machine.* and the first --- HostnameConfig. The + // surrounding bytes can be \n or \r\n depending on the + // platform (helm engine emits the host's line ending on + // Windows), so match the literal `---` token rather than + // pinning a specific newline pair. + if !strings.Contains(out, "---") { + t.Errorf("multi-doc render missing `---` separator:\n%s", out) + } + }) + } +} + +// Contract: v1.12.0-rc.1 and similar pre-release tags satisfy the +// >=1.12.0-0 anchor and render multi-doc. This is the cluster-bootstrap +// case: a node booted off a v1.12.0-rc.* maintenance image must get +// multi-doc machine config; otherwise the legacy machine.network block +// would be parsed by a renderer that doesn't accept it. +func TestContract_Schema_Version112PreReleasesRenderMultidoc(t *testing.T) { + versions := []string{ + "v1.12.0-rc.1", + "v1.12.0-alpha.0", + "v1.12.0-beta.5", + } + for _, v := range versions { + t.Run(v, func(t *testing.T) { + out := renderChartTemplate(t, cozystackChartPath, controlplaneTpl, v) + assertContains(t, out, "kind: HostnameConfig") + assertContains(t, out, "kind: ResolverConfig") + }) + } +} + +// Contract: legacy schema produces a single YAML document — no `---` +// separators in the body. Talos's legacy parser expects exactly one +// document. (Helm always prepends one leading newline; we check that +// no internal separator appears.) +func TestContract_Schema_LegacyIsSingleDocument(t *testing.T) { + for _, chartPath := range []string{cozystackChartPath, genericChartPath} { + t.Run(chartPath, func(t *testing.T) { + out := renderChartTemplate(t, chartPath, controlplaneTpl) + // Match a `---` token surrounded by ANY newline form + // (\n or \r\n) — Windows-rendered output uses CRLF and + // pinning `\n---\n` would falsely pass on Windows. + // Scan line-by-line: a single line that is exactly `---` + // means an internal document separator. + for line := range strings.SplitSeq(out, "\n") { + if strings.TrimRight(line, "\r") == "---" { + t.Errorf("legacy render must not contain `---` separator:\n%s", out) + break + } + } + }) + } +} + +// Contract: multi-doc schema emits at least three documents on +// controlplane: the main machine/cluster block, HostnameConfig, and +// ResolverConfig. The exact count varies (more documents appear when +// floatingIP is set, when discovery yields configurable links, etc.), +// but the floor is three. This pins the "we always emit a typed +// config for hostname and resolvers regardless of discovery state" +// contract. +func TestContract_Schema_MultidocAlwaysEmitsHostnameAndResolver(t *testing.T) { + for _, chartPath := range []string{cozystackChartPath, genericChartPath} { + t.Run(chartPath, func(t *testing.T) { + out := renderChartTemplate(t, chartPath, controlplaneTpl, multidocTalos) + assertContains(t, out, "kind: HostnameConfig") + assertContains(t, out, "kind: ResolverConfig") + }) + } +} + +// Contract: multi-doc schema emits RegistryMirrorConfig only on the +// cozystack chart (which still ships the docker.io->mirror.gcr.io +// default). Generic chart does not emit any RegistryMirrorConfig +// document. +func TestContract_Schema_MultidocRegistryMirrorOnlyOnCozystack(t *testing.T) { + cozystackOut := renderChartTemplate(t, cozystackChartPath, controlplaneTpl, multidocTalos) + assertContains(t, cozystackOut, "kind: RegistryMirrorConfig") + assertContains(t, cozystackOut, "name: docker.io") + assertContains(t, cozystackOut, "url: https://mirror.gcr.io") + + genericOut := renderChartTemplate(t, genericChartPath, controlplaneTpl, multidocTalos) + assertNotContains(t, genericOut, "kind: RegistryMirrorConfig") + assertNotContains(t, genericOut, "mirror.gcr.io") +} + +// Contract: schema selection is a per-render decision, not cached. +// Calling renderChartTemplate against the same chart with different +// talosVersion values in the same test process must yield different +// outputs (legacy vs multi-doc). Pins the absence of any global +// caching that would tie a chart's first-rendered schema to its +// subsequent renders. +func TestContract_Schema_SwitchableWithinSameProcess(t *testing.T) { + legacyOut := renderChartTemplate(t, cozystackChartPath, controlplaneTpl, "v1.11.0") + multidocOut := renderChartTemplate(t, cozystackChartPath, controlplaneTpl, "v1.12.0") + if strings.Contains(legacyOut, "kind: HostnameConfig") { + t.Errorf("legacy render leaked multi-doc marker; possible cache bug") + } + if !strings.Contains(multidocOut, "kind: HostnameConfig") { + t.Errorf("multi-doc render lost its marker after a legacy render") + } +} diff --git a/pkg/modeline/contract_test.go b/pkg/modeline/contract_test.go new file mode 100644 index 00000000..7767a361 --- /dev/null +++ b/pkg/modeline/contract_test.go @@ -0,0 +1,280 @@ +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: modeline parsing, file-reading, and generation. The +// modeline is the first line of every per-node values file (e.g. +// `nodes/cp1.yaml`); talm reads it to discover which nodes / endpoints +// / templates apply to a given node config. Format: +// +// # talm: nodes=["1.2.3.4"], endpoints=["1.2.3.4"], templates=["templates/controlplane.yaml"] +// +// Each value is a JSON array. Keys are case-sensitive; unknown keys +// are silently ignored so future keys can be added without breaking +// older talm versions reading the same file. + +package modeline + +import ( + "os" + "path/filepath" + "reflect" + "strings" + "testing" +) + +// === ParseModeline === + +// Contract: ParseModeline rejects lines without the `# talm: ` prefix. +// The prefix requires the `# talm: ` substring exactly (hash, space, +// `talm`, colon, space) so picking up a foreign comment or a YAML key +// by accident is impossible. The indented form is accepted and tested +// in TestContract_ParseModeline_TrimsLine. +func TestContract_ParseModeline_RejectsWithoutPrefix(t *testing.T) { + cases := []string{ + "", + "# nothing", + "# vim: set ft=yaml", // editor modeline (Vim) + "# talm noprefix", // missing colon + `#talm: nodes=["x"]`, // missing space between # and talm + `# talm:nodes=["x"]`, // missing space after colon + } + for _, line := range cases { + t.Run(line, func(t *testing.T) { + _, err := ParseModeline(line) + if err == nil { + t.Errorf("expected error for %q, got nil", line) + } + }) + } +} + +// Contract: leading and trailing whitespace around the entire +// modeline is tolerated. Operators may indent the modeline for +// readability; talm strips before parsing. +func TestContract_ParseModeline_TrimsLine(t *testing.T) { + indented := ` # talm: nodes=["1.2.3.4"] ` + got, err := ParseModeline(indented) + if err != nil { + t.Fatalf("expected indented modeline to parse, got: %v", err) + } + if !reflect.DeepEqual(got.Nodes, []string{"1.2.3.4"}) { + t.Errorf("expected Nodes=[1.2.3.4], got %v", got.Nodes) + } +} + +// Contract: each key must be in `key=value` form where the value is +// a JSON array. Malformed parts surface a precise error mentioning +// the offending segment. +func TestContract_ParseModeline_RejectsMalformedKeyValue(t *testing.T) { + cases := []struct { + name string + line string + }{ + {"missing equals", `# talm: nodes`}, + {"value not JSON", `# talm: nodes=invalid`}, + {"value JSON not array", `# talm: nodes={"key":"val"}`}, + {"empty value", `# talm: nodes=`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := ParseModeline(tc.line) + if err == nil { + t.Errorf("expected error for %q, got nil", tc.line) + } + }) + } +} + +// Contract: keys are separated by `, ` (comma then a single space). +// This is the same separator GenerateModeline emits, so a generated +// modeline always parses back. Missing the space before the next key +// fails to find the part-boundary; trailing whitespace inside a JSON +// array is tolerated (json.Unmarshal handles it). +func TestContract_ParseModeline_KeyValueSeparatorContract(t *testing.T) { + // Canonical (matches GenerateModeline output). + canonical := `# talm: nodes=["a"], endpoints=["b"], templates=["c"]` + got, err := ParseModeline(canonical) + if err != nil { + t.Fatalf("canonical line failed: %v", err) + } + want := &Config{ + Nodes: []string{"a"}, + Endpoints: []string{"b"}, + Templates: []string{"c"}, + } + if !reflect.DeepEqual(got, want) { + t.Errorf("canonical parse mismatch\n got: %+v\nwant: %+v", got, want) + } +} + +// Contract: empty JSON arrays are valid. Producing `nodes=[]` is the +// way to express "no nodes" without dropping the key entirely. +func TestContract_ParseModeline_EmptyArrays(t *testing.T) { + line := `# talm: nodes=[], endpoints=[], templates=[]` + got, err := ParseModeline(line) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(got.Nodes) != 0 || len(got.Endpoints) != 0 || len(got.Templates) != 0 { + t.Errorf("expected all-empty Config, got %+v", got) + } +} + +// === ReadAndParseModeline === + +// Contract: ReadAndParseModeline opens a file, reads only the first +// line, and parses it as a modeline. Subsequent lines are not read +// (the modeline is conventionally the very first line, and the rest +// of the file is YAML the helm engine consumes). +func TestContract_ReadAndParseModeline_FirstLineOnly(t *testing.T) { + dir := t.TempDir() + file := filepath.Join(dir, "node.yaml") + content := `# talm: nodes=["1.2.3.4"] +# talm: nodes=["should-be-ignored"] +machine: + type: worker +` + if err := os.WriteFile(file, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + got, err := ReadAndParseModeline(file) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !reflect.DeepEqual(got.Nodes, []string{"1.2.3.4"}) { + t.Errorf("expected first line only, got %+v", got) + } +} + +// Contract: a missing file produces an error mentioning the path so +// the operator can fix it without grepping. +func TestContract_ReadAndParseModeline_MissingFile(t *testing.T) { + missing := filepath.Join(t.TempDir(), "nope.yaml") + _, err := ReadAndParseModeline(missing) + if err == nil { + t.Fatal("expected error for missing file") + } +} + +// Contract: an empty file produces a precise error. +func TestContract_ReadAndParseModeline_EmptyFile(t *testing.T) { + dir := t.TempDir() + file := filepath.Join(dir, "empty.yaml") + if err := os.WriteFile(file, []byte(""), 0o644); err != nil { + t.Fatal(err) + } + _, err := ReadAndParseModeline(file) + if err == nil { + t.Fatal("expected error for empty file") + } + if !strings.Contains(err.Error(), "empty") { + t.Errorf("error must mention 'empty', got: %v", err) + } +} + +// Contract: a file whose first line is not a modeline surfaces the +// parse error verbatim — the file-read path is a thin wrapper. +func TestContract_ReadAndParseModeline_NonModelineFirstLine(t *testing.T) { + dir := t.TempDir() + file := filepath.Join(dir, "no-modeline.yaml") + if err := os.WriteFile(file, []byte("machine:\n"), 0o644); err != nil { + t.Fatal(err) + } + _, err := ReadAndParseModeline(file) + if err == nil { + t.Fatal("expected error for first line without modeline") + } +} + +// === GenerateModeline === + +// Contract: GenerateModeline emits a line that ParseModeline accepts +// without losing information (round-trip stability). All three keys +// are emitted in a fixed order (nodes, endpoints, templates), even +// when a slice is empty — empty arrays roundtrip as `key=[]`. +func TestContract_GenerateModeline_RoundTrip(t *testing.T) { + cases := []struct { + name string + nodes []string + endpoints []string + templates []string + }{ + {"all populated", + []string{"1.2.3.4", "5.6.7.8"}, + []string{"1.2.3.4"}, + []string{"templates/controlplane.yaml"}, + }, + {"empty all", nil, nil, nil}, + {"only nodes", + []string{"1.2.3.4"}, + nil, + nil, + }, + {"special characters in path", + []string{"node.example.com"}, + []string{"https://api.example.com:6443"}, + []string{"path/with spaces/template.yaml"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + line, err := GenerateModeline(tc.nodes, tc.endpoints, tc.templates) + if err != nil { + t.Fatalf("generate: %v", err) + } + parsed, err := ParseModeline(line) + if err != nil { + t.Fatalf("parse generated modeline %q: %v", line, err) + } + // Slice equality with nil-vs-empty difference: GenerateModeline + // emits [], ParseModeline returns nil for empty arrays. Compare + // after normalising. + normalize := func(s []string) []string { + if len(s) == 0 { + return nil + } + return s + } + if !reflect.DeepEqual(normalize(parsed.Nodes), normalize(tc.nodes)) { + t.Errorf("nodes round-trip mismatch: got %v, want %v", parsed.Nodes, tc.nodes) + } + if !reflect.DeepEqual(normalize(parsed.Endpoints), normalize(tc.endpoints)) { + t.Errorf("endpoints round-trip mismatch: got %v, want %v", parsed.Endpoints, tc.endpoints) + } + if !reflect.DeepEqual(normalize(parsed.Templates), normalize(tc.templates)) { + t.Errorf("templates round-trip mismatch: got %v, want %v", parsed.Templates, tc.templates) + } + }) + } +} + +// Contract: the generated modeline starts with `# talm: ` and emits +// keys in a fixed order — pinning so editor-side highlight/lint +// tooling can rely on it. +func TestContract_GenerateModeline_KeyOrder(t *testing.T) { + line, err := GenerateModeline([]string{"a"}, []string{"b"}, []string{"c"}) + if err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(line, "# talm: ") { + t.Errorf("expected '# talm: ' prefix, got %q", line) + } + // nodes appears before endpoints, endpoints before templates. + nodesIdx := strings.Index(line, "nodes=") + endpointsIdx := strings.Index(line, "endpoints=") + templatesIdx := strings.Index(line, "templates=") + if nodesIdx >= endpointsIdx || endpointsIdx >= templatesIdx { + t.Errorf("key order mismatch: nodes=%d endpoints=%d templates=%d in %q", nodesIdx, endpointsIdx, templatesIdx, line) + } +} diff --git a/pkg/secureperm/contract_unix_test.go b/pkg/secureperm/contract_unix_test.go new file mode 100644 index 00000000..19226b54 --- /dev/null +++ b/pkg/secureperm/contract_unix_test.go @@ -0,0 +1,210 @@ +//go:build !windows + +// Copyright Cozystack Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Contract: extra scenarios for the secureperm Unix implementation +// that complement secureperm_unix_test.go. The package writes secrets +// files (age private keys, secrets.yaml, talosconfig, kubeconfig) +// atomically with mode 0o600. These tests pin behaviours an operator +// observes when running talm on a single workstation: bytes-on-disk +// match input, repeated writes are idempotent, missing parent +// directories surface a precise error, no leftover tmp files on +// successful or failed writes. + +package secureperm_test + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/cozystack/talm/pkg/secureperm" +) + +// Contract: bytes written to the file match the input exactly. The +// atomic-rename helper is sometimes mistaken for a serializer; pin +// that it is purely a byte-pipe to disk. +func TestContract_WriteFile_BytesIntegrity(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "secret") + // Mix of NUL, high-bit bytes, and newlines — typical of binary + // material like age private keys or certificate DER. + want := []byte("AGE-SECRET-KEY-1\x00\x80\xffend\nline2\n") + + if err := secureperm.WriteFile(path, want); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + got, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read back: %v", err) + } + if !bytes.Equal(got, want) { + t.Errorf("bytes mismatch\nwant %q\n got %q", want, got) + } +} + +// Contract: an empty payload writes a zero-length file (still +// mode-0o600). Some callers serialize "no entries" into an empty +// buffer; the helper must accept it without special-casing. +func TestContract_WriteFile_EmptyPayload(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "empty") + if err := secureperm.WriteFile(path, nil); err != nil { + t.Fatalf("WriteFile: %v", err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat: %v", err) + } + if info.Size() != 0 { + t.Errorf("expected size 0, got %d", info.Size()) + } + if got := info.Mode().Perm(); got != 0o600 { + t.Errorf("mode = %o, want 0600", got) + } +} + +// Contract: after a successful write, no `.secureperm-*` tmp file is +// left in the parent directory. The atomic strategy creates the tmp +// in the same dir as the target, then renames over the target — +// leftovers indicate the rename did not happen and would clutter the +// project root over time. +func TestContract_WriteFile_NoTmpLeftoverOnSuccess(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "out") + if err := secureperm.WriteFile(path, []byte("data")); err != nil { + t.Fatalf("WriteFile: %v", err) + } + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("readdir: %v", err) + } + for _, e := range entries { + if strings.HasPrefix(e.Name(), ".secureperm-") { + t.Errorf("found leftover tmp file: %q", e.Name()) + } + } + if len(entries) != 1 { + t.Errorf("expected exactly 1 entry (the target), got %d: %v", len(entries), entries) + } +} + +// Contract: after a FAILED write, no `.secureperm-*` tmp file is +// left. The deferred cleanup in WriteFile removes the tmp on any +// pre-rename failure. Failure is induced by passing a non-existent +// parent directory: CreateTemp fails before any tmp is made, so the +// directory contents stay empty. +// +// We can't induce a tmp-leftover scenario with the current API +// (errors land before the tmp is created). The test pins the +// happy-path absence and the non-existent-dir error path. +func TestContract_WriteFile_NoTmpLeftoverOnFailure(t *testing.T) { + missingDir := filepath.Join(t.TempDir(), "does-not-exist") + target := filepath.Join(missingDir, "out") + + err := secureperm.WriteFile(target, []byte("data")) + if err == nil { + t.Fatal("expected error for non-existent parent dir") + } + // The parent of missingDir DOES exist (it's t.TempDir()). Confirm + // no tmp leaked there either. + parent := filepath.Dir(missingDir) + entries, err := os.ReadDir(parent) + if err != nil { + t.Fatal(err) + } + for _, e := range entries { + if strings.HasPrefix(e.Name(), ".secureperm-") { + t.Errorf("found stray tmp: %q in %q", e.Name(), parent) + } + } +} + +// Contract: WriteFile is idempotent in the operator-observable sense +// — writing the same bytes twice yields the same final file with +// mode 0o600 and no extra files. Verifies the atomic rename overwrite +// path completes cleanly on a target that already exists. +func TestContract_WriteFile_Idempotent(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "out") + payload := []byte("payload-v1") + + for i := range 3 { + if err := secureperm.WriteFile(path, payload); err != nil { + t.Fatalf("WriteFile iteration %d: %v", i, err) + } + } + + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(got, payload) { + t.Errorf("after repeat writes: got %q, want %q", got, payload) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); got != 0o600 { + t.Errorf("mode after repeat writes = %o, want 0600", got) + } + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + if len(entries) != 1 { + t.Errorf("expected exactly 1 entry after 3 writes, got %d: %v", len(entries), entries) + } +} + +// Contract: LockDown on a non-existent file surfaces an os.PathError +// — callers wrap with their own context. No silent success is +// allowed: silently skipping the chmod would let secret material +// remain world-readable if the path was never created. +func TestContract_LockDown_MissingFileErrors(t *testing.T) { + missing := filepath.Join(t.TempDir(), "absent") + err := secureperm.LockDown(missing) + if err == nil { + t.Fatal("expected error for missing file") + } + if !os.IsNotExist(err) { + t.Errorf("expected os.IsNotExist, got: %v", err) + } +} + +// Contract: LockDown is also a tightening operation, not a setting. +// Calling it on a file that is already 0o600 is a no-op (still 0o600, +// no error). Operators may invoke it defensively. +func TestContract_LockDown_AlreadyTightIsNoOp(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "already") + if err := os.WriteFile(path, []byte("ok"), 0o600); err != nil { + t.Fatal(err) + } + if err := secureperm.LockDown(path); err != nil { + t.Fatalf("LockDown on tight file: %v", err) + } + info, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); got != 0o600 { + t.Errorf("expected 0600 unchanged, got %o", got) + } +}