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

Conversation

corymhall
Copy link
Contributor

The
docs have strongly worded note about not using inline rules along with separate security group resources. This PR adds a new rule which will warn the user when they use both together.

re pulumi/pulumi-aws#3788

The
[docs](https://www.pulumi.com/registry/packages/aws/api-docs/ec2/securitygroup/)
have strongly worded note about not using inline rules along with
separate security group resources. This PR adds a new rule which will
warn the user when they use both together.

re pulumi/pulumi-aws#3788
@corymhall corymhall self-assigned this Apr 23, 2024
@t0yv0
Copy link
Member

t0yv0 commented Apr 24, 2024

Great to have a test! Code looks good. Waiting on an ack that we can ship this in this repo, similar to my other PR.

@corymhall corymhall added this to the 0.103 milestone Apr 24, 2024
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.

src/compute.ts Show resolved Hide resolved
Copy link
Member

@t0yv0 t0yv0 left a comment

Choose a reason for hiding this comment

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

LGTM, I'd err on the side of merging. I think we're leaning to this being the right place for the check. but can always move later.

@corymhall corymhall merged commit 4b50aae into master Apr 26, 2024
5 checks passed
@corymhall corymhall deleted the corymhall/sg-rule branch April 26, 2024 18:28
@t0yv0 t0yv0 mentioned this pull request May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants