diff --git a/config/app_dynamics_agent.yml b/config/app_dynamics_agent.yml index 69b2b1f9f4..c898bcdfef 100644 --- a/config/app_dynamics_agent.yml +++ b/config/app_dynamics_agent.yml @@ -18,6 +18,6 @@ version: + repository_root: https://packages.appdynamics.com/java default_application_name: $(jq -r -n "$VCAP_APPLICATION | .space_name + \":\" + .application_name | @sh") -default_node_name: $(jq -r -n "\"$APPD_CF_NODE_PREFIX\" + ($VCAP_APPLICATION | .application_name) + \":$CF_INSTANCE_INDEX\"") +default_node_name: $(jq -r -n "\"$APPD_CF_NODE_PREFIX\" + ($VCAP_APPLICATION | .application_name) + \":$CF_INSTANCE_INDEX\" | @sh") default_tier_name: -default_unique_host_name: $(jq -r -n "$VCAP_APPLICATION | .application_id + \":$CF_INSTANCE_INDEX\"") +default_unique_host_name: $(jq -r -n "$VCAP_APPLICATION | .application_id + \":$CF_INSTANCE_INDEX\" | @sh") diff --git a/docs/framework-app_dynamics_agent.md b/docs/framework-app_dynamics_agent.md index 4a700dfcb8..7a74a0d3c4 100644 --- a/docs/framework-app_dynamics_agent.md +++ b/docs/framework-app_dynamics_agent.md @@ -13,21 +13,23 @@ The AppDynamics Agent Framework causes an application to be automatically config Tags are printed to standard output by the buildpack detect script ## User-Provided Service -When binding AppDynamics using a user-provided service, it must have name or tag with `app-dynamics` or `appdynamics` in it. The credential payload can contain the following entries. **Note:** Credentials marked as "(Optional)" may be required for some versions of the AppDynamics agent. Please see the [AppDynamics Java Agent Configuration Properties][] for the version of the agent used by your application for more details. +When binding AppDynamics using a user-provided service, it must have name or tag with `app-dynamics` or `appdynamics` in it. The credential payload can contain the following entries. | Name | Description | ---- | ----------- -| `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 application's name +| `account-access-key` | The account access key to use when authenticating with the controller +| `account-name` | The account name to use when authenticating with the controller | `host-name` | The controller host name +| `port` | The controller port +| `ssl-enabled` | Whether or not to use an SSL connection to the controller +| `application-name` | (Optional) the application's 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. +**Note:** Some credentials were previously marked as "(Optional)" as requirements have changed across versions of the AppDynamics agent. Please see the [AppDynamics Java Agent Configuration Properties][] for the version of the agent used by your application for more details. + ## Configuration For general information on configuring the buildpack, including how to specify configuration values through environment variables, refer to [Configuration and Extension][]. diff --git a/lib/java_buildpack/framework/app_dynamics_agent.rb b/lib/java_buildpack/framework/app_dynamics_agent.rb index ed763db979..9e21b7510b 100644 --- a/lib/java_buildpack/framework/app_dynamics_agent.rb +++ b/lib/java_buildpack/framework/app_dynamics_agent.rb @@ -16,6 +16,7 @@ # limitations under the License. require 'fileutils' +require 'shellwords' require 'java_buildpack/component/versioned_dependency_component' require 'java_buildpack/framework' @@ -79,30 +80,36 @@ def supports? private_constant :CONFIG_FILES, :FILTER def application_name(java_opts, credentials) - name = credentials['application-name'] || @configuration['default_application_name'] || - @application.details['application_name'] - java_opts.add_system_property('appdynamics.agent.applicationName', "\\\"#{name}\\\"") + name = Shellwords.escape(@application.details['application_name']) + name = @configuration['default_application_name'] if @configuration['default_application_name'] + name = Shellwords.escape(credentials['application-name']) if credentials['application-name'] + + java_opts.add_system_property('appdynamics.agent.applicationName', name.to_s) end def account_access_key(java_opts, credentials) account_access_key = credentials['account-access-key'] || credentials.dig('account-access-secret', 'secret') + account_access_key = Shellwords.escape(account_access_key) + java_opts.add_system_property 'appdynamics.agent.accountAccessKey', account_access_key if account_access_key end def account_name(java_opts, credentials) account_name = credentials['account-name'] - java_opts.add_system_property 'appdynamics.agent.accountName', account_name if account_name + java_opts.add_system_property 'appdynamics.agent.accountName', Shellwords.escape(account_name) if account_name end def host_name(java_opts, credentials) host_name = credentials['host-name'] raise "'host-name' credential must be set" unless host_name - java_opts.add_system_property 'appdynamics.controller.hostName', host_name + java_opts.add_system_property 'appdynamics.controller.hostName', Shellwords.escape(host_name) end def node_name(java_opts, credentials) - name = credentials['node-name'] || @configuration['default_node_name'] + name = @configuration['default_node_name'] + name = Shellwords.escape(credentials['node-name']) if credentials['node-name'] + java_opts.add_system_property('appdynamics.agent.nodeName', name.to_s) end @@ -117,13 +124,17 @@ def ssl_enabled(java_opts, credentials) end def tier_name(java_opts, credentials) - name = credentials['tier-name'] || @configuration['default_tier_name'] || - @application.details['application_name'] + name = Shellwords.escape(@application.details['application_name']) + name = @configuration['default_tier_name'] if @configuration['default_tier_name'] + name = Shellwords.escape(credentials['tier-name']) if credentials['tier-name'] + java_opts.add_system_property('appdynamics.agent.tierName', name.to_s) end def unique_host_name(java_opts) - name = @configuration['default_unique_host_name'] || @application.details['application_name'] + name = @configuration['default_unique_host_name'] + name = Shellwords.escape(@application.details['application_name']) if @application.details['application_name'] + java_opts.add_system_property('appdynamics.agent.uniqueHostId', name.to_s) end diff --git a/spec/java_buildpack/framework/app_dynamics_agent_spec.rb b/spec/java_buildpack/framework/app_dynamics_agent_spec.rb index ac981959a6..f8f988d567 100644 --- a/spec/java_buildpack/framework/app_dynamics_agent_spec.rb +++ b/spec/java_buildpack/framework/app_dynamics_agent_spec.rb @@ -66,7 +66,7 @@ expect(java_opts).to include('-javaagent:$PWD/.java-buildpack/app_dynamics_agent/javaagent.jar') expect(java_opts).to include('-Dappdynamics.controller.hostName=test-host-name') - expect(java_opts).to include('-Dappdynamics.agent.applicationName=\"test-application-name\"') + expect(java_opts).to include('-Dappdynamics.agent.applicationName=test-application-name') expect(java_opts).to include('-Dappdynamics.agent.tierName=test-application-name') expect(java_opts).to include('-Dappdynamics.agent.nodeName=$(expr "$VCAP_APPLICATION" : ' \ '\'.*instance_index[": ]*\\([[:digit:]]*\\).*\')') @@ -83,12 +83,38 @@ end context do - let(:credentials) { super().merge 'application-name' => 'another-test-application-name' } + let(:credentials) { super().merge 'tier-name' => 'another-test tier-name' } - it 'adds application_name from credentials to JAVA_OPTS if specified' do + it 'adds tier_name from credentials with space in name to JAVA_OPTS if specified' do component.release - expect(java_opts).to include('-Dappdynamics.agent.applicationName=\"another-test-application-name\"') + expect(java_opts).to include('-Dappdynamics.agent.tierName=another-test\ tier-name') + end + end + + context do + let(:credentials) { super().merge 'application-name' => 'another-test application-name' } + + it 'adds application_name from credentials with space in name 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(:configuration) do + { 'default_tier_name' => nil, + 'default_node_name' => nil, + 'default_application_name' => 'default application-name' } + end + + it 'adds application_name from default config to JAVA_OPTS if specified' do + component.release + + # should not be escaped, escaping happens at runtime because default value is a sub-command + # executed in the runtime container + expect(java_opts).to include('-Dappdynamics.agent.applicationName=default application-name') end end