Skip to content

Commit

Permalink
Merge branch 'master' into otaviom/integ-test-case-construct
Browse files Browse the repository at this point in the history
  • Loading branch information
mergify[bot] authored Apr 12, 2022
2 parents fb4f15d + 42e5d08 commit 8db678f
Show file tree
Hide file tree
Showing 45 changed files with 1,331 additions and 69 deletions.
23 changes: 23 additions & 0 deletions .github/workflows/close-stale-prs.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
on:
schedule:
# Cron format: min hr day month dow
- cron: "0 0 * * *"
jobs:
rix0rrr/close-stale-prs:
permissions:
pull-requests: write
runs-on: ubuntu-latest
steps:
- uses: rix0rrr/close-stale-prs@main
with:
# Required
github-token: ${{ secrets.GITHUB_TOKEN }}
stale-days: 21
response-days: 7

# Optional
important-checks-regex: AutoBuildProject89A8053A
warn-message: This PR has been in the STATE state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.
close-message: This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.
skip-labels: contribution/core
close-label: closed-for-staleness
22 changes: 22 additions & 0 deletions packages/@aws-cdk/aws-certificatemanager/test/certificate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,28 @@ describe('CertificateValidation.fromDns', () => {
});
});

test('with an imported hosted zone', () => {
const stack = new Stack();

const exampleCom = route53.PublicHostedZone.fromHostedZoneId(stack, 'ExampleCom', 'sampleid');

new Certificate(stack, 'Certificate', {
domainName: 'test.example.com',
validation: CertificateValidation.fromDns(exampleCom),
});

Template.fromStack(stack).hasResourceProperties('AWS::CertificateManager::Certificate', {
DomainName: 'test.example.com',
DomainValidationOptions: [
{
DomainName: 'test.example.com',
HostedZoneId: 'sampleid',
},
],
ValidationMethod: 'DNS',
});
});

