-
Notifications
You must be signed in to change notification settings - Fork 133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Vpc upgrade version aws module #456
Conversation
@@ -21,7 +21,7 @@ module "kms_cloudtrail" { | |||
} | |||
|
|||
module "cloudtrail_s3_bucket" { | |||
source = "github.com/ManagedKube/terraform-aws-cloudtrail-s3-bucket.git//?ref=0.24.0" | |||
source = "github.com/ManagedKube/terraform-aws-cloudtrail-s3-bucket.git//?ref=0.25.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was generated by you @sekka1 in the following release: https://github.com/ManagedKube/terraform-aws-cloudtrail-s3-bucket/releases/tag/0.25.0
} | ||
module "iam_assumable_role_admin" { | ||
source = "terraform-aws-modules/iam/aws//modules/iam-assumable-role-with-oidc" | ||
version = "5.33.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgrade version in order to get compatibility with TF 1.6.x
name_prefix = "cluster-autoscaler-${var.cluster_name}" | ||
description = "EKS cluster-autoscaler policy for cluster ${var.eks_cluster_id}" | ||
policy = data.aws_iam_policy_document.cluster_autoscaler.json | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No changes
values = ["true"] | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like github take this as change ,I didn't change this policy
|
||
map_public_ip_on_launch = var.map_public_ip_on_launch | ||
manage_default_network_acl = var.manage_default_network_acl | ||
manage_default_route_table = var.manage_default_route_table |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New fields , I have wrote an explanation in the var area.
@@ -172,3 +172,20 @@ variable "default_security_group_tags" { | |||
default = {} | |||
} | |||
|
|||
variable "map_public_ip_on_launch" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This field is mandatory in the new terrarform version, it was true by default before, We need false due we don't want to assign public ips in the instances.
default = false | ||
} | ||
|
||
variable "manage_default_network_acl" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is true by default , in our case, we want to expose this field to say "no" if we don't want a default ACL.
} | ||
|
||
variable "manage_default_route_table" { | ||
description = "Should be true to manage default route table" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is true by default , in our case, we want to expose this field to say "no" if we don't want a default route table..
This pr helps to get compatibility to terraform 1.6.x