Repository: elastic/synthetics (ts)

Issue:
Error when pushing monitors from the CI environment
+ Currently we rely on `project.json` internally during the initialization phase to be able to reuse the configuration when pushing monitors later once the synthetics project is set up. This works good on local dev machines, But wont work on the CI as we need the configurations like `url, project and space` to be able to push to Kibana.

Push should be idempotent when running from Dev machine and also on CI. 

Pull request:
fix: save project settings in synthetics config
+ fix #590 
+ Changed the way how the Synthetics agent works with project monitor settings and how we can keep the settings idempotent when users run push on their local dev machine and also on the CI.  Sparked out of discussion here - https://elastic.slack.com/archives/C022SU2U6DC/p1662556348346669
+ As a result, Introduced the project setting in the synthetics.config file instead of having yet another project.json file
There was other way, we could have solved this using the same project.json  and constructing them on demand and still be able to provide the same idempotentcy. But having them on the config file, has tons of advantages right now and also in the future.
  - Users can immediately see how the values like id, space and url  values are filled in when they initialize the agent, instead of hidden under .synthetics folder.
  - Helps with code reviews, when one user change a given value, others can validate it before getting pushed from CI
   - This is the best one so far, it will allow us to support multiple projects in the long term with few lines of code and can be explicit about it.
```
    projects: [
      { id: 'uptime', tags: ['synthetics:ui'],space: 'synthetics',url: 'uptime-synthetics.kibana.com'},
      {id: 'apm',tags: ['apm:ui'],space: 'apm',url: 'apm-synthetics.kibana.com'},
    ]
```

### Testing
- Build the local project using - npm run build

#### Existing Synthetics project
- We used `project.json` before where configuration were stored, Its removed from this PR, so you should see the CLI throwing error if you try to push from the repo.
- Perform init again to reinitialize the project OR follow along the errors and set up required values in the config file.
- Go to any existing scaffolded synthetics project using prior versions of the agent and try pushing to experience different errors. For this to work properly. You would need to link to the current synthetics version. You can do so by changing package.json to @elastic/synthetics: file:/path/to/locally/built/synthetics

#### Brand new synthetics project
- Setup a brand new Synthetics project - node dist/cli.js init [optional-dir]
- Follow along and check if project settings have been initialized properly
- Validate by pushing with auth specified `npm run push -- --auth <key>`. 



