From 495a935f1fe8403ed4ef77f3529c8055aa59be0e Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Thu, 8 Jul 2021 19:17:55 +0200 Subject: [PATCH 01/14] Define migration interface. Signed-off-by: Eric Peterson --- .../src/stages/publish/types.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/techdocs-common/src/stages/publish/types.ts b/packages/techdocs-common/src/stages/publish/types.ts index a7f6ff7a45..229c853427 100644 --- a/packages/techdocs-common/src/stages/publish/types.ts +++ b/packages/techdocs-common/src/stages/publish/types.ts @@ -55,6 +55,19 @@ export type TechDocsMetadata = { etag: string; }; +export type MigrateRequest = { + /** + * Whether or not to remove the source file. Defaults to false (acting like a + * copy instead of a move). + */ + removeOriginal?: boolean; + + /** + * Maximum number of files/objects to migrate at once. Defaults to 25. + */ + concurrency?: number; +}; + /** * Base class for a TechDocs publisher (e.g. Local, Google GCS Bucket, AWS S3, etc.) * The publisher handles publishing of the generated static files after the prepare and generate steps of TechDocs. @@ -92,4 +105,14 @@ export interface PublisherBase { * Check if the index.html is present for the Entity at the Storage location. */ hasDocsBeenGenerated(entityName: Entity): Promise; + + /** + * Migrates documentation objects with case sensitive entity triplets to + * lowercase entity triplets. This was (will be) a change introduced in + * techdocs-cli v{0.x.y} and techdocs-backend v{0.x.y}. + * + * Implementation of this method is unnecessary in publishers introduced + * after v{0.x.y} of techdocs-common. + */ + migrateDocsCase?(migrateRequest: MigrateRequest): Promise; } From 8b653ba877fa9c8b9388bffeb9c2e3721d5c8798 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Thu, 8 Jul 2021 19:24:50 +0200 Subject: [PATCH 02/14] Local implementation just for illustrative purposes. Signed-off-by: Eric Peterson --- .../src/stages/publish/local.ts | 60 ++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/packages/techdocs-common/src/stages/publish/local.ts b/packages/techdocs-common/src/stages/publish/local.ts index 3ad0ed65ad..9bbc02e34f 100644 --- a/packages/techdocs-common/src/stages/publish/local.ts +++ b/packages/techdocs-common/src/stages/publish/local.ts @@ -22,6 +22,7 @@ import { Config } from '@backstage/config'; import express from 'express'; import fs from 'fs-extra'; import os from 'os'; +import createLimiter from 'p-limit'; import path from 'path'; import { Logger } from 'winston'; import { @@ -31,7 +32,7 @@ import { ReadinessResponse, TechDocsMetadata, } from './types'; -import { getHeadersForFileExtension } from './helpers'; +import { getFileTreeRecursively, getHeadersForFileExtension } from './helpers'; // TODO: Use a more persistent storage than node_modules or /tmp directory. // Make it configurable with techdocs.publisher.local.publishDirectory @@ -164,4 +165,61 @@ export class LocalPublish implements PublisherBase { return false; } } + + /** + * This code will never run in practice. It is merely here to illustrate how + * to implement this method for other storage providers. + */ + async migrateDocsCase({ + removeOriginal = false, + concurrency = 25, + }): Promise { + // Iterate through every file in the root of the publisher. + const files = await getFileTreeRecursively(staticDocsDir); + const limit = createLimiter(concurrency); + + await Promise.all( + files.map(f => + limit(async file => { + const relativeFile = file.replace(`${staticDocsDir}${path.sep}`, ''); + const [namespace, kind, name, ...parts] = relativeFile.split( + path.sep, + ); + const lowerNamespace = namespace.toLowerCase(); + const lowerKind = kind.toLowerCase(); + const lowerName = name.toLowerCase(); + + // If all parts are already lowercase, ignore. + if ( + namespace === lowerNamespace && + kind === lowerKind && + name === lowerName + ) { + return; + } + + // Otherwise, copy or move the file. + const newFile = [ + staticDocsDir, + lowerNamespace, + lowerKind, + lowerName, + ...parts, + ].join(path.sep); + await new Promise(resolve => { + const migrate = removeOriginal ? fs.move : fs.copyFile; + this.logger.debug(`Migrating ${relativeFile}`); + migrate(file, newFile, err => { + if (err) { + this.logger.warn( + `Unable to migrate ${relativeFile}: ${err.message}`, + ); + } + resolve(); + }); + }); + }, f), + ), + ); + } } From fd00a11bb48909c29d574afa85bf0b2d2b1625e4 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Thu, 8 Jul 2021 19:26:37 +0200 Subject: [PATCH 03/14] Provide a way to easily test the implementation. Signed-off-by: Eric Peterson --- .../techdocs-backend/src/service/router.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index 7563dfa2f9..a4b5d73650 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -246,6 +246,28 @@ export async function createRouter( } }); + /** + * TODO: Remove this endpoint before merging. This is just a convenient way + * to test publisher.migrateDocsCase() implementations. + */ + router.get('/do-not-commit/migrate', async (req, res) => { + if (publisher.migrateDocsCase) { + try { + await publisher.migrateDocsCase({ + // removeOriginal: !!req.query?.removeOriginal, + concurrency: + parseInt((req.query?.concurrency as string) || '', 10) || undefined, + }); + res.status(200).send('Good job'); + } catch (e) { + logger.error(e); + res.status(500).send('Uh-oh'); + } + } else { + res.status(400).send('Not valid'); + } + }); + // Route middleware which serves files from the storage set in the publisher. router.use('/static/docs', publisher.docsRouter()); From 8fa94e3accbaaeee9b82b16977b692373d6cd6d8 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Thu, 8 Jul 2021 19:31:46 +0200 Subject: [PATCH 04/14] Google Cloud Storage implementation Signed-off-by: Eric Peterson --- .../src/stages/publish/googleStorage.ts | 21 +++++ .../publish/migrations/GoogleMigration.ts | 80 +++++++++++++++++++ .../src/stages/publish/migrations/index.ts | 17 ++++ .../techdocs-backend/src/service/router.ts | 2 +- 4 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 packages/techdocs-common/src/stages/publish/migrations/GoogleMigration.ts create mode 100644 packages/techdocs-common/src/stages/publish/migrations/index.ts diff --git a/packages/techdocs-common/src/stages/publish/googleStorage.ts b/packages/techdocs-common/src/stages/publish/googleStorage.ts index bca7b86856..9def24722a 100644 --- a/packages/techdocs-common/src/stages/publish/googleStorage.ts +++ b/packages/techdocs-common/src/stages/publish/googleStorage.ts @@ -24,8 +24,10 @@ 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 { MigrateWriteStream } from './migrations'; import { PublisherBase, PublishRequest, @@ -235,4 +237,23 @@ export class GoogleGCSPublish implements PublisherBase { }); }); } + + migrateDocsCase({ removeOriginal = false, concurrency = 25 }): Promise { + return new Promise((resolve, reject) => { + // Iterate through every file in the root of the publisher. + const allFileMetadata: Readable = this.storageClient + .bucket(this.bucketName) + .getFilesStream(); + const migrateFiles = new MigrateWriteStream( + this.logger, + removeOriginal, + concurrency, + ); + migrateFiles.on('finish', resolve).on('error', reject); + allFileMetadata.pipe(migrateFiles).on('error', error => { + migrateFiles.destroy(); + reject(error); + }); + }); + } } diff --git a/packages/techdocs-common/src/stages/publish/migrations/GoogleMigration.ts b/packages/techdocs-common/src/stages/publish/migrations/GoogleMigration.ts new file mode 100644 index 0000000000..6bac7ace4a --- /dev/null +++ b/packages/techdocs-common/src/stages/publish/migrations/GoogleMigration.ts @@ -0,0 +1,80 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { File } from '@google-cloud/storage'; +import { Writable } from 'stream'; +import { Logger } from 'winston'; + +/** + * Writable stream to handle object copy/move operations. This implementation + * ensures we don't read in files from GCS faster than GCS can copy/move them. + */ +export class MigrateWriteStream extends Writable { + protected logger: Logger; + protected removeOriginal: boolean; + protected maxConcurrency: number; + protected inFlight = 0; + + constructor(logger: Logger, removeOriginal: boolean, concurrency: number) { + super({ objectMode: true }); + this.logger = logger; + this.removeOriginal = removeOriginal; + this.maxConcurrency = concurrency; + } + + _write(file: File, _encoding: BufferEncoding, next: Function) { + let shouldCallNext = true; + const [namespace, kind, name, ...parts] = file.name.split('/'); + const lowerNamespace = namespace.toLowerCase(); + const lowerKind = kind.toLowerCase(); + const lowerName = name.toLowerCase(); + + // If all parts are already lowercase, ignore. + if ( + namespace === lowerNamespace && + kind === lowerKind && + name === lowerName + ) { + next(); + return; + } + + // Allow up to n-many files to be migrated at a time. + this.inFlight++; + if (this.inFlight < this.maxConcurrency) { + next(); + shouldCallNext = false; + } + + // Otherwise, copy or move the file. + const newFile = [lowerNamespace, lowerKind, lowerName, ...parts].join('/'); + // todo: Use file.move instead of file.copy when removeOriginal is true. + const migrate = this.removeOriginal + ? file.copy.bind(file) + : file.copy.bind(file); + this.logger.debug(`Migrating ${file.name}`); + migrate(newFile) + .catch(e => + this.logger.warn(`Unable to migrate ${file.name}: ${e.message}`), + ) + .finally(() => { + this.inFlight--; + if (shouldCallNext) { + next(); + } + }); + } +} diff --git a/packages/techdocs-common/src/stages/publish/migrations/index.ts b/packages/techdocs-common/src/stages/publish/migrations/index.ts new file mode 100644 index 0000000000..f82ab054a4 --- /dev/null +++ b/packages/techdocs-common/src/stages/publish/migrations/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2021 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export { MigrateWriteStream } from './GoogleMigration'; diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index a4b5d73650..a5a4a3f647 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -256,7 +256,7 @@ export async function createRouter( await publisher.migrateDocsCase({ // removeOriginal: !!req.query?.removeOriginal, concurrency: - parseInt((req.query?.concurrency as string) || '', 10) || undefined, + parseInt((req.query?.concurrency as string) || '', 25) || undefined, }); res.status(200).send('Good job'); } catch (e) { From 6471d211234652716b6f89e668352dbda8d8b616 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 9 Jul 2021 14:04:08 +0200 Subject: [PATCH 05/14] Initial AWS migrate method. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Camila Belo Co-authored-by: Ruben Lindström Signed-off-by: Eric Peterson --- .../src/stages/publish/awsS3.ts | 90 ++++++++++++++++++- 1 file changed, 89 insertions(+), 1 deletion(-) diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index 15fe5a467f..4ffc3edae6 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 { ManagedUpload } from 'aws-sdk/clients/s3'; +import { ListObjectsV2Output, ManagedUpload } from 'aws-sdk/clients/s3'; import { CredentialsOptions } from 'aws-sdk/lib/credentials'; import express from 'express'; import fs from 'fs-extra'; @@ -308,4 +308,92 @@ export class AwsS3Publish implements PublisherBase { return Promise.resolve(false); } } + + /** + * todo: Put this documentation somewhere the user will actually see and read + * it. + * + * Note: In addition to the usual `PutObject`, `ListBucket` and `GetObject` + * actions, will need to add `PutObjectAcl` for copying files as well as + * `DeleteObject` if the removeOriginal flag is set to true. You may need to + * ensure access to the bucket resource in the following way: + * [ + * "arn:aws:s3:::your-bucket", + * "arn:aws:s3:::your-bucket/*" + * ] + */ + async migrateDocsCase({ + removeOriginal = false, + concurrency = 25, + }): Promise { + // Iterate through every file in the root of the publisher. + const allObjects = await this.getAllObjectsFromBucket(); + const limiter = createLimiter(concurrency); + await Promise.all( + allObjects.map(f => + limiter(async file => { + const [namespace, kind, name, ...parts] = file.split('/'); + const lowerNamespace = namespace.toLowerCase(); + const lowerKind = kind.toLowerCase(); + const lowerName = name.toLowerCase(); + + // If all parts are already lowercase, ignore. + if ( + namespace === lowerNamespace && + kind === lowerKind && + name === lowerName + ) { + return; + } + + try { + this.logger.debug(`Migrating ${file}`); + await this.storageClient + .copyObject({ + Bucket: this.bucketName, + CopySource: [this.bucketName, file].join('/'), + Key: [lowerNamespace, lowerKind, lowerName, ...parts].join('/'), + }) + .promise(); + + if (removeOriginal) { + await this.storageClient + .deleteObject({ + Bucket: this.bucketName, + Key: file, + }) + .promise(); + } + } catch (e) { + this.logger.warn(`Unable to migrate ${file}: ${e.message}`); + } + }, f), + ), + ); + } + + /** + * Returns a list of all object keys from the configured bucket. + */ + protected async getAllObjectsFromBucket(): 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, + }) + .promise(); + objects.push( + ...(allObjects.Contents || []).map(f => f.Key || '').filter(f => !!f), + ); + nextContinuation = allObjects.NextContinuationToken; + } while (nextContinuation); + + return objects; + } } From d4535da73341dc28e7782bf8681846a33e764dde Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Fri, 9 Jul 2021 17:30:16 +0200 Subject: [PATCH 06/14] Initial Azure migrate method Signed-off-by: Camila Belo --- .../src/stages/publish/azureBlobStorage.ts | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index 3ed75f532e..ddc70d5b46 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -289,4 +289,66 @@ export class AzureBlobStoragePublish implements PublisherBase { .getBlockBlobClient(`${entityRootDir}/index.html`) .exists(); } + + protected toLowerCase(originalName: string): string { + const [namespace, kind, name, ...parts] = originalName.split('/'); + const lowerNamespace = namespace.toLowerCase(); + const lowerKind = kind.toLowerCase(); + const lowerName = name.toLowerCase(); + return [lowerNamespace, lowerKind, lowerName, ...parts].join('/'); + } + + protected async renameBlob( + originalName: string, + newName: string, + removeOriginal = false, + ): Promise { + const container = this.storageClient.getContainerClient(this.containerName); + const blob = container.getBlobClient(newName); + const { url } = container.getBlobClient(originalName); + const response = await blob.beginCopyFromURL(url); + await response.pollUntilDone(); + if (removeOriginal) { + await container.deleteBlob(originalName); + } + } + + protected async renameBlobToLowerCase( + originalName: string, + removeOriginal: boolean, + ) { + const newName = this.toLowerCase(originalName); + if (originalName === newName) return; + try { + this.logger.debug(`Migrating ${originalName}`); + await this.renameBlob(originalName, newName, removeOriginal); + } catch (e) { + this.logger.warn(`Unable to migrate ${originalName}: ${e.message}`); + } + } + + /** + * todo: Put this documentation somewhere the user will actually see and read + * it. + */ + async migrateDocsCase({ + removeOriginal = false, + concurrency = 25, + }): Promise { + const promises = []; + const limiter = limiterFactory(concurrency); + const container = this.storageClient.getContainerClient(this.containerName); + + for await (const blob of container.listBlobsFlat()) { + promises.push( + limiter( + this.renameBlobToLowerCase.bind(this), + blob.name, + removeOriginal, + ), + ); + } + + await Promise.all(promises); + } } From 5ee76b076948479117bdbbfbce606ee8cb7dd35c Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Tue, 13 Jul 2021 13:27:58 +0200 Subject: [PATCH 07/14] Update API Reports Co-authored-by: Camila Belo Signed-off-by: Eric Peterson --- packages/techdocs-common/api-report.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/techdocs-common/api-report.md b/packages/techdocs-common/api-report.md index 8cca624e11..f3cfd29bcd 100644 --- a/packages/techdocs-common/api-report.md +++ b/packages/techdocs-common/api-report.md @@ -194,6 +194,7 @@ export interface PublisherBase { fetchTechDocsMetadata(entityName: EntityName): Promise; getReadiness(): Promise; hasDocsBeenGenerated(entityName: Entity): Promise; + migrateDocsCase?(migrateRequest: MigrateRequest): Promise; publish(request: PublishRequest): Promise; } From c23b09d52dd8623fbd6f37cea6568f0d4db65d4e Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 12 Jul 2021 11:12:06 +0200 Subject: [PATCH 08/14] Centralize lowercase logic to tested helper. Signed-off-by: Eric Peterson --- .../src/stages/publish/awsS3.ts | 19 ++++++-------- .../src/stages/publish/azureBlobStorage.ts | 26 ++++++++----------- .../src/stages/publish/helpers.test.ts | 20 +++++++++++++- .../src/stages/publish/helpers.ts | 21 +++++++++++++++ .../publish/migrations/GoogleMigration.ts | 13 +++------- 5 files changed, 62 insertions(+), 37 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index 4ffc3edae6..b53bd0ca0a 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -25,7 +25,11 @@ import createLimiter from 'p-limit'; import path from 'path'; import { Readable } from 'stream'; import { Logger } from 'winston'; -import { getFileTreeRecursively, getHeadersForFileExtension } from './helpers'; +import { + getFileTreeRecursively, + getHeadersForFileExtension, + lowerCaseEntityTripletInStoragePath, +} from './helpers'; import { PublisherBase, PublishRequest, @@ -332,17 +336,10 @@ export class AwsS3Publish implements PublisherBase { await Promise.all( allObjects.map(f => limiter(async file => { - const [namespace, kind, name, ...parts] = file.split('/'); - const lowerNamespace = namespace.toLowerCase(); - const lowerKind = kind.toLowerCase(); - const lowerName = name.toLowerCase(); + const newPath = lowerCaseEntityTripletInStoragePath(file); // If all parts are already lowercase, ignore. - if ( - namespace === lowerNamespace && - kind === lowerKind && - name === lowerName - ) { + if (file === newPath) { return; } @@ -352,7 +349,7 @@ export class AwsS3Publish implements PublisherBase { .copyObject({ Bucket: this.bucketName, CopySource: [this.bucketName, file].join('/'), - Key: [lowerNamespace, lowerKind, lowerName, ...parts].join('/'), + Key: newPath, }) .promise(); diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index ddc70d5b46..e94589b8df 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -25,7 +25,11 @@ import JSON5 from 'json5'; import limiterFactory from 'p-limit'; import { default as path, default as platformPath } from 'path'; import { Logger } from 'winston'; -import { getFileTreeRecursively, getHeadersForFileExtension } from './helpers'; +import { + getFileTreeRecursively, + getHeadersForFileExtension, + lowerCaseEntityTripletInStoragePath, +} from './helpers'; import { PublisherBase, PublishRequest, @@ -290,14 +294,6 @@ export class AzureBlobStoragePublish implements PublisherBase { .exists(); } - protected toLowerCase(originalName: string): string { - const [namespace, kind, name, ...parts] = originalName.split('/'); - const lowerNamespace = namespace.toLowerCase(); - const lowerKind = kind.toLowerCase(); - const lowerName = name.toLowerCase(); - return [lowerNamespace, lowerKind, lowerName, ...parts].join('/'); - } - protected async renameBlob( originalName: string, newName: string, @@ -314,16 +310,16 @@ export class AzureBlobStoragePublish implements PublisherBase { } protected async renameBlobToLowerCase( - originalName: string, + originalPath: string, removeOriginal: boolean, ) { - const newName = this.toLowerCase(originalName); - if (originalName === newName) return; + const newPath = lowerCaseEntityTripletInStoragePath(originalPath); + if (originalPath === newPath) return; try { - this.logger.debug(`Migrating ${originalName}`); - await this.renameBlob(originalName, newName, removeOriginal); + this.logger.debug(`Migrating ${originalPath}`); + await this.renameBlob(originalPath, newPath, removeOriginal); } catch (e) { - this.logger.warn(`Unable to migrate ${originalName}: ${e.message}`); + this.logger.warn(`Unable to migrate ${originalPath}: ${e.message}`); } } diff --git a/packages/techdocs-common/src/stages/publish/helpers.test.ts b/packages/techdocs-common/src/stages/publish/helpers.test.ts index cfb9c7930d..a650b59a54 100644 --- a/packages/techdocs-common/src/stages/publish/helpers.test.ts +++ b/packages/techdocs-common/src/stages/publish/helpers.test.ts @@ -16,7 +16,11 @@ import mockFs from 'mock-fs'; import * as os from 'os'; import * as path from 'path'; -import { getFileTreeRecursively, getHeadersForFileExtension } from './helpers'; +import { + getFileTreeRecursively, + getHeadersForFileExtension, + lowerCaseEntityTripletInStoragePath, +} from './helpers'; describe('getHeadersForFileExtension', () => { const correctMapOfExtensions = [ @@ -73,3 +77,17 @@ describe('getFileTreeRecursively', () => { expect(fileList).toContain(path.resolve(root, 'subDirA/file2')); }); }); + +describe('lowerCaseEntityTripletInStoragePath', () => { + it('returns lower-cased entity triplet path', () => { + const originalPath = 'default/Component/backstage/index.html'; + const actualPath = lowerCaseEntityTripletInStoragePath(originalPath); + expect(actualPath).toBe('default/component/backstage/index.html'); + }); + + it('does not lowercase beyond the triplet', () => { + const originalPath = 'default/Component/backstage/assets/IMAGE.png'; + const actualPath = lowerCaseEntityTripletInStoragePath(originalPath); + expect(actualPath).toBe('default/component/backstage/assets/IMAGE.png'); + }); +}); diff --git a/packages/techdocs-common/src/stages/publish/helpers.ts b/packages/techdocs-common/src/stages/publish/helpers.ts index eac5ba05c8..d26a4d71f1 100644 --- a/packages/techdocs-common/src/stages/publish/helpers.ts +++ b/packages/techdocs-common/src/stages/publish/helpers.ts @@ -82,3 +82,24 @@ export const getFileTreeRecursively = async ( }); return fileList; }; + +/** + * Returns the version of an object's storage path where the first three parts + * of the path (the entity triplet of namespace, kind, and name) are + * lower-cased. + * + * Path must not include a starting slash. + * + * @example + * lowerCaseEntityTripletInStoragePath('default/Component/backstage') + * // return default/component/backstage + */ +export const lowerCaseEntityTripletInStoragePath = ( + originalPath: string, +): string => { + const [namespace, kind, name, ...parts] = originalPath.split('/'); + const lowerNamespace = namespace.toLowerCase(); + const lowerKind = kind.toLowerCase(); + const lowerName = name.toLowerCase(); + return [lowerNamespace, lowerKind, lowerName, ...parts].join('/'); +}; diff --git a/packages/techdocs-common/src/stages/publish/migrations/GoogleMigration.ts b/packages/techdocs-common/src/stages/publish/migrations/GoogleMigration.ts index 6bac7ace4a..a2b5b8d719 100644 --- a/packages/techdocs-common/src/stages/publish/migrations/GoogleMigration.ts +++ b/packages/techdocs-common/src/stages/publish/migrations/GoogleMigration.ts @@ -17,6 +17,7 @@ import { File } from '@google-cloud/storage'; import { Writable } from 'stream'; import { Logger } from 'winston'; +import { lowerCaseEntityTripletInStoragePath } from '../helpers'; /** * Writable stream to handle object copy/move operations. This implementation @@ -37,17 +38,10 @@ export class MigrateWriteStream extends Writable { _write(file: File, _encoding: BufferEncoding, next: Function) { let shouldCallNext = true; - const [namespace, kind, name, ...parts] = file.name.split('/'); - const lowerNamespace = namespace.toLowerCase(); - const lowerKind = kind.toLowerCase(); - const lowerName = name.toLowerCase(); + const newFile = lowerCaseEntityTripletInStoragePath(file.name); // If all parts are already lowercase, ignore. - if ( - namespace === lowerNamespace && - kind === lowerKind && - name === lowerName - ) { + if (newFile === file.name) { next(); return; } @@ -60,7 +54,6 @@ export class MigrateWriteStream extends Writable { } // Otherwise, copy or move the file. - const newFile = [lowerNamespace, lowerKind, lowerName, ...parts].join('/'); // todo: Use file.move instead of file.copy when removeOriginal is true. const migrate = this.removeOriginal ? file.copy.bind(file) From e8302ea5a414f7936f2a581f97bb0174335da0a3 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 12 Jul 2021 11:12:39 +0200 Subject: [PATCH 09/14] Use move instead of copy in GCS migrate when requested. Signed-off-by: Eric Peterson --- .../src/stages/publish/migrations/GoogleMigration.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/migrations/GoogleMigration.ts b/packages/techdocs-common/src/stages/publish/migrations/GoogleMigration.ts index a2b5b8d719..f1dac0c6e6 100644 --- a/packages/techdocs-common/src/stages/publish/migrations/GoogleMigration.ts +++ b/packages/techdocs-common/src/stages/publish/migrations/GoogleMigration.ts @@ -54,9 +54,8 @@ export class MigrateWriteStream extends Writable { } // Otherwise, copy or move the file. - // todo: Use file.move instead of file.copy when removeOriginal is true. const migrate = this.removeOriginal - ? file.copy.bind(file) + ? file.move.bind(file) : file.copy.bind(file); this.logger.debug(`Migrating ${file.name}`); migrate(newFile) From 6a2b41ae24e9bf3cd638330fae69ad4fbad51ccb Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 12 Jul 2021 11:15:38 +0200 Subject: [PATCH 10/14] Remove temporary test router. Signed-off-by: Eric Peterson --- .../techdocs-backend/src/service/router.ts | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index a5a4a3f647..7563dfa2f9 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -246,28 +246,6 @@ export async function createRouter( } }); - /** - * TODO: Remove this endpoint before merging. This is just a convenient way - * to test publisher.migrateDocsCase() implementations. - */ - router.get('/do-not-commit/migrate', async (req, res) => { - if (publisher.migrateDocsCase) { - try { - await publisher.migrateDocsCase({ - // removeOriginal: !!req.query?.removeOriginal, - concurrency: - parseInt((req.query?.concurrency as string) || '', 25) || undefined, - }); - res.status(200).send('Good job'); - } catch (e) { - logger.error(e); - res.status(500).send('Uh-oh'); - } - } else { - res.status(400).send('Not valid'); - } - }); - // Route middleware which serves files from the storage set in the publisher. router.use('/static/docs', publisher.docsRouter()); From 567c95284ff796f3ef84ad91c0d65b2cdae458ad Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 12 Jul 2021 11:46:10 +0200 Subject: [PATCH 11/14] Move AWS policy docs to documentation page. Signed-off-by: Eric Peterson --- docs/features/techdocs/using-cloud-storage.md | 33 +++++++++++++++++++ .../src/stages/publish/awsS3.ts | 13 -------- .../src/stages/publish/azureBlobStorage.ts | 4 --- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/docs/features/techdocs/using-cloud-storage.md b/docs/features/techdocs/using-cloud-storage.md index 119629915e..159137c879 100644 --- a/docs/features/techdocs/using-cloud-storage.md +++ b/docs/features/techdocs/using-cloud-storage.md @@ -150,6 +150,39 @@ permissions to: - `s3:ListBucket` - To retrieve bucket metadata - `s3:GetObject` - To retrieve files from the bucket +> Note: If you need to migrate documentation objects from an older-style path +> format including case-sensitive entity metadata, you will need to add some +> additional permissions to be able to perform the migration, including: +> +> - `s3:PutBucketAcl` (for copying files, +> [more info here](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObjectAcl.html)) +> - `s3:DeleteObject` and `s3:DeleteObjectVersion` (for deleting migrated files, +> [more info here](https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObject.html)) +> +> ...And you will need to ensure the permissions apply to the bucket itself, as +> well as all resources under the bucket. See the example policy below. + +```json +{ + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "TechDocsWithMigration", + "Effect": "Allow", + "Action": [ + "s3:PutObject", + "s3:GetObject", + "s3:DeleteObjectVersion", + "s3:ListBucket", + "s3:DeleteObject", + "s3:PutObjectAcl" + ], + "Resource": ["arn:aws:s3:::your-bucket", "arn:aws:s3:::your-bucket/*"] + } + ] +} +``` + **4a. (Recommended) Setup authentication the AWS way, using environment variables** diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index b53bd0ca0a..55e968baac 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -313,19 +313,6 @@ export class AwsS3Publish implements PublisherBase { } } - /** - * todo: Put this documentation somewhere the user will actually see and read - * it. - * - * Note: In addition to the usual `PutObject`, `ListBucket` and `GetObject` - * actions, will need to add `PutObjectAcl` for copying files as well as - * `DeleteObject` if the removeOriginal flag is set to true. You may need to - * ensure access to the bucket resource in the following way: - * [ - * "arn:aws:s3:::your-bucket", - * "arn:aws:s3:::your-bucket/*" - * ] - */ async migrateDocsCase({ removeOriginal = false, concurrency = 25, diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index e94589b8df..4e012a40d5 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -323,10 +323,6 @@ export class AzureBlobStoragePublish implements PublisherBase { } } - /** - * todo: Put this documentation somewhere the user will actually see and read - * it. - */ async migrateDocsCase({ removeOriginal = false, concurrency = 25, From 94a54dd477e0e4b78c9aa9c09c252888a50a4d9e Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 12 Jul 2021 11:51:01 +0200 Subject: [PATCH 12/14] Changeset Signed-off-by: Eric Peterson --- .changeset/techdocs-migration.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 .changeset/techdocs-migration.md diff --git a/.changeset/techdocs-migration.md b/.changeset/techdocs-migration.md new file mode 100644 index 0000000000..939ace17a5 --- /dev/null +++ b/.changeset/techdocs-migration.md @@ -0,0 +1,13 @@ +--- +'@backstage/plugin-techdocs': minor +--- + +Added a `migrateDocsCase()` method to TechDocs publishers, along with +implementations for AWS, Azure, and GCS. + +This change is in support of a future update to TechDocs that will allow for +case-insensitive entity triplet URL access to documentation pages which will +require a migration of existing documentation objects in external storage +solutions. + +See [#4367](https://github.com/backstage/backstage/issues/4367) for details. From 34efce9c44ed2fd8414da77089e04f055405d361 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Mon, 12 Jul 2021 13:03:18 +0200 Subject: [PATCH 13/14] Apply helper to local, just to be consistent. Signed-off-by: Eric Peterson --- .../src/stages/publish/local.ts | 26 +++++-------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/local.ts b/packages/techdocs-common/src/stages/publish/local.ts index 9bbc02e34f..3414d273dd 100644 --- a/packages/techdocs-common/src/stages/publish/local.ts +++ b/packages/techdocs-common/src/stages/publish/local.ts @@ -32,7 +32,11 @@ import { ReadinessResponse, TechDocsMetadata, } from './types'; -import { getFileTreeRecursively, getHeadersForFileExtension } from './helpers'; +import { + getFileTreeRecursively, + getHeadersForFileExtension, + lowerCaseEntityTripletInStoragePath, +} from './helpers'; // TODO: Use a more persistent storage than node_modules or /tmp directory. // Make it configurable with techdocs.publisher.local.publishDirectory @@ -182,30 +186,14 @@ export class LocalPublish implements PublisherBase { files.map(f => limit(async file => { const relativeFile = file.replace(`${staticDocsDir}${path.sep}`, ''); - const [namespace, kind, name, ...parts] = relativeFile.split( - path.sep, - ); - const lowerNamespace = namespace.toLowerCase(); - const lowerKind = kind.toLowerCase(); - const lowerName = name.toLowerCase(); + const newFile = lowerCaseEntityTripletInStoragePath(relativeFile); // If all parts are already lowercase, ignore. - if ( - namespace === lowerNamespace && - kind === lowerKind && - name === lowerName - ) { + if (relativeFile === newFile) { return; } // Otherwise, copy or move the file. - const newFile = [ - staticDocsDir, - lowerNamespace, - lowerKind, - lowerName, - ...parts, - ].join(path.sep); await new Promise(resolve => { const migrate = removeOriginal ? fs.move : fs.copyFile; this.logger.debug(`Migrating ${relativeFile}`); From 3b3729f299588162a4cce0f94449b0bac9807d42 Mon Sep 17 00:00:00 2001 From: Camila Belo Date: Wed, 14 Jul 2021 13:46:06 +0200 Subject: [PATCH 14/14] Throws an error when there is no triplet Signed-off-by: Camila Belo --- packages/techdocs-common/src/stages/publish/awsS3.ts | 8 +++++++- .../src/stages/publish/azureBlobStorage.ts | 9 ++++++++- .../techdocs-common/src/stages/publish/helpers.test.ts | 8 ++++++++ packages/techdocs-common/src/stages/publish/helpers.ts | 8 ++++++++ .../src/stages/publish/migrations/GoogleMigration.ts | 9 ++++++++- 5 files changed, 39 insertions(+), 3 deletions(-) diff --git a/packages/techdocs-common/src/stages/publish/awsS3.ts b/packages/techdocs-common/src/stages/publish/awsS3.ts index 55e968baac..2942e33380 100644 --- a/packages/techdocs-common/src/stages/publish/awsS3.ts +++ b/packages/techdocs-common/src/stages/publish/awsS3.ts @@ -323,7 +323,13 @@ export class AwsS3Publish implements PublisherBase { await Promise.all( allObjects.map(f => limiter(async file => { - const newPath = lowerCaseEntityTripletInStoragePath(file); + let newPath; + try { + newPath = lowerCaseEntityTripletInStoragePath(file); + } catch (e) { + this.logger.warn(e.message); + return; + } // If all parts are already lowercase, ignore. if (file === newPath) { diff --git a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts index 4e012a40d5..7ee67a0016 100644 --- a/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts +++ b/packages/techdocs-common/src/stages/publish/azureBlobStorage.ts @@ -313,7 +313,14 @@ export class AzureBlobStoragePublish implements PublisherBase { originalPath: string, removeOriginal: boolean, ) { - const newPath = lowerCaseEntityTripletInStoragePath(originalPath); + let newPath; + try { + newPath = lowerCaseEntityTripletInStoragePath(originalPath); + } catch (e) { + this.logger.warn(e.message); + return; + } + if (originalPath === newPath) return; try { this.logger.debug(`Migrating ${originalPath}`); diff --git a/packages/techdocs-common/src/stages/publish/helpers.test.ts b/packages/techdocs-common/src/stages/publish/helpers.test.ts index a650b59a54..27a56cd06d 100644 --- a/packages/techdocs-common/src/stages/publish/helpers.test.ts +++ b/packages/techdocs-common/src/stages/publish/helpers.test.ts @@ -90,4 +90,12 @@ describe('lowerCaseEntityTripletInStoragePath', () => { const actualPath = lowerCaseEntityTripletInStoragePath(originalPath); expect(actualPath).toBe('default/component/backstage/assets/IMAGE.png'); }); + + it('throws error when there is no triplet', () => { + const originalPath = '/default/component/IMAGE.png'; + const error = `Encountered file unmanaged by TechDocs ${originalPath}. Skipping.`; + expect(() => + lowerCaseEntityTripletInStoragePath(originalPath), + ).toThrowError(error); + }); }); diff --git a/packages/techdocs-common/src/stages/publish/helpers.ts b/packages/techdocs-common/src/stages/publish/helpers.ts index d26a4d71f1..ed681eb8ba 100644 --- a/packages/techdocs-common/src/stages/publish/helpers.ts +++ b/packages/techdocs-common/src/stages/publish/helpers.ts @@ -97,6 +97,14 @@ export const getFileTreeRecursively = async ( export const lowerCaseEntityTripletInStoragePath = ( originalPath: string, ): string => { + const trimmedPath = + originalPath[0] === '/' ? originalPath.substring(1) : originalPath; + const matches = trimmedPath.match(/\//g) || []; + if (matches.length <= 2) { + throw new Error( + `Encountered file unmanaged by TechDocs ${originalPath}. Skipping.`, + ); + } const [namespace, kind, name, ...parts] = originalPath.split('/'); const lowerNamespace = namespace.toLowerCase(); const lowerKind = kind.toLowerCase(); diff --git a/packages/techdocs-common/src/stages/publish/migrations/GoogleMigration.ts b/packages/techdocs-common/src/stages/publish/migrations/GoogleMigration.ts index f1dac0c6e6..cc35b351ea 100644 --- a/packages/techdocs-common/src/stages/publish/migrations/GoogleMigration.ts +++ b/packages/techdocs-common/src/stages/publish/migrations/GoogleMigration.ts @@ -38,7 +38,14 @@ export class MigrateWriteStream extends Writable { _write(file: File, _encoding: BufferEncoding, next: Function) { let shouldCallNext = true; - const newFile = lowerCaseEntityTripletInStoragePath(file.name); + let newFile; + try { + newFile = lowerCaseEntityTripletInStoragePath(file.name); + } catch (e) { + this.logger.warn(e.message); + next(); + return; + } // If all parts are already lowercase, ignore. if (newFile === file.name) {