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

feat: add new securityGroupNoRuleManagementConflicts rule #108

Merged
merged 4 commits into from
Apr 26, 2024
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
1 change: 1 addition & 0 deletions .github/workflows/master.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ jobs:
with:
path: ci-scripts
repository: pulumi/scripts
- run: echo "ci-scripts" >> .git/info/exclude # actions/checkout#197
- name: Configure AWS Credentials
uses: aws-actions/configure-aws-credentials@v1
with:
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ jobs:
with:
path: ci-scripts
repository: pulumi/scripts
- run: echo "ci-scripts" >> .git/info/exclude # actions/checkout#197
- name: Configure AWS Credentials
uses: aws-actions/configure-aws-credentials@v1
with:
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/run-acceptance-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ jobs:
with:
path: ci-scripts
repository: pulumi/scripts
- run: echo "ci-scripts" >> .git/info/exclude # actions/checkout#197
- name: Configure AWS Credentials
uses: aws-actions/configure-aws-credentials@v1
with:
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@
**/package-lock.json

yarn-error.log
**/Pulumi.*.yaml
6 changes: 3 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
## HEAD (Unreleased)

- Added in checks for `alb` and `lb` to the compute and network policies
## 0.5.0

---
- Added in checks for `alb` and `lb` to the compute and network policies
- add new securityGroupNoRuleManagementConflicts rule [#108](https://github.com/pulumi/pulumi-policy-aws/pull/108)

## 0.4.0 (2022-09-22)

Expand Down
112 changes: 112 additions & 0 deletions integration-tests/compute/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ const testScenario = config.getNumber("scenario");

console.log(`Running test scenario #${testScenario}`);

const vpc = new aws.ec2.Vpc('test-vpc', { });
const subnet = new aws.ec2.Subnet('test-subnet-1', {
vpcId: vpc.id,
});
const subnet2 = new aws.ec2.Subnet('test-subnet-2', {
vpcId: vpc.id,
});

const ami = pulumi.output(aws.ec2.getAmi({
filters: [{
name: "name",
Expand Down Expand Up @@ -53,6 +61,7 @@ let elbArgs: aws.elb.LoadBalancerArgs = {
enabled: true,
bucket: elbBucket.arn,
},
subnets: [subnet.id, subnet2.id],
listeners: [],
};

Expand All @@ -61,6 +70,7 @@ let elbV2Args: aws.lb.LoadBalancerArgs = {
enabled: true,
bucket: elbBucket.arn,
},
subnets: [subnet.id, subnet2.id],
enableDeletionProtection: true,
};

Expand All @@ -69,9 +79,15 @@ let albArgs: aws.alb.LoadBalancerArgs = {
enabled: true,
bucket: elbBucket.arn,
},
subnets: [subnet.id, subnet2.id],
enableDeletionProtection: true,
};

let sgArgs: aws.ec2.SecurityGroupArgs = {};
let sgRuleArgs: ((id: pulumi.Input<string>) => aws.ec2.SecurityGroupRuleArgs) | undefined;
let sgEgressRuleArgs: ((id: pulumi.Input<string>) => aws.vpc.SecurityGroupEgressRuleArgs) | undefined;
let sgIngressRuleArgs: ((id: pulumi.Input<string>) => aws.vpc.SecurityGroupIngressRuleArgs) | undefined;

switch (testScenario) {
case 1:
// Happy Path.
Expand Down Expand Up @@ -170,6 +186,92 @@ switch (testScenario) {
}]
};
break;
case 10:
// No SecurityGroupRule of type 'egress' for a SecurityGroup with inline egress rules.
sgArgs = {
egress: [{
toPort: 80,
fromPort: 80,
cidrBlocks: ['0.0.0.0/0'],
protocol: 'tcp',
}],
};

sgRuleArgs = (id: pulumi.Input<string>): aws.ec2.SecurityGroupRuleArgs => {
return {
type: 'egress',
protocol: 'tcp',
cidrBlocks: ['0.0.0.0/0'],
fromPort: 81,
toPort: 81,
securityGroupId: id,
}
}
break;
case 11:
// No SecurityGroupRule of type 'ingress' for a SecurityGroup with inline ingress rules.
sgArgs = {
ingress: [{
toPort: 80,
fromPort: 80,
cidrBlocks: ['0.0.0.0/0'],
protocol: 'tcp',
}],
};

sgRuleArgs = (id: pulumi.Input<string>): aws.ec2.SecurityGroupRuleArgs => {
return {
type: 'ingress',
protocol: 'tcp',
cidrBlocks: ['0.0.0.0/0'],
fromPort: 81,
toPort: 81,
securityGroupId: id,
}
}
break;
case 12:
// No SecurityGroupIngressRule for a SecurityGroup with inline ingress rules.
sgArgs = {
ingress: [{
toPort: 80,
fromPort: 80,
cidrBlocks: ['0.0.0.0/0'],
protocol: 'tcp',
}],
};

sgIngressRuleArgs = (id: pulumi.Input<string>): aws.vpc.SecurityGroupIngressRuleArgs => {
return {
ipProtocol: 'tcp',
cidrIpv4: '0.0.0.0/0',
fromPort: 81,
toPort: 81,
securityGroupId: id,
}
}
break;
case 13:
// No SecurityGroupEgressRule for a SecurityGroup with inline egress rules.
sgArgs = {
egress: [{
toPort: 80,
fromPort: 80,
cidrBlocks: ['0.0.0.0/0'],
protocol: 'tcp',
}],
};

sgEgressRuleArgs = (id: pulumi.Input<string>): aws.vpc.SecurityGroupEgressRuleArgs => {
return {
ipProtocol: 'tcp',
cidrIpv4: '0.0.0.0/0',
fromPort: 81,
toPort: 81,
securityGroupId: id,
}
}
break;
default:
throw new Error(`Unexpected test scenario ${testScenario}`);
}
Expand All @@ -178,3 +280,13 @@ export const ec2Instance = new aws.ec2.Instance("test-ec2-instance", ec2Instance
export const elb = new aws.elb.LoadBalancer("test-elb", elbArgs);
export const elbV2 = new aws.lb.LoadBalancer("test-elb-v2", elbV2Args);
export const alb = new aws.alb.LoadBalancer("test-alb", albArgs);
export const sg = new aws.ec2.SecurityGroup('test-sg', sgArgs);
if (sgRuleArgs !== undefined) {
new aws.ec2.SecurityGroupRule('test-sg-rule', sgRuleArgs(sg.id));
}
if (sgEgressRuleArgs) {
new aws.vpc.SecurityGroupEgressRule('test-sg-egress-rule', sgEgressRuleArgs(sg.id));
}
if (sgIngressRuleArgs) {
new aws.vpc.SecurityGroupIngressRule('test-sg-ingress-rule', sgIngressRuleArgs(sg.id));
}
4 changes: 2 additions & 2 deletions integration-tests/compute/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
"main": "index.ts",
"dependencies": {
"@pulumi/pulumi": "^3.0.0",
"@pulumi/aws": "^4.0.0",
"@pulumi/aws": "^6.0.0",
"@pulumi/awsx": "^0.30.0"
},
"resolutions": {
"@pulumi/aws": "^4.0.0"
"@pulumi/aws": "^6.0.0"
}
}
32 changes: 32 additions & 0 deletions integration-tests/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,5 +250,37 @@ func TestComputeEC2(t *testing.T) {
"The EC2 instance root block device must be encrypted.",
},
},
// Test scenario 10 - A egress type SecurityGroupRule attached to a SecurityGroup with inline rules
{
WantErrors: []string{
"mandatory",
"security-group-no-rule-management-conflicts",
"SecurityGroupRule test-sg-rule defines rules for SecurityGroup test-sg which has inline 'egress' rules",
},
},
// Test scenario 11 - A ingress type SecurityGroupRule attached to a SecurityGroup with inline rules
{
WantErrors: []string{
"mandatory",
"security-group-no-rule-management-conflicts",
"SecurityGroupRule test-sg-rule defines rules for SecurityGroup test-sg which has inline 'ingress' rules",
},
},
// Test scenario 12 - A SecurityGroupIngressRule attached to a SecurityGroup with inline rules
{
WantErrors: []string{
"mandatory",
"security-group-no-rule-management-conflicts",
"SecurityGroupIngressRule test-sg-ingress-rule defines rules for SecurityGroup test-sg which has inline 'ingress' rules",
},
},
// Test scenario 13 - A SecurityGroupEgressRule attached to a SecurityGroup with inline rules
{
WantErrors: []string{
"mandatory",
"security-group-no-rule-management-conflicts",
"SecurityGroupEgressRule test-sg-egress-rule defines rules for SecurityGroup test-sg which has inline 'egress' rules",
},
},
})
}
59 changes: 58 additions & 1 deletion src/compute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,14 @@

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

