TO DISCUSS. what if we enforce contextPropagationOnly to be configure…

…d when active is defined
build-security · Nov 5, 2021 · 62dda4f · 62dda4f
1 parent c25a31b
commit 62dda4f
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 53 deletions.
diff --git a/docs/developer/getting-started/debugging.asciidoc b/docs/developer/getting-started/debugging.asciidoc
@@ -53,7 +53,7 @@ APM
 https://www.elastic.co/guide/en/apm/agent/rum-js/current/index.html[Real
 User Monitoring agent] is not available in the {kib} distributables,
 however the agent can be enabled by setting `ELASTIC_APM_ACTIVE` to
-`true`. flags
+`true` and `ELASTIC_APM_CONTEXT_PROPAGATION_ONLY` to `false`. flags
 
 ....
 ELASTIC_APM_ACTIVE=true yarn start

diff --git a/packages/kbn-apm-config-loader/src/config.test.ts b/packages/kbn-apm-config-loader/src/config.test.ts
@@ -133,6 +133,7 @@ describe('ApmConfiguration', () => {
       elastic: {
         apm: {
           active: true,
+          contextPropagationOnly: false,
           serverUrl: 'https://url',
           secretToken: 'secret',
         },
@@ -151,6 +152,7 @@ describe('ApmConfiguration', () => {
   it('loads the configuration from the dev config is present', () => {
     devConfigMock.raw = {
       active: true,
+      contextPropagationOnly: false,
       serverUrl: 'https://dev-url.co',
     };
     const config = new ApmConfiguration(mockedRootDir, {}, false);
@@ -179,6 +181,7 @@ describe('ApmConfiguration', () => {
       elastic: {
         apm: {
           active: true,
+          contextPropagationOnly: false,
           serverUrl: 'https://url',
           secretToken: 'secret',
         },
@@ -272,35 +275,7 @@ describe('ApmConfiguration', () => {
       );
     });
 
-    it('is "false" if "active: true" configured and "contextPropagationOnly" is not specified', () => {
-      const kibanaConfig = {
-        elastic: {
-          apm: {
-            active: true,
-          },
-        },
-      };
-
-      expect(
-        new ApmConfiguration(mockedRootDir, kibanaConfig, false).getConfig('serviceName')
-      ).toEqual(
-        expect.objectContaining({
-          active: true,
-          contextPropagationOnly: false,
-        })
-      );
-
-      expect(
-        new ApmConfiguration(mockedRootDir, kibanaConfig, true).getConfig('serviceName')
-      ).toEqual(
-        expect.objectContaining({
-          active: true,
-          contextPropagationOnly: false,
-        })
-      );
-    });
-
-    it('throws if "active: false" set without configuring "contextPropagationOnly: false"', () => {
+    it('throws if "active: false" set without configuring "contextPropagationOnly"', () => {
       const kibanaConfig = {
         elastic: {
           apm: {
@@ -312,42 +287,35 @@ describe('ApmConfiguration', () => {
       expect(() =>
         new ApmConfiguration(mockedRootDir, kibanaConfig, false).getConfig('serviceName')
       ).toThrowErrorMatchingInlineSnapshot(
-        `"APM is disabled, but context propagation is enabled. Please disable context propagation with contextPropagationOnly:false"`
+        `"[apm.active] is configured but contextPropagationOnly is not. Please, set [apm.contextPropagationOnly] value explicitly."`
       );
 
       expect(() =>
         new ApmConfiguration(mockedRootDir, kibanaConfig, true).getConfig('serviceName')
       ).toThrowErrorMatchingInlineSnapshot(
-        `"APM is disabled, but context propagation is enabled. Please disable context propagation with contextPropagationOnly:false"`
+        `"[apm.active] is configured but contextPropagationOnly is not. Please, set [apm.contextPropagationOnly] value explicitly."`
       );
     });
 
-    it('does not throw if "active: false" and "contextPropagationOnly: false" configured', () => {
+    it('throws if "active: true" set without configuring "contextPropagationOnly"', () => {
       const kibanaConfig = {
         elastic: {
           apm: {
-            active: false,
-            contextPropagationOnly: false,
+            active: true,
           },
         },
       };
 
-      expect(
+      expect(() =>
         new ApmConfiguration(mockedRootDir, kibanaConfig, false).getConfig('serviceName')
-      ).toEqual(
-        expect.objectContaining({
-          active: false,
-          contextPropagationOnly: false,
-        })
+      ).toThrowErrorMatchingInlineSnapshot(
+        `"[apm.active] is configured but contextPropagationOnly is not. Please, set [apm.contextPropagationOnly] value explicitly."`
       );
 
-      expect(
+      expect(() =>
         new ApmConfiguration(mockedRootDir, kibanaConfig, true).getConfig('serviceName')
-      ).toEqual(
-        expect.objectContaining({
-          active: false,
-          contextPropagationOnly: false,
-        })
+      ).toThrowErrorMatchingInlineSnapshot(
+        `"[apm.active] is configured but contextPropagationOnly is not. Please, set [apm.contextPropagationOnly] value explicitly."`
       );
     });
   });

diff --git a/packages/kbn-apm-config-loader/src/config.ts b/packages/kbn-apm-config-loader/src/config.ts
@@ -113,6 +113,8 @@ export class ApmConfiguration {
 
     if (process.env.ELASTIC_APM_ACTIVE === 'true') {
       config.active = true;
+    } else if (process.env.ELASTIC_APM_ACTIVE === 'false') {
+      config.active = false;
     }
 
     if (process.env.ELASTIC_APM_CONTEXT_PROPAGATION_ONLY === 'true') {
@@ -268,16 +270,12 @@ export class ApmConfiguration {
       this.getConfigFromEnv()
     );
 
-    if (config.active === false && config.contextPropagationOnly !== false) {
+    if (config.active !== undefined && config.contextPropagationOnly === undefined) {
       throw new Error(
-        'APM is disabled, but context propagation is enabled. Please disable context propagation with contextPropagationOnly:false'
+        '[apm.active] is configured but contextPropagationOnly is not. Please, set [apm.contextPropagationOnly] value explicitly.'
       );
     }
 
-    if (config.active === true) {
-      config.contextPropagationOnly = config.contextPropagationOnly ?? false;
-    }
-
     return config;
   }
 }