Merge remote-tracking branch 'origin/master' into sbruens/prometheus-console

.gitignore

@@ -1,5 +1,6 @@
 .DS_Store
 .idea/
+.task/
 .vscode/
 /build/
 /src/server_manager/install_scripts/do_install_script.ts

Taskfile.yml

@@ -22,6 +22,10 @@ includes:
     taskfile: ./src/shadowbox/Taskfile.yml
     vars: {OUTPUT_BASE: '{{joinPath .BUILD_ROOT "shadowbox"}}'}
 
+  third_party:
+    taskfile: ./third_party/Taskfile.yml
+    vars: {OUTPUT_BASE: '{{joinPath .BUILD_ROOT "third_party"}}'}
+
 tasks:
   clean:
     desc: Clean output files

src/metrics_server/connection_metrics.spec.ts

@@ -32,14 +32,22 @@ const VALID_USER_REPORT2: HourlyUserConnectionMetricsReport = {
 };
 
 /*
- * A user report to test legacy fields to ensure backwards compatibility with
- * older servers that may still send reports like this.
+ * Legacy access key user reports to ensure backwards compatibility with servers not
+ * synced past https://github.com/Jigsaw-Code/outline-server/pull/1529).
  */
-const LEGACY_USER_REPORT = {
+const LEGACY_PER_KEY_USER_REPORT: HourlyUserConnectionMetricsReport = {
   userId: 'foo',
+  bytesTransferred: 123,
+};
+
+/*
+ * Legacy multiple countries user reports to ensure backwards compatibility with servers
+ * not synced past https://github.com/Jigsaw-Code/outline-server/pull/1242.
+ */
+const LEGACY_PER_LOCATION_USER_REPORT: HourlyUserConnectionMetricsReport = {
+  userId: 'foobar',
   countries: ['US', 'UK'],
   bytesTransferred: 123,
-  tunnelTimeSec: 789,
 };
 
 const VALID_REPORT: HourlyConnectionMetricsReport = {
@@ -49,10 +57,17 @@ const VALID_REPORT: HourlyConnectionMetricsReport = {
   userReports: [
     structuredClone(VALID_USER_REPORT),
     structuredClone(VALID_USER_REPORT2),
-    structuredClone(LEGACY_USER_REPORT),
+    structuredClone(LEGACY_PER_LOCATION_USER_REPORT),
   ],
 };
 
+const LEGACY_REPORT: HourlyConnectionMetricsReport = {
+  serverId: 'legacy-id',
+  startUtcMs: 3,
+  endUtcMs: 4,
+  userReports: [structuredClone(LEGACY_PER_KEY_USER_REPORT)],
+};
+
 class FakeConnectionsTable implements InsertableTable<ConnectionRow> {
   public rows: ConnectionRow[] | undefined;
 
@@ -62,54 +77,46 @@ class FakeConnectionsTable implements InsertableTable<ConnectionRow> {
 }
 
 describe('postConnectionMetrics', () => {
-  it('correctly inserts feature metrics rows', async () => {
+  it('correctly inserts connection metrics rows', async () => {
     const table = new FakeConnectionsTable();
-    const userReports = [
-      {
-        countries: ['UK'],
-        bytesTransferred: 123,
-        tunnelTimeSec: 987,
-      },
-      {
-        countries: ['EC'],
-        bytesTransferred: 456,
-        tunnelTimeSec: 654,
-      },
-      {
-        countries: ['BR'],
-        bytesTransferred: 789,
-      },
-    ];
-    const report = {serverId: 'id', startUtcMs: 1, endUtcMs: 2, userReports};
-    await postConnectionMetrics(table, report);
+
+    await postConnectionMetrics(table, VALID_REPORT);
+
     const rows: ConnectionRow[] = [
       {
-        serverId: report.serverId,
-        startTimestamp: new Date(report.startUtcMs).toISOString(),
-        endTimestamp: new Date(report.endUtcMs).toISOString(),
-        bytesTransferred: userReports[0].bytesTransferred,
-        tunnelTimeSec: userReports[0].tunnelTimeSec,
-        countries: userReports[0].countries,
+        serverId: VALID_REPORT.serverId,
+        startTimestamp: new Date(VALID_REPORT.startUtcMs).toISOString(),
+        endTimestamp: new Date(VALID_REPORT.endUtcMs).toISOString(),
+        bytesTransferred: VALID_USER_REPORT.bytesTransferred,
+        tunnelTimeSec: VALID_USER_REPORT.tunnelTimeSec,
+        countries: VALID_USER_REPORT.countries,
       },
       {
-        serverId: report.serverId,
-        startTimestamp: new Date(report.startUtcMs).toISOString(),
-        endTimestamp: new Date(report.endUtcMs).toISOString(),
-        bytesTransferred: userReports[1].bytesTransferred,
-        tunnelTimeSec: userReports[1].tunnelTimeSec,
-        countries: userReports[1].countries,
+        serverId: VALID_REPORT.serverId,
+        startTimestamp: new Date(VALID_REPORT.startUtcMs).toISOString(),
+        endTimestamp: new Date(VALID_REPORT.endUtcMs).toISOString(),
+        bytesTransferred: VALID_USER_REPORT2.bytesTransferred,
+        tunnelTimeSec: VALID_USER_REPORT2.tunnelTimeSec,
+        countries: VALID_USER_REPORT2.countries,
       },
       {
-        serverId: report.serverId,
-        startTimestamp: new Date(report.startUtcMs).toISOString(),
-        endTimestamp: new Date(report.endUtcMs).toISOString(),
-        bytesTransferred: userReports[2].bytesTransferred,
-        tunnelTimeSec: undefined,
-        countries: userReports[2].countries,
+        serverId: VALID_REPORT.serverId,
+        startTimestamp: new Date(VALID_REPORT.startUtcMs).toISOString(),
+        endTimestamp: new Date(VALID_REPORT.endUtcMs).toISOString(),
+        bytesTransferred: LEGACY_PER_LOCATION_USER_REPORT.bytesTransferred,
+        tunnelTimeSec: LEGACY_PER_LOCATION_USER_REPORT.tunnelTimeSec,
+        countries: LEGACY_PER_LOCATION_USER_REPORT.countries,
       },
     ];
     expect(table.rows).toEqual(rows);
   });
+  it('correctly drops legacy connection metrics', async () => {
+    const table = new FakeConnectionsTable();
+
+    await postConnectionMetrics(table, LEGACY_REPORT);
+
+    expect(table.rows).toEqual([]);
+  });
 });
 
 describe('isValidConnectionMetricsReport', () => {
@@ -117,6 +124,10 @@ describe('isValidConnectionMetricsReport', () => {
     const report = structuredClone(VALID_REPORT);
     expect(isValidConnectionMetricsReport(report)).toBeTrue();
   });
+  it('returns true for legacy report', () => {
+    const report = structuredClone(LEGACY_REPORT);
+    expect(isValidConnectionMetricsReport(report)).toBeTrue();
+  });
   it('returns false for missing report', () => {
     expect(isValidConnectionMetricsReport(undefined)).toBeFalse();
   });
@@ -166,14 +177,6 @@ describe('isValidConnectionMetricsReport', () => {
     delete report['endUtcMs'];
     expect(isValidConnectionMetricsReport(report)).toBeFalse();
   });
-  it('returns false for missing user report field `countries`', () => {
-    const report = structuredClone(VALID_REPORT);
-    const userReport: Partial<HourlyUserConnectionMetricsReport> =
-      structuredClone(VALID_USER_REPORT);
-    delete userReport['countries'];
-    report.userReports[0] = userReport as HourlyUserConnectionMetricsReport;
-    expect(isValidConnectionMetricsReport(report)).toBeFalse();
-  });
   it('returns false for missing user report field `bytesTransferred`', () => {
     const report = structuredClone(VALID_REPORT);
     const userReport: Partial<HourlyUserConnectionMetricsReport> =
@@ -202,7 +205,17 @@ describe('isValidConnectionMetricsReport', () => {
     report.endUtcMs = '100' as unknown as number;
     expect(isValidConnectionMetricsReport(report)).toBeFalse();
   });
-  it('returns false for `countries` field type that is not a string', () => {
+  it('returns false for `userId` field type that is not a string', () => {
+    const report = structuredClone(VALID_REPORT);
+    report.userReports[0].userId = 1234 as unknown as string;
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
+  });
+  it('returns false for `countries` field type that is not an array', () => {
+    const report = structuredClone(VALID_REPORT);
+    report.userReports[0].countries = 'US' as unknown as string[];
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
+  });
+  it('returns false for `countries` arry items that are not strings', () => {
     const report = structuredClone(VALID_REPORT);
     report.userReports[0].countries = [1, 2, 3] as unknown as string[];
     expect(isValidConnectionMetricsReport(report)).toBeFalse();

src/metrics_server/connection_metrics.ts

@@ -14,7 +14,13 @@
 
 import {Table} from '@google-cloud/bigquery';
 import {InsertableTable} from './infrastructure/table';
-import {HourlyConnectionMetricsReport} from './model';
+import {
+  HourlyConnectionMetricsReport,
+  HourlyUserConnectionMetricsReport,
+  HourlyUserConnectionMetricsReportByLocation,
+} from './model';
+
+const TERABYTE = Math.pow(2, 40);
 
 export interface ConnectionRow {
   serverId: string;
@@ -45,18 +51,29 @@ function getConnectionRowsFromReport(report: HourlyConnectionMetricsReport): Con
   const endTimestampStr = new Date(report.endUtcMs).toISOString();
   const rows = [];
   for (const userReport of report.userReports) {
-    rows.push({
-      serverId: report.serverId,
-      startTimestamp: startTimestampStr,
-      endTimestamp: endTimestampStr,
-      bytesTransferred: userReport.bytesTransferred,
-      tunnelTimeSec: userReport.tunnelTimeSec || undefined,
-      countries: userReport.countries || [],
-    });
+    // User reports come in 2 flavors: "per location" and "per key". We no longer store the
+    // "per key" reports.
+    if (isPerLocationUserReport(userReport)) {
+      rows.push({
+        serverId: report.serverId,
+        startTimestamp: startTimestampStr,
+        endTimestamp: endTimestampStr,
+        bytesTransferred: userReport.bytesTransferred,
+        tunnelTimeSec: userReport.tunnelTimeSec || undefined,
+        countries: userReport.countries,
+      });
+    }
   }
   return rows;
 }
 
+function isPerLocationUserReport(
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  userReport: HourlyUserConnectionMetricsReport
+): userReport is HourlyUserConnectionMetricsReportByLocation {
+  return 'countries' in userReport;
+}
+
 // Returns true iff testObject contains a valid HourlyConnectionMetricsReport.
 export function isValidConnectionMetricsReport(
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -93,14 +110,21 @@ export function isValidConnectionMetricsReport(
     return false;
   }
 
-  const MIN_BYTES_TRANSFERRED = 0;
-  const MAX_BYTES_TRANSFERRED = 1 * Math.pow(2, 40); // 1 TB.
   for (const userReport of testObject.userReports) {
-    // Check that `bytesTransferred` is a number between min and max transfer limits
+    // Check that `userId` is a string.
+    if (userReport.userId && typeof userReport.userId !== 'string') {
+      return false;
+    }
+
+    // We used to set a limit of 1TB per access key, then per location. We later
+    // realized that a server may use a single key, or all the traffic may come
+    // from a single location.
+    // However, as we report hourly, it's unlikely we hit 1TB, so we keep the
+    // check for now to try and prevent malicious reports.
     if (
       typeof userReport.bytesTransferred !== 'number' ||
-      userReport.bytesTransferred < MIN_BYTES_TRANSFERRED ||
-      userReport.bytesTransferred > MAX_BYTES_TRANSFERRED
+      userReport.bytesTransferred < 0 ||
+      userReport.bytesTransferred > TERABYTE
     ) {
       return false;
     }
@@ -112,16 +136,16 @@ export function isValidConnectionMetricsReport(
       return false;
     }
 
-    // We require at least 1 country to be set
-    const countries = userReport.countries ?? [];
-    if (countries.length === 0) {
-      return false;
-    }
-    // Check that all `countries` are strings.
-    for (const country of countries) {
-      if (typeof country !== 'string') {
+    // Check that `countries` is an array of strings.
+    if (userReport.countries) {
+      if (!Array.isArray(userReport.countries)) {
         return false;
       }
+      for (const country of userReport.countries) {
+        if (typeof country !== 'string') {
+          return false;
+        }
+      }
     }
   }
 

src/metrics_server/model.ts

@@ -22,11 +22,17 @@ export interface HourlyConnectionMetricsReport {
 }
 
 export interface HourlyUserConnectionMetricsReport {
-  countries: string[];
+  userId?: string;
+  countries?: string[];
   bytesTransferred: number;
   tunnelTimeSec?: number;
 }
 
+export interface HourlyUserConnectionMetricsReportByLocation
+  extends Omit<HourlyUserConnectionMetricsReport, 'countries'> {
+  countries: string[];
+}
+
 export interface DailyFeatureMetricsReport {
   serverId: string;
   serverVersion: string;

src/shadowbox/Taskfile.yml

@@ -9,7 +9,6 @@ tasks:
     vars:
       TARGET_OS: '{{.TARGET_OS | default "linux"}}'
       TARGET_ARCH: '{{.TARGET_ARCH | default "x86_64"}}'
-      GOOS: '{{get (dict "macos" "darwin") .TARGET_OS | default .TARGET_OS}}'
       GOARCH: '{{get (dict "x86_64" "amd64") .TARGET_ARCH | default .TARGET_ARCH}}'
       TARGET_DIR: '{{.TARGET_DIR | default (joinPath .OUTPUT_BASE .TARGET_OS .TARGET_ARCH)}}'
       NODE_DIR: '{{joinPath .TARGET_DIR "app"}}'
@@ -21,25 +20,19 @@ tasks:
       - cp '{{joinPath .TASKFILE_DIR "package.json"}}' '{{.TARGET_DIR}}'
       # Build Node.js app
       - npx webpack --config='{{joinPath .TASKFILE_DIR "webpack.config.js"}}' --output-path='{{.NODE_DIR}}' ${BUILD_ENV:+--mode="${BUILD_ENV}"}
-      # Copy third_party dependencies
-      - mkdir -p '{{.BIN_DIR}}'
-      - |
-        {
-          cd '{{joinPath .USER_WORKING_DIR "third_party" "prometheus"}}'
-          make 'bin/{{.TARGET_OS}}-{{.TARGET_ARCH}}/prometheus'
-          cp 'bin/{{.TARGET_OS}}-{{.TARGET_ARCH}}/prometheus' '{{.BIN_DIR}}/'
-        }
       # Copy Prometheus Console files.
       - cp -r '{{joinPath .TASKFILE_DIR "prometheus"}}' '{{.TARGET_DIR}}'
+      # Copy third_party dependencies
+      - task: ':third_party:prometheus:copy-{{.TARGET_OS}}-{{.GOARCH}}'
+        vars: {TARGET_DIR: '{{.BIN_DIR}}'}
       # Set CGO_ENABLED=0 to force static linkage. See https://mt165.co.uk/blog/static-link-go/.
-      - GOOS={{.GOOS}} GOARCH={{.GOARCH}} CGO_ENABLED=0 go build -ldflags='-s -w -X main.version=embedded' -o '{{.BIN_DIR}}/' github.com/Jigsaw-Code/outline-ss-server/cmd/outline-ss-server
+      - GOOS={{.TARGET_OS}} GOARCH={{.GOARCH}} CGO_ENABLED=0 go build -ldflags='-s -w -X main.version=embedded' -o '{{.BIN_DIR}}/' github.com/Jigsaw-Code/outline-ss-server/cmd/outline-ss-server
 
   start:
     desc: Run the Outline server locally
     deps: [{task: build, vars: {TARGET_OS: '{{.TARGET_OS}}', TARGET_ARCH: '{{.TARGET_ARCH}}'}}]
     vars:
-      UNAME_OS: {sh: 'uname -s'}
-      TARGET_OS: '{{get (dict "Linux" "linux" "Darwin" "macos") .UNAME_OS}}'
+      TARGET_OS: {sh: "uname -s | tr '[:upper:]' '[:lower:]'"}
       TARGET_ARCH: {sh: 'uname -m'}
       RUN_ID: '{{.RUN_ID | default (now | date "2006-01-02-150405")}}'
       RUN_DIR: '{{joinPath "/tmp/outline" .RUN_ID}}'

third_party/Taskfile.yml

@@ -0,0 +1,50 @@
+version: '3'
+
+requires:
+  vars: [OUTPUT_BASE]
+
+tasks:
+  prometheus:debug:
+    cmds:
+      - echo {{.GOOS}}
+
+  prometheus:download-*-*:
+    desc: Download and extract prometheus binary
+    vars:
+      VERSION: '2.37.1'
+      GOOS: '{{index .MATCH 0}}'
+      GOARCH: '{{index .MATCH 1}}'
+      TEMPFILE: {sh: mktemp}
+      SHA256: '{{printf "%v/%v" .GOOS .GOARCH | get
+          (dict
+            "linux/amd64" "753f66437597cf52ada98c2f459aa8c03745475c249c9f2b40ac7b3919131ba6"
+            "linux/arm64" "b59a66fb5c7ec5acf6bf426793528a5789a1478a0dad8c64edc2843caf31b1b8"
+            "darwin/amd64" "e03a43d98955ac3500f57353ea74b5df829074205f195ea6b3b88f55c4575c79"
+            "darwin/arm64" "eb8a174c82a0fb6c84e81d9a73214318fb4a605115ad61505d7883d02e5a6f52"           
+          )                   
+        }}'
+      TARGET_DIR: '{{joinPath .OUTPUT_BASE "prometheus" .GOOS .GOARCH}}'
+      TARGET: '{{joinPath .TARGET_DIR "prometheus"}}'
+    generates: ['{{.TARGET}}']
+    sources: ['Taskfile.yml']
+    preconditions:
+      - {sh: "[[ '{{.GOOS}}' =~ 'linux|darwin' ]]", msg: "invalid GOOS {{.GOOS}}"}
+      - {sh: "[[ '{{.GOARCH}}' =~ 'amd64|arm64' ]]", msg: "invalid GOARCH {{.GOARCH}}"}
+    cmds:
+      - node '{{joinPath .ROOT_DIR "src/build/download_file.mjs"}}' --url='https://github.com/prometheus/prometheus/releases/download/v{{.VERSION}}/prometheus-{{.VERSION}}.{{.GOOS}}-{{.GOARCH}}.tar.gz' --out='{{.TEMPFILE}}' --sha256='{{.SHA256}}'
+      - defer: rm -f '{{.TEMPFILE}}'
+      - mkdir -p '{{.TARGET_DIR}}'
+      - tar -zx -f '{{.TEMPFILE}}' --strip-components=1 -C '{{.TARGET_DIR}}' 'prometheus-{{.VERSION}}.{{.GOOS}}-{{.GOARCH}}/prometheus'
+      - chmod +x '{{.TARGET}}'
+
+  prometheus:copy-*-*:
+    desc: Copy prometheus binary to target directory
+    requires:
+      vars: [TARGET_DIR]
+    vars:
+      GOOS: '{{index .MATCH 0}}'
+      GOARCH: '{{index .MATCH 1}}'
+    cmds:
+      - task: prometheus:download-{{.GOOS}}-{{.GOARCH}}
+      - mkdir -p '{{.TARGET_DIR}}'
+      - cp -R '{{joinPath .OUTPUT_BASE "prometheus" .GOOS .GOARCH}}'/* '{{.TARGET_DIR}}/'

third_party/prometheus/Makefile

@@ -1,32 +0,0 @@
-VERSION=2.37.1
-
-.PHONY: all
-all: bin/linux-x86_64/prometheus bin/linux-arm64/prometheus bin/macos-x86_64/prometheus bin/macos-arm64/prometheus
-
-bin/linux-x86_64/prometheus: OS=linux
-bin/linux-x86_64/prometheus: GOARCH=amd64
-bin/linux-x86_64/prometheus: SHA256=753f66437597cf52ada98c2f459aa8c03745475c249c9f2b40ac7b3919131ba6
-
-bin/linux-arm64/prometheus: OS=linux
-bin/linux-arm64/prometheus: GOARCH=arm64
-bin/linux-arm64/prometheus: SHA256=b59a66fb5c7ec5acf6bf426793528a5789a1478a0dad8c64edc2843caf31b1b8
-
-bin/macos-x86_64/prometheus: OS=darwin
-bin/macos-x86_64/prometheus: GOARCH=amd64
-bin/macos-x86_64/prometheus: SHA256=e03a43d98955ac3500f57353ea74b5df829074205f195ea6b3b88f55c4575c79
-
-bin/macos-arm64/prometheus: OS=darwin
-bin/macos-arm64/prometheus: GOARCH=arm64
-bin/macos-arm64/prometheus: SHA256=eb8a174c82a0fb6c84e81d9a73214318fb4a605115ad61505d7883d02e5a6f52
-
-bin/%/prometheus: TEMPFILE := $(shell mktemp)
-bin/%/prometheus:
-	node ../../src/build/download_file.mjs --url="https://github.com/prometheus/prometheus/releases/download/v$(VERSION)/prometheus-$(VERSION).$(OS)-$(GOARCH).tar.gz" --out="$(TEMPFILE)" --sha256=$(SHA256)
-	mkdir -p "$(dir $@)"
-	tar -zx -f "$(TEMPFILE)" --strip-components=1 -C "$(dir $@)" prometheus-$(VERSION).$(OS)-$(GOARCH)/prometheus
-	chmod +x "$@"
-	rm -f $(TEMPFILE)
-
-.PHONY: clean
-clean:
-	rm -rf bin