Support nil configuration values

Christopher Frost · Christopher Frost · commit 309e8c5e507e · 2015-07-22T16:00:40.000+01:00
This commit makes a number of changes all required to support `nil` or empty values in the configuration files. Any `key` with a `nil` value will be ignored but as it is present it can be modified with a new value via an environment variable. Aditional changes ensure offline buildpacks still contain `nil` configuration values after version pinning. Updates the Java Opts suppport to have a `nil` `java_opts` configuration. Add in some additional `nil` configuration for the memory calculator so these can also be updated via environment variables. closes cloudfoundry#202 and closes cloudfoundry#204 [#98822486]
diff --git a/README.md b/README.md
@@ -27,7 +27,7 @@ The following are _very_ simple examples for deploying the artifact types that w
 ## Configuration and Extension
 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.
+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`. It is not possible to add new configuration properties and properties with `nil` or empty values will be ignored by the buildpack. 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 '[jre: {version: 1.7.0_+}, memory_calculator: {memory_heuristics: {heap: 85, stack: 10}}]'```
 
diff --git a/config/java_opts.yml b/config/java_opts.yml
@@ -16,4 +16,4 @@
 # JAVA_OPTS configuration
 ---
 from_environment: true
-# java_opts:
+java_opts:
diff --git a/config/open_jdk_jre.yml b/config/open_jdk_jre.yml
@@ -23,11 +23,14 @@ memory_calculator:
   version: 1.+
   repository_root: ! '{default.repository.root}/memory-calculator/{platform}/{architecture}'
   memory_sizes:
+    heap: 
     metaspace: 64m..
+    native: 
     permgen: 64m..
+    stack: 
   memory_heuristics:
     heap: 75
     metaspace: 10
+    native: 10
     permgen: 10
     stack: 5
-    native: 10
diff --git a/config/oracle_jre.yml b/config/oracle_jre.yml
@@ -15,22 +15,25 @@
 
 # Configuration for JRE repositories keyed by vendor
 # Pre Java 1.8, permgen was used instead of metaspace.  Please see the documentation for more detail.
----
+
 # You must specify a the repository root of an Oracle JRE repository. Please see the documentation for more detail.
 # e.g.  repository_root: "http://example.com/oracle-jre/{platform}/{architecture}"
-
+---
 jre:
   version: 1.8.0_+
   repository_root: ""
 memory_calculator:
   version: 1.+
-  repository_root: "{default.repository.root}/memory-calculator/{platform}/{architecture}"
+  repository_root: ! '{default.repository.root}/memory-calculator/{platform}/{architecture}'
   memory_sizes:
+    heap:
     metaspace: 64m..
+    native:
     permgen: 64m..
+    stack:
   memory_heuristics:
     heap: 75
     metaspace: 10
+    native: 10
     permgen: 10
     stack: 5
-    native: 10
diff --git a/docs/jre-open_jdk_jre.md b/docs/jre-open_jdk_jre.md
@@ -26,7 +26,7 @@ The JRE can be configured by modifying the [`config/open_jdk_jre.yml`][] file in
 | `version` | The version of Java runtime to use.  Candidate versions can be found in the listings for [centos6][], [lucid][], [mountainlion][], and [precise][]. Note: version 1.8.0 and higher require the `memory_sizes` and `memory_heuristics` mappings to specify `metaspace` rather than `permgen`.
 
 ### Additional Resources
-The JRE can also be configured by overlaying a set of resources on the default distribution.  To do this, add files to the `resources/open_jdk_jre` directory in the buildpack fork.  For example, to add the JCE Unlimited Strength `local_policy.jar` add your file to `resources/open_jdk_jre/lib/security/local_policy.jar`.
+The JRE can also be configured by overlaying a set of resources on the default distribution. To do this, add files to the `resources/open_jdk_jre` directory in the buildpack fork. For example, to add the JCE Unlimited Strength `local_policy.jar` add your file to `resources/open_jdk_jre/lib/security/local_policy.jar`.
 
 ### Memory
 The total available memory is specified when an application is pushed as part of it's configuration. The Java buildpack uses this value to control the JRE's use of various regions of memory. The JRE memory settings can be influenced by configuring the `memory_sizes` and/or `memory_heuristics` mappings.
@@ -48,7 +48,7 @@ Memory sizes together with _memory weightings_ (described in the next section) a
 
 Memory sizes consist of a non-negative integer followed by a unit (`k` for kilobytes, `m` for megabytes, `g` for gigabytes; the case is not significant). Only the memory size `0` may be specified without a unit.
 
