Skip to content
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

Provide a way to create and configure AWSGuard #33

Merged
merged 7 commits into from
Nov 26, 2019
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
3 changes: 0 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ install:
- export PATH="$HOME/.pulumi/bin:$PATH"
# Install toolchain
- curl -o- -L https://yarnpkg.com/install.sh | bash -s -- --version 1.15.2
- yarn global add tslint
- yarn global add typescript
- yarn global add mocha
# Install dependencies
- make ensure
script:
Expand Down
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ build::
rm -rf bin/

# Build
tsc --project ./src --outDir ./bin/
cd src && yarn run build

# Set version and copy non-source assets.
sed -e 's/\$${VERSION}/$(VERSION)/g' < ./src/package.json > bin/package.json
Expand All @@ -26,10 +26,10 @@ build::

.PHONY: lint
lint::
tslint -c ./src/tslint.json -p ./src/tsconfig.json
cd src && yarn run lint

test_fast::
cd src && mocha --require ./node_modules/ts-node/register tests/**/*.spec.ts
cd src && yarn run test

.PHONY: test_all
test_all::
Expand Down
3 changes: 0 additions & 3 deletions Pulumi.yaml

This file was deleted.

10 changes: 8 additions & 2 deletions integration-tests/compute/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,15 @@ let elbV2Args: aws.elasticloadbalancingv2.LoadBalancerArgs = {
enabled: true,
bucket: elbBucket.arn,
},
enableDeletionProtection: true,
};

let albArgs: aws.applicationloadbalancing.LoadBalancerArgs = {
accessLogs: {
enabled: true,
bucket: elbBucket.arn,
},
enableDeletionProtection: true,
};