import { EnforcementLevel, ResourceValidationPolicy, validateResourceOfType } from "@pulumi/policy";
import {
EnforcementLevel,
ReportViolation,
ResourceValidationPolicy,
StackValidationArgs,
StackValidationPolicy,
validateResourceOfType,
} from "@pulumi/policy";

import { registerPolicy } from "./awsGuard";
import { PolicyArgs } from "./policyArgs";
Expand All @@ -27,6 +34,7 @@ declare module "./awsGuard" {
ec2VolumeInUse?: EnforcementLevel | (Ec2VolumeInUseArgs & PolicyArgs);
elbAccessLoggingEnabled?: EnforcementLevel;
encryptedVolumes?: EnforcementLevel | (EncryptedVolumesArgs & PolicyArgs);
securityGroupNoRuleManagementConflicts?: EnforcementLevel;
}
}

Expand Down Expand Up @@ -168,3 +176,52 @@ export const encryptedVolumes: ResourceValidationPolicy = {
}),
};
registerPolicy("encryptedVolumes", encryptedVolumes);

/** @internal */
export const securityGroupNoRuleManagementConflicts: StackValidationPolicy = {
name: "security-group-no-rule-management-conflicts",
description:
"Checks that ec2.SecurityGroup resources do not conflict with vpc.SecurityGroupEgressRule, vpc.SecurityGroupIngressRule, or ec2.SecurityGroupRule.\n"+
Copy link
Member

Choose a reason for hiding this comment

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

The description gets printed by the CLI output, not sure how good multi-line looks there but probably readable.

"See https://www.pulumi.com/registry/packages/aws/api-docs/ec2/securitygroup/ for more details",
validateStack: (args: StackValidationArgs, reportViolation: ReportViolation) => {
args.resources.forEach((resource) => {
let currentType: string;
let type: "ingress" | "egress";
switch (resource.type) {
case "aws:vpc/securityGroupEgressRule:SecurityGroupEgressRule":
currentType = "SecurityGroupEgressRule";
type = "egress";
break;
case "aws:vpc/securityGroupIngressRule:SecurityGroupIngressRule":
currentType = "SecurityGroupIngressRule";
type = "ingress";
break;
case "aws:ec2/securityGroupRule:SecurityGroupRule":
currentType = "SecurityGroupRule";
type = resource.props["type"];
break;
default:
return;
}

if (resource.propertyDependencies["securityGroupId"]) {
resource.propertyDependencies["securityGroupId"].forEach((dep) => {
if (dep.type !== "aws:ec2/securityGroup:SecurityGroup" || !dep.props) {
return;
}
if (type === "ingress" && dep.props["ingress"] && dep.props["ingress"].length > 0) {
reportViolation(
`${currentType} ${resource.name} defines rules for SecurityGroup ${dep.name} which has inline 'ingress' rules`, resource.urn,
);
}
if (type === "egress" && dep.props["egress"] && dep.props["egress"].length > 0) {
reportViolation(
corymhall marked this conversation as resolved.
Show resolved Hide resolved
`${currentType} ${resource.name} defines rules for SecurityGroup ${dep.name} which has inline 'egress' rules`, resource.urn,
);
}
});
}
});
},
};
registerPolicy("securityGroupNoRuleManagementConflicts", securityGroupNoRuleManagementConflicts);
Loading