snyk / driftctl

Detect, track and alert on infrastructure drift
Apache License 2.0
2.46k stars 156 forks source link

Limit noise of unmanaged child resources #1451

Open eliecharra opened 2 years ago

eliecharra commented 2 years ago

Description By working on adding support for aws_s3_bucket_public_access_block and idea came to my mind. We know the certain kind of resources are child resource of others:

A child resource basically mean that a child resource cannot exist without any parent (no orphans). We have started to define those hierarchy rules in driftctl in pkg/resource/resource_types.go and it look like that:

    "aws_security_group": {children: []ResourceType{
        "aws_security_group_rule",
    }},
    "aws_security_group_rule": {},
    "aws_sns_topic": {children: []ResourceType{
        "aws_sns_topic_policy",
    }},
    "aws_sns_topic_policy":       {},
    "aws_sns_topic_subscription": {},
    "aws_sqs_queue": {children: []ResourceType{
        "aws_sqs_queue_policy",
    }},

We have done that initially to be able to be consistent when for example we are ignoring aws_sqs_queue from the drift ignore but we still want to report aws_sqs_queue_policy (maybe not the right example). The idea was to being able to avoid ignoring some resources that are created in expander middlewares (understand directly created from their parents, not coming from their enumerator). More details about that can be found in that PR https://github.com/snyk/driftctl/pull/1131


Now we have that context, I think it can make sense to omit from scan results child resources if their parent is unmanaged.

Example

In the example below, I don't find that it makes a lot of sense to show the aws_s3_bucket_public_access_block resource.

Found resources not covered by IaC:
  aws_s3_bucket:
    - test-elie-driftctl
  aws_s3_bucket_public_access_block:
    - test-elie-driftctl

A single middleware could maybe be responsible of doing that if we can provide a way to identify the children-parent relation. Maybe we could also introduce an opt-out flag for that behavior, or reuse the --strict one ? I'm afraid that the strict flag will became an to big holdall in the future.

p0tr3c commented 2 years ago

One thing to consider is the experience of using driftctl to import all unmanaged resources into terraform. If by default the child resources are not displayed, it may be confusing to the user that suddenly more unmanaged resources are discover after they imported the parent. That nuance I think has to be somehow visible in the output (some sort of notification that children are filtered out).

eliecharra commented 2 years ago

One thing to consider is the experience of using driftctl to import all unmanaged resources into terraform

This is really a good catch, I didn't have that use-case in mind while writing the issues and this is definitely something that we need to consider. As you said, maybe a notice in the output to say that some resource have been ignored, combined with an opt-out or --strict flag can maybe compensate that