spring-cloud / spring-cloud-deployer-cloudfoundry

The Spring Cloud Deployer implementation for Cloud Foundry
Apache License 2.0
28 stars 39 forks source link

CfEnvAwareResource.hasCfEnv() always returns false #358

Closed otheriault closed 4 years ago

otheriault commented 4 years ago

When playing around with spring-cloud-task using the spring-cloud-deployer-cloudfoundry, I realized that I always got the following warning: Unable to determine dependencies for [...].

The CfEnvAwareResource.hasCfEnv property is initialized in the constructor with this.hasCfEnv = CfEnvResolver.hasCfEnv(this); and the CfEnvResolver inner class does the following:

            try {
                JarFileArchive archive = new JarFileArchive(app.getFile());
                List<URL> urls = new ArrayList<>();
                archive.getNestedArchives(entry -> entry.getName().endsWith(".jar")).forEach(...);
            ...
            }
            catch (Exception e) {
                logger.warn("Unable to determine dependencies for " + app.getFilename());
            }
            return false;

The problem is that the org.springframework.boot.loader.archive.JarFileArchive does not implement the getNestedArchives(EntryFilter filter) method and instead the default method from org.springframework.boot.loader.archive.Archive is used instead:

    @Deprecated
    default List<Archive> getNestedArchives(EntryFilter filter) throws IOException {
        throw new IllegalStateException("Unexpected call to getNestedArchives(filter)");
    }

This will always throw an exception and CfEnvAwareResource.hasCfEnv will always be false.

Versions

otheriault commented 4 years ago

I just looked at the tests and they appear to cover both the true and false scenarios correctly, but the build status is currently red, so this test may be failing.

dturanski commented 4 years ago

@otheriault - Thanks for raising this issue. spring-cloud-deployer:2.4.2 has a dependency on spring-boot:2.2.8. Apparently, you are upgrading to boot 2.3.3 in your app. What you say is true. But since the required method https://github.com/spring-projects/spring-boot/blob/2.3.x/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/archive/Archive.java#L65 , introduced in boot 2.3, does not exist in 2.2.8. We cannot fix this issue until we upgrade to boot 2.3.x. Upgrading spring boot a big deal, since spring-cloud-dataflow depends on the cloud-foundry-deployer, and other components that also depend on boot. We normally upgrade boot for everything in the SCDF ecosystem at the same time. When that happens, our CFEnv tests will break, (since it will never return true as you say) and we will fix that method call. The failed build is not related this.

dturanski commented 4 years ago

Actually, I just realized this is already fixed on the master branch https://github.com/spring-cloud/spring-cloud-deployer-cloudfoundry/commit/fc8153953e3f0400bc509cf2cee404c9d37003d4 . Try using spring-cloud-deployer:2.5.0-M1