Merge branch 'cgfrost-86997400-user-provided-config'

[resolves #122]

README.md

@@ -24,7 +24,13 @@ The following are _very_ simple examples for deploying the artifact types that w
 * [Spring Boot CLI](docs/example-spring_boot_cli.md)
 
 ## Configuration and Extension
-The buildpack supports configuration and extension through the use of Git repository forking.  The easiest way to accomplish this is to use [GitHub's forking functionality][] to create a copy of this repository.  Make the required configuration and extension changes in the copy of the repository.  Then specify the URL of the new repository when pushing Cloud Foundry applications.  If the modifications are generally applicable to the Cloud Foundry community, please submit a [pull request][] with the changes.
+The buildpack supports extension through the use of Git repository forking. The easiest way to accomplish this is to use [GitHub's forking functionality][] to create a copy of this repository.  Make the required extension changes in the copy of the repository. Then specify the URL of the new repository when pushing Cloud Foundry applications. If the modifications are generally applicable to the Cloud Foundry community, please submit a [pull request][] with the changes.
+
+Buildpack configuration can be overridden with an environment variable matching the configuration file you wish to override minus the `.yml` extension and with a prefix of `JBP_CONFIG`. The value of the variable should be valid inline yaml. For example, to change the default version of Java to 7 and adjust the memory heuristics apply this environment variable to the application.
+
+```cf set-env my-application JBP_CONFIG_OPEN_JDK_JRE '[version: 1.7.0_+, memory_heuristics: {heap: 85, stack: 10}]'```
+
+Environment variable can also be specified in the applications `manifest` file. See the [Environment Variables][] documentation for more information.
 
 To learn how to configure various properties of the buildpack, follow the "Configuration" links below. More information on extending the buildpack is available [here](docs/extending.md).
 
@@ -125,6 +131,7 @@ This buildpack is released under version 2.0 of the [Apache License][].
 [Cloud Foundry]: http://www.cloudfoundry.com
 [contributor guidelines]: CONTRIBUTING.md
 [disables `remote_downloads`]: docs/extending-caches.md#configuration
+[Environment Variables]: http://docs.cloudfoundry.org/devguide/deploy-apps/manifest.html#env-block
 [GitHub's forking functionality]: https://help.github.com/articles/fork-a-repo
 [Grails]: http://grails.org
 [Groovy]: http://groovy.codehaus.org

config/app_dynamics_agent.yml

@@ -17,5 +17,4 @@
 ---
 version: 4.0.+
 repository_root: "{default.repository.root}/app-dynamics"
-default_node_name: "$(expr \"$VCAP_APPLICATION\" : \'.*instance_index[\": ]*\\([[:digit:]]*\\).*\')"
 default_tier_name: CloudFoundry

config/open_jdk_jre.yml

@@ -14,16 +14,16 @@
 # limitations under the License.
 
 # Configuration for JRE repositories keyed by vendor
-# To go back to Java 7, permgen should be used instead of metaspace.  Please see the documentation for more detail.
+# If Java 7 is required, permgen will be used instead of metaspace. Please see the documentation for more detail.
 ---
 repository_root: "{default.repository.root}/openjdk/{platform}/{architecture}"
 version: 1.8.0_+
 memory_sizes:
   metaspace: 64m..
-  # permgen: 64m..
+  permgen: 64m..
 memory_heuristics:
   heap: 75
   metaspace: 10
-  # permgen: 10
+  permgen: 10
   stack: 5
   native: 10

docs/framework-app_dynamics_agent.md

@@ -19,25 +19,19 @@ When binding AppDynamics using a user-provided service, it must have name or tag
 | ---- | -----------
 | `account-access-key` | (Optional) The account access key to use when authenticating with the controller
 | `account-name` | (Optional) The account name to use when authenticating with the controller
-| `application-name` | (Optional) the applicationa's name
 | `host-name` | The controller host name
-| `node-name` | (Optional) the application's node name
 | `port` | (Optional) The controller port
 | `ssl-enabled` | (Optional) Whether or not to use an SSL connection to the controller
 | `tier-name` | (Optional) the application's tier name
 
-To provide more complex values such as the `tier-name`, using the interactive mode when creating a user-provided service will manage the character escaping automatically. For example, the default `tier-name` could be set with a value of `Tier-$(expr "$VCAP_APPLICATION" : '.*instance_index[": ]*\([[:digit:]]*\).*')` to calculate a value from the Cloud Foundry instance index.
-
 ## Configuration
 For general information on configuring the buildpack, refer to [Configuration and Extension][].
 
 The framework can be configured by modifying the [`config/app_dynamics_agent.yml`][] file in the buildpack fork.  The framework uses the [`Repository` utility support][repositories] and so it supports the [version syntax][] defined there.
 
 | Name | Description
 | ---- | -----------
-| `default_application_name` | This is not provided by default but can be added to specify the application name in the AppDynamics dashboard.  This can be overridden with an `application-name` entry in the credentials payload.
 | `default_tier_name` | The default tier name for this application in the AppDynamics dashboard.  This can be overridden with a `tier-name` entry in the credentials payload.
-| `default_node_name` | The default node name for this application in the AppDynamics dashboard.  The default value is an expression that will be evaluated based on the `instance_index` of the application. This can be overridden with a `node-name` entry in the credentials payload.
 | `repository_root` | The URL of the AppDynamics repository index ([details][repositories]).
 | `version` | The version of AppDynamics to use. Candidate versions can be found in [this listing][].
 

lib/java_buildpack/framework/app_dynamics_agent.rb

@@ -34,11 +34,14 @@ def compile
       def release
         credentials = @application.services.find_service(FILTER)['credentials']
         java_opts   = @droplet.java_opts
-        java_opts.add_javaagent(@droplet.sandbox + 'javaagent.jar')
 
-        application_name(java_opts, credentials)
-        tier_name(java_opts, credentials)
-        node_name(java_opts, credentials)
+        java_opts
+          .add_javaagent(@droplet.sandbox + 'javaagent.jar')
+          .add_system_property('appdynamics.agent.applicationName', "'#{application_name}'")
+          .add_system_property('appdynamics.agent.tierName', "'#{tier_name(credentials)}'")
+          .add_system_property('appdynamics.agent.nodeName',
+                               "$(expr \"$VCAP_APPLICATION\" : '.*instance_index[\": ]*\\([[:digit:]]*\\).*')")
+
         account_access_key(java_opts, credentials)
         account_name(java_opts, credentials)
         host_name(java_opts, credentials)
@@ -59,11 +62,12 @@ def supports?
 
       private_constant :FILTER
 
-      def application_name(java_opts, credentials)
-        name = credentials.key?('application-name') ? credentials['application-name'] :
-          @configuration['default_application_name']
-        name = name ? name : @application.details['application_name']
-        java_opts.add_system_property('appdynamics.agent.applicationName', "'#{name}'")
+      def tier_name(credentials)
+        credentials.key?('tier-name') ? credentials['tier-name'] : @configuration['default_tier_name']
+      end
+
+      def application_name
+        @application.details['application_name']
       end
 
       def account_access_key(java_opts, credentials)
@@ -82,11 +86,6 @@ def host_name(java_opts, credentials)
         java_opts.add_system_property 'appdynamics.controller.hostName', host_name
       end
 
-      def node_name(java_opts, credentials)
-        name = credentials.key?('node-name') ? credentials['node-name'] : @configuration['default_node_name']
-        java_opts.add_system_property('appdynamics.agent.nodeName', "#{name}")
-      end
-
       def port(java_opts, credentials)
         port = credentials['port']
         java_opts.add_system_property 'appdynamics.controller.port', port if port
@@ -97,11 +96,6 @@ def ssl_enabled(java_opts, credentials)
         java_opts.add_system_property 'appdynamics.controller.ssl.enabled', ssl_enabled if ssl_enabled
       end
 
-      def tier_name(java_opts, credentials)
-        name = credentials.key?('tier-name') ? credentials['tier-name'] : @configuration['default_tier_name']
-        java_opts.add_system_property('appdynamics.agent.tierName', "'#{name}'")
-      end
-
     end
 
   end

lib/java_buildpack/jre/open_jdk_like.rb

@@ -18,6 +18,7 @@
 require 'java_buildpack/component/versioned_dependency_component'
 require 'java_buildpack/jre'
 require 'java_buildpack/jre/memory/openjdk_memory_heuristic_factory'
+require 'java_buildpack/util/tokenized_version'
 
 module JavaBuildpack
   module Jre
@@ -65,15 +66,26 @@ def release
 
       KEY_MEMORY_SIZES = 'memory_sizes'.freeze
 
-      private_constant :KEY_MEMORY_HEURISTICS, :KEY_MEMORY_SIZES
+      VERSION_8 = JavaBuildpack::Util::TokenizedVersion.new('1.8.0').freeze
+
+      private_constant :KEY_MEMORY_HEURISTICS, :KEY_MEMORY_SIZES, :VERSION_8
 
       def killjava
         @droplet.sandbox + 'bin/killjava.sh'
       end
 
       def memory
-        sizes      = @configuration[KEY_MEMORY_SIZES] || {}
-        heuristics = @configuration[KEY_MEMORY_HEURISTICS] || {}
+        sizes      = @configuration[KEY_MEMORY_SIZES] ? @configuration[KEY_MEMORY_SIZES].clone : {}
+        heuristics = @configuration[KEY_MEMORY_HEURISTICS] ? @configuration[KEY_MEMORY_HEURISTICS].clone : {}
+
+        if @version < VERSION_8
+          heuristics.delete 'metaspace'
+          sizes.delete 'metaspace'
+        else
+          heuristics.delete 'permgen'
+          sizes.delete 'permgen'
+        end
+
         OpenJDKMemoryHeuristicFactory.create_memory_heuristic(sizes, heuristics, @version).resolve
       end
 

lib/java_buildpack/util/configuration_utils.rb

@@ -17,20 +17,22 @@
 require 'pathname'
 require 'java_buildpack/util'
 require 'java_buildpack/logging/logger_factory'
+require 'shellwords'
 require 'yaml'
 
 module JavaBuildpack
   module Util
 
-    # Utilities for dealing with Groovy applications
+    # Utility for loading configuration
     class ConfigurationUtils
 
       private_class_method :new
 
       class << self
 
         # Loads a configuration file from the buildpack configuration directory.  If the configuration file does not
-        # exist, returns an empty hash.
+        # exist, returns an empty hash. Overlays configuration in a matching environment variable, on top of the loaded
+        # configuration, if present. Will not add a new configuration key where an existing one does not exist.
         #
         # @param [String] identifier the identifier of the configuration
         # @param [Boolean] should_log whether the contents of the configuration file should be logged.  This value
@@ -42,6 +44,15 @@ def load(identifier, should_log = true)
           if file.exist?
             configuration = YAML.load_file(file)
             logger.debug { "Configuration from #{file}: #{configuration}" } if should_log
+
+            user_provided = ENV[environment_variable_name(identifier)]
+
+            if user_provided
+              YAML.load(user_provided).each do |new_prop|
+                configuration = do_merge(configuration, new_prop, should_log)
+              end
+              logger.debug { "Configuration from #{file} modified with: #{user_provided}" } if should_log
+            end
           else
             logger.debug { "No configuration file #{file} found" } if should_log
           end
@@ -53,7 +64,31 @@ def load(identifier, should_log = true)
 
         CONFIG_DIRECTORY = Pathname.new(File.expand_path('../../../config', File.dirname(__FILE__))).freeze
 
-        private_constant :CONFIG_DIRECTORY
+        ENVIRONMENT_VARIABLE_PATTERN = 'JBP_CONFIG_'
+
+        private_constant :CONFIG_DIRECTORY, :ENVIRONMENT_VARIABLE_PATTERN
+
+        def do_merge(hash_v1, hash_v2, should_log)
+          hash_v2.each do |key, value|
+            if hash_v1.key? key
+              hash_v1[key] = do_resolve_value(key, hash_v1[key], value, should_log)
+            else
+              logger.warn { "User config value for '#{key}' is not valid, existing property not present" } if should_log
+            end
+          end
+          hash_v1
+        end
+
+        def do_resolve_value(key, v1, v2, should_log)
+          return do_merge(v1, v2, should_log) if v1.is_a?(Hash) && v2.is_a?(Hash)
+          return v2 if (!v1.is_a?(Hash)) && (!v2.is_a?(Hash))
+          logger.warn { "User config value for '#{key}' is not valid, must be of a similar type" } if should_log
+          v1
+        end
+
+        def environment_variable_name(config_name)
+          ENVIRONMENT_VARIABLE_PATTERN + config_name.upcase
+        end
 
         def logger
           JavaBuildpack::Logging::LoggerFactory.instance.get_logger ConfigurationUtils

spec/java_buildpack/framework/app_dynamics_agent_spec.rb

@@ -21,10 +21,7 @@
 describe JavaBuildpack::Framework::AppDynamicsAgent do
   include_context 'component_helper'
 
-  let(:configuration) do
-    { 'default_tier_name' => 'test-tier-name',
-      'default_node_name' => "$(expr \"$VCAP_APPLICATION\" : '.*instance_index[\": ]*\\([[:digit:]]*\\).*')" }
-  end
+  let(:configuration) { { 'default_tier_name' => 'test-tier-name' } }
 
   it 'does not detect without app-dynamics-n/a service' do
     expect(component.detect).to be_nil
@@ -80,26 +77,6 @@
         end
       end
 
-      context do
-        let(:credentials) { super().merge 'application-name' => 'another-test-application-name' }
-
-        it 'adds application_name from credentials to JAVA_OPTS if specified' do
-          component.release
-
-          expect(java_opts).to include("-Dappdynamics.agent.applicationName='another-test-application-name'")
-        end
-      end
-
-      context do
-        let(:credentials) { super().merge 'node-name' => 'another-test-node-name' }
-
-        it 'adds node_name from credentials to JAVA_OPTS if specified' do
-          component.release
-
-          expect(java_opts).to include('-Dappdynamics.agent.nodeName=another-test-node-name')
-        end
-      end
-
       context do
         let(:credentials) { super().merge 'port' => 'test-port' }
 

spec/java_buildpack/jre/open_jdk_jre_spec.rb

@@ -23,16 +23,38 @@
 describe JavaBuildpack::Jre::OpenJdkJRE do
   include_context 'component_helper'
 
+  let(:version_8) { VERSION_8 = JavaBuildpack::Util::TokenizedVersion.new('1.8.0_+') }
+
+  let(:version_7) { VERSION_7 = JavaBuildpack::Util::TokenizedVersion.new('1.7.0_+') }
+
+  let(:configuration) do
+    { 'memory_sizes'      => { 'metaspace' => '64m..',
+                               'permgen'   => '64m..' },
+      'memory_heuristics' => { 'heap'      => '75',
+                               'metaspace' => '10',
+                               'permgen'   => '10',
+                               'stack'     => '5',
+                               'native'    => '10' } }
+  end
+
   let(:java_home) { JavaBuildpack::Component::MutableJavaHome.new }
 
-  let(:memory_heuristic) { double('MemoryHeuristic', resolve: %w(opt-1 opt-2)) }
+  let(:memory_heuristic_7) { double('MemoryHeuristic', resolve: %w(opt-7-1 opt-7-2)) }
+
+  let(:memory_heuristic_8) { double('MemoryHeuristic', resolve: %w(opt-8-1 opt-8-2)) }
 
   before do
-    allow(JavaBuildpack::Jre::WeightBalancingMemoryHeuristic).to receive(:new).and_return(memory_heuristic)
+    allow(JavaBuildpack::Repository::ConfiguredItem).to receive(:find_item).and_return([version_8, 'test-uri'])
+    allow(JavaBuildpack::Jre::WeightBalancingMemoryHeuristic).to receive(:new).with({ 'permgen' => '64m..' },
+                                                                                    anything, anything, anything)
+                                                                   .and_return(memory_heuristic_7)
+    allow(JavaBuildpack::Jre::WeightBalancingMemoryHeuristic).to receive(:new).with({ 'metaspace' => '64m..' },
+                                                                                    anything, anything, anything)
+                                                                   .and_return(memory_heuristic_8)
   end
 
   it 'detects with id of openjdk_jre-<version>' do
-    expect(component.detect).to eq("open-jdk-jre=#{version}")
+    expect(component.detect).to eq("open-jdk-jre=#{version_8}")
   end
 
   it 'extracts Java from a GZipped TAR',
@@ -50,14 +72,6 @@
     expect(java_home.root).to eq(sandbox)
   end
 
-  it 'adds memory options to java_opts' do
-    component.detect
-    component.release
-
-    expect(java_opts).to include('opt-1')
-    expect(java_opts).to include('opt-2')
-  end
-
   it 'adds OnOutOfMemoryError to java_opts' do
     component.detect
     component.release
@@ -81,4 +95,32 @@
     expect(java_opts).to include('-Djava.io.tmpdir=$TMPDIR')
   end
 
+  it 'removes memory options for a java 8 app',
+     cache_fixture: 'stub-java.tar.gz' do
+
+    component.detect
+    component.release
+
+    expect(java_opts).to include('opt-8-1')
+    expect(java_opts).to include('opt-8-2')
+  end
+
+  context do
+
+    before do
+      allow(JavaBuildpack::Repository::ConfiguredItem).to receive(:find_item).and_return([version_7, 'test-uri'])
+    end
+
+    it 'removes memory options for a java 7 app',
+       cache_fixture: 'stub-java.tar.gz' do
+
+      component.detect
+      component.release
+
+      expect(java_opts).to include('opt-7-1')
+      expect(java_opts).to include('opt-7-2')
+    end
+
+  end
+
 end