From 2049690461396a52e8a9ae59ed8a11cc89a0d8b3 Mon Sep 17 00:00:00 2001 From: Ravi Kiran Date: Fri, 10 Apr 2026 16:19:53 +0530 Subject: [PATCH 1/2] Allow partial results for get ec2 instance action --- .../actions/ec2/ec2-get-instances-action.ts | 75 +++++++++- .../test/ec2/ec2-get-instances-action.test.ts | 83 ++++++++++- .../src/lib/aws/ec2/ec2-get-instances.ts | 139 ++++++++++++++---- .../test/aws/ec2/ec2-get-instances.test.ts | 74 +++++++++- 4 files changed, 340 insertions(+), 31 deletions(-) diff --git a/packages/blocks/aws/src/lib/actions/ec2/ec2-get-instances-action.ts b/packages/blocks/aws/src/lib/actions/ec2/ec2-get-instances-action.ts index 70f1f37ad5..664fbe73d6 100644 --- a/packages/blocks/aws/src/lib/actions/ec2/ec2-get-instances-action.ts +++ b/packages/blocks/aws/src/lib/actions/ec2/ec2-get-instances-action.ts @@ -10,6 +10,7 @@ import { getAwsAccountsMultiSelectDropdown, getCredentialsListFromAuth, getEc2Instances, + getEc2InstancesWithPartialResults, groupARNsByRegion, parseArn, } from '@openops/common'; @@ -55,6 +56,13 @@ export const ec2GetInstancesAction = createAction({ }), ...filterTagsProperties(), dryRun: dryRunCheckBox(), + allowPartialResults: Property.Checkbox({ + displayName: 'Allow partial results', + description: + 'When enabled, the step returns { results, failedRegions } and continues when some regions fail.', + required: false, + defaultValue: false, + }), }, async run(context) { try { @@ -65,6 +73,7 @@ export const ec2GetInstancesAction = createAction({ tags, condition, dryRun, + allowPartialResults, } = context.propsValue; const filters: Filter[] = getFilters(context); const credentials = await getCredentialsListFromAuth( @@ -72,6 +81,64 @@ export const ec2GetInstancesAction = createAction({ accounts['accounts'], ); + const partial = allowPartialResults === true; + + if (partial) { + const partialPromises = []; + if (filterByARNs) { + const arns = convertToStringArrayWithValidation( + filterProperty['arns'] as unknown as string[], + 'Invalid input for ARNs: input should be a single string or an array of strings', + ); + const groupedARNs = groupARNsByRegion(arns); + + for (const region in groupedARNs) { + const arnsForRegion = groupedARNs[region]; + const instanceIdFilter = { + Name: 'instance-id', + Values: arnsForRegion.map((arn) => parseArn(arn).resourceId), + }; + partialPromises.push( + ...credentials.map((creds) => + getEc2InstancesWithPartialResults( + creds, + [region] as [string, ...string[]], + dryRun, + [...filters, instanceIdFilter], + ), + ), + ); + } + } else { + const regions = convertToStringArrayWithValidation( + filterProperty['regions'], + 'Invalid input for regions: input should be a single string or an array of strings', + ); + partialPromises.push( + ...credentials.map((creds) => + getEc2InstancesWithPartialResults( + creds, + regions as [string, ...string[]], + dryRun, + filters, + ), + ), + ); + } + + const partialOutcomes = await Promise.all(partialPromises); + let instances = partialOutcomes.flatMap((o) => o.results); + const failedRegions = partialOutcomes.flatMap((o) => o.failedRegions); + + if (tags?.length) { + instances = instances.filter((instance) => + filterTags((instance.Tags as AwsTag[]) ?? [], tags, condition), + ); + } + + return { results: instances, failedRegions }; + } + const promises: any[] = []; if (filterByARNs) { const arns = convertToStringArrayWithValidation( @@ -87,9 +154,9 @@ export const ec2GetInstancesAction = createAction({ Values: arnsForRegion.map((arn) => parseArn(arn).resourceId), }; promises.push( - ...credentials.map((credentials) => + ...credentials.map((creds) => getEc2Instances( - credentials, + creds, [region] as [string, ...string[]], dryRun, [...filters, instanceIdFilter], @@ -103,8 +170,8 @@ export const ec2GetInstancesAction = createAction({ 'Invalid input for regions: input should be a single string or an array of strings', ); promises.push( - ...credentials.map((credentials) => - getEc2Instances(credentials, regions, dryRun, filters), + ...credentials.map((creds) => + getEc2Instances(creds, regions, dryRun, filters), ), ); } diff --git a/packages/blocks/aws/test/ec2/ec2-get-instances-action.test.ts b/packages/blocks/aws/test/ec2/ec2-get-instances-action.test.ts index 1936898f2b..653dca6b83 100644 --- a/packages/blocks/aws/test/ec2/ec2-get-instances-action.test.ts +++ b/packages/blocks/aws/test/ec2/ec2-get-instances-action.test.ts @@ -2,6 +2,7 @@ const openopsCommonMock = { ...jest.requireActual('@openops/common'), getCredentialsListFromAuth: jest.fn(), getEc2Instances: jest.fn(), + getEc2InstancesWithPartialResults: jest.fn(), dryRunCheckBox: jest.fn().mockReturnValue({ required: false, defaultValue: false, @@ -52,7 +53,7 @@ describe('ec2GetInstancesAction', () => { }; test('should create action with input regions property', () => { - expect(Object.keys(ec2GetInstancesAction.props).length).toBe(8); + expect(Object.keys(ec2GetInstancesAction.props).length).toBe(9); expect(ec2GetInstancesAction.props).toMatchObject({ accounts: { required: true, @@ -79,6 +80,11 @@ describe('ec2GetInstancesAction', () => { defaultValue: false, type: 'CHECKBOX', }, + allowPartialResults: { + required: false, + defaultValue: false, + type: 'CHECKBOX', + }, filterByARNs: { type: 'CHECKBOX', }, @@ -514,4 +520,79 @@ describe('ec2GetInstancesAction', () => { [{ Name: 'instance-id', Values: ['i-2', 'i-4'] }], ); }); + + test('when allowPartialResults, uses partial helper and returns object shape', async () => { + openopsCommonMock.getEc2InstancesWithPartialResults.mockResolvedValue({ + results: [{ instance_id: 'i-1' }], + failedRegions: [ + { region: 'eu-west-1', accountId: '111', error: 'AccessDenied' }, + ], + }); + + const context = { + ...jest.requireActual('@openops/blocks-framework'), + auth: auth, + propsValue: { + filterProperty: { regions: ['us-east-1', 'eu-west-1'] }, + dryRun: false, + accounts: {}, + allowPartialResults: true, + }, + }; + + const result = (await ec2GetInstancesAction.run(context)) as any; + + expect(result).toEqual({ + results: [{ instance_id: 'i-1' }], + failedRegions: [ + { region: 'eu-west-1', accountId: '111', error: 'AccessDenied' }, + ], + }); + expect( + openopsCommonMock.getEc2InstancesWithPartialResults, + ).toHaveBeenCalledWith( + 'credentials', + ['us-east-1', 'eu-west-1'], + false, + [], + ); + expect(openopsCommonMock.getEc2Instances).not.toHaveBeenCalled(); + }); + + test('when allowPartialResults, aggregates multiple credentials', async () => { + openopsCommonMock.getCredentialsListFromAuth.mockResolvedValue([ + 'cred-a', + 'cred-b', + ]); + openopsCommonMock.getEc2InstancesWithPartialResults + .mockResolvedValueOnce({ + results: [{ id: 'a' }], + failedRegions: [], + }) + .mockResolvedValueOnce({ + results: [{ id: 'b' }], + failedRegions: [{ region: 'us-west-2', accountId: '2', error: 'e' }], + }); + + const context = { + ...jest.requireActual('@openops/blocks-framework'), + auth: auth, + propsValue: { + filterProperty: { regions: ['us-east-1'] }, + dryRun: true, + accounts: {}, + allowPartialResults: true, + }, + }; + + const result = (await ec2GetInstancesAction.run(context)) as any; + + expect(result.results).toEqual([{ id: 'a' }, { id: 'b' }]); + expect(result.failedRegions).toEqual([ + { region: 'us-west-2', accountId: '2', error: 'e' }, + ]); + expect( + openopsCommonMock.getEc2InstancesWithPartialResults, + ).toHaveBeenCalledTimes(2); + }); }); diff --git a/packages/openops/src/lib/aws/ec2/ec2-get-instances.ts b/packages/openops/src/lib/aws/ec2/ec2-get-instances.ts index 8927786dc7..6a6672d2c9 100644 --- a/packages/openops/src/lib/aws/ec2/ec2-get-instances.ts +++ b/packages/openops/src/lib/aws/ec2/ec2-get-instances.ts @@ -4,6 +4,55 @@ import { getAwsClient } from '../get-client'; import { getAccountName } from '../organizations-common'; import { getAccountId } from '../sts-common'; +export type AwsPartialFetchFailedRegion = { + region: string; + accountId?: string; + error: string; +}; + +export type AwsPartialFetchResult = { + results: T[]; + failedRegions: AwsPartialFetchFailedRegion[]; +}; + +export function formatAwsPartialFetchError(error: unknown): string { + if (error instanceof Error) { + return error.message; + } + return String(error); +} + +async function describeInstancesInRegion( + credentials: any, + region: string, + dryRun: boolean, + filters: EC2.Filter[] | undefined, + accountId: string, + accountName?: string, +): Promise { + const ec2 = getAwsClient(EC2.EC2, credentials, region) as EC2.EC2; + + const command = new EC2.DescribeInstancesCommand({ + Filters: filters, + DryRun: dryRun, + }); + const { Reservations } = await ec2.send(command); + + return ( + Reservations?.flatMap( + (reservation) => + reservation.Instances?.map((instance) => + mapInstanceToOpenOpsEc2Instance( + instance, + region, + accountId, + accountName, + ), + ) || [], + ) || [] + ); +} + export async function getEc2Instances( credentials: any, regions: [string, ...string[]], @@ -13,36 +62,76 @@ export async function getEc2Instances( const accountId = await getAccountId(credentials, regions[0]); const accountName = await getAccountName(credentials, regions[0], accountId); - const fetchInstancesInRegion = async (region: string): Promise => { - const ec2 = getAwsClient(EC2.EC2, credentials, region) as EC2.EC2; - - const command = new EC2.DescribeInstancesCommand({ - Filters: filters, - DryRun: dryRun, - }); - const { Reservations } = await ec2.send(command); - - return ( - Reservations?.flatMap( - (reservation) => - reservation.Instances?.map((instance) => - mapInstanceToOpenOpsEc2Instance( - instance, - region, - accountId, - accountName, - ), - ) || [], - ) || [] - ); - }; - const instancesFromAllRegions = await Promise.all( - regions.map(fetchInstancesInRegion), + regions.map((region) => + describeInstancesInRegion( + credentials, + region, + dryRun, + filters, + accountId, + accountName, + ), + ), ); return instancesFromAllRegions.flat(); } +export async function getEc2InstancesWithPartialResults( + credentials: any, + regions: [string, ...string[]], + dryRun: boolean, + filters?: EC2.Filter[], +): Promise> { + let accountId: string; + let accountName: string | undefined; + + try { + accountId = await getAccountId(credentials, regions[0]); + accountName = await getAccountName(credentials, regions[0], accountId); + } catch (error) { + const message = formatAwsPartialFetchError(error); + return { + results: [], + failedRegions: regions.map((region) => ({ + region, + error: message, + })), + }; + } + + const settled = await Promise.allSettled( + regions.map((region) => + describeInstancesInRegion( + credentials, + region, + dryRun, + filters, + accountId, + accountName, + ), + ), + ); + + const results: any[] = []; + const failedRegions: AwsPartialFetchFailedRegion[] = []; + + settled.forEach((outcome, index) => { + const region = regions[index]; + if (outcome.status === 'fulfilled') { + results.push(...outcome.value); + } else { + failedRegions.push({ + region, + accountId, + error: formatAwsPartialFetchError(outcome.reason), + }); + } + }); + + return { results, failedRegions }; +} + function mapInstanceToOpenOpsEc2Instance( instance: EC2.Instance, region: string, diff --git a/packages/openops/test/aws/ec2/ec2-get-instances.test.ts b/packages/openops/test/aws/ec2/ec2-get-instances.test.ts index 2145da3eeb..3a4117b1f6 100644 --- a/packages/openops/test/aws/ec2/ec2-get-instances.test.ts +++ b/packages/openops/test/aws/ec2/ec2-get-instances.test.ts @@ -28,7 +28,10 @@ const getAwsClientMock = { jest.mock('../../../src/lib/aws/get-client', () => getAwsClientMock); import * as EC2 from '@aws-sdk/client-ec2'; -import { getEc2Instances } from '../../../src/lib/aws/ec2/ec2-get-instances'; +import { + getEc2Instances, + getEc2InstancesWithPartialResults, +} from '../../../src/lib/aws/ec2/ec2-get-instances'; describe('getEc2Instances', () => { beforeEach(() => { @@ -173,3 +176,72 @@ describe('getEc2Instances', () => { }); }); }); + +describe('getEc2InstancesWithPartialResults', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + test('returns instances from successful regions and skips failed regions', async () => { + const sendMock = jest + .fn() + .mockResolvedValueOnce({ + Reservations: [ + { + Instances: [ + { InstanceId: 'ok-instance', InstanceType: 'c1.medium' }, + ], + }, + ], + }) + .mockRejectedValueOnce(new Error('UnauthorizedOperation')); + + getAwsClientMock.getAwsClient.mockImplementation(() => ({ + send: sendMock, + })); + + const result = await getEc2InstancesWithPartialResults( + CREDENTIALS, + ['some-region1', 'some-region2'], + false, + [], + ); + + expect(result.results).toMatchObject([ + { + instance_id: 'ok-instance', + region: 'some-region1', + account_id: ACCOUNT_ID, + }, + ]); + expect(result.failedRegions).toEqual([ + { + region: 'some-region2', + accountId: ACCOUNT_ID, + error: 'UnauthorizedOperation', + }, + ]); + expect(sendMock).toHaveBeenCalledTimes(2); + }); + + test('when getAccountId fails, marks all regions failed without accountId', async () => { + const { getAccountId } = jest.requireMock( + '../../../src/lib/aws/sts-common', + ) as { getAccountId: jest.Mock }; + getAccountId.mockRejectedValueOnce(new Error('STS failed')); + + const result = await getEc2InstancesWithPartialResults( + CREDENTIALS, + ['r1', 'r2'], + false, + [], + ); + + expect(result.results).toEqual([]); + expect(result.failedRegions).toEqual([ + { region: 'r1', error: 'STS failed' }, + { region: 'r2', error: 'STS failed' }, + ]); + expect(getAwsClientMock.getAwsClient).not.toHaveBeenCalled(); + }); +}); From c8e49397f24f05b5dc3add8b3a7f49786593557e Mon Sep 17 00:00:00 2001 From: Ravi Kiran Date: Fri, 10 Apr 2026 16:33:16 +0530 Subject: [PATCH 2/2] Remove duplicated code in branches --- .../actions/ec2/ec2-get-instances-action.ts | 155 +++++++++--------- 1 file changed, 77 insertions(+), 78 deletions(-) diff --git a/packages/blocks/aws/src/lib/actions/ec2/ec2-get-instances-action.ts b/packages/blocks/aws/src/lib/actions/ec2/ec2-get-instances-action.ts index 664fbe73d6..c48167bca4 100644 --- a/packages/blocks/aws/src/lib/actions/ec2/ec2-get-instances-action.ts +++ b/packages/blocks/aws/src/lib/actions/ec2/ec2-get-instances-action.ts @@ -82,51 +82,24 @@ export const ec2GetInstancesAction = createAction({ ); const partial = allowPartialResults === true; + const batches = buildEc2GetInstancesBatches( + filterByARNs, + filterProperty, + credentials, + filters, + ); if (partial) { - const partialPromises = []; - if (filterByARNs) { - const arns = convertToStringArrayWithValidation( - filterProperty['arns'] as unknown as string[], - 'Invalid input for ARNs: input should be a single string or an array of strings', - ); - const groupedARNs = groupARNsByRegion(arns); - - for (const region in groupedARNs) { - const arnsForRegion = groupedARNs[region]; - const instanceIdFilter = { - Name: 'instance-id', - Values: arnsForRegion.map((arn) => parseArn(arn).resourceId), - }; - partialPromises.push( - ...credentials.map((creds) => - getEc2InstancesWithPartialResults( - creds, - [region] as [string, ...string[]], - dryRun, - [...filters, instanceIdFilter], - ), - ), - ); - } - } else { - const regions = convertToStringArrayWithValidation( - filterProperty['regions'], - 'Invalid input for regions: input should be a single string or an array of strings', - ); - partialPromises.push( - ...credentials.map((creds) => - getEc2InstancesWithPartialResults( - creds, - regions as [string, ...string[]], - dryRun, - filters, - ), + const partialOutcomes = await Promise.all( + batches.map((batch) => + getEc2InstancesWithPartialResults( + batch.creds, + batch.regions, + dryRun, + batch.fetchFilters, ), - ); - } - - const partialOutcomes = await Promise.all(partialPromises); + ), + ); let instances = partialOutcomes.flatMap((o) => o.results); const failedRegions = partialOutcomes.flatMap((o) => o.failedRegions); @@ -139,44 +112,18 @@ export const ec2GetInstancesAction = createAction({ return { results: instances, failedRegions }; } - const promises: any[] = []; - if (filterByARNs) { - const arns = convertToStringArrayWithValidation( - filterProperty['arns'] as unknown as string[], - 'Invalid input for ARNs: input should be a single string or an array of strings', - ); - const groupedARNs = groupARNsByRegion(arns); - - for (const region in groupedARNs) { - const arnsForRegion = groupedARNs[region]; - const instanceIdFilter = { - Name: 'instance-id', - Values: arnsForRegion.map((arn) => parseArn(arn).resourceId), - }; - promises.push( - ...credentials.map((creds) => - getEc2Instances( - creds, - [region] as [string, ...string[]], - dryRun, - [...filters, instanceIdFilter], - ), + const instances = ( + await Promise.all( + batches.map((batch) => + getEc2Instances( + batch.creds, + batch.regions, + dryRun, + batch.fetchFilters, ), - ); - } - } else { - const regions = convertToStringArrayWithValidation( - filterProperty['regions'], - 'Invalid input for regions: input should be a single string or an array of strings', - ); - promises.push( - ...credentials.map((creds) => - getEc2Instances(creds, regions, dryRun, filters), ), - ); - } - - const instances = (await Promise.all(promises)).flat(); + ) + ).flat(); if (tags?.length) { return instances.filter((instance) => @@ -193,6 +140,58 @@ export const ec2GetInstancesAction = createAction({ }, }); +type Ec2GetInstancesBatch = { + creds: any; + regions: [string, ...string[]]; + fetchFilters: Filter[]; +}; + +function buildEc2GetInstancesBatches( + filterByARNs: boolean, + filterProperty: Record, + credentials: any[], + filters: Filter[], +): Ec2GetInstancesBatch[] { + const batches: Ec2GetInstancesBatch[] = []; + + if (filterByARNs) { + const arns = convertToStringArrayWithValidation( + filterProperty['arns'] as unknown as string[], + 'Invalid input for ARNs: input should be a single string or an array of strings', + ); + const groupedARNs = groupARNsByRegion(arns); + + for (const region in groupedARNs) { + const arnsForRegion = groupedARNs[region]; + const instanceIdFilter: Filter = { + Name: 'instance-id', + Values: arnsForRegion.map((arn) => parseArn(arn).resourceId), + }; + for (const creds of credentials) { + batches.push({ + creds, + regions: [region] as [string, ...string[]], + fetchFilters: [...filters, instanceIdFilter], + }); + } + } + } else { + const regions = convertToStringArrayWithValidation( + filterProperty['regions'], + 'Invalid input for regions: input should be a single string or an array of strings', + ); + for (const creds of credentials) { + batches.push({ + creds, + regions: regions as [string, ...string[]], + fetchFilters: filters, + }); + } + } + + return batches; +} + function getFilters(context: any): Filter[] { const filters: Filter[] = [];