Fix fabric8:helm feature in kubernetes mode

[Beennnn]

Why do I need HELM ?

The documentation https://maven.fabric8.io/#fabric8:helm states

Please raise your voice if you want to keep it. Even better, if you want to support this goal, we are always looking for contributions ;-)

So, I raise my voice... 👍 I think the HELM feature can be considered as the de-facto standard for kubernetes configuration file packaging:

• Artifactory hosts HELM repositories:https://jfrog.com/integration/helm-repository/
• HELM is identified in trends: https://containerjournal.com/2019/04/15/trend-micro-advances-devsecops-via-kubernetes-integration/
• Todays news are also about HELM : https://www.zdnet.com/article/airship-1-0-marries-kubernetes-and-openstack-for-5gs-good/
• Helm provides a standardized directory of ready-to-deploy kubernetes applications: https://hub.helm.sh/

Why fabric8:helm ?

Because I love fabric8 , and because fabric8 provides HELM support. 👍

Why fabric8:helm does not work ?

I tried to run fabric8:helm using the documentation, with what I consider as a general use case as described here : #1072 (comment)

It did not work as is, but with several bug fixes I successfully made it work. I propose contributions below to fix it.

FIX BUG 1: use of .yaml extensions instead of .yml

HELM uses ".yaml" extension for

