Skip to content

Commit

Permalink
[monitor] Fix possible case sensitivity issues for Windows env vars (#…
Browse files Browse the repository at this point in the history 
…32380)

### Packages impacted by this PR

- @azure/monitor-opentelemetry-exporter

### Issues associated with this PR

- #32374

### Describe the problem that is addressed by this PR

Fixes the case-sensitivity issue with getting environment variables such
as enforcing `SYSTEMDRIVE` instead of a cased `systemdrive`.

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?


### Are there test cases added in this PR? _(If not, why?)_


### Provide a list of related PRs _(if any)_


### Command used to generate this PR:**_(Applicable only to SDK release
request PRs)_

### Checklists
- [ ] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so,
create an Issue in the
[Autorest/typescript](https://github.com/Azure/autorest.typescript)
repository and link it here)_
- [ ] Added a changelog (if necessary)
  • Loading branch information
@mpodwysocki
mpodwysocki authored Dec 30, 2024
1 parent e03185f commit 323f44e
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 53 deletions.
29 changes: 14 additions & 15 deletions sdk/monitor/monitor-opentelemetry-exporter/src/platform/nodejs/persist/fileAccessControl.ts
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import * as fs from "node:fs";
import * as os from "node:os";
import * as child_process from "child_process";
import { existsSync } from "node:fs";
import { type as osType } from "node:os";
import { spawn, spawnSync } from "node:child_process";
import { diag } from "@opentelemetry/api";
import process from "node:process";

