From be87bc05846d61bf1ebbb78922c89197928940b0 Mon Sep 17 00:00:00 2001 From: Otto Sichert Date: Thu, 15 Jul 2021 11:45:43 +0200 Subject: [PATCH 01/16] Remove stale TechDocs artifacts after publishing new version for GCS Co-authored-by: Camila Belo Co-authored-by: Eric Peterson Signed-off-by: Otto Sichert --- .../src/stages/publish/googleStorage.ts | 100 +++++++++++------- .../src/stages/publish/helpers.ts | 40 +++++++ 2 files changed, 100 insertions(+), 40 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index 9def24722a..349d6ead97 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -15,18 +15,19 @@ */ import { Entity, EntityName } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import { - FileExistsResponse, - Storage, - UploadResponse, -} from '@google-cloud/storage'; +import { FileExistsResponse, Storage } from '@google-cloud/storage'; import express from 'express'; import JSON5 from 'json5'; -import createLimiter from 'p-limit'; import path from 'path'; import { Readable } from 'stream'; import { Logger } from 'winston'; -import { getFileTreeRecursively, getHeadersForFileExtension } from './helpers'; +import { + bulkStorageOperation, + getCloudPathForLocalPath, + getFileTreeRecursively, + getHeadersForFileExtension, + getStaleFiles, +} from './helpers'; import { MigrateWriteStream } from './migrations'; import { PublisherBase, @@ -114,49 +115,68 @@ export class GoogleGCSPublish implements PublisherBase { * Directory structure used in the bucket is - entityNamespace/entityKind/entityName/index.html */ async publish({ entity, directory }: PublishRequest): Promise { + const bucket = this.storageClient.bucket(this.bucketName); + + // First, retrieve a list of all individual files in currently existing + const remoteFolder = getCloudPathForLocalPath(entity); + const existingFiles = ( + await bucket.getFiles({ prefix: remoteFolder }) + )[0].map(file => file.name); + + // Then, merge new files into the same folder + let absoluteFilesToUpload; try { - // Note: GCS manages creation of parent directories if they do not exist. - // So collecting path of only the files is good enough. - const allFilesToUpload = await getFileTreeRecursively(directory); + // Remove the absolute path prefix of the source directory + // Path of all files to upload, relative to the root of the source directory + // e.g. ['index.html', 'sub-page/index.html', 'assets/images/favicon.png'] + absoluteFilesToUpload = await getFileTreeRecursively(directory); - const limiter = createLimiter(10); - const uploadPromises: Array> = []; - allFilesToUpload.forEach(sourceFilePath => { - // Remove the absolute path prefix of the source directory - // Path of all files to upload, relative to the root of the source directory - // e.g. ['index.html', 'sub-page/index.html', 'assets/images/favicon.png'] - const relativeFilePath = path.relative(directory, sourceFilePath); - - // Convert destination file path to a POSIX path for uploading. - // GCS expects / as path separator and relativeFilePath will contain \\ on Windows. - // https://cloud.google.com/storage/docs/gsutil/addlhelp/HowSubdirectoriesWork - const relativeFilePathPosix = relativeFilePath - .split(path.sep) - .join(path.posix.sep); - - // The / delimiter is intentional since it represents the cloud storage and not the local file system. - const entityRootDir = `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}`; - const destination = `${entityRootDir}/${relativeFilePathPosix}`; // GCS Bucket file relative path - - // Rate limit the concurrent execution of file uploads to batches of 10 (per publish) - const uploadFile = limiter(() => - this.storageClient.bucket(this.bucketName).upload(sourceFilePath, { - destination, - }), - ); - uploadPromises.push(uploadFile); - }); - - await Promise.all(uploadPromises); + await bulkStorageOperation( + async absoluteFilePath => { + const relativeFilePath = path.relative(directory, absoluteFilePath); + return await bucket.upload(absoluteFilePath, { + destination: getCloudPathForLocalPath(entity, relativeFilePath), + }); + }, + absoluteFilesToUpload, + { concurrencyLimit: 10 }, + ); this.logger.info( - `Successfully uploaded all the generated files for Entity ${entity.metadata.name}. Total number of files: ${allFilesToUpload.length}`, + `Successfully uploaded all the generated files for Entity ${entity.metadata.name}. Total number of files: ${absoluteFilesToUpload.length}`, ); } catch (e) { const errorMessage = `Unable to upload file(s) to Google Cloud Storage. ${e}`; this.logger.error(errorMessage); throw new Error(errorMessage); } + + // Last, try to remove the files that were *only* present previously + try { + const relativeFilesToUpload = absoluteFilesToUpload.map( + absoluteFilePath => + getCloudPathForLocalPath( + entity, + path.relative(directory, absoluteFilePath), + ), + ); + const staleFiles = getStaleFiles(relativeFilesToUpload, existingFiles); + + await bulkStorageOperation( + async relativeFilePath => { + return await bucket.file(relativeFilePath).delete(); + }, + staleFiles, + { concurrencyLimit: 10 }, + ); + + this.logger.info( + `Successfully deleted stale files for Entity ${entity.metadata.name}. Total number of files: ${staleFiles.length}`, + ); + } catch (error) { + const errorMessage = `Unable to delete file(s) from Google Cloud Storage. ${error}`; + this.logger.error(errorMessage); + } } fetchTechDocsMetadata(entityName: EntityName): Promise { diff --git a/packages/techdocs-common/src/stages/publish/helpers.ts b/packages/techdocs-common/src/stages/publish/helpers.ts index ed681eb8ba..1f77d33971 100644 --- a/packages/techdocs-common/src/stages/publish/helpers.ts +++ b/packages/techdocs-common/src/stages/publish/helpers.ts @@ -13,7 +13,10 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import { Entity } from '@backstage/catalog-model'; import mime from 'mime-types'; +import path from 'path'; +import createLimiter from 'p-limit'; import recursiveReadDir from 'recursive-readdir'; /** @@ -110,4 +113,41 @@ export const lowerCaseEntityTripletInStoragePath = ( const lowerKind = kind.toLowerCase(); const lowerName = name.toLowerCase(); return [lowerNamespace, lowerKind, lowerName, ...parts].join('/'); +} + +// Only returns the files that existed previously and are not present anymore. +export const getStaleFiles = ( + newFiles: string[], + oldFiles: string[], +): string[] => { + const staleFiles = new Set(oldFiles); + newFiles.forEach(newFile => { + staleFiles.delete(newFile); + }); + return Array.from(staleFiles); +}; + +// Compose actual filename on remote bucket including entity information +export const getCloudPathForLocalPath = ( + entity: Entity, + localPath = '', +): string => { + // Convert destination file path to a POSIX path for uploading. + // GCS expects / as path separator and relativeFilePath will contain \\ on Windows. + // https://cloud.google.com/storage/docs/gsutil/addlhelp/HowSubdirectoriesWork + const relativeFilePathPosix = localPath.split(path.sep).join(path.posix.sep); + + // The / delimiter is intentional since it represents the cloud storage and not the local file system. + const entityRootDir = `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}`; + return `${entityRootDir}/${relativeFilePathPosix}`; // GCS Bucket file relative path +}; + +// Perform rate limited generic operations by passing a function and a list of arguments +export const bulkStorageOperation = async ( + operation: (arg: T) => Promise, + args: T[], + { concurrencyLimit } = { concurrencyLimit: 25 }, +) => { + const limiter = createLimiter(concurrencyLimit); + await Promise.all(args.map(arg => limiter(operation, arg))); }; From 06b507283b940dd9ebeef2f43c11a43255d7ef58 Mon Sep 17 00:00:00 2001 From: Otto Sichert Date: Wed, 21 Jul 2021 14:53:31 +0200 Subject: [PATCH 02/16] WIP: Remove stale TechDOcs files from AWS Signed-off-by: Otto Sichert --- .../src/stages/publish/awsS3.ts | 98 ++++++++++++------- 1 file changed, 63 insertions(+), 35 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index 7bdc91ffd4..7306320fd2 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -26,8 +26,11 @@ import path from 'path'; import { Readable } from 'stream'; import { Logger } from 'winston'; import { + bulkStorageOperation, + getCloudPathForLocalPath, getFileTreeRecursively, getHeadersForFileExtension, + getStaleFiles, lowerCaseEntityTripletInStoragePath, } from './helpers'; import { @@ -175,54 +178,77 @@ export class AwsS3Publish implements PublisherBase { * Directory structure used in the bucket is - entityNamespace/entityKind/entityName/index.html */ async publish({ entity, directory }: PublishRequest): Promise { + // First, retrieve a list of all individual files in currently existing + const remoteFolder = getCloudPathForLocalPath(entity); + const existingFiles = await this.getAllObjectsFromBucket({ + prefix: remoteFolder, + }); + + // Then, merge new files into the same folder + let absoluteFilesToUpload; try { - // Note: S3 manages creation of parent directories if they do not exist. - // So collecting path of only the files is good enough. - const allFilesToUpload = await getFileTreeRecursively(directory); + // Remove the absolute path prefix of the source directory + // Path of all files to upload, relative to the root of the source directory + // e.g. ['index.html', 'sub-page/index.html', 'assets/images/favicon.png'] + absoluteFilesToUpload = await getFileTreeRecursively(directory); - const limiter = createLimiter(10); - const uploadPromises: Array> = []; - for (const filePath of allFilesToUpload) { - // Remove the absolute path prefix of the source directory - // Path of all files to upload, relative to the root of the source directory - // e.g. ['index.html', 'sub-page/index.html', 'assets/images/favicon.png'] - const relativeFilePath = path.relative(directory, filePath); - - // Convert destination file path to a POSIX path for uploading. - // S3 expects / as path separator and relativeFilePath will contain \\ on Windows. - // https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html - const relativeFilePathPosix = relativeFilePath - .split(path.sep) - .join(path.posix.sep); - - // The / delimiter is intentional since it represents the cloud storage and not the local file system. - const entityRootDir = `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}`; - const destination = `${entityRootDir}/${relativeFilePathPosix}`; // S3 Bucket file relative path - - // Rate limit the concurrent execution of file uploads to batches of 10 (per publish) - const uploadFile = limiter(() => { - const fileStream = fs.createReadStream(filePath); + await bulkStorageOperation( + async absoluteFilePath => { + const relativeFilePath = path.relative(directory, absoluteFilePath); + const fileStream = fs.createReadStream(absoluteFilePath); const params = { Bucket: this.bucketName, - Key: destination, + Key: getCloudPathForLocalPath(entity, relativeFilePath), Body: fileStream, }; return this.storageClient.upload(params).promise(); - }); - uploadPromises.push(uploadFile); - } - await Promise.all(uploadPromises); - this.logger.info( - `Successfully uploaded all the generated files for Entity ${entity.metadata.name}. Total number of files: ${allFilesToUpload.length}`, + }, + absoluteFilesToUpload, + { concurrencyLimit: 10 }, + ); + + this.logger.info( + `Successfully uploaded all the generated files for Entity ${entity.metadata.name}. Total number of files: ${absoluteFilesToUpload.length}`, ); - return; } catch (e) { const errorMessage = `Unable to upload file(s) to AWS S3. ${e}`; this.logger.error(errorMessage); throw new Error(errorMessage); } + + // Last, try to remove the files that were *only* present previously + try { + const relativeFilesToUpload = absoluteFilesToUpload.map( + absoluteFilePath => + getCloudPathForLocalPath( + entity, + path.relative(directory, absoluteFilePath), + ), + ); + const staleFiles = getStaleFiles(relativeFilesToUpload, existingFiles); + + await bulkStorageOperation( + async relativeFilePath => { + return await this.storageClient + .deleteObject({ + Bucket: this.bucketName, + Key: relativeFilePath, + }) + .promise(); + }, + staleFiles, + { concurrencyLimit: 10 }, + ); + + this.logger.info( + `Successfully deleted stale files for Entity ${entity.metadata.name}. Total number of files: ${staleFiles.length}`, + ); + } catch (error) { + const errorMessage = `Unable to delete file(s) from AWS S3. ${error}`; + this.logger.error(errorMessage); + } } async fetchTechDocsMetadata( @@ -365,17 +391,19 @@ export class AwsS3Publish implements PublisherBase { /** * Returns a list of all object keys from the configured bucket. */ - protected async getAllObjectsFromBucket(): Promise { + protected async getAllObjectsFromBucket( + { prefix } = { prefix: '' }, + ): Promise { const objects: string[] = []; let nextContinuation: string | undefined; let allObjects: ListObjectsV2Output; - // Iterate through every file in the root of the publisher. do { allObjects = await this.storageClient .listObjectsV2({ Bucket: this.bucketName, ContinuationToken: nextContinuation, + ...(prefix ? { Prefix: prefix } : {}), }) .promise(); objects.push( From 064f8d24876a96fba2489c77451b2bb32cf6fd50 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Fri, 23 Jul 2021 20:28:52 +0200 Subject: [PATCH 03/16] feat(techdocs): delete stale files when publishing to Azure Signed-off-by: Camila Belo --- .../src/stages/publish/azureBlobStorage.ts | 139 ++++++++++-------- .../src/stages/publish/helpers.ts | 11 +- 2 files changed, 85 insertions(+), 65 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index fe8fe9c2ce..5dc868ba2b 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -26,8 +26,11 @@ import limiterFactory from 'p-limit'; import { default as path, default as platformPath } from 'path'; import { Logger } from 'winston'; import { + bulkStorageOperation, + getCloudPathForLocalPath, getFileTreeRecursively, getHeadersForFileExtension, + getStaleFiles, lowerCaseEntityTripletInStoragePath, } from './helpers'; import { @@ -133,75 +136,85 @@ export class AzureBlobStoragePublish implements PublisherBase { * Directory structure used in the container is - entityNamespace/entityKind/entityName/index.html */ async publish({ entity, directory }: PublishRequest): Promise { + // First, retrieve a list of all individual files in currently existing + const remoteFolder = getCloudPathForLocalPath(entity); + const existingFiles: string[] = []; + const container = this.storageClient.getContainerClient(this.containerName); try { - // Note: Azure Blob Storage manages creation of parent directories if they do not exist. - // So collecting path of only the files is good enough. - const allFilesToUpload = await getFileTreeRecursively(directory); - - // Bound the number of concurrent batches. We want a bit of concurrency for - // performance reasons, but not so much that we starve the connection pool - // or start thrashing. - const limiter = limiterFactory(BATCH_CONCURRENCY); - - const promises = allFilesToUpload.map(sourceFilePath => { - // Remove the absolute path prefix of the source directory - // Path of all files to upload, relative to the root of the source directory - // e.g. ['index.html', 'sub-page/index.html', 'assets/images/favicon.png'] - const relativeFilePath = path.normalize( - path.relative(directory, sourceFilePath), - ); - - // Convert destination file path to a POSIX path for uploading. - // Azure Blob Storage expects / as path separator and relativeFilePath will contain \\ on Windows. - // https://docs.microsoft.com/en-us/rest/api/storageservices/naming-and-referencing-containers--blobs--and-metadata#blob-names - const relativeFilePathPosix = relativeFilePath - .split(path.sep) - .join(path.posix.sep); - - // The / delimiter is intentional since it represents the cloud storage and not the local file system. - const entityRootDir = `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}`; - const destination = `${entityRootDir}/${relativeFilePathPosix}`; // Azure Blob Storage Container file relative path - return limiter(async () => { - const response = await this.storageClient - .getContainerClient(this.containerName) - .getBlockBlobClient(destination) - .uploadFile(sourceFilePath); - - if (response._response.status >= 400) { - return { - ...response, - error: new Error( - `Upload failed for ${sourceFilePath} with status code ${response._response.status}`, - ), - }; - } - return { - ...response, - error: undefined, - }; - }); - }); - - const responses = await Promise.all(promises); - - const failed = responses.filter(r => r.error); - if (failed.length === 0) { - this.logger.info( - `Successfully uploaded the ${responses.length} generated file(s) for Entity ${entity.metadata.name}. Total number of files: ${allFilesToUpload.length}`, - ); - } else { - throw new Error( - failed - .map(r => r.error?.message) - .filter(Boolean) - .join(' '), - ); + for await (const blob of container.listBlobsFlat()) { + existingFiles.push(blob.name); } } catch (e) { - const errorMessage = `Unable to upload file(s) to Azure Blob Storage. ${e}`; + const errorMessage = `Unable to list file(s) to Azure. ${e}`; this.logger.error(errorMessage); throw new Error(errorMessage); } + + // Then, merge new files into the same folder + let absoluteFilesToUpload; + try { + // Remove the absolute path prefix of the source directory + // Path of all files to upload, relative to the root of the source directory + // e.g. ['index.html', 'sub-page/index.html', 'assets/images/favicon.png'] + absoluteFilesToUpload = await getFileTreeRecursively(directory); + + await bulkStorageOperation( + async absoluteFilePath => { + // const relativeFilePath = path.relative(directory, absoluteFilePath); + const relativeFilePath = path.normalize( + path.relative(directory, absoluteFilePath), + ); + return await this.storageClient + .getContainerClient(this.containerName) + .getBlockBlobClient( + getCloudPathForLocalPath(entity, relativeFilePath), + ) + .uploadFile(absoluteFilePath); + }, + absoluteFilesToUpload, + { concurrencyLimit: BATCH_CONCURRENCY }, + ); + + this.logger.info( + `Successfully uploaded all the generated files for Entity ${entity.metadata.name}. Total number of files: ${absoluteFilesToUpload.length}`, + ); + } catch (e) { + const errorMessage = `Unable to upload file(s) to Azure. ${e}`; + this.logger.error(errorMessage); + throw new Error(errorMessage); + } + + // Last, try to remove the files that were *only* present previously + try { + const relativeFilesToUpload = absoluteFilesToUpload.map( + absoluteFilePath => + getCloudPathForLocalPath( + entity, + path.relative(directory, absoluteFilePath), + ), + ); + + const staleFiles = getStaleFiles( + relativeFilesToUpload, + existingFiles, + remoteFolder, + ); + + await bulkStorageOperation( + async relativeFilePath => { + return await container.deleteBlob(relativeFilePath); + }, + staleFiles, + { concurrencyLimit: BATCH_CONCURRENCY }, + ); + + this.logger.info( + `Successfully deleted stale files for Entity ${entity.metadata.name}. Total number of files: ${staleFiles.length}`, + ); + } catch (error) { + const errorMessage = `Unable to delete file(s) from AWS S3. ${error}`; + this.logger.error(errorMessage); + } } private download(containerName: string, blobPath: string): Promise { diff --git a/packages/techdocs-common/src/stages/publish/helpers.ts b/packages/techdocs-common/src/stages/publish/helpers.ts index 1f77d33971..2aabcb3259 100644 --- a/packages/techdocs-common/src/stages/publish/helpers.ts +++ b/packages/techdocs-common/src/stages/publish/helpers.ts @@ -113,14 +113,21 @@ export const lowerCaseEntityTripletInStoragePath = ( const lowerKind = kind.toLowerCase(); const lowerName = name.toLowerCase(); return [lowerNamespace, lowerKind, lowerName, ...parts].join('/'); -} +}; // Only returns the files that existed previously and are not present anymore. export const getStaleFiles = ( newFiles: string[], oldFiles: string[], + remoteFolder?: string, ): string[] => { - const staleFiles = new Set(oldFiles); + let filteredFiles = [...oldFiles]; + if (remoteFolder) { + filteredFiles = filteredFiles.filter(filePath => + filePath.match(remoteFolder), + ); + } + const staleFiles = new Set(filteredFiles); newFiles.forEach(newFile => { staleFiles.delete(newFile); }); From fedf69a396809146197255bb2327cebdf47d5b52 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Fri, 23 Jul 2021 20:39:20 +0200 Subject: [PATCH 04/16] fix(techdocs): remove unused imported module Signed-off-by: Camila Belo --- packages/techdocs-common/src/stages/publish/awsS3.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index 7306320fd2..3ba5ffe4e3 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -16,7 +16,7 @@ import { Entity, EntityName } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import aws, { Credentials } from 'aws-sdk'; -import { ListObjectsV2Output, ManagedUpload } from 'aws-sdk/clients/s3'; +import { ListObjectsV2Output } from 'aws-sdk/clients/s3'; import { CredentialsOptions } from 'aws-sdk/lib/credentials'; import express from 'express'; import fs from 'fs-extra'; From 6f9e155973077d9958d21c68250f2249f1f1532c Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Tue, 27 Jul 2021 14:48:06 +0200 Subject: [PATCH 05/16] fix(techdocs): make azure tests pass Co-authored-by: Eric Peterson Signed-off-by: Camila Belo --- .../__mocks__/@azure/storage-blob.ts | 4 +++ .../stages/publish/azureBlobStorage.test.ts | 23 +++++++--------- .../src/stages/publish/azureBlobStorage.ts | 27 ++++++++++++++++--- 3 files changed, 38 insertions(+), 16 deletions(-) diff --git a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts index 73c5873832..56574dedf7 100644 --- a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts +++ b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts @@ -125,6 +125,10 @@ export class ContainerClient { getBlockBlobClient(blobName: string) { return new BlockBlobClient(blobName); } + + listBlobsFlat() { + return []; + } } class ContainerClientFailGetProperties extends ContainerClient { diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index e55314a71b..8c31153201 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -64,10 +64,11 @@ const getEntityRootDir = (entity: Entity) => { const logger = getVoidLogger(); jest.spyOn(logger, 'info').mockReturnValue(logger); -jest.spyOn(logger, 'error').mockReturnValue(logger); +const loggerSpy = jest.spyOn(logger, 'error').mockReturnValue(logger); let publisher: PublisherBase; beforeEach(async () => { + loggerSpy.mockClear(); mockFs.restore(); const mockConfig = new ConfigReader({ techdocs: { @@ -175,14 +176,12 @@ describe('publishing with valid credentials', () => { directory: wrongPathToGeneratedDirectory, }); - await expect(fails).rejects.toMatchObject({ - message: expect.stringContaining( - `Unable to upload file(s) to Azure Blob Storage. Error: Failed to read template directory: ENOENT, no such file or directory`, - ), - }); - await expect(fails).rejects.toMatchObject({ - message: expect.stringContaining(wrongPathToGeneratedDirectory), - }); + await expect(fails).rejects.toThrow( + /Unable to upload file\(s\) to Azure. Error: Failed to read template directory: ENOENT, no such file or directory/, + ); + await expect(fails).rejects.toThrow( + new RegExp(wrongPathToGeneratedDirectory), + ); mockFs.restore(); }); @@ -225,13 +224,11 @@ describe('publishing with valid credentials', () => { error = e; } - expect(error.message).toContain( - `Unable to upload file(s) to Azure Blob Storage.`, - ); + expect(error.message).toContain(`Unable to upload file(s) to Azure`); expect(logger.error).toHaveBeenCalledWith( expect.stringContaining( - `Unable to upload file(s) to Azure Blob Storage. Error: Upload failed for ${path.join( + `Unable to upload file(s) to Azure. Error: Upload failed for ${path.join( entityRootDir, 'index.html', )} with status code 500`, diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index 5dc868ba2b..c7b86ed39e 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -16,6 +16,7 @@ import { DefaultAzureCredential } from '@azure/identity'; import { BlobServiceClient, + ContainerClient, StorageSharedKeyCredential, } from '@azure/storage-blob'; import { Entity, EntityName } from '@backstage/catalog-model'; @@ -139,8 +140,9 @@ export class AzureBlobStoragePublish implements PublisherBase { // First, retrieve a list of all individual files in currently existing const remoteFolder = getCloudPathForLocalPath(entity); const existingFiles: string[] = []; - const container = this.storageClient.getContainerClient(this.containerName); + let container: ContainerClient; try { + container = this.storageClient.getContainerClient(this.containerName); for await (const blob of container.listBlobsFlat()) { existingFiles.push(blob.name); } @@ -158,23 +160,42 @@ export class AzureBlobStoragePublish implements PublisherBase { // e.g. ['index.html', 'sub-page/index.html', 'assets/images/favicon.png'] absoluteFilesToUpload = await getFileTreeRecursively(directory); + const failedOperations: Error[] = []; await bulkStorageOperation( async absoluteFilePath => { // const relativeFilePath = path.relative(directory, absoluteFilePath); const relativeFilePath = path.normalize( path.relative(directory, absoluteFilePath), ); - return await this.storageClient - .getContainerClient(this.containerName) + const response = await container .getBlockBlobClient( getCloudPathForLocalPath(entity, relativeFilePath), ) .uploadFile(absoluteFilePath); + + if (response._response.status >= 400) { + failedOperations.push( + new Error( + `Upload failed for ${absoluteFilePath} with status code ${response._response.status}`, + ), + ); + } + + return response; }, absoluteFilesToUpload, { concurrencyLimit: BATCH_CONCURRENCY }, ); + if (failedOperations.length > 0) { + throw new Error( + failedOperations + .map(r => r.message) + .filter(Boolean) + .join(' '), + ); + } + this.logger.info( `Successfully uploaded all the generated files for Entity ${entity.metadata.name}. Total number of files: ${absoluteFilesToUpload.length}`, ); From caf519f69cc0139b34598244aaf01c8e321d689e Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Wed, 28 Jul 2021 11:46:33 +0200 Subject: [PATCH 06/16] fix(techdocs): make aws and gcs tests pass Co-authored-by: Eric Peterson Signed-off-by: Camila Belo --- .../__mocks__/@google-cloud/storage.ts | 11 +++++++ packages/techdocs-common/__mocks__/aws-sdk.ts | 8 +++++ .../src/stages/publish/googleStorage.ts | 31 ++++++++++++++++--- 3 files changed, 46 insertions(+), 4 deletions(-) diff --git a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts index 18fe62af14..ae2bce7a8f 100644 --- a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts +++ b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts @@ -107,6 +107,17 @@ class Bucket { file(destinationFilePath: string) { return new GCSFile(destinationFilePath); } + + getFilesStream() { + const readable = new Readable(); + readable._read = () => {}; + + process.nextTick(() => { + readable.emit('end'); + }); + + return readable; + } } export class Storage { diff --git a/packages/techdocs-common/__mocks__/aws-sdk.ts b/packages/techdocs-common/__mocks__/aws-sdk.ts index 2826a8d8bb..ec0273e277 100644 --- a/packages/techdocs-common/__mocks__/aws-sdk.ts +++ b/packages/techdocs-common/__mocks__/aws-sdk.ts @@ -104,6 +104,14 @@ export class S3 { }), }; } + + listObjectsV2() { + return { + promise: () => { + return Promise.resolve([]); + }, + }; + } } export default { diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index 349d6ead97..24f84f0136 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -15,7 +15,7 @@ */ import { Entity, EntityName } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; -import { FileExistsResponse, Storage } from '@google-cloud/storage'; +import { File, FileExistsResponse, Storage } from '@google-cloud/storage'; import express from 'express'; import JSON5 from 'json5'; import path from 'path'; @@ -119,9 +119,7 @@ export class GoogleGCSPublish implements PublisherBase { // First, retrieve a list of all individual files in currently existing const remoteFolder = getCloudPathForLocalPath(entity); - const existingFiles = ( - await bucket.getFiles({ prefix: remoteFolder }) - )[0].map(file => file.name); + const existingFiles = await this.getFilesForFolder(remoteFolder); // Then, merge new files into the same folder let absoluteFilesToUpload; @@ -276,4 +274,29 @@ export class GoogleGCSPublish implements PublisherBase { }); }); } + + private getFilesForFolder(folder: string): Promise { + const fileMetadataStream: Readable = this.storageClient + .bucket(this.bucketName) + .getFilesStream({ prefix: folder }); + + return new Promise((resolve, reject) => { + const files: string[] = []; + + fileMetadataStream.on('error', error => { + // push file to file array + reject(error); + }); + + fileMetadataStream.on('data', (file: File) => { + // push file to file array + files.push(file.name); + }); + + fileMetadataStream.on('end', () => { + // resolve promise + resolve(files); + }); + }); + } } From da705a74e0dd7aec23827fecbcc8efc4a907b582 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Mon, 2 Aug 2021 17:17:29 +0200 Subject: [PATCH 07/16] test(techdocs-common): cover stale files deletion for GCS Signed-off-by: Camila Belo --- .../__mocks__/@google-cloud/storage.ts | 13 +++++ .../src/stages/publish/googleStorage.test.ts | 58 +++++++++++-------- 2 files changed, 48 insertions(+), 23 deletions(-) diff --git a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts index ae2bce7a8f..f5616236cf 100644 --- a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts +++ b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts @@ -77,6 +77,10 @@ class GCSFile { }); return readable; } + + delete() { + return Promise.resolve(); + } } class Bucket { @@ -105,6 +109,9 @@ class Bucket { } file(destinationFilePath: string) { + if (this.bucketName === 'delete_stale_files_error') { + throw Error('Message'); + } return new GCSFile(destinationFilePath); } @@ -113,6 +120,12 @@ class Bucket { readable._read = () => {}; process.nextTick(() => { + if ( + this.bucketName === 'delete_stale_files_success' || + this.bucketName === 'delete_stale_files_error' + ) { + readable.emit('data', { name: 'stale-file.png' }); + } readable.emit('end'); }); diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts index 8a00fc9708..65cbf3c5ac 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.test.ts @@ -63,12 +63,10 @@ const getEntityRootDir = (entity: Entity) => { }; const logger = getVoidLogger(); -jest.spyOn(logger, 'info').mockReturnValue(logger); +const loggerInfoSpy = jest.spyOn(logger, 'info').mockReturnValue(logger); +const loggerErrorSpy = jest.spyOn(logger, 'error').mockReturnValue(logger); -let publisher: PublisherBase; - -beforeEach(async () => { - mockFs.restore(); +const createPublisherMock = (bucketName: string) => { const mockConfig = new ConfigReader({ techdocs: { requestUrl: 'http://localhost:7000', @@ -76,13 +74,21 @@ beforeEach(async () => { type: 'googleGcs', googleGcs: { credentials: '{}', - bucketName: 'bucketName', + bucketName, }, }, }, }); + return GoogleGCSPublish.fromConfig(mockConfig, logger); +}; - publisher = await GoogleGCSPublish.fromConfig(mockConfig, logger); +let publisher: PublisherBase; + +beforeEach(async () => { + loggerInfoSpy.mockClear(); + loggerErrorSpy.mockClear(); + mockFs.restore(); + publisher = createPublisherMock('bucketName'); }); describe('GoogleGCSPublish', () => { @@ -94,20 +100,7 @@ describe('GoogleGCSPublish', () => { }); it('should reject incorrect config', async () => { - const mockConfig = new ConfigReader({ - techdocs: { - requestUrl: 'http://localhost:7000', - publisher: { - type: 'googleGcs', - googleGcs: { - credentials: '{}', - bucketName: 'errorBucket', - }, - }, - }, - }); - - const errorPublisher = GoogleGCSPublish.fromConfig(mockConfig, logger); + const errorPublisher = createPublisherMock('errorBucket'); expect(await errorPublisher.getReadiness()).toEqual({ isAvailable: false, @@ -145,7 +138,6 @@ describe('GoogleGCSPublish', () => { directory: entityRootDir, }), ).toBeUndefined(); - mockFs.restore(); }); it('should fail to publish a directory', async () => { @@ -181,8 +173,28 @@ describe('GoogleGCSPublish', () => { await expect(fails).rejects.toMatchObject({ message: expect.stringContaining(wrongPathToGeneratedDirectory), }); + }); - mockFs.restore(); + it('should delete stale files after upload', async () => { + const entity = createMockEntity(); + const directory = getEntityRootDir(entity); + const bucketName = 'delete_stale_files_success'; + publisher = createPublisherMock(bucketName); + await publisher.publish({ entity, directory }); + expect(loggerInfoSpy).toHaveBeenLastCalledWith( + `Successfully deleted stale files for Entity ${entity.metadata.name}. Total number of files: 1`, + ); + }); + + it('should log error when the stale files deletion fails', async () => { + const entity = createMockEntity(); + const directory = getEntityRootDir(entity); + const bucketName = 'delete_stale_files_error'; + publisher = createPublisherMock(bucketName); + await publisher.publish({ entity, directory }); + expect(loggerErrorSpy).toHaveBeenLastCalledWith( + 'Unable to delete file(s) from Google Cloud Storage. Error: Message', + ); }); }); From 5386bcc989e2a45db39064d41161965d70703384 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Mon, 2 Aug 2021 19:41:36 +0200 Subject: [PATCH 08/16] test(techdocs-common): cover stale files deletion for AWS Signed-off-by: Camila Belo --- packages/techdocs-common/__mocks__/aws-sdk.ts | 23 ++++++- .../src/stages/publish/awsS3.test.ts | 61 +++++++++++-------- 2 files changed, 57 insertions(+), 27 deletions(-) diff --git a/packages/techdocs-common/__mocks__/aws-sdk.ts b/packages/techdocs-common/__mocks__/aws-sdk.ts index ec0273e277..9b3218f6dc 100644 --- a/packages/techdocs-common/__mocks__/aws-sdk.ts +++ b/packages/techdocs-common/__mocks__/aws-sdk.ts @@ -105,10 +105,29 @@ export class S3 { }; } - listObjectsV2() { + listObjectsV2({ Bucket }) { return { promise: () => { - return Promise.resolve([]); + if ( + Bucket === 'delete_stale_files_success' || + Bucket === 'delete_stale_files_error' + ) { + return Promise.resolve({ + Contents: [{ Key: 'stale_file.png' }], + }); + } + return Promise.resolve({}); + }, + }; + } + + deleteObject({ Bucket }) { + return { + promise: () => { + if (Bucket === 'delete_stale_files_error') { + throw new Error('Message'); + } + return Promise.resolve(); }, }; } diff --git a/packages/techdocs-common/src/stages/publish/awsS3.test.ts b/packages/techdocs-common/src/stages/publish/awsS3.test.ts index 81f06716b1..b94518c857 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.test.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.test.ts @@ -63,11 +63,10 @@ const getEntityRootDir = (entity: Entity) => { }; const logger = getVoidLogger(); +const loggerInfoSpy = jest.spyOn(logger, 'info'); +const loggerErrorSpy = jest.spyOn(logger, 'error'); -let publisher: PublisherBase; - -beforeEach(() => { - mockFs.restore(); +const createPublisherMock = (bucketName: string) => { const mockConfig = new ConfigReader({ techdocs: { requestUrl: 'http://localhost:7000', @@ -78,13 +77,22 @@ beforeEach(() => { accessKeyId: 'accessKeyId', secretAccessKey: 'secretAccessKey', }, - bucketName: 'bucketName', + bucketName, }, }, }, }); - publisher = AwsS3Publish.fromConfig(mockConfig, logger); + return AwsS3Publish.fromConfig(mockConfig, logger); +}; + +let publisher: PublisherBase; + +beforeEach(() => { + loggerInfoSpy.mockClear(); + loggerErrorSpy.mockClear(); + mockFs.restore(); + publisher = createPublisherMock('bucketName'); }); describe('AwsS3Publish', () => { @@ -96,24 +104,7 @@ describe('AwsS3Publish', () => { }); it('should reject incorrect config', async () => { - const mockConfig = new ConfigReader({ - techdocs: { - requestUrl: 'http://localhost:7000', - publisher: { - type: 'awsS3', - awsS3: { - credentials: { - accessKeyId: 'accessKeyId', - secretAccessKey: 'secretAccessKey', - }, - // this bucket name will throw an error - bucketName: 'errorBucket', - }, - }, - }, - }); - - const errorPublisher = AwsS3Publish.fromConfig(mockConfig, logger); + const errorPublisher = createPublisherMock('errorBucket'); expect(await errorPublisher.getReadiness()).toEqual({ isAvailable: false, @@ -188,8 +179,28 @@ describe('AwsS3Publish', () => { await expect(fails).rejects.toMatchObject({ message: expect.stringContaining(wrongPathToGeneratedDirectory), }); + }); - mockFs.restore(); + it('should delete stale files after upload', async () => { + const entity = createMockEntity(); + const directory = getEntityRootDir(entity); + const bucketName = 'delete_stale_files_success'; + publisher = createPublisherMock(bucketName); + await publisher.publish({ entity, directory }); + expect(loggerInfoSpy).toHaveBeenLastCalledWith( + `Successfully deleted stale files for Entity ${entity.metadata.name}. Total number of files: 1`, + ); + }); + + it('should log error when the stale files deletion fails', async () => { + const entity = createMockEntity(); + const directory = getEntityRootDir(entity); + const bucketName = 'delete_stale_files_error'; + publisher = createPublisherMock(bucketName); + await publisher.publish({ entity, directory }); + expect(loggerErrorSpy).toHaveBeenLastCalledWith( + 'Unable to delete file(s) from AWS S3. Error: Message', + ); }); }); From 9cf37164b1802687a7aa468645da5c70ab5f8a9a Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Mon, 2 Aug 2021 22:15:36 +0200 Subject: [PATCH 09/16] test(techdocs-common): cover stale files deletion for Azure Signed-off-by: Camila Belo --- .../__mocks__/@azure/storage-blob.ts | 14 ++- .../stages/publish/azureBlobStorage.test.ts | 103 +++++++++--------- .../src/stages/publish/azureBlobStorage.ts | 13 +-- .../src/stages/publish/helpers.ts | 9 +- 4 files changed, 72 insertions(+), 67 deletions(-) diff --git a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts index 56574dedf7..f7303ce1cb 100644 --- a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts +++ b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts @@ -126,9 +126,21 @@ export class ContainerClient { return new BlockBlobClient(blobName); } - listBlobsFlat() { + listBlobsFlat({ prefix }: { prefix: string }) { + if ( + this.containerName === 'delete_stale_files_success' || + this.containerName === 'delete_stale_files_error' + ) { + return [{ name: `${prefix}stale_file.png` }]; + } return []; } + + deleteBlob() { + if (this.containerName === 'delete_stale_files_error') { + throw new Error('Message'); + } + } } class ContainerClientFailGetProperties extends ContainerClient { diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index 8c31153201..2e17b43b62 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -63,13 +63,18 @@ const getEntityRootDir = (entity: Entity) => { }; const logger = getVoidLogger(); -jest.spyOn(logger, 'info').mockReturnValue(logger); -const loggerSpy = jest.spyOn(logger, 'error').mockReturnValue(logger); +const loggerInfoSpy = jest.spyOn(logger, 'info'); +const loggerErrorSpy = jest.spyOn(logger, 'error'); -let publisher: PublisherBase; -beforeEach(async () => { - loggerSpy.mockClear(); - mockFs.restore(); +const createPublisherMock = ({ + accountName = 'accountName', + containerName = 'containerName', +}: + | { + accountName?: string; + containerName?: string; + } + | undefined = {}) => { const mockConfig = new ConfigReader({ techdocs: { requestUrl: 'http://localhost:7000', @@ -77,16 +82,24 @@ beforeEach(async () => { type: 'azureBlobStorage', azureBlobStorage: { credentials: { - accountName: 'accountName', + accountName, accountKey: 'accountKey', }, - containerName: 'containerName', + containerName, }, }, }, }); - publisher = AzureBlobStoragePublish.fromConfig(mockConfig, logger); + return AzureBlobStoragePublish.fromConfig(mockConfig, logger); +}; + +let publisher: PublisherBase; +beforeEach(async () => { + loggerInfoSpy.mockClear(); + loggerErrorSpy.mockClear(); + mockFs.restore(); + publisher = createPublisherMock(); }); describe('publishing with valid credentials', () => { @@ -98,32 +111,15 @@ describe('publishing with valid credentials', () => { }); it('should reject incorrect config', async () => { - const mockConfig = new ConfigReader({ - techdocs: { - requestUrl: 'http://localhost:7000', - publisher: { - type: 'azureBlobStorage', - azureBlobStorage: { - credentials: { - accountName: 'accountName', - accountKey: 'accountKey', - }, - containerName: 'bad_container', - }, - }, - }, + const errorPublisher = createPublisherMock({ + containerName: 'bad_container', }); - const errorPublisher = await AzureBlobStoragePublish.fromConfig( - mockConfig, - logger, - ); - expect(await errorPublisher.getReadiness()).toEqual({ isAvailable: false, }); - expect(logger.error).toHaveBeenCalledWith( + expect(loggerErrorSpy).toHaveBeenCalledWith( expect.stringContaining( `Could not retrieve metadata about the Azure Blob Storage container bad_container.`, ), @@ -147,6 +143,10 @@ describe('publishing with valid credentials', () => { }); }); + afterEach(() => { + mockFs.restore(); + }); + it('should publish a directory', async () => { const entity = createMockEntity(); const entityRootDir = getEntityRootDir(entity); @@ -157,7 +157,6 @@ describe('publishing with valid credentials', () => { directory: entityRootDir, }), ).toBeUndefined(); - mockFs.restore(); }); it('should fail to publish a directory', async () => { @@ -182,29 +181,13 @@ describe('publishing with valid credentials', () => { await expect(fails).rejects.toThrow( new RegExp(wrongPathToGeneratedDirectory), ); - - mockFs.restore(); }); it('reports an error when bad account credentials', async () => { - const mockConfig = new ConfigReader({ - techdocs: { - requestUrl: 'http://localhost:7000', - publisher: { - type: 'azureBlobStorage', - azureBlobStorage: { - credentials: { - accountName: 'failupload', - accountKey: 'accountKey', - }, - containerName: 'containerName', - }, - }, - }, + publisher = createPublisherMock({ + accountName: 'failupload', }); - publisher = await AzureBlobStoragePublish.fromConfig(mockConfig, logger); - const entity = createMockEntity(); const entityRootDir = getEntityRootDir(entity); @@ -226,7 +209,7 @@ describe('publishing with valid credentials', () => { expect(error.message).toContain(`Unable to upload file(s) to Azure`); - expect(logger.error).toHaveBeenCalledWith( + expect(loggerErrorSpy).toHaveBeenCalledWith( expect.stringContaining( `Unable to upload file(s) to Azure. Error: Upload failed for ${path.join( entityRootDir, @@ -234,8 +217,28 @@ describe('publishing with valid credentials', () => { )} with status code 500`, ), ); + }); - mockFs.restore(); + it('should delete stale files after upload', async () => { + const entity = createMockEntity(); + const directory = getEntityRootDir(entity); + const containerName = 'delete_stale_files_success'; + publisher = createPublisherMock({ containerName }); + await publisher.publish({ entity, directory }); + expect(loggerInfoSpy).toHaveBeenLastCalledWith( + `Successfully deleted stale files for Entity ${entity.metadata.name}. Total number of files: 1`, + ); + }); + + it('should log error when the stale files deletion fails', async () => { + const entity = createMockEntity(); + const directory = getEntityRootDir(entity); + const containerName = 'delete_stale_files_error'; + publisher = createPublisherMock({ containerName }); + await publisher.publish({ entity, directory }); + expect(loggerErrorSpy).toHaveBeenLastCalledWith( + 'Unable to delete file(s) from Azure. Error: Message', + ); }); }); diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index c7b86ed39e..f49b317ecf 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -143,7 +143,9 @@ export class AzureBlobStoragePublish implements PublisherBase { let container: ContainerClient; try { container = this.storageClient.getContainerClient(this.containerName); - for await (const blob of container.listBlobsFlat()) { + for await (const blob of container.listBlobsFlat({ + prefix: remoteFolder, + })) { existingFiles.push(blob.name); } } catch (e) { @@ -163,7 +165,6 @@ export class AzureBlobStoragePublish implements PublisherBase { const failedOperations: Error[] = []; await bulkStorageOperation( async absoluteFilePath => { - // const relativeFilePath = path.relative(directory, absoluteFilePath); const relativeFilePath = path.normalize( path.relative(directory, absoluteFilePath), ); @@ -215,11 +216,7 @@ export class AzureBlobStoragePublish implements PublisherBase { ), ); - const staleFiles = getStaleFiles( - relativeFilesToUpload, - existingFiles, - remoteFolder, - ); + const staleFiles = getStaleFiles(relativeFilesToUpload, existingFiles); await bulkStorageOperation( async relativeFilePath => { @@ -233,7 +230,7 @@ export class AzureBlobStoragePublish implements PublisherBase { `Successfully deleted stale files for Entity ${entity.metadata.name}. Total number of files: ${staleFiles.length}`, ); } catch (error) { - const errorMessage = `Unable to delete file(s) from AWS S3. ${error}`; + const errorMessage = `Unable to delete file(s) from Azure. ${error}`; this.logger.error(errorMessage); } } diff --git a/packages/techdocs-common/src/stages/publish/helpers.ts b/packages/techdocs-common/src/stages/publish/helpers.ts index 2aabcb3259..6f6eb7dfa5 100644 --- a/packages/techdocs-common/src/stages/publish/helpers.ts +++ b/packages/techdocs-common/src/stages/publish/helpers.ts @@ -119,15 +119,8 @@ export const lowerCaseEntityTripletInStoragePath = ( export const getStaleFiles = ( newFiles: string[], oldFiles: string[], - remoteFolder?: string, ): string[] => { - let filteredFiles = [...oldFiles]; - if (remoteFolder) { - filteredFiles = filteredFiles.filter(filePath => - filePath.match(remoteFolder), - ); - } - const staleFiles = new Set(filteredFiles); + const staleFiles = new Set(oldFiles); newFiles.forEach(newFile => { staleFiles.delete(newFile); }); From ab9b315e2a748658d628dac0bdb31aefb1390661 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Tue, 3 Aug 2021 09:33:58 +0200 Subject: [PATCH 10/16] test(techdocs-common): cover getStaleFiles helper Signed-off-by: Camila Belo --- .../src/stages/publish/helpers.test.ts | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/packages/techdocs-common/src/stages/publish/helpers.test.ts b/packages/techdocs-common/src/stages/publish/helpers.test.ts index 27a56cd06d..63b4e0aeec 100644 --- a/packages/techdocs-common/src/stages/publish/helpers.test.ts +++ b/packages/techdocs-common/src/stages/publish/helpers.test.ts @@ -17,6 +17,7 @@ import mockFs from 'mock-fs'; import * as os from 'os'; import * as path from 'path'; import { + getStaleFiles, getFileTreeRecursively, getHeadersForFileExtension, lowerCaseEntityTripletInStoragePath, @@ -99,3 +100,27 @@ describe('lowerCaseEntityTripletInStoragePath', () => { ).toThrowError(error); }); }); + +describe('getStaleFiles', () => { + const defaultFiles = [ + 'default/Component/backstage/index.html', + 'default/Component/backstage/techdocs_metadata.json', + 'default/Component/backstage/assests/javascripts/bundle.7f4f3c92.min.js', + 'default/Component/backstage/assets/stylesheets/main.fe0cca5b.min.css', + ]; + + it('should return empty array if there is no stale file', () => { + const oldFiles = [...defaultFiles]; + const newFiles = [...defaultFiles]; + const staleFiles = getStaleFiles(newFiles, oldFiles); + expect(staleFiles).toHaveLength(0); + }); + + it('should return all stale files when they exists', () => { + const oldFiles = [...defaultFiles, 'stale_file.png']; + const newFiles = [...defaultFiles]; + const staleFiles = getStaleFiles(newFiles, oldFiles); + expect(staleFiles).toHaveLength(1); + expect(staleFiles).toEqual(expect.arrayContaining(['stale_file.png'])); + }); +}); From 7eef84a0404e43aebf440749fd25f8ec171e205e Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Tue, 3 Aug 2021 10:02:30 +0200 Subject: [PATCH 11/16] test(techdocs-common): cover getCloudPathForLocalPath helper Signed-off-by: Camila Belo --- .../src/stages/publish/helpers.test.ts | 44 +++++++++++++++++++ .../src/stages/publish/helpers.ts | 6 ++- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/helpers.test.ts b/packages/techdocs-common/src/stages/publish/helpers.test.ts index 63b4e0aeec..54761cf311 100644 --- a/packages/techdocs-common/src/stages/publish/helpers.test.ts +++ b/packages/techdocs-common/src/stages/publish/helpers.test.ts @@ -16,9 +16,11 @@ import mockFs from 'mock-fs'; import * as os from 'os'; import * as path from 'path'; +import { Entity, ENTITY_DEFAULT_NAMESPACE } from '@backstage/catalog-model'; import { getStaleFiles, getFileTreeRecursively, + getCloudPathForLocalPath, getHeadersForFileExtension, lowerCaseEntityTripletInStoragePath, } from './helpers'; @@ -124,3 +126,45 @@ describe('getStaleFiles', () => { expect(staleFiles).toEqual(expect.arrayContaining(['stale_file.png'])); }); }); + +describe('getCloudPathForLocalPath', () => { + const entity: Entity = { + apiVersion: 'version', + metadata: { namespace: 'default', name: 'backstage' }, + kind: 'Component', + }; + + it('should compose a remote bucket path including entity information', () => { + const remoteBucket = getCloudPathForLocalPath(entity); + expect(remoteBucket).toBe( + `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}/`, + ); + }); + + it('should compose a remote filename including entity information', () => { + const localPath = 'index.html'; + const remoteBucket = getCloudPathForLocalPath(entity, localPath); + expect(remoteBucket).toBe( + `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}/${localPath}`, + ); + }); + + it('should use the default namespace when it is undefined', () => { + const localPath = 'index.html'; + const { + kind, + metadata: { name }, + } = entity; + const remoteBucket = getCloudPathForLocalPath( + { kind, metadata: { name } } as Entity, + localPath, + ); + expect(remoteBucket).toBe( + `${ENTITY_DEFAULT_NAMESPACE}/${entity.kind}/${entity.metadata.name}/${localPath}`, + ); + }); + + it('should throw error when entity is invalid', () => { + expect(() => getCloudPathForLocalPath({} as Entity)).toThrow(); + }); +}); diff --git a/packages/techdocs-common/src/stages/publish/helpers.ts b/packages/techdocs-common/src/stages/publish/helpers.ts index 6f6eb7dfa5..fda03e7415 100644 --- a/packages/techdocs-common/src/stages/publish/helpers.ts +++ b/packages/techdocs-common/src/stages/publish/helpers.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Entity } from '@backstage/catalog-model'; +import { Entity, ENTITY_DEFAULT_NAMESPACE } from '@backstage/catalog-model'; import mime from 'mime-types'; import path from 'path'; import createLimiter from 'p-limit'; @@ -138,7 +138,9 @@ export const getCloudPathForLocalPath = ( const relativeFilePathPosix = localPath.split(path.sep).join(path.posix.sep); // The / delimiter is intentional since it represents the cloud storage and not the local file system. - const entityRootDir = `${entity.metadata.namespace}/${entity.kind}/${entity.metadata.name}`; + const entityRootDir = `${ + entity.metadata?.namespace ?? ENTITY_DEFAULT_NAMESPACE + }/${entity.kind}/${entity.metadata.name}`; return `${entityRootDir}/${relativeFilePathPosix}`; // GCS Bucket file relative path }; From 5486071aa8867653df75d158c15aed51c8967015 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Tue, 3 Aug 2021 11:52:19 +0200 Subject: [PATCH 12/16] test(techdocs-common): cover bulkStorageOperation helper Signed-off-by: Camila Belo --- .../src/stages/publish/helpers.test.ts | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/packages/techdocs-common/src/stages/publish/helpers.test.ts b/packages/techdocs-common/src/stages/publish/helpers.test.ts index 54761cf311..b288d221d3 100644 --- a/packages/techdocs-common/src/stages/publish/helpers.test.ts +++ b/packages/techdocs-common/src/stages/publish/helpers.test.ts @@ -22,6 +22,7 @@ import { getFileTreeRecursively, getCloudPathForLocalPath, getHeadersForFileExtension, + bulkStorageOperation, lowerCaseEntityTripletInStoragePath, } from './helpers'; @@ -168,3 +169,52 @@ describe('getCloudPathForLocalPath', () => { expect(() => getCloudPathForLocalPath({} as Entity)).toThrow(); }); }); + +describe('bulkStorageOperation', () => { + const length = 26; + const args = Array.from({ length }); + const createConcurrentRequestCounter = ( + callback: (count: number) => void, + ) => { + let count = 0; + return () => + new Promise(resolve => { + callback(++count); + setTimeout(() => { + count--; + resolve(null); + }, 100); + }); + }; + + it('should take care of rate limit by default', async () => { + const operation = createConcurrentRequestCounter((count: number) => { + expect(count <= 25).toBeTruthy(); + }); + await bulkStorageOperation(operation, args); + }); + + it('should accept the number of concurrency limit', async () => { + const concurrencyLimit = 10; + const operation = createConcurrentRequestCounter((count: number) => { + expect(count <= concurrencyLimit).toBeTruthy(); + }); + await bulkStorageOperation(operation, args, { concurrencyLimit }); + }); + + it('should wait for all promises be resolved', async () => { + const callback = jest.fn(); + const operation = createConcurrentRequestCounter(callback); + await bulkStorageOperation(operation, args); + expect(callback).toHaveBeenCalledTimes(length); + }); + + it('should call operation with the correct argument', async () => { + const files = ['file1.txt', 'file2.txt']; + const fn = jest.fn(); + await bulkStorageOperation(fn, files); + expect(fn).toHaveBeenCalledTimes(2); + expect(fn).toHaveBeenNthCalledWith(1, files[0]); + expect(fn).toHaveBeenNthCalledWith(2, files[1]); + }); +}); From 822240373ea0da6f016a5c9e7b5174438f78f1a0 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Tue, 3 Aug 2021 14:26:02 +0200 Subject: [PATCH 13/16] refactor(techdocs-common): use continuation token for listing blob files Co-authored-by: Eric Peterson Signed-off-by: Camila Belo --- .../src/stages/publish/azureBlobStorage.ts | 39 +++++++++++++++---- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index f49b317ecf..4fbe071d1c 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -139,15 +139,12 @@ export class AzureBlobStoragePublish implements PublisherBase { async publish({ entity, directory }: PublishRequest): Promise { // First, retrieve a list of all individual files in currently existing const remoteFolder = getCloudPathForLocalPath(entity); - const existingFiles: string[] = []; - let container: ContainerClient; + let existingFiles: string[]; try { - container = this.storageClient.getContainerClient(this.containerName); - for await (const blob of container.listBlobsFlat({ + existingFiles = await this.getAllBlobsFromContainer({ prefix: remoteFolder, - })) { - existingFiles.push(blob.name); - } + maxPageSize: BATCH_CONCURRENCY, + }); } catch (e) { const errorMessage = `Unable to list file(s) to Azure. ${e}`; this.logger.error(errorMessage); @@ -156,12 +153,14 @@ export class AzureBlobStoragePublish implements PublisherBase { // Then, merge new files into the same folder let absoluteFilesToUpload; + let container: ContainerClient; try { // Remove the absolute path prefix of the source directory // Path of all files to upload, relative to the root of the source directory // e.g. ['index.html', 'sub-page/index.html', 'assets/images/favicon.png'] absoluteFilesToUpload = await getFileTreeRecursively(directory); + container = this.storageClient.getContainerClient(this.containerName); const failedOperations: Error[] = []; await bulkStorageOperation( async absoluteFilePath => { @@ -381,4 +380,30 @@ export class AzureBlobStoragePublish implements PublisherBase { await Promise.all(promises); } + + protected async getAllBlobsFromContainer({ + prefix, + maxPageSize, + }: { + prefix: string; + maxPageSize: number; + }): Promise { + const blobs: string[] = []; + const container = this.storageClient.getContainerClient(this.containerName); + + let iterator = container.listBlobsFlat({ prefix }).byPage({ maxPageSize }); + let response = (await iterator.next()).value; + + do { + for (const blob of response.segment.blobItems) { + blobs.push(blob.name); + } + iterator = container + .listBlobsFlat({ prefix }) + .byPage({ continuationToken: response.continuationToken, maxPageSize }); + response = (await iterator.next()).value; + } while (response && response.continuationToken); + + return blobs; + } } From 11117811aa5897ea26309a5e867541dfbe1b8832 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Tue, 3 Aug 2021 14:53:53 +0200 Subject: [PATCH 14/16] test(techdocs-common): fix by page mock for azure Co-authored-by: Eric Peterson Signed-off-by: Camila Belo --- .../__mocks__/@azure/storage-blob.ts | 44 +++++++++++++++---- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts index f7303ce1cb..7279fc31eb 100644 --- a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts +++ b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts @@ -102,6 +102,36 @@ class BlockBlobClientFailUpload extends BlockBlobClient { } } +class ContainerClientIterator { + private containerName: string; + + constructor(containerName) { + this.containerName = containerName; + } + + async next() { + if ( + this.containerName === 'delete_stale_files_success' || + this.containerName === 'delete_stale_files_error' + ) { + return { + value: { + segment: { + blobItems: [{ name: `stale_file.png` }], + }, + }, + }; + } + return { + value: { + segment: { + blobItems: [], + }, + }, + }; + } +} + export class ContainerClient { private readonly containerName; @@ -126,14 +156,12 @@ export class ContainerClient { return new BlockBlobClient(blobName); } - listBlobsFlat({ prefix }: { prefix: string }) { - if ( - this.containerName === 'delete_stale_files_success' || - this.containerName === 'delete_stale_files_error' - ) { - return [{ name: `${prefix}stale_file.png` }]; - } - return []; + listBlobsFlat() { + return { + byPage: () => { + return new ContainerClientIterator(this.containerName); + }, + }; } deleteBlob() { From bc405be6edada0002b371a1691eae66043274026 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Tue, 3 Aug 2021 16:09:25 +0200 Subject: [PATCH 15/16] Changeset and docs updates. Co-authored-by: Camila Loiola Signed-off-by: Eric Peterson --- .changeset/techdocs-stale-breakfast-king.md | 19 +++++++++++++++++++ docs/features/techdocs/using-cloud-storage.md | 8 +++++++- 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 .changeset/techdocs-stale-breakfast-king.md diff --git a/.changeset/techdocs-stale-breakfast-king.md b/.changeset/techdocs-stale-breakfast-king.md new file mode 100644 index 0000000000..cdbdb4d0ad --- /dev/null +++ b/.changeset/techdocs-stale-breakfast-king.md @@ -0,0 +1,19 @@ +--- +'@backstage/techdocs-common': patch +--- + +Stale TechDocs content (files that had previously been published but which have +since been removed) is now removed from storage at publish-time. This is now +supported by the following publishers: + +- Google GCS +- AWS S3 +- Azure Blob Storage + +You may need to apply a greater level of permissions (e.g. the ability to +delete objects in your storage provider) to any credentials/accounts used by +the TechDocs CLI or TechDocs backend in order for this change to take effect. + +For more details, see [#6132][issue-ref]. + +[issue-ref]: https://github.com/backstage/backstage/issues/6132 diff --git a/docs/features/techdocs/using-cloud-storage.md b/docs/features/techdocs/using-cloud-storage.md index 159137c879..6d221bd9ad 100644 --- a/docs/features/techdocs/using-cloud-storage.md +++ b/docs/features/techdocs/using-cloud-storage.md @@ -61,7 +61,7 @@ If you do not prefer (3a) and optionally like to use a service account, you can follow these steps. Create a new Service Account and a key associated with it. In roles of the -service account, use "Storage Admin". +service account, use "Storage Object Admin". If you want to create a custom role, make sure to include both `get` and `create` permissions for both "Objects" and "Buckets". See @@ -143,6 +143,8 @@ permissions to: - `s3:ListBucket` to retrieve bucket metadata - `s3:PutObject` to upload files to the bucket +- `s3:DeleteObject` and `s3:DeleteObjectVersion` to delete stale content during + re-publishing To _read_ TechDocs from the S3 bucket the IAM policy needs to have at a minimum permissions to: @@ -345,6 +347,10 @@ techdocs: accountKey: ${TECHDOCS_AZURE_BLOB_STORAGE_ACCOUNT_KEY} ``` +In either case, the account or credentials used to access your container and all +TechDocs objects underneath it should have the `Storage Blog Data Owner` role +applied, in order to read, write, and delete objects as needed. + **4. That's it!** Your Backstage app is now ready to use Azure Blob Storage for TechDocs, to store From c2d6c8054db8de6fe33847b9869307cadf22b54f Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 9 Aug 2021 14:38:33 +0200 Subject: [PATCH 16/16] Publish should not be blocked on inability to list Signed-off-by: Eric Peterson --- .../techdocs-common/src/stages/publish/awsS3.ts | 17 ++++++++++++----- .../src/stages/publish/azureBlobStorage.ts | 12 ++++++------ .../src/stages/publish/googleStorage.ts | 13 ++++++++++--- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index 3ba5ffe4e3..02d4f7226f 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -178,11 +178,18 @@ export class AwsS3Publish implements PublisherBase { * Directory structure used in the bucket is - entityNamespace/entityKind/entityName/index.html */ async publish({ entity, directory }: PublishRequest): Promise { - // First, retrieve a list of all individual files in currently existing - const remoteFolder = getCloudPathForLocalPath(entity); - const existingFiles = await this.getAllObjectsFromBucket({ - prefix: remoteFolder, - }); + // First, try to retrieve a list of all individual files currently existing + let existingFiles: string[] = []; + try { + const remoteFolder = getCloudPathForLocalPath(entity); + existingFiles = await this.getAllObjectsFromBucket({ + prefix: remoteFolder, + }); + } catch (e) { + this.logger.error( + `Unable to list files for Entity ${entity.metadata.name}: ${e.message}`, + ); + } // Then, merge new files into the same folder let absoluteFilesToUpload; diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index 4fbe071d1c..f1c62f0819 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -137,18 +137,18 @@ export class AzureBlobStoragePublish implements PublisherBase { * Directory structure used in the container is - entityNamespace/entityKind/entityName/index.html */ async publish({ entity, directory }: PublishRequest): Promise { - // First, retrieve a list of all individual files in currently existing + // First, try to retrieve a list of all individual files currently existing const remoteFolder = getCloudPathForLocalPath(entity); - let existingFiles: string[]; + let existingFiles: string[] = []; try { existingFiles = await this.getAllBlobsFromContainer({ prefix: remoteFolder, maxPageSize: BATCH_CONCURRENCY, }); } catch (e) { - const errorMessage = `Unable to list file(s) to Azure. ${e}`; - this.logger.error(errorMessage); - throw new Error(errorMessage); + this.logger.error( + `Unable to list files for Entity ${entity.metadata.name}: ${e.message}`, + ); } // Then, merge new files into the same folder @@ -395,7 +395,7 @@ export class AzureBlobStoragePublish implements PublisherBase { let response = (await iterator.next()).value; do { - for (const blob of response.segment.blobItems) { + for (const blob of response?.segment?.blobItems ?? []) { blobs.push(blob.name); } iterator = container diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index 24f84f0136..9d8566f0b3 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -117,9 +117,16 @@ export class GoogleGCSPublish implements PublisherBase { async publish({ entity, directory }: PublishRequest): Promise { const bucket = this.storageClient.bucket(this.bucketName); - // First, retrieve a list of all individual files in currently existing - const remoteFolder = getCloudPathForLocalPath(entity); - const existingFiles = await this.getFilesForFolder(remoteFolder); + // First, try to retrieve a list of all individual files currently existing + let existingFiles: string[] = []; + try { + const remoteFolder = getCloudPathForLocalPath(entity); + existingFiles = await this.getFilesForFolder(remoteFolder); + } catch (e) { + this.logger.error( + `Unable to list files for Entity ${entity.metadata.name}: ${e.message}`, + ); + } // Then, merge new files into the same folder let absoluteFilesToUpload;