Skip to content

feat(parameters): added BaseProvider class #1168

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.

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

Merged
merged 10 commits into from
Nov 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,11 @@ jobs:
if: steps.cache-node-modules.outputs.cache-hit == 'true'
run: |
npm run build -w packages/commons
npm run build -w packages/logger & npm run build -w packages/tracer & npm run build -w packages/metrics
npm run build -w packages/logger & npm run build -w packages/tracer & npm run build -w packages/metrics -w packages/parameters
- name: Run linting
run: npm run lint -w packages/commons -w packages/logger -w packages/tracer -w packages/metrics
run: npm run lint -w packages/commons -w packages/logger -w packages/tracer -w packages/metrics -w packages/parameters
- name: Run unit tests
run: npm t -w packages/commons -w packages/logger -w packages/tracer -w packages/metrics
run: npm t -w packages/commons -w packages/logger -w packages/tracer -w packages/metrics -w packages/parameters
check-examples:
runs-on: ubuntu-latest
env:
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
"packages/commons",
"packages/logger",
"packages/metrics",
"packages/tracer"
"packages/tracer",
"packages/parameters"
],
"scripts": {
"init-environment": "husky install",
Expand Down
45 changes: 45 additions & 0 deletions packages/parameters/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
module.exports = {
displayName: {
name: 'AWS Lambda Powertools utility: PARAMETERS',
color: 'magenta',
},
'runner': 'groups',
'preset': 'ts-jest',
'transform': {
'^.+\\.ts?$': 'ts-jest',
},
moduleFileExtensions: [ 'js', 'ts' ],
'collectCoverageFrom': [
'**/src/**/*.ts',
'!**/node_modules/**',
],
'testMatch': ['**/?(*.)+(spec|test).ts'],
'roots': [
'<rootDir>/src',
'<rootDir>/tests',
],
'testPathIgnorePatterns': [
'/node_modules/',
],
'testEnvironment': 'node',
'coveragePathIgnorePatterns': [
'/node_modules/',
'/types/',
],
'coverageThreshold': {
'global': {
'statements': 100,
'branches': 100,
'functions': 100,
'lines': 100,
},
},
'coverageReporters': [
'json-summary',
'text',
'lcov'
],
'setupFiles': [
'<rootDir>/tests/helpers/populateEnvironmentVariables.ts'
]
};
54 changes: 54 additions & 0 deletions packages/parameters/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
{
"name": "@aws-lambda-powertools/parameters",
"version": "1.4.1",
"description": "The parameters package for the AWS Lambda Powertools for TypeScript library",
"author": {
"name": "Amazon Web Services",
"url": "https://aws.amazon.com"
},
"publishConfig": {
"access": "public"
},
"scripts": {
"commit": "commit",
"test": "npm run test:unit",
"test:unit": "jest --group=unit --detectOpenHandles --coverage --verbose",
"test:e2e:nodejs12x": "echo \"Not implemented\"",
"test:e2e:nodejs14x": "echo \"Not implemented\"",
"test:e2e:nodejs16x": "echo \"Not implemented\"",
"test:e2e": "echo \"Not implemented\"",
"watch": "jest --watch",
"build": "tsc",
"lint": "eslint --ext .ts --no-error-on-unmatched-pattern src tests",
"lint-fix": "eslint --fix --ext .ts --no-error-on-unmatched-pattern src tests",
"package": "mkdir -p dist/ && npm pack && mv *.tgz dist/",
"package-bundle": "../../package-bundler.sh parameters-bundle ./dist",
"prepare": "npm run build",
"postversion": "git push --tags"
},
"homepage": "https://github.com/awslabs/aws-lambda-powertools-typescript/tree/main/packages/parameters#readme",
"license": "MIT-0",
"main": "./lib/index.js",
"types": "./lib/index.d.ts",
"devDependencies": {},
"files": [
"lib"
],
"repository": {
"type": "git",
"url": "git+https://github.com/awslabs/aws-lambda-powertools-typescript.git"
},
"bugs": {
"url": "https://github.com/awslabs/aws-lambda-powertools-typescript/issues"
},
"dependencies": {},
"keywords": [
"aws",
"lambda",
"powertools",
"ssm",
"secrets",
"serverless",
"nodejs"
]
}
170 changes: 170 additions & 0 deletions packages/parameters/src/BaseProvider.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
import { fromBase64 } from '@aws-sdk/util-base64-node';
import { GetOptions } from './GetOptions';
import { GetMultipleOptions } from './GetMultipleOptions';
import { ExpirableValue } from './ExpirableValue';
import { TRANSFORM_METHOD_BINARY, TRANSFORM_METHOD_JSON } from './constants';
import { GetParameterError, TransformParameterError } from './Exceptions';
import type { BaseProviderInterface, GetMultipleOptionsInterface, GetOptionsInterface, TransformOptions } from './types';

abstract class BaseProvider implements BaseProviderInterface {
protected store: Map<string, ExpirableValue>;

public constructor () {
this.store = new Map();
}

public addToCache(key: string, value: string | Record<string, unknown>, maxAge: number): void {
if (maxAge <= 0) return;

this.store.set(key, new ExpirableValue(value, maxAge));
}

public clearCache(): void {
this.store.clear();
}

/**
* Retrieve a parameter value or return the cached value
*
* If there are multiple calls to the same parameter but in a different transform, they will be stored multiple times.
* This allows us to optimize by transforming the data only once per retrieval, thus there is no need to transform cached values multiple times.
*
* However, this means that we need to make multiple calls to the underlying parameter store if we need to return it in different transforms.
*
* Since the number of supported transform is small and the probability that a given parameter will always be used in a specific transform,
* this should be an acceptable tradeoff.
*
* @param {string} name - Parameter name
* @param {GetOptionsInterface} options - Options to configure maximum age, trasformation, AWS SDK options, or force fetch
*/
public async get(name: string, options?: GetOptionsInterface): Promise<undefined | string | Record<string, unknown>> {
const configs = new GetOptions(options);
const key = [ name, configs.transform ].toString();
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 couldn't think of a more elegant way of constructing this key.

For context:

  • Python uses a Tuple, which are not supported in JS (only stage 2)
  • I was initially using objects like { name: string, transform: TransformOptions } since we are using a Map, but given that Map.has and Map.get use object identity to compare the key it would never return a match - also it's not possible to extend/customize the equality function used by a Map
  • Set have the same limitations described above and that apply to Map
  • I briefly considered using external modules like immutable-js or custom implementations of Set, but opted not to in order to avoid introducing a dependency over this otherwise limited usage.

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as the test cases are rigorous enough, this should be ok. I cannot see how we can run into having a duplicated here, can we?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree.

I wasn't worried about duplicates as much as special characters and such.


if (!configs.forceFetch && !this.hasKeyExpiredInCache(key)) {
// If the code enters in this block, then the key must exist & not have been expired
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return this.store.get(key)!.value;
}

let value;
try {
value = await this._get(name, options?.sdkOptions);
} catch (error) {
throw new GetParameterError((error as Error).message);
}

if (value && configs.transform) {
value = transformValue(value, configs.transform, true);
}

if (value) {
this.addToCache(key, value, configs.maxAge);
}

// TODO: revisit return type once providers are implemented, it might be missing binary when not transformed
return value;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As mentioned in the comment here, I stayed generic with types because at this stage I'm not yet sure what each provider/AWS SDK (from _get) will return.

I have opened #1172 to track this work.

}

public async getMultiple(path: string, options?: GetMultipleOptionsInterface): Promise<undefined | Record<string, unknown>> {
const configs = new GetMultipleOptions(options || {});
const key = [ path, configs.transform ].toString();

if (!configs.forceFetch && !this.hasKeyExpiredInCache(key)) {
// If the code enters in this block, then the key must exist & not have been expired
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return this.store.get(key)!.value as Record<string, unknown>;
}

let values: Record<string, unknown> = {};
try {
values = await this._getMultiple(path, options?.sdkOptions);
} catch (error) {
throw new GetParameterError((error as Error).message);
}

if (Object.keys(values) && configs.transform) {
values = transformValues(values, configs.transform, configs.throwOnTransformError);
}

if (Array.from(Object.keys(values)).length !== 0) {
this.addToCache(key, values, configs.maxAge);
}

// TODO: revisit return type once providers are implemented, it might be missing something
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 have opened #1172 to track this work.

return values;
}

/**
* Retrieve parameter value from the underlying parameter store
*
* @param {string} name - Parameter name
* @param {unknown} sdkOptions - Options to pass to the underlying AWS SDK
*/
protected abstract _get(name: string, sdkOptions?: unknown): Promise<string | undefined>;

protected abstract _getMultiple(path: string, sdkOptions?: unknown): Promise<Record<string, string|undefined>>;

/**
* Check whether a key has expired in the cache or not
*
* It returns true if the key is expired or not present in the cache.
*
* @param {string} key - Stringified representation of the key to retrieve
*/
private hasKeyExpiredInCache(key: string): boolean {
const value = this.store.get(key);
if (value) return value.isExpired();

return true;
}

}

// TODO: revisit `value` type once we are clearer on the types returned by the various SDKs
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 have opened #1172 to track this work.

const transformValue = (value: unknown, transform: TransformOptions, throwOnTransformError: boolean, key: string = ''): string | Record<string, unknown> | undefined => {
try {
const normalizedTransform = transform.toLowerCase();
if (
(normalizedTransform === TRANSFORM_METHOD_JSON ||
(normalizedTransform === 'auto' && key.toLowerCase().endsWith(`.${TRANSFORM_METHOD_JSON}`))) &&
typeof value === 'string'
) {
return JSON.parse(value) as Record<string, unknown>;
} else if (
(normalizedTransform === TRANSFORM_METHOD_BINARY ||
(normalizedTransform === 'auto' && key.toLowerCase().endsWith(`.${TRANSFORM_METHOD_BINARY}`))) &&
typeof value === 'string'
) {
return new TextDecoder('utf-8').decode(fromBase64(value));
} else {
// TODO: revisit this type once we are clearer on types returned by SDKs
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 have opened #1172 to track this work.

return value as string;
}
} catch (error) {
if (throwOnTransformError)
throw new TransformParameterError(transform, (error as Error).message);

return;
}
};

const transformValues = (value: Record<string, unknown>, transform: TransformOptions, throwOnTransformError: boolean): Record<string, unknown> => {
const transformedValues: Record<string, unknown> = {};
for (const [ entryKey, entryValue ] of Object.entries(value)) {
try {
transformedValues[entryKey] = transformValue(entryValue, transform, throwOnTransformError, entryKey);
} catch (error) {
if (throwOnTransformError)
throw new TransformParameterError(transform, (error as Error).message);
}
}

return transformedValues;
};

export {
BaseProvider,
ExpirableValue,
transformValue,
};
14 changes: 14 additions & 0 deletions packages/parameters/src/Exceptions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
class GetParameterError extends Error {}

class TransformParameterError extends Error {
public constructor(transform: string, message: string) {
super(message);

this.message = `Unable to transform value using '${transform}' transform: ${message}`;
}
}

export {
GetParameterError,
TransformParameterError,
};
20 changes: 20 additions & 0 deletions packages/parameters/src/ExpirableValue.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import type { ExpirableValueInterface } from './types';

class ExpirableValue implements ExpirableValueInterface {
public ttl: number;
public value: string | Record<string, unknown>;

public constructor(value: string | Record<string, unknown>, maxAge: number) {
this.value = value;
const timeNow = new Date();
this.ttl = timeNow.setSeconds(timeNow.getSeconds() + maxAge);
}

public isExpired(): boolean {
return this.ttl < Date.now();
}
}

export {
ExpirableValue
};
18 changes: 18 additions & 0 deletions packages/parameters/src/GetMultipleOptions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { DEFAULT_MAX_AGE_SECS } from './constants';
import type { GetMultipleOptionsInterface, TransformOptions } from './types';

class GetMultipleOptions implements GetMultipleOptionsInterface {
public forceFetch: boolean = false;
public maxAge: number = DEFAULT_MAX_AGE_SECS;
public sdkOptions?: unknown;
public throwOnTransformError: boolean = false;
public transform?: TransformOptions;

public constructor(options: GetMultipleOptionsInterface) {
Object.assign(this, options);
}
}

export {
GetMultipleOptions
};
17 changes: 17 additions & 0 deletions packages/parameters/src/GetOptions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { DEFAULT_MAX_AGE_SECS } from './constants';
import type { GetOptionsInterface, TransformOptions } from './types';

class GetOptions implements GetOptionsInterface {
public forceFetch: boolean = false;
public maxAge: number = DEFAULT_MAX_AGE_SECS;
public sdkOptions?: unknown;
public transform?: TransformOptions;

public constructor(options: GetOptionsInterface = {}) {
Object.assign(this, options);
}
}

export {
GetOptions
};
3 changes: 3 additions & 0 deletions packages/parameters/src/constants.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const DEFAULT_MAX_AGE_SECS = 5;
export const TRANSFORM_METHOD_JSON = 'json';
export const TRANSFORM_METHOD_BINARY = 'binary';
2 changes: 2 additions & 0 deletions packages/parameters/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export * from './BaseProvider';
export * from './Exceptions';
Loading