Make annotation app.quarkus.io/vcs-uri optional in Kubernetes extension

[antoniomacri]

Description

The Kubernetes extension adds the annotation app.quarkus.io/vcs-uri to the generated deployment.

However, in our case it is integrated in our GitLab CI pipelines, so when the repository is cloned the GL user and token are specified in the URL and as a consequence the token is written as part of the annotation:

apiVersion: apps/v1
kind: Deployment
metadata:
  annotations:
    app.quarkus.io/vcs-uri: https://gitlab-ci-token:<token>@gitlab.onprem.com/project.git
...

It should be possible to opt-out for this annotation.

Implementation ideas

No response

[quarkus-bot]

/cc @geoand (kubernetes), @iocanel (kubernetes)

[geoand]

Sounds reasonable to me @iocanel

[crumohr]

Would it be possible to consider just omitting the token from the URL format in the configuration?

Like https://{omit_including_at@}gitlab.onprem.com/project.git.

This change would help in preventing the accidental exposure of sensitive credentials in our Argo CD repository.

[crumohr]

As a workaround, you can use Kustomize to remove this annotation. Here's how we do it:

apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
  - kubernetes.yml

patches:
  - target:
      kind: .*
    patch: |-
      - op: add
        path: "/metadata/annotations/app.quarkus.io~1vcs-uri"
        value: "dummy"
      - op: remove
        path: "/metadata/annotations/app.quarkus.io~1vcs-uri"   

This approach first adds a dummy value to ensure the annotation exists on all resources, then removes it.

[jcechace]

Is there any progress on this?

The problem with this annotation is that it also seems to be hardcoded to take the origin from git metadata. Which is problematic in certain cases, when "origin" and the the public address wi wish to communicate differ. At least one of the following needs to happen:

1. Have an opt-out
2. Have a possibility to set override the value
3. Use maven VCS configuration

@crumohr That is quite an invasive workaround. Especially since it needs deployment side patching for something produced during build time.

[geoand]

I'll take a look

[geoand]

#38748 turns the annotation into:

app.quarkus.io/vcs-uri: https://gitlab.onprem.com/project.git

[crumohr]

@crumohr That is quite an invasive workaround. Especially since it needs deployment side patching for something produced during build time.

I completely agree. I was just sharing this to help till the underlying issue was tackled.

[gsmet]

FWIW, this is part of 3.7.3 released on Wednesday.

[jcechace]

#38748 turns the annotation into:

app.quarkus.io/vcs-uri: https://gitlab.onprem.com/project.git

This solves only the obvious security issue. In our case we often build from a fork (or have team member manually update certain generated descriptors), which leads to the annotation referencing the fork URL instead of upstream (from which the actual app sources are always build).

[gsmet]

Agreed, it could probably be made optional or configurable. Contributions welcome.

[jcechace]

We just discovered that our release pipeline leaked a security token into the public space due to this bug (prior to adding the VCS URL sanitisation). I think it might be worth somehow make sure that the users are aware of this potential security vulnerability introduced by quarkus. Is this something somebody already considered?

[jcechace]

There is now a public CVE for this issue https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-1979

[xstefank]

@jcechace reached out to me to take a look so I'm working on a fix for this now.

[gsmet]

There is a fix already for the security issue.
Making this thing optional has value too but it's not related to the security issue and is a feature request.

As for communicating better about CVEs, we are in agreement. We are discussing how we could improve that in the future.

[xstefank]

I'm making it optional and overridable. OK with you @gsmet ?

[geoand]

Go ahead 👌

[jcechace]

BTW what is even the reasoning behind KubernetesCommonHelper getting VCS information directly for VCS config instead of project configuration in e.g. pom.xml? Digging into repository configuration is definitively something I would not expect application framework to do.

Additionally it seems that this functionality is being adopted by plugins already (e.g. see QOSDK PR here https://github.com/quarkiverse/quarkus-operator-sdk/pull/851/files#diff-7328825e75606a3487b41ede21e7adafdc7b7d11e20c1c1256855c904fe9edffR144) which I don't think is a good precedence.

[Naros]

I agree; this seems a bit strange, especially if there is no way to configure it via the build system. Is there not a way to drive this beyond just scavenging the repository?

[geoand]

Digging into repository configuration is definitively something I would not expect application framework to do

On the contrary, I think that it's perfectly reasonable for something that runs at build time (and is also exactly what a bunch of Maven plugins do)

[geoand]

Is there not a way to drive this beyond just scavenging the repository?

Not currently, but it could be made that way

[Naros]

Not currently, but it could be made that way

That would be awesome if possible, just gives more options and control where needed 👍

[geoand]

What do you have in mind as alternatives and how would a user enable them?

[Naros]

Hi @geoand, I'm going to need to defer to @jcechace here as he's the best to answer that question. Jakub, can you share some ideas or thoughts on this since you're in the trenches on the K8 and Operator side of things?

[jcechace]

I think that the override option created by @xstefank is a good solution. I just maight be a bit keen on calling it workaround since as I stated -- the default is still Quarkus digging into VCS configuration which personally I would not expect. However I respect that the team around Quarkus feels otherwise.

[gsmet]

I must admit maybe having the default use the <scm> info would be a bit safer.

[gsmet]

Now I have no idea how to deal with this when using Gradle.

[jcechace]

I must admit maybe having the default use the <scm> info would be a bit safer.

That would be perfect. The direct SCM info extraction seems reasonable as the last resort. However if I explicitly have that info in my project descriptor I would expect it to be honored.

Now I have no idea how to deal with this when using Gradle.

Unfortunately I also wasn't able to find a standardised SCM configuration for Gradle

[jcechace]

@gsmet When getting the default value, can you maybe detect whether the project environment is Maven or Gradle and if maven then check the SCM information from project?

[geoand]

That would be pretty surprising behavior

[jcechace]

@geoand well... Even here we have few people who find the situation where default is extracted directly from the SCM surprising. So maybe this needs some broader discussion about what the user base really thinks (I might be part of minority of course).

[geoand]

Feel free to open one

[jcechace]

Feel free to open one

This was a general evaluation of the situation and I personally think that such discussion should be driven by the Quarkus development core members -- which I'm certainly not part of.

[geoand]

Anyone can open a discussion on the quarkus-dev mailing list. And in this case, you have a lot more context about the issue than I have