export class FileAccessControl {
private static ICACLS_PATH = `${process.env.systemdrive}/windows/system32/icacls.exe`;
private static POWERSHELL_PATH = `${process.env.systemdrive}/windows/system32/windowspowershell/v1.0/powershell.exe`;
private static ICACLS_PATH = `${process.env.SYSTEMDRIVE}/windows/system32/icacls.exe`;
private static POWERSHELL_PATH = `${process.env.SYSTEMDRIVE}/windows/system32/windowspowershell/v1.0/powershell.exe`;
private static ACLED_DIRECTORIES: { [id: string]: boolean } = {};
private static ACL_IDENTITY: string | null = null;
private static OS_FILE_PROTECTION_CHECKED = false;
public static OS_PROVIDES_FILE_PROTECTION = false;
public static USE_ICACLS = os.type() === "Windows_NT";
public static USE_ICACLS = osType() === "Windows_NT";

// Check if file access control could be enabled
public static checkFileProtection(): void {
Expand All @@ -29,9 +30,7 @@ export class FileAccessControl {
// This should be async - but it's currently safer to have this synchronous
// This guarantees we can immediately fail setDiskRetryMode if we need to
try {
FileAccessControl.OS_PROVIDES_FILE_PROTECTION = fs.existsSync(
FileAccessControl.ICACLS_PATH,
);
FileAccessControl.OS_PROVIDES_FILE_PROTECTION = existsSync(FileAccessControl.ICACLS_PATH);
} catch (e: any) {
// Ignore error
}
Expand Down Expand Up @@ -87,7 +86,7 @@ export class FileAccessControl {

private static _runICACLS(args: string[]): Promise<void> {
return new Promise((resolve, reject) => {
const aclProc = child_process.spawn(FileAccessControl.ICACLS_PATH, args, <any>{
const aclProc = spawn(FileAccessControl.ICACLS_PATH, args, <any>{
windowsHide: true,
});
aclProc.on("error", (e: Error) => reject(e));
Expand All @@ -105,8 +104,8 @@ export class FileAccessControl {

private static _runICACLSSync(args: string[]): void {
// Some very old versions of Node (< 0.11) don't have this
if (child_process.spawnSync) {
const aclProc = child_process.spawnSync(FileAccessControl.ICACLS_PATH, args, <any>{
if (spawnSync) {
const aclProc = spawnSync(FileAccessControl.ICACLS_PATH, args, <any>{
windowsHide: true,
});
if (aclProc.error) {
Expand All @@ -126,7 +125,7 @@ export class FileAccessControl {
if (FileAccessControl.ACL_IDENTITY) {
resolve(FileAccessControl.ACL_IDENTITY);
}
const psProc = child_process.spawn(
const psProc = spawn(
FileAccessControl.POWERSHELL_PATH,
["-Command", "[System.Security.Principal.WindowsIdentity]::GetCurrent().Name"],
<any>{
Expand All @@ -153,8 +152,8 @@ export class FileAccessControl {
return FileAccessControl.ACL_IDENTITY;
}
// Some very old versions of Node (< 0.11) don't have this
if (child_process.spawnSync) {
const psProc = child_process.spawnSync(
if (spawnSync) {
const psProc = spawnSync(
FileAccessControl.POWERSHELL_PATH,
["-Command", "[System.Security.Principal.WindowsIdentity]::GetCurrent().Name"],
<any>{
Expand Down
21 changes: 8 additions & 13 deletions sdk/monitor/monitor-opentelemetry-exporter/src/platform/nodejs/persist/fileSystemHelpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,9 @@
// Licensed under the MIT License.

import { diag } from "@opentelemetry/api";
import * as fs from "node:fs";
import * as path from "node:path";
import { promisify } from "node:util";

const readdirAsync = promisify(fs.readdir);
const statAsync = promisify(fs.stat);
const lstatAsync = promisify(fs.lstat);
const mkdirAsync = promisify(fs.mkdir);
import type { MakeDirectoryOptions } from "node:fs";
import { join } from "node:path";
import { lstat, mkdir, readdir, stat } from "node:fs/promises";

/**
* Computes the size (in bytes) of all files in a directory at the root level. Asynchronously.
Expand All @@ -19,11 +14,11 @@ export const getShallowDirectorySize = async (directory: string): Promise<number
let totalSize = 0;
try {
// Get the directory listing
const files = await readdirAsync(directory);
const files = await readdir(directory);

// Query all file sizes
for (const file of files) {
const fileStats = await statAsync(path.join(directory, file));
const fileStats = await stat(join(directory, file));
if (fileStats.isFile()) {
totalSize += fileStats.size;
}
Expand All @@ -42,15 +37,15 @@ export const getShallowDirectorySize = async (directory: string): Promise<number
*/
export const confirmDirExists = async (directory: string): Promise<void> => {
try {
const stats = await lstatAsync(directory);
const stats = await lstat(directory);
if (!stats.isDirectory()) {
throw new Error("Path existed but was not a directory");
}
} catch (err: any) {
if (err && err.code === "ENOENT") {
try {
const options: fs.MakeDirectoryOptions = { recursive: true };
await mkdirAsync(directory, options);
const options: MakeDirectoryOptions = { recursive: true };
await mkdir(directory, options);
} catch (mkdirErr: any) {
if (mkdirErr && mkdirErr.code !== "EEXIST") {
// Handle race condition by ignoring EEXIST
Expand Down
43 changes: 18 additions & 25 deletions sdk/monitor/monitor-opentelemetry-exporter/src/platform/nodejs/persist/fileSystemPersist.ts
View file
Original file line number Diff line number Diff line change
@@ -1,21 +1,14 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import * as fs from "node:fs";
import * as os from "node:os";
import * as path from "node:path";
import { tmpdir } from "node:os";
import { basename, join } from "node:path";
import { diag } from "@opentelemetry/api";
import type { PersistentStorage } from "../../../types.js";
import { FileAccessControl } from "./fileAccessControl.js";
import { confirmDirExists, getShallowDirectorySize } from "./fileSystemHelpers.js";
import { promisify } from "node:util";
import type { AzureMonitorExporterOptions } from "../../../config.js";

const statAsync = promisify(fs.stat);
const readdirAsync = promisify(fs.readdir);
const readFileAsync = promisify(fs.readFile);
const unlinkAsync = promisify(fs.unlink);
const writeFileAsync = promisify(fs.writeFile);
import { readdir, readFile, stat, unlink, writeFile } from "node:fs/promises";

/**
* File system persist class.
Expand Down Expand Up @@ -60,8 +53,8 @@ export class FileSystemPersist implements PersistentStorage {
);
}
if (this._enabled) {
this._tempDirectory = path.join(
this._options?.storageDirectory || os.tmpdir(),
this._tempDirectory = join(
this._options?.storageDirectory || tmpdir(),
"Microsoft",
"AzureMonitor",
FileSystemPersist.TEMPDIR_PREFIX + this._instrumentationKey,
Expand Down Expand Up @@ -117,20 +110,20 @@ export class FileSystemPersist implements PersistentStorage {
*/
private async _getFirstFileOnDisk(): Promise<Buffer | null> {
try {
const stats = await statAsync(this._tempDirectory);
const stats = await stat(this._tempDirectory);
if (stats.isDirectory()) {
const origFiles = await readdirAsync(this._tempDirectory);
const origFiles = await readdir(this._tempDirectory);
const files = origFiles.filter((f) =>
path.basename(f).includes(FileSystemPersist.FILENAME_SUFFIX),
basename(f).includes(FileSystemPersist.FILENAME_SUFFIX),
);
if (files.length === 0) {
return null;
} else {
const firstFile = files[0];
const filePath = path.join(this._tempDirectory, firstFile);
const payload = await readFileAsync(filePath);
const filePath = join(this._tempDirectory, firstFile);
const payload = await readFile(filePath);
// delete the file first to prevent double sending
await unlinkAsync(filePath);
await unlink(filePath);
return payload;
}
}
Expand Down Expand Up @@ -167,12 +160,12 @@ export class FileSystemPersist implements PersistentStorage {
}

const fileName = `${new Date().getTime()}${FileSystemPersist.FILENAME_SUFFIX}`;
const fileFullPath = path.join(this._tempDirectory, fileName);
const fileFullPath = join(this._tempDirectory, fileName);

// Mode 600 is w/r for creator and no read access for others
diag.info(`saving data to disk at: ${fileFullPath}`);
try {
await writeFileAsync(fileFullPath, payload, { mode: 0o600 });
await writeFile(fileFullPath, payload, { mode: 0o600 });
} catch (writeError: any) {
diag.warn(`Error writing file to persistent file storage`, writeError);
return false;
Expand All @@ -182,11 +175,11 @@ export class FileSystemPersist implements PersistentStorage {

private async _fileCleanupTask(): Promise<boolean> {
try {
const stats = await statAsync(this._tempDirectory);
const stats = await stat(this._tempDirectory);
if (stats.isDirectory()) {
const origFiles = await readdirAsync(this._tempDirectory);
const origFiles = await readdir(this._tempDirectory);
const files = origFiles.filter((f) =>
path.basename(f).includes(FileSystemPersist.FILENAME_SUFFIX),
basename(f).includes(FileSystemPersist.FILENAME_SUFFIX),
);
if (files.length === 0) {
return false;
Expand All @@ -198,8 +191,8 @@ export class FileSystemPersist implements PersistentStorage {
);
const expired = new Date(+new Date() - this.fileRetemptionPeriod) > fileCreationDate;
if (expired) {
const filePath = path.join(this._tempDirectory, file);
await unlinkAsync(filePath);
const filePath = join(this._tempDirectory, file);
await unlink(filePath);
}
});
return true;
Expand Down

0 comments on commit 323f44e

Please sign in to comment.