Skip to content

Commit

Permalink
Address remaining feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
@lukeelmers
lukeelmers committed Apr 8, 2021
1 parent 6d71207 commit 6e26141
Show file tree
Hide file tree
Showing 10 changed files with 168 additions and 134 deletions.
10 changes: 7 additions & 3 deletions src/plugins/kibana_usage_collection/server/collectors/cloud/cloud_provider_collector.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,13 @@ describe('registerCloudProviderUsageCollector', () => {
expect(collector).not.toBeUndefined();
});

test('isReady() => true', () => {
test('isReady() => false when cloud details are not available', () => {
cloudDetailsMock.mockReturnValueOnce(undefined);
expect(collector.isReady()).toBe(false);
});

test('isReady() => true when cloud details are available', () => {
cloudDetailsMock.mockReturnValueOnce({ foo: true });
expect(collector.isReady()).toBe(true);
});

Expand All @@ -49,7 +55,6 @@ describe('registerCloudProviderUsageCollector', () => {
describe('fetch()', () => {
test('returns undefined when no details are available', async () => {
cloudDetailsMock.mockReturnValueOnce(undefined);

await expect(collector.fetch(mockedFetchContext)).resolves.toBeUndefined();
});

Expand All @@ -62,7 +67,6 @@ describe('registerCloudProviderUsageCollector', () => {
};

cloudDetailsMock.mockReturnValueOnce(mockDetails);

await expect(collector.fetch(mockedFetchContext)).resolves.toEqual(mockDetails);
});

Expand Down
2 changes: 1 addition & 1 deletion src/plugins/kibana_usage_collection/server/collectors/cloud/cloud_provider_collector.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export function registerCloudProviderUsageCollector(usageCollection: UsageCollec

const collector = usageCollection.makeUsageCollector<Usage | undefined>({
type: 'cloud_provider',
isReady: () => true,
isReady: () => Boolean(cloudDetector.getCloudDetails()),
async fetch() {
const details = cloudDetector.getCloudDetails();
if (!details) {
Expand Down
78 changes: 53 additions & 25 deletions src/plugins/kibana_usage_collection/server/collectors/cloud/detector/aws.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ describe('AWS', () => {
});
});

describe('_parseBody', () => {
describe('parseBody', () => {
it('parses object in expected format', () => {
const body: AWSResponse = {
devpayProductCodes: null,
Expand All @@ -144,11 +144,12 @@ describe('AWS', () => {
marketplaceProductCodes: null,
};

const response = AWS._parseBody(body);
const response = AWSCloudService.parseBody(AWS.getName(), body)!;
expect(response).not.toBeNull();

expect(response?.getName()).toEqual(AWS.getName());
expect(response?.isConfirmed()).toEqual(true);
expect(response?.toJSON()).toEqual({
expect(response.getName()).toEqual(AWS.getName());
expect(response.isConfirmed()).toEqual(true);
expect(response.toJSON()).toEqual({
name: 'aws',
id: 'i-0c7a5b7590a4d811c',
vm_type: 't2.micro',
Expand All @@ -168,13 +169,13 @@ describe('AWS', () => {

it('ignores unexpected response body', () => {
// @ts-expect-error
expect(AWS._parseBody(undefined)).toBe(null);
expect(AWSCloudService.parseBody(AWS.getName(), undefined)).toBe(null);
// @ts-expect-error
expect(AWS._parseBody(null)).toBe(null);
expect(AWSCloudService.parseBody(AWS.getName(), null)).toBe(null);
// @ts-expect-error
expect(AWS._parseBody({})).toBe(null);
expect(AWSCloudService.parseBody(AWS.getName(), {})).toBe(null);
// @ts-expect-error
expect(AWS._parseBody({ privateIp: 'a.b.c.d' })).toBe(null);
expect(AWSCloudService.parseBody(AWS.getName(), { privateIp: 'a.b.c.d' })).toBe(null);
});
});

Expand Down Expand Up @@ -231,6 +232,49 @@ describe('AWS', () => {
metadata: undefined,
});
});

it('returns confirmed if only one file exists', async () => {
let callCount = 0;
const awsCheckedFileSystem = new AWSCloudService({
_fs: {
readFile: (filename: string, encoding: string, callback: Callback) => {
if (callCount === 0) {
callCount++;
throw new Error('oops');
}
callback(null, ec2Uuid);
},
} as typeof fs,
_isWindows: false,
});

const response = await awsCheckedFileSystem._tryToDetectUuid();

expect(response.isConfirmed()).toEqual(true);
expect(response.toJSON()).toEqual({
name: AWS.getName(),
id: ec2Uuid.trim().toLowerCase(),
region: undefined,
zone: undefined,
vm_type: undefined,
metadata: undefined,
});
});

it('returns unconfirmed if all files return errors', async () => {
const awsFailedFileSystem = new AWSCloudService({
_fs: ({
readFile: () => {
throw new Error('oops');
},
} as unknown) as typeof fs,
_isWindows: false,
});

const response = await awsFailedFileSystem._tryToDetectUuid();

expect(response.isConfirmed()).toEqual(false);
});
});

it('ignores UUID if it does not start with ec2', async () => {
Expand Down Expand Up @@ -263,21 +307,5 @@ describe('AWS', () => {

expect(response.isConfirmed()).toEqual(false);
});

it('does NOT handle file system exceptions', async () => {
const fileDNE = new Error('File DNE');
const awsFailedFileSystem = new AWSCloudService({
_fs: ({
readFile: () => {
throw fileDNE;
},
} as unknown) as typeof fs,
_isWindows: false,
});

expect(async () => {
await awsFailedFileSystem._tryToDetectUuid();
}).rejects.toThrow(fileDNE);
});
});
});
61 changes: 33 additions & 28 deletions src/plugins/kibana_usage_collection/server/collectors/cloud/detector/aws.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,28 +43,6 @@ export class AWSCloudService extends CloudService {
private readonly _isWindows: boolean;
private readonly _fs: typeof fs;

constructor(options: CloudServiceOptions = {}) {
super('aws', options);

// Allow the file system handler to be swapped out for tests
const { _fs = fs, _isWindows = process.platform.startsWith('win') } = options;

this._fs = _fs;
this._isWindows = _isWindows;
}

async _checkIfService(request: Request) {
const req: RequestOptions = {
method: 'GET',
uri: SERVICE_ENDPOINT,
json: true,
};

return promisify(request)(req)
.then((response) => this._parseResponse(response.body, (body) => this._parseBody(body)))
.catch(() => this._tryToDetectUuid());
}

/**
* Parse the AWS response, if possible.
*
Expand All @@ -86,7 +64,7 @@ export class AWSCloudService extends CloudService {
* "version" : "2010-08-31",
* }
*/
_parseBody(body: AWSResponse): CloudServiceResponse | null {
static parseBody(name: string, body: AWSResponse): CloudServiceResponse | null {
const id: string | undefined = get(body, 'instanceId');
const vmType: string | undefined = get(body, 'instanceType');
const region: string | undefined = get(body, 'region');
Expand All @@ -106,12 +84,38 @@ export class AWSCloudService extends CloudService {

// ensure we actually have some data
if (id || vmType || region || zone) {
return new CloudServiceResponse(this._name, true, { id, vmType, region, zone, metadata });
return new CloudServiceResponse(name, true, { id, vmType, region, zone, metadata });
}

return null;
}

constructor(options: CloudServiceOptions = {}) {
super('aws', options);

// Allow the file system handler to be swapped out for tests
const { _fs = fs, _isWindows = process.platform.startsWith('win') } = options;

this._fs = _fs;
this._isWindows = _isWindows;
}

async _checkIfService(request: Request) {
const req: RequestOptions = {
method: 'GET',
uri: SERVICE_ENDPOINT,
json: true,
};

return promisify(request)(req)
.then((response) =>
this._parseResponse(response.body, (body) =>
AWSCloudService.parseBody(this.getName(), body)
)
)
.catch(() => this._tryToDetectUuid());
}

/**
* Attempt to load the UUID by checking `/sys/hypervisor/uuid`.
*
Expand All @@ -123,11 +127,12 @@ export class AWSCloudService extends CloudService {
const pathsToCheck = ['/sys/hypervisor/uuid', '/sys/devices/virtual/dmi/id/product_uuid'];
const promises = pathsToCheck.map((path) => promisify(this._fs.readFile)(path, 'utf8'));

return Promise.all(promises).then((uuids) => {
for (let uuid of uuids) {
if (isString(uuid)) {
return Promise.allSettled(promises).then((responses) => {
for (const response of responses) {
let uuid;
if (response.status === 'fulfilled' && isString(response.value)) {
// Some AWS APIs return it lowercase (like the file did in testing), while others return it uppercase
uuid = uuid.trim().toLowerCase();
uuid = response.value.trim().toLowerCase();

// There is a small chance of a false positive here in the unlikely event that a uuid which doesn't
// belong to ec2 happens to be generated with `ec2` as the first three characters.
Expand Down
26 changes: 14 additions & 12 deletions src/plugins/kibana_usage_collection/server/collectors/cloud/detector/azure.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,11 +119,12 @@ describe('Azure', () => {
},
};

const response = AZURE._parseBody(body);
const response = AzureCloudService.parseBody(AZURE.getName(), body)!;
expect(response).not.toBeNull();

expect(response?.getName()).toEqual(AZURE.getName());
expect(response?.isConfirmed()).toEqual(true);
expect(response?.toJSON()).toEqual({
expect(response.getName()).toEqual(AZURE.getName());
expect(response.isConfirmed()).toEqual(true);
expect(response.toJSON()).toEqual({
name: 'azure',
id: 'd4c57456-2b3b-437a-9f1f-7082cf123456',
vm_type: 'Standard_A1',
Expand Down Expand Up @@ -171,11 +172,12 @@ describe('Azure', () => {
},
};

const response = AZURE._parseBody(body);
const response = AzureCloudService.parseBody(AZURE.getName(), body)!;
expect(response).not.toBeNull();

expect(response?.getName()).toEqual(AZURE.getName());
expect(response?.isConfirmed()).toEqual(true);
expect(response?.toJSON()).toEqual({
expect(response.getName()).toEqual(AZURE.getName());
expect(response.isConfirmed()).toEqual(true);
expect(response.toJSON()).toEqual({
name: 'azure',
id: undefined,
vm_type: undefined,
Expand All @@ -189,13 +191,13 @@ describe('Azure', () => {

it('ignores unexpected response body', () => {
// @ts-expect-error
expect(AZURE._parseBody(undefined)).toBe(null);
expect(AzureCloudService.parseBody(AZURE.getName(), undefined)).toBe(null);
// @ts-expect-error
expect(AZURE._parseBody(null)).toBe(null);
expect(AzureCloudService.parseBody(AZURE.getName(), null)).toBe(null);
// @ts-expect-error
expect(AZURE._parseBody({})).toBe(null);
expect(AzureCloudService.parseBody(AZURE.getName(), {})).toBe(null);
// @ts-expect-error
expect(AZURE._parseBody({ privateIp: 'a.b.c.d' })).toBe(null);
expect(AzureCloudService.parseBody(AZURE.getName(), { privateIp: 'a.b.c.d' })).toBe(null);
});
});
});
58 changes: 30 additions & 28 deletions src/plugins/kibana_usage_collection/server/collectors/cloud/detector/azure.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,31 +25,6 @@ interface AzureResponse {
* @internal
*/
export class AzureCloudService extends CloudService {
constructor(options = {}) {
super('azure', options);
}

async _checkIfService(request: Request) {
const req = {
method: 'GET',
uri: SERVICE_ENDPOINT,
headers: {
// Azure requires this header
Metadata: 'true',
},
json: true,
};

const response = await promisify(request)(req);

// Note: there is no fallback option for Azure
if (!response || response.statusCode === 404) {
throw new Error('Azure request failed');
}

return this._parseResponse(response.body, (body) => this._parseBody(body));
}

/**
* Parse the Azure response, if possible.
*
Expand All @@ -76,7 +51,7 @@ export class AzureCloudService extends CloudService {
* }
* }
*/
_parseBody(body: AzureResponse): CloudServiceResponse | null {
static parseBody(name: string, body: AzureResponse): CloudServiceResponse | null {
const compute: Record<string, string> | undefined = get(body, 'compute');
const id = get<Record<string, string>, string>(compute, 'vmId');
const vmType = get<Record<string, string>, string>(compute, 'vmSize');
Expand All @@ -90,12 +65,39 @@ export class AzureCloudService extends CloudService {

// ensure we actually have some data
if (id || vmType || region) {
return new CloudServiceResponse(this._name, true, { id, vmType, region, metadata });
return new CloudServiceResponse(name, true, { id, vmType, region, metadata });
} else if (network) {
// classic-managed VMs in Azure don't provide compute so we highlight the lack of info
return new CloudServiceResponse(this._name, true, { metadata: { classic: true } });
return new CloudServiceResponse(name, true, { metadata: { classic: true } });
}

return null;
}

constructor(options = {}) {
super('azure', options);
}

async _checkIfService(request: Request) {
const req = {
method: 'GET',
uri: SERVICE_ENDPOINT,
headers: {
// Azure requires this header
Metadata: 'true',
},
json: true,
};

const response = await promisify(request)(req);

// Note: there is no fallback option for Azure
if (!response || response.statusCode === 404) {
throw new Error('Azure request failed');
}

return this._parseResponse(response.body, (body) =>
AzureCloudService.parseBody(this.getName(), body)
);
}
}
Loading

0 comments on commit 6e26141

Please sign in to comment.