aviatrixsystems / terraform-modules Goto Github PK

Looking to see if this repository was supposed to have a license.

If it is possible to add a license that would be great to know the intention of use for this code. For now, one must take the assumption made if repositories have no license.

Can you please add a version tag so we can specify which version is being used to avoid breaking changes?

Using the "full stack" code provided in the documentation (literally copy/paste/adjust), I initially deployed (terraform apply) it without changing the value for the controller admin password. I updated the code this morning to change that password value, and during the terraform plan I received an email from the controller that the account had been created on the controller. I have not yet applied the configuration but i am able to login successfully to the controller with the new password.

Hello
Latest refactor changed the format of the 'Name' tag for EC2 instance
From:
format("%s%s-%d", local.name_prefix, "AviatrixController", count.index)
To:
Name = "${local.name_prefix} AviatrixController ${count.index}"

change visible in terraform plan:
~ "Name" = "AviatrixController-0" -> " AviatrixController 0"

IMO there is unnecessary white space char at the beginning of the string, value of this tag is also used here:
https://github.com/AviatrixSystems/Controller-HA-for-AWS/blob/master/aviatrix_ha.py
It's easy to make a mistake and ends up with error:

"Can't find Controller instance with name tag AviatrixController 0. list index out of range"

we are using this tag also to create lambda function but now it fails:
"Error: Error creating Lambda function: ValidationException: 1 validation error detected: Value ' AviatrixController 0-ha' at 'functionName' failed to satisfy constraint: Member must satisfy regular expression pattern: (arn:(aws[a-zA-Z-]*)?:lambda:)?([a-z]{2}((-gov)|(-iso(b?)))?-[a-z]+-\d{1}:)?(\d{12}:)?(function:)?([a-zA-Z0-9-]+)(:($LATEST|[a-zA-Z0-9-]+))?"

Can we back to old format ?

When setting up a new aviatrix controller I had to specify the iam role for EC2 instances. Aviatrix appears to now require the role to be named exactly as "aviatrix-role-ec2", with no prefix allowed. I have a ticket open with support to see if this is necessary.

In the meantime, running this terraform module with a name_prefix does not work.

The module 'aviatrix-controller-build' variable.tf file states 'default' license type of 'Metered Platinum Copilot' which no longer exists in AWS Marketplace, subsequently the Terraform deployment fails if an explicit 'TYPE' is not added to the module configuration.

• Suggest making license type default 'BYOL', which works?

The assume_role_policy of the aviatrix-role-app allows everyone in the account to assume the role:
"AWS": "arn:aws:iam::${data.aws_caller_identity.current.account_id}:root"

It should be closed down, so that only the aviatrix-role-ec2 role can assume the role.

The incoming_ssl list needs to include the local subnet cidr for the lambda to execute against the controller during controller initialisation. The should be documented in the build or initialisation modules.

In the snippet of code above, it references a file hosted on S3. This presents a security and versioning problem. It would be more stable and more secure to use the Template provider described here in the Terraform documentation:

https://www.terraform.io/docs/providers/template/index.html

The security vulnerability is that it would allow anyone with the ability to change the content hosted at that URL to escalate the privileges held by the IAM roles created in this terraform module. This could be part of a set of vulnerabilities taken advantage of in order to breach a customer via this vendor-provided module.

Secondly, it circumvents the customer's ability to have thoroughly reviewed and approved this Terraform code for use in their production environments. This could lead to instability in their environment, including troublesome deployment of Terraform code to potentially downtime.

If the information held at that URL was intended to be fluid, that was the intent of the function of this module, and it was documented as such for the customer, then that would be acceptable. For instance, an authoritative list of IP addresses for whilelisting. However, this is making the assignment of IAM permissions flexible.

Aviatrix SGs are not being destroyed in TF code and preventing the VPC from being deleted. Manual delete of SG results in VPC deletion success.

Error: error deleting EC2 VPC (vpc-07b3607f79dbf30d8): DependencyViolation: The vpc 'vpc-07b3607f79dbf30d8' has dependencies and cannot be deleted.
│ status code: 400, request id: 72a11fec-03a9-45a3-99c7-4d6ff6de6594

i am getting the following error when i try to execute the initialize controller module.

│ Error: error waiting for Lambda Function (AvxLambda_x.y.z.a) creation: InsufficientRolePermissions: The function's execution role doesn't have permission to perform this operation.
│
│ with module.aviatrix_controller_init.aws_lambda_function.lambda,
│ on .terraform/modules/aviatrix_controller_init/aviatrix-controller-initialize/main.tf line 64, in resource "aws_lambda_function" "lambda":
│ 64: resource aws_lambda_function lambda {

Hi there, I encountered a couple issues when trying to run this module, specifically around this line "assume_role_policy = "${var.external-controller-account-id == "" ? local.policy_primary : local.policy_cross}"

(1) local.policy_primary and local.policy_cross is not defined in locals {}, so terraform plan complains about it and I had to define these 2 values in locals {}.
(2) after the local.policy_primary and local.policy_cross is defined, terraform apply encounters an error saying that "aws_iam_role.aviatrix-role-app: Error creating IAM Role aviatrix-role-app: MalformedPolicyDocument: This policy contains invalid Json"

I had to workaround this issue by providing a pull JSON file representation (like the ec2-role)

Should this be ami?

locals {
name_prefix = var.name_prefix != "" ? "${var.name_prefix}-" : ""
images_byol = jsondecode(data.http.avx_iam_id.body).BYOL
images_platinum = jsondecode(data.http.avx_iam_id.body).MeteredPlatinum
images_custom = jsondecode(data.http.avx_iam_id.body).Custom
images_copilot = jsondecode(data.http.avx_iam_id.body).MeteredPlatinumCopilot
ami_id = var.type == "MeteredPlatinumCopilot"? local.images_copilot[data.aws_region.current.name] : ( var.type == "Custom" ? local.images_custom[data.aws_region.current.name] : (var.type == "BYOL" || var.type == "byol" ? local.images_byol[data.aws_region.current.name] : local.images_platinum[data.aws_region.current.name]))
common_tags = merge(
var.tags, {
module = "aviatrix-controller-build"
Createdby = "Terraform+Aviatrix"
})
}

data http avx_iam_id {
url = "https://s3-us-west-2.amazonaws.com/aviatrix-download/AMI_ID/ami_id.json"
request_headers = {
"Accept" = "application/json"
}
}

As we only support BYOL, can you set the type to byol as default for controller instances?

The module outputs the role names, but not the ARNs. Other aviatrix resources tend to accept ARNs as input. It's easy enough to put together the ARNs manually but seems like it would be a good approach to just output them from this module so they can be referenced elsewhere.