External configuration provided by BP_TOMCAT_EXT_CONF_URI is cached if the same image is built twice

[c0d1ngm0nk3y]

If you provide BP_TOMCAT_EXT_CONF_URI to use an external configuration, the external configuration is extracted to $CATALINA_BASE. But when you build the image a second time and the external configuration changed, the layer cache will be used and therefor the old external configuration.

Note: BP_TOMCAT_EXT_CONF_SHA256 is not used.

Expected Behavior

The external configuration is always applied and only cached if $BP_TOMCAT_EXT_CONF_SHA256 is provided and unchanged.

Current Behavior

If the previous image is found, it will reuse its configuration.

Possible Solution

Steps to Reproduce

1. Build an image with an external configuration

pack build -e BP_TOMCAT_EXT_CONF_URI="https:/<redacted>.tgz" -B paketobuildpacks/builder-jammy-base  tomcat-example:issue

2. Check the log that the external configuration is applied

[builder]   Tomcat External Configuration
[builder]   Warning: Dependency has no SHA256. Skipping cache.
[builder]     Downloading from https://int.repositories.cloud.sap/artifactory/cki-generic/tomcat-configuration-sjb.tgz
[builder]     Expanding to /layers/paketo-buildpacks_apache-tomcat/catalina-base

3. Change the configuration and build the same tag again

pack build -e BP_TOMCAT_EXT_CONF_URI="https:/<redacted>.tgz" -B paketobuildpacks/builder-jammy-base  tomcat-example:issue

4. Check the log

[builder] Paketo Buildpack for Apache Tomcat 7.9.0
[builder]   https://github.com/paketo-buildpacks/apache-tomcat
[builder]   Build Configuration:
[builder]     $BP_JAVA_APP_SERVER                                                                                                               the application server to use
[builder]     $BP_TOMCAT_CONTEXT_PATH                                                                                                           the application context path
[builder]     $BP_TOMCAT_ENV_PROPERTY_SOURCE_DISABLED  false                                                                                    Disable Tomcat's EnvironmentPropertySource
[builder]     $BP_TOMCAT_EXT_CONF_SHA256                                                                                                        the SHA256 hash of the external Tomcat configuration archive
[builder]     $BP_TOMCAT_EXT_CONF_STRIP                0                                                                                        the number of directory components to strip from the external Tomcat configuration archive
[builder]     $BP_TOMCAT_EXT_CONF_URI                  https://<redacted>.tgz  the download location of the external Tomcat configuration
[builder]     $BP_TOMCAT_EXT_CONF_VERSION                                                                                                       the version of the external Tomcat configuration
[builder]     $BP_TOMCAT_VERSION                       9.*                                                                                      the Tomcat version
[builder]   Launch Configuration:
[builder]     $BPL_TOMCAT_ACCESS_LOGGING_ENABLED                                                                                                the Tomcat access logging state
[builder]   Apache Tomcat 9.0.69: Reusing cached layer
[builder]   Launch Helper: Reusing cached layer
[builder]   Apache Tomcat Support: Reusing cached layer
[builder]   Process types:
[builder]     task:   bash catalina.sh run (direct)
[builder]     tomcat: bash catalina.sh run (direct)
[builder]     web:    bash catalina.sh run (direct)

Motivations

Changing the external configuration should have an effect if rebuilding the image.

[dmikusa]

I'm curious, is there a reason you're not setting BP_TOMCAT_EXT_CONF_SHA256? If you set that, does it behave properly? If that's not set, I believe it will refuse to cache the download dependency, which is what Warning: Dependency has no SHA256. Skipping cache. is telling you.

That's different from layer caching though. At the same time, I believe we put the dependency information into the layer cache, so if the dependency information changes it'll break the layer cache and rebuild the layer. I believe the SHA256 hash is part of that dependency information, so I'm wondering if that makes the layer rebuild in this scenario.

[c0d1ngm0nk3y]

I'm curious, is there a reason you're not setting BP_TOMCAT_EXT_CONF_SHA256?

Currently I was developing, trying out things. So updating the sha256 wouldhave been a git cumbersome.

If that's not set, I believe it will refuse to cache the download dependency, which is what Warning: Dependency has no SHA256. Skipping cache. is telling you.

That is exactly what I would have expected.

That's different from layer caching though.

Fair point. I think I will update the issue to make it a bit clearer what I mean.

I believe the SHA256 hash is part of that dependency information, so I'm wondering if that makes the layer rebuild in this scenario.

I can try this out tomorrow or so.

But without sha256, it should not always use the layer cache and assume the external configuration is unchanged, right? I guess that would be nice for the "development flow". With the current behaviour I have to to call pack build --clear-cache which takes (of course) significant longer.

[dmikusa]

But without sha256, it should not always use the layer cache and assume the external configuration is unchanged, right? I guess that would be nice for the "development flow". With the current behaviour I have to to call pack build --clear-cache which takes (of course) significant longer.

Yes, I agree. It seems like we could make that a little friendlier.

If the SHA256 is empty, we could stuff in a random value and it'd force the layer to change every time. I think we've done that in other places to force a layer to always update. We'd probably also want to make sure the output is clear that not having the SHA256 is causing the layer to be rebuilt more frequently. Right now it prints a message about not caching the dependency, but the layer cache is another level of caching and it would being doing more work/slower build so we should probably make sure it's clear why.

[c0d1ngm0nk3y]

If the SHA256 is empty, we could stuff in a random value and it'd force the layer to change every time.

Sounds good.

I think we've done that in other places to force a layer to always update. We'd probably also want to make sure the output is clear that not having the SHA256 is causing the layer to be rebuilt more frequently. Right now it prints a message about not caching the dependency, but the layer cache is another level of caching and it would being doing more work/slower build so we should probably make sure it's clear why.

Maybe we can have a look and propose a pr if that is OK. We are looking for ways to contribute to the project anyway... @loewenstein Isn't that right? :)