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

Add appVersion to the generated Chart.yaml file #2472

Closed
ajeans opened this issue Nov 23, 2023 · 13 comments · Fixed by #2473
Closed

Add appVersion to the generated Chart.yaml file #2472

ajeans opened this issue Nov 23, 2023 · 13 comments · Fixed by #2473
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@ajeans
Copy link

ajeans commented Nov 23, 2023

Component

Kubernetes Maven Plugin

Is your enhancement related to a problem? Please describe

I would like to get the appVersion added to the chart, so that when I do

 helm list -n dev

When I do a simple:

./mvnw -pl microservice-app k8s:resource k8s:helm

The generated Chart.yaml contains a version key, but no appVersion key

apiVersion: v1
name: microservice-app
<snip>
version: "0.3.1"
<snip>

Output is

NAME            	NAMESPACE	REVISION	UPDATED                             	STATUS  	CHART                 	APP VERSION
microservice-app	dev      	1       	2023-11-23 10:58:51.765527 +0100 CET	deployed	microservice-app-0.3.1	0.3.1

Rather than

NAME            	NAMESPACE	REVISION	UPDATED                             	STATUS  	CHART                          	APP VERSION
microservice-app	dev      	1       	2023-11-23 10:32:28.876677 +0100 CET	deployed	microservice-app-0.3.1

This was mentioned in the following issue 3 years ago, at the time when helm 2 apparently didn't have appVersion #113

Describe the solution you'd like

I would like the appVersion to be automatically added with the project version (same behaviour as version)

Describe alternatives you've considered

Another option is to have Chart resource fragments that allow adding a Chart.helm.yml in src/main/jkube.
And if this file contains

appVersion: ${project.version}

then this gets templated and added to the output Chart.yml

Additional context

No response

@ajeans ajeans added the enhancement New feature or request label Nov 23, 2023
@manusa
Copy link
Member

manusa commented Nov 23, 2023

Another option is to have Chart resource fragments that allow adding a Chart.helm.yml in src/main/jkube.

Should be fixed by #2390

I would like the appVersion to be automatically added with the project version (same behaviour as version)

Can be fixed by adding the appVersion field to the HelmConfig class and then propagating it.

@ajeans
Copy link
Author

ajeans commented Nov 23, 2023

Thanks for the quick reply @manusa

Another option is to have Chart resource fragments that allow adding a Chart.helm.yml in src/main/jkube.

Should be fixed by #2390

Cool, is a SNAPSHOT of 1.16.0 available somewhere or do I need to build it locally?

Looking at documentation and https://mvnrepository.com/artifact/org.eclipse.jkube/kubernetes-maven-plugin didn't help much.

I would like the appVersion to be automatically added with the project version (same behaviour as version)

Can be fixed by adding the appVersion field to the HelmConfig class and then propagating it.

Is that a comment about the code and pointing at https://github.com/eclipse/jkube/blob/master/jkube-kit/resource/helm/src/main/java/org/eclipse/jkube/kit/resource/helm/HelmConfig.java?

Or is there a way to do that today with 1.15.0?

In doubt, I looked again at https://eclipse.dev/jkube/docs/kubernetes-maven-plugin/#jkube:helm and played around in the pom.xml, but this doesn't help...

            <plugin>
                <groupId>org.eclipse.jkube</groupId>
                <artifactId>kubernetes-maven-plugin</artifactId>
                <version>${jkube.version}</version>
                <configuration>
                    <helm>
                        <parameters>
                            <parameter>
                                <name>appVersion</name>
                                <value>${project.version}</value>
                            </parameter>
                        </parameters>
                    </helm>
                </configuration>
            </plugin>

@manusa
Copy link
Member

manusa commented Nov 23, 2023

Cool, is a SNAPSHOT of 1.16.0 available somewhere or do I need to build it locally?

We publish nightly snapshots (1.16-SNAPSHOT), see:

https://github.com/eclipse/jkube/blob/master/USING-SNAPSHOT-ARTIFACTS.md

Or is there a way to do that today with 1.15.0?

No. However, with the current snapshot, you can take the fragment approach.

The rest of my message was about a pointer on how to resolve this particular issue by enhancing the HelmConfig class. But this would need to be implemented.

@ajeans
Copy link
Author

ajeans commented Nov 23, 2023

Cool, is a SNAPSHOT of 1.16.0 available somewhere or do I need to build it locally?

We publish nightly snapshots (1.16-SNAPSHOT), see:

https://github.com/eclipse/jkube/blob/master/USING-SNAPSHOT-ARTIFACTS.md

Thanks for the pointer.

With 1.16-SNAPSHOT:

  • If I remove the helm fragments (no src/main/Chart.helm.yml), there is no appVersion in the output Chart.yaml
  • If I add back a helm fragment
    src/main/Chart.helm.yml
appVersion: ${project.version}

this yields
Chart.yaml

appVersion: "${project.version}"

so the latest plugin allows the appVersionattribute but the maven variable is not templated in AFAICS.

Is there a way to make this work?

@manusa
Copy link
Member

manusa commented Nov 23, 2023

so the latest plugin allows the appVersionattribute but the maven variable is not templated in AFAICS.