test('with hosted zone and a wildcard name', () => {
const stack = new Stack();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,9 @@ export class EdgeFunction extends Resource implements lambda.IVersion {
public grantInvoke(identity: iam.IGrantable): iam.Grant {
return this.lambda.grantInvoke(identity);
}
public grantInvokeUrl(identity: iam.IGrantable): iam.Grant {
return this.lambda.grantInvokeUrl(identity);
}
public metric(metricName: string, props?: cloudwatch.MetricOptions): cloudwatch.Metric {
return this.lambda.metric(metricName, { ...props, region: EdgeFunction.EDGE_REGION });
}
Expand All @@ -137,6 +140,9 @@ export class EdgeFunction extends Resource implements lambda.IVersion {
public configureAsyncInvoke(options: lambda.EventInvokeConfigOptions): void {
return this.lambda.configureAsyncInvoke(options);
}
public addFunctionUrl(options?: lambda.FunctionUrlOptions): lambda.FunctionUrl {
return this.lambda.addFunctionUrl(options);
}

/** Create a function in-region */
private createInRegionFunction(props: lambda.FunctionProps): FunctionConfig {
Expand Down
6 changes: 5 additions & 1 deletion packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ArnFormat, Lazy, Stack, Token } from '@aws-cdk/core';
import { ArnFormat, Lazy, Stack, Token, Annotations } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { IAlarmAction } from './alarm-action';
import { AlarmBase, IAlarm } from './alarm-base';
Expand Down Expand Up @@ -203,6 +203,10 @@ export class Alarm extends AlarmBase {
label: `${this.metric} ${OPERATOR_SYMBOLS[comparisonOperator]} ${props.threshold} for ${datapoints} datapoints within ${describePeriod(props.evaluationPeriods * metricPeriod(props.metric).toSeconds())}`,
value: props.threshold,
};

for (const w of this.metric.warnings ?? []) {
Annotations.of(this).addWarning(w);
}
}

/**
Expand Down
24 changes: 23 additions & 1 deletion packages/@aws-cdk/aws-cloudwatch/lib/dashboard.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Lazy, Resource, Stack, Token } from '@aws-cdk/core';
import { Lazy, Resource, Stack, Token, Annotations } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { CfnDashboard } from './cloudwatch.generated';
import { Column, Row } from './layout';
Expand Down Expand Up @@ -127,7 +127,29 @@ export class Dashboard extends Resource {
return;
}

const warnings = allWidgetsDeep(widgets).flatMap(w => w.warnings ?? []);
for (const w of warnings) {
Annotations.of(this).addWarning(w);
}

const w = widgets.length > 1 ? new Row(...widgets) : widgets[0];
this.rows.push(w);
}
}

function allWidgetsDeep(ws: IWidget[]) {
const ret = new Array<IWidget>();
ws.forEach(recurse);
return ret;

function recurse(w: IWidget) {
ret.push(w);
if (hasSubWidgets(w)) {
w.widgets.forEach(recurse);
}
}
}

function hasSubWidgets(w: IWidget): w is IWidget & { widgets: IWidget[] } {
return 'widgets' in w;
}
4 changes: 4 additions & 0 deletions packages/@aws-cdk/aws-cloudwatch/lib/graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ export class GraphWidget extends ConcreteWidget {
this.props = props;
this.leftMetrics = props.left ?? [];
this.rightMetrics = props.right ?? [];
this.copyMetricWarnings(...this.leftMetrics, ...this.rightMetrics);
}

/**
Expand All @@ -265,6 +266,7 @@ export class GraphWidget extends ConcreteWidget {
*/
public addLeftMetric(metric: IMetric) {
this.leftMetrics.push(metric);
this.copyMetricWarnings(metric);
}

/**
Expand All @@ -274,6 +276,7 @@ export class GraphWidget extends ConcreteWidget {
*/
public addRightMetric(metric: IMetric) {
this.rightMetrics.push(metric);
this.copyMetricWarnings(metric);
}

public toJson(): any[] {
Expand Down Expand Up @@ -343,6 +346,7 @@ export class SingleValueWidget extends ConcreteWidget {
constructor(props: SingleValueWidgetProps) {
super(props.width || 6, props.height || 3);
this.props = props;
this.copyMetricWarnings(...props.metrics);
}

public toJson(): any[] {
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-cloudwatch/lib/layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export class Row implements IWidget {
/**
* List of contained widgets
*/
private readonly widgets: IWidget[];
public readonly widgets: IWidget[];

/**
* Relative position of each widget inside this row
Expand Down Expand Up @@ -70,7 +70,7 @@ export class Column implements IWidget {
/**
* List of contained widgets
*/
private readonly widgets: IWidget[];
public readonly widgets: IWidget[];

constructor(...widgets: IWidget[]) {
this.widgets = widgets;
Expand Down
9 changes: 9 additions & 0 deletions packages/@aws-cdk/aws-cloudwatch/lib/metric-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@ import { Duration } from '@aws-cdk/core';
* Interface for metrics
*/
export interface IMetric {
/**
* Any warnings related to this metric
*
* Should be attached to the consuming construct.
*
* @default - None
*/
readonly warnings?: string[];

/**
* Inspect the details of the metric object
*/
Expand Down
52 changes: 50 additions & 2 deletions packages/@aws-cdk/aws-cloudwatch/lib/metric.ts
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,11 @@ export class MathExpression implements IMetric {
*/
public readonly searchRegion?: string;

/**
* Warnings generated by this math expression
*/
public readonly warnings?: string[];

constructor(props: MathExpressionProps) {
this.period = props.period || cdk.Duration.minutes(5);
this.expression = props.expression;
Expand All @@ -568,6 +573,27 @@ export class MathExpression implements IMetric {
}

this.validateNoIdConflicts();

// Check that all IDs used in the expression are also in the `usingMetrics` map. We
// can't throw on this anymore since we didn't use to do this validation from the start
// and now there will be loads of people who are violating the expected contract, but
// we can add warnings.
const missingIdentifiers = allIdentifiersInExpression(this.expression).filter(i => !this.usingMetrics[i]);

const warnings = [];

if (missingIdentifiers.length > 0) {
warnings.push(`Math expression '${this.expression}' references unknown identifiers: ${missingIdentifiers.join(', ')}. Please add them to the 'usingMetrics' map.`);
}

// Also copy warnings from deeper levels so graphs, alarms only have to inspect the top-level objects
for (const m of Object.values(this.usingMetrics)) {
warnings.push(...m.warnings ?? []);
}

if (warnings.length > 0) {
this.warnings = warnings;
}
}

/**
Expand Down Expand Up @@ -677,15 +703,27 @@ export class MathExpression implements IMetric {
});
}
}

}

const VALID_VARIABLE = new RegExp('^[a-z][a-zA-Z0-9_]*$');
/**
* Pattern for a variable name. Alphanum starting with lowercase.
*/
const VARIABLE_PAT = '[a-z][a-zA-Z0-9_]*';

const VALID_VARIABLE = new RegExp(`^${VARIABLE_PAT}$`);
const FIND_VARIABLE = new RegExp(VARIABLE_PAT, 'g');

function validVariableName(x: string) {
return VALID_VARIABLE.test(x);
}

/**
* Return all variable names used in an expression
*/
function allIdentifiersInExpression(x: string) {
return Array.from(matchAll(x, FIND_VARIABLE)).map(m => m[0]);
}

/**
* Properties needed to make an alarm from a metric
*/
Expand Down Expand Up @@ -842,3 +880,13 @@ interface IModifiableMetric {
function isModifiableMetric(m: any): m is IModifiableMetric {
return typeof m === 'object' && m !== null && !!m.with;
}

// Polyfill for string.matchAll(regexp)
function matchAll(x: string, re: RegExp): RegExpMatchArray[] {
const ret = new Array<RegExpMatchArray>();
let m: RegExpExecArray | null;
while (m = re.exec(x)) {
ret.push(m);
}
return ret;
}
16 changes: 16 additions & 0 deletions packages/@aws-cdk/aws-cloudwatch/lib/widget.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { IMetric } from './metric-types';

/**
* The width of the grid we're filling
*/
Expand All @@ -17,6 +19,11 @@ export interface IWidget {
*/
readonly height: number;

/**
* Any warnings that are produced as a result of putting together this widget
*/
readonly warnings?: string[];

/**
* Place the widget at a given position
*/
Expand All @@ -39,6 +46,8 @@ export abstract class ConcreteWidget implements IWidget {
protected x?: number;
protected y?: number;

public readonly warnings: string[] | undefined = [];

constructor(width: number, height: number) {
this.width = width;
this.height = height;
Expand All @@ -54,4 +63,11 @@ export abstract class ConcreteWidget implements IWidget {
}

public abstract toJson(): any[];

/**
* Copy the warnings from the given metric
*/
protected copyMetricWarnings(...ms: IMetric[]) {
this.warnings?.push(...ms.flatMap(m => m.warnings ?? []));
}
}
18 changes: 17 additions & 1 deletion packages/@aws-cdk/aws-cloudwatch/test/alarm.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Match, Template } from '@aws-cdk/assertions';
import { Match, Template, Annotations } from '@aws-cdk/assertions';
import { Duration, Stack } from '@aws-cdk/core';
import { Construct } from 'constructs';
import { Alarm, IAlarm, IAlarmAction, Metric, MathExpression, IMetric } from '../lib';
Expand Down Expand Up @@ -252,6 +252,22 @@ describe('Alarm', () => {
ExtendedStatistic: 'tm99.9999999999',
});
});

test('metric warnings are added to Alarm', () => {
const stack = new Stack(undefined, 'MyStack');
const m = new MathExpression({ expression: 'oops' });

// WHEN
new Alarm(stack, 'MyAlarm', {
metric: m,
evaluationPeriods: 1,
threshold: 1,
});

// THEN
const template = Annotations.fromStack(stack);
template.hasWarning('/MyStack/MyAlarm', Match.stringLikeRegexp("Math expression 'oops' references unknown identifiers"));
});
});

class TestAlarmAction implements IAlarmAction {
Expand Down
19 changes: 17 additions & 2 deletions packages/@aws-cdk/aws-cloudwatch/test/dashboard.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Template } from '@aws-cdk/assertions';
import { Template, Annotations, Match } from '@aws-cdk/assertions';
import { App, Stack } from '@aws-cdk/core';
import { Dashboard, GraphWidget, PeriodOverride, TextWidget } from '../lib';
import { Dashboard, GraphWidget, PeriodOverride, TextWidget, MathExpression } from '../lib';

describe('Dashboard', () => {
test('widgets in different adds are laid out underneath each other', () => {
Expand Down Expand Up @@ -175,8 +175,23 @@ describe('Dashboard', () => {

// THEN
expect(() => toThrow()).toThrow(/field dashboardName contains invalid characters/);
});

test('metric warnings are added to dashboard', () => {
const app = new App();
const stack = new Stack(app, 'MyStack');
const m = new MathExpression({ expression: 'oops' });

// WHEN
new Dashboard(stack, 'MyDashboard', {
widgets: [
[new GraphWidget({ left: [m] }), new TextWidget({ markdown: 'asdf' })],
],
});

// THEN
const template = Annotations.fromStack(stack);
template.hasWarning('/MyStack/MyDashboard', Match.stringLikeRegexp("Math expression 'oops' references unknown identifiers"));
});
});

Expand Down
4 changes: 0 additions & 4 deletions packages/@aws-cdk/aws-cloudwatch/test/graphs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -681,8 +681,6 @@ describe('Graphs', () => {
setPeriodToTimeRange: true,
},
}]);


});

test('GraphWidget supports stat and period', () => {
Expand Down Expand Up @@ -710,7 +708,5 @@ describe('Graphs', () => {
period: 172800,
},
}]);


});
});
Loading

0 comments on commit 8db678f

Please sign in to comment.