switch (testScenario) {
Expand Down Expand Up @@ -110,8 +112,12 @@ switch (testScenario) {
case 5:
// Elastic Load Balancers do not have access logs specified.
elbArgs = { listeners: [] };
elbV2Args = {};
albArgs = {};
elbV2Args = {
enableDeletionProtection: true,
};
albArgs = {
enableDeletionProtection: true,
};
break;
case 6:
// No EBS volume attached to EC2 instance.
Expand Down
2 changes: 2 additions & 0 deletions integration-tests/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ func runPolicyPackIntegrationTest(

e.RunCommand("pulumi", "config", "set", "scenario", fmt.Sprintf("%d", idx+1))

os.Setenv("PULUMI_AWSGUARD_TESTING", "true")

if len(scenario.WantErrors) == 0 {
t.Log("No errors are expected.")
e.RunCommand("pulumi", "preview", "--policy-pack", policyPackDir)
Expand Down
185 changes: 185 additions & 0 deletions src/awsGuard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
// Copyright 2016-2019, Pulumi Corporation.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import {
EnforcementLevel,
Policies,
PolicyPack,
ResourceValidationPolicy,
StackValidationPolicy,
} from "@pulumi/policy";

import { defaultEnforcementLevel, isEnforcementLevel } from "./enforcementLevel";

const defaultPolicyPackName = "pulumi-awsguard";

// Internal map that holds all the registered policy factories.
const factoryMap: Record<string, PolicyFactory<any>> = {};

/**
* A policy pack of rules to enforce AWS best practices for security, reliability, cost, and more!
*
* Create an instance of AwsGuard without any arguments to enable all policies with their default configuration:
*
* ```typescript
* const awsGuard = new AwsGuard();
* ```
*
* The above is equivalent to:
*
* ```typescript
* const awsGuard = new AwsGuard({ all: "advisory" });
* ```
*
* To make all policies mandatory rather than advisory:
*
* ```typescript
* const awsGuard = new AwsGuard({ all: "mandatory" });
* ```
*
* To make all policies mandatory, but change a couple to be advisory:
*
* ```typescript
* const awsGuard = new AwsGuard({
* all: "mandatory",
* ec2InstanceNoPublicIP: "advisory",
* elbAccessLoggingEnabled: "advisory",
* });
* ```
*
* To disable a particular policy:
*
* ```typescript
* const awsGuard = new AwsGuard({
* ec2InstanceNoPublicIP: "disabled",
* });
* ```
*
* To disable all policies except ones explicitly enabled:
*
* ```typescript
* const awsGuard = new AwsGuard({
* all: "disabled",
* ec2InstanceNoPublicIP: "mandatory",
* elbAccessLoggingEnabled: "mandatory",
* });
* ```
*
* To specify configuration for policies that have it:
*
* ```typescript
* const awsGuard = new AwsGuard({
* ec2VolumeInUseCheck: { checkDeletion: false },
* encryptedVolumes: { enforcementLevel: "mandatory", kmsId: "id" },
* redshiftClusterMaintenanceSettingsCheck: { preferredMaintenanceWindow: "Mon:09:30-Mon:10:00" },
* acmCheckCertificateExpiration: { maxDaysUntilExpiration: 10 },
* });
* ```
*/
export class AwsGuard extends PolicyPack {
Copy link
Member Author

Choose a reason for hiding this comment

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

Originally, I had named this AWSGuardPolicyPack, but then opted for simply AWSGuard, and finally changed the casing to match how we case Aws in API signatures in @pulumi/aws resulting in AwsGuard.

constructor(args?: AwsGuardArgs);
constructor(name: string, args?: AwsGuardArgs);
constructor(nameOrArgs?: string | AwsGuardArgs, args?: AwsGuardArgs) {
Comment on lines +91 to +93
Copy link
Member Author

@justinvp justinvp Nov 25, 2019

Choose a reason for hiding this comment

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

This may look odd, if you're unfamiliar with overloads in TypeScript. The first two signatures are the two available overloads, and the third is the implementation (the latter not being shown in intellisense).

This allows the following possible ways of constructing:

new AwsGuard();
new AwsGuard({ all: "advisory"});
new AwsGuard("my-custom-name", { all: "advisory" });

const [n, a] = getNameAndArgs(nameOrArgs, args);
super(n, { policies: getPolicies(factoryMap, a) });
}
}

/**
* Argument bag for configuring AwsGuard policies.
*/
export interface AwsGuardArgs {
all?: EnforcementLevel;
// Note: Properties to configure each policy are added to this interface (mixins) by each module.
}

/** @internal */
export type PolicyFactory<TArgs> = (args?: TArgs) => ResourceValidationPolicy | StackValidationPolicy;

/** @internal */
export function registerPolicy<K extends keyof AwsGuardArgs>(
property: Exclude<K, "all">,
factory: PolicyFactory<AwsGuardArgs[K]>): void {

if (typeof factory !== "function") {
throw new Error("'factory' must be a function.");
}
if (property === "all") {
throw new Error("'all' is reserved.");
}
if (property in factoryMap) {
throw new Error(`${property} already exists.`);
}
factoryMap[property] = factory;
}

/**
* Testable helper to get the name and args from the parameters,
* for use in the policy pack's constructor.
* @internal
*/
export function getNameAndArgs(
nameOrArgs?: string | AwsGuardArgs,
args?: AwsGuardArgs): [string, AwsGuardArgs | undefined] {

let name = defaultPolicyPackName;
if (typeof nameOrArgs === "string") {
justinvp marked this conversation as resolved.
Show resolved Hide resolved
name = nameOrArgs;
} else if (typeof nameOrArgs === "object") {
args = nameOrArgs;
}
return [name, args];
}

/**
* Generates the array of policies based on the specified args.
* @internal
*/
export function getPolicies(factories: Record<string, PolicyFactory<any>>, args?: AwsGuardArgs): Policies {
// If `args` is falsy or empty, enable all policies with the default enforcement level.
if (!args || Object.keys(args).length === 0) {
args = { all: defaultEnforcementLevel };
}
// If `all` isn't set explicitly, clone `args` and set it to the default enforcement level.
if (!args.all) {
args = Object.assign({}, args);
args.all = defaultEnforcementLevel;
}

const policies: Policies = [];

const keys = Object.keys(factories).sort();
for (const key of keys) {
const factory = factories[key];

let factoryArgs: any = undefined;
if (args.hasOwnProperty(key)) {
factoryArgs = (<Record<string, any>>args)[key];
}
if (!factoryArgs) {
factoryArgs = args.all;
}

// If the policy is disabled, skip it.
if (isEnforcementLevel(factoryArgs) && factoryArgs === "disabled") {
continue;
}

const policy = factory(factoryArgs);
policies.push(policy);
}

// Filter out any disabled policies.
return policies.filter(p => p.enforcementLevel !== "disabled");
}
78 changes: 59 additions & 19 deletions src/compute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,50 +13,79 @@
// limitations under the License.

import * as aws from "@pulumi/aws";

import { EnforcementLevel, ResourceValidationPolicy, validateTypedResource } from "@pulumi/policy";

export const compute: ResourceValidationPolicy[] = [
ec2InstanceDetailedMonitoringEnabled("mandatory"),
ec2InstanceNoPublicIP("mandatory"),
ec2VolumeInUseCheck("mandatory", true),
elbAccessLoggingEnabled("mandatory"),
encryptedVolumes("mandatory"),
];
import { registerPolicy } from "./awsGuard";
import { defaultEnforcementLevel } from "./enforcementLevel";
import { PolicyArgs } from "./policyArgs";
import { getValueOrDefault } from "./util";

// Mixin additional properties onto AwsGuardArgs.
declare module "./awsGuard" {
interface AwsGuardArgs {
ec2InstanceDetailedMonitoringEnabled?: EnforcementLevel;
ec2InstanceNoPublicIP?: EnforcementLevel;
ec2VolumeInUseCheck?: EnforcementLevel | Ec2VolumeInUseCheckArgs;
elbAccessLoggingEnabled?: EnforcementLevel;
encryptedVolumes?: EnforcementLevel | EncryptedVolumesArgs;
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the conciseness of how you are declaring the AwsGuardArgs args here using mix-ins. But it has the problem of being really confusing as more and more policies are added to the policy pack. A value like encryptedVolumes not not be unique across all args to all policies.... and the alternative of prefixing the arg with the policy name is kinda lame too.

All this to say: I like the pattern, but I am not sure if it is going to work out for AWS Guard given the large number of rules we plan on adding.

Every alternative I can think of is strictly worse, so let's keep this as-is. Just want calling it out in case this does end up causing a lot of friction, that we should be willing to switch to one of the alternative interfaces you considered if it becomes a big problem. (But it could be I'm being paranoid, and so let's try it this way as it makes policies much easier to author.)

}
}
Comment on lines +25 to +33
Copy link
Member Author

Choose a reason for hiding this comment

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

I considered having separate interfaces for each policy, e.g.:

export interface Ec2InstanceDetailedMonitoringEnabledPolicyPackArgs {
    ec2InstanceDetailedMonitoringEnabled?: EnforcementLevel;
}

export interface Ec2InstanceNoPublicIPPolicyPackArgs {
    ec2InstanceNoPublicIP?: EnforcementLevel;
}

// etc.

And then defining AwsGuardArgs as:

export interface AwsGuardArgs extends
    Ec2InstanceDetailedMonitoringEnabledPolicyPackArgs,
    Ec2InstanceNoPublicIPPolicyPackArgs,
    // etc.
    {
    all?: EnforcementLevel;
}

Or:

export type AwsGuardArgs =
    Ec2InstanceDetailedMonitoringEnabledPolicyPackArgs & 
    Ec2InstanceNoPublicIPPolicyPackArgs &
    // etc.

But then it adds a bunch of unnecessary types and forces you to add the definitions where AwsGuardArgs is declared.

Doing it the way I've done it is consistent with how we apply "mixins" elsewhere, for example: https://github.com/pulumi/pulumi-aws/blob/daa4d0d4ea9ca5c785f2673f2c87c1865473a506/sdk/nodejs/s3/s3Mixins.ts#L215-L246


// Register policy factories.
registerPolicy("ec2InstanceDetailedMonitoringEnabled", ec2InstanceDetailedMonitoringEnabled);
registerPolicy("ec2InstanceNoPublicIP", ec2InstanceNoPublicIP);
registerPolicy("ec2VolumeInUseCheck", ec2VolumeInUseCheck);
registerPolicy("elbAccessLoggingEnabled", elbAccessLoggingEnabled);
registerPolicy("encryptedVolumes", encryptedVolumes);
justinvp marked this conversation as resolved.
Show resolved Hide resolved

export function ec2InstanceDetailedMonitoringEnabled(enforcementLevel: EnforcementLevel = "advisory"): ResourceValidationPolicy {

/** @internal */
export function ec2InstanceDetailedMonitoringEnabled(enforcementLevel?: EnforcementLevel): ResourceValidationPolicy {
return {
name: "ec2-instance-detailed-monitoring-enabled",
description: "Checks whether detailed monitoring is enabled for EC2 instances.",
enforcementLevel: enforcementLevel,
validateResource: validateTypedResource(aws.ec2.Instance, (instance, args, reportViolation) => {
enforcementLevel: enforcementLevel || defaultEnforcementLevel,
validateResource: validateTypedResource(aws.ec2.Instance, (instance, _, reportViolation) => {
if (!instance.monitoring) {
reportViolation("EC2 instances must have detailed monitoring enabled.");
}
}),
};
}

export function ec2InstanceNoPublicIP(enforcementLevel: EnforcementLevel = "advisory"): ResourceValidationPolicy {
/** @internal */
export function ec2InstanceNoPublicIP(enforcementLevel?: EnforcementLevel): ResourceValidationPolicy {
return {
name: "ec2-instance-no-public-ip",
description: "Checks whether Amazon EC2 instances have a public IP association. " +
"This rule applies only to IPv4.",
enforcementLevel: enforcementLevel,
validateResource: validateTypedResource(aws.ec2.Instance, (instance, args, reportViolation) => {
enforcementLevel: enforcementLevel || defaultEnforcementLevel,
validateResource: validateTypedResource(aws.ec2.Instance, (instance, _, reportViolation) => {
if (instance.associatePublicIpAddress) {
reportViolation("EC2 instance must not have a public IP.");
}
}),
};
}

function ec2VolumeInUseCheck(enforcementLevel: EnforcementLevel = "advisory", checkDeletion: boolean): ResourceValidationPolicy {
export interface Ec2VolumeInUseCheckArgs extends PolicyArgs {
checkDeletion?: boolean;
}

/** @internal */
export function ec2VolumeInUseCheck(args?: EnforcementLevel | Ec2VolumeInUseCheckArgs): ResourceValidationPolicy {
const { enforcementLevel, checkDeletion } = getValueOrDefault(args, {
enforcementLevel: defaultEnforcementLevel,
checkDeletion: true,
});

return {
name: "ec2-volume-inuse-check",
description: "Checks whether EBS volumes are attached to EC2 instances. " +
"Optionally checks if EBS volumes are marked for deletion when an instance is terminated.",
enforcementLevel: enforcementLevel,
validateResource: validateTypedResource(aws.ec2.Instance, (instance, args, reportViolation) => {
validateResource: validateTypedResource(aws.ec2.Instance, (instance, _, reportViolation) => {
if (!instance.ebsBlockDevices || instance.ebsBlockDevices.length === 0) {
reportViolation("EC2 instance must have an EBS volume attached.");
}
Expand All @@ -72,11 +101,12 @@ function ec2VolumeInUseCheck(enforcementLevel: EnforcementLevel = "advisory", ch
};
}

export function elbAccessLoggingEnabled(enforcementLevel: EnforcementLevel = "advisory"): ResourceValidationPolicy {
/** @internal */
export function elbAccessLoggingEnabled(enforcementLevel?: EnforcementLevel): ResourceValidationPolicy {
return {
name: "elb-logging-enabled",
description: "Checks whether the Application Load Balancers and the Classic Load Balancers have logging enabled.",
enforcementLevel: enforcementLevel,
enforcementLevel: enforcementLevel || defaultEnforcementLevel,
justinvp marked this conversation as resolved.
Show resolved Hide resolved
validateResource: [
validateTypedResource(aws.elasticloadbalancing.LoadBalancer, (loadBalancer, args, reportViolation) => {
if (loadBalancer.accessLogs === undefined || !loadBalancer.accessLogs.enabled) {
Expand All @@ -97,14 +127,24 @@ export function elbAccessLoggingEnabled(enforcementLevel: EnforcementLevel = "ad
};
}

export function encryptedVolumes(enforcementLevel: EnforcementLevel = "advisory", kmsId?: string): ResourceValidationPolicy {
export interface EncryptedVolumesArgs extends PolicyArgs {
kmsId?: string;
}

/** @internal */
export function encryptedVolumes(args?: EnforcementLevel | EncryptedVolumesArgs): ResourceValidationPolicy {
const { enforcementLevel, kmsId } = getValueOrDefault(args, {
enforcementLevel: defaultEnforcementLevel,
kmsId: undefined,
});

return {
name: "encrypted-volumes",
description: "Checks whether the EBS volumes that are in an attached state are encrypted. " +
"If you specify the ID of a KMS key for encryption using the kmsId parameter, " +
"the rule checks if the EBS volumes in an attached state are encrypted with that KMS key.",
enforcementLevel: enforcementLevel,
validateResource: validateTypedResource(aws.ec2.Instance, (instance, args, reportViolation) => {
validateResource: validateTypedResource(aws.ec2.Instance, (instance, _, reportViolation) => {
if (instance.ebsBlockDevices && instance.ebsBlockDevices.length > 0) {
for (const ebs of instance.ebsBlockDevices) {
if (!ebs.encrypted) {
Expand Down
Loading