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
169 stars 21 forks source link

Managing labels on image building #303

Open imitbn opened 1 year ago

imitbn commented 1 year ago

The origin of this issue is #80 Label org.springframework.boot.spring-configuration-metadata.json was removed for all Spring Boot applications, except for Spring Cloud Data Flow applications. But Spring Cloud Data Flow UI (through Spring Cloud Data Flow Server) doesn't use it. Of course it might be used by other tools and motivation not to remove it completely, because of breaking changes, is understandable, however it's already done for the majority of all Spring Boot applications. On the other hand, getting rid of this particular label doesn't completely solve all potential problems with exceeding metadata size limits. IMHO, I would delete this label for Spring Cloud Data Flow applications as well.

But it's more important to have some reliable and flexible approach to manage labels creation. The simplest way is to specify exclusion of specific labels on build ("black list"). However, another long label may arise in the future. So it might be a solution of having "white list". The main idea is pretty simple - "If I need a label, I'll add it explicitly to the list and I don't care about other labels". The main advantage here - metadata size is more solid, but there are downsides too.

dmikusa commented 1 year ago

Part of the buildpack specification is a way for individual buildpacks to set labels on an image. The spring-boot buildpack does this for the SCDF label you mentioned above.

What I would suggest is that we add an env variable config setting on the spring-boot buildpack to turn this off. You can then set that env variable and it'll disable the label. That solves the specific and current need, and it's easy to implement. I'll leave this issue open as a feature request to implement this.


Also worth noting is that per the buildpack spec, one buildpack can override the labels set by another buildpack. If two buildpacks set label "FOO" the last buildpack to run wins.

If multiple buildpacks define labels with the same key, the lifecycle MUST use the last label defintion ordered by buildpack execution for the image label.

Based on that, I would think that the image-labels buildpack would be another option here. You could have the image-labels buildpack set the same label name as the spring-boot buildpack, org.springframework.cloud.dataflow.spring-configuration-metadata.json and the image-labels buildpack should overwrite the spring-boot buildpack because the default buildpack order in paketo-buildpacks/java is to run image-labels towards the end. You would still need to know all of the labels you want to override though, there's no way to wildcard/override all labels.

Ex: pack build -e BP_IMAGE_LABELS='org.springframework.cloud.dataflow.spring-configuration-metadata.json="{}"' ...

If you want a simple flag like --no-labels, you'd have to talk upstream with pack or the Spring Boot Build tools teams to see. That would need to be implemented at the platform level.

Hope that helps!

imitbn commented 1 year ago

Disabling labels by env variables seems like a straightforward approach.

However, I'm still concerned about uncontrolled growth of labels size. Stopping Kubernetes nodes because of that is a real threat. I'm even considering building an image by a plain dockerfile with a label, that is fetched from an image built preliminarily by buildpacks. In our case it's org.springframework.cloud.dataflow.spring-configuration-metadata.json (used by Spring Cloud Dataflow Server).

By the way, I've tried using image-labels on pack build --builder paketobuildpacks/builder:base ... for overriding org.springframework.boot.spring-configuration-metadata.json, but it removes all other labels org.springframework.*

dmikusa commented 1 year ago

However, I'm still concerned about uncontrolled growth of labels size. Stopping Kubernetes nodes because of that is a real threat.

I thought that Kubernetes applied mitigation for this a while back. Previously it would DoS your node. While it still cannot handle images with labels that large, I don't believe that it crashes like it used to. It just refused to run your image. Feel free to correct me on this, I haven't actually tried it for a while.

By the way, I've tried using image-labels on pack build --builder paketobuildpacks/builder:base ... for overriding org.springframework.boot.spring-configuration-metadata.json, but it removes all other labels org.springframework.*

That doesn't sound right. Not according to the spec. It shouldn't be doing any kind of wildcard matching. If one label name matches another label name, then it should overwrite that label. Can you open an issue for that and include steps to reproduce? That buildpack stuffs all of the labels into one space-separated env variable so it might be something with the way the buildpack parses that out. I can take a look.

imitbn commented 1 year ago

I thought that Kubernetes applied mitigation for this a while back

We are still using an old version of Kubernetes, unfortunately. I can't find, how Kubernetes handles it now (they got rid of "docker shim" where this issue happened before), but people are still complaining about it: https://github.com/kubernetes/kubernetes/issues/63858#issuecomment-1355970034 (the fix of this particular issue was simply increasing grpc max message size)

That doesn't sound right

Agree. Please check the command, maybe there is a mistake: pack build --path target/my-project-name-1.0.0.jar --builder paketobuildpacks/builder:base --buildpack paketo-buildpacks/image-labels --env 'BP_IMAGE_LABELS=org.springframework.boot.spring-configuration-metadata.json=""' my-image The image is created, it has the label org.springframework.boot.spring-configuration-metadata.json with empty value and there are other labels besides org.springframework.*