Skip to content

Improve configmap test coverage #6158

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cfitzw
Copy link
Contributor

@cfitzw cfitzw commented May 9, 2025

Address #6157.

The goal of the PR is to address missing coverage as it pertains to configmaps that were generated with the --from-file function, specifically for .properties files (e.g. kubectl create configmap my-cm-properties-file --from-file=./files/my.properties).

This was built off the tags/v2.6.0.

Additions:

  1. File-based configmaps housing a properties file
  2. Variable interpolation (substitution), very common (https://commons.apache.org/proper/commons-configuration/userguide/howto_basicfeatures.html#Variable_Interpolation).

This PR highlights that these new tests pass uses the default quarkus runtime, but when switching to plain-quarkus runtime, it fails.

Tested via:

  1. make
  2. make images
  3. kubectl create namespace camel-k
  4. make install-k8s-global
  5. kubectl patch itp camel-k -n camel-k -p '{"spec":{"traits":{"camel":{"runtimeProvider":"plain-quarkus"}}}}' --type=merge
  6. make test-common

Error from plain-quarkus:

dump.go:417:     > 2025-05-09 19:24:51,164 ERROR [org.apa.cam.qua.mai.CamelMainRuntime] (main) Failed to start application: org.apache.camel.FailedToCreateRouteException: Failed to create route route1 at: >>> SetBody[simple{configmap content is: {{my.key.1}} {{my.key.2}}}] <<< in route: Route(route1)[From[timer:configmap] -> [SetBody[simple{confi... because of Property with key [my.key.1] not found in properties from text: configmap content is: {{my.key.1}} {{my.key.2}}

NOTE: The initial make was failing on keystore_test.go due to something with the cert -- did not troubleshoot, but noting it for awareness.

Release Note

NONE

@cfitzw
Copy link
Contributor Author

cfitzw commented May 9, 2025

@squakez -- please review this.

@cfitzw cfitzw marked this pull request as ready for review May 9, 2025 19:44
Copy link
Contributor

@squakez squakez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The feature you're asking for is a Camel feature available since version 4.8: https://camel.apache.org/manual/using-propertyplaceholder.html#_resolving_property_placeholders_on_cloud

We can include the test for sure, but to make it work also for the plain runtime I think it should be run with the camel.main.cloud-properties-location explicitly. Can you give it a try please?

@cfitzw
Copy link
Contributor Author

cfitzw commented May 12, 2025

Correct, and setting camel.main.cloud-properties-location explicitly does work.

  • When using plain-quarkus and providing a generic configmap (non file-based), it is already automatically setting this cloud location and the properties are able to be read successfully.
  • Even if this configmap is file-based, it still auto-sets the location, it is just unaware that it needs to parse the configmap a bit differently.

The default runtime currently has awareness of how to handle this scenario, somehow, as it works (as shown in these tests). What I am questioning is why similar logic can't be used to auto-handle this scenario.

@squakez
Copy link
Contributor

squakez commented May 13, 2025

Ok, I got your point. The "problem" is that the plain runtime is not aware of where it is running (it could not be necessarily on a cloud environment), so, it must be instructed specifically where to look for by setting the following camel property camel.component.properties.location=file:/tmp/my.properties. You can add multiple locations and, ideally, you can combine the possibility to mount the configmap in a given location togheter with this property. In that case the runtime will be able to parse the file properly.

We can think to add something on Camel core, in order to be able to automatically parse any file with .properties in camel.main.cloud-properties-location similarly of what the Camel K runtime is doing. I suggest you to report on main Camel project in order to be widely discussed with the community.

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

Successfully merging this pull request may close these issues.

2 participants