Files
helmfile/pkg/app/create_test.go
T
yxxheroandcopilot-swe-agent[bot] 902c5ced17 feat: add 'create' subcommand to scaffold helmfile deployment projects (#2574)
* feat: add 'create' subcommand to scaffold helmfile deployment projects

Add 'helmfile create [NAME]' command that generates a best-practice
helmfile project structure with:
- helmfile.yaml with commented examples (helmDefaults, repositories,
  environments, releases)
- environments/default.yaml for environment-specific values
- values/.gitkeep placeholder for release values

Supports --output-dir/-o for custom output path and --force to
overwrite existing files. Validates project name to prevent path
traversal.

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: add overwrite protection for all scaffold files and unit tests for create command

- pkg/app/create.go: extract writeFileIfNotExists helper that respects the
  --force flag; all three scaffold files (helmfile.yaml,
  environments/default.yaml, values/.gitkeep) now refuse to overwrite
  without --force
- pkg/config/create.go: ValidateConfig now checks all three scaffold paths
  and reports every already-existing file in a single error before
  proceeding, instead of only checking helmfile.yaml
- pkg/app/create_test.go: add unit tests covering new directory, current
  directory, per-file overwrite rejection without --force, and full
  overwrite with --force

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/eb6d9e4b-0f72-4e26-b841-e1e39a2b2e83

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: remove redundant absDir from ValidateConfig error message

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/eb6d9e4b-0f72-4e26-b841-e1e39a2b2e83

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: address create command review feedback

- cmd/create.go: add config.NewCLIConfigImpl() call for consistency with other
  subcommands; update --force flag help text to list all overwritten files
- pkg/config/create.go: delegate to c.GlobalImpl.ValidateConfig() at end of
  ValidateConfig() for global option validation (--color/--no-color)
- pkg/config/create_test.go: add unit tests for CreateImpl.ValidateConfig()
  covering path separator rejection, '..' rejection, existing-file detection
  per-file and with --force, and global color conflict delegation

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/6327d657-5888-4b94-85fb-def80c0a193f

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: clarify test helper name and comment in create_test.go

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/6327d657-5888-4b94-85fb-def80c0a193f

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: atomic preflight check in App.Create before any writes

Refactor Create to collect all conflicting scaffold paths up front
before writing anything. When --force is not set and any scaffold file
already exists, the command returns a single error listing all
conflicts without touching the filesystem.

Also removes the now-unnecessary writeFileIfNotExists helper and adds a
test (TestCreate_PreflightAtomicOnLaterConflict) verifying that a
conflict on a later file (e.g. environments/default.yaml) prevents even
the first file (helmfile.yaml) from being created.

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/aae6f2e6-7f9e-42b8-afa3-78edd3215127

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: handle non-IsNotExist Stat errors in preflight check; add whitespace name test; fix gci formatting

- pkg/app/create.go: treat os.Stat errors that are NOT os.IsNotExist as
  hard errors in the preflight scan, surfacing permission/IO issues before
  any writes happen; remove trailing blank line that caused gci failure
- pkg/config/create.go: same non-IsNotExist error handling in ValidateConfig
- pkg/config/create_test.go: add TestCreateImpl_ValidateConfig_WhitespaceOnlyName
  covering the "   " (whitespace-only) name rejection branch

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/d6574f56-f46d-46f7-99d9-e0b0b897b3b5

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* refactor: eliminate duplicated scaffold existence check; use O_EXCL for TOCTOU protection

- pkg/config/create.go: remove file-existence check from ValidateConfig
  (duplicate of App.Create's preflight); ValidateConfig now only validates
  the project name and delegates to GlobalImpl.ValidateConfig. Remove unused
  os/path/filepath imports.
- pkg/app/create.go: add writeScaffoldFile helper that uses O_CREATE|O_EXCL
  when force=false, so a file appearing between the preflight check and the
  actual write is caught rather than silently overwritten (TOCTOU protection).
- pkg/config/create_test.go: remove four file-existence tests that tested the
  now-deleted ValidateConfig logic; file-conflict coverage remains in
  pkg/app/create_test.go. Simplify ValidName and GlobalColorConflict tests.

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/82f82e72-934f-416c-8662-5060e92284fa

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: wrap O_EXCL error with --force hint; add writeScaffoldFile unit tests

- pkg/app/create.go: wrap os.IsExist error from writeScaffoldFile with a
  message that names the conflicting file and suggests --force, so the user
  gets actionable output even in the TOCTOU case
- pkg/app/create_test.go: add TestWriteScaffoldFile_CreatesNewFile,
  TestWriteScaffoldFile_ExistingFileNoForce, and
  TestWriteScaffoldFile_ExistingFileWithForce to cover the helper directly

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/82f82e72-934f-416c-8662-5060e92284fa

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: wrap App.Create errors in *app.Error; reject '.' as project name; add '.' name test

- pkg/app/create.go: wrap all App.Create fmt.Errorf returns with appError("", ...)
  so toCLIError produces a clean user-friendly message instead of
  "unexpected error: *fmt.wrapError: ..."
- pkg/config/create.go: reject "." as a NAME alongside ".." to prevent
  accidentally scaffolding into the current directory via a named argument
- pkg/config/create_test.go: add TestCreateImpl_ValidateConfig_NameDot

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/6d64508e-2d66-47e9-a02a-7669a2f481b7

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: drop unused outputDir param from test helper to fix unparam lint error

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/11cd65e9-c5ef-4195-9375-bc929169616b

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: drop unused force param from test helper to fix unparam lint error

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/0e1bdac5-708f-4615-ae6d-e22fc1e921f2

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
2026-05-03 19:03:11 +08:00

209 lines
6.9 KiB
Go

package app
import (
"os"
"path/filepath"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
)
// mockCreateConfigProvider is a test double for CreateConfigProvider.
type mockCreateConfigProvider struct {
name string
outputDir string
force bool
logger *zap.SugaredLogger
}
func (m *mockCreateConfigProvider) Name() string { return m.name }
func (m *mockCreateConfigProvider) OutputDir() string { return m.outputDir }
func (m *mockCreateConfigProvider) Force() bool { return m.force }
func (m *mockCreateConfigProvider) Logger() *zap.SugaredLogger {
if m.logger != nil {
return m.logger
}
return newTestLogger()
}
func newMockCreateConfig(outputDir string, force bool) *mockCreateConfigProvider {
return &mockCreateConfigProvider{outputDir: outputDir, force: force}
}
func TestCreate_NewDirectory(t *testing.T) {
dir := t.TempDir()
outDir := filepath.Join(dir, "myproject")
a := &App{}
cfg := newMockCreateConfig(outDir, false)
require.NoError(t, a.Create(cfg))
// Verify all scaffold files were created.
assertFileContent(t, filepath.Join(outDir, "helmfile.yaml"), helmfileYAMLTemplate)
assertFileContent(t, filepath.Join(outDir, "environments", "default.yaml"), envDefaultYAMLTemplate)
assertFileExists(t, filepath.Join(outDir, "values", ".gitkeep"))
}
func TestCreate_CurrentDirectory(t *testing.T) {
dir := t.TempDir()
a := &App{}
cfg := newMockCreateConfig(dir, false)
require.NoError(t, a.Create(cfg))
assertFileContent(t, filepath.Join(dir, "helmfile.yaml"), helmfileYAMLTemplate)
assertFileContent(t, filepath.Join(dir, "environments", "default.yaml"), envDefaultYAMLTemplate)
assertFileExists(t, filepath.Join(dir, "values", ".gitkeep"))
}
func TestCreate_ExistingHelmfileYAMLNoForce(t *testing.T) {
dir := t.TempDir()
// Pre-create helmfile.yaml
require.NoError(t, os.WriteFile(filepath.Join(dir, "helmfile.yaml"), []byte("existing"), 0o644))
a := &App{}
cfg := newMockCreateConfig(dir, false)
err := a.Create(cfg)
require.Error(t, err)
assert.Contains(t, err.Error(), "already exist")
assert.Contains(t, err.Error(), "--force")
// Verify the existing file was not overwritten.
content, readErr := os.ReadFile(filepath.Join(dir, "helmfile.yaml"))
require.NoError(t, readErr)
assert.Equal(t, "existing", string(content))
}
func TestCreate_ExistingEnvDefaultYAMLNoForce(t *testing.T) {
dir := t.TempDir()
envDir := filepath.Join(dir, "environments")
require.NoError(t, os.MkdirAll(envDir, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(envDir, "default.yaml"), []byte("existing"), 0o644))
a := &App{}
cfg := newMockCreateConfig(dir, false)
err := a.Create(cfg)
require.Error(t, err)
assert.Contains(t, err.Error(), "already exist")
assert.Contains(t, err.Error(), "--force")
// Verify the existing file was not overwritten.
content, readErr := os.ReadFile(filepath.Join(envDir, "default.yaml"))
require.NoError(t, readErr)
assert.Equal(t, "existing", string(content))
}
func TestCreate_ExistingGitkeepNoForce(t *testing.T) {
dir := t.TempDir()
valuesDir := filepath.Join(dir, "values")
require.NoError(t, os.MkdirAll(valuesDir, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(valuesDir, ".gitkeep"), []byte("existing"), 0o644))
a := &App{}
cfg := newMockCreateConfig(dir, false)
err := a.Create(cfg)
require.Error(t, err)
assert.Contains(t, err.Error(), "already exist")
assert.Contains(t, err.Error(), "--force")
// Verify the existing file was not overwritten.
content, readErr := os.ReadFile(filepath.Join(valuesDir, ".gitkeep"))
require.NoError(t, readErr)
assert.Equal(t, "existing", string(content))
}
// TestCreate_PreflightAtomicOnLaterConflict verifies that when only a later
// scaffold file exists (e.g. environments/default.yaml but not helmfile.yaml),
// the preflight check catches it and no files are written at all.
func TestCreate_PreflightAtomicOnLaterConflict(t *testing.T) {
dir := t.TempDir()
envDir := filepath.Join(dir, "environments")
require.NoError(t, os.MkdirAll(envDir, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(envDir, "default.yaml"), []byte("existing"), 0o644))
a := &App{}
cfg := newMockCreateConfig(dir, false)
err := a.Create(cfg)
require.Error(t, err)
assert.Contains(t, err.Error(), "already exist")
// helmfile.yaml must NOT have been created (preflight aborted before any write).
_, statErr := os.Stat(filepath.Join(dir, "helmfile.yaml"))
assert.True(t, os.IsNotExist(statErr), "helmfile.yaml should not have been created")
}
func TestCreate_ExistingFilesWithForce(t *testing.T) {
dir := t.TempDir()
// Pre-create all scaffold files with different content.
require.NoError(t, os.WriteFile(filepath.Join(dir, "helmfile.yaml"), []byte("old"), 0o644))
envDir := filepath.Join(dir, "environments")
require.NoError(t, os.MkdirAll(envDir, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(envDir, "default.yaml"), []byte("old"), 0o644))
valuesDir := filepath.Join(dir, "values")
require.NoError(t, os.MkdirAll(valuesDir, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(valuesDir, ".gitkeep"), []byte("old"), 0o644))
a := &App{}
cfg := newMockCreateConfig(dir, true)
require.NoError(t, a.Create(cfg))
// Verify scaffold files were overwritten with the template content.
assertFileContent(t, filepath.Join(dir, "helmfile.yaml"), helmfileYAMLTemplate)
assertFileContent(t, filepath.Join(dir, "environments", "default.yaml"), envDefaultYAMLTemplate)
assertFileExists(t, filepath.Join(dir, "values", ".gitkeep"))
}
// assertFileContent asserts that the file at path exists and contains wantContent.
func assertFileContent(t *testing.T, path, wantContent string) {
t.Helper()
content, err := os.ReadFile(path)
require.NoError(t, err, "file %s should exist", path)
assert.Equal(t, wantContent, string(content))
}
// assertFileExists asserts that the file at path exists.
func assertFileExists(t *testing.T, path string) {
t.Helper()
_, err := os.Stat(path)
assert.NoError(t, err, "file %s should exist", path)
}
func TestWriteScaffoldFile_CreatesNewFile(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "new.yaml")
require.NoError(t, writeScaffoldFile(path, []byte("hello"), false))
assertFileContent(t, path, "hello")
}
func TestWriteScaffoldFile_ExistingFileNoForce(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "existing.yaml")
require.NoError(t, os.WriteFile(path, []byte("original"), 0o644))
err := writeScaffoldFile(path, []byte("new"), false)
require.Error(t, err)
assert.Contains(t, err.Error(), "--force")
// Original content must be unchanged.
assertFileContent(t, path, "original")
}
func TestWriteScaffoldFile_ExistingFileWithForce(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "existing.yaml")
require.NoError(t, os.WriteFile(path, []byte("original"), 0o644))
require.NoError(t, writeScaffoldFile(path, []byte("new"), true))
assertFileContent(t, path, "new")
}