• the main files (Chart.yaml,values.yaml ) : https://helm.sh/docs/developing_charts/#charts
• the templates (templates/*.yaml ) : https://helm.sh/docs/chart_template_guide/#a-first-template

But the fabric core code is base on ".yml" extension: https://github.com/fabric8io/fabric8-maven-plugin/blob/master/core/src/main/java/io/fabric8/maven/core/util/ResourceFileType.java#L42

yaml("yml","yml") {
    @Override
    public ObjectMapper getObjectMapper() {
        return new ObjectMapper(new YAMLFactory()
           .configure(YAMLGenerator.Feature.MINIMIZE_QUOTES, true)
           .configure(YAMLGenerator.Feature.ALWAYS_QUOTE_NUMBERS_AS_STRINGS, true));
    }

As a consequence, generating HELM resources will required converting ".yml" extensions to ".yaml".

This is taken into account into the code of the HelmMojo, but not in the code base of the fabric8 plugin.

FIX BUG 1A: FileUtil.stripPostfix is broken

In HelmMojo::copyResourceFilesToTemplatesDir there is a call to FileUtil.stripPostfix(...) to remove the ".yml" extension: name = FileUtil.stripPostfix(name, ".yml") + YAML_EXTENSION;
see : https://github.com/fabric8io/fabric8-maven-plugin/blob/master/plugin/src/main/java/io/fabric8/maven/plugin/mojo/build/HelmMojo.java#L362

I suggest the fix below:

public static String stripPostfix(String text, String postfix) {
    if (text.endsWith(postfix)) {
        //the line below does not strip postfix, it extracts the postfix
        //return text.substring(text.length() - postfix.length());
        return text.substring(0, text.length() - postfix.length());
    }
    return text;
}

The content of the stripPostfixMethod should be fixed adding a first parameter with value "0" as this:

FIX BUG 1B: ResourceUtil.save cannot write ".yaml" extension files

There are 3 calls to ResourceUtil.save in HelmMojo:

• https://github.com/fabric8io/fabric8-maven-plugin/blob/master/plugin/src/main/java/io/fabric8/maven/plugin/mojo/build/HelmMojo.java#L224
• https://github.com/fabric8io/fabric8-maven-plugin/blob/master/plugin/src/main/java/io/fabric8/maven/plugin/mojo/build/HelmMojo.java#L246
• https://github.com/fabric8io/fabric8-maven-plugin/blob/master/plugin/src/main/java/io/fabric8/maven/plugin/mojo/build/HelmMojo.java#L386

All are made with a file name with the ".yaml" extension.
For example in line 224:

File outputChartFile = new File(outputDir, "Chart.yaml");
try {
  ResourceUtil.save(outputChartFile, chart, ResourceFileType.yaml);
} catch (IOException e) {
  throw new MojoExecutionException("Failed to save chart " + outputChartFile + ": " + e, e);
}

But the ResourceUtil.save() method adds the ".yml" extension without testing if an extension is not already set.

In fact, the entry point is the save() method see:
https://github.com/fabric8io/fabric8-maven-plugin/blob/master/core/src/main/java/io/fabric8/maven/core/util/ResourceUtil.java#L62

    public static File save(File file, Object data) throws IOException {
        return save(file, data, ResourceFileType.fromFile(file));
    }

    public static File save(File file, Object data, ResourceFileType type) throws IOException {
        File output = type.addExtensionIfMissing(file);
        ensureDir(file);
        getObjectMapper(type).writeValue(output, data);
        return output;
    }

This method calls type.addExtensionIfMissing(file);

https://github.com/fabric8io/fabric8-maven-plugin/blob/master/core/src/main/java/io/fabric8/maven/core/util/ResourceFileType.java#L62

    public File addExtensionIfMissing(File file) {
        String path = file.getAbsolutePath();
        if (!path.endsWith("." + extension)) {
            return new File(path + "." + extension);
        } else {
            return file;
        }
    }

I suggest the fix below:

public static File save(File file, Object data, ResourceFileType type) throws IOException {
  boolean hasExtension = FilenameUtils.indexOfExtension(file.getAbsolutePath()) != -1;
  // File output = type.addExtensionIfMissing(file);
  File output = hasExtension ? file : type.addExtensionIfMissing(file);
  ensureDir(file);
  getObjectMapper(type).writeValue(output, data);
  return output;
}

With this approach, the extension is added only if there is no existing extension. And so, I works with ".yaml" extensions if already set as in the HelmMojo code.

Note: this approach is the more flexible, but other alternatives could be considered:

• In file ResourceFileType, the extension is a private variable, it could be made accessible using a getter, but I think it is not the initial point of view of the author
• The HelmMojo could create ".yml" files, then rename them to ".yaml" but It is not a very good choice for maintainability

FIX BUG? 2: Unable to find the template file and resources

How is the sourceDir documented ?

According to the documentation, the source dir is defined as this:
sourceDir | Where to find the resource descriptors generated with fabric8:resource. By default this is:

sourceDir	Where to find the resource descriptors generated with fabric8:resource. By default this is ${basedir}/target/classes/META-INF/fabric8, which is also the output directory used by fabric8:resource.	fabric8.helm.sourceDir

How is it expected by the HelmMojo code ?

But in the HelmMojo, the default value for sourceDir is set like this (see https://github.com/fabric8io/fabric8-maven-plugin/blob/master/plugin/src/main/java/io/fabric8/maven/plugin/mojo/build/HelmMojo.java#L177):

String dir = getProperty("fabric8.helm.sourceDir");
if (dir == null) {
  dir = project.getBuild().getOutputDirectory() + "/META-INF/fabric8/" + type.getSourceDir();
}

Where type.getSourceDir() is set to "k8s-template" in HelmConfig (see https://github.com/fabric8io/fabric8-maven-plugin/blob/master/core/src/main/java/io/fabric8/maven/core/config/HelmConfig.java#L79) :

kubernetes("helm", "k8s-template", "Kubernetes"),

The value "k8s-template" is also used for the expected location of the template file (see https://github.com/fabric8io/fabric8-maven-plugin/blob/master/plugin/src/main/java/io/fabric8/maven/plugin/mojo/build/HelmMojo.java#L83 ):

@Parameter(property = "fabric8.kubernetesTemplate", defaultValue = "${basedir}/target/classes/META-INF/fabric8/k8s-template.yml")
    private File kubernetesTemplate;

This appears to be very odd to me... I cannot find documentation on this usage...

So... How to make it work ?

Very easily !

The HelmMojo code is clearly expecting the template file to be located at the sample place as the template resources. So I added a file named "template.yml" in the src/main/facbric8 directory:
src/main/fabric8 contains:

• configmap.yml
• deployment.yml
• secret.yml
• template.yml

My template.yml file content is:

kind: Template
parameters:
- name: PROJECT_NAMESPACE
- name: limits.memory
  value: "1400Mi"
- name: requests.memory
  value: "700Mi"

To fix it, I simply overrode the default values in the pom.xml file:

<!-- FIX HELM INPUT DIR (by default use k8s-template directory)-->
<fabric8.helm.sourceDir>${project.build.directory}/classes/META-INF/fabric8/kubernetes</fabric8.helm.sourceDir>
<fabric8.kubernetesTemplate>${fabric8.helm.sourceDir}/${project.artifactId}-template.yml</fabric8.kubernetesTemplate>

So... Why isn't it configured as this by default ?

Do you think the default configuration could be reverted ?

CONCLUSION

With very simple changes, the helm feature can be easily fixed:

• FileUtil.stripPostfix(...) should call substring(....) with a first parameter with "0" value
• ResourceFileType.save(...) should add the extension only if it does not exist
• fabric8.helm.sourceDir and fabric8.kubernetesTemplate default values should be changed

How do this contribution can be integrated ?

Thanks,

Ben.

[rohanKanojia]

if these changes don't harm the current plugin's flow, I would be happy to integrate this.

[Beennnn]

@rohanKanojia They don't :

• Bug fix 1A and 1B don't harm. They only better manage some use cases.
• Big fix 2 only changes the default values for properties, so it doesn't harm too. It just fixes the code for the properties to match the official documentation. The current (weird) behaviour could be set defining alternative values for the properties.

[miledean]

@Beennnn why don't you create a PR?

[Beennnn]

Ok. I will work on this soon.

[rohanKanojia]

@Beennnn : Are you working on PR for this? Otherwise I can pick it up.

[djotanov]

I had a bit of time to create PR for this.
I had to add some changes to ResourceMojo as well, as without it apply and deploy plugins wouldn't work because template variables wouldn't be resolved.
Basically this is the functionality from 3.5 branch.
Also fixed some NPE issue I noticed with undeploy plugin

[Beennnn]

Sorry I cannot work on this these days. Feel free to pick it up.

[rohanKanojia]

@Beennnn : No worries. Thanks to @djotanov PR is created and seems good to be merged