diff --git a/.changeset/short-dodos-sparkle.md b/.changeset/short-dodos-sparkle.md new file mode 100644 index 0000000000..342b8cc22b --- /dev/null +++ b/.changeset/short-dodos-sparkle.md @@ -0,0 +1,10 @@ +--- +'@backstage/plugin-catalog-backend-module-aws': patch +--- + +Fix S3 object URL creation at AwsS3EntityProvider by + +- handle absence of region config, +- handle regions with region-less URIs (us-east-1), +- apply URI encoding, +- and simplify the logic overall. diff --git a/.github/styles/vocab.txt b/.github/styles/vocab.txt index d2257f1b6f..95e54de3e0 100644 --- a/.github/styles/vocab.txt +++ b/.github/styles/vocab.txt @@ -339,6 +339,7 @@ untracked upsert upvote url +URIs URLs utils validator diff --git a/plugins/catalog-backend-module-aws/src/providers/AwsS3EntityProvider.test.ts b/plugins/catalog-backend-module-aws/src/providers/AwsS3EntityProvider.test.ts index 5e5901a770..b186b3d1b8 100644 --- a/plugins/catalog-backend-module-aws/src/providers/AwsS3EntityProvider.test.ts +++ b/plugins/catalog-backend-module-aws/src/providers/AwsS3EntityProvider.test.ts @@ -38,24 +38,8 @@ class PersistingTaskRunner implements TaskRunner { const logger = getVoidLogger(); describe('AwsS3EntityProvider', () => { - const config = new ConfigReader({ - catalog: { - providers: { - awsS3: { - anyProviderId: { - bucketName: 'bucket-1', - region: 'us-east-1', - prefix: 'sub/dir/', - }, - }, - }, - }, - }); - - const schedule = new PersistingTaskRunner(); - AWSMock.setSDKInstance(aws); - const createObjectList = (...keys: string[]): aws.S3.ObjectList => { + const createObjectList = (keys: string[]): aws.S3.ObjectList => { const objects = keys.map(key => { return { Key: key, @@ -65,24 +49,55 @@ describe('AwsS3EntityProvider', () => { return objects as aws.S3.ObjectList; }; + const keys = ['key1.yaml', 'key2.yaml', 'key3.yaml', 'key 4.yaml']; + AWSMock.mock('S3', 'listObjectsV2', async req => { const prefix = req.Prefix ?? ''; if (!req.ContinuationToken) { return { - Contents: createObjectList(`${prefix}key1.yaml`, `${prefix}key2.yaml`), + Contents: createObjectList( + keys + .slice(0, Math.ceil(keys.length / 2)) + .map(key => `${prefix}${key}`), + ), NextContinuationToken: 'next-token', } as aws.S3.Types.ListObjectsV2Output; } return { - Contents: createObjectList(`${prefix}key3.yaml`, `${prefix}key4.yaml`), + Contents: createObjectList( + keys.slice(Math.ceil(keys.length / 2)).map(key => `${prefix}${key}`), + ), } as aws.S3.Types.ListObjectsV2Output; }); - afterEach(() => jest.resetAllMocks()); + afterEach(() => { + jest.resetAllMocks(); + process.env.AWS_REGION = undefined; + }); - it('apply full update on scheduled execution', async () => { + const expectMutation = async ( + providerId: string, + providerConfig: object, + expectedBaseUrl: string, + names: Record, + integrationConfig?: object, + ) => { + const config = new ConfigReader({ + integrations: { + awsS3: integrationConfig ? [integrationConfig] : [], + }, + catalog: { + providers: { + awsS3: { + [providerId]: providerConfig, + }, + }, + }, + }); + + const schedule = new PersistingTaskRunner(); const entityProviderConnection: EntityProviderConnection = { applyMutation: jest.fn(), }; @@ -91,106 +106,121 @@ describe('AwsS3EntityProvider', () => { logger, schedule, })[0]; - expect(provider.getProviderName()).toEqual('awsS3-provider:anyProviderId'); + expect(provider.getProviderName()).toEqual(`awsS3-provider:${providerId}`); await provider.connect(entityProviderConnection); const taskDef = schedule.getTasks()[0]; - expect(taskDef.id).toEqual('awsS3-provider:anyProviderId:refresh'); + expect(taskDef.id).toEqual(`awsS3-provider:${providerId}:refresh`); await (taskDef.fn as () => Promise)(); + const expectedEntities = keys.map(key => { + const url = encodeURI(`${expectedBaseUrl}${key}`); + return { + entity: { + apiVersion: 'backstage.io/v1alpha1', + kind: 'Location', + metadata: { + annotations: { + 'backstage.io/managed-by-location': `url:${url}`, + 'backstage.io/managed-by-origin-location': `url:${url}`, + }, + name: names[key], + }, + spec: { + presence: 'required', + target: `${url}`, + type: 'url', + }, + }, + locationKey: `awsS3-provider:${providerId}`, + }; + }); + expect(entityProviderConnection.applyMutation).toBeCalledWith({ type: 'full', - entities: [ - { - entity: { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Location', - metadata: { - annotations: { - 'backstage.io/managed-by-location': - 'url:https://bucket-1.s3.us-east-1.amazonaws.com/sub/dir/key1.yaml', - 'backstage.io/managed-by-origin-location': - 'url:https://bucket-1.s3.us-east-1.amazonaws.com/sub/dir/key1.yaml', - }, - name: 'generated-980e6ad47fbfbfeead708a9c7c87331b7540296a', - }, - spec: { - presence: 'required', - target: - 'https://bucket-1.s3.us-east-1.amazonaws.com/sub/dir/key1.yaml', - type: 'url', - }, - }, - locationKey: 'awsS3-provider:anyProviderId', - }, - { - entity: { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Location', - metadata: { - annotations: { - 'backstage.io/managed-by-location': - 'url:https://bucket-1.s3.us-east-1.amazonaws.com/sub/dir/key2.yaml', - 'backstage.io/managed-by-origin-location': - 'url:https://bucket-1.s3.us-east-1.amazonaws.com/sub/dir/key2.yaml', - }, - name: 'generated-266794d8e789089dddba2b42cd79e70b149aa61c', - }, - spec: { - presence: 'required', - target: - 'https://bucket-1.s3.us-east-1.amazonaws.com/sub/dir/key2.yaml', - type: 'url', - }, - }, - locationKey: 'awsS3-provider:anyProviderId', - }, - { - entity: { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Location', - metadata: { - annotations: { - 'backstage.io/managed-by-location': - 'url:https://bucket-1.s3.us-east-1.amazonaws.com/sub/dir/key3.yaml', - 'backstage.io/managed-by-origin-location': - 'url:https://bucket-1.s3.us-east-1.amazonaws.com/sub/dir/key3.yaml', - }, - name: 'generated-96f0cdcd7e33aa687c19d160ec7d5b1975cb9ea1', - }, - spec: { - presence: 'required', - target: - 'https://bucket-1.s3.us-east-1.amazonaws.com/sub/dir/key3.yaml', - type: 'url', - }, - }, - locationKey: 'awsS3-provider:anyProviderId', - }, - { - entity: { - apiVersion: 'backstage.io/v1alpha1', - kind: 'Location', - metadata: { - annotations: { - 'backstage.io/managed-by-location': - 'url:https://bucket-1.s3.us-east-1.amazonaws.com/sub/dir/key4.yaml', - 'backstage.io/managed-by-origin-location': - 'url:https://bucket-1.s3.us-east-1.amazonaws.com/sub/dir/key4.yaml', - }, - name: 'generated-cd1a799b5ecfc055a0c672654420af3afeb648d3', - }, - spec: { - presence: 'required', - target: - 'https://bucket-1.s3.us-east-1.amazonaws.com/sub/dir/key4.yaml', - type: 'url', - }, - }, - locationKey: 'awsS3-provider:anyProviderId', - }, - ], + entities: expectedEntities, }); + }; + + // eslint-disable-next-line jest/expect-expect + it('apply full update on scheduled execution', async () => { + return expectMutation( + 'regionalStatic', + { + bucketName: 'bucket-1', + prefix: 'sub/dir/', + region: 'eu-west-1', + }, + 'https://s3.eu-west-1.amazonaws.com/bucket-1/sub/dir/', + { + 'key1.yaml': 'generated-7f6d5861b0b3401a38b5fe62e6c7ca11da5fd6d8', + 'key2.yaml': 'generated-a290be145586042af7d80715626399c9d661718d', + 'key3.yaml': 'generated-8d75f78ed9fa618ce433b226dc24eeab441f3a2d', + 'key 4.yaml': 'generated-1e0249dcb5805fc2ce6ac2d3c4d2a3ef4f1270c0', + }, + ); + }); + + // eslint-disable-next-line jest/expect-expect + it('us-east-1 has region-less URLs', async () => { + return expectMutation( + 'usEast1', + { + bucketName: 'bucket-1', + prefix: 'sub/dir/', + region: 'us-east-1', + }, + 'https://s3.amazonaws.com/bucket-1/sub/dir/', + { + 'key1.yaml': 'generated-f7e3f1c89f62cbf0d82db16452faaa7c040fc331', + 'key2.yaml': 'generated-925173f7dba1acaa73cac5ef4d10c3ed7660aa25', + 'key3.yaml': 'generated-d94cf017911ed7fb3be4a62a3dae1f5202879de3', + 'key 4.yaml': 'generated-f917fca0cfacc2be478ca2a3cff94d00917a1cde', + }, + ); + }); + + // eslint-disable-next-line jest/expect-expect + it('fallback region if absent', async () => { + // logic will use region information provided from + // profile < AWS_REGION < argument to client/command + process.env.AWS_REGION = 'eu-central-1'; + + return expectMutation( + 'absentRegion', + { + bucketName: 'bucket-1', + prefix: 'sub/dir/', + }, + 'https://s3.eu-central-1.amazonaws.com/bucket-1/sub/dir/', + { + 'key1.yaml': 'generated-285d144b5c1e24e801b97f61f794d566dabc7236', + 'key2.yaml': 'generated-36b78d7fa69690059797d4cc4f40c2a5eaed1d6d', + 'key3.yaml': 'generated-a39ad804bf65993cbc0f3810ff128a3c91daf768', + 'key 4.yaml': 'generated-bd7622ac86c313cb812d90e335b2f5f6bff14670', + }, + ); + }); + + // eslint-disable-next-line jest/expect-expect + it('custom endpoint', async () => { + return expectMutation( + 'customEndpoint', + { + bucketName: 'bucket-1', + prefix: 'sub/dir/', + }, + 'http://localhost:1234/bucket-1/sub/dir/', + { + 'key1.yaml': 'generated-e1f1dcfe44967b899a49d856cadcfa1ffc72a9f6', + 'key2.yaml': 'generated-6b09503942fe41566339f2eccfbb3380a022494b', + 'key3.yaml': 'generated-17274cdefac2feb3702f41605ca48aa4370d20a9', + 'key 4.yaml': 'generated-659f17a2429f107db04850f9081e3e9c56084aeb', + }, + { + endpoint: 'http://localhost:1234', + }, + ); }); }); diff --git a/plugins/catalog-backend-module-aws/src/providers/AwsS3EntityProvider.ts b/plugins/catalog-backend-module-aws/src/providers/AwsS3EntityProvider.ts index 2e64699d8b..cee5eea1a4 100644 --- a/plugins/catalog-backend-module-aws/src/providers/AwsS3EntityProvider.ts +++ b/plugins/catalog-backend-module-aws/src/providers/AwsS3EntityProvider.ts @@ -57,7 +57,7 @@ export class AwsS3EntityProvider implements EntityProvider { // Even though the awsS3 integration allows a config array // there is no *real* support for multiple configs. // Usually, there will be just the integration for the default host. - // In case, a config custom endpoint is used, the host from this endpoint + // In case, a custom endpoint is used, the host from this endpoint // will be extracted and used as host (e.g., localhost when used with LocalStack) // and the default integration will be added as second integration. // In this case, we still want the first one though, but have no means to select it @@ -80,7 +80,7 @@ export class AwsS3EntityProvider implements EntityProvider { private constructor( private readonly config: AwsS3Config, - private readonly integration: AwsS3Integration, + integration: AwsS3Integration, logger: Logger, schedule: TaskRunner, ) { @@ -196,16 +196,8 @@ export class AwsS3EntityProvider implements EntityProvider { private createObjectUrl(key: string): string { const bucketName = this.config.bucketName; - const endpoint = this.integration.config.endpoint; + const endpoint = this.s3.endpoint.href; - if (endpoint) { - if (endpoint.startsWith(`https://${bucketName}.`)) { - return `${endpoint}/${key}`; - } - - return `${endpoint}/${bucketName}/${key}`; - } - - return `https://${bucketName}.s3.${this.config.region}.amazonaws.com/${key}`; + return encodeURI(`${endpoint}${bucketName}/${key}`); } }