Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helm sanity check #1814

Open
2 tasks
saksmt opened this issue Feb 17, 2024 · 0 comments
Open
2 tasks

Helm sanity check #1814

saksmt opened this issue Feb 17, 2024 · 0 comments
Labels
effort/medium 1 week tops feature-request New/Enhanced functionality wanted priority/p2 Dependent on community feedback. PR's are welcome :)

Comments

@saksmt
Copy link

saksmt commented Feb 17, 2024

Make some sort of sanity check for helm

Use Case:

This will prevent embarassingly stupid mistakes (like writing buildInputs = pkgs.helm in nix) from failing with undecipherable error messages. Current checks seems to be limited to just verifying that binary exists, but it does not actually check whether it is kubernetes related helm and not something else entirely.

Proposed Solution:

There could be added a check for helm version output, which can be additionally used to verify minimal accepted helm version. Alternatively sample hello-world chart or something similar could be run.

Other:

Current behavior given following setup for nix project:
./default.nix

let
  pkgs = import <nixpkgs> {};
in {
  shell = pkgs.mkShell {
    buildInputs = with pkgs; [
      nodejs
      helm # <-- this is not helm you're looking for!
    ];
  };
}

leads to incomprehensible errors from typescript cdk8s package:

Synthesizing application
$PROJECT_PATH/node_modules/cdk8s/src/api-object.ts:230
  const v = apiVersion.split('/');
                       ^
TypeError: Cannot read properties of undefined (reading 'split')
    at parseApiGroup ($PROJECT_PATH/node_modules/cdk8s/src/api-object.ts:230:24)
    at new ApiObject ($PROJECT_PATH/node_modules/cdk8s/src/api-object.ts:148:21)
    at new Include ($PROJECT_PATH/node_modules/cdk8s/src/include.ts:33:7)
    at new Helm ($PROJECT_PATH/node_modules/cdk8s/src/helm.ts:131:5)
    ...

Obviously it is low-priority feature and can only be met at absurdly stupid situations (like one described above), but nevertheless it is a worthy feature at least as a "good first issue" material.

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@saksmt saksmt added feature-request New/Enhanced functionality wanted needs-triage Priority and effort undetermined yet labels Feb 17, 2024
@iliapolo iliapolo added effort/medium 1 week tops priority/p2 Dependent on community feedback. PR's are welcome :) and removed needs-triage Priority and effort undetermined yet labels Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/medium 1 week tops feature-request New/Enhanced functionality wanted priority/p2 Dependent on community feedback. PR's are welcome :)
Projects
None yet
Development

No branches or pull requests

2 participants