Skip to content

Conversation

@naveenkoppula
Copy link
Contributor

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@naveenkoppula naveenkoppula marked this pull request as ready for review September 17, 2020 11:04
@@ -0,0 +1,36 @@
# Security Hub Enable Standards
Copy link
Contributor

Choose a reason for hiding this comment

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

update the documentation

@@ -0,0 +1,36 @@
# Security Hub Enable Standards
Copy link
Contributor

Choose a reason for hiding this comment

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

update the documentation

import { CfnStaticResourcesOutput } from './outputs';

// Changing these values will lead to redeploying all Phase-4 RuleAssociation stacks
const MAX_RESOURCES_IN_STACK = 190;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is Resource per stack (190) for rule association correct? Could you please do a cross check

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah we only need one resource per associating all rules to VPC

Comment on lines 385 to +390
function domainToName(domain: string): string {
return domain.replace(/\./gi, '-');
}

export function createRuleName(name: string): string {
const hash = hashPath([name], 8);
Copy link
Contributor

Choose a reason for hiding this comment

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

Move these 2 functions to common package so that in future, we can reuse them in other modules

@naveenkoppula naveenkoppula merged commit f10ecf5 into feat/PHZ-resolver-rules Sep 18, 2020
@naveenkoppula naveenkoppula deleted the fix/Make-VPC-Related-Deployment-in-Same-Stack branch September 18, 2020 07:24
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