Merge pull request #1084 from DataDog/revert-1068-jakepruitt/SYNTH-10…

…966/support-blocking-ip-addresses-for-tunnel Revert "[SYNTH-10966] Support blocking IP addresses in the tunnel"
DataDog · Oct 11, 2023 · b238a40 · b238a40
2 parents 1d4e6b1 + f29c479
commit b238a40
Show file tree

Hide file tree

Showing 14 changed files with 7 additions and 271 deletions.
diff --git a/LICENSE-3rdparty.csv b/LICENSE-3rdparty.csv
@@ -43,8 +43,6 @@ fuzzy,import,MIT,Copyright (c) 2012 Matt York
 glob,import,ISC,Copyright (c) Isaac Z. Schlueter and Contributors
 inquirer,import,MIT,Copyright (c) 2012 Simon Boudrias
 inquirer-checkbox-plus-prompt,import,MIT,Copyright (c) 2018 Mohammad Anas Fares
-ipaddr.js,import,MIT,github.com/whitequark/ipaddr.js
-is-in-subnet,import,MIT,github.com/natesilva/is-in-subnet
 jest,dev,MIT,"Copyright (c) Facebook, Inc. and its affiliates."
 js-yaml,import,MIT,Copyright (C) 2011-2015 by Vitaly Puzrin
 ora,import,MIT,Copyright (c) Sindre Sorhus <[email protected]> (https://sindresorhus.com)

diff --git a/package.json b/package.json
@@ -91,8 +91,6 @@
     "http-proxy-agent": "^7.0.0",
     "inquirer": "^8.2.5",
     "inquirer-checkbox-plus-prompt": "^1.4.2",
-    "ipaddr.js": "^2.1.0",
-    "is-in-subnet": "^4.0.1",
     "js-yaml": "3.13.1",
     "jszip": "^3.10.1",
     "ora": "5.4.1",

diff --git a/src/commands/synthetics/__tests__/cli.test.ts b/src/commands/synthetics/__tests__/cli.test.ts
@@ -72,19 +72,10 @@ describe('run-test', () => {
 
     test('override from config file', async () => {
       const overrideConfigFile = {
-        allowedIPRanges: {
-          4: ['127.0.0.1'],
-          6: ['::1'],
-        },
         apiKey: 'fake_api_key',
         appKey: 'fake_app_key',
-        blockedIPRanges: {
-          4: ['127.0.0.1/8'],
-          6: ['::1/8'],
-        },
         configPath: 'src/commands/synthetics/__tests__/config-fixtures/config-with-all-keys.json',
         datadogSite: 'datadoghq.eu',
-        enableDefaultBlockedIPRanges: true,
         failOnCriticalErrors: true,
         failOnMissingTests: true,
         failOnTimeout: false,

diff --git a/src/commands/synthetics/__tests__/config-fixtures/config-with-all-keys.json b/src/commands/synthetics/__tests__/config-fixtures/config-with-all-keys.json
@@ -1,17 +1,8 @@
 {
-  "allowedIPRanges": {
-    "4": ["127.0.0.1"],
-    "6": ["::1"]
-  },
   "apiKey": "fake_api_key",
   "appKey": "fake_app_key",
-  "blockedIPRanges": {
-    "4": ["127.0.0.1/8"],
-    "6": ["::1/8"]
-  },
   "configPath": "fake-datadog-ci.json",
   "datadogSite": "datadoghq.eu",
-  "enableDefaultBlockedIPRanges": true,
   "failOnCriticalErrors": true,
   "failOnMissingTests": true,
   "failOnTimeout": false,

diff --git a/src/commands/synthetics/__tests__/fixtures.ts b/src/commands/synthetics/__tests__/fixtures.ts
@@ -64,13 +64,10 @@ export const mockReporter: MainReporter = {
 }
 
 export const ciConfig: RunTestsCommandConfig = {
-  allowedIPRanges: {4: [], 6: []},
   apiKey: '',
   appKey: '',
-  blockedIPRanges: {4: [], 6: []},
   configPath: 'datadog-ci.json',
   datadogSite: 'datadoghq.com',
-  enableDefaultBlockedIPRanges: false,
   failOnCriticalErrors: false,
   failOnMissingTests: false,
   failOnTimeout: true,

diff --git a/src/commands/synthetics/__tests__/tunnel/tunnel.test.ts b/src/commands/synthetics/__tests__/tunnel/tunnel.test.ts
@@ -4,17 +4,11 @@ import {getProxyAgent} from '../../../../../src/helpers/utils'
 
 import {getTunnelReporter} from '../../reporters/default'
 import {Tunnel} from '../../tunnel'
-import {RESERVED_ADDRESS_BLOCKS} from '../../tunnel/blockedIPs'
 import {WebSocket} from '../../tunnel/websocket'
 jest.mock('../../tunnel/websocket')
 
 import {mockReporter} from '../fixtures'
 
-const EMPTY_FIREWALL_RULES = {
-  allowedSubnetsByFamily: {4: [], 6: []},
-  blockedSubnetsByFamily: {4: [], 6: []},
-}
-
 describe('Tunnel', () => {
   const mockConnect = jest.fn()
   const mockClose = jest.fn()
@@ -38,7 +32,7 @@ describe('Tunnel', () => {
   test('starts by connecting over WebSocket and closes the WebSocket when stopping', async () => {
     mockedWebSocket.mockImplementation(() => mockWebSocket as any)
 
-    const tunnel = new Tunnel(wsPresignedURL, testIDs, EMPTY_FIREWALL_RULES, undefined, mockTunnelReporter)
+    const tunnel = new Tunnel(wsPresignedURL, testIDs, undefined, mockTunnelReporter)
     const connectionInfo = await tunnel.start()
     expect(WebSocket).toHaveBeenCalledWith(wsPresignedURL, undefined)
     expect(mockConnect).toHaveBeenCalled()
@@ -66,7 +60,7 @@ describe('Tunnel', () => {
     mockConnect.mockImplementation(() => {
       throw websocketConnectError
     })
-    const tunnel = new Tunnel(wsPresignedURL, testIDs, EMPTY_FIREWALL_RULES, undefined, mockTunnelReporter)
+    const tunnel = new Tunnel(wsPresignedURL, testIDs, undefined, mockTunnelReporter)
     await expect(tunnel.start()).rejects.toThrow(websocketConnectError)
     expect(mockClose).toHaveBeenCalled()
     mockConnect.mockRestore()
@@ -79,34 +73,11 @@ describe('Tunnel', () => {
       port: 8080,
       protocol: 'http',
     })
-    const tunnel = new Tunnel(wsPresignedURL, testIDs, EMPTY_FIREWALL_RULES, localProxyAgent, mockTunnelReporter)
+    const tunnel = new Tunnel(wsPresignedURL, testIDs, localProxyAgent, mockTunnelReporter)
     await tunnel.start()
     expect(WebSocket).toHaveBeenCalledWith(wsPresignedURL, localProxyAgent)
 
     // Stop the tunnel
     await tunnel.stop()
   })
-
-  test('blocks default IP addresses', async () => {
-    const defaultBlockedIPs = {
-      allowedSubnetsByFamily: {4: [], 6: []},
-      blockedSubnetsByFamily: RESERVED_ADDRESS_BLOCKS,
-    }
-
-    const tunnel = new Tunnel(wsPresignedURL, testIDs, defaultBlockedIPs, undefined, mockTunnelReporter)
-    expect(tunnel.validateIP('127.0.0.1')).toBe(false)
-    expect(tunnel.validateIP('::1')).toBe(false)
-    expect(tunnel.validateIP('169.254.169.254')).toBe(false)
-    expect(tunnel.validateIP('54.52.123.45')).toBe(true)
-  })
-
-  test('allows localhost IP address', async () => {
-    const defaultBlockedIPs = {
-      allowedSubnetsByFamily: {4: ['127.0.0.1/32'], 6: []},
-      blockedSubnetsByFamily: {4: ['127.0.0.0/8'], 6: []},
-    }
-
-    const tunnel = new Tunnel(wsPresignedURL, testIDs, defaultBlockedIPs, undefined, mockTunnelReporter)
-    expect(tunnel.validateIP('127.0.0.1')).toBe(true)
-  })
 })
diff --git a/src/commands/synthetics/__tests__/tunnel/websocket.test.ts b/src/commands/synthetics/__tests__/tunnel/websocket.test.ts
@@ -35,10 +35,8 @@ describe('Proxy configuration', () => {
       // Instead we mock the util function that is called in the command.
       jest.spyOn(ciUtils, 'resolveConfigFromFile').mockImplementationOnce(async (config) => ({
         ...(config as Record<string, unknown>),
-        allowedIPRanges: {4: [], 6: []},
         apiKey: '123',
         appKey: '123',
-        blockedIPRanges: {4: [], 6: []},
         proxy: proxyOpts,
         publicIds: ['123-456-789'],
         tunnel: true,

diff --git a/src/commands/synthetics/interfaces.ts b/src/commands/synthetics/interfaces.ts
@@ -2,7 +2,6 @@ import {Metadata} from '../../helpers/interfaces'
 import {ProxyConfiguration} from '../../helpers/utils'
 
 import {TunnelInfo} from './tunnel'
-import {RESERVED_ADDRESS_BLOCKS} from './tunnel/blockedIPs'
 
 export type SupportedReporter = 'junit' | 'default'
 
@@ -376,10 +375,7 @@ export interface APIHelperConfig {
 export interface SyntheticsCIConfig extends APIHelperConfig {}
 
 export interface RunTestsCommandConfig extends SyntheticsCIConfig {
-  allowedIPRanges: typeof RESERVED_ADDRESS_BLOCKS
-  blockedIPRanges: typeof RESERVED_ADDRESS_BLOCKS
   configPath: string
-  enableDefaultBlockedIPRanges: boolean
   failOnCriticalErrors: boolean
   failOnMissingTests: boolean
   failOnTimeout: boolean

diff --git a/src/commands/synthetics/run-tests-command.ts b/src/commands/synthetics/run-tests-command.ts
@@ -11,7 +11,6 @@ import {MainReporter, Reporter, Result, RunTestsCommandConfig, Summary} from './
 import {DefaultReporter} from './reporters/default'
 import {JUnitReporter} from './reporters/junit'
 import {executeTests} from './run-tests-lib'
-import {hasValidRanges, IpFamily, RESERVED_ADDRESS_BLOCKS} from './tunnel/blockedIPs'
 import {
   getExitReason,
   getOrgSettings,
@@ -28,13 +27,10 @@ export const MAX_TESTS_TO_TRIGGER = 100
 export const DEFAULT_POLLING_TIMEOUT = 30 * 60 * 1000
 
 export const DEFAULT_COMMAND_CONFIG: RunTestsCommandConfig = {
-  allowedIPRanges: {[IpFamily.v4]: [], [IpFamily.v6]: []} as typeof RESERVED_ADDRESS_BLOCKS,
   apiKey: '',
   appKey: '',
-  blockedIPRanges: {[IpFamily.v4]: [], [IpFamily.v6]: []} as typeof RESERVED_ADDRESS_BLOCKS,
   configPath: 'datadog-ci.json',
   datadogSite: 'datadoghq.com',
-  enableDefaultBlockedIPRanges: false,
   failOnCriticalErrors: false,
   failOnMissingTests: false,
   failOnTimeout: true,
@@ -132,23 +128,6 @@ export class RunTestsCommand extends Command {
   })
   private variableStrings = Option.Array('-v,--variable', {description: 'Pass a variable override.'})
 
-  private enableDefaultBlockedIPRanges = Option.Boolean('--enableDefaultBlockedIPRanges', {
-    description:
-      'Deny access to reserved IP ranges (IANA [IPv4](https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml) and [IPv6](https://www.iana.org/assignments/iana-ipv6-special-registry/iana-ipv6-special-registry.xhtml) Special-Purpose Address Registry), except for those explicitly set in --allowedIPRanges.',
-  })
-  private allowedIPv4Ranges = Option.Array('--allowedIPRanges.4', {
-    description: 'Grant access to IPv4 ranges while using the tunnel (has precedence over --blockedIPRanges.4).',
-  })
-  private allowedIPv6Ranges = Option.Array('--allowedIPRanges.6', {
-    description: 'Grant access to IPv6 ranges while using the tunnel (has precedence over --blockedIPRanges.6).',
-  })
-  private blockedIPv4Ranges = Option.Array('--blockedIPRanges.4', {
-    description: 'Deny access to IP ranges while using the tunnel. (such as --blockedIPRanges.4="127.0.0.0/8").',
-  })
-  private blockedIPv6Ranges = Option.Array('--blockedIPRanges.6', {
-    description: 'Deny access to IP ranges while using the tunnel. (such as --blockedIPRanges.6="::1/128").',
-  })
-
   private reporter?: MainReporter
   private config: RunTestsCommandConfig = JSON.parse(JSON.stringify(DEFAULT_COMMAND_CONFIG)) // Deep copy to avoid mutation during unit tests
 
@@ -239,7 +218,6 @@ export class RunTestsCommand extends Command {
         appKey: this.appKey,
         configPath: this.configPath,
         datadogSite: this.datadogSite,
-        enableDefaultBlockedIPRanges: this.enableDefaultBlockedIPRanges,
         failOnCriticalErrors: this.failOnCriticalErrors,
         failOnMissingTests: this.failOnMissingTests,
         failOnTimeout: this.failOnTimeout,
@@ -249,22 +227,6 @@ export class RunTestsCommand extends Command {
         subdomain: this.subdomain,
         testSearchQuery: this.testSearchQuery,
         tunnel: this.tunnel,
-        ...(this.allowedIPv4Ranges || this.allowedIPv6Ranges
-          ? {
-              allowedIPRanges: {
-                [IpFamily.v4]: this.allowedIPv4Ranges ?? [],
-                [IpFamily.v6]: this.allowedIPv6Ranges ?? [],
-              },
-            }
-          : {}),
-        ...(this.blockedIPv4Ranges || this.blockedIPv6Ranges
-          ? {
-              blockedIPRanges: {
-                [IpFamily.v4]: this.blockedIPv4Ranges ?? [],
-                [IpFamily.v6]: this.blockedIPv6Ranges ?? [],
-              },
-            }
-          : {}),
       })
     )
 
@@ -291,11 +253,5 @@ export class RunTestsCommand extends Command {
         )}) must match one of the sites supported by Datadog.\nFor more information, see "Site parameter" in our documentation: https://docs.datadoghq.com/getting_started/site/#access-the-datadog-site`
       )
     }
-
-    for (const range of [this.config.allowedIPRanges, this.config.blockedIPRanges]) {
-      if (!hasValidRanges(range)) {
-        throw new CiError('INVALID_CONFIG', `Invalid IP range (${JSON.stringify(range)})`)
-      }
-    }
   }
 }
diff --git a/src/commands/synthetics/run-tests-lib.ts b/src/commands/synthetics/run-tests-lib.ts
@@ -20,7 +20,7 @@ import {
 import {DefaultReporter, getTunnelReporter} from './reporters/default'
 import {JUnitReporter} from './reporters/junit'
 import {DEFAULT_COMMAND_CONFIG, MAX_TESTS_TO_TRIGGER} from './run-tests-command'
-import {getTunnelFirewallRules, Tunnel} from './tunnel'
+import {Tunnel} from './tunnel'
 import {
   getReporter,
   getOrgSettings,
@@ -123,8 +123,7 @@ export const executeTests = async (
     try {
       const tunnelProxyAgent = getProxyAgent(config.proxy)
       const tunnelReporter = getTunnelReporter(reporter)
-      const tunnelFirewallRules = getTunnelFirewallRules(config)
-      tunnel = new Tunnel(presignedURL, publicIdsToTrigger, tunnelFirewallRules, tunnelProxyAgent, tunnelReporter)
+      tunnel = new Tunnel(presignedURL, publicIdsToTrigger, tunnelProxyAgent, tunnelReporter)
 
       const tunnelInfo = await tunnel.start()
       overriddenTestsToTrigger.forEach((testToTrigger) => {

diff --git a/src/commands/synthetics/tunnel/blockedIPs.ts b/src/commands/synthetics/tunnel/blockedIPs.ts
diff --git a/src/commands/synthetics/tunnel/index.ts b/src/commands/synthetics/tunnel/index.ts
@@ -1 +1 @@
-export {getTunnelFirewallRules, Tunnel, TunnelInfo, TunnelReporter} from './tunnel'
+export {Tunnel, TunnelInfo, TunnelReporter} from './tunnel'