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 diff --git a/packages/techdocs-common/__mocks__/@azure/storage-blob.ts b/packages/techdocs-common/__mocks__/@azure/storage-blob.ts index 73c5873832..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; @@ -125,6 +155,20 @@ export class ContainerClient { getBlockBlobClient(blobName: string) { return new BlockBlobClient(blobName); } + + listBlobsFlat() { + return { + byPage: () => { + return new ContainerClientIterator(this.containerName); + }, + }; + } + + deleteBlob() { + if (this.containerName === 'delete_stale_files_error') { + throw new Error('Message'); + } + } } class ContainerClientFailGetProperties extends ContainerClient { diff --git a/packages/techdocs-common/__mocks__/@google-cloud/storage.ts b/packages/techdocs-common/__mocks__/@google-cloud/storage.ts index 18fe62af14..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,8 +109,28 @@ class Bucket { } file(destinationFilePath: string) { + if (this.bucketName === 'delete_stale_files_error') { + throw Error('Message'); + } return new GCSFile(destinationFilePath); } + + getFilesStream() { + const readable = new Readable(); + 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'); + }); + + 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..9b3218f6dc 100644 --- a/packages/techdocs-common/__mocks__/aws-sdk.ts +++ b/packages/techdocs-common/__mocks__/aws-sdk.ts @@ -104,6 +104,33 @@ export class S3 { }), }; } + + listObjectsV2({ Bucket }) { + return { + promise: () => { + 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(); + }, + }; + } } export default { 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', + ); }); }); diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index 7bdc91ffd4..02d4f7226f 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'; @@ -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,84 @@ export class AwsS3Publish implements PublisherBase { * Directory structure used in the bucket is - entityNamespace/entityKind/entityName/index.html */ async publish({ entity, directory }: PublishRequest): Promise { + // First, try to retrieve a list of all individual files currently existing + let existingFiles: string[] = []; 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); + 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}`, + ); + } - 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); + // 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); - // 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 +398,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( diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts index e55314a71b..2e17b43b62 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.test.ts @@ -63,12 +63,18 @@ const getEntityRootDir = (entity: Entity) => { }; const logger = getVoidLogger(); -jest.spyOn(logger, 'info').mockReturnValue(logger); -jest.spyOn(logger, 'error').mockReturnValue(logger); +const loggerInfoSpy = jest.spyOn(logger, 'info'); +const loggerErrorSpy = jest.spyOn(logger, 'error'); -let publisher: PublisherBase; -beforeEach(async () => { - mockFs.restore(); +const createPublisherMock = ({ + accountName = 'accountName', + containerName = 'containerName', +}: + | { + accountName?: string; + containerName?: string; + } + | undefined = {}) => { const mockConfig = new ConfigReader({ techdocs: { requestUrl: 'http://localhost:7000', @@ -76,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', () => { @@ -97,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.`, ), @@ -146,6 +143,10 @@ describe('publishing with valid credentials', () => { }); }); + afterEach(() => { + mockFs.restore(); + }); + it('should publish a directory', async () => { const entity = createMockEntity(); const entityRootDir = getEntityRootDir(entity); @@ -156,7 +157,6 @@ describe('publishing with valid credentials', () => { directory: entityRootDir, }), ).toBeUndefined(); - mockFs.restore(); }); it('should fail to publish a directory', async () => { @@ -175,37 +175,19 @@ 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), - }); - - mockFs.restore(); + 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), + ); }); 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); @@ -225,20 +207,38 @@ 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(loggerErrorSpy).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`, ), ); + }); - 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 fe8fe9c2ce..f1c62f0819 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'; @@ -26,8 +27,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 +137,101 @@ export class AzureBlobStoragePublish implements PublisherBase { * Directory structure used in the container is - entityNamespace/entityKind/entityName/index.html */ async publish({ entity, directory }: PublishRequest): Promise { + // First, try to retrieve a list of all individual files currently existing + const remoteFolder = getCloudPathForLocalPath(entity); + let existingFiles: string[] = []; 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); + existingFiles = await this.getAllBlobsFromContainer({ + prefix: remoteFolder, + maxPageSize: BATCH_CONCURRENCY, + }); + } catch (e) { + this.logger.error( + `Unable to list files for Entity ${entity.metadata.name}: ${e.message}`, + ); + } - // 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); + // 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); - 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); + container = this.storageClient.getContainerClient(this.containerName); + const failedOperations: Error[] = []; + await bulkStorageOperation( + async absoluteFilePath => { + const relativeFilePath = path.normalize( + path.relative(directory, absoluteFilePath), + ); + const response = await container + .getBlockBlobClient( + getCloudPathForLocalPath(entity, relativeFilePath), + ) + .uploadFile(absoluteFilePath); if (response._response.status >= 400) { - return { - ...response, - error: new Error( - `Upload failed for ${sourceFilePath} with status code ${response._response.status}`, + failedOperations.push( + new Error( + `Upload failed for ${absoluteFilePath} with status code ${response._response.status}`, ), - }; + ); } - return { - ...response, - error: undefined, - }; - }); - }); - const responses = await Promise.all(promises); + return response; + }, + absoluteFilesToUpload, + { concurrencyLimit: BATCH_CONCURRENCY }, + ); - 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 { + if (failedOperations.length > 0) { throw new Error( - failed - .map(r => r.error?.message) + 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}`, + ); } catch (e) { - const errorMessage = `Unable to upload file(s) to Azure Blob Storage. ${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); + + 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 Azure. ${error}`; + this.logger.error(errorMessage); + } } private download(containerName: string, blobPath: string): Promise { @@ -350,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; + } } 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', + ); }); }); diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index cbf76cd3d7..49156e07f2 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 { File, 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,73 @@ 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, try to retrieve a list of all individual files currently existing + let existingFiles: string[] = []; 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); + 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}`, + ); + } - 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); + // 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); - // 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 { @@ -255,4 +280,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); + }); + }); + } } diff --git a/packages/techdocs-common/src/stages/publish/helpers.test.ts b/packages/techdocs-common/src/stages/publish/helpers.test.ts index 27a56cd06d..b288d221d3 100644 --- a/packages/techdocs-common/src/stages/publish/helpers.test.ts +++ b/packages/techdocs-common/src/stages/publish/helpers.test.ts @@ -16,9 +16,13 @@ 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, + bulkStorageOperation, lowerCaseEntityTripletInStoragePath, } from './helpers'; @@ -99,3 +103,118 @@ 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'])); + }); +}); + +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(); + }); +}); + +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]); + }); +}); diff --git a/packages/techdocs-common/src/stages/publish/helpers.ts b/packages/techdocs-common/src/stages/publish/helpers.ts index ed681eb8ba..fda03e7415 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, ENTITY_DEFAULT_NAMESPACE } from '@backstage/catalog-model'; import mime from 'mime-types'; +import path from 'path'; +import createLimiter from 'p-limit'; import recursiveReadDir from 'recursive-readdir'; /** @@ -111,3 +114,42 @@ export const lowerCaseEntityTripletInStoragePath = ( 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_DEFAULT_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))); +};