Skip to content

Commit 2cb20f2

Browse files
laileni-awslaileni-aws
committed
Addressing comments
1 parent 2bf7c38 commit 2cb20f2

File tree

2 files changed

+117
-35
lines changed

2 files changed

+117
-35
lines changed

‎packages/core/src/shared/vscode/env.ts‎

Lines changed: 36 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,35 @@ export function isRemoteWorkspace(): boolean {
124124
return vscode.env.remoteName === 'ssh-remote'
125125
}
126126

127+
/**
128+
* Parses an os-release file according to the freedesktop.org standard.
129+
*
130+
* @param content The content of the os-release file
131+
* @returns A record of key-value pairs from the os-release file
132+
*
133+
* @see https://www.freedesktop.org/software/systemd/man/latest/os-release.html
134+
*/
135+
function parseOsRelease(content: string): Record<string, string> {
136+
const result: Record<string, string> = {}
137+
138+
for (let line of content.split('\n')) {
139+
line = line.trim()
140+
// Skip empty lines and comments
141+
if (!line || line.startsWith('#')) {
142+
continue
143+
}
144+
145+
const eqIndex = line.indexOf('=')
146+
if (eqIndex > 0) {
147+
const key = line.slice(0, eqIndex)
148+
const value = line.slice(eqIndex + 1).replace(/^["']|["']$/g, '')
149+
result[key] = value
150+
}
151+
}
152+
153+
return result
154+
}
155+
127156
/**
128157
* Checks if the current environment has SageMaker-specific environment variables
129158
* @returns true if SageMaker environment variables are detected
@@ -152,15 +181,11 @@ export function hasSageMakerEnvVars(): boolean {
152181
* Detection Process (in order):
153182
* 1. Returns false for web environments (browser-based)
154183
* 2. Returns false for SageMaker environments (even if host is AL2)
155-
* 3. Checks `/etc/image-id` for Amazon Linux specific identification
156-
* - Most reliable method for Amazon Linux systems
157-
* - Contains `image_name="Amazon Linux 2"` for AL2
158-
* - Returns false for Amazon Linux 2023 or other versions
159-
* 4. Checks `/etc/os-release` with fallback to `/usr/lib/os-release`
184+
* 3. Checks `/etc/os-release` with fallback to `/usr/lib/os-release`
160185
* - Standard Linux OS identification files
161186
* - Explicitly checks for and rejects Amazon Linux 2023
162187
* - Looks for `ID="amzn"` and `VERSION_ID="2"` for AL2
163-
* 5. Falls back to kernel version check as last resort
188+
* 4. Falls back to kernel version check as last resort
164189
* - Checks for `.amzn2.` or `.amzn2int.` in kernel release
165190
* - Only used if file-based detection fails or confirms AL2
166191
*
@@ -200,47 +225,23 @@ export function isAmazonLinux2() {
200225
// not the host kernel version
201226
try {
202227
const fs = require('fs')
203-
204-
// First, try Amazon Linux specific file /etc/image-id. This is the most reliable way to identify Amazon Linux
205-
if (fs.existsSync('/etc/image-id')) {
206-
try {
207-
const imageId = fs.readFileSync('/etc/image-id', 'utf8')
208-
// Check if this is Amazon Linux 2 (not 2023 or other versions)
209-
// Example content: image_name="Amazon Linux 2"
210-
if (imageId.includes('image_name="Amazon Linux 2"')) {
211-
return true
212-
}
213-
// If it's Amazon Linux but not version 2, return false
214-
if (imageId.includes('image_name="Amazon Linux')) {
215-
return false
216-
}
217-
} catch (e) {
218-
// Continue to other checks if we can't read the file
219-
getLogger().error(`Checking for Amazon Linux specific file /etc/image-id failed with error: ${e}`)
220-
}
221-
}
222-
223228
// Check /etc/os-release with fallback to /usr/lib/os-release as per https://docs.aws.amazon.com/linux/al2/ug/ident-os-release.html
224229
const osReleasePaths = ['/etc/os-release', '/usr/lib/os-release']
225230
for (const osReleasePath of osReleasePaths) {
226231
if (fs.existsSync(osReleasePath)) {
227232
try {
228-
const osRelease = fs.readFileSync(osReleasePath, 'utf8')
233+
const osReleaseContent = fs.readFileSync(osReleasePath, 'utf8')
234+
const osRelease = parseOsRelease(osReleaseContent)
229235

230236
// Check if this is Amazon Linux 2023 (not AL2)
231-
if (
232-
osRelease.includes('VERSION_ID="2023"') ||
233-
osRelease.includes('PLATFORM_ID="platform:al2023"')
234-
) {
237+
if (osRelease.VERSION_ID === '2023' || osRelease.PLATFORM_ID === 'platform:al2023') {
235238
// This is Amazon Linux 2023, not AL2
236239
return false
237240
}
238241

239242
// Check if this is actually Amazon Linux 2
240243
// Must be specifically version 2, not 2023 or other versions
241-
const isAL2 =
242-
osRelease.includes('Amazon Linux 2') ||
243-
(osRelease.includes('ID="amzn"') && osRelease.includes('VERSION_ID="2"'))
244+
const isAL2 = osRelease.ID === 'amzn' && osRelease.VERSION_ID === '2'
244245

245246
// If we found os-release file, trust its content over kernel version
246247
if (!isAL2) {
@@ -251,7 +252,7 @@ export function isAmazonLinux2() {
251252
break // Found and processed os-release, no need to check fallback
252253
} catch (e) {
253254
// Continue to next path or fallback check
254-
getLogger().error(`Checking for Amazon Linux 2023 (not AL2) failed with error: ${e}`)
255+
getLogger().error(`Parsing os-release file failed with error: ${e}`)
255256
}
256257
}
257258
}

‎packages/core/src/test/shared/vscode/env.test.ts‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,87 @@ VERSION_ID="22.04"
310310
// Should trust container OS over kernel
311311
assert.strictEqual(isAmazonLinux2(), false)
312312
})
313+
314+
it('handles os-release with comments correctly', function () {
315+
fsExistsStub.returns(true)
316+
fsReadFileStub.returns(`
317+
# This is a comment with VERSION_ID="2023" that should be ignored
318+
NAME="Amazon Linux 2"
319+
VERSION="2"
320+
ID="amzn"
321+
# Another comment with PLATFORM_ID="platform:al2023"
322+
VERSION_ID="2"
323+
PRETTY_NAME="Amazon Linux 2"
324+
`)
325+
326+
// Should correctly identify as AL2 despite comments containing AL2023 identifiers
327+
assert.strictEqual(isAmazonLinux2(), true)
328+
})
329+
330+
it('handles os-release with quoted values correctly', function () {
331+
fsExistsStub.returns(true)
332+
fsReadFileStub.returns(`
333+
NAME="Amazon Linux 2"
334+
VERSION='2'
335+
ID=amzn
336+
VERSION_ID="2"
337+
PRETTY_NAME='Amazon Linux 2'
338+
`)
339+
340+
// Should correctly parse both single and double quoted values
341+
assert.strictEqual(isAmazonLinux2(), true)
342+
})
343+
344+
it('handles os-release with empty lines and whitespace', function () {
345+
fsExistsStub.returns(true)
346+
fsReadFileStub.returns(`
347+
348+
NAME="Amazon Linux 2"
349+
350+
VERSION="2"
351+
ID="amzn"
352+
VERSION_ID="2"
353+
354+
PRETTY_NAME="Amazon Linux 2"
355+
356+
`)
357+
358+
// Should correctly parse despite empty lines and whitespace
359+
assert.strictEqual(isAmazonLinux2(), true)
360+
})
361+
362+
it('rejects Amazon Linux 2023 even with misleading comments', function () {
363+
fsExistsStub.returns(true)
364+
fsReadFileStub.returns(`
365+
# This comment mentions Amazon Linux 2 but should not affect parsing
366+
NAME="Amazon Linux"
367+
VERSION="2023"
368+
ID="amzn"
369+
# Comment with VERSION_ID="2" should be ignored
370+
VERSION_ID="2023"
371+
PLATFORM_ID="platform:al2023"
372+
PRETTY_NAME="Amazon Linux 2023"
373+
`)
374+
375+
// Should correctly identify as AL2023 (not AL2) despite misleading comments
376+
assert.strictEqual(isAmazonLinux2(), false)
377+
})
378+
379+
it('handles malformed os-release lines gracefully', function () {
380+
fsExistsStub.returns(true)
381+
fsReadFileStub.returns(`
382+
NAME="Amazon Linux 2"
383+
VERSION="2"
384+
ID="amzn"
385+
INVALID_LINE_WITHOUT_EQUALS
386+
=INVALID_LINE_STARTING_WITH_EQUALS
387+
VERSION_ID="2"
388+
PRETTY_NAME="Amazon Linux 2"
389+
`)
390+
391+
// Should correctly parse valid lines and ignore malformed ones
392+
assert.strictEqual(isAmazonLinux2(), true)
393+
})
313394
})
314395

315396
describe('hasSageMakerEnvVars', function () {

0 commit comments

Comments
 (0)