Patch:
diff --git a/src/cli.ts b/src/cli.ts
index 42d927f..c3cb998 100644
--- a/src/cli.ts
+++ b/src/cli.ts
@@ -177,8 +177,8 @@ program
   )
   .option('--url <url>', 'Kibana URL to upload the monitors')
   .option(
-    '--project <id>',
-    'id that will be used for logically grouping monitors'
+    '--id <id>',
+    'project id that will be used for logically grouping monitors'
   )
   .option(
     '--space <space>',
@@ -191,8 +191,8 @@ program
   .addOption(playwrightOpts)
   .action(async (cmdOpts: PushOptions) => {
     try {
-      const settings = await loadSettings();
       await loadTestFiles({ inline: false }, [cwd()]);
+      const settings = await loadSettings();
       const options = normalizeOptions({
         ...program.opts(),
         ...settings,
diff --git a/src/common_types.ts b/src/common_types.ts
index b4ad07c..876f3fa 100644
--- a/src/common_types.ts
+++ b/src/common_types.ts
@@ -241,21 +241,25 @@ export type RunOptions = BaseArgs & {
   reporter?: BuiltInReporterName | ReporterInstance;
 };
 
-export type PushOptions = {
+export type PushOptions = ProjectSettings & {
   auth: string;
-  url: string;
-  project: string;
-  space: string;
   schedule?: MonitorConfig['schedule'];
   locations?: MonitorConfig['locations'];
   privateLocations?: MonitorConfig['privateLocations'];
 };
 
+export type ProjectSettings = {
+  id: string;
+  url: string;
+  space: string;
+};
+
 export type PlaywrightOptions = LaunchOptions & BrowserContextOptions;
 export type SyntheticsConfig = {
   params?: Params;
   playwrightOptions?: PlaywrightOptions;
   monitor?: MonitorConfig;
+  project?: ProjectSettings;
 };
 
 /** Runner Payload types */
diff --git a/src/config.ts b/src/config.ts
index 467962f..42128e9 100644
--- a/src/config.ts
+++ b/src/config.ts
@@ -80,7 +80,7 @@ function getConfigFile(ext: string) {
   return 'synthetics.config' + ext;
 }
 
-function findSyntheticsConfig(resolvePath, cwd) {
+export function findSyntheticsConfig(resolvePath, cwd) {
   const configPath = ['.js', '.ts']
     .map(ext => resolve(resolvePath, getConfigFile(ext)))
     .find(isFile);
diff --git a/src/generator/index.ts b/src/generator/index.ts
index c2fbd6d..a5a575c 100644
--- a/src/generator/index.ts
+++ b/src/generator/index.ts
@@ -37,16 +37,11 @@ import {
   cloudIDToKibanaURL,
 } from './utils';
 import { formatLocations, getLocations, groupLocations } from '../locations';
+import type { ProjectSettings } from '../common_types';
 
 // Templates that are required for setting up new synthetics project
 const templateDir = join(__dirname, '..', '..', 'templates');
 
-export type ProjectSettings = {
-  url: string;
-  space: string;
-  project: string;
-};
-
 type PromptOptions = ProjectSettings & {
   locations: Array<string>;
   privateLocations: Array<string>;
@@ -61,11 +56,10 @@ export const REGULAR_FILES_PATH = [
   '.github/workflows/run-synthetics.yml',
   'README.md',
 ];
-export const SETTINGS_PATH = '.synthetics/project.json';
 export const CONFIG_PATH = 'synthetics.config.ts';
 
 // Files to be overriden by default if the project is initialized multiple times
-const DEFAULT_OVERRIDES = [SETTINGS_PATH, CONFIG_PATH];
+const DEFAULT_OVERRIDES = [CONFIG_PATH];
 
 export class Generator {
   pkgManager = 'npm';
@@ -123,8 +117,7 @@ export class Generator {
     const allLocations = await getLocations({ url, auth });
     const locChoices = formatLocations(allLocations);
     if (locChoices.length === 0) {
-      // TODO - Add link to docs
-      throw 'Follow the docs to set up your first private locations - <link>';
+      throw 'Follow the docs to set up your first private locations - https://www.elastic.co/guide/en/observability/current/uptime-set-up-choose-agent.html#private-locations';
     }
 
     const monitorQues = [
@@ -147,7 +140,7 @@ export class Generator {
       },
       {
         type: 'input',
-        name: 'project',
+        name: 'id',
         message: 'Choose project id to logically group monitors',
         initial: basename(this.projectDir),
       },
@@ -168,14 +161,6 @@ export class Generator {
   async files(answers: PromptOptions) {
     const fileMap = new Map<string, string>();
 
-    // Setup project.json
-    const projectJSON = {
-      url: answers.url,
-      space: answers.space,
-      project: answers.project,
-    };
-    fileMap.set(SETTINGS_PATH, JSON.stringify(projectJSON, null, 2) + '\n');
-
     // Setup Synthetics config file
     fileMap.set(
       CONFIG_PATH,
diff --git a/src/generator/utils.ts b/src/generator/utils.ts
index 0fc7539..a37bfc0 100644
--- a/src/generator/utils.ts
+++ b/src/generator/utils.ts
@@ -52,6 +52,9 @@ export function replaceTemplates(input: string, values: Record<string, any>) {
       if (typeof finalValue == 'number') {
         return Number(finalValue);
       }
+      if (typeof finalValue == 'string') {
+        return `'${finalValue}'`;
+      }
       return finalValue;
     });
   }
diff --git a/src/push/index.ts b/src/push/index.ts
index 3eb9ce3..654eb67 100644
--- a/src/push/index.ts
+++ b/src/push/index.ts
@@ -23,8 +23,6 @@
  *
  */
 
-import { join } from 'path';
-import { existsSync } from 'fs';
 import { readFile, writeFile } from 'fs/promises';
 import { prompt } from 'enquirer';
 import { bold, yellow } from 'kleur/colors';
@@ -36,20 +34,19 @@ import {
   formatStaleMonitors,
 } from './request';
 import { buildMonitorSchema, createMonitors, MonitorSchema } from './monitor';
-import { ProjectSettings } from '../generator';
 import { Monitor } from '../dsl/monitor';
 import {
   progress,
   apiProgress,
   write,
   error,
-  SYNTHETICS_PATH,
   aborted,
   indent,
   safeNDJSONParse,
   done,
 } from '../helpers';
-import type { PushOptions } from '../common_types';
+import type { PushOptions, ProjectSettings } from '../common_types';
+import { findSyntheticsConfig, readConfig } from '../config';
 
 export async function push(monitors: Monitor[], options: PushOptions) {
   if (monitors.length === 0) {
@@ -148,34 +145,39 @@ export function formatDuplicateError(monitors: Set<Monitor>) {
   return outer;
 }
 
-const PROJECT_SETTINGS_PATH = join(SYNTHETICS_PATH, 'project.json');
 const INSTALLATION_HELP = `Run 'npx @elastic/synthetics init' to create project with default settings.`;
 
 export async function loadSettings() {
-  if (!existsSync(PROJECT_SETTINGS_PATH)) {
-    throw error(`Aborted. Synthetics project not set up.
+  try {
+    const config = await readConfig('asd');
+    // Missing config file, fake throw to capture as missing file
+    if (Object.keys(config).length === 0) {
+      throw '';
+    }
+    return config.project || ({} as ProjectSettings);
+  } catch (e) {
+    throw error(`Aborted (missing synthetics config file), Project not set up corrrectly.
 
 ${INSTALLATION_HELP}`);
   }
-
-  const contents = await readFile(PROJECT_SETTINGS_PATH, 'utf-8');
-  return JSON.parse(contents) as ProjectSettings;
 }
 
 export function validateSettings(opts: PushOptions) {
   const INVALID = 'Aborted. Invalid synthetics project settings.';
 
   let reason = '';
-  if (!opts.project) {
-    reason = 'Set project id via CLI as `--project <id>`';
+  if (!opts.id) {
+    reason = `Set project id via
+  - CLI '--id <id>'
+  - Config file 'project.id' field`;
   } else if (!opts.locations && !opts.privateLocations) {
     reason = `Set default location for all monitors via
-  - CLI as '--locations <values...> or --privateLocations <values...>'
-  - Synthetics config file 'monitors.locations' | 'monitors.privateLocations' field`;
+  - CLI '--locations <values...> or --privateLocations <values...>'
+  - Config file 'monitors.locations' | 'monitors.privateLocations' field`;
   } else if (!opts.schedule) {
     reason = `Set default schedule in minutes for all monitors via
-  - CLI as '--schedule <mins>'
-  - Synthetics config file 'monitors.schedule' field`;
+  - CLI '--schedule <mins>'
+  - Config file 'monitors.schedule' field`;
   }
 
   if (!reason) return;
@@ -187,24 +189,27 @@ ${reason}
 ${INSTALLATION_HELP}`);
 }
 
-async function overrideSettings(project: string) {
-  const settings = JSON.parse(await readFile(PROJECT_SETTINGS_PATH, 'utf-8'));
-  settings.project = project;
-
-  await writeFile(
-    PROJECT_SETTINGS_PATH,
-    JSON.stringify(settings, null, 2),
-    'utf-8'
+async function overrideSettings(oldValue: string, newValue: string) {
+  const cwd = process.cwd();
+  const configPath = await findSyntheticsConfig(cwd, cwd);
+  if (!configPath) {
+    throw aborted(`Unable to find synthetics config file: ${configPath}`);
+  }
+  const config = await readFile(configPath, 'utf-8');
+  const updatedConfig = config.replace(
+    `id: '${oldValue}'`,
+    `id: '${newValue}'`
   );
+  await writeFile(configPath, updatedConfig, 'utf-8');
 }
 
 export async function catchIncorrectSettings(
   settings: ProjectSettings,
   options: PushOptions
 ) {
-  let override = !settings.project;
-  if (settings.project && settings.project !== options.project) {
-    if (typeof process.env.TEST_OVERRIDE != null) {
+  let override = !settings.id;
+  if (settings.id && settings.id !== options.id) {
+    if (process.env.TEST_OVERRIDE != null) {
       override = Boolean(process.env.TEST_OVERRIDE);
     } else {
       // Add an extra line to make it easier to read the prompt
@@ -212,7 +217,7 @@ export async function catchIncorrectSettings(
       ({ override } = await prompt<{ override: boolean }>({
         type: 'confirm',
         name: 'override',
-        message: `Monitors were pushed under the '${settings.project}' project. Are you sure you want to push them under the new '${options.project}' (note that this will duplicate the monitors, the old ones being orphaned)`,
+        message: `Monitors were pushed under the '${settings.id}' project. Are you sure you want to push them under the new '${options.id}' (note that this will duplicate the monitors, the old ones being orphaned)`,
         initial: false,
       }));
     }
@@ -221,6 +226,6 @@ export async function catchIncorrectSettings(
     }
   }
   if (override) {
-    await overrideSettings(options.project);
+    await overrideSettings(settings.id, options.id);
   }
 }
diff --git a/src/push/monitor.ts b/src/push/monitor.ts
index a2232da..35a4120 100644
--- a/src/push/monitor.ts
+++ b/src/push/monitor.ts
@@ -81,7 +81,7 @@ export async function createMonitors(
   keepStale: boolean
 ) {
   const schema: APISchema = {
-    project: options.project,
+    project: options.id,
     keep_stale: keepStale,
     monitors,
   };
diff --git a/templates/synthetics.config.ts b/templates/synthetics.config.ts
index 37e3569..243fe79 100644
--- a/templates/synthetics.config.ts
+++ b/templates/synthetics.config.ts
@@ -16,6 +16,14 @@ export default env => {
       locations: ['{{locations}}'],
       privateLocations: ['{{privateLocations}}'],
     },
+    /**
+     * Project monitors settings
+     */
+    project: {
+      id: '{{id}}',
+      url: '{{url}}',
+      space: '{{space}}',
+    },
   };
   if (env !== 'development') {
     /**


Test patch:
diff --git a/__tests__/generator/index.test.ts b/__tests__/generator/index.test.ts
index 38eda88..698f53f 100644
--- a/__tests__/generator/index.test.ts
+++ b/__tests__/generator/index.test.ts
@@ -27,11 +27,7 @@ import { existsSync } from 'fs';
 import { readFile, rm } from 'fs/promises';
 import { join } from 'path';
 import { CLIMock } from '../utils/test-config';
-import {
-  REGULAR_FILES_PATH,
-  CONFIG_PATH,
-  SETTINGS_PATH,
-} from '../../src/generator';
+import { REGULAR_FILES_PATH, CONFIG_PATH } from '../../src/generator';
 
 describe('Generator', () => {
   const scaffoldDir = join(__dirname, 'scaffold-test');
@@ -57,6 +53,9 @@ describe('Generator', () => {
           locations: ['us_east'],
           privateLocations: ['custom'],
           schedule: 30,
+          id: 'test',
+          space: 'kbn',
+          url: 'foo:bar',
         }),
       },
     });
@@ -67,9 +66,6 @@ describe('Generator', () => {
       expect(output).toBe('');
     }
 
-    // Project setup file
-    expect(existsSync(join(scaffoldDir, SETTINGS_PATH))).toBeTruthy();
-
     // Verify files
     expect(existsSync(join(scaffoldDir, 'package.json'))).toBeTruthy();
     expect(existsSync(join(scaffoldDir, 'package-lock.json'))).toBeTruthy();
@@ -87,11 +83,14 @@ describe('Generator', () => {
       expect(existsSync(join(scaffoldDir, fn))).toBeTruthy();
     });
     expect(existsSync(join(scaffoldDir, CONFIG_PATH))).toBeTruthy();
-    // Verify schedule and locations
+    // Verify project and monitor settings
     const configFile = await readFile(join(scaffoldDir, CONFIG_PATH), 'utf-8');
     expect(configFile).toContain(`locations: ['us_east']`);
     expect(configFile).toContain(`privateLocations: ['custom']`);
     expect(configFile).toContain(`schedule: 30`);
+    expect(configFile).toContain(`id: 'test'`);
+    expect(configFile).toContain(`url: 'foo:bar'`);
+    expect(configFile).toContain(`space: 'kbn'`);
 
     // Verify stdout
     const stderr = cli.stderr();
diff --git a/__tests__/push/__snapshots__/index.test.ts.snap b/__tests__/push/__snapshots__/index.test.ts.snap
index 0092515..b753468 100644
--- a/__tests__/push/__snapshots__/index.test.ts.snap
+++ b/__tests__/push/__snapshots__/index.test.ts.snap
@@ -26,7 +26,9 @@ exports[`Push abort on push with different project id 1`] = `
 exports[`Push error on empty project id 1`] = `
 "Aborted. Invalid synthetics project settings.
 
-Set project id via CLI as \`--project <id>\`
+Set project id via
+  - CLI '--id <id>'
+  - Config file 'project.id' field
 
 Run 'npx @elastic/synthetics init' to create project with default settings.
 "
@@ -36,8 +38,8 @@ exports[`Push error on invalid location 1`] = `
 "Aborted. Invalid synthetics project settings.
 
 Set default location for all monitors via
-  - CLI as '--locations <values...> or --privateLocations <values...>'
-  - Synthetics config file 'monitors.locations' | 'monitors.privateLocations' field
+  - CLI '--locations <values...> or --privateLocations <values...>'
+  - Config file 'monitors.locations' | 'monitors.privateLocations' field
 
 Run 'npx @elastic/synthetics init' to create project with default settings.
 "
@@ -47,8 +49,8 @@ exports[`Push error on invalid schedule 1`] = `
 "Aborted. Invalid synthetics project settings.
 
 Set default schedule in minutes for all monitors via
-  - CLI as '--schedule <mins>'
-  - Synthetics config file 'monitors.schedule' field
+  - CLI '--schedule <mins>'
+  - Config file 'monitors.schedule' field
 
 Run 'npx @elastic/synthetics init' to create project with default settings.
 "
diff --git a/__tests__/push/index.test.ts b/__tests__/push/index.test.ts
index 8043879..5503d44 100644
--- a/__tests__/push/index.test.ts
+++ b/__tests__/push/index.test.ts
@@ -45,15 +45,11 @@ describe('Push', () => {
   }
 
   async function fakeProjectSetup(settings, monitor) {
-    await mkdir(join(PROJECT_DIR, '.synthetics'), { recursive: true });
-    await writeFile(
-      join(PROJECT_DIR, '.synthetics', 'project.json'),
-      JSON.stringify(settings, null, 2)
-    );
-
     await writeFile(
       join(PROJECT_DIR, 'synthetics.config.ts'),
-      `export default { monitor: ${JSON.stringify(monitor)} }`
+      `export default { monitor: ${JSON.stringify(
+        monitor
+      )}, project: ${JSON.stringify(settings)} }`
     );
   }
 
@@ -72,7 +68,9 @@ describe('Push', () => {
 
   it('error when project is not setup', async () => {
     const output = await runPush();
-    expect(output).toContain('Aborted. Synthetics project not set up');
+    expect(output).toContain(
+      'Aborted (missing synthetics config file), Project not set up corrrectly.'
+    );
   });
 
   it('error on empty project id', async () => {
@@ -82,51 +80,42 @@ describe('Push', () => {
   });
 
   it('error on invalid location', async () => {
-    await fakeProjectSetup({ project: 'test-project' }, {});
+    await fakeProjectSetup({ id: 'test-project' }, {});
     const output = await runPush();
     expect(output).toMatchSnapshot();
   });
 
   it('error on invalid schedule', async () => {
-    await fakeProjectSetup(
-      { project: 'test-project' },
-      { locations: ['test-loc'] }
-    );
+    await fakeProjectSetup({ id: 'test-project' }, { locations: ['test-loc'] });
     const output = await runPush();
     expect(output).toMatchSnapshot();
   });
 
   it('abort on push with different project id', async () => {
     await fakeProjectSetup(
-      { project: 'test-project' },
+      { id: 'test-project' },
       { locations: ['test-loc'], schedule: 2 }
     );
-    const output = await runPush(
-      [...DEFAULT_ARGS, '--project', 'new-project'],
-      {
-        TEST_OVERRIDE: '',
-      }
-    );
+    const output = await runPush([...DEFAULT_ARGS, '--id', 'new-project'], {
+      TEST_OVERRIDE: '',
+    });
     expect(output).toMatchSnapshot();
   });
 
   it('push with different id when overriden', async () => {
     await fakeProjectSetup(
-      { project: 'test-project' },
+      { id: 'test-project' },
       { locations: ['test-loc'], schedule: 2 }
     );
-    const output = await runPush(
-      [...DEFAULT_ARGS, '--project', 'new-project'],
-      {
-        TEST_OVERRIDE: 'true',
-      }
-    );
+    const output = await runPush([...DEFAULT_ARGS, '--id', 'new-project'], {
+      TEST_OVERRIDE: 'true',
+    });
     expect(output).toContain('No Monitors found');
   });
 
   it('errors on duplicate monitors', async () => {
     await fakeProjectSetup(
-      { project: 'test-project' },
+      { id: 'test-project' },
       { locations: ['test-loc'], schedule: 2 }
     );
 
@@ -187,7 +176,7 @@ journey('duplicate name', () => monitor.use({ schedule: 20 }));`
       );
       await fakeProjectSetup(
         {
-          project: 'test-project',
+          id: 'test-project',
           space: 'dummy',
         },
         { locations: ['test-loc'], schedule: 2 }
diff --git a/__tests__/push/monitor.test.ts b/__tests__/push/monitor.test.ts
index 086ff3b..4210bd1 100644
--- a/__tests__/push/monitor.test.ts
+++ b/__tests__/push/monitor.test.ts
@@ -74,7 +74,7 @@ describe('Monitors', () => {
       {
         url: `${server.PREFIX}`,
         auth: 'apiKey',
-        project: 'blah',
+        id: 'blah',
         space: 'dummy',
       },
       false