Fix 2337 helm4 stale repo indexes (#2369)
* fix: add --force-update flag for Helm 4 to prevent stale repository indexes Fixes #2337 Problem: Helmfile with Helm v4 doesn't update repository indexes when adding repos, leading to stale indexes and errors like: "chart matching version not found in example index. (try 'helm repo update')" This happens because Helm 4 changed behavior compared to Helm 3: - Helm 3: Always downloads index when running "helm repo add", even if repo exists - Helm 4: Skips downloading index if repo already exists with same config (see: https://github.com/helm/helm/blob/v4.0.4/pkg/cmd/repo_add.go#L200) Without --force-update, helmfile only works initially because Helm 4 downloads index on fresh repo setup, but subsequent "helmfile repos" commands result in stale indexes. Root Cause: The code only added --force-update for Helm 3.3.2+, but not for Helm 4, since it was believed to be default behavior in Helm 4. However, Helm 4 requires explicit --force-update flag to update indexes for existing repos. Solution: Add --force-update flag for Helm 4 in AddRepo function to ensure repository indexes are updated even when repository already exists. Refactoring: Simplified the conditional logic from nested if statements to a single readable condition using existing IsVersionAtLeast() helper: if !helm.options.DisableForceUpdate && (helm.IsHelm4() || helm.IsVersionAtLeast("3.3.2")) { args = append(args, "--force-update") } Changes: - pkg/helmexec/exec.go: Add --force-update for Helm 4 - pkg/helmexec/exec_test.go: Update test expectations for both Helm 3.3.2+ and Helm 4 - AGENTS.md: Add development guide for the repository Testing: - All helmexec package tests pass - Verified build succeeds - Tested against Helm 3.2.0 (no force-update) - Tested against Helm 3.3.2+ (with force-update) - Tested against Helm 4.0.1 (with force-update) Signed-off-by: opencode <opencode@users.noreply.github.com> Signed-off-by: yxxhero <aiopsclub@163.com> * test: update expected output for Helm 4 repo add message Update integration test expectations to match Helm 4 behavior with --force-update flag. When --force-update is used, Helm 4 now outputs "has been added to your repositories" instead of "already exists with the same configuration, skipping", because it forcibly updates the repository index. Related to #2337 Signed-off-by: opencode <opencode@users.noreply.github.com> Signed-off-by: yxxhero <aiopsclub@163.com> --------- Signed-off-by: opencode <opencode@users.noreply.github.com> Signed-off-by: yxxhero <aiopsclub@163.com>
This commit is contained in:
parent
693637d88b
commit
b17c5be9cf
|
|
@ -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
|
||||
|
|
@ -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 != "" {
|
||||
|
|
|
|||
|
|
@ -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/
|
||||
`,
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue