Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cli): automatically determine region on EC2 instances #9313

Merged
merged 16 commits into from
Aug 25, 2020
Merged

feat(cli): automatically determine region on EC2 instances #9313

Changes from 8 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
4ab3429
Fix: allow EC2 instances to query IMDS to determine the region
joel-aws Jul 28, 2020
bb9fba6
Merge branch 'master' into joel-aws/instance-imds-region
joel-aws Jul 29, 2020
d0061ef
Cache isEc2Instance value and return it if available
joel-aws Aug 12, 2020
9f8ffad
Properly signal failures in imds-related promises
joel-aws Aug 12, 2020
0bcf8a6
Reformat and update semantics
joel-aws Aug 13, 2020
bfea5cf
Updated logging and handling of errors for IMDS calls
joel-aws Aug 13, 2020
3e5bf4e
Merge branch 'master' into joel-aws/instance-imds-region
joel-aws Aug 13, 2020
1f2b830
Fix missing semi error
joel-aws Aug 13, 2020
f1b364c
Catch potential json parsing errors
joel-aws Aug 17, 2020
6113707
Removed isec2instance logging on grab from cache
joel-aws Aug 17, 2020
835552b
replace let instead of var for isec2instancecache
joel-aws Aug 17, 2020
5108dbf
Rewrite logic a little
rix0rrr Aug 18, 2020
e13ac15
Add mocked test
rix0rrr Aug 18, 2020
815a4f7
Merge remote-tracking branch 'origin/master' into pr/joel-aws/9313
rix0rrr Aug 18, 2020
b624482
Merge remote-tracking branch 'origin/master' into pr/joel-aws/9313
rix0rrr Aug 24, 2020
fb46a7a
Merge branch 'master' into joel-aws/instance-imds-region
mergify[bot] Aug 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 107 additions & 33 deletions packages/aws-cdk/lib/api/aws-auth/awscli-compatible.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export class AwsCliCompatible {

if (containerCreds ?? hasEcsCredentials()) {
sources.push(() => new AWS.ECSCredentials());
} else if (ec2creds ?? await hasEc2Credentials()) {
} else if (ec2creds ?? await isEc2Instance()) {
// else if: don't get EC2 creds if we should have gotten ECS creds--ECS instances also
// run on EC2 boxes but the creds represent something different. Same behavior as
// upstream code.
Expand All @@ -76,8 +76,6 @@ export class AwsCliCompatible {
* 2. $AWS_DEFAULT_PROFILE and $AWS_DEFAULT_REGION are also respected.
*
* Lambda and CodeBuild set the $AWS_REGION variable.
*
* FIXME: EC2 instances require querying the metadata service to determine the current region.
*/
public static async region(profile: string | undefined): Promise<string> {
profile = profile || process.env.AWS_PROFILE || process.env.AWS_DEFAULT_PROFILE || 'default';
Expand All @@ -101,6 +99,25 @@ export class AwsCliCompatible {
}
}

if (!region && isEc2Instance()) {
debug('Looking up AWS region in the EC2 Instance Metadata Service (IMDS).');
const imdsOptions = {
httpOptions: { timeout: 1000, connectTimeout: 1000 }, maxRetries: 2,
};
const metadataService = new AWS.MetadataService(imdsOptions);
const token = await getImdsV2Token(metadataService)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda don't like the style this is written. It confuses me to combine async/await and thenables in this way.

Why not:

try {
  const token = await getImdsV2Token(metadataService);
  region = await getRegionFromImds(metadataService, token);
  debug(`Retrieved AWS region "${region}" from the IMDS.`);
} catch (e) {
  debug(`Unable to retrieve region from IMDS: ${e.message}`);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about that, but was concerned about a case where IMDSv2 was not supported -- potentially in partition where it's not implemented. In that situation, getImdsV2Token would throw an error but IMDSv1 should still be queried to get the region, just without a token.

.catch((error) => {
debug(`No token returned from IMDS; Error: ${error}`);
return undefined;
});
region = await getRegionFromImds(metadataService, token)
.catch((error) => {
debug(`No Instance Identity Document returned from IMDS; Error: ${error}`);
return undefined;
});
debug(region ? `Retrieved AWS region "${region}" from the IMDS.` : 'Unable to retrieve the AWS region from the IMDS.');
}

if (!region) {
const usedProfile = !profile ? '' : ` (profile: "${profile}")`;
region = 'us-east-1'; // This is what the AWS CLI does
Expand All @@ -121,39 +138,96 @@ function hasEcsCredentials(): boolean {
/**
* Return whether we're on an EC2 instance
*/
async function hasEc2Credentials() {
debug("Determining whether we're on an EC2 instance.");

let instance = false;
if (process.platform === 'win32') {
// https://docs.aws.amazon.com/AWSEC2/latest/WindowsGuide/identify_ec2_instances.html
const result = await util.promisify(child_process.exec)('wmic path win32_computersystemproduct get uuid', { encoding: 'utf-8' });
// output looks like
// UUID
// EC2AE145-D1DC-13B2-94ED-01234ABCDEF
const lines = result.stdout.toString().split('\n');
instance = lines.some(x => matchesRegex(/^ec2/i, x));
} else {
// https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/identify_ec2_instances.html
const files: Array<[string, RegExp]> = [
// This recognizes the Xen hypervisor based instances (pre-5th gen)
['/sys/hypervisor/uuid', /^ec2/i],

// This recognizes the new Hypervisor (5th-gen instances and higher)
// Can't use the advertised file '/sys/devices/virtual/dmi/id/product_uuid' because it requires root to read.
// Instead, sys_vendor contains something like 'Amazon EC2'.
['/sys/devices/virtual/dmi/id/sys_vendor', /ec2/i],
];
for (const [file, re] of files) {
if (matchesRegex(re, readIfPossible(file))) {
instance = true;
break;
async function isEc2Instance() {
if (isEc2InstanceCache === undefined) {
debug("Determining if we're on an EC2 instance.");
let instance = false;
if (process.platform === 'win32') {
// https://docs.aws.amazon.com/AWSEC2/latest/WindowsGuide/identify_ec2_instances.html
const result = await util.promisify(child_process.exec)('wmic path win32_computersystemproduct get uuid', { encoding: 'utf-8' });
// output looks like
// UUID
// EC2AE145-D1DC-13B2-94ED-01234ABCDEF
const lines = result.stdout.toString().split('\n');
instance = lines.some(x => matchesRegex(/^ec2/i, x));
} else {
// https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/identify_ec2_instances.html
const files: Array<[string, RegExp]> = [
Comment on lines +144 to +158
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider adding similar test cases for this method and its conditionals. Since this method already existed, I'll let you make the final call.

// This recognizes the Xen hypervisor based instances (pre-5th gen)
['/sys/hypervisor/uuid', /^ec2/i],

// This recognizes the new Hypervisor (5th-gen instances and higher)
// Can't use the advertised file '/sys/devices/virtual/dmi/id/product_uuid' because it requires root to read.
// Instead, sys_vendor contains something like 'Amazon EC2'.
['/sys/devices/virtual/dmi/id/sys_vendor', /ec2/i],
];
for (const [file, re] of files) {
if (matchesRegex(re, readIfPossible(file))) {
instance = true;
break;
}
}
}
isEc2InstanceCache = instance;
}
else {
joel-aws marked this conversation as resolved.
Show resolved Hide resolved
debug("Looking in the cache to see if we're on an EC2 instance.");
}
debug(isEc2InstanceCache ? 'Looks like an EC2 instance.' : 'Does not look like an EC2 instance.');
joel-aws marked this conversation as resolved.
Show resolved Hide resolved
return isEc2InstanceCache;
}


debug(instance ? 'Looks like EC2 instance.' : 'Does not look like EC2 instance.');
return instance;
var isEc2InstanceCache: boolean | undefined = undefined;
joel-aws marked this conversation as resolved.
Show resolved Hide resolved

/**
* Attempts to get a Instance Metadata Service V2 token
*/
async function getImdsV2Token(metadataService: AWS.MetadataService): Promise<string> {
debug('Attempting to retrieve an IMDSv2 token.');
return new Promise((resolve, reject) => {
metadataService.request(
'/latest/api/token',
{
method: 'PUT',
headers: { 'x-aws-ec2-metadata-token-ttl-seconds': '60' },
},
(err: AWS.AWSError, token: string | undefined) => {
if (err) {
reject(err);
} else if (!token) {
reject(new Error('IMDS did not return a token.'));
} else {
resolve(token);
}
});
});
}

/**
* Attempts to get the region from the Instance Metadata Service
*/
async function getRegionFromImds(metadataService: AWS.MetadataService, token: string | undefined): Promise<string> {
debug('Retrieving the AWS region from the IMDS.');
let options: { method?: string | undefined; headers?: { [key: string]: string; } | undefined; } = {};
if (token) {
options = { headers: { 'x-aws-ec2-metadata-token': token } };
}
return new Promise((resolve, reject) => {
metadataService.request(
'/latest/dynamic/instance-identity/document',
options,
(err: AWS.AWSError, instanceIdentityDocument: string | undefined) => {
if (err) {
reject(err);
} else if (!instanceIdentityDocument) {
reject(new Error('IMDS did not return an Instance Identity Document.'));
} else {
const region = JSON.parse(instanceIdentityDocument).region;
joel-aws marked this conversation as resolved.
Show resolved Hide resolved
resolve(region);
}
});
});
}

function homeDir() {
Expand All @@ -172,7 +246,7 @@ function configFileName() {
/**
* Force the JS SDK to honor the ~/.aws/config file (and various settings therein)
*
* For example, ther is just *NO* way to do AssumeRole credentials as long as AWS_SDK_LOAD_CONFIG is not set,
* For example, there is just *NO* way to do AssumeRole credentials as long as AWS_SDK_LOAD_CONFIG is not set,
* or read credentials from that file.
*
* The SDK crashes if the variable is set but the file does not exist, so conditionally set it.
Expand Down