The linter ignores inline security group rules

[excavator-matt]

Hi!

I just tried the linter to see if it could the common trap of the AWS provider of using the inline security group rules instead of the new egress and ingress rule resources.

tflint --version
TFLint version 0.58.0
+ ruleset.terraform (0.12.0-bundled)
+ ruleset.aws (0.40.0)

According the the documentation.

Using aws_vpc_security_group_egress_rule and aws_vpc_security_group_ingress_rule resources is the current best practice. Avoid using the aws_security_group_rule resource and the ingress and egress arguments of the aws_security_group resource for configuring in-line rules, as they struggle with managing multiple CIDR blocks, and tags and descriptions due to the historical lack of unique IDs.

Further if you use the inline rule, it blocks other people from using the recommended resource.

You should not use the aws_vpc_security_group_egress_rule and aws_vpc_security_group_ingress_rule resources in conjunction with the aws_security_group resource with in-line rules (using the ingress and egress arguments of aws_security_group) or the aws_security_group_rule resource. Doing so may cause rule conflicts, perpetual differences, and result in rules being overwritten.

Yet the linter simply ignores this. If my quick search of the code base is correctly interpreted, there seems to be a check against using the deprecated aws_security_group_rule, but it only checks how the inline rule is used.

Is this a conscious decision? Could we change this?

It would be smooth to handle the rules as part of the group, but sadly it isn't possible for reason I don't quite understand.

[wata727]

Can you explain what you expect and what the actual behavior is?
As you mention, the aws_security_group_inline_rules inspects inline rules, so they are not ignored.

[excavator-matt]

@wata727 and (@bendrucker for moving it from an issue to a general discussion), I think most people would expect the aws rule set to raise a warning about using inline rules. As per official AWS documentation linked and cited above, using inline is not recommended. This mistake is why I started googling Terraform linters.

Avoid using [...] and the ingress and egress arguments of the aws_security_group resource

We could in theory remove checking the inline rules as using them is inherently a bad practice.

[bendrucker]

We generally keep issues for actionable issues/changes. If it's needs discussion, it's a discussion.

You are certainly right about the recommendation against using in line rules. A PR is welcome to add a new rule to enforce that.

The mention of the existing rules sounds like a distraction. Inline blocks are deprecated but not removed. I'm not into the idea that we need to remove the rule for its existing users, as long as the provider supports it. Address it with a new explicit rule, and any user of the deprecated form can always disable that new rule if they're not ready to switch.

[excavator-matt]

Sounds reasonable! I don't know Go, but there are plenty of examples and a template, so I think I can fix that.

[bendrucker]

Great, happy to help get something over the line either via review or directly if you need help. The terraform ruleset can get into some trickier areas. But in aws, all the complexity is in the generated rules. The hand-written ones are pretty straightforward and should pick up the pattern for attribute/block existence checks pretty quickly.

[excavator-matt]

I started implementing, but I realise there has been a confusion.

The existing inline rule does raise error for using it, it is only that it isn't enabled by default. After enabling it, it works as expected.