I think we had an issue with some of those variables.
Could you please try to add this to the pom.xml

<properties>
 <helm.appVersion>${project.version}</helm.appVersion>
</properties>

Then use

appVersion: ${helm.appVersion}

in your chart.

That should definitely work.

@ajeans
Copy link
Author

ajeans commented Nov 23, 2023

Awesome @manusa , this works! 🎉

So as a summary:

  • you need at least version 1.16.X
  • you need to declare an intermediate variable in your pom.xml
    <properties>
        <helm.appVersion>${project.version}</helm.appVersion>
    </properties>
  • you need to add a helm fragment in your src/main/jkube/Chart.helm.yaml
# Add appVersion through intermediate variable - https://github.com/eclipse/jkube/issues/2472
appVersion: ${helm.appVersion}

Thanks a lot

@manusa
Copy link
Member

manusa commented Nov 24, 2023

Since this is an easy fix (adding the appVersion to HelmConfig), I'm going to implement it now so that you can avoid the tedious workaround using fragments.

@manusa manusa self-assigned this Nov 24, 2023
@manusa manusa added this to the 1.16.0 milestone Nov 24, 2023
@manusa manusa moved this to In Progress in Eclipse JKube Nov 24, 2023
@manusa manusa moved this from In Progress to Review in Eclipse JKube Nov 24, 2023
@ajeans
Copy link
Author

ajeans commented Nov 24, 2023

@manusa thanks a lot!

please note however that if you add appVersion as is, then helm lint will complain.

./mvnw clean install -Dmaven.test.skip
./mvnw -pl microservice-app k8s:resource k8s:helm
helm lint microservice-app/target/jkube/helm/microservice-app/kubernetes/microservice-app-0.0.1-SNAPSHOT.tar.gz
==> Linting microservice-app/target/jkube/helm/microservice-app/kubernetes/microservice-app-0.0.1-SNAPSHOT.tar.gz
[ERROR] Chart.yaml: chart type is not valid in apiVersion 'v1'. It is valid in apiVersion 'v2'

Error: 1 chart(s) linted, 1 chart(s) failed

Using fragments, I overrode the apiVersion with an entry in Chart.helm.yaml.

I think that if you want to add it unconditionnally, you would need to bump apiVersion

@manusa
Copy link
Member

manusa commented Nov 24, 2023

I think that if you want to add it unconditionnally, you would need to bump apiVersion

Nice, good catch.

@manusa
Copy link
Member

manusa commented Nov 24, 2023

I think that in your case it complains about the chart type.

I've double checked docs and appVersion is valid with apiVersion: v1

Also did a helm lint with the following Chart.yaml:

apiVersion: v1
name: helm-zero-config
version: 0.0.1-SNAPSHOT
appVersion: 0.0.1-SNAPSHOT

and there were no complaints.

@github-project-automation github-project-automation bot moved this from Review to Done in Eclipse JKube Nov 24, 2023
@ajeans
Copy link
Author

ajeans commented Nov 24, 2023

You are absolutely correct, re-reading the helm error message, and everything was there. 🙃

Indeed, I also added to my project

type: application

... and got confused when testing.

Thanks for the quick change and marking it for the next release!

@manusa
Copy link
Member

manusa commented Nov 24, 2023

Changes have been merged, you should be able to test everything after SNAPSHOTS get released hopefully tonight.

Thanks for the feedback, please keep it coming, especially for the Helm features which we want to improve in the near future.

@ajeans
Copy link
Author

ajeans commented Nov 27, 2023

Thanks, I confirm that it now works with the SNAPSHOT, now that my hacks have been removed.

diff --git a/microservice-app/pom.xml b/microservice-app/pom.xml
index 92b1f2a..9c97831 100644
--- a/microservice-app/pom.xml
+++ b/microservice-app/pom.xml
@@ -11,11 +11,6 @@
     <modelVersion>4.0.0</modelVersion>
     <artifactId>microservice-app</artifactId>

-    <properties>
-        <!-- TEMP see https://github.com/eclipse/jkube/issues/2472 -->
-        <helm.appVersion>${project.version}</helm.appVersion>
-    </properties>
-
     <dependencies>
         <dependency>
             <groupId>com.rakuten</groupId>
diff --git a/microservice-app/src/main/jkube/Chart.helm.yml b/microservice-app/src/main/jkube/Chart.helm.yml
index fd8402c..bafd1ad 100644
--- a/microservice-app/src/main/jkube/Chart.helm.yml
+++ b/microservice-app/src/main/jkube/Chart.helm.yml
@@ -1,7 +1,5 @@
-# JKube defaults to v1, move to later version
+# JKube defaults to v1, move to later version for "type" attribute
 apiVersion: v2
-# Add appVersion through intermediate variable - https://github.com/eclipse/jkube/issues/2472
-appVersion: ${helm.appVersion}
 type: application
 # Our own custom icon - makes 'helm lint' happy
 icon: https://fr.shopping.rakuten.com/visuels/0_content_square/autres/rakuten-logo6.svg
\ No newline at end of file

Thanks for the feedback, please keep it coming, especially for the Helm features which we want to improve in the near future.

Thanks for the kind words, other question / issue coming tonight :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants