From 29ecc66564a13cc1ae71b7f1cc62c4c081446107 Mon Sep 17 00:00:00 2001 From: Daniel Mikusa Date: Thu, 29 Apr 2021 13:33:41 -0400 Subject: [PATCH 1/3] Adjust the way arguments are being escaped. Values that are selected during build time by the buildpack are escaped now. Values that are pulled at runtime before an app starts are escaped at that point in time, so we skip that here or it results in double escaping of values. --- .../framework/app_dynamics_agent.rb | 9 +++++--- .../framework/app_dynamics_agent_spec.rb | 22 ++++++++++++++++--- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/lib/java_buildpack/framework/app_dynamics_agent.rb b/lib/java_buildpack/framework/app_dynamics_agent.rb index ed763db97..f8fa14f7e 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,9 +80,11 @@ 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) diff --git a/spec/java_buildpack/framework/app_dynamics_agent_spec.rb b/spec/java_buildpack/framework/app_dynamics_agent_spec.rb index ac981959a..5c00703e6 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,28 @@ end context do - let(:credentials) { super().merge 'application-name' => 'another-test-application-name' } + 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\"') + 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 From 7e8f8d80f6ded6920480df876342f4ba61495a0e Mon Sep 17 00:00:00 2001 From: Daniel Mikusa Date: Wed, 5 May 2021 11:05:38 -0400 Subject: [PATCH 2/3] Also escape the other App Dynamics configuration properties --- config/app_dynamics_agent.yml | 4 ++-- .../framework/app_dynamics_agent.rb | 20 +++++++++++++------ .../framework/app_dynamics_agent_spec.rb | 12 ++++++++++- 3 files changed, 27 insertions(+), 9 deletions(-) diff --git a/config/app_dynamics_agent.yml b/config/app_dynamics_agent.yml index 69b2b1f9f..c898bcdfe 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/lib/java_buildpack/framework/app_dynamics_agent.rb b/lib/java_buildpack/framework/app_dynamics_agent.rb index f8fa14f7e..9e21b7510 100644 --- a/lib/java_buildpack/framework/app_dynamics_agent.rb +++ b/lib/java_buildpack/framework/app_dynamics_agent.rb @@ -89,23 +89,27 @@ def application_name(java_opts, credentials) 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 @@ -120,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 5c00703e6..f8f988d56 100644 --- a/spec/java_buildpack/framework/app_dynamics_agent_spec.rb +++ b/spec/java_buildpack/framework/app_dynamics_agent_spec.rb @@ -82,10 +82,20 @@ end end + context do + let(:credentials) { super().merge 'tier-name' => 'another-test tier-name' } + + 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.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 to JAVA_OPTS if specified' do + 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') From cd136e409886184ef808072d859718493c5d1e6e Mon Sep 17 00:00:00 2001 From: Daniel Mikusa Date: Wed, 5 May 2021 11:38:50 -0400 Subject: [PATCH 3/3] Updates which fields are optional for AppDynamics --- docs/framework-app_dynamics_agent.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/framework-app_dynamics_agent.md b/docs/framework-app_dynamics_agent.md index 4a700dfcb..7a74a0d3c 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][].