Fil/onramp review (#1805)

* documentation

* reformat and type

* simpler

* clarify the logic in validateGitHubLink
fixes an uncaught Error ('Setting source repository for continuous deployment failed' was never reached)

Author: Fil

docs/deploying.md

@@ -38,19 +38,19 @@ npm run deploy -- --help
 
 ## Continuous deployment
 
-<!-- TODO: decide whether to primarily use “continuous deployment” or “automatic deploys” -->
+### Cloud builds
 
-You can connect your app to Observable to handle deploys automatically. You can automate deploys both [on commit](https://observablehq.com/documentation/data-apps/github) (whenever you push a new commit to your project’s default branch) and [on schedule](https://observablehq.com/documentation/data-apps/schedules) (such as daily or weekly).
+Connect your app to Observable to handle deploys automatically. You can automate deploys both [on commit](https://observablehq.com/documentation/data-apps/github) (whenever you push a new commit to your project’s default branch) and [on schedule](https://observablehq.com/documentation/data-apps/schedules) (such as daily or weekly).
 
-Automatic deploys — also called _continuous deployment_ or _CD_ — ensure that your data is always up to date, and that any changes you make to your app are immediately reflected in the deployed version.
+Continuous deployment (for short, _CD_) ensures that your data is always up to date, and that any changes you make to your app are immediately reflected in the deployed version.
 
 On your app settings page on Observable, open the **Build settings** tab to set up a link to a GitHub repository hosting your project’s files. Observable will then listen for changes in the repo and deploy the app automatically.
 
-The settings page also allows you to trigger a manual deploy on Observable Cloud, add secrets (for data loaders to use private APIs and passwords), view logs, configure sharing, _etc._ For details, see the [Building & deploying](https://observablehq.com/documentation/data-apps/deploys) documentation.
+The settings page also allows you to trigger a manual deploy, add secrets for data loaders to use private APIs and passwords, view logs, configure sharing, _etc._ For details, see the [Building & deploying](https://observablehq.com/documentation/data-apps/deploys) documentation.
 
-## GitHub Actions
+### GitHub Actions
 
-As an alternative to building on Observable Cloud, you can use [GitHub Actions](https://github.com/features/actions) and have GitHub build a new version of your app and deploy it to Observable. In your git repository, create and commit a file at `.github/workflows/deploy.yml`. Here is a starting example:
+Alternatively, you can use [GitHub Actions](https://github.com/features/actions) to have GitHub build a new version of your app and deploy it to Observable. In your git repository, create and commit a file at `.github/workflows/deploy.yml`. Here is a starting example:
 
 ```yaml
 name: Deploy
@@ -139,6 +139,8 @@ This uses one cache per calendar day (in the `America/Los_Angeles` time zone). I
 
 <div class="note">You’ll need to edit the paths above if you’ve configured a source root other than <code>src</code>.</div>
 
+<div class="tip">Caching is limited for now to manual builds and GitHub Actions. In the future, it will be available as a configuration option for Observable Cloud builds.</div>
+
 ## Deploy configuration
 
 The deploy command creates a file at <code>.observablehq/deploy.json</code> under the source root (typically <code>src</code>) with information on where to deploy the app. This file allows you to re-deploy an app without having to repeat where you want the app to live on Observable.

src/deploy.ts

@@ -39,41 +39,21 @@ const BUILD_AGE_WARNING_MS = 1000 * 60 * 5;
 const OBSERVABLE_UI_ORIGIN = getObservableUiOrigin();
 
 function settingsUrl(deployTarget: DeployTargetInfo) {
-  if (deployTarget.create) {
-    throw new Error("Incorrect deploy target state");
-  }
+  if (deployTarget.create) throw new Error("Incorrect deploy target state");
   return `${OBSERVABLE_UI_ORIGIN}projects/@${deployTarget.workspace.login}/${deployTarget.project.slug}`;
 }
 
 /**
  * Returns the ownerName and repoName of the first GitHub remote (HTTPS or SSH)
- * on the current repository, or null.
+ * on the current repository. Supports both https and ssh URLs:
+ * - https://github.com/observablehq/framework.git
+ * - [email protected]:observablehq/framework.git
  */
-async function getGitHubRemote() {
-  const remotes = (await promisify(exec)("git remote -v")).stdout
-    .split("\n")
-    .filter((d) => d)
-    .map((d) => {
-      const [, url] = d.split(/\s/g);
-      if (url.startsWith("https://github.com/")) {
-        // HTTPS: https://github.com/observablehq/framework.git
-        const [ownerName, repoName] = new URL(url).pathname
-          .slice(1)
-          .replace(/\.git$/, "")
-          .split("/");
-        return {ownerName, repoName};
-      } else if (url.startsWith("[email protected]:")) {
-        // SSH: git@github.com:observablehq/framework.git
-        const [ownerName, repoName] = url
-          .replace(/^git@github.com:/, "")
-          .replace(/\.git$/, "")
-          .split("/");
-        return {ownerName, repoName};
-      }
-    });
-  const remote = remotes.find((d) => d && d.ownerName && d.repoName);
-  if (!remote) throw new CliError("No GitHub remote found.");
-  return remote ?? null;
+async function getGitHubRemote(): Promise<{ownerName: string; repoName: string} | undefined> {
+  const firstRemote = (await promisify(exec)("git remote -v")).stdout.match(
+    /^\S+\s(https:\/\/github.com\/|git@github.com:)(?<ownerName>[^/]+)\/(?<repoName>[^/]*?)(\.git)?\s/m
+  );
+  return firstRemote?.groups as {ownerName: string; repoName: string} | undefined;
 }
 
 export interface DeployOptions {
@@ -223,20 +203,13 @@ class Deployer {
     const {deployId} = this.deployOptions;
     if (!deployId) throw new Error("invalid deploy options");
     await this.checkDeployCreated(deployId);
-
-    const buildFilePaths = await this.getBuildFilePaths();
-
-    await this.uploadFiles(deployId, buildFilePaths);
+    await this.uploadFiles(deployId, await this.getBuildFilePaths());
     await this.markDeployUploaded(deployId);
-    const deployInfo = await this.pollForProcessingCompletion(deployId);
-
-    return deployInfo;
+    return await this.pollForProcessingCompletion(deployId);
   }
 
   private async cloudBuild(deployTarget: DeployTargetInfo) {
-    if (deployTarget.create) {
-      throw new Error("Incorrect deploy target state");
-    }
+    if (deployTarget.create) throw new Error("Incorrect deploy target state");
     const {deployPollInterval: pollInterval = DEPLOY_POLL_INTERVAL_MS} = this.deployOptions;
     await this.apiClient.postProjectBuild(deployTarget.project.id);
     const spinner = this.effects.clack.spinner();
@@ -264,51 +237,46 @@ class Deployer {
     }
   }
 
-  // Throws error if local and remote GitHub repos don’t match or are invalid
+  // Throws error if local and remote GitHub repos don’t match or are invalid.
+  // Ignores this.deployOptions.config.root as we only support cloud builds from
+  // the root directory.
   private async validateGitHubLink(deployTarget: DeployTargetInfo): Promise<void> {
-    if (deployTarget.create) {
-      throw new Error("Incorrect deploy target state");
-    }
-    if (!deployTarget.project.build_environment_id) {
-      // TODO: allow setting build environment from CLI
-      throw new CliError("No build environment configured.");
-    }
-    // We only support cloud builds from the root directory so this ignores
-    // this.deployOptions.config.root
-    const isGit = existsSync(".git");
-    if (!isGit) throw new CliError("Not at root of a git repository.");
-
-    const {ownerName, repoName} = await getGitHubRemote();
+    if (deployTarget.create) throw new Error("Incorrect deploy target state");
+    if (!deployTarget.project.build_environment_id) throw new CliError("No build environment configured.");
+    if (!existsSync(".git")) throw new CliError("Not at root of a git repository.");
+    const remote = await getGitHubRemote();
+    if (!remote) throw new CliError("No GitHub remote found.");
     const branch = (await promisify(exec)("git rev-parse --abbrev-ref HEAD")).stdout.trim();
-    let localRepo = await this.apiClient.getGitHubRepository({ownerName, repoName});
+    if (!branch) throw new Error("Branch not found.");
 
     // If a source repository has already been configured, check that it’s
-    // accessible and matches the local repository and branch.
-    // TODO: validate local/remote refs match, "Your branch is up to date",
-    // and "nothing to commit, working tree clean".
+    // accessible and matches the linked repository and branch. TODO: validate
+    // local/remote refs match, "Your branch is up to date", and "nothing to
+    // commit, working tree clean".
     const {source} = deployTarget.project;
     if (source) {
-      if (localRepo && source.provider_id !== localRepo.provider_id) {
-        throw new CliError(
-          `Configured repository does not match local repository; check build settings on ${link(
-            `${settingsUrl(deployTarget)}/settings`
-          )}`
-        );
-      }
-      if (localRepo && source.branch && source.branch !== branch) {
-        // TODO: If source.branch is empty, it'll use the default repository
-        // branch (usually main or master), which we don't know from our current
-        // getGitHubRepository response, and thus can't check here.
-        throw new CliError(
-          `Configured branch does not match local branch; check build settings on ${link(
-            `${settingsUrl(deployTarget)}/settings`
-          )}`
-        );
+      const linkedRepo = await this.apiClient.getGitHubRepository(remote);
+      if (linkedRepo) {
+        if (source.provider_id !== linkedRepo.provider_id) {
+          throw new CliError(
+            `Configured repository does not match local repository; check build settings on ${link(
+              `${settingsUrl(deployTarget)}/settings`
+            )}`
+          );
+        }
+        if (source.branch !== branch) {
+          // TODO: If source.branch is empty, it'll use the default repository
+          // branch (usually main or master), which we don't know from our current
+          // getGitHubRepository response, and thus can't check here.
+          throw new CliError(
+            `Configured branch ${source.branch} does not match local branch ${branch}; check build settings on ${link(
+              `${settingsUrl(deployTarget)}/settings`
+            )}`
+          );
+        }
       }
-      const remoteAuthedRepo = await this.apiClient.getGitHubRepository({
-        providerId: source.provider_id
-      });
-      if (!remoteAuthedRepo) {
+
+      if (!(await this.apiClient.getGitHubRepository({providerId: source.provider_id}))) {
         // TODO: This could poll for auth too, but is a distinct case because it
         // means the repo was linked at one point and then something went wrong
         throw new CliError(
@@ -322,51 +290,49 @@ class Deployer {
       return;
     }
 
-    if (!localRepo) {
+    // If the source has not been configured, first check that the remote repo
+    // is linked in CD settings. If not, prompt the user to auth & link.
+    let linkedRepo = await this.apiClient.getGitHubRepository(remote);
+    if (!linkedRepo) {
       if (!this.effects.isTty)
         throw new CliError(
           "Cannot access repository for continuous deployment and cannot request access in non-interactive mode"
         );
 
       // Repo is not authorized; link to auth page and poll for auth
       const authUrl = new URL("/auth-github", OBSERVABLE_UI_ORIGIN);
-      authUrl.searchParams.set("owner", ownerName);
-      authUrl.searchParams.set("repo", repoName);
-      this.effects.clack.log.info(`Authorize Observable to access the ${bold(repoName)} repository: ${link(authUrl)}`);
+      authUrl.searchParams.set("owner", remote.ownerName);
+      authUrl.searchParams.set("repo", remote.repoName);
+      this.effects.clack.log.info(
+        `Authorize Observable to access the ${bold(remote.repoName)} repository: ${link(authUrl)}`
+      );
 
       const spinner = this.effects.clack.spinner();
-      spinner.start("Waiting for repository to be authorized");
+      spinner.start("Waiting for authorization");
       const {deployPollInterval: pollInterval = DEPLOY_POLL_INTERVAL_MS} = this.deployOptions;
       const pollExpiration = Date.now() + DEPLOY_POLL_MAX_MS;
-      while (!localRepo) {
+      do {
         await new Promise((resolve) => setTimeout(resolve, pollInterval));
         if (Date.now() > pollExpiration) {
-          spinner.stop("Waiting for repository to be authorized timed out.");
+          spinner.stop("Authorization timed out.");
           throw new CliError("Repository authorization failed");
         }
-        localRepo = await this.apiClient.getGitHubRepository({ownerName, repoName});
-        if (localRepo) spinner.stop("Repository authorized.");
-      }
+      } while (!(linkedRepo = await this.apiClient.getGitHubRepository(remote)));
+      spinner.stop("Repository authorized.");
     }
 
-    const response = await this.apiClient.postProjectEnvironment(deployTarget.project.id, {
-      source: {
-        provider: localRepo.provider,
-        provider_id: localRepo.provider_id,
-        url: localRepo.url,
-        branch
-      }
-    });
-
-    if (!response) throw new CliError("Setting source repository for continuous deployment failed");
-
-    // Configured repo is OK; proceed
-    return;
+    // Save the linked repo as the configured source.
+    const {provider, provider_id, url} = linkedRepo;
+    await this.apiClient
+      .postProjectEnvironment(deployTarget.project.id, {source: {provider, provider_id, url, branch}})
+      .catch((error) => {
+        throw new CliError("Setting source repository for continuous deployment failed", {cause: error});
+      });
   }
 
   private async startNewDeploy(): Promise<GetDeployResponse> {
     const {deployConfig, deployTarget} = await this.getDeployTarget(await this.getUpdatedDeployConfig());
-    let deployId: string | null;
+    let deployId: string;
     if (deployConfig.continuousDeployment) {
       await this.validateGitHubLink(deployTarget);
       deployId = await this.cloudBuild(deployTarget);
@@ -388,11 +354,7 @@ class Deployer {
       }
       return deployInfo;
     } catch (error) {
-      if (isHttpError(error)) {
-        throw new CliError(`Deploy ${deployId} not found.`, {
-          cause: error
-        });
-      }
+      if (isHttpError(error)) throw new CliError(`Deploy ${deployId} not found.`, {cause: error});
       throw error;
     }
   }
@@ -580,7 +542,7 @@ class Deployer {
       continuousDeployment = enable;
     }
 
-    const newDeployConfig = {
+    deployConfig = {
       projectId: deployTarget.project.id,
       projectSlug: deployTarget.project.slug,
       workspaceLogin: deployTarget.workspace.login,
@@ -590,11 +552,11 @@ class Deployer {
     await this.effects.setDeployConfig(
       this.deployOptions.config.root,
       this.deployOptions.deployConfigPath,
-      newDeployConfig,
+      deployConfig,
       this.effects
     );
 
-    return {deployConfig: newDeployConfig, deployTarget};
+    return {deployConfig, deployTarget};
   }
 
   // Create the new deploy on the server.

src/observableApiClient.ts

@@ -130,22 +130,20 @@ export class ObservableApiClient {
   async getGitHubRepository(
     props: {ownerName: string; repoName: string} | {providerId: string}
   ): Promise<GetGitHubRepositoryResponse | null> {
-    let url: URL;
-    if ("providerId" in props) {
-      url = new URL(`/cli/github/repository?provider_id=${props.providerId}`, this._apiOrigin);
-    } else {
-      url = new URL(`/cli/github/repository?owner=${props.ownerName}&repo=${props.repoName}`, this._apiOrigin);
-    }
-    try {
-      return await this._fetch<GetGitHubRepositoryResponse>(url, {method: "GET"});
-    } catch (err) {
-      // TODO: err.details.errors may be [{code: "NO_GITHUB_TOKEN"}] or [{code: "NO_REPO_ACCESS"}],
-      // which could be handled separately
-      return null;
-    }
+    const params =
+      "providerId" in props ? `provider_id=${props.providerId}` : `owner=${props.ownerName}&repo=${props.repoName}`;
+    return await this._fetch<GetGitHubRepositoryResponse>(
+      new URL(`/cli/github/repository?${params}`, this._apiOrigin),
+      {method: "GET"}
+    ).catch(() => null);
+    // TODO: err.details.errors may be [{code: "NO_GITHUB_TOKEN"}] or [{code: "NO_REPO_ACCESS"}],
+    // which could be handled separately
   }
 
-  async postProjectEnvironment(id, body): Promise<PostProjectEnvironmentResponse> {
+  async postProjectEnvironment(
+    id: string,
+    body: {source: {provider: "github"; provider_id: string; url: string; branch: string}}
+  ): Promise<PostProjectEnvironmentResponse> {
     const url = new URL(`/cli/project/${id}/environment`, this._apiOrigin);
     return await this._fetch<PostProjectEnvironmentResponse>(url, {
       method: "POST",
@@ -155,9 +153,7 @@ export class ObservableApiClient {
   }
 
   async postProjectBuild(id): Promise<{id: string}> {
-    return await this._fetch<{id: string}>(new URL(`/cli/project/${id}/build`, this._apiOrigin), {
-      method: "POST"
-    });
+    return await this._fetch<{id: string}>(new URL(`/cli/project/${id}/build`, this._apiOrigin), {method: "POST"});
   }
 
   async postProject({