Introduce Initdata subcommand#41
Conversation
Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>
Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>
Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>
Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>
Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>
Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>
Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>
Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>
Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a new initdata command group to generate, dump, and validate initdata artifacts for Confidential Containers, while extending the pkg/initdata library with raw generation and decoding support.
Changes:
- Add
kubectl coco initdata {create,dump,validate}subcommands (plus shared helpers and fixtures). - Extend
pkg/initdatawithGenerateRaw(raw TOML) andDecode(base64+gzip → data map), and refactorGenerateto reuseGenerateRaw. - Remove the legacy
dump-initdatacommand and its tests; update root command wiring and local-only planning artifact guidance.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/initdata/initdata.go | Refactors generation to add GenerateRaw, adds Decode, adjusts policy loading logic. |
| pkg/initdata/initdata_test.go | Adds unit tests for GenerateRaw, Generate, and Decode round-trips/error cases. |
| cmd/root.go | Registers the new initdata command group. |
| cmd/initdata/initdata.go | Adds initdata root command and wires create, dump, validate subcommands. |
| cmd/initdata/create.go | Implements initdata create (config loading, optional CA bundle embed, writes raw TOML). |
| cmd/initdata/dump.go | Implements initdata dump (reads saved TOML, outputs raw or base64+gzip). |
| cmd/initdata/validate.go | Implements initdata validate (structure/version/algorithm/key presence + embedded cert checks). |
| cmd/initdata/common.go | Shared helpers: config loading, blob decompress, cert parsing/validation, cert extraction. |
| cmd/initdata/common_test.go | Test coverage for shared helpers (cert parsing/dir loading, blob decode, extraction). |
| cmd/initdata/create_test.go | Test coverage for create behavior including cert validation and output mode. |
| cmd/initdata/dump_test.go | Test coverage for dump raw/encoded output and missing file handling. |
| cmd/initdata/validate_test.go | Test coverage for validate from file/stdin and invalid cases (version/algorithm/missing keys/certs). |
| cmd/initdata/testdata/*.toml | Validation fixtures for valid/invalid initdata TOML inputs. |
| cmd/dump_initdata.go | Removes the legacy dump-initdata command implementation. |
| cmd/dump_initdata_test.go | Removes tests for the legacy dump-initdata command. |
| .gitignore | Ignores local-only docs/ planning artifacts. |
| CLAUDE.md | Documents that docs/ is for local-only AI planning artifacts and should not be committed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| path := filepath.Join(dir, entry.Name()) | ||
| // #nosec G304 | ||
| data, err := os.ReadFile(path) | ||
| if err != nil { |
There was a problem hiding this comment.
Repo convention is to include a justification on gosec suppressions (e.g., // #nosec G304 -- <reason>). Please add a reason here so it’s clear why iterating and reading files from this directory is safe/expected.
| if filePath != "" { | ||
| // #nosec G304 | ||
| data, err := os.ReadFile(filePath) | ||
| if err != nil { |
There was a problem hiding this comment.
Repo convention is to include a justification on gosec suppressions (e.g., // #nosec G304 -- <reason>). Please add a reason here (the path comes from --file), so the suppression is auditable.
| filePath = filepath.Join(home, ".kube", "coco-initdata.toml") | ||
| } | ||
|
|
||
| // #nosec G304 |
There was a problem hiding this comment.
Repo convention is to include a justification on gosec suppressions (e.g., // #nosec G304 -- <reason>). Please add a reason here (path is user-provided via --file or defaulted), so the suppression is consistent and reviewable.
| // #nosec G304 | |
| // #nosec G304 -- path is user-provided via --file or defaults to ~/.kube/coco-initdata.toml for this CLI command |
| createCmd.Flags().StringVar(&createCACert, "cacert", "", "Path to CA cert PEM file") | ||
| createCmd.Flags().StringVar(&createCAPath, "capath", "", "Path to directory of CA cert PEM files") | ||
| createCmd.Flags().StringVar(&createOutput, "output", "", "Output file for raw TOML (default: ~/.kube/coco-initdata.toml)") | ||
| createCmd.MarkFlagsMutuallyExclusive("cacert", "capath") |
There was a problem hiding this comment.
createCmd.MarkFlagsMutuallyExclusive(...) returns an error in cobra v1.10.x; ignoring it can mask issues if flag names change or initialization order is altered. Please handle the returned error (e.g., if err := ...; err != nil { return/panic }) to make failures explicit during command setup.
| createCmd.MarkFlagsMutuallyExclusive("cacert", "capath") | |
| if err := createCmd.MarkFlagsMutuallyExclusive("cacert", "capath"); err != nil { | |
| panic(err) | |
| } |
| cleanPath := filepath.Clean(path) | ||
|
|
||
| // For absolute paths, validate they don't escape the filesystem root | ||
| // For relative paths, ensure they're relative to current directory | ||
| if filepath.IsAbs(cleanPath) { | ||
| // Absolute paths are allowed for policy files | ||
| // but ensure path doesn't contain traversal attempts | ||
| if strings.Contains(path, "..") { | ||
| return "", fmt.Errorf("invalid policy path: contains directory traversal") | ||
| } | ||
| } else { | ||
| // For relative paths, ensure they resolve within current directory | ||
| if !filepath.IsAbs(cleanPath) { | ||
| cwd, err := os.Getwd() | ||
| if err != nil { | ||
| return "", fmt.Errorf("failed to get current directory: %w", err) | ||
| } | ||
| absPath := filepath.Join(cwd, cleanPath) | ||
| if !strings.HasPrefix(absPath, cwd) { | ||
| return "", fmt.Errorf("invalid policy path: escapes current directory") | ||
| } | ||
| cleanPath = absPath | ||
| cleanPath = filepath.Join(cwd, cleanPath) | ||
| } |
There was a problem hiding this comment.
loadPolicyFile no longer prevents relative paths from escaping the current working directory. With the current Clean + Join(cwd, cleanPath) logic, an input like ../secrets/policy.rego will resolve outside cwd, which reintroduces directory traversal risk for user-controlled config values (e.g., kata_agent_policy). Consider restoring the previous guard (e.g., compute an absolute path and ensure filepath.Rel(cwd, absPath) does not start with ..), or otherwise restrict/validate allowed locations before os.ReadFile.
| } | ||
|
|
||
| // #nosec G304 - Path is validated above | ||
| // #nosec G304 |
There was a problem hiding this comment.
Repo convention is to include a justification on gosec suppressions (e.g., // #nosec G304 -- <reason>). Please add a reason here so future readers understand why reading this path is considered safe in this context.
| // #nosec G304 | |
| // #nosec G304 -- path is cleaned and resolved to an absolute path before reading a user-specified local policy file |
| // #nosec G304 | ||
| data, err := os.ReadFile(path) | ||
| if err != nil { |
There was a problem hiding this comment.
Repo convention is to include a justification on gosec suppressions (e.g., // #nosec G304 -- <reason>). Please add a reason here so future readers understand why reading this user-provided path is acceptable.
Signed-off-by: Pradipta Banerjee <pradipta.banerjee@gmail.com>
No description provided.