hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.61k stars 9k forks source link

Include more client side validation #14379

Open bcsgh opened 3 years ago

bcsgh commented 3 years ago

Community Note

Description

Please include more client side validation for all rules where it can be done hermetically.

For example, wit aws_security_group there are some interactions between the ingress port and protocol parameter that can be checked locally.

In general, for any error that can be found without interacting with external resources, there is significant practical utility in reporting it during terraform plan or before updates are attempted by terraform apply rather than deferring that reporting to the point where someone is actually trying to modify the live state.

(That said, things should err on the side of lenience to avoid blocking things that AWS it self would allow. Or maybe make those case warnings?)

New or Affected Resource(s)

Potential Terraform Configuration

resource "aws_security_group" "main" {
  ingress {
    from_port = 22
    to_port = 22
    protocol = "-1"  # ERROR/WARNING: to/from ports must be zero when protocol=-1.  
    ...
  }
  ...
}
github-actions[bot] commented 2 months ago

Marking this issue as stale due to inactivity. This helps our maintainers find and focus on the active issues. If this issue receives no comments in the next 30 days it will automatically be closed. Maintainers can also remove the stale label.

If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thank you!

bcsgh commented 2 months ago

FWIW, I still think this is a good idea and don't think it should be closed.

(I also don't think a lack of activity on an issue is a sound reason for assuming the maintainers should decide to avoid doing something. Furthermore, closing issues as "not important enough" seems like a bad idea to me. That's what the lowest priority setting should be used for.)