Skip to content

feat: implement helm history command#353

Open
gcarda99 wants to merge 12 commits intomanusa:mainfrom
gcarda99:main
Open

feat: implement helm history command#353
gcarda99 wants to merge 12 commits intomanusa:mainfrom
gcarda99:main

Conversation

@gcarda99
Copy link

@gcarda99 gcarda99 commented Feb 9, 2026

Closes #350

Summary

• Adds support for the helm history command
• Implements the full stack: Go native layer, JNA bridge, and Java command API

What's Changed

Go Implementation (native/helm/history.go)

  • Implemented History() function with HistoryOptions struct
  • Applied manual Max filter (Helm's action.History.Run() doesn't honor the Max field)
  • Output formatted as URL-encoded key-value pairs for consistency with other commands

Java API

  • Added HistoryCommand with fluent API builder pattern
  • Added ReleaseHistory model to parse release history entries
  • Exposed Helm.history(String releaseName) static method
  • Supports withMax(), withNamespace(), withKubeConfig(), withKubeConfigContents()

Tests

  • Added comprehensive integration tests using KindContainer
  • Covered valid scenarios: install, upgrade, max filter, namespace isolation, kubeconfig options
  • Added invalid scenario test for non-existent releases

Test plan

  • Go tests pass (go test ./... in native/)
  • Java compilation passes (mvn clean install -Dquickly)
  • Integration tests pass with KindContainer (mvn test -Dtest=HelmHistoryTest)
  • Manual verification: history retrieval works correctly with real cluster

Usage

// Get history for a release
List<ReleaseHistory> history = Helm.history("my-release")
    .withKubeConfig(kubeConfigPath)
    .call();

// Get last 2 revisions only
List<ReleaseHistory> recentHistory = Helm.history("my-release")
    .withMax(2)
    .withKubeConfig(kubeConfigPath)
    .call();

// Get history in specific namespace
List<ReleaseHistory> history = Helm.history("my-release")
    .withNamespace("production")
    .withKubeConfig(kubeConfigPath)
    .call();

// Get history using kubeconfig contents
List<ReleaseHistory> history = Helm.history("my-release")
    .withKubeConfigContents(kubeConfigYaml)
    .call();

Copy link
Owner

@manusa manusa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Review: #353

Summary

This PR implements the helm history command as specified in issue #350. It adds the full stack implementation: Go native layer, JNA bridge, and Java command API. The implementation allows users to retrieve release history with options for max revisions, namespace, and kubeconfig.

Review Verdict

REQUEST_CHANGES

Findings

Critical Issues (Must Fix)

  1. Missing Apache License header in ReleaseHistory.java

    All source files in this project require the Apache License 2.0 header. The ReleaseHistory.java file is missing this header. Compare with Release.java which has the proper license header.

    The file should start with:

    /*
     * Copyright 2024 Marc Nuri
     *
     * Licensed under the Apache License, Version 2.0 (the "License");
     * you may not use this file except in compliance with the License.
     * You may obtain a copy of the License at
     *
     *     http://www.apache.org/licenses/LICENSE-2.0
     *
     * Unless required by applicable law or agreed to in writing, software
     * distributed under the License is distributed on an "AS IS" BASIS,
     * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
     * See the License for the specific language governing permissions and
     * limitations under the License.
     */
  2. Go implementation should use url.Values pattern (native/internal/helm/history.go)

    The current implementation uses fmt.Sprintf with url.QueryEscape:

    out.WriteString(fmt.Sprintf("revision=%d&updated=%s&status=%s&chart=%s&appVersion=%s&description=%s\n",
        rel.Version,
        url.QueryEscape(rel.Info.LastDeployed.Format(time.RFC1123Z)),
        ...

    For consistency with the rest of the codebase, this should use url.Values like list.go does:

    out := bytes.NewBuffer(make([]byte, 0))
    for _, rel := range releases {
        values := make(url.Values)
        values.Set("revision", strconv.Itoa(rel.Version))
        if tspb := rel.Info.LastDeployed; !tspb.IsZero() {
            values.Set("updated", tspb.Format(time.RFC1123Z))
        }
        values.Set("status", rel.Info.Status.String())
        values.Set("chart", formatChartname(rel.Chart))
        values.Set("appVersion", formatAppVersion(rel.Chart))
        values.Set("description", rel.Info.Description)
        _, _ = fmt.Fprintln(out, values.Encode())
    }

    This also addresses the need to use formatAppVersion() helper which handles nil chart edge cases (per Helm issue #1347).

  3. Tests should be nested in HelmKubernetesTest for performance

    All tests using KinD container are grouped within HelmKubernetesTest as nested classes. This avoids spinning up multiple KinD containers (expensive operation) across different test files.

    The HelmHistoryTest class should be refactored to become a nested class History inside HelmKubernetesTest, following the same pattern as HelmList, Uninstall, Upgrade, etc.

    I've updated issue #350 to reflect this testing pattern requirement.

Suggestions (Should Consider)

  1. Test assertions could be more idiomatic

    Multiple assertThat() calls on the same object could be combined for better readability:

    // Current:
    assertThat(releaseHistories).hasSize(1);
    assertThat(releaseHistories.get(0).getRevision()).isEqualTo(1);
    assertThat(releaseHistories.get(0).getDescription()).containsIgnoringCase("Install complete");
    
    // Suggested:
    assertThat(releaseHistories)
        .hasSize(1)
        .first()
        .returns(1, ReleaseHistory::getRevision)
        .extracting(ReleaseHistory::getDescription).asString()
        .containsIgnoringCase("Install complete");

    This follows the pattern used elsewhere in HelmKubernetesTest.

Security Concerns

None identified. The implementation properly handles kubeconfig and uses URL encoding for data serialization.

Testing Assessment

Coverage: Good

The test suite covers:

  • ✅ Basic history after install
  • ✅ History after upgrade (multiple revisions)
  • ✅ Max filter functionality
  • ✅ Namespace isolation
  • ✅ KubeConfig file path
  • ✅ KubeConfig contents
  • ✅ Error case (non-existent release)

Overall Assessment

This is a well-structured PR that follows the existing patterns in the codebase. The implementation is clean and matches the API design specified in issue #350. The Go implementation correctly handles the manual Max filter (with appropriate documentation explaining why).

Three critical issues must be addressed before merge:

  1. Add the missing Apache License header to ReleaseHistory.java
  2. Refactor Go code to use url.Values pattern and formatAppVersion() helper
  3. Move tests to be nested within HelmKubernetesTest

After these fixes, the PR is ready to merge.


🤖 Generated with Claude Code

@gcarda99
Copy link
Author

Thanks for the review. As soon as I have some time, I'll fix the issues.

@gcarda99 gcarda99 requested a review from manusa February 13, 2026 11:10
@gcarda99
Copy link
Author

@manusa - The requested changes have been applied

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: implement helm history command

2 participants