gruntwork-io / terratest

Terratest is a Go library that makes it easier to write automated tests for your infrastructure code.
https://terratest.gruntwork.io/
Apache License 2.0
7.52k stars 1.33k forks source link

helm/testing: Support for umbrella charts. #342

Open tariq1890 opened 5 years ago

tariq1890 commented 5 years ago

Thanks for this project, do you have any plans to support unit testing of helm charts which import sub-charts?

yorinasub17 commented 5 years ago

Hi @tariq1890 , can you elaborate on the use cases of the testing? Specifically, I haven't personally worked with umbrella charts yet, so I am not sure what about the current functions prevent testing them. If there are new functions you think would be useful in testing umbrella charts, knowing those would be helpful as well! We certainly would want to support it if there are deficiencies in the current code!

tariq1890 commented 5 years ago

In our org, we have a standard helm template chart that is imported in all the charts that we write and maintain.

Unit tests would we very useful if terratest can detect these imported charts and render templates successfully.

dee-kryvenko commented 4 years ago

Similarly to how terratest does terraform init, it also has to take care of helm dependencies before running tests.

I was thinking of the following algorithm:

yorinasub17 commented 4 years ago

Thanks for those pointers! Perhaps for the short term, we can treat this the same as how terratest handles terraform init? That is, expose a manual function that does this (helm.UpdateDependencies?) and call that in your test based on what chart you are testing?

dee-kryvenko commented 4 years ago

Yeah, that's what I was thinking about. Here's some code I came up with to workaround this for now:

func ensureHelmDependencies(t *testing.T, chartDir string) {
    if !files.FileExists(chartDir) {
        require.FailNow(t, "Chart not found: "+chartDir)
    }

    if !files.FileExists(chartDir + "/requirements.yaml") {
        logger.Logf(t, "Skip chart dependencies - no requirements.yaml")
        return
    }

    if files.FileExists(chartDir + "/charts") {
        logger.Logf(t, "Skip chart dependencies - charts folder already present")
        return
    }

    var buildOrUpdate string
    if files.FileExists(chartDir + "/requirements.lock") {
        buildOrUpdate = "build"
    } else {
        buildOrUpdate = "update"
    }

    if _, err := helm.RunHelmCommandAndGetOutputE(t, &helm.Options{}, "dependency", buildOrUpdate, chartDir); err != nil {
        require.NoError(t, err)
    }
}
xhu-cloudera commented 2 years ago

Is requirements.yaml file necessary? I ran helm dependency update and it will create a charts folder, with dependencies in tgz files. But I don't see a requirements.yaml file. Moreover, right now, the code check for existence of individual template files, however, with dependencies in zip files, that is not the case anymore. What do you guys think the best way to approach this?

xhu-cloudera commented 2 years ago

Is someone working on this? I would like to give it a try.

dee-kryvenko commented 2 years ago

Not anymore - helm got rid of requirements.yaml since I wrote my comment, and now dependency info is embedded into the Chart.yaml. Lock file is still there though.

xhu-cloudera commented 2 years ago

Here are some thoughts on conditional checking file existence for template files.

    for _, templateFile := range templateFiles {
        // validate this is a valid template file
        absTemplateFile := filepath.Join(absChartDir, templateFile)
                // Check file existence only if the template file is from the current chart, not in dependencies
        if !strings.HasPrefix(templateFile, "charts") && !files.FileExists(absTemplateFile) {
            return "", errors.WithStackTrace(TemplateFileNotFoundError{Path: templateFile, ChartDir: absChartDir})
        }

        // Note: we only get the abs template file path to check it actually exists, but the `helm template` command
        // expects the relative path from the chart.
        args = append(args, "--show-only", templateFile)
    }
aabouzaid commented 2 years ago

Is there any update about this issue?

I have an umbrella chart with sub-charts, and Terratest fails when the parent chart uses templates from the sub-charts ... it's not able to resolve the helper function names from the sub-charts (I want to use a helper function from the sub-chart in the umbrella chart).

I will create a PoC issue report soon.

grem11n commented 2 years ago

One can workaround this issue by managing dependencies separately. For example, executing helm repo add ... and helm dep update ... as a separate step in CI.

However, if dependency charts are packaged with the standard Helm mechanism e.g. as a .tgz archive, you cannot use RenderTemplate() and RenderTemplateE() functions. This validation step would fail.

Potentially, be removing that validation step Terratest can allow running templating against those packaged sub-charts. An obvious downside for this is that it has to rely on Helm error messages in this case, which are not always clear enough.

Another possibility is to check if a sub-chart's template is provided there. For example, if a templateFile has charts/ prefix, then skip that check.

For now, one can use RunHelmCommandAndGetStdOutE() function to render sub-chart's templates, but you have to re-implement all the helpers of the RenderTemplateE() function yourself in this case.

aabouzaid commented 2 years ago

OK, just by installing the dependencies (using helm dependency update) before running tests, I was able to access a dependency from the sub-chart and write a test for it.

Here is the hierarchy:

main chart |== has ==> sub-chart |== depends_on ==> external chart

I'm not sure what precisely the goal to achieve in the current issue is. Since there is neither Definition of Done nor Acceptance Criteria.