Skip to content
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

feat: Inital release of terraform module #6

Merged
merged 30 commits into from
Aug 31, 2022
Merged

feat: Inital release of terraform module #6

merged 30 commits into from
Aug 31, 2022

Conversation

jsbroks
Copy link
Member

@jsbroks jsbroks commented Jul 11, 2022

No description provided.

modules/app_aks/main.tf Outdated Show resolved Hide resolved
modules/app_aks/main.tf Outdated Show resolved Hide resolved
Comment on lines 1 to 29
resource "azurerm_kubernetes_cluster" "default" {
name = "${var.namespace}-cluster"
location = var.location
resource_group_name = var.resource_group_name
dns_prefix = var.namespace

automatic_channel_upgrade = "stable"
role_based_access_control_enabled = true

default_node_pool {
name = "default"
node_count = 2
vm_size = "Standard_D4s_v3"
vnet_subnet_id = var.cluster_subnet_id
type = "VirtualMachineScaleSets"
enable_auto_scaling = false
zones = ["1", "2"]
}

identity {
type = "SystemAssigned"
}

network_profile {
network_plugin = "azure"
}

tags = var.tags
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tfsec check azure-container-logging failed.

Description: Cluster does not have logging enabled via OMS Agent.

Severity: MEDIUM

For more information, see:

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will definitely want logging enabled, but can punt for now.

}

