-
Notifications
You must be signed in to change notification settings - Fork 89
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
Defaults & configuration policy #25
Comments
Design concept: We add a method to public getPropsForResource(className: string, props: any): any {
return props;
} Then, we somehow enforce that all L2s will call this method when they are initialized and pass in the This will allow users to override this method at the stack level (e.g. their base public getPropsForResource(className: string, props: any): any {
if (className === 'aws-s3.Bucket') {
if (props.encryption && props.encryption !== ENCRYPTED) { throw new Error('buckets must be encrypted'); }
props.encryption = ENCRYPTED;
}
return props; // passthrough
} Something along those lines... Just an option. |
I have tried to create one example of enforcing user to follow organization defaults and if you want you can create new resources or modify existing resource using Aspects. |
Isn't this already possible to be implemented using Aspects? |
@Dzhuneyt Aspects can (potentially) mutate constructs after they have been initialized but there are many initialization properties that cannot be modified after the instance is created. |
One thing I've experimented with is in my attempt to create an "organization standard library" is trying to add my own logic to the |
Ha! We recently changed the api for validations and you should be able to just do this: this.node.addValidation(...); See: https://docs.aws.amazon.com/cdk/api/latest/docs/constructs.Node.html#addwbrvalidationvalidation |
would an upstream base validation aspect to support easily adding these validations to a given construct type be useful for this rfc? I am thinking something like this (pseudo code-ish): class ValidatorAspect implements core.IAspect {
type: constructs.IConstructs;
fn: constructs.IValidation
constructor(type: constructs.IConstruct, fn: constructs.IValidation) {
this.type = type
this.fn = fn
}
public visit(node: constructs.IConstruct): void {
const n = node as constructs.Node;
if (node instanceof this.type) {
n.addValidation({
validate: this.fn
});
}
}
} it's more reactive compared to the getPropsForResource design, but i can see both being useful. I'd be happy to work on adding something like this if there's interest. |
@ericzbeard gave an awesome talk to my company a few months ago. He mentioned that companies keep trying to make a CDK L2.5 library. (That's what I wanted to do first too!) He mentioned that this was not recommended. This concept really seems to fix that issue. In his blog post, he has a section about "Don't use constructs for compliance"
While, I totally agree that we should use tools like CFN Guard, there is still a need and it sounds like Aspects is not the full answer.
I want to sell the idea that the current level of upvotes (8) does not reflect the actual need for this concept. From the blog post, this sounds like this might "serve a need" to a "common pattern" but doesn't have the drawbacks and headaches of a company specific wrapper. |
I can vouch for @maria-jackson's thoughts above. Currently in my organization, we are struggling to find a good balance. On the one hand, we want to provide the ability for engineers to create infrastructure which follows industry best practice. On the other hand, we want engineers to build infrastructure easily which is safe, secure, and adheres to our organization requirements. For example, one of our organization policies is that no S3 bucket should be public by default. Presently we really have 3 ways to accomplish this:
While cfn-guard would potentially solve this, the downside is that it would likely get you feedback in the pipeline (unless you run it locally). Instead, if we were able to bake these checks into CDK synthing (or just into the native constructs via a hook like |
None of these is integral to the solution, but they would be nice to have. Take the pseudo code as an illustration of an idea, not a preferred implementation. (I do not understand CDK or Typescript enough) Easy to Read Configuration CodeI'd really like the reusable library to be really easy to read and look a lot like how you pass CDK parameters. I want it to be very clear what people are getting and very clear to our Security folks what the defaults are.
L1, L2, and L3 Default ConfigurationsAs the CDK community expands aws-solutions-constructs, cdk-patterns, and their own L3 libraries, I see a need to provide defaults to L3, not just L2s. For example, I use aws-apigateway-dynamodb and pretend that it defaults to no encryption. While I could pass the value of the decryption every time to the L3 construct, I don't want to. If the solution just provides L2 defaults, those would be overwritten every time.
Multiple Default ConfigurationsI'm also wondering how multiple defaults would work. If my company had C-L2, my BU had B-L2, and my team had T-L2. I'd have things in my BU that I'd want to come before my company, but I'd also like the latest updates from my company.
Easy Translation from cfn-guardIt would be cool if your cfn-guard rules could generate your CDK defaults. Like, this could translated to something very specific But this could not |
Thanks for the input, @maria-jackson ! |
@maria-jackson How does the defaults approach help when using libraries like aws-solutions-constructs? You would have to create each resource manually and pass them into the L3 constructs which certainly loses some of the benefit of those libraries. |
The solution below meets "Allow users to specify or install some code that controls the defaults of AWS resources," but not "or applies some policy or validation at the stack or app level." It more meets the Best practices for building company default constructs #3235, but as that is considered a duplicate of this issue I thought it was appropriate to add the discussion here. Looking at https://github.com/awslabs/aws-solutions-constructs, here's the pattern I'd like the aws-cdk maintainers thoughts on as an alternative to the L2.5 library for default properties for "Defaults & configuration polic[ies]" or "compliant constructs."
|
I also like the AWS Solutions Constructs approach (thanks @maria-jackson for the great summary!). The company could have its own libraries with the various defaults. The security team can own I think that another important capability is to layer multiple defaults, and allow the application owners to reset some properties before creating the construct. For example, the defaults for public website might include Note: I added non-existing Edit: If you want to experiment with that approach, I created a simple example: https://github.com/alexpulver/company-guardrails company_cdk_security/aws_s3.py from aws_cdk import aws_s3 as s3
from aws_cdk import core as cdk
class BucketPropsCollection:
public_access = s3.BucketProps(enforce_ssl=True, public_read_access=True)
__expiration_lifecycle_rule = s3.LifecycleRule(expiration=cdk.Duration.days(30))
retention_policy = s3.BucketProps(lifecycle_rules=[__expiration_lifecycle_rule]) app.py from aws_cdk import aws_s3 as s3
from aws_cdk import core as cdk
# import company_cdk_solutions
from company_cdk_security import aws_s3 as security_s3
# from company_cdk_operations import aws_s3 as operations_s3
# from company_cdk8s_security import deployment as security_deployment
app = cdk.App()
stack = cdk.Stack(app, "Website")
# I am using Python's dict.update([other]) semantics:
# https://docs.python.org/3/library/stdtypes.html#dict.update
bucket_props = security_s3.BucketPropsCollection.public_access
bucket_props.update(security_s3.BucketPropsCollection.retention_policy)
bucket_props.update(s3.BucketProps(versioned=True, enforce_ssl=False))
bucket = s3.Bucket.from_props(stack, "Bucket", bucket_props)
app.synth() |
I really like this discussion, and I agree with the general direction where this is going! I have a few comments on the details of each of the proposed solutions. @maria-jackson I think, instead of having an So, the code becomes: const logGroup = new logs.LogGroup(this, 'LogGroup', defaultLogGroupProps({
retention: logs.RetentionDays.TWO_MONTHS, // we can override any property of defaultLogGroupProps() this way
}); Which I think looks great. @alexpulver I think you want to make these functions as well - making them class fields can have weird side-effects, as they are mutable. You don't have the nice class BucketPropsCollection:
def public_access_props(**kwargs):
return s3.BucketProps(enforce_ssl=True, public_read_access=True, **kwargs) And then, in your CDK app: from aws_cdk import aws_s3 as s3
from aws_cdk import core as cdk
# import company_cdk_solutions
from company_cdk_security import aws_s3 as security_s3
app = cdk.App()
stack = cdk.Stack(app, "Website")
bucket = s3.Bucket(stack, "Bucket", security_s3.BucketPropsCollection.public_access_props(
versioned=True,
enforce_ssl=False,
))
app.synth() I'm sure we can get some nice types here too, but I'm not that familiar with Python to add them 🙂. |
Thanks for the feedback! I changed to functions; it makes sense :). In Python, when specifying the same parameter twice, interpreter bombs. For example, it can be As I mentioned above, I think we should support layered/chained construction of properties. For example, there could be methods to support compliance with FedRAMP (see example at the end from AWS Config conformance pack sample template), SOC, and other standards. If I need to comply with both FedRAMP and SOC, as a policy library consumer, I want to apply both, and trust the AWS CDK API to do the right/defined thing. @skinny85 if I iterate a bit on the experience you suggested, it would be something like below - what do you think?
Another reason for having the from aws_cdk import aws_s3 as s3
from aws_cdk import core as cdk
from company_cdk_security import aws_s3 as security_s3
app = cdk.App()
stack = cdk.Stack(app, "Website")
bucket_props = cdk.Props.merge(
security_s3.BucketPropsCollection.public_access(),
security_s3.BucketPropsCollection.retention_policy(),
s3.BucketProps(versioned=True, enforce_ssl=False),
)
bucket = s3.Bucket.from_props(stack, "Bucket", bucket_props)
app.synth() This way, policy library providers would just define collections of policies, and let the clients to combine them in the desired order. |
No, it's not black and white unfortunately. There are policies which should apply to 99% of apps. When you have 100+ apps, there's going to be an app where the risk of not complying to a particular control is worth the reward and an exception gets put in place. |
@shawnsparks-work I agree, and probably more than 1%, those should be ok with "choose your own adventure", no? |
Ultimately, I think there are a couple of different components to this discussion, which might benefit from splitting it out into multiple topics. There's a library-level component, like an Aspect-style enforcement or L2.5 constructs. There's also an organization-level component, which would allow an Org to enforce best practices uniformly. I'm working on a formal proposal for the second piece, but to give a high level summary: This proposal attempts to address the Org component with a new L3 Construct and CLI command. It enables static analysis locally and as part of a CI/CD pipeline, which allow developers and organizations to enforce best-practices for their cloud infrastructure. The CLI command ( The L3 construct is a conforming implementation of the API, and uses cloudformation-guard for analysis. Benefits of an API over a Library for Orgs
|
Thanks @GriffinMB ! How would you suggest to handle exceptions from the enforcement? |
Additionally, how can checks/enforcements from multiple domains/vendors be combined? For example, there are operational and security best practices (e.g. Well-Architected pillars) that can be checked for. Additionally, different ISVs might want to provide rule packs, and a project can use several of them. |
Exceptions can be handled in a lot of ways ha. We're working on a solution to that and will include it in the full proposal. There's a kind of natural way to do it by encoding exception handling in guard rules, but that's not the solution we're looking at right now since it isn't necessarily as flexible. Different domains can be managed by rulesets. When the API gets a scan request, it will check the template against all rulesets the user has permissions for. Different Orgs, teams, etc would only have permissions to the rulesets applicable to them. This is something else that'll have more detail in the full proposal, but it would look something like:
Then, when a principal associated with the account hits the API, their template would be checked against all available rulesets. If you want to only grant permissions to specific rulesets, it would look like this:
On the user side of things, if they want to kick off a local scan and only care about a subset of rulesets they have access to, they can use the |
Also, the idea is that the deployment of this would ideally be in a separate account, where you have a single stack that creates and manages the Validator, Rulesets, and permissions for each. Users can also add additional Validator endpoints if an org has multiple Validators with different Well Architected Rulesets. |
When you say "Aspect-style enforcement" - do you mean checks and surfacing issues, like cdk-nag does, or mutating the application in place without developer control? |
I mean something along the lines of this: #25 (comment) More generally, I just mean any solution where Construct validation is done on the Construct objects directly, during synthesis or compilation, rather than on build artifacts. I'm trying to differentiate between the (as I see it) two different problem spaces that can be addressed:
|
Working that the props level might work, but I can't help but notice that what we're looking for is Dependency injection. We might want to substitute an internal call to
We now have the ability to
This is strictly more powerful, and convenient APIs that leave the class the same but just override some props defaults could still be built on top of this. |
I looked deeper into For an example, look at the following CloudFormation logical ID: I think that can help organizations working with both raw CloudFormation and AWS CDK. It can also be a basis for a service-based solution like @GriffinMB mentioned. What do you think? deployment.py class UserManagementBackend(cdk.Stage):
def __init__(
self,
scope: cdk.Construct,
id_: str,
*,
database_dynamodb_billing_mode: dynamodb.BillingMode,
api_lambda_reserved_concurrency: int,
**kwargs: Any,
):
super().__init__(scope, id_, **kwargs)
stateful = cdk.Stack(self, "Stateful")
database = Database(
stateful, "Database", dynamodb_billing_mode=database_dynamodb_billing_mode
)
stateless = cdk.Stack(self, "Stateless")
api = API(
stateless,
"API",
database_dynamodb_table_name=database.dynamodb_table.table_name,
lambda_reserved_concurrency=api_lambda_reserved_concurrency,
) |
@GriffinMB thanks for sharing valuable insights on how you are approaching the problem spaces. A lot of your comments are making sense to me. What I am confused about is the enforcement part of things. How I currently understand it:
How is the above workflow different than the CDK Aspects?
The CDK Aspects library can be developed internally at its own pace and the developers are not blocked by it. |
Probably I am missing out on something or misunderstanding the intent of the discussion On the matter of providing defaults, one can provide a custom >=L2 construct with defaults of choice and it is the matter of exposing the resources encapsulated within a construct so that the CDK escape hatches can be leveraged. On the construct provided on an organisation level export class MyDummyConstruct extends Construct {
public readonly role: IRole;
public readonly bucket: IBucket;
constructor(scope: Construct, id:string, props: MyDummyProps) {
super(scope, id);
this.role =. new Role(this, "Role", {... can enter the sane defaults ...})
this.bucket = new Bucket(this, "Bucket", {... can enter the sane defaults ...})
}
} On the consumption side, making use of the escape hatch const myDummyResource = new MyDummyConstruct(this, "DummyConstruct", {... some props ...})
// The underlying resource is accessed and behavior other than the sane default is applied
myDummyResource.bucket.applyRemovalPolicy(RemovalPolicy.DESTROY) Additionally, the props for a construct can also be used as a mechanism to override the sane defaults. Would be helpful to clarify my thoughts if someone can point out the drawbacks and in blind spots in this approach. |
My 2 cents on using escape hatches :). Each construct library has a public API. To me, using escape hatches is like using internal implementation details of a library. These are subject to change without notice, because they are not part of the API contract. And this can break my code unexpectedly, so I prefer to avoid this. I'd rather prefer the library to add the required functionality (e.g. expose the underlying resources through an intentional public API). If the library generates many resources (like ApplicationLoadBalancedFargateService), it will take each consumer a significant effort to understand the underlying construct tree and find the relevant resource to patch. That breaks the intent of a higher-level construct library - encapsulate the details through abstraction to save library consumers time. I think the organizational constructs should support at least the following properties to avoid being a blocker while putting guardrails in place:
There are additional topics being discussed in this thread, unless I missed something :):
|
I wonder how much of this could be implemented through the current and possibly extended functionality of the If The |
Closing this ticket as it is not a current priority. |
(See #163 for a previous attempt at an RFC.)
Description
Allow users to specify or install some code that controls the defaults of AWS resources or applies some policy or validation at the stack or app level.
Progress
The text was updated successfully, but these errors were encountered: