Skip to content

Commit b24548a

Browse files
authored
ec2: explicitly check if SSM agent is running (#3617)
Problem: If the error is not related to permissions or instance status, we throw a general error about the SSM agent not running. However, this may not always be the case and we shouldn't assume other errors couldn't occur. Solution: We can leverage DescribeInstanceInformation from the SSM SDK to check the ping status of the SSM agent of target EC2 instance. If this call returns an error, or the ping status is outdated, we can conclude the SSM agent isn't running. Now, there is a specific error message for when an instance is not able to connect. Therefore, the default error message has been changed so that it does not mention the SSM agent (since we would have already checked that).
1 parent 0fe8594 commit b24548a

File tree

3 files changed

+57
-8
lines changed

3 files changed

+57
-8
lines changed

src/ec2/model.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { ToolkitError } from '../shared/errors'
1212
import { SsmClient } from '../shared/clients/ssmClient'
1313
import { Ec2Client } from '../shared/clients/ec2Client'
1414

15-
export type Ec2ConnectErrorCode = 'EC2SSMStatus' | 'EC2SSMPermission' | 'EC2SSMConnect'
15+
export type Ec2ConnectErrorCode = 'EC2SSMStatus' | 'EC2SSMPermission' | 'EC2SSMConnect' | 'EC2SSMAgentStatus'
1616

1717
import { openRemoteTerminal } from '../shared/remoteSession'
1818
import { DefaultIamClient } from '../shared/clients/iamClient'
@@ -71,6 +71,7 @@ export class Ec2ConnectionManager {
7171
public async checkForStartSessionError(selection: Ec2Selection): Promise<void> {
7272
const isInstanceRunning = await this.isInstanceRunning(selection.instanceId)
7373
const hasProperPolicies = await this.hasProperPolicies(selection.instanceId)
74+
const isSsmAgentRunning = (await this.ssmClient.getInstanceAgentPingStatus(selection.instanceId)) == 'Online'
7475

7576
if (!isInstanceRunning) {
7677
const message = 'Ensure the target instance is running and not currently starting, stopping, or stopped.'
@@ -87,6 +88,15 @@ export class Ec2ConnectionManager {
8788
documentationUri: documentationUri,
8889
})
8990
}
91+
92+
if (!isSsmAgentRunning) {
93+
this.throwConnectionError('Is SSM Agent running on the target instance?', selection, {
94+
code: 'EC2SSMAgentStatus',
95+
documentationUri: vscode.Uri.parse(
96+
'https://docs.aws.amazon.com/systems-manager/latest/userguide/ssm-agent.html'
97+
),
98+
})
99+
}
90100
}
91101

92102
private async openSessionInTerminal(session: Session, selection: Ec2Selection) {
@@ -110,8 +120,8 @@ export class Ec2ConnectionManager {
110120
await this.openSessionInTerminal(response, selection)
111121
} catch (err: unknown) {
112122
// Default error if pre-check fails.
113-
this.throwConnectionError('Check that the SSM Agent is running on target instance', selection, {
114-
code: 'EC2SSMConnect',
123+
this.throwConnectionError('Unable to connect to target instance. ', selection, {
124+
code: 'EC2SSMAgentStatus',
115125
cause: err as Error,
116126
})
117127
}

src/shared/clients/ssmClient.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import { SSM } from 'aws-sdk'
77
import { getLogger } from '../logger/logger'
88
import globals from '../extensionGlobals'
9+
import { pageableToCollection } from '../utilities/collectionUtils'
910

1011
export class SsmClient {
1112
public constructor(public readonly regionCode: string) {}
@@ -32,4 +33,30 @@ export class SsmClient {
3233
const response = await client.startSession({ Target: target }).promise()
3334
return response
3435
}
36+
37+
public async describeInstance(target: string): Promise<SSM.InstanceInformation> {
38+
const client = await this.createSdkClient()
39+
const requester = async (req: SSM.DescribeInstanceInformationRequest) =>
40+
client.describeInstanceInformation(req).promise()
41+
const request: SSM.DescribeInstanceInformationRequest = {
42+
InstanceInformationFilterList: [
43+
{
44+
key: 'InstanceIds',
45+
valueSet: [target],
46+
},
47+
],
48+
}
49+
50+
const response = await pageableToCollection(requester, request, 'NextToken', 'InstanceInformationList')
51+
.flatten()
52+
.flatten()
53+
.promise()
54+
55+
return response[0]!
56+
}
57+
58+
public async getInstanceAgentPingStatus(target: string): Promise<string> {
59+
const instanceInformation = await this.describeInstance(target)
60+
return instanceInformation ? instanceInformation.PingStatus! : 'Inactive'
61+
}
3562
}

src/test/ec2/model.test.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ describe('Ec2ConnectClient', function () {
1717
public constructor() {
1818
super('test-region')
1919
}
20+
21+
public override async getInstanceAgentPingStatus(target: string): Promise<string> {
22+
return target.split(':')[2]
23+
}
2024
}
2125

2226
class MockEc2Client extends Ec2Client {
@@ -82,36 +86,44 @@ describe('Ec2ConnectClient', function () {
8286

8387
await assertThrowsErrorCode(
8488
{
85-
instanceId: 'pending:noPolicies',
89+
instanceId: 'pending:noPolicies:Online',
8690
region: 'test-region',
8791
},
8892
'EC2SSMStatus'
8993
)
9094

9195
await assertThrowsErrorCode(
9296
{
93-
instanceId: 'shutting-down:noPolicies',
97+
instanceId: 'shutting-down:noPolicies:Online',
9498
region: 'test-region',
9599
},
96100
'EC2SSMStatus'
97101
)
98102

99103
await assertThrowsErrorCode(
100104
{
101-
instanceId: 'running:noPolicies',
105+
instanceId: 'running:noPolicies:Online',
102106
region: 'test-region',
103107
},
104108
'EC2SSMPermission'
105109
)
106110

107111
await assertThrowsErrorCode(
108112
{
109-
instanceId: 'running:hasPolicies',
113+
instanceId: 'running:hasPolicies:Offline',
110114
region: 'test-region',
111115
},
112-
'EC2SSMConnect'
116+
'EC2SSMAgentStatus'
113117
)
114118
})
119+
120+
it('does not throw an error if all checks pass', async function () {
121+
const passingInstance = {
122+
instanceId: 'running:hasPolicies:Online',
123+
region: 'test-region',
124+
}
125+
assert.doesNotThrow(async () => await client.checkForStartSessionError(passingInstance))
126+
})
115127
})
116128

117129
describe('hasProperPolicies', async function () {

0 commit comments

Comments
 (0)