search

vainkop / terraform-aws-wireguard

Terraform Module for Wireguard VPN
GNU General Public License v3.0
25 stars 23 forks source link

Null prometheus server ip causes security group resource issue #3

Open MarcMeszaros opened 3 years ago

MarcMeszaros commented 3 years ago

If you don't specify a prometheus server IP in the module variables, you get an error provisioning the security group.

│ Error: Null value found in list
│ 
│   with module.wireguard.aws_security_group.sg_wireguard,
│   on .terraform/modules/wireguard/sg.tf line 1, in resource "aws_security_group" "sg_wireguard":
│    1: resource "aws_security_group" "sg_wireguard" {
│ 
│ Null values are not allowed for this attribute value.

Q: is the prometheus server a hard requirement? We don't use prometheus, can it be optional?

MarcMeszaros commented 3 years ago

The variable description should probably be updated to mention that the prometheus_server_ip is actually a CIDR block string since the variable is used directly in the security group.

vainkop commented 3 years ago

The variable description should probably be updated to mention that the prometheus_server_ip is actually a CIDR block string since the variable is used directly in the security group.

@MarcMeszaros it's a in CIDR block notation according to AWS SG docs: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/security-group-rules-reference.html

So you need to use /smth even for a single ip, for example 1.2.3.4/32 is a single address.

vainkop commented 3 years ago

If you don't specify a prometheus server IP in the module variables, you get an error provisioning the security group.

│ Error: Null value found in list
│ 
│   with module.wireguard.aws_security_group.sg_wireguard,
│   on .terraform/modules/wireguard/sg.tf line 1, in resource "aws_security_group" "sg_wireguard":
│    1: resource "aws_security_group" "sg_wireguard" {
│ 
│ Null values are not allowed for this attribute value.

Q: is the prometheus server a hard requirement? We don't use prometheus, can it be optional?

I think it should be optional but unfortunately I don't have enough time to support this module. My colleagues are currently working on an upgraded version of this module so I will post it as soon as they finish or maybe take some parts from there as I'm 100% sure I agree with some decisions they made when implementing it.

MarcMeszaros commented 3 years ago

@vainkop Is the intent to deprecate this module? I have branches with fixes for most of the issues I'm creating in my fork for PRs, but it only makes sense for me to make PRs if this module is maintained/not deprecated.

Update: I made the PR's anyway since I already had the work done in my fork.