paketo-buildpacks / spring-boot

A Cloud Native Buildpack that contributes Spring Boot dependency information and slices an application into multiple layers
Apache License 2.0
171 stars 22 forks source link

Trailing comma in the dataflow-configuration-metadata.properties causes undesired results #438

Closed kohlmu-pivotal closed 5 months ago

kohlmu-pivotal commented 8 months ago

Whilst generating a docker image for a Spring Cloud Dataflow Stream Application, it was noticed that if there is a trailing comma in the dataflow-configuration-metadata.properties, the resultant image label org.springframework.cloud.dataflow.spring-configuration-metadata.json contains more entries than expected.

Expected Behavior

When generating a docker image the org.springframework.cloud.dataflow.spring-configuration-metadata.json should only contain entries for the filtered classes (defined in the property configuration-properties.classes).

Current Behavior

When there is a trailing comma in the list of configuration-properties.classes from the dataflow-configuration-metadata.properties file, the code does not handle an empty String and causes the filtering not to work as expected and it includes ALL properties. In our case 408 vs 16.

Possible Solution

Check for empty string after splitting the value for the property configuration-properties.classes. Currently the string is already trimmed, but it does not check for empty strings.

Steps to Reproduce

Add a trailing comma in the string stored in the property configuration-properties.classes in the dataflow-configuration-metadata.properties file. This should result in the org.springframework.cloud.dataflow.spring-configuration-metadata.json label to contain ALL properties from the project instead of only the filtered properties defined in the configuration-properties.classes property.

Motivations

This issue has caused the listing of too many properties for a defined Spring Cloud Dataflow Stream app. This issue is subtle and easy to make. It is harder to find or root-cause without debugging the buildpack code or by knowing what to look for.

tinedel commented 6 months ago

not sure if it's a blocker, but all these properties do have sourceType missing from the spring metadata json file, and that's why they are matched with an empty class name.

Might it be ever useful to include them? I don't believe it, but maybe I'm missing some context

for example this property will be included in the current implementation if trailing comma is added

{
  "name": "spring.cloud.task.events.enabled",
  "type": "java.lang.Boolean",
  "description": "This property is used to determine if a task app should emit task events.",
  "defaultValue": true
}

but it will be ignored if my pull request is merged and released

kohlmu-pivotal commented 6 months ago

@tinedel given that all of this works without the trailing comma, makes me think that by ignoring the empty class name will be a perfect solution.

pivotal-david-osullivan commented 5 months ago

@kohlmu-pivotal can you confirm if this issue is resolved? The PR with the fix was merged and should be released

kohlmu-pivotal commented 5 months ago

I think we can mark this issue as resolved. I will raise a ticket on our internal systems to test this out, the next time we are in the project where we found this issue. Given that this issue was stumbled upon by accident, and the "fix"/ work around was to remove the trailing comma.