diff --git a/AGENTS.md b/AGENTS.md index 01993450..adaa227f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,289 +1,185 @@ -# helm-java - AI Agents Instructions +# helm-java — AI Agent Instructions -Always reference these instructions first and fallback to search or bash commands only when you encounter unexpected information that does not match the info here. +This file is the primary context for AI coding agents (Claude Code, Copilot, etc.) working in this repo. Read it first; fall back to grep/search only when something here is wrong or missing. -This file provides guidance to AI coding agents (GitHub Copilot, Claude Code, etc.) when working with code in this repository. +## What this project is -## Project Overview +A Java client library for Helm with no external CLI dependency. The Helm Go SDK (`helm.sh/helm/v3`) is compiled to a per-platform shared library via CGO and loaded from Java with JNA. The public Java API is a fluent builder that mirrors Helm CLI verbs: `install`, `upgrade`, `uninstall`, `template`, `package`, `lint`, `repo`, `registry`, `search`, `show`, `status`, `history`, `get` (values), `test`, `list`, `dependency`, `push`, `version`, `create`. -helm-java is a Java client library that allows running Helm commands directly from Java code without requiring a separate Helm CLI installation. It uses JNA (Java Native Access) to call native Go Helm libraries compiled as shared libraries for multiple platforms (darwin-amd64, darwin-arm64, linux-amd64, linux-arm64, windows-amd64). The library provides a fluent API that mirrors Helm CLI commands like `install`, `upgrade`, `uninstall`, `template`, `package`, `lint`, `repo`, `registry`, etc. +- Helm SDK version: tracked by `native/go.mod` (currently `helm.sh/helm/v3 v3.21.0`). +- Java baseline: source/target 1.8. Tests run on 8 (Linux CI) or 11 (macOS/Windows CI, because macos-latest dropped 8). +- Go baseline: pinned in `native/go.mod` and `.github/workflows/build.yml` (`GO_VERSION`); currently `1.25.7`. -## Working Effectively +## Repository layout -### Bootstrap and Setup +Multi-module Maven build (parent `pom.xml`) + a sibling Go module under `native/`. To find a file, use the naming patterns in [Conventions](#conventions) — don't trust enumerations here, they go stale. -```bash -# Clone and enter the project -git clone https://github.com/manusa/helm-java.git -cd helm-java - -# Build native libraries first (required before Maven build) -# Native binaries must exist in native/out/ before Maven enforcer will pass -cd native -go build -buildmode=c-shared -o out/helm-darwin-10.12-amd64.dylib . -# (or the appropriate target for your platform) -cd .. - -# Alternatively, use the Makefile for simplified builds -make build-current-platform # Builds native + Java for current platform only -``` - -### Build Commands - -```bash -# Full build with tests (requires native binaries in native/out/) -./mvnw clean install +| Path | Role | +|---|---| +| `helm-java/` | **Public Java API** (the published artifact). Holds `Helm.java` entry point, one `{Verb}Command.java` per command, and result/domain types (`Release`, `Repository`, …). Tests in `src/test/java/`. | +| `lib/api/` | **JNA layer**, shared across platforms. Holds `HelmLib` (the JNA interface — one method per exported native function), `NativeLibrary` (SPI loader with `RemoteJarLoader` fallback for Gradle), `Result`, and one `{Operation}Options.java` per native call. | +| `lib/{darwin,linux,windows}-{amd64,arm64}/` | **Per-platform native binary modules**. Each ships the compiled shared library as a classpath resource and an SPI entry at `META-INF/services/com.marcnuri.helm.jni.NativeLibrary`. Selected automatically by OS-activated profiles in `helm-java/pom.xml`. | +| `lib/wasi/` | Abandoned WebAssembly experiment (issue [#239](https://github.com/manusa/helm-java/issues/239), PR [#243](https://github.com/manusa/helm-java/pull/243)). Not in parent `pom.xml`. Don't touch; will be removed. | +| `native/main.go` | **CGO bridge only**: C struct definitions + `//export` wrappers that delegate to `internal/helm`. No business logic. | +| `native/internal/helm/` | **Real Go implementations** — one file per verb (`install.go`, `upgrade.go`, …) plus `_test.go` siblings. `helm.go` holds shared config (`NewCfg`), `debug.go` the stdout/stderr capture. | +| `native/internal/test/` | Go test helpers (envtest). | +| `native/wasm/`, `native/out/` | TinyGo/WASI experiment entrypoint (see `lib/wasi/` note); build output for shared libraries. | +| `docs/research/` | Audits and decision records (e.g. native-codebase audit). Snapshots — may be stale; check the date. | +| `scripts/check-authors.sh` | Validates `@author` Javadoc tags. | +| `Makefile` | Build/test/release entrypoints (see [Build & test](#build--test)). | -# Quick build without tests -./mvnw clean install -Dquickly +## Build & test -# Build specific module -./mvnw clean install -pl helm-java -am - -# Build with javadoc and sources -./mvnw clean package - -# Using Makefile targets -make build-java # Maven build only -make build-native # Build native library for current platform -make build-native-cross-platform # Build for all platforms (requires xgo) -make build-all # Cross-platform natives + Java build -make clean # Clean all build artifacts -``` - -### Testing - -**IMPORTANT**: Tests use Testcontainers with KinD (Kubernetes in Docker) for integration tests. These tests spawn actual Kubernetes clusters and may take significant time. +The Maven parent has `requireFilesExist` enforcer that needs all 5 native binaries in `native/out/`. Submodule `helm-java` and `lib/api` skip the rule, but a top-level `./mvnw install` will fail without them. ```bash -# Run all tests -./mvnw test +# Local dev (current platform only — skips the cross-platform enforcer check) +make build-current-platform # = build-native + mvn clean verify with -Denforcer.skipRules=requireFilesExist + +# Full Maven build (needs all 5 native binaries built first) +./mvnw clean install +./mvnw clean install -Dquickly # skips tests AND invoker plugin -# Run tests in the main helm-java module only -./mvnw test -pl helm-java +# Native binaries +make build-native # current platform (auto-detected: OS/arch → helm--.) +make build-native-cross-platform # all 5 platforms via xgo (Docker required) -# Run a specific test class +# Single Java test ./mvnw test -pl helm-java -Dtest=HelmInstallTest - -# Run a specific test method ./mvnw test -pl helm-java -Dtest=HelmInstallTest#withName -# Run Go tests only -make test-go -# Or directly: -cd native && go clean -testcache && go test ./... -``` - -**NEVER CANCEL** tests that involve Kubernetes operations - they may leave resources in an inconsistent state. - -### Running the Application - -This is a library, not a standalone application. Use it in your Java project: +# Go tests +make test-go # = cd native && go clean -testcache && go test ./... -```java -// Example: Create and install a chart -Helm helm = new Helm(Paths.get("path/to/chart")); -Release release = helm.install() - .withKubeConfig(kubeConfigPath) - .withName("my-release") - .call(); +# Other useful targets +make build-all # cross-platform natives + Java build +make update-go-deps # bulk-bump non-indirect Go deps +make license # apply license-header.txt to .go/.java files +make release V=1.2.3 VS=1.3.0 # tag release and bump to next snapshot (maintainer only) ``` -## Architecture +CI (`.github/workflows/build.yml`): Linux job runs `make test-go` + `make build-all` on Java 8. Matrix jobs run `make build-current-platform` on Windows/macOS with Java 11. + +**Don't cancel running tests** that hit Kubernetes — they leak cluster resources. -### Technical Structure +## How a call flows (the model to keep in your head) ``` -helm-java/ -├── helm-java/ # Main client library (public API) -│ └── src/ -│ ├── main/java/com/marcnuri/helm/ -│ │ ├── Helm.java # Main entry point -│ │ ├── *Command.java # Command implementations (InstallCommand, etc.) -│ │ └── *.java # Result types (Release, Repository, etc.) -│ └── test/java/ # JUnit 5 tests with Testcontainers -├── lib/ -│ ├── api/ # JNA interface definitions (HelmLib, NativeLibrary) -│ ├── darwin-amd64/ # macOS Intel native library wrapper -│ ├── darwin-arm64/ # macOS Apple Silicon native library wrapper -│ ├── linux-amd64/ # Linux x64 native library wrapper -│ ├── linux-arm64/ # Linux ARM64 native library wrapper -│ └── windows-amd64/ # Windows x64 native library wrapper -├── native/ # Go source code that wraps Helm SDK -│ ├── main.go # CGO exports for JNA -│ ├── main_test.go # Go tests -│ ├── go.mod # Go module dependencies -│ └── out/ # Compiled native libraries (.dylib, .so, .dll) -├── scripts/ # Utility scripts -└── pom.xml # Parent POM (multi-module Maven project) +Java caller + → Helm.install() returns InstallCommand (helm-java/.../InstallCommand.java) + → builder methods set fields + → .call() invokes HelmLibHolder.INSTANCE.Install(installOptions) + (lib/api/.../HelmLib.java — JNA interface) + → JNA marshals InstallOptions → C struct + → //export Install in native/main.go (CGO bridge, no logic) + → helm.Install(...) in native/internal/helm/install.go (real implementation) + → returns C.Result (out/err/stdOut/stdErr) → JNA → Result → parsed into Release ``` -### Design Patterns - -1. **Fluent Builder Pattern**: All commands use method chaining for configuration - ```java - helm.install() - .withName("release") - .withNamespace("namespace") - .createNamespace() - .waitReady() - .call(); - ``` - -2. **Native Bridge via JNA**: `HelmLib` interface defines native method signatures; platform-specific modules contain the actual `.dylib`/`.so`/`.dll` files - -3. **Lazy Initialization**: Native library loaded on first use via `HelmLibHolder.INSTANCE` - -4. **Options Structs**: Go code defines C structs that map to Java options classes in `lib/api` +Native library loading: `Helm` class holds `HelmLibHolder.INSTANCE`, initialized lazily by `NativeLibrary.getInstance().load()`. `getInstance()` uses Java `ServiceLoader` to discover the platform module on the classpath (each platform JAR has `META-INF/services/com.marcnuri.helm.jni.NativeLibrary`); if none found, falls back to `RemoteJarLoader` (downloads the correct platform JAR at runtime — works for Gradle, fails when air-gapped). `helm-java/pom.xml` has OS-activated profiles that pull in the right platform module automatically; both `amd64`/`x86_64` and `arm64`/`aarch64` aliases are handled. -## Code Style +## Public API shape (Helm.java) -### Java +Verbs come in two forms where it makes sense: +- **Static** `Helm.install(chart)` — operates on an external chart path/URL. +- **Instance** `new Helm(path).install()` — operates on the chart at the `Path` the `Helm` was constructed with. -- Java 8 compatibility required (`maven.compiler.source=1.8`) -- Apache License 2.0 header on all source files -- Use AssertJ for assertions in tests -- No external mocking frameworks - use real implementations +Verbs with both forms: `install`, `template`, `show`, `upgrade`. Most others are static-only (`list`, `history`, `status`, `get`, `repo`, `registry`, `search`, `push`, `test`, `uninstall`, `version`) or instance-only (`dependency`, `lint`, `packageIt` — naming dodges the `package` keyword). -### Go (native/) +## Conventions -- Standard Go formatting (`go fmt`) -- CGO exports with `//export` comments -- C structs defined in comments for JNA interop +**Java** +- Source/target 1.8 — no `var`, no `Map.of(...)`, no Optional in APIs, no records. +- Apache 2.0 header on every `.go` and `.java` file (`make license` enforces). +- `@author` Javadoc tags maintained (`scripts/check-authors.sh`). +- Tests: JUnit 5 + AssertJ. **No mocking libraries** — use real impls (Testcontainers/KinD for k8s). -### Naming Conventions +**Go** +- `go fmt`. CGO `//export` functions in `native/main.go` are wrappers ONLY; logic goes in `native/internal/helm/.go`. -- Command classes: `{Verb}Command.java` (e.g., `InstallCommand`, `UpgradeCommand`) -- Test classes: `Helm{Feature}Test.java` (e.g., `HelmInstallTest`, `HelmKubernetesTest`) -- JNI options: `{Operation}Options.java` (e.g., `InstallOptions`, `LintOptions`) +**Naming** +- Java command: `{Verb}Command.java` (e.g. `InstallCommand`). +- Java JNI options: `{Operation}Options.java` (e.g. `InstallOptions`, `HistoryOptions`, `GetValuesOptions`). +- Result/domain types: noun, no suffix (e.g. `Release`, `Repository`, `LintResult`, `SearchResult`). +- Tests: `Helm{Feature}Test.java` (`HelmInstallTest`, `HelmKubernetesTest`); exception: infrastructure tests like `NativeLibraryTest`. +- Go: file per verb in `internal/helm/` (`install.go`, `install_test.go`). -## Testing Guidelines +## Adding a new Helm command (end-to-end) -### Philosophy +1. `native/internal/helm/.go` — write the real Go function (params struct + function returning `(string, error)`). +2. `native/internal/helm/_test.go` — Go test (use `internal/test/env.go` helpers if k8s needed). +3. `native/main.go`: + - Add `struct Options { ... }` in the CGO comment block. + - Add `//export ` wrapper calling `helm.(...)` via `runCommand(...)`. +4. `lib/api/src/main/java/com/marcnuri/helm/jni/Options.java` — JNA `Structure` mirroring the C struct (field order MUST match). +5. `lib/api/.../HelmLib.java` — add `Result (Options options);`. +6. `helm-java/src/main/java/com/marcnuri/helm/Command.java` — fluent builder + `call()` that builds the options and invokes `HelmLib`. If the command returns structured data, add a result type alongside (e.g. `Release`). +7. `helm-java/src/main/java/com/marcnuri/helm/Helm.java` — factory method (static, instance, or both). +8. `helm-java/src/test/java/com/marcnuri/helm/Helm{Verb}Test.java` — JUnit test (see structure below). +9. Rebuild native: `make build-native` then `./mvnw test -pl helm-java`. -1. **Black-box Testing**: Tests verify behavior and observable outcomes, not implementation details. Test the public API only. +## Testing -2. **Avoid Mocks**: Use real implementations and test infrastructure whenever possible. The project uses Testcontainers with KinD for Kubernetes integration tests. - -3. **Nested Test Structure**: Use JUnit 5 `@Nested` annotations with inner classes to organize tests by scenario. - -4. **Scenario-Based Setup**: Define common scenario in the outer `@BeforeEach`; define specific conditions in nested class setup. - -5. **Single Assertion Per Test**: Each test block should assert ONE specific condition for clear failure identification. - -### Test Structure Example +- Black-box: test the public Java API, not internals. +- Use `@Nested` classes to group scenarios (typical: `Valid` / `Invalid`, or per-mode like `ClientOnly` / `Kubernetes`). +- Common setup in outer `@BeforeEach`; scenario-specific in nested. +- Prefer one assertion concern per `@Test` for clear failure isolation. +- `@TempDir` as a field for shared dirs; as a parameter for per-test dirs (see `HelmInstallTest`). +- k8s integration: `HelmKubernetesTest` spins up KinD via Testcontainers in `@BeforeAll`; clean releases in `@AfterEach`. +- Surefire sets `KUBECONFIG=/dev/null` in `helm-java/pom.xml` so tests cannot accidentally hit your local cluster — pass `kubeConfigFile`/`kubeConfigContents` explicitly. ```java class HelmFeatureTest { - private Helm helm; - - @BeforeEach - void setUp(@TempDir Path tempDir) { - helm = Helm.create().withName("test").withDir(tempDir).call(); + @TempDir private Path tempDir; + private Helm helm; + + @BeforeEach + void setUp() { + helm = Helm.create().withName("test").withDir(tempDir).call(); + } + + @Nested + class Valid { + @Test + void withName() { + Release result = helm.install().clientOnly().withName("test").call(); + assertThat(result) + .returns("test", Release::getName) + .returns("deployed", Release::getStatus); } - - @Nested - class Install { - @Nested - class Valid { - @Test - void withName() { - final Release result = helm.install() - .clientOnly() - .withName("test") - .call(); - assertThat(result) - .returns("test", Release::getName) - .returns("deployed", Release::getStatus); - } - } - - @Nested - class Invalid { - @Test - void withMissingChart() { - final InstallCommand install = Helm.install("/tmp/nothing") - .clientOnly() - .withName("test"); - assertThatThrownBy(install::call) - .message() - .contains("not found"); - } - } + } + + @Nested + class Invalid { + @Test + void withMissingChart() { + InstallCommand cmd = Helm.install("/tmp/nothing").clientOnly().withName("test"); + assertThatThrownBy(cmd::call).message().contains("not found"); } + } } ``` -### Kubernetes Integration Tests +## Debugging native calls -- `HelmKubernetesTest` uses KinD via Testcontainers -- `@BeforeAll` starts the KinD cluster (expensive operation) -- Tests use `kubeConfigFile` or `kubeConfigContents` for cluster access -- Use `@AfterEach` to clean up releases to avoid test pollution - -## Common Tasks - -### Adding a New Helm Command - -1. Create options class in `lib/api`: `lib/api/src/main/java/com/marcnuri/helm/jni/{Operation}Options.java` -2. Add C struct definition in `native/main.go` -3. Implement Go function with `//export` in `native/main.go` -4. Add method to `HelmLib` interface in `lib/api` -5. Create command class in `helm-java`: `helm-java/src/main/java/com/marcnuri/helm/{Operation}Command.java` -6. Add factory method in `Helm.java` -7. Write tests in `helm-java/src/test/java/com/marcnuri/helm/Helm{Feature}Test.java` - -### Updating Native Library - -```bash -cd native -# Make changes to main.go -go test ./... # Run Go tests first -# Build for your platform -go build -buildmode=c-shared -o out/helm-darwin-10.12-arm64.dylib . -cd .. -./mvnw test -pl helm-java - -# Or use Makefile (auto-detects platform): -make build-native -``` - -### Debugging Native Calls - -Enable debug output in commands: +Most commands have `.debug()`: ```java helm.install().debug().withName("test").call(); ``` +Stdout/stderr captured by `runCommand` in `native/main.go` and surfaced on the `Result`. ## Troubleshooting -### Native library not found - -Ensure the native binaries exist in `native/out/` before running Maven. The enforcer plugin requires all platform binaries to exist: -- `helm-darwin-10.12-amd64.dylib` -- `helm-darwin-10.12-arm64.dylib` -- `helm-linux-amd64.so` -- `helm-linux-arm64.so` -- `helm-windows-4.0-amd64.dll` - -To build only for your current platform (bypassing the enforcer check): -```bash -make build-current-platform -# This sets -Denforcer.skipRules=requireFilesExist automatically -``` - -### Tests fail with "KUBECONFIG" errors - -The Surefire plugin is configured with `KUBECONFIG=/dev/null` to prevent tests from using your local kubeconfig. This is intentional. - -### KinD/Testcontainers tests hang - -Ensure Docker is running and has sufficient resources. KinD requires a working Docker environment. On macOS, increase Docker Desktop memory allocation if tests fail with OOM. +- **Enforcer "files do not exist"** — missing binaries in `native/out/`. Use `make build-current-platform` (skips the rule) for local dev. +- **`UnsatisfiedLinkError` / no NativeLibrary found** — platform module isn't on the classpath. Check the OS profile in `helm-java/pom.xml` activated for your `os.family`/`os.arch`. +- **KinD tests hang / OOM** — Docker not running or under-resourced. +- **Go build fails** — check `go version` matches `native/go.mod`; run `cd native && go mod tidy`. +- **Windows native build** — needs MinGW (CGO toolchain). -### Go build fails +## Things to ignore -Check Go version (`go version`) matches `go.mod` requirement (Go 1.25.0+). Run `go mod tidy` to sync dependencies. +- `lib/wasi/` and `native/wasm/` — abandoned WebAssembly experiment (issue [#239](https://github.com/manusa/helm-java/issues/239)). Not in the parent pom, no source files. Don't extend it; don't fix it. -### Windows-specific issues +## Audits & background -Building native libraries on Windows requires MinGW or similar CGO-compatible toolchain. \ No newline at end of file +- `docs/research/native-codebase-audit.md` — point-in-time snapshot of the Go layer (coverage, missing commands, known issues). Check the date before trusting it. diff --git a/ANALYSIS.md b/ANALYSIS.md deleted file mode 100644 index d7fa4fe7..00000000 --- a/ANALYSIS.md +++ /dev/null @@ -1,525 +0,0 @@ -# Helm-Java Go Codebase Analysis - -**Date:** 2025-11-14 -**Analyzer:** Claude Code -**Scope:** /home/user/00-MN/projects/manusa/helm-java/native - ---- - -## Executive Summary - -The helm-java Go codebase provides a comprehensive native interface to Helm functionality with 27 exported commands. The code is well-structured with good separation of concerns, but has critical gaps in test coverage (35%) and contains 1 remaining critical bug that should be addressed. Key missing features include `helm rollback`, `helm pull`, and `helm status` (though status reporting exists internally). - -**UPDATE (2025-11-14):** The panic issue in `NewCfg()` has been fixed. All panics have been replaced with proper error handling. - -### Key Metrics -- **Total Lines of Code:** ~3,311 (excluding tests) -- **Exported Functions:** 27 -- **Implementation Files:** 20 -- **Test Files:** 7 -- **Test Coverage:** 35% of implementation files -- **Error Handling Sites:** 70+ -- **Critical Bugs:** ~~2~~ 1 (panic issue fixed ✅) -- **TODOs:** 3 -- **Go Vet Issues:** 0 ✓ - ---- - -## 1. Implemented Features - -### Release Management -- **Install** - Full implementation with dependency update, dry-run, values, atomic installs -- **Upgrade** - Supports install-if-missing, force, reset/reuse values, atomic upgrades -- **Uninstall** - Includes dry-run, hooks control, cascade deletion, keep history -- **List** - Filter by status (deployed, failed, pending, etc.), all namespaces support -- **Test** - Execute chart tests with timeout configuration - -### Chart Development -- **Create** - Generate new chart scaffolding -- **Template** - Render templates locally (client-only mode) -- **Lint** - Validate chart syntax with strict/quiet modes -- **Package** - Create chart archives with optional signing -- **Show** - Display chart information (all, chart, values, readme, CRDs) - -### Dependency Management -- **DependencyBuild** - Build dependencies with keyring/verify support -- **DependencyList** - List chart dependencies -- **DependencyUpdate** - Update dependencies with verification - -### Repository Management -- **RepoAdd** - Add chart repositories with authentication -- **RepoList** - List configured repositories -- **RepoRemove** - Remove repositories -- **RepoUpdate** - Refresh repository indexes -- **SearchRepo** - Search repositories with regex support - -### Registry Operations (OCI) -- **RegistryLogin** - Authenticate to OCI registries -- **RegistryLogout** - Logout from registries -- **Push** - Push charts to OCI registries - -### Testing Infrastructure -- **RepoServerStart** - Start HTTP chart repository server -- **RepoOciServerStart** - Start OCI registry server -- **RepoServerStop/StopAll** - Manage test servers - -### Utilities -- **Version** - Get Helm library version -- **Debug capture system** - Capture stdout/stderr for operations - ---- - -## 2. Missing Helm Features - -Compared to the official Helm CLI, the following commands are **NOT** implemented: - -### Critical Missing Commands -- ❌ `helm pull` - Download charts from repositories -- ❌ `helm rollback` - Rollback to previous release revision (**CRITICAL FOR PRODUCTION**) -- ❌ `helm status` - Display release status (internal `StatusReport()` exists but not exposed) -- ❌ `helm history` - View release history -- ❌ `helm get` - Retrieve release information (all, hooks, manifest, metadata, notes, values) -- ❌ `helm verify` - Verify chart signature - -### Additional Missing Features -- `helm env` - Display Helm environment information -- `helm completion` - Shell completion generation -- `helm plugin` - Plugin management commands (install, list, update, uninstall) -- `helm search hub` - Search Artifact Hub (only search repo implemented) -- `helm repo index` - Generate repository index file - -### Configuration Options Not Exposed -- Context management (no context switching support) -- Advanced timeout configurations for some operations -- Some debug/verbose output options - ---- - -## 3. Test Coverage Analysis - -### Files WITH Tests (7/20 = 35%) -- ✅ `debug.go` - Has `debug_test.go` (comprehensive coverage) -- ✅ `helm.go` - Has `helm_test.go` -- ✅ `install.go` - Has `install_test.go` (7+ test cases) -- ✅ `plugins.go` - Has `plugins_test.go` (auth provider tests) -- ✅ `template.go` - Has `template_test.go` (2+ test cases) -- ✅ `upgrade.go` - Has `upgrade_test.go` -- ✅ Integration tests in `envtest_test.go` (12+ test cases) - -### Files WITHOUT Tests (13/20 = 65%) -- ❌ `create.go` - **NO TESTS** -- ❌ `dependency.go` - **NO TESTS** -- ❌ `lint.go` - **NO TESTS** -- ❌ `list.go` - **NO TESTS** -- ❌ `package.go` - **NO TESTS** -- ❌ `push.go` - **NO TESTS** -- ❌ `registry.go` - **NO TESTS** -- ❌ `repo.go` - **NO TESTS** -- ❌ `repotest.go` - **NO TESTS** (testing infrastructure itself untested) -- ❌ `search.go` - **NO TESTS** -- ❌ `show.go` - **NO TESTS** -- ❌ `test.go` - **NO TESTS** -- ❌ `uninstall.go` - **NO TESTS** -- ❌ `version.go` - **NO TESTS** - -### Test Quality -- **Total test cases:** 26+ identified test functions -- **All tests passed:** Yes (last run: 201.641s execution time) -- **Integration tests:** Uses envtest for Kubernetes interaction -- **Good coverage areas:** Install/Upgrade critical paths, Template rendering -- **Critical gap:** Repository operations, dependency management, and chart packaging have **ZERO unit tests** - ---- - -## 4. Code Quality Observations - -### ✅ Strengths -1. **Consistent error handling** - 70+ proper error checks across codebase -2. **Good separation of concerns** - Each file handles specific Helm command -3. **Proper licensing** - Apache 2.0 headers on all files -4. **Manageable file sizes** - Largest file: install.go at 9.5KB -5. **References to upstream** - Comments link to official Helm implementation -6. **Clean go vet output** - No warnings - -### 🐛 Critical Issues - -#### 1. ~~Panic Usage in Production Code~~ ✅ FIXED -**Location:** `helm.go:81, 89` - -```go -if err != nil { - panic(err) // ❌ Crashes entire process! -} -``` - -**Status:** ✅ **FIXED** - `NewCfg()` now returns `(*action.Configuration, error)` and all callers have been updated to handle errors gracefully. - -**Changes Made:** -- Changed `NewCfg` signature to return error -- Updated all 14 callers (9 implementation files + 5 test files) -- Replaced panics with proper error returns using `fmt.Errorf` with error wrapping - -#### 2. Context Cancellation Leak -**Location:** `install.go:167` - -```go -ctx, cancel := context.WithCancel(ctx) -// ❌ Missing: defer cancel() -``` - -**Impact:** MEDIUM - Potential goroutine leak if context is not properly cancelled. - -**Recommendation:** Add deferred cancellation: -```go -ctx, cancel := context.WithCancel(ctx) -defer cancel() -``` - -#### 3. Missing ValuesFiles in Upgrade Fallback -**Location:** `upgrade.go:104` - -When upgrade falls back to install, the `ValuesFiles` parameter is not passed, only `Values`. - -**Impact:** MEDIUM - User-specified values files will be ignored during install fallback. - -**Recommendation:** Add missing parameter: -```go -ValuesFiles: options.ValuesFiles, -``` - -### 📝 TODOs/FIXMEs (3 Found) - -1. **helm.go:78** - "TODO: we could actually merge both kubeconfigs" - - Currently KubeConfigContents overrides instead of merging - - Impact: Users cannot combine multiple kubeconfig sources - -2. **search.go:69** - "TODO: see how to propagate warnings to the Java implementation" - - Warnings are silently suppressed - - Impact: Users miss important repository warnings - -3. **repotest.go:138** - "TODO can't be stopped for now" - - OCI server cannot be properly stopped - - Impact: Resource leaks in test environments - -### 🔄 Code Duplication & Inconsistencies - -#### Certificate Options Handling -**Occurrences:** 63+ repetitions across codebase - -```go -CertOptions: helm.CertOptions{ - CertFile: C.GoString(options.certFile), - KeyFile: C.GoString(options.keyFile), - CaFile: C.GoString(options.caFile), - InsecureSkipTLSverify: options.insecureSkipTlsVerify == 1, - PlainHttp: options.plainHttp == 1, - Keyring: C.GoString(options.keyring), -} -``` - -**Recommendation:** Extract to helper function. - -#### Registry Client Creation -**Duplicated in:** install.go, upgrade.go, push.go, registry.go, dependency.go - -**Recommendation:** Extract common pattern: -```go -func newRegistryClientWithDebug(opts CertOptions, debug bool) (*registry.Client, *DebugCapture, error) -``` - -#### Debug Output Handling -**Inconsistency:** Some functions use `debugCapture` (registry, dependency) while others use `kubeOut` buffer. - -**Recommendation:** Standardize on single approach. - -### ⚠️ Potential Issues - -#### 1. Silent Error Suppression -**Location:** `search.go:68-71` - -```go -if err != nil { - // TODO: see how to propagate warnings to the Java implementation - continue // ❌ Silently skips broken repositories -} -``` - -**Impact:** Users unaware of repository configuration issues. - -#### 2. Race Condition Potential -**Location:** `repotest.go:124` - -```go -go server.ListenAndServe() // ❌ No wait/ready check before returning -``` - -**Impact:** Server may not be ready when function returns, causing test flakiness. - -#### 3. No Retry on Lock Failure -**Location:** `repo.go:71` - -```go -lockCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) -defer cancel() -``` - -Good timeout, but no retry mechanism if lock acquisition fails. - ---- - -## 5. Recommended Enhancements - -### Priority 1 - Critical Fixes (Do First!) - -1. ~~**Replace panics with error returns**~~ ✅ **FIXED** - - ~~Lines: 81, 89~~ - - ~~Effort: Low (1-2 hours)~~ - - ~~Impact: High (prevents process crashes)~~ - - **Status:** Completed - `NewCfg` now properly returns errors - -2. **Add defer cancel()** in `install.go` context management - - Line: 167 - - Effort: Trivial (5 minutes) - - Impact: Medium (prevents goroutine leaks) - -3. **Fix ValuesFiles** missing in upgrade fallback to install - - File: `upgrade.go:104` - - Effort: Trivial (5 minutes) - - Impact: Medium (fixes values file handling) - -4. **Add context cancellation** to `upgrade.go` as `install.go` has it - - Effort: Low (1 hour) - - Impact: Medium (consistency and leak prevention) - -### Priority 2 - Missing Core Features - -1. **Implement `helm pull`** - - Commonly used for downloading charts - - Effort: Medium (4-8 hours) - - Impact: High - -2. **Implement `helm rollback`** - - **CRITICAL for production use** - - Effort: Medium (4-8 hours) - - Impact: Critical - -3. **Implement `helm status`** - - Already has `StatusReport()`, just needs export - - Effort: Low (2-4 hours) - - Impact: High - -4. **Implement `helm history`** - - View release revisions - - Effort: Medium (4-6 hours) - - Impact: High - -5. **Implement `helm get` subcommands** - - Access release data (manifest, values, hooks, etc.) - - Effort: Medium (6-10 hours) - - Impact: High - -### Priority 3 - Test Coverage Improvements - -**Target: 80%+ coverage (currently 35%)** - -1. **Add unit tests for repository operations** (`repo.go`) - - Currently 0% coverage - - Effort: High (8-12 hours) - - Impact: Critical (repos are core functionality) - -2. **Add tests for dependency management** (`dependency.go`) - - Currently 0% coverage - - Effort: Medium (6-8 hours) - - Impact: High - -3. **Add tests for package/push operations** - - Files: `package.go`, `push.go` - - Effort: Medium (4-6 hours) - - Impact: Medium - -4. **Add tests for search functionality** (`search.go`) - - Effort: Low (2-4 hours) - - Impact: Medium - -5. **Add tests for create, lint, show, version** - - Simple operations, easy to test - - Effort: Low (4-6 hours total) - - Impact: Medium-High - -### Priority 4 - Refactoring - -1. **Extract certificate handling helper** - -```go -func applyCertOptions(client interface{}, opts CertOptions) { - // Centralize cert file application logic -} -``` - -Effort: Medium (4-6 hours) -Impact: High (eliminates 63+ duplications) - -2. **Extract registry client creation** - -```go -func newRegistryClientWithDebug(opts CertOptions, debug bool) (*registry.Client, *DebugCapture, error) { - // Reusable across install, upgrade, push, registry, dependency -} -``` - -Effort: Medium (3-5 hours) -Impact: Medium (reduces duplication) - -3. **Standardize debug output handling** - - Create consistent interface for debug capture - - Apply uniformly across all commands - - Effort: Medium (4-6 hours) - - Impact: Medium (better consistency) - -4. **Add error context with wrapping** - -```go -return fmt.Errorf("failed to load chart %s: %w", chartPath, err) -``` - -Effort: Low (2-3 hours) -Impact: Medium (better debugging) - -### Priority 5 - Code Quality & Documentation - -1. **Add godoc comments** to exported types and functions - - Effort: Medium (6-8 hours) - - Impact: High (improves maintainability) - -2. **Implement proper shutdown** for OCI test server - - Resolve `repotest.go` TODO - - Effort: Medium (3-5 hours) - - Impact: Low (test infrastructure only) - -3. **Add warning propagation mechanism** - - Resolve `search.go` TODO - - Effort: Medium (4-6 hours) - - Impact: Medium (better error visibility) - -4. **Consider kubeconfig merging** instead of override - - Resolve `helm.go` TODO - - Effort: High (8-12 hours) - - Impact: Medium (better flexibility) - -5. **Add validation** for option combinations - - Example: Validate DryRun modes are compatible - - Effort: Medium (4-6 hours) - - Impact: Medium (prevents user errors) - -6. **Add metrics/observability hooks** - - Effort: High (12-16 hours) - - Impact: Low-Medium (production monitoring) - -### Priority 6 - Performance Optimizations - -1. **Add connection pooling** for registry clients - - Effort: Medium (6-8 hours) - - Impact: Medium (faster repeated operations) - -2. **Cache repository indexes** when possible - - Effort: Medium (4-6 hours) - - Impact: Medium (reduces network calls) - -3. **Parallel dependency downloads** - - Already parallel in repo update - - Could extend to dependency operations - - Effort: Low (2-4 hours) - - Impact: Low (marginal speedup) - ---- - -## Quick Wins (High Value, Low Effort) - -1. ⚡ **Add tests for `create.go`, `version.go`** - - Simplest operations to test - - Effort: 2-3 hours - - Impact: Improves coverage by 10% - -2. ⚡ **Expose `helm status`** - - `StatusReport()` already implemented - - Effort: 2-4 hours - - Impact: Adds critical missing feature - -3. ⚡ **Fix the 3 TODOs** - - Clear requirements, straightforward implementations - - Effort: 6-10 hours total - - Impact: Resolves known issues - -4. ⚡ **Add defer cancel()** - - One-line fix - - Effort: 5 minutes - - Impact: Prevents resource leaks - ---- - -## Testing Recommendations - -### Immediate Actions -1. Add tests for all untested files (13 files) -2. Focus first on critical paths: repo.go, dependency.go -3. Achieve minimum 80% code coverage -4. Add integration tests for missing features - -### Test Structure (Follow Existing Pattern) -```go -func TestFeature(t *testing.T) { - t.Run("should handle expected case", func(t *testing.T) { - // Single assertion - if got != want { - t.Errorf("Expected %v, got %v", want, got) - } - }) - t.Run("should handle error case", func(t *testing.T) { - // Single assertion - if err == nil { - t.Error("Expected error, got nil") - } - }) -} -``` - -### Coverage Goals -- **Phase 1:** 50% coverage (add 15% - ~20 hours) -- **Phase 2:** 70% coverage (add 20% - ~30 hours) -- **Phase 3:** 85% coverage (add 15% - ~20 hours) - ---- - -## Conclusion - -The helm-java Go codebase is **well-architected** and implements the majority of Helm's core functionality. The code follows good practices with consistent error handling and clear separation of concerns. - -### Strengths -✅ Comprehensive command coverage (27 commands) -✅ Good code organization -✅ Proper error handling patterns -✅ Zero go vet warnings -✅ Well-tested critical paths (install, upgrade, template) - -### Areas for Improvement -⚠️ **Critical:** ~~2~~ 1 bug needs immediate fixing (~~panic~~ ✅ fixed, missing defer remains) -⚠️ **High Priority:** Missing production-critical features (rollback, status, history) -⚠️ **Medium Priority:** Test coverage at only 35% (target: 80%+) -⚠️ **Low Priority:** Code duplication and TODO resolution - -### Production Readiness Assessment -- **Current State:** Good for non-production use, comprehensive feature set -- **Blocking Issues:** ~~Panic bugs~~ ✅ (fixed), missing rollback feature, context leak -- **Time to Production Ready:** ~2 weeks of focused development - - Week 1: ~~Fix critical bugs~~ (panic fixed ✅), fix context leak, add rollback/status/history - - Week 2: Improve test coverage to 70%+ - - Week 3: Address refactoring and remaining tests - -The codebase provides excellent foundation and with focused effort on the identified issues, can become production-ready quickly. - ---- - -**Generated by:** Claude Code -**Analysis Date:** 2025-11-14 -**Total Analysis Time:** ~30 minutes -**Files Analyzed:** 20 implementation files, 7 test files diff --git a/docs/research/native-codebase-audit.md b/docs/research/native-codebase-audit.md new file mode 100644 index 00000000..ba539842 --- /dev/null +++ b/docs/research/native-codebase-audit.md @@ -0,0 +1,77 @@ +# Native Go Codebase — Audit + +_Date: 2026-05-21_ +_Scope: `native/` (CGO bridge + `internal/helm` implementations)_ +_Related: [#101](https://github.com/manusa/helm-java/issues/101) (pull), [#351](https://github.com/manusa/helm-java/issues/351) (rollback), [#239](https://github.com/manusa/helm-java/issues/239) (wasi, abandoned). Supersedes the original 2025-11-14 revision (see git history of this file)._ + +Point-in-time snapshot of what's implemented, what's missing vs upstream Helm, and known issues. Re-run when something here looks suspect — file counts and TODO line numbers go stale fast. + +## TL;DR + +The Go bridge exposes **30 functions** across **23 implementation files**, covering every common Helm verb except **rollback** and **pull**. Since the previous audit (2025-11-14), `status`, `history`, and `get values` have been added (PRs [#352](https://github.com/manusa/helm-java/pull/352), [#353](https://github.com/manusa/helm-java/pull/353), [#345](https://github.com/manusa/helm-java/pull/345)); the `NewCfg` panic was fixed; the `newRegistryClient` extraction landed. Test coverage remains thin (7 of 23 files, ~30%) — three new commands shipped untested. Two known correctness bugs from the prior audit are still open: install-only `defer cancel()` leak, and the search.go warning swallow. + +## What changed since 2025-11-14 + +Verified against current source and merged PRs. + +| Previous finding | Status | +|---|---| +| `NewCfg()` panics in `helm.go:81,89` | **Fixed** — now returns `(*action.Configuration, error)`; all call sites handle it. | +| Missing `helm status` | **Shipped** in PR [#352](https://github.com/manusa/helm-java/pull/352) (issue [#349](https://github.com/manusa/helm-java/issues/349)). `native/internal/helm/status.go` + Java `StatusCommand`. | +| Missing `helm history` | **Shipped** in PR [#353](https://github.com/manusa/helm-java/pull/353) (issue [#350](https://github.com/manusa/helm-java/issues/350)). `native/internal/helm/history.go` + Java `HistoryCommand`. | +| Missing `helm get` | **Partially shipped**: `get values` only, in PR [#345](https://github.com/manusa/helm-java/pull/345) (issue [#322](https://github.com/manusa/helm-java/issues/322)). Other subcommands (manifest, hooks, notes, metadata, all) still unimplemented. | +| `ValuesFiles` missing in upgrade→install fallback | **Fixed** — `upgrade.go:114` now forwards `ValuesFiles`. | +| `newRegistryClient` extraction recommended | **Done** — defined in `registry.go:94`, reused across install/upgrade/push/show/dependency/registry (8 call sites). | +| Install context leak (`install.go:167`, no `defer cancel()`) | **Still open.** Now at `install.go:182`; the `cancel()` only fires from the signal goroutine — if no signal arrives, the context isn't cancelled until process exit. Upgrade does *not* have the same shape: `upgrade.go:171` uses `context.Background()` with no cancellation path at all (no leak, but also no signal handling). | +| TODO `helm.go:78` — merge kubeconfigs instead of override | **Still open.** | +| TODO `search.go:69` — propagate warnings | **Still open** — broken repos are silently `continue`d. | +| TODO `repotest.go:138` — OCI server can't be stopped | **Still open** — resource leak in test infra. | + +## Current command coverage + +Exported functions: 30. Source of truth: `lib/api/src/main/java/com/marcnuri/helm/jni/HelmLib.java`. + +**Implemented:** create, install, upgrade, uninstall, list, test, template, lint, package, show, dependency (build/list/update), repo (add/list/remove/update), search (repo), registry (login/logout), push, history, status, getValues, repoServerStart/repoOciServerStart/repoServerStop/repoServerStopAll, version. + +**Missing vs upstream Helm CLI:** + +| Command | Tracking issue | Notes | +|---|---|---| +| `helm rollback` | [#351](https://github.com/manusa/helm-java/issues/351) (open) | Production-critical gap. | +| `helm pull` | [#101](https://github.com/manusa/helm-java/issues/101) (open) | Chart download from repos/OCI. | +| `helm get {manifest,hooks,notes,metadata,all}` | — | Only `get values` is implemented (PR [#345](https://github.com/manusa/helm-java/pull/345)). | +| `helm verify` | — | Chart signature verification. | +| `helm search hub` | — | Only `search repo` is implemented. | +| `helm repo index` | — | Generate index file. | +| `helm env`, `helm completion`, `helm plugin` | — | Lower priority; CLI-flavoured. | + +## Test coverage + +7 test files for 23 implementation files (~30%). Worse than the previous audit (35%) because `get.go`, `history.go`, `status.go` shipped without `_test.go` siblings. (Java-side tests in `helm-java/src/test/` do exercise these end-to-end, but the Go layer has no isolated tests.) + +**With Go tests:** `debug.go`, `helm.go`, `install.go`, `plugins.go`, `template.go`, `upgrade.go`, plus integration coverage in `envtest_test.go`. + +**Without Go tests:** `create.go`, `dependency.go`, `get.go`, `history.go`, `lint.go`, `list.go`, `package.go`, `push.go`, `registry.go`, `repo.go`, `repotest.go`, `search.go`, `show.go`, `status.go`, `test.go`, `uninstall.go`, `version.go`. + +Highest-value gaps to close at the Go layer: `repo.go` and `dependency.go` (network + filesystem; easy to regress); `repotest.go` (test infra is itself untested). + +## Open code-quality items + +Verified against current `HEAD`. + +1. **Context leak in install.** `install.go:182` creates a cancellable context but only cancels on signal — add `defer cancel()` (trivial fix, prevents goroutine leak). Upgrade is a separate question: `upgrade.go:171` uses `context.Background()` with no signal handling at all, so the decision is whether to grow install's WithCancel + signal pattern there (and add `defer cancel()` from the start). +2. **Silent error suppression in search.** `search.go:69` skips broken repositories without surfacing to Java callers. TODO is explicit. +3. **OCI test server can't be stopped.** `repotest.go:138` (`server.ociServer.Stop()` commented out). Affects test-resource cleanup. +4. **Kubeconfig override vs merge.** `helm.go:78` — `KubeConfigContents` clobbers `KubeConfig` instead of merging. TODO is explicit. +5. **`CertOptions` boilerplate** appears in ~9 places. Lower-priority refactor; the `newRegistryClient` extraction already absorbed the worst of the duplication. + +## Recommended next work + +Ordered by impact-per-effort: + +1. **`defer cancel()` in install.** 5-minute fix; closes a real leak. (Upgrade is a separate decision — see Open code-quality items.) +2. **`helm rollback`** ([#351](https://github.com/manusa/helm-java/issues/351)). Highest-impact missing feature for production users. +3. **Backfill Go tests for `status.go`, `history.go`, `get.go`.** They shipped untested; cheapest coverage wins. +4. **`helm pull`** ([#101](https://github.com/manusa/helm-java/issues/101)). +5. **Finish `helm get`** (manifest/hooks/notes/metadata) on top of the existing `get values` scaffolding. +6. **Warning propagation** (`search.go` TODO) — needs a Java-side mechanism, so design first.