fix: more integration tests

src/aws-provider/aws-organization-reader.ts

@@ -1,6 +1,7 @@
 import * as IAM from '@aws-sdk/client-iam';
 import * as Organizations from '@aws-sdk/client-organizations';
 import * as Support from '@aws-sdk/client-support';
+import * as STS from '@aws-sdk/client-sts';
 import { AwsUtil } from '../util/aws-util';
 import { ConsoleUtil } from '../util/console-util';
 import { GetOrganizationAccessRoleInTargetAccount, ICrossAccountConfig } from './aws-account-access';
@@ -256,7 +257,7 @@ export class AwsOrganizationReader {
                             ]);
 
                         } catch (err) {
-                            if (err.name === 'AccessDenied') {
+                            if (err instanceof STS.STSServiceException) {
                                 ConsoleUtil.LogWarning(`AccessDenied: unable to log into account ${acc.Id}. This might have various causes, to troubleshoot:`
                                     + '\nhttps://github.com/OlafConijn/AwsOrganizationFormation/blob/master/docs/access-denied.md');
                             } else {
@@ -313,7 +314,7 @@ export class AwsOrganizationReader {
             }
             return 'developer';
         } catch (err) {
-            if (err.name === 'SubscriptionRequiredException') {
+            if (err instanceof Support.SupportServiceException) {
                 return 'basic';
             }
             throw err;

src/build-tasks/build-configuration.ts

@@ -160,11 +160,7 @@ export class BuildConfiguration {
         try {
             if (organizationFileLocation.startsWith('s3://')) {
                 if (textTemplatingContext) { throw new Error('Text templating context is not supported on s3 hosted organization files'); }
-                const s3client = new S3Client({
-                    credentials: AwsUtil.credentialsProvider,
-                    region: AwsUtil.GetDefaultRegion(),
-                    followRegionRedirects: true,
-                }); // we don't know which role to assume yet....
+                const s3client = AwsUtil.GetS3Service(); // we don't know which role to assume yet....
                 const bucketAndKey = organizationFileLocation.substring(5);
                 const bucketAndKeySplit = bucketAndKey.split('/');
                 const response = await s3client.send(

src/commands/base-command.ts

@@ -185,7 +185,7 @@ export abstract class BaseCliCommand<T extends ICommandArgs> {
         if (partitionCredentials) {
             const partitionMasterAccountId = await AwsUtil.GetPartitionMasterAccountId();
             const partitionCrossAccountConfig = { masterAccountId: partitionMasterAccountId, masterAccountRoleName: roleInMasterAccount };
-            const partitionOrgService = await AwsUtil.GetOrganizationsService(partitionMasterAccountId, roleInMasterAccount, null, true);
+            const partitionOrgService = AwsUtil.GetOrganizationsService(partitionMasterAccountId, roleInMasterAccount, null, true);
             partitionReader = new AwsOrganizationReader(partitionOrgService, partitionCrossAccountConfig);
             partitionOrganization = new AwsOrganization(partitionReader);
             await partitionOrganization.initialize();

src/commands/init-organization-pipeline.ts

@@ -134,7 +134,8 @@ export class InitPipelineCommand extends BaseCliCommand<IInitPipelineCommandArgs
             ResourcePrefix: resourcePrefix,
             ...additionalParameters,
         };
-        await InitialCommitUtil.parameterizeAndUpload(templateDefinition, parameters, template, stateBucketName, this.s3credentials);
+        const s3client = AwsUtil.GetS3Service(this.buildAccountId);
+        await InitialCommitUtil.parameterizeAndUpload(templateDefinition, parameters, template, stateBucketName, s3client);
 
     }
     private async createInitialCommitFromResources(path: string, template: DefaultTemplate, resourcePrefix: string, stateBucketName: string, stackName: string, repositoryName: string, region: string, command: IInitPipelineCommandArgs): Promise<void> {
@@ -185,7 +186,7 @@ export class InitPipelineCommand extends BaseCliCommand<IInitPipelineCommandArgs
     public uploadInitialCommit(stateBucketName: string, initialCommitPath: string, templateContents: string, buildSpecContents: string, organizationTasksContents: string, cloudformationTemplateContents: string, orgParametersInclude: string, buildAccessRoleTemplateContents: string): Promise<void> {
         return new Promise((resolve, reject) => {
             try {
-                const s3client = new S3.S3Client({ ...(this.s3credentials ? { credentials: this.s3credentials } : {}), region: AwsUtil.GetDefaultRegion(), followRegionRedirects: true });
+                const s3client = AwsUtil.GetS3Service(this.buildAccountId);
                 const output = new WritableStream();
                 const archive = archiver('zip');
 

src/state/storage-provider.ts

@@ -23,7 +23,6 @@ export class S3StorageProvider implements IStorageProvider {
 
     public readonly bucketName: string;
     public readonly objectKey: string;
-    private readonly credentials: ClientCredentialsConfig;
     private readonly region: string;
     public dontPut = false;
 
@@ -39,7 +38,6 @@ export class S3StorageProvider implements IStorageProvider {
 
         this.bucketName = stateBucketName;
         this.objectKey = stateObject;
-        this.credentials = credentials ?? AwsUtil.credentialsProvider;
         this.region = region ? region : defaultRegion;
     }
 
@@ -91,7 +89,7 @@ export class S3StorageProvider implements IStorageProvider {
 
     public async get(): Promise<string | undefined> {
 
-        const s3client = AwsUtil.GetS3Service(undefined, this.region);
+        const s3client = AwsUtil.GetS3Service();
         const request: S3.GetObjectCommandInput = {
             Bucket: this.bucketName,
             Key: this.objectKey,
@@ -109,7 +107,7 @@ export class S3StorageProvider implements IStorageProvider {
             if (err && err.code === 'NoSuchBucket') {
                 return undefined;
             }
-            throw err;
+                        throw err;
         }
 
     }

src/util/aws-util.ts

@@ -279,7 +279,7 @@ export class AwsUtil {
             isPartition,
         });
         const config: OrganizationsClientConfig = {
-            region: (isPartition) ? this.partitionRegion : 'us-east-1',
+            region: (isPartition) ? this.partitionRegion : AwsUtil.GetDefaultRegion(),
             credentials: provider,
             defaultsMode: 'standard',
             retryMode: 'standard',
@@ -301,7 +301,7 @@ export class AwsUtil {
             isPartition,
         });
         const config: EC2ClientConfig = {
-            region: (isPartition) ? this.partitionRegion : region ?? 'us-east-1',
+            region: (isPartition) ? this.partitionRegion : region ?? AwsUtil.GetDefaultRegion(),
             credentials: provider,
             defaultsMode: 'standard',
             retryMode: 'standard',
@@ -320,8 +320,8 @@ export class AwsUtil {
             accountId,
             roleInTargetAccount,
         });
-        const config: S3ClientConfig = {
-            region: region ?? 'us-east-1',
+                const config: S3ClientConfig = {
+            region: region ?? AwsUtil.GetDefaultRegion(),
             followRegionRedirects: true,
             credentials: provider,
             defaultsMode: 'standard',
@@ -344,7 +344,7 @@ export class AwsUtil {
             isPartition,
         });
         const config: SupportClientConfig = {
-            region: (isPartition) ? this.partitionRegion : 'us-east-1',
+            region: (isPartition) ? this.partitionRegion : AwsUtil.GetDefaultRegion(),
             credentials: provider,
             defaultsMode: 'standard',
             retryMode: 'standard',
@@ -366,7 +366,7 @@ export class AwsUtil {
             isPartition,
         });
         const config: IAMClientConfig = {
-            region: (isPartition) ? this.partitionRegion : 'us-east-1',
+            region: (isPartition) ? this.partitionRegion : AwsUtil.GetDefaultRegion(),
             credentials: provider,
             defaultsMode: 'standard',
             retryMode: 'standard',
@@ -395,7 +395,7 @@ export class AwsUtil {
             isPartition,
         });
         const config: CloudFormationClientConfig = {
-            region: (isPartition) ? this.partitionRegion : region,
+            region: (isPartition) ? this.partitionRegion : region ?? AwsUtil.GetDefaultRegion(),
             credentials: provider,
             defaultsMode: 'standard',
             retryMode: 'standard',
@@ -412,14 +412,20 @@ export class AwsUtil {
      * we don't assume a role if we are running the master account AND the roleInTarget account is OrganizationAccessRoleName
      */
     private static UseCurrentPrincipal(accountId: string, roleInTargetAccount?: string): boolean {
-        if (AwsUtil.masterAccountId !== accountId && AwsUtil.masterAccountId !== undefined) {
-            return false;
+        // simplest case: if there is not account given, we must use current principal
+        // because we don't know where to assume and we don't make assumptions about the target account.
+        if (accountId === undefined) {
+            return true;
         }
-        // if AwsUtil.masterAccountId is undefined, we assume we are running in the master account so we only need to check the role
-        if (roleInTargetAccount && roleInTargetAccount !== GlobalState.GetOrganizationAccessRoleName(accountId)) {
-            return false;
+        // if we are in the master account and we don't provide a role then we must also use the current principal
+        if (AwsUtil.masterAccountId === accountId && roleInTargetAccount === undefined) {
+            return true;
         }
-        return true;
+        // if we are in the master account and the role is the default role, we also skip assuming. This is a special case
+        if (AwsUtil.masterAccountId === accountId && roleInTargetAccount === GlobalState.GetOrganizationAccessRoleName(accountId)) {
+            return true;
+        }
+        return false;
     }
 
     public static async DeleteObject(bucketName: string, objectKey: string, credentials: ClientCredentialsConfig = undefined): Promise<void> {
@@ -447,7 +453,7 @@ export class AwsUtil {
         // Determine which role to assume, if any
         const roleNameToAssume = AwsUtil.UseCurrentPrincipal(accountId, roleInTargetAccount) ? undefined : roleInTargetAccount ?? GlobalState.GetCrossAccountRoleName(accountId);
         // fall back to default region if none is provided. for STS commercial partition we always select us-east-1 because it's a global service
-        const region = (isPartition) ? this.partitionRegion : 'us-east-1';
+        const region = (isPartition) ? this.partitionRegion : AwsUtil.GetDefaultRegion();
 
         let tempCredsProviderOptions: FromTemporaryCredentialsOptions;
         if (viaRoleArn) {

src/util/initial-commit-util.ts

@@ -10,8 +10,6 @@ import { renderString } from 'nunjucks';
 import * as S3 from '@aws-sdk/client-s3';
 import archiver from 'archiver';
 import { Upload } from '@aws-sdk/lib-storage';
-import { ClientCredentialsConfig } from './aws-types';
-import { AwsUtil } from './aws-util';
 import { DefaultTemplate, ITemplateGenerationSettings } from '~writer/default-template-writer';
 
 interface TemplateDefinition {
@@ -48,15 +46,15 @@ export class InitialCommitUtil {
   }
 
 
-  static async parameterizeAndUpload(extractedTemplate: ExtractedTemplate, params: Record<string, any>, template: DefaultTemplate, stateBucketName: string, s3credentials?: ClientCredentialsConfig): Promise<void> {
+  static async parameterizeAndUpload(extractedTemplate: ExtractedTemplate, params: Record<string, any>, template: DefaultTemplate, stateBucketName: string, s3client: S3.S3Client): Promise<void> {
     const { definition: templateDefinition, tempDir } = extractedTemplate;
     for (const declaredParam of templateDefinition.parameters) {
       if ((declaredParam.required) && (params[declaredParam.name] === undefined)) {
         throw new Error(`template zip declares required parameter ${declaredParam.name}, which is not provided.`);
       }
     }
     const archive = archiver('zip');
-    const upload = uploadStream(stateBucketName, 'initial-commit.zip', s3credentials);
+    const upload = uploadStream(stateBucketName, 'initial-commit.zip', s3client);
     archive.pipe(upload.writeStream);
     this.replaceFiles(tempDir, params, archive, '');
     const renderedTemplateContents = renderString(template.template, params);
@@ -85,8 +83,7 @@ export class InitialCommitUtil {
 
 }
 
-const uploadStream = (bucket: string, key: string, credentials?: ClientCredentialsConfig): { writeStream: stream.PassThrough; promise: Promise<any> } => {
-  const s3 = new S3.S3Client({ credentials, region: AwsUtil.GetDefaultRegion(), followRegionRedirects: true });
+const uploadStream = (bucket: string, key: string, s3: S3.S3Client): { writeStream: stream.PassThrough; promise: Promise<any> } => {
   const pass = new stream.PassThrough();
   return {
     writeStream: pass,

test/integration-tests/nested-ou-duplicate-names.test.ts

@@ -1,30 +1,32 @@
-import { Organizations } from "aws-sdk";
 import { UpdateOrganizationCommand } from "~commands/index";
 import { readFileSync } from "fs";
 import { AwsOrganizationReader } from "~aws-provider/aws-organization-reader";
 import { AwsOrganization } from "~aws-provider/aws-organization";
 import { IIntegrationTestContext, baseBeforeAll, profileForIntegrationTests, baseAfterAll, sleepForTest } from "./base-integration-test";
+import { AwsUtil } from "~util/aws-util";
+import { OrganizationsClient } from "@aws-sdk/client-organizations";
+import { CreateBucketCommand, PutObjectCommand } from "@aws-sdk/client-s3";
 
 const basePathForScenario = './test/integration-tests/resources/scenario-nested-ou/';
 
 
 describe('when nesting ou\'s', () => {
     let context: IIntegrationTestContext;
-    let orgClient: Organizations;
+    let orgClient: OrganizationsClient;
 
     let organizationAfterInit: AwsOrganization;
     let organizationAfterDuplicateNames: AwsOrganization;
     let organizationAfterCleanup: AwsOrganization;
 
     beforeAll(async () => {
         context = await baseBeforeAll();
-        orgClient = new Organizations({ region: 'us-east-1' });
+        orgClient = AwsUtil.GetOrganizationsService()
         const command = {stateBucketName: context.stateBucketName, stateObject: 'state.json', profile: profileForIntegrationTests, verbose: true };
 
         try {
-            await context.s3client.createBucket({ Bucket: context.stateBucketName }).promise();
+            await context.s3client.send(new CreateBucketCommand({ Bucket: context.stateBucketName }));
             await sleepForTest(200);
-            await context.s3client.upload({ Bucket: command.stateBucketName, Key: command.stateObject, Body: readFileSync(basePathForScenario + '0-state.json') }).promise();
+            await context.s3client.send(new PutObjectCommand({ Bucket: command.stateBucketName, Key: command.stateObject, Body: readFileSync(basePathForScenario + '0-state.json') }));
 
             await UpdateOrganizationCommand.Perform({...command, templateFile: basePathForScenario + '1-init-organization.yml'});
             await sleepForTest(500);
@@ -42,6 +44,7 @@ describe('when nesting ou\'s', () => {
             await organizationAfterCleanup.initialize();
 
         } catch(err) {
+            console.error("Unexpected", err)
             expect(err.message).toBe('');
         }
     })

test/integration-tests/nested-ou-three-levels-deep.test.ts

@@ -1,16 +1,18 @@
-import { Organizations } from "aws-sdk";
 import { UpdateOrganizationCommand } from "~commands/index";
 import { readFileSync } from "fs";
 import { AwsOrganizationReader } from "~aws-provider/aws-organization-reader";
 import { AwsOrganization } from "~aws-provider/aws-organization";
 import { IIntegrationTestContext, baseBeforeAll, profileForIntegrationTests, baseAfterAll, sleepForTest } from "./base-integration-test";
+import { CreateBucketCommand, PutObjectCommand } from "@aws-sdk/client-s3";
+import { AwsUtil } from "~util/aws-util";
+import { OrganizationsClient } from "@aws-sdk/client-organizations";
 
 const basePathForScenario = './test/integration-tests/resources/scenario-nested-ou/';
 
 
 describe('when nesting ou\'s', () => {
     let context: IIntegrationTestContext;
-    let orgClient: Organizations;
+    let orgClient: OrganizationsClient;
 
     let organizationAfterInit: AwsOrganization;
     let organizationAfterThreeLevelsDeep: AwsOrganization;
@@ -19,12 +21,12 @@ describe('when nesting ou\'s', () => {
 
     beforeAll(async () => {
         context = await baseBeforeAll();
-        orgClient = new Organizations({ region: 'us-east-1' });
+        orgClient = AwsUtil.GetOrganizationsService()
         const command = {stateBucketName: context.stateBucketName, stateObject: 'state.json', profile: profileForIntegrationTests, verbose: true };
 
-        await context.s3client.createBucket({ Bucket: context.stateBucketName }).promise();
+        await context.s3client.send(new CreateBucketCommand({ Bucket: context.stateBucketName }));
         await sleepForTest(200);
-        await context.s3client.upload({ Bucket: command.stateBucketName, Key: command.stateObject, Body: readFileSync(basePathForScenario + '0-state.json') }).promise();
+        await context.s3client.send(new PutObjectCommand({ Bucket: command.stateBucketName, Key: command.stateObject, Body: readFileSync(basePathForScenario + '0-state.json') }));
 
         await UpdateOrganizationCommand.Perform({...command, templateFile: basePathForScenario + '1-init-organization.yml'});
         await sleepForTest(500);

test/integration-tests/nested-ou.test.ts

@@ -1,16 +1,18 @@
-import { Organizations } from "aws-sdk";
 import { UpdateOrganizationCommand } from "~commands/index";
 import { readFileSync } from "fs";
 import { AwsOrganizationReader } from "~aws-provider/aws-organization-reader";
 import { AwsOrganization } from "~aws-provider/aws-organization";
 import { IIntegrationTestContext, baseBeforeAll, profileForIntegrationTests, baseAfterAll, sleepForTest } from "./base-integration-test";
+import { AwsUtil } from "~util/aws-util";
+import { OrganizationsClient } from "@aws-sdk/client-organizations";
+import { CreateBucketCommand, PutObjectCommand } from "@aws-sdk/client-s3";
 
 const basePathForScenario = './test/integration-tests/resources/scenario-nested-ou/';
 
 
 describe('when nesting ou\'s', () => {
     let context: IIntegrationTestContext;
-    let orgClient: Organizations;
+    let orgClient: OrganizationsClient;
 
     let organizationAfterInit: AwsOrganization;
     let organizationAfterCreateParentChild: AwsOrganization;
@@ -22,13 +24,13 @@ describe('when nesting ou\'s', () => {
 
     beforeAll(async () => {
         context = await baseBeforeAll();
-        orgClient = new Organizations({ region: 'us-east-1' });
+        orgClient = AwsUtil.GetOrganizationsService()
         const command = {stateBucketName: context.stateBucketName, stateObject: 'state.json', profile: profileForIntegrationTests, verbose: true };
 
         try {
-            await context.s3client.createBucket({ Bucket: context.stateBucketName }).promise();
+            await context.s3client.send(new CreateBucketCommand({ Bucket: context.stateBucketName }));
             await sleepForTest(200);
-            await context.s3client.upload({ Bucket: command.stateBucketName, Key: command.stateObject, Body: readFileSync(basePathForScenario + '0-state.json') }).promise();
+            await context.s3client.send(new PutObjectCommand({ Bucket: command.stateBucketName, Key: command.stateObject, Body: readFileSync(basePathForScenario + '0-state.json') }));
 
             await UpdateOrganizationCommand.Perform({...command, templateFile: basePathForScenario + '1-init-organization.yml'});
             await sleepForTest(500);