identity {
type = "SystemAssigned"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be ideal if we could use the Azure native identity solution here.

modules/app_aks/main.tf Outdated Show resolved Hide resolved
modules/app_aks/main.tf Outdated Show resolved Hide resolved
Comment on lines 1 to 29
resource "azurerm_kubernetes_cluster" "default" {
name = "${var.namespace}-cluster"
location = var.location
resource_group_name = var.resource_group_name
dns_prefix = var.namespace

automatic_channel_upgrade = "stable"
role_based_access_control_enabled = true

default_node_pool {
name = "default"
node_count = 2
vm_size = "Standard_D4s_v3"
vnet_subnet_id = var.cluster_subnet_id
type = "VirtualMachineScaleSets"
enable_auto_scaling = false
zones = ["1", "2"]
}

identity {
type = "SystemAssigned"
}

network_profile {
network_plugin = "azure"
}

tags = var.tags
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will definitely want logging enabled, but can punt for now.

variable "database_availability_mode" {
description = ""
type = string
default = "SameZone"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagine we want the default to be "ZoneRedundant". When ever possible we want our infra to weather a Zone going down. We won't support multi-region, but our official terraform should adhere to the cloud contract which says we should assume a single zone can just go away. Again, not necessarily required for MVP, but we should definitely think about and work towards multi zone operations for MySQL, k8s, and Redis

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With ZoneRedundant, the limitation we might have is the regions it's currently available at. There's very few regions that support ZoneRedundant today. In the US, it's only East US and South Central US only. If customers want their deployment in other regions we might have to default back to SameZone if not the deployment would fail.

variables.tf Outdated
# Most users will not need these settings. They are ment for users who want a
# bucket in a different account.

variable "bucket_name" {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Azure land they call buckets "Blob Containers". Nit, but might want to align with their vocabulary.

versions.tf Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Comment on lines 24 to 27
network_profile {
network_plugin = "azure"
load_balancer_sku = "standard"
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tfsec check azure-container-configured-network-policy failed.

Description: Kubernetes cluster does not have a network policy set.

Severity: HIGH

For more information, see:

Comment on lines 1 to 30
resource "azurerm_kubernetes_cluster" "default" {
name = "${var.namespace}-cluster"
location = var.location
resource_group_name = var.resource_group_name
dns_prefix = var.namespace

automatic_channel_upgrade = "stable"
role_based_access_control_enabled = true

default_node_pool {
name = "default"
node_count = 2
vm_size = "Standard_D4s_v3"
vnet_subnet_id = var.cluster_subnet_id
type = "VirtualMachineScaleSets"
enable_auto_scaling = false
zones = ["1", "2"]
}

identity {
type = "SystemAssigned"
}

network_profile {
network_plugin = "azure"
load_balancer_sku = "standard"
}

tags = var.tags
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tfsec check azure-container-limit-authorized-ips failed.

Description: Cluster does not limit API access to specific IP addresses.

Severity: CRITICAL

For more information, see:

Comment on lines 1 to 30
resource "azurerm_kubernetes_cluster" "default" {
name = "${var.namespace}-cluster"
location = var.location
resource_group_name = var.resource_group_name
dns_prefix = var.namespace

automatic_channel_upgrade = "stable"
role_based_access_control_enabled = true

default_node_pool {
name = "default"
node_count = 2
vm_size = "Standard_D4s_v3"
vnet_subnet_id = var.cluster_subnet_id
type = "VirtualMachineScaleSets"
enable_auto_scaling = false
zones = ["1", "2"]
}

identity {
type = "SystemAssigned"
}

network_profile {
network_plugin = "azure"
load_balancer_sku = "standard"
}

tags = var.tags
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tfsec check azure-container-logging failed.

Description: Cluster does not have logging enabled via OMS Agent.

Severity: MEDIUM

For more information, see:

Comment on lines 1 to 18
resource "azurerm_storage_account" "default" {
name = replace("${var.namespace}-storage", "-", "")
resource_group_name = var.resource_group_name
location = var.location
account_tier = "Standard"
account_replication_type = "ZRS"
min_tls_version = "TLS1_2"

blob_properties {
cors_rule {
allowed_headers = ["*"]
allowed_methods = ["GET", "HEAD", "PUT"]
allowed_origins = ["*"]
exposed_headers = ["*"]
max_age_in_seconds = 3600
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tfsec check azure-storage-queue-services-logging-enabled failed.

Description: Queue services storage account does not have logging enabled.

Severity: MEDIUM

For more information, see:

name = "Standard_v2"
tier = "Standard_v2"
name = "WAF_v2"
tier = "WAF_v2"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use WAF we have to disable a bunch of rules as it falsely flags a ton of requests. It's also pretty expensive. If we can find a way to avoid it, probably ideal.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reverted to Standard_v2, we can punt on this

Comment on lines 1 to 36
resource "azurerm_kubernetes_cluster" "default" {
name = "${var.namespace}-cluster"
location = var.location
resource_group_name = var.resource_group_name
dns_prefix = var.namespace

automatic_channel_upgrade = "stable"
role_based_access_control_enabled = true
http_application_routing_enabled = true

ingress_application_gateway {
gateway_id = var.gateway.id
}

default_node_pool {
name = "default"
node_count = 2
vm_size = "Standard_D4s_v3"
vnet_subnet_id = var.cluster_subnet_id
type = "VirtualMachineScaleSets"
enable_auto_scaling = false
zones = ["1", "2"]
}

identity {
type = "SystemAssigned"
}

network_profile {
network_plugin = "azure"
network_policy = "azure"
load_balancer_sku = "standard"
}

tags = var.tags
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tfsec check azure-container-limit-authorized-ips failed.

Description: Cluster does not limit API access to specific IP addresses.

Severity: CRITICAL

For more information, see:

Comment on lines 1 to 36
resource "azurerm_kubernetes_cluster" "default" {
name = "${var.namespace}-cluster"
location = var.location
resource_group_name = var.resource_group_name
dns_prefix = var.namespace

automatic_channel_upgrade = "stable"
role_based_access_control_enabled = true
http_application_routing_enabled = true

ingress_application_gateway {
gateway_id = var.gateway.id
}

default_node_pool {
name = "default"
node_count = 2
vm_size = "Standard_D4s_v3"
vnet_subnet_id = var.cluster_subnet_id
type = "VirtualMachineScaleSets"
enable_auto_scaling = false
zones = ["1", "2"]
}

identity {
type = "SystemAssigned"
}

network_profile {
network_plugin = "azure"
network_policy = "azure"
load_balancer_sku = "standard"
}

tags = var.tags
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tfsec check azure-container-logging failed.

Description: Cluster does not have logging enabled via OMS Agent.

Severity: MEDIUM

For more information, see:

Copy link

@mrb113 mrb113 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This initial setup looks pretty good to me. Are you addressing the bot comments about the security issues it's found?

zone = "1"

# high_availability {
# mode = var.database_availability_mode
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this supposed to be here?

geo_redundant_backup_enabled = false

zone = "1"

Copy link
Contributor

@venky-wandb venky-wandb Aug 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the customer were to provide a region that doesn't support availability zones, this would fail.
Reference

variables.tf Outdated
variable "database_version" {
description = "Version for MySQL"
type = string
default = "8.0.21"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we default this MySQL 5.7 for now? (we could also do it in the core azure terraform)

Comment on lines +1 to +36
resource "azurerm_kubernetes_cluster" "default" {
name = "${var.namespace}-cluster"
location = var.location
resource_group_name = var.resource_group.name
dns_prefix = var.namespace

automatic_channel_upgrade = "stable"
role_based_access_control_enabled = true
http_application_routing_enabled = false

ingress_application_gateway {
gateway_id = var.gateway.id
}

default_node_pool {
name = "default"
node_count = 2
vm_size = "Standard_D4s_v3"
vnet_subnet_id = var.cluster_subnet_id
type = "VirtualMachineScaleSets"
enable_auto_scaling = false
zones = ["1", "2"]
}

identity {
type = "SystemAssigned"
}

network_profile {
network_plugin = "azure"
network_policy = "azure"
load_balancer_sku = "standard"
}

tags = var.tags
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tfsec check azure-container-limit-authorized-ips failed.

Description: Cluster does not limit API access to specific IP addresses.

Severity: CRITICAL

For more information, see:

Comment on lines +1 to +36
resource "azurerm_kubernetes_cluster" "default" {
name = "${var.namespace}-cluster"
location = var.location
resource_group_name = var.resource_group.name
dns_prefix = var.namespace

automatic_channel_upgrade = "stable"
role_based_access_control_enabled = true
http_application_routing_enabled = false

ingress_application_gateway {
gateway_id = var.gateway.id
}

default_node_pool {
name = "default"
node_count = 2
vm_size = "Standard_D4s_v3"
vnet_subnet_id = var.cluster_subnet_id
type = "VirtualMachineScaleSets"
enable_auto_scaling = false
zones = ["1", "2"]
}

identity {
type = "SystemAssigned"
}

network_profile {
network_plugin = "azure"
network_policy = "azure"
load_balancer_sku = "standard"
}

tags = var.tags
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tfsec check azure-container-logging failed.

Description: Cluster does not have logging enabled via OMS Agent.

Severity: MEDIUM

For more information, see:

@jsbroks jsbroks marked this pull request as ready for review August 31, 2022 23:45
@jsbroks jsbroks merged commit 65e545c into main Aug 31, 2022
@jsbroks jsbroks deleted the init branch August 31, 2022 23:46
@jsbroks
Copy link
Member Author

jsbroks commented Sep 1, 2022

This PR is included in version 1.0.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants