Merge pull request #4411 from backstage/orkohunter/url-reader-proper-caching-and-cleanup

2021-02-10 13:31:08 +01:00
parent 9d1426a04a 9e6e4b89a8
commit 73d60f6822
16 changed files with 316 additions and 188 deletions
@@ -0,0 +1,6 @@
+---
+'@backstage/techdocs-common': minor
+'@backstage/plugin-techdocs-backend': minor
+---
+
+URL Preparer will now use proper etag based caching introduced in https://github.com/backstage/backstage/pull/4120. Previously, builds used to be cached for 30 minutes.
@@ -0,0 +1,8 @@
+---
+'@backstage/techdocs-common': patch
+'@backstage/plugin-techdocs-backend': patch
+---
+
+TechDocs will throw warning in backend logs when legacy git preparer or dir preparer is used to preparer docs. Migrate to URL Preparer by updating `backstage.io/techdocs-ref` annotation to be prefixed with `url:`.
+Detailed docs are here https://backstage.io/docs/features/techdocs/how-to-guides#how-to-use-url-reader-in-techdocs-prepare-step
+See benefits and reason for doing so https://github.com/backstage/backstage/issues/4409
@@ -157,15 +157,17 @@ techdocs:
  builder: 'local'
 ```

-Set `techdocs.builder` to `'local'` if you want your TechDocs Backend to be
-responsible for generating documentation sites. If set to `'external'`,
-Backstage will assume that the sites are being generated on each entity's CI/CD
-pipeline, and are being stored in a storage somewhere.
+Note that we recommend generating docs on CI/CD instead. Read more in the
+"Basic" and "Recommended" sections of the
+[TechDocs Architecture](architecture.md). But if you want to get started quickly
+set `techdocs.builder` to `'local'` so that TechDocs Backend is responsible for
+generating documentation sites. If set to `'external'`, Backstage will assume
+that the sites are being generated on each entity's CI/CD pipeline, and are
+being stored in a storage somewhere.

 When `techdocs.builder` is set to `'external'`, TechDocs becomes more or less a
 read-only experience where it serves static files from a storage containing all
-the generated documentation. Read more in the "Basic" and "Recommended" sections
-of the [TechDocs Architecture](architecture.md).
+the generated documentation.

 ### Choosing storage (publisher)

@@ -139,7 +139,7 @@ describe('getDocFilesFromRepository', () => {
          archive: async () => {
            return Readable.from('');
          },
-          etag: '',
+          etag: 'etag123abc',
        };
      }

@@ -156,6 +156,7 @@ describe('getDocFilesFromRepository', () => {
      mockEntityWithAnnotation,
    );

-    expect(output).toBe('/tmp/testfolder');
+    expect(output.preparedDir).toBe('/tmp/testfolder');
+    expect(output.etag).toBe('etag123abc');
  });
 });
@@ -14,17 +14,17 @@
 * limitations under the License.
 */

-import os from 'os';
-import path from 'path';
-import parseGitUrl from 'git-url-parse';
-import fs from 'fs-extra';
-import { InputError, UrlReader, Git } from '@backstage/backend-common';
+import { Git, InputError, UrlReader } from '@backstage/backend-common';
 import { Entity } from '@backstage/catalog-model';
 import { Config } from '@backstage/config';
+import fs from 'fs-extra';
+import parseGitUrl from 'git-url-parse';
+import os from 'os';
+import path from 'path';
+import { Logger } from 'winston';
 import { getDefaultBranch } from './default-branch';
 import { getGitRepoType, getTokenForGitRepo } from './git-auth';
-import { RemoteProtocol } from './stages/prepare/types';
-import { Logger } from 'winston';
+import { PreparerResponse, RemoteProtocol } from './stages/prepare/types';

 export type ParsedLocationAnnotation = {
  type: RemoteProtocol;
@@ -196,16 +196,9 @@ export const checkoutGitRepository = async (
 };

 export const getLastCommitTimestamp = async (
-  repositoryUrl: string,
-  config: Config,
+  repositoryLocation: string,
  logger: Logger,
 ): Promise<number> => {
-  const repositoryLocation = await checkoutGitRepository(
-    repositoryUrl,
-    config,
-    logger,
-  );
-
  const git = Git.fromAuth({ logger });
  const sha = await git.resolveRef({ dir: repositoryLocation, ref: 'HEAD' });
  const commit = await git.readCommit({ dir: repositoryLocation, sha });
@@ -216,13 +209,22 @@ export const getLastCommitTimestamp = async (
 export const getDocFilesFromRepository = async (
  reader: UrlReader,
  entity: Entity,
-): Promise<any> => {
+  opts?: { etag?: string; logger?: Logger },
+): Promise<PreparerResponse> => {
  const { target } = parseReferenceAnnotation(
    'backstage.io/techdocs-ref',
    entity,
  );

-  const response = await reader.readTree(target);
+  opts?.logger?.info(`Reading files from ${target}`);
+  // readTree will throw NotModifiedError if etag has not changed.
+  const readTreeResponse = await reader.readTree(target, { etag: opts?.etag });
+  const preparedDir = await readTreeResponse.dir();

-  return await response.dir();
+  opts?.logger?.info(`Tree downloaded and stored at ${preparedDir}`);
+
+  return {
+    preparedDir,
+    etag: readTreeResponse.etag,
+  };
 };
@@ -16,8 +16,8 @@

 import { getVoidLogger } from '@backstage/backend-common';
 import { ConfigReader } from '@backstage/config';
-import { CommonGitPreparer } from './commonGit';
 import { checkoutGitRepository } from '../../helpers';
+import { CommonGitPreparer } from './commonGit';

 function normalizePath(path: string) {
  return path
@@ -29,6 +29,7 @@ function normalizePath(path: string) {
 jest.mock('../../helpers', () => ({
  ...jest.requireActual<{}>('../../helpers'),
  checkoutGitRepository: jest.fn(() => '/tmp/backstage-repo/org/name/branch'),
+  getLastCommitTimestamp: jest.fn(() => 12345678),
 }));

 const createMockEntity = (annotations = {}) => {
@@ -57,9 +58,9 @@ describe('commonGit preparer', () => {
        'github:https://github.com/backstage/backstage/blob/master/plugins/techdocs-backend/examples/documented-component',
    });

-    const tempDocsPath = await preparer.prepare(mockEntity);
+    const { preparedDir } = await preparer.prepare(mockEntity);
    expect(checkoutGitRepository).toHaveBeenCalledTimes(1);
-    expect(normalizePath(tempDocsPath)).toEqual(
+    expect(normalizePath(preparedDir)).toEqual(
      '/tmp/backstage-repo/org/name/branch/plugins/techdocs-backend/examples/documented-component',
    );
  });
@@ -72,9 +73,9 @@ describe('commonGit preparer', () => {
        'gitlab:https://gitlab.com/xesjkeee/go-logger/blob/master/catalog-info.yaml',
    });

-    const tempDocsPath = await preparer.prepare(mockEntity);
+    const { preparedDir } = await preparer.prepare(mockEntity);
    expect(checkoutGitRepository).toHaveBeenCalledTimes(2);
-    expect(normalizePath(tempDocsPath)).toEqual(
+    expect(normalizePath(preparedDir)).toEqual(
      '/tmp/backstage-repo/org/name/branch/catalog-info.yaml',
    );
  });
@@ -87,9 +88,9 @@ describe('commonGit preparer', () => {
        'azure/api:https://dev.azure.com/backstage-org/backstage-project/_git/template-repo?path=%2Ftemplate.yaml',
    });

-    const tempDocsPath = await preparer.prepare(mockEntity);
+    const { preparedDir } = await preparer.prepare(mockEntity);
    expect(checkoutGitRepository).toHaveBeenCalledTimes(3);
-    expect(normalizePath(tempDocsPath)).toEqual(
+    expect(normalizePath(preparedDir)).toEqual(
      '/tmp/backstage-repo/org/name/branch/template.yaml',
    );
  });
@@ -13,14 +13,18 @@
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
-import path from 'path';
-import parseGitUrl from 'git-url-parse';
+import { NotModifiedError } from '@backstage/backend-common';
 import { Entity } from '@backstage/catalog-model';
 import { Config } from '@backstage/config';
-import { PreparerBase } from './types';
-import { parseReferenceAnnotation, checkoutGitRepository } from '../../helpers';
-
+import parseGitUrl from 'git-url-parse';
+import path from 'path';
 import { Logger } from 'winston';
+import {
+  checkoutGitRepository,
+  getLastCommitTimestamp,
+  parseReferenceAnnotation,
+} from '../../helpers';
+import { PreparerBase, PreparerResponse } from './types';

 export class CommonGitPreparer implements PreparerBase {
  private readonly config: Config;
@@ -31,23 +35,45 @@ export class CommonGitPreparer implements PreparerBase {
    this.logger = logger;
  }

-  async prepare(entity: Entity): Promise<string> {
+  async prepare(
+    entity: Entity,
+    options?: { etag?: string },
+  ): Promise<PreparerResponse> {
+    this.logger.warn(
+      'You are using the legacy git preparer in TechDocs which will be removed in near future (March 2021). ' +
+        'Migrate to URL reader by updating `backstage.io/techdocs-ref` annotation in `catalog-info.yaml` ' +
+        'to be prefixed with `url:`. Read the migration guide and benefits at https://github.com/backstage/backstage/issues/4409 ',
+    );
+
    const { target } = parseReferenceAnnotation(
      'backstage.io/techdocs-ref',
      entity,
    );

    try {
+      // Update repository or do a fresh clone.
      const repoPath = await checkoutGitRepository(
        target,
        this.config,
        this.logger,
      );
-      const parsedGitLocation = parseGitUrl(target);
+      // Check if etag has changed for cache invalidation.
+      const etag = await getLastCommitTimestamp(repoPath, this.logger);
+      if (options?.etag === etag.toString()) {
+        throw new NotModifiedError();
+      }

-      return path.join(repoPath, parsedGitLocation.filepath);
+      const parsedGitLocation = parseGitUrl(target);
+      return {
+        preparedDir: path.join(repoPath, parsedGitLocation.filepath),
+        etag: etag.toString(),
+      };
    } catch (error) {
-      this.logger.debug(`Repo checkout failed with error ${error.message}`);
+      if (error instanceof NotModifiedError) {
+        this.logger.debug(`Cache is valid for etag ${options?.etag}`);
+      } else {
+        this.logger.debug(`Repo checkout failed with error ${error.message}`);
+      }
      throw error;
    }
  }
@@ -15,8 +15,8 @@
 */
 import { getVoidLogger, UrlReader } from '@backstage/backend-common';
 import { ConfigReader } from '@backstage/config';
-import { DirectoryPreparer } from './dir';
 import { checkoutGitRepository } from '../../helpers';
+import { DirectoryPreparer } from './dir';

 function normalizePath(path: string) {
  return path
@@ -28,6 +28,7 @@ function normalizePath(path: string) {
 jest.mock('../../helpers', () => ({
  ...jest.requireActual<{}>('../../helpers'),
  checkoutGitRepository: jest.fn(() => '/tmp/backstage-repo/org/name/branch/'),
+  getLastCommitTimestamp: jest.fn(() => 12345678),
 }));

 const logger = getVoidLogger();
@@ -66,9 +67,8 @@ describe('directory preparer', () => {
      'backstage.io/techdocs-ref': 'dir:./our-documentation',
    });

-    expect(normalizePath(await directoryPreparer.prepare(mockEntity))).toEqual(
-      '/directory/our-documentation',
-    );
+    const { preparedDir } = await directoryPreparer.prepare(mockEntity);
+    expect(normalizePath(preparedDir)).toEqual('/directory/our-documentation');
  });

  it('should merge managed-by-location and techdocs-ref when techdocs-ref is absolute', async () => {
@@ -84,9 +84,8 @@ describe('directory preparer', () => {
      'backstage.io/techdocs-ref': 'dir:/our-documentation/techdocs',
    });

-    expect(normalizePath(await directoryPreparer.prepare(mockEntity))).toEqual(
-      '/our-documentation/techdocs',
-    );
+    const { preparedDir } = await directoryPreparer.prepare(mockEntity);
+    expect(normalizePath(preparedDir)).toEqual('/our-documentation/techdocs');
  });

  it('should merge managed-by-location and techdocs-ref when managed-by-location is a git repository', async () => {
@@ -102,7 +101,8 @@ describe('directory preparer', () => {
      'backstage.io/techdocs-ref': 'dir:./docs',
    });

-    expect(normalizePath(await directoryPreparer.prepare(mockEntity))).toEqual(
+    const { preparedDir } = await directoryPreparer.prepare(mockEntity);
+    expect(normalizePath(preparedDir)).toEqual(
      '/tmp/backstage-repo/org/name/branch/docs',
    );
    expect(checkoutGitRepository).toHaveBeenCalledTimes(1);
@@ -13,14 +13,22 @@
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
-import { PreparerBase } from './types';
+import {
+  InputError,
+  NotModifiedError,
+  UrlReader,
+} from '@backstage/backend-common';
 import { Entity } from '@backstage/catalog-model';
 import { Config } from '@backstage/config';
-import path from 'path';
-import { parseReferenceAnnotation, checkoutGitRepository } from '../../helpers';
-import { UrlReader, InputError } from '@backstage/backend-common';
 import parseGitUrl from 'git-url-parse';
+import path from 'path';
 import { Logger } from 'winston';
+import {
+  checkoutGitRepository,
+  getLastCommitTimestamp,
+  parseReferenceAnnotation,
+} from '../../helpers';
+import { PreparerBase, PreparerResponse } from './types';

 export class DirectoryPreparer implements PreparerBase {
  constructor(
@@ -33,7 +41,10 @@ export class DirectoryPreparer implements PreparerBase {
    this.reader = reader;
  }

-  private async resolveManagedByLocationToDir(entity: Entity) {
+  private async resolveManagedByLocationToDir(
+    entity: Entity,
+    options?: { etag?: string },
+  ): Promise<PreparerResponse> {
    const { type, target } = parseReferenceAnnotation(
      'backstage.io/managed-by-location',
      entity,
@@ -44,8 +55,14 @@ export class DirectoryPreparer implements PreparerBase {
    );
    switch (type) {
      case 'url': {
-        const response = await this.reader.readTree(target);
-        return await response.dir();
+        const response = await this.reader.readTree(target, {
+          etag: options?.etag,
+        });
+        const preparedDir = await response.dir();
+        return {
+          preparedDir,
+          etag: response.etag,
+        };
      }
      case 'github':
      case 'gitlab':
@@ -57,29 +74,47 @@ export class DirectoryPreparer implements PreparerBase {
          this.logger,
        );

-        return path.dirname(
-          path.join(repoLocation, parsedGitLocation.filepath),
-        );
+        // Check if etag has changed for cache invalidation.
+        const etag = await getLastCommitTimestamp(repoLocation, this.logger);
+        if (options?.etag === etag.toString()) {
+          throw new NotModifiedError();
+        }
+        return {
+          preparedDir: path.dirname(
+            path.join(repoLocation, parsedGitLocation.filepath),
+          ),
+          etag: etag.toString(),
+        };
      }
      case 'file':
-        return path.dirname(target);
+        return {
+          preparedDir: path.dirname(target),
+          // Instead of supporting caching on local sources, use techdocs-cli for local development and debugging.
+          etag: '',
+        };
      default:
        throw new InputError(`Unable to resolve location type ${type}`);
    }
  }

-  async prepare(entity: Entity): Promise<string> {
+  async prepare(entity: Entity): Promise<PreparerResponse> {
+    this.logger.warn(
+      'You are using the legacy dir preparer in TechDocs which will be removed in near future (March 2021). ' +
+        'Migrate to URL reader by updating `backstage.io/techdocs-ref` annotation in `catalog-info.yaml` ' +
+        'to be prefixed with `url:`. Read the migration guide and benefits at https://github.com/backstage/backstage/issues/4409 ',
+    );
+
    const { target } = parseReferenceAnnotation(
      'backstage.io/techdocs-ref',
      entity,
    );

-    const managedByLocationDirectory = await this.resolveManagedByLocationToDir(
-      entity,
-    );
+    // This will throw NotModified error if etag has not changed.
+    const response = await this.resolveManagedByLocationToDir(entity);

-    return new Promise(resolve => {
-      resolve(path.resolve(managedByLocationDirectory, target));
-    });
+    return {
+      preparedDir: path.resolve(response.preparedDir, target),
+      etag: response.etag,
+    };
  }
 }
@@ -16,13 +16,31 @@
 import type { Entity } from '@backstage/catalog-model';
 import { Logger } from 'winston';

+export type PreparerResponse = {
+  /**
+   * The path to directory where the tree is downloaded.
+   */
+  preparedDir: string;
+  /**
+   * A unique identifer of the tree blob, usually the commit SHA or etag from the target.
+   */
+  etag: string;
+};
+
 export type PreparerBase = {
  /**
   * Given an Entity definition from the Service Catalog, go and prepare a directory
-   * with contents from the location in temporary storage and return the path
+   * with contents from the location in temporary storage and return the path.
+   *
   * @param entity The entity from the Service Catalog
+   * @param options.etag (Optional) If etag is provider, it will be used to check if the target has
+   * updated since the last build.
+   * @throws {NotModifiedError} when the prepared directory has not been changed since the last build.
   */
-  prepare(entity: Entity, opts?: { logger: Logger }): Promise<string>;
+  prepare(
+    entity: Entity,
+    options?: { logger?: Logger; etag?: string },
+  ): Promise<PreparerResponse>;
 };

 export type PreparerBuilder = {
@@ -30,10 +48,14 @@ export type PreparerBuilder = {
  get(entity: Entity): PreparerBase;
 };

+/**
+ * Everything except `url` will be deprecated.
+ * Read more https://github.com/backstage/backstage/issues/4409
+ */
 export type RemoteProtocol =
+  | 'url'
  | 'dir'
  | 'github'
  | 'gitlab'
  | 'file'
-  | 'azure/api'
-  | 'url';
+  | 'azure/api';
@@ -13,11 +13,11 @@
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
-import { Logger } from 'winston';
+import { NotModifiedError, UrlReader } from '@backstage/backend-common';
 import { Entity } from '@backstage/catalog-model';
-import { UrlReader } from '@backstage/backend-common';
-import { PreparerBase } from './types';
+import { Logger } from 'winston';
 import { getDocFilesFromRepository } from '../../helpers';
+import { PreparerBase, PreparerResponse } from './types';

 export class UrlPreparer implements PreparerBase {
  private readonly logger: Logger;
@@ -28,13 +28,25 @@ export class UrlPreparer implements PreparerBase {
    this.reader = reader;
  }

-  async prepare(entity: Entity): Promise<string> {
+  async prepare(
+    entity: Entity,
+    options?: { etag?: string },
+  ): Promise<PreparerResponse> {
    try {
-      return getDocFilesFromRepository(this.reader, entity);
+      return await getDocFilesFromRepository(this.reader, entity, {
+        etag: options?.etag,
+        logger: this.logger,
+      });
    } catch (error) {
-      this.logger.debug(
-        `Unable to fetch files for building docs ${error.message}`,
-      );
+      // NotModifiedError means that etag based cache is still valid.
+      if (error instanceof NotModifiedError) {
+        this.logger.debug(`Cache is valid for etag ${options?.etag}`);
+      } else {
+        this.logger.debug(
+          `Unable to fetch files for building docs ${error.message}`,
+        );
+      }
+
      throw error;
    }
  }
@@ -88,7 +88,7 @@ export class LocalPublish implements PublisherBase {
          );
          reject(err);
        }
-
+        this.logger.info(`Published site stored at ${publishDir}`);
        this.discovery
          .getBaseUrl('techdocs')
          .then(techdocsApiUrl => {
@@ -17,11 +17,11 @@ import { BuildMetadataStorage } from './BuildMetadataStorage';

 describe('BuildMetadataStorage', () => {
  it('should return build timestamp', () => {
-    const newMetadataStorage = new BuildMetadataStorage('123abc');
-    newMetadataStorage.storeBuildTimestamp();
+    const newMetadataStorage = new BuildMetadataStorage('entityID123abc');
+    newMetadataStorage.setEtag('etag123abc');

-    const timestamp = newMetadataStorage.getTimestamp();
+    const timestamp = newMetadataStorage.getEtag();

-    expect(timestamp).toBeLessThanOrEqual(Date.now());
+    expect(timestamp).toBe('etag123abc');
  });
 });
@@ -14,10 +14,15 @@
 * limitations under the License.
 */
 type buildInfo = {
-  // uid: timestamp
-  [key: string]: number;
+  // Entity uid: etag
+  [key: string]: string;
 };

+// TODO: Build info should be part of TechDocs storage, inside `techdocs_metadata.json`
+// instead of in-memory storage of the Backstage instance.
+// In case of multi-region Backstage deployments, or even using multiple Kubernetes pods,
+// if each instance creates its separate build info in-memory, it will result in duplicate
+// builds per instance. Also if the pod restarts, all the sites will have to be re-built.
 const builds = {} as buildInfo;

 /**
@@ -34,11 +39,11 @@ export class BuildMetadataStorage {
    this.builds = builds;
  }

-  storeBuildTimestamp() {
-    this.builds[this.entityUid] = Date.now();
+  setEtag(etag: string): void {
+    this.builds[this.entityUid] = etag;
  }

-  getTimestamp() {
+  getEtag(): string | undefined {
    return this.builds[this.entityUid];
  }
 }
@@ -13,22 +13,22 @@
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
+import { NotModifiedError } from '@backstage/backend-common';
+import { Entity } from '@backstage/catalog-model';
+import {
+  GeneratorBase,
+  GeneratorBuilder,
+  getLocationForEntity,
+  PreparerBase,
+  PreparerBuilder,
+  PublisherBase,
+  UrlPreparer,
+} from '@backstage/techdocs-common';
+import Docker from 'dockerode';
 import fs from 'fs-extra';
 import os from 'os';
 import path from 'path';
-import Docker from 'dockerode';
 import { Logger } from 'winston';
-import { Entity } from '@backstage/catalog-model';
-import { Config } from '@backstage/config';
-import {
-  PreparerBuilder,
-  PublisherBase,
-  GeneratorBuilder,
-  PreparerBase,
-  GeneratorBase,
-  getLocationForEntity,
-  getLastCommitTimestamp,
-} from '@backstage/techdocs-common';
 import { BuildMetadataStorage } from '.';

 const getEntityId = (entity: Entity) => {
@@ -44,7 +44,6 @@ type DocsBuilderArguments = {
  entity: Entity;
  logger: Logger;
  dockerClient: Docker;
-  config: Config;
 };

 export class DocsBuilder {
@@ -54,7 +53,6 @@ export class DocsBuilder {
  private entity: Entity;
  private logger: Logger;
  private dockerClient: Docker;
-  private config: Config;

  constructor({
    preparers,
@@ -63,7 +61,6 @@ export class DocsBuilder {
    entity,
    logger,
    dockerClient,
-    config,
  }: DocsBuilderArguments) {
    this.preparer = preparers.get(entity);
    this.generator = generators.get(entity);
@@ -71,14 +68,53 @@ export class DocsBuilder {
    this.entity = entity;
    this.logger = logger;
    this.dockerClient = dockerClient;
-    this.config = config;
  }

-  public async build() {
-    this.logger.info(`Running preparer on entity ${getEntityId(this.entity)}`);
-    const preparedDir = await this.preparer.prepare(this.entity);
+  public async build(): Promise<void> {
+    if (!this.entity.metadata.uid) {
+      throw new Error(
+        'Trying to build documentation for entity not in service catalog',
+      );
+    }

-    const parsedLocationAnnotation = getLocationForEntity(this.entity);
+    /**
+     * Prepare and cache check
+     */
+
+    // Use the in-memory storage for setting and getting etag for this entity.
+    const buildMetadataStorage = new BuildMetadataStorage(
+      this.entity.metadata.uid,
+    );
+
+    // TODO: As of now, this happens on each and every request to TechDocs.
+    // In a high traffic environment, this will cause a lot of requests to the source code provider.
+    // After Async build is implemented https://github.com/backstage/backstage/issues/3717,
+    // make sure to limit checking for cache invalidation to once per minute or so.
+    let preparedDir: string;
+    let etag: string;
+    try {
+      const preparerResponse = await this.preparer.prepare(this.entity, {
+        etag: buildMetadataStorage.getEtag(),
+      });
+
+      preparedDir = preparerResponse.preparedDir;
+      etag = preparerResponse.etag;
+    } catch (err) {
+      if (err instanceof NotModifiedError) {
+        // No need to prepare anymore since cache is valid.
+        return;
+      }
+      throw new Error(err.message);
+    }
+
+    this.logger.info(
+      `TechDocs prepare step completed for entity ${getEntityId(this.entity)}.`,
+    );
+    this.logger.debug(`Prepared files temporarily stored at ${preparedDir}`);
+
+    /**
+     * Generate
+     */

    this.logger.info(`Running generator on entity ${getEntityId(this.entity)}`);
    // Create a temporary directory to store the generated files in.
@@ -88,7 +124,7 @@ export class DocsBuilder {
    const outputDir = await fs.mkdtemp(
      path.join(tmpdirResolvedPath, 'techdocs-tmp-'),
    );
-
+    const parsedLocationAnnotation = getLocationForEntity(this.entity);
    await this.generator.run({
      inputDir: preparedDir,
      outputDir,
@@ -96,69 +132,42 @@ export class DocsBuilder {
      parsedLocationAnnotation,
    });

+    this.logger.debug(`Generated files temporarily stored at ${outputDir}`);
+    // Remove Prepared directory since it is no longer needed.
+    // Caveat: Can not remove prepared directory in case of git preparer since the
+    // local git repository is used to get etag on subsequent requests.
+    if (this.preparer instanceof UrlPreparer) {
+      this.logger.debug(
+        `Removing prepared directory ${preparedDir} since the site has been generated.`,
+      );
+      try {
+        // Not a blocker hence no need to await this.
+        fs.remove(preparedDir);
+      } catch (error) {
+        this.logger.debug(`Error removing prepared directory ${error.message}`);
+      }
+    }
+
+    /**
+     * Publish
+     */
+
    this.logger.info(`Running publisher on entity ${getEntityId(this.entity)}`);
    await this.publisher.publish({
      entity: this.entity,
      directory: outputDir,
    });
-
-    // TODO: Remove the generated directory once published.
-
-    if (!this.entity.metadata.uid) {
-      throw new Error(
-        'Trying to build documentation for entity not in service catalog',
-      );
-    }
-
-    new BuildMetadataStorage(this.entity.metadata.uid).storeBuildTimestamp();
-  }
-
-  public async docsUpToDate() {
-    if (!this.entity.metadata.uid) {
-      throw new Error(
-        'Trying to build documentation for entity not in service catalog',
-      );
-    }
-
-    const buildMetadataStorage = new BuildMetadataStorage(
-      this.entity.metadata.uid,
-    );
-    const { type, target } = getLocationForEntity(this.entity);
-
-    // Unless docs are stored locally
-    const nonAgeCheckTypes = ['dir', 'file', 'url'];
-    if (!nonAgeCheckTypes.includes(type)) {
-      const lastCommit = await getLastCommitTimestamp(
-        target,
-        this.config,
-        this.logger,
-      );
-      const storageTimeStamp = buildMetadataStorage.getTimestamp();
-
-      // Check if documentation source is newer than what we have
-      if (storageTimeStamp && storageTimeStamp >= lastCommit) {
-        this.logger.debug(
-          `Docs for entity ${getEntityId(this.entity)} is up to date.`,
-        );
-        return true;
-      }
-    }
-
-    // Cache downloaded source files for 30 minutes.
-    // TODO: When urlReader/readTree supports some way to get latest commit timestamp,
-    // it should be used to invalidate cache.
-    if (type === 'url') {
-      const builtAt = buildMetadataStorage.getTimestamp();
-      const now = Date.now();
-
-      if (builtAt > now - 1800000) {
-        return true;
-      }
-    }
-
    this.logger.debug(
-      `Docs for entity ${getEntityId(this.entity)} was outdated.`,
+      `Removing generated directory ${outputDir} since the site has been published`,
    );
-    return false;
+    try {
+      // Not a blocker hence no need to await this.
+      fs.remove(outputDir);
+    } catch (error) {
+      this.logger.debug(`Error removing generated directory ${error.message}`);
+    }
+
+    // Store the latest build etag for the entity
+    new BuildMetadataStorage(this.entity.metadata.uid).setEtag(etag);
  }
 }
@@ -13,23 +13,23 @@
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
-import { Logger } from 'winston';
-import Router from 'express-promise-router';
-import express from 'express';
-import Knex from 'knex';
-import fetch from 'cross-fetch';
-import { Config } from '@backstage/config';
-import Docker from 'dockerode';
-import {
-  GeneratorBuilder,
-  PreparerBuilder,
-  PublisherBase,
-  getLocationForEntity,
-} from '@backstage/techdocs-common';
 import { PluginEndpointDiscovery } from '@backstage/backend-common';
 import { Entity } from '@backstage/catalog-model';
-import { getEntityNameFromUrlPath } from './helpers';
+import { Config } from '@backstage/config';
+import {
+  GeneratorBuilder,
+  getLocationForEntity,
+  PreparerBuilder,
+  PublisherBase,
+} from '@backstage/techdocs-common';
+import fetch from 'cross-fetch';
+import Docker from 'dockerode';
+import express from 'express';
+import Router from 'express-promise-router';
+import Knex from 'knex';
+import { Logger } from 'winston';
 import { DocsBuilder } from '../DocsBuilder';
+import { getEntityNameFromUrlPath } from './helpers';

 type RouterOptions = {
  preparers: PreparerBuilder;
@@ -141,13 +141,10 @@ export async function createRouter({
        dockerClient,
        logger,
        entity,
-        config,
      });
      switch (publisherType) {
        case 'local':
-          if (!(await docsBuilder.docsUpToDate())) {
-            await docsBuilder.build();
-          }
+          await docsBuilder.build();
          break;
        case 'awsS3':
        case 'azureBlobStorage':
@@ -186,9 +183,11 @@ export async function createRouter({
              'Found pre-generated docs for this entity. Serving them.',
            );
            // TODO: re-trigger build for cache invalidation.
-            // Compare the date modified of the requested file on storage and compare it against
-            // the last modified or last commit timestamp in the repository.
+            // Add build info in techdocs_metadata.json and compare it against
+            // the etag/commit in the repository.
            // Without this, docs will not be re-built once they have been generated.
+            // Although it is unconventional that anyone will face this issue - because
+            // if you have an external storage, you should be using CI/CD to build and publish docs.
          }
          break;
        default: