diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000..6c7a6e0d --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,196 @@ +# AGENTS.md + +## Build and Test Commands + +### Essential Setup +```bash +# Check Go version (requires 1.24.2+) +go version + +# Check Helm dependency (required at runtime) +helm version # Must show version 3.x + +# Install gci tool for import formatting +go install github.com/daixiang0/gci@latest +``` + +### Build Commands +```bash +# Standard build +make build + +# Direct build +go build -o helmfile . + +# Cross-platform builds +make cross + +# Build test tools (required for integration tests) +make build-test-tools +``` + +### Linting and Formatting +```bash +# Run go vet (always available) +make check + +# Format code with gci +make fmt + +# Run golangci-lint +golangci-lint run +``` + +### Testing Commands +```bash +# Run all unit tests +make test + +# Run specific test package +go test -v ./pkg/app/... + +# Run single test function +go test -v ./pkg/app/... -run TestSpecificFunction + +# Run tests with coverage +go test -v ./pkg/... -coverprofile cover.out -race -p=1 +go tool cover -func cover.out + +# Run integration tests (requires Kubernetes cluster) +make integration +``` + +## Code Style Guidelines + +### Imports +Import ordering (enforced by gci): +1. Standard library (stdlib) +2. Default (third-party) +3. Local (github.com/helmfile/helmfile prefix) + +Always use aliases for common stdlib packages to avoid conflicts: +```go +import ( + goContext "context" // Alias to avoid naming conflicts + goruntime "runtime" + "fmt" + "os" + + "github.com/helmfile/helmfile/pkg/app" +) +``` + +### Formatting +- Use `go fmt` for standard formatting +- Use `gci` for import organization: `gci write --skip-generated -s standard -s default -s 'prefix(github.com/helmfile/helmfile)' .` +- Run `make fmt` before committing (requires gci installation) + +### Types +- Exported types use PascalCase: `type App struct` +- Private types use PascalCase: `type helmKey struct` +- Interface names should describe behavior: `type Interface interface` +- Use `any` instead of `interface{}` + +### Naming Conventions +- Exported functions/variables: PascalCase +- Private functions/variables: camelCase +- Constants: PascalCase +- Test functions: TestXxx with descriptive names +- Package names: lowercase, single word when possible +- Error variables: ErrXxx + +### Error Handling +- Use fmt.Errorf with %w for wrapping errors +- Check errors explicitly; never ignore +- Use custom error types in pkg/errors/ when appropriate +- Return error as last return value +```go +if err != nil { + return fmt.Errorf("failed to parse helm version '%s': %w", version, err) +} +``` + +### Testing +- Use testify/assert for assertions: `assert.Equal(t, expected, actual)` +- Use testify/require for critical assertions: `require.NoError(t, err)` +- Test files: *_test.go +- Use table-driven tests for multiple scenarios +- Run tests with -race flag: `-race -p=1` +- Use test helper packages: pkg/testutil, pkg/testhelper +```go +func TestFunctionName(t *testing.T) { + tests := []struct { + name string + input string + want string + wantErr bool + }{ + { + name: "valid input", + input: "test", + want: "result", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := FunctionUnderTest(tt.input) + if (err != nil) != tt.wantErr { + t.Errorf("error = %v, wantErr %v", err, tt.wantErr) + return + } + assert.Equal(t, tt.want, got) + }) + } +} +``` + +### Logging +- Use zap.SugaredLogger: `logger.Infof("message")` +- Logger injected via dependency injection +- Don't use fmt.Print for production logging + +### Comments +- Exported functions must have comments +- Comments should be complete sentences with capital first letter +- Use // for single-line comments +- Use /* */ for package documentation + +### Linting Configuration (from .golangci.yaml) +Key enabled linters: +- errcheck: Check unhandled errors +- staticcheck: Advanced static analysis +- revive: Fast linter +- govet: Go vet checks +- ineffassign: Detect ineffectual assignments +- misspell: Spell checking +- unused: Detect unused code + +Important thresholds: +- Max function length: 280 lines +- Max statements per function: 140 +- Max cognitive complexity: 110 +- Max naked return lines: 50 +- Line length: 120 characters + +### Structure +- Package main: Entry point only (main.go) +- cmd/: CLI commands using cobra +- pkg/: Core library code organized by domain +- test/: Integration and E2E tests +- Use dependency injection for testability +- Prefer composition over inheritance + +### Critical Rules +1. Always handle errors +2. Run `make check` before committing +3. Run `golangci-lint run` and fix all issues +4. Write tests for new pkg/ functionality +5. Update docs/ for user-facing changes +6. Follow declarative design principles (desired state in config, operational via flags) + +### Common Issues +- First build downloads 200+ packages (2-3 minutes) +- Integration tests require Kubernetes cluster (minikube/kind) +- Make fmt requires gci installation +- Use -p=1 for tests to avoid race conditions +- Always initialize Helm plugins with `helmfile init` after installation diff --git a/pkg/helmexec/exec.go b/pkg/helmexec/exec.go index b0b94ef4..34a6e5dc 100644 --- a/pkg/helmexec/exec.go +++ b/pkg/helmexec/exec.go @@ -211,16 +211,11 @@ func (helm *execer) AddRepo(name, repository, cafile, certfile, keyfile, usernam case "": args = append(args, "repo", "add", name, repository) - // --force-update is the default behavior in Helm 4, but needs to be explicit in Helm 3 + // --force-update is needed for both Helm 3.3.2+ and Helm 4 + // to ensure repository indexes are updated when a repository already exists // See https://github.com/helm/helm/pull/8777 - if helm.IsHelm3() { - if cons, err := semver.NewConstraint(">= 3.3.2"); err == nil { - if !helm.options.DisableForceUpdate && cons.Check(helm.version) { - args = append(args, "--force-update") - } - } else { - panic(err) - } + if !helm.options.DisableForceUpdate && (helm.IsHelm4() || helm.IsVersionAtLeast("3.3.2")) { + args = append(args, "--force-update") } if certfile != "" && keyfile != "" { diff --git a/pkg/helmexec/exec_test.go b/pkg/helmexec/exec_test.go index 27029755..cea2c966 100644 --- a/pkg/helmexec/exec_test.go +++ b/pkg/helmexec/exec_test.go @@ -147,9 +147,17 @@ exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.e `, }, { - name: "Helm 4.0.1 (force-update is default)", + name: "Helm 4.0.1 (force-update needed)", version: "4.0.1", expected: `Adding repo myRepo https://repo.example.com/ +exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.example.com/ --force-update --cert-file cert.pem --key-file key.pem +`, + }, + { + name: "Helm 4.0.1 (force-update disabled)", + version: "4.0.1", + disableForceUpdate: true, + expected: `Adding repo myRepo https://repo.example.com/ exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.example.com/ --cert-file cert.pem --key-file key.pem `, }, @@ -188,7 +196,7 @@ func Test_AddRepo(t *testing.T) { buffer.Reset() err = helm.AddRepo("myRepo", "https://repo.example.com/", "", "cert.pem", "key.pem", "", "", "", false, false) expected := `Adding repo myRepo https://repo.example.com/ -exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.example.com/ --cert-file cert.pem --key-file key.pem +exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.example.com/ --force-update --cert-file cert.pem --key-file key.pem ` if err != nil { t.Errorf("unexpected error: %v", err) @@ -202,7 +210,7 @@ exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.e buffer.Reset() err = helm.AddRepo("myRepo", "https://repo.example.com/", "ca.crt", "", "", "", "", "", false, false) expected = `Adding repo myRepo https://repo.example.com/ -exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.example.com/ --ca-file ca.crt +exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.example.com/ --force-update --ca-file ca.crt ` if err != nil { t.Errorf("unexpected error: %v", err) @@ -216,7 +224,7 @@ exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.e buffer.Reset() err = helm.AddRepo("myRepo", "https://repo.example.com/", "", "", "", "", "", "", false, false) expected = `Adding repo myRepo https://repo.example.com/ -exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.example.com/ +exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.example.com/ --force-update ` if err != nil { t.Errorf("unexpected error: %v", err) @@ -257,11 +265,12 @@ exec: az acr helm repo add --name acrRepo: buffer.Reset() err = helm.AddRepo("myRepo", "https://repo.example.com/", "", "", "", "example_user", "example_password", "", false, false) expected = `Adding repo myRepo https://repo.example.com/ -exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.example.com/ --username example_user --password-stdin +exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.example.com/ --force-update --username example_user --password-stdin ` if err != nil { t.Errorf("unexpected error: %v", err) } + if buffer.String() != expected { t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -270,7 +279,7 @@ exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.e buffer.Reset() err = helm.AddRepo("myRepo", "https://repo.example.com/", "", "", "", "example_user", "example_password", "", true, false) expected = `Adding repo myRepo https://repo.example.com/ -exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.example.com/ --pass-credentials --username example_user --password-stdin +exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.example.com/ --force-update --pass-credentials --username example_user --password-stdin ` if err != nil { t.Errorf("unexpected error: %v", err) @@ -285,8 +294,8 @@ exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.e buffer.Reset() err = helm.AddRepo("myRepo", "https://repo.example.com/", "", "", "", "", "", "", false, true) expected = `Adding repo myRepo https://repo.example.com/ - exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.example.com/ --insecure-skip-tls-verify - ` +exec: helm --kubeconfig config --kube-context dev repo add myRepo https://repo.example.com/ --force-update --insecure-skip-tls-verify +` if err != nil { t.Errorf("unexpected error: %v", err) } @@ -1174,7 +1183,7 @@ exec: helm --kubeconfig config --kube-context dev chart export chart --destinati var logLevelTests = map[string]string{ "debug": `Adding repo myRepo https://repo.example.com/ -exec: helm repo add myRepo https://repo.example.com/ --username example_user --password example_password +exec: helm repo add myRepo https://repo.example.com/ --force-update --username example_user --password-stdin `, "info": `Adding repo myRepo https://repo.example.com/ `, diff --git a/test/integration/test-cases/suppress-output-line-regex/output/diff-live-after-helm-diff-3.11.0-helm4 b/test/integration/test-cases/suppress-output-line-regex/output/diff-live-after-helm-diff-3.11.0-helm4 index 417b0877..1eab0c03 100644 --- a/test/integration/test-cases/suppress-output-line-regex/output/diff-live-after-helm-diff-3.11.0-helm4 +++ b/test/integration/test-cases/suppress-output-line-regex/output/diff-live-after-helm-diff-3.11.0-helm4 @@ -1,4 +1,4 @@ -"ingress-nginx" already exists with the same configuration, skipping +"ingress-nginx" has been added to your repositories helmfile-tests, ingress-nginx, ClusterRole (rbac.authorization.k8s.io) has changed, but diff is empty after suppression. helmfile-tests, ingress-nginx, ClusterRoleBinding (rbac.authorization.k8s.io) has changed: # Source: ingress-nginx/templates/clusterrolebinding.yaml