-The above memory size properties may be omitted, specified as a single value, or specified as a range. Ranges use the syntax `<lower bound>..<upper bound>`, although either bound may be omitted in which case the defaults of zero and the total available memory are used for the lower bound and upper bound, respectively. Examples of ranges are `100m..200m` (any value between 100 and 200 megabytes, inclusive) and `100m..` (any value greater than or equal to 100 megabytes).
+The above memory size properties may be omitted with an empty value, specified as a single value, or specified as a range. Ranges use the syntax `<lower bound>..<upper bound>`, although either bound may be omitted in which case the defaults of zero and the total available memory are used for the lower bound and upper bound, respectively. Examples of ranges are `100m..200m` (any value between 100 and 200 megabytes, inclusive) and `100m..` (any value greater than or equal to 100 megabytes).
 
 Each form of memory size is equivalent to a range. Omitting a memory size is equivalent to specifying the range `0..`. Specifying a single value is equivalent to specifying the range with that value as both the lower and upper bound, for example `128m` is equivalent to the range `128m..128m`.
 
@@ -58,26 +58,28 @@ Memory weightings are configured in the `memory_heuristics` mapping of [`config/
 ```yaml
 memory_heuristics:
   heap: 15
+  native: 2
   permgen: 5
   stack: 1
-  native: 2
 ```
 
 represent a maximum heap size three times as large as the maximum PermGen size, and so on.
 
 Memory weightings are used together with memory ranges to calculate the amount of memory for each memory type, as follows.
 
 #### Memory Calculation
-The total available memory is allocated into heap, Metaspace or PermGen (depending on the version of OpenJDK), stack, and native memory types.
+Memory calculation happens before every `start` of an application and is performed by an external program, the [Java Buildpack Memory Calculator]. There is no need to `restage` an application after scaling the memory as restarting will cause the memory settings to be recalculated. 
+
+The total available memory is allocated into heap, Metaspace or PermGen (depending on the version of Oracle), stack, and native memory types.
 
-The total available memory is allocated to each memory type in proportion to its weighting. If the resultant size of a memory type lies outside its range, the size is constrained to
-the range, the constrained size is excluded from the remaining memory, and no further calculation is required for the memory type. If the resultant size of a memory size lies within its range, the size is included in the remaining memory. The remaining memory is then allocated to the remaining memory types in a similar fashion. Allocation terminates when none of the sizes of the remaining memory types is constrained by the corresponding range.
+The total available memory is first allocated to each memory type in proportion to its weighting (this is called ‘balancing'). If the resultant size of any memory type lies outside its range, the size is constrained to the range, the constrained size is excluded from the remaining memory, and no further calculation is required for that memory type. The remaining memory is then balanced against the memory types that are left, and the check is repeated until no calculated memory sizes lie outside their ranges. The remaining memory is then allocated to the remaining memory types according to the last balance step. This iteration terminates when none of the sizes of the remaining memory types is constrained by their corresponding ranges.
 
 Termination is guaranteed since there is a finite number of memory types and in each iteration either none of the remaining memory sizes is constrained by the corresponding range and allocation terminates or at least one memory size is constrained by the corresponding range and is omitted from the next iteration.
 
 [`config/open_jdk_jre.yml`]: ../config/open_jdk_jre.yml
 [Configuration and Extension]: ../README.md#configuration-and-extension
 [centos6]: http://download.pivotal.io.s3.amazonaws.com/openjdk/centos6/x86_64/index.yml
+[Java Buildpack Memory Calculator]: https://github.com/cloudfoundry/java-buildpack-memory-calculator
 [lucid]: http://download.pivotal.io.s3.amazonaws.com/openjdk/lucid/x86_64/index.yml
 [mountainlion]: http://download.pivotal.io.s3.amazonaws.com/openjdk/mountainlion/x86_64/index.yml
 [OpenJDK]: http://openjdk.java.net
diff --git a/docs/jre-oracle_jre.md b/docs/jre-oracle_jre.md
@@ -35,7 +35,7 @@ The JRE can be configured by modifying the [`config/oracle_jre.yml`][] file in t
 | `version` | The version of Java runtime to use.  Candidate versions can be found in the the repository that you have created to house the JREs. Note: version 1.8.0 and higher require the `memory_sizes` and `memory_heuristics` mappings to specify `metaspace` rather than `permgen`.
 
 ### Additional Resources
-The JRE can also be configured by overlaying a set of resources on the default distribution.  To do this, add files to the `resources/oracle_jre` directory in the buildpack fork.  For example, to add the JCE Unlimited Strength `local_policy.jar` add your file to `resources/oracle_jre/lib/security/local_policy.jar`.
+The JRE can also be configured by overlaying a set of resources on the default distribution. To do this, add files to the `resources/oracle_jre` directory in the buildpack fork. For example, to add the JCE Unlimited Strength `local_policy.jar` add your file to `resources/oracle_jre/lib/security/local_policy.jar`.
 
 ### Memory
 The total available memory is specified when an application is pushed as part of it's configuration. The Java buildpack uses this value to control the JRE's use of various regions of memory. The JRE memory settings can be influenced by configuring the `memory_sizes` and/or `memory_heuristics` mappings.
@@ -57,7 +57,7 @@ Memory sizes together with _memory weightings_ (described in the next section) a
 
 Memory sizes consist of a non-negative integer followed by a unit (`k` for kilobytes, `m` for megabytes, `g` for gigabytes; the case is not significant). Only the memory size `0` may be specified without a unit.
 
-The above memory size properties may be omitted, specified as a single value, or specified as a range. Ranges use the syntax `<lower bound>..<upper bound>`, although either bound may be omitted in which case the defaults of zero and the total available memory are used for the lower bound and upper bound, respectively. Examples of ranges are `100m..200m` (any value between 100 and 200 megabytes, inclusive) and `100m..` (any value greater than or equal to 100 megabytes).
+The above memory size properties may be omitted with an empty value, specified as a single value, or specified as a range. Ranges use the syntax `<lower bound>..<upper bound>`, although either bound may be omitted in which case the defaults of zero and the total available memory are used for the lower bound and upper bound, respectively. Examples of ranges are `100m..200m` (any value between 100 and 200 megabytes, inclusive) and `100m..` (any value greater than or equal to 100 megabytes).
 
 Each form of memory size is equivalent to a range. Omitting a memory size is equivalent to specifying the range `0..`. Specifying a single value is equivalent to specifying the range with that value as both the lower and upper bound, for example `128m` is equivalent to the range `128m..128m`.
 
@@ -67,26 +67,28 @@ Memory weightings are configured in the `memory_heuristics` mapping of [`config/
 ```yaml
 memory_heuristics:
   heap: 15
+  native: 2
   permgen: 5
   stack: 1
-  native: 2
 ```
 
 represent a maximum heap size three times as large as the maximum PermGen size, and so on.
 
 Memory weightings are used together with memory ranges to calculate the amount of memory for each memory type, as follows.
 
 #### Memory Calculation
+Memory calculation happens before every `start` of an application and is performed by an external program, the [Java Buildpack Memory Calculator]. There is no need to `restage` an application after scaling the memory as restarting will cause the memory settings to be recalculated. 
+
 The total available memory is allocated into heap, Metaspace or PermGen (depending on the version of Oracle), stack, and native memory types.
 
-The total available memory is allocated to each memory type in proportion to its weighting. If the resultant size of a memory type lies outside its range, the size is constrained to
-the range, the constrained size is excluded from the remaining memory, and no further calculation is required for the memory type. If the resultant size of a memory size lies within its range, the size is included in the remaining memory. The remaining memory is then allocated to the remaining memory types in a similar fashion. Allocation terminates when none of the sizes of the remaining memory types is constrained by the corresponding range.
+The total available memory is first allocated to each memory type in proportion to its weighting (this is called ‘balancing'). If the resultant size of any memory type lies outside its range, the size is constrained to the range, the constrained size is excluded from the remaining memory, and no further calculation is required for that memory type. The remaining memory is then balanced against the memory types that are left, and the check is repeated until no calculated memory sizes lie outside their ranges. The remaining memory is then allocated to the remaining memory types according to the last balance step. This iteration terminates when none of the sizes of the remaining memory types is constrained by their corresponding ranges.
 
 Termination is guaranteed since there is a finite number of memory types and in each iteration either none of the remaining memory sizes is constrained by the corresponding range and allocation terminates or at least one memory size is constrained by the corresponding range and is omitted from the next iteration.
 
 [`config/components.yml`]: ../config/components.yml
 [`config/oracle_jre.yml`]: ../config/oracle_jre.yml
 [Configuration and Extension]: ../README.md#configuration-and-extension
+[Java Buildpack Memory Calculator]: https://github.com/cloudfoundry/java-buildpack-memory-calculator
 [OpenJDK JRE]: jre-open_jdk_jre.md
 [Oracle]: http://www.oracle.com/technetwork/java/index.html
 [repositories]: extending-repositories.md
diff --git a/lib/java_buildpack/buildpack_version.rb b/lib/java_buildpack/buildpack_version.rb
@@ -45,7 +45,7 @@ class BuildpackVersion
 
     # Creates a new instance
     def initialize(should_log = true)
-      configuration = JavaBuildpack::Util::ConfigurationUtils.load 'version', should_log
+      configuration = JavaBuildpack::Util::ConfigurationUtils.load('version', true, should_log)
       @hash         = configuration['hash'] || hash
       @offline      = configuration['offline'] || ENV['OFFLINE'].to_b
       @remote       = configuration['remote'] || remote
diff --git a/lib/java_buildpack/framework/java_opts.rb b/lib/java_buildpack/framework/java_opts.rb
@@ -75,7 +75,7 @@ def parsed_java_opts
       end
 
       def supports_configuration?
-        @configuration.key? CONFIGURATION_PROPERTY
+        @configuration.key?(CONFIGURATION_PROPERTY) && !@configuration[CONFIGURATION_PROPERTY].nil?
       end
 
       def supports_environment?
diff --git a/lib/java_buildpack/logging/logger_factory.rb b/lib/java_buildpack/logging/logger_factory.rb
@@ -129,7 +129,8 @@ def ruby_mode
       def severity
         severity = ENV['JBP_LOG_LEVEL']
         severity = ruby_mode unless severity
-        severity = JavaBuildpack::Util::ConfigurationUtils.load('logging', false)['default_log_level'] unless severity
+        severity =
+          JavaBuildpack::Util::ConfigurationUtils.load('logging', true, false)['default_log_level'] unless severity
         severity = 'INFO' unless severity
 
         "::Logger::Severity::#{severity.upcase}".constantize
diff --git a/lib/java_buildpack/util/configuration_utils.rb b/lib/java_buildpack/util/configuration_utils.rb
@@ -35,15 +35,17 @@ class << self
         # 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 to load
+        # @param [Boolean] clean_nil_values whether empty/nil values should be removed along with their keys from the
+        #                                  returned configuration.
         # @param [Boolean] should_log whether the contents of the configuration file should be logged.  This value
         #                             should be left to its default and exists to allow the logger to use the utility.
         # @return [Hash] the configuration or an empty hash if the configuration file does not exist
-        def load(identifier, should_log = true)
+        def load(identifier, clean_nil_values = true, should_log = true)
           file = file_name(identifier)
 
           if file.exist?
             user_provided = ENV[environment_variable_name(identifier)]
-            configuration = load_configuration(file, user_provided, should_log)
+            configuration = load_configuration(file, user_provided, clean_nil_values, should_log)
           else
             logger.debug { "No configuration file #{file} found" } if should_log
           end
@@ -80,6 +82,17 @@ def write(identifier, new_content, should_log = true)
 
         private_constant :CONFIG_DIRECTORY, :ENVIRONMENT_VARIABLE_PATTERN
 
+        def clean_nil_values(configuration)
+          configuration.each do |key, value|
+            if value.is_a?(Hash)
+              configuration[key] = clean_nil_values value
+            elsif value.nil?
+              configuration.delete key
+            end
+          end
+          configuration
+        end
+
         def file_name(identifier)
           CONFIG_DIRECTORY + "#{identifier}.yml"
         end
@@ -96,7 +109,7 @@ def header(file)
           header
         end
 
-        def load_configuration(file, user_provided, should_log)
+        def load_configuration(file, user_provided, clean_nil_values, should_log)
           configuration = YAML.load_file(file)
           logger.debug { "Configuration from #{file}: #{configuration}" } if should_log
 
@@ -105,15 +118,14 @@ def load_configuration(file, user_provided, should_log)
             if user_provided_value.is_a?(Hash)
               configuration = do_merge(configuration, user_provided_value, should_log)
             elsif user_provided_value.is_a?(Array)
-              user_provided_value.each do |new_prop|
-                configuration = do_merge(configuration, new_prop, should_log)
-              end
+              user_provided_value.each { |new_prop| configuration = do_merge(configuration, new_prop, should_log) }
             else
               fail "User configuration value is not valid: #{user_provided_value}"
             end
             logger.debug { "Configuration from #{file} modified with: #{user_provided}" } if should_log
           end
 
+          clean_nil_values configuration if clean_nil_values
           configuration
         end
 
diff --git a/rakelib/dependency_cache_task.rb b/rakelib/dependency_cache_task.rb
diff --git a/spec/java_buildpack/buildpack_version_spec.rb b/spec/java_buildpack/buildpack_version_spec.rb
diff --git a/spec/java_buildpack/framework/java_opts_spec.rb b/spec/java_buildpack/framework/java_opts_spec.rb
diff --git a/spec/java_buildpack/logging/logger_factory_spec.rb b/spec/java_buildpack/logging/logger_factory_spec.rb
diff --git a/spec/java_buildpack/util/configuration_utils_spec.rb b/spec/java_buildpack/util/configuration_utils_spec.rb
