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

Use terraform-core-helpers child module #7

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ changelog:
labels:
- dependencies

- title: 🔩 Dependencies
- title: 🔩 Dependencies
labels:
- dependencies

# This file is managed by the osinfra-io/github-organization-management repository and should not be edited directly.
# This file is managed by the osinfra-io/github-organization-management repository and should not be edited directly.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ override.tf.json
# Ignore plan output files
plan.out

# Ignore checkov directories and files
.external_modules

# Ignore Infracost directories and files
.infracost

Expand Down
5 changes: 4 additions & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,12 @@ repos:
- id: terraform_docs

- repo: https://github.com/bridgecrewio/checkov.git
rev: 3.2.257
rev: 3.2.271
hooks:
- id: checkov
verbose: true
args:
- --download-external-modules=true
- --skip-check
- "CKV_TF_1"
- --quiet
11 changes: 9 additions & 2 deletions regional/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ No requirements.

| Name | Version |
|------|---------|
| <a name="provider_helm"></a> [helm](#provider\_helm) | 2.16.0 |
| <a name="provider_helm"></a> [helm](#provider\_helm) | 2.16.1 |

## Modules

No modules.
| Name | Source | Version |
|------|--------|---------|
| <a name="module_helpers"></a> [helpers](#module\_helpers) | github.com/osinfra-io/terraform-core-helpers | v0.1.0 |

## Resources

Expand All @@ -35,6 +37,11 @@ No modules.
| <a name="input_cain_injector_resources_requests_memory"></a> [cain\_injector\_resources\_requests\_memory](#input\_cain\_injector\_resources\_requests\_memory) | The memory request for the cain-injector resources | `string` | `"32Mi"` | no |
| <a name="input_cert_manager_version"></a> [cert\_manager\_version](#input\_cert\_manager\_version) | The version to install, this is used for the chart as well as the image tag | `string` | `"1.16.1"` | no |
| <a name="input_chart_repository"></a> [chart\_repository](#input\_chart\_repository) | The repository to pull the cert-manager Helm chart from | `string` | `"https://charts.jetstack.io"` | no |
| <a name="input_helpers_cost_center"></a> [helpers\_cost\_center](#input\_helpers\_cost\_center) | The cost center the resources will be billed to, must start with 'x' followed by three or four digits | `string` | n/a | yes |
| <a name="input_helpers_data_classification"></a> [helpers\_data\_classification](#input\_helpers\_data\_classification) | The data classification of the resources can be public, internal, or confidential | `string` | n/a | yes |
| <a name="input_helpers_email"></a> [helpers\_email](#input\_helpers\_email) | The email address of the team responsible for the resources | `string` | n/a | yes |
| <a name="input_helpers_repository"></a> [helpers\_repository](#input\_helpers\_repository) | The repository name (should be in the format 'owner/repo' containing only lowercase alphanumeric characters or hyphens) | `string` | n/a | yes |
| <a name="input_helpers_team"></a> [helpers\_team](#input\_helpers\_team) | The team name (should contain only lowercase alphanumeric characters and hyphens) | `string` | n/a | yes |
| <a name="input_replicas"></a> [replicas](#input\_replicas) | The number of replicas to run | `number` | `1` | no |
| <a name="input_resources_limits_cpu"></a> [resources\_limits\_cpu](#input\_resources\_limits\_cpu) | The CPU limit for the resources | `string` | `"20m"` | no |
| <a name="input_resources_limits_memory"></a> [resources\_limits\_memory](#input\_resources\_limits\_memory) | The memory limit for the resources | `string` | `"64Mi"` | no |
Expand Down
11 changes: 9 additions & 2 deletions regional/istio-csr/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ No requirements.

| Name | Version |
|------|---------|
| <a name="provider_helm"></a> [helm](#provider\_helm) | 2.16.0 |
| <a name="provider_helm"></a> [helm](#provider\_helm) | 2.16.1 |
| <a name="provider_kubernetes"></a> [kubernetes](#provider\_kubernetes) | 2.33.0 |

## Modules

No modules.
| Name | Source | Version |
|------|--------|---------|
| <a name="module_helpers"></a> [helpers](#module\_helpers) | github.com/osinfra-io/terraform-core-helpers | v0.1.0 |

## Resources

Expand All @@ -35,6 +37,11 @@ No modules.
| <a name="input_cert_manager_istio_csr_version"></a> [cert\_manager\_istio\_csr\_version](#input\_cert\_manager\_istio\_csr\_version) | The version to install for the Istio CSR, this is used for the chart as well as the image tag | `string` | `"0.12.0"` | no |
| <a name="input_chart_repository"></a> [chart\_repository](#input\_chart\_repository) | The repository to pull the cert-manager Helm chart from | `string` | `"https://charts.jetstack.io"` | no |
| <a name="input_cluster_prefix"></a> [cluster\_prefix](#input\_cluster\_prefix) | Prefix for your cluster name, region, and zone (if applicable) will be added to the end of the cluster name | `string` | n/a | yes |
| <a name="input_helpers_cost_center"></a> [helpers\_cost\_center](#input\_helpers\_cost\_center) | The cost center the resources will be billed to, must start with 'x' followed by three or four digits | `string` | n/a | yes |
| <a name="input_helpers_data_classification"></a> [helpers\_data\_classification](#input\_helpers\_data\_classification) | The data classification of the resources can be public, internal, or confidential | `string` | n/a | yes |
| <a name="input_helpers_email"></a> [helpers\_email](#input\_helpers\_email) | The email address of the team responsible for the resources | `string` | n/a | yes |
| <a name="input_helpers_repository"></a> [helpers\_repository](#input\_helpers\_repository) | The repository name (should be in the format 'owner/repo' containing only lowercase alphanumeric characters or hyphens) | `string` | n/a | yes |
| <a name="input_helpers_team"></a> [helpers\_team](#input\_helpers\_team) | The team name (should contain only lowercase alphanumeric characters and hyphens) | `string` | n/a | yes |
Comment on lines +40 to +44
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Missing validation rules for helper variables

The variables are defined in multiple variables.tf files but none of them implement the validation rules specified in the README. The variables are declared only with type definitions, missing crucial validation blocks to enforce:

  • helpers_cost_center: pattern validation for 'x' followed by 3-4 digits
  • helpers_data_classification: allowed values validation for public/internal/confidential
  • helpers_repository: format validation for owner/repo pattern and character restrictions
  • helpers_team: character set validation for lowercase alphanumeric and hyphens
🔗 Analysis chain

Verify validation rules implementation.

The new helper variables have clear descriptions with specific validation rules. Let's verify that these rules are properly implemented in the variables.tf file.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for validation blocks in variables.tf
# Expected: Find variable declarations with validation rules matching the README

# Look for the variable declarations and their validation blocks
ast-grep --pattern 'variable "helpers_$_" {
  $_
  validation {
    $$$
  }
  $$$
}'

Length of output: 86


Script:

#!/bin/bash
# Let's first find the variables.tf file in the repository
fd "variables.tf" --type f

# Then let's examine its content
rg "variable \"helpers_" -A 10 --no-heading

Length of output: 6220

| <a name="input_resources_limits_cpu"></a> [resources\_limits\_cpu](#input\_resources\_limits\_cpu) | The CPU limit for the Istio CSR container | `string` | `"50m"` | no |
| <a name="input_resources_limits_memory"></a> [resources\_limits\_memory](#input\_resources\_limits\_memory) | The memory limit for the Istio CSR container | `string` | `"64Mi"` | no |
| <a name="input_resources_requests_cpu"></a> [resources\_requests\_cpu](#input\_resources\_requests\_cpu) | The CPU request for the Istio CSR container | `string` | `"25m"` | no |
Expand Down
34 changes: 2 additions & 32 deletions regional/istio-csr/locals.tf
Original file line number Diff line number Diff line change
Expand Up @@ -2,45 +2,15 @@
# https://www.terraform.io/docs/language/values/locals.html

locals {
cluster_id = local.zone != null ? "${var.cluster_prefix}-${local.region}-${local.zone}-${local.env}" : "${var.cluster_prefix}-${local.region}-${local.env}"
env = lookup(local.env_map, local.environment, "none")

environment = (
terraform.workspace == "default" ?
"mock-environment" :
regex(".*-(?P<environment>[^-]+)$", terraform.workspace)["environment"]
)

env_map = {
"non-production" = "nonprod"
"production" = "prod"
"sandbox" = "sb"
}
cluster_id = module.helpers.zone != null ? "${var.cluster_prefix}-${module.helpers.region}-${module.helpers.zone}-${module.helpers.env}" : "${var.cluster_prefix}-${module.helpers.region}-${module.helpers.env}"

helm_values = {
"app.server.clusterID" = local.cluster_id
"podLabels.tags\\.datadoghq\\.com/env" = local.environment
"podLabels.tags\\.datadoghq\\.com/env" = module.helpers.environment
"podLabels.tags\\.datadoghq\\.com/version" = var.cert_manager_istio_csr_version
"resources.limits.cpu" = var.resources_limits_cpu
"resources.limits.memory" = var.resources_limits_memory
"resources.requests.cpu" = var.resources_requests_cpu
"resources.requests.memory" = var.resources_requests_memory
}

region = (
terraform.workspace == "default" ?
"mock-region" :
regex("^(?P<region>[^-]+-[^-]+)", terraform.workspace)["region"]
)


zone = (
terraform.workspace == "default" ?
"mock-zone" :
(
regex("^(?P<region>[^-]+-[^-]+)(?:-(?P<zone>[^-]+))?-.*$", terraform.workspace)["zone"] != "" ?
regex("^(?P<region>[^-]+-[^-]+)(?:-(?P<zone>[^-]+))?-.*$", terraform.workspace)["zone"] :
null
)
)
}
13 changes: 13 additions & 0 deletions regional/istio-csr/main.tf
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
# Terraform Core Helpers Module (osinfra.io)
# https://github.com/osinfra-io/terraform-core-helpers

module "helpers" {
source = "github.com/osinfra-io/terraform-core-helpers?ref=v0.1.0"

cost_center = var.helpers_cost_center
data_classification = var.helpers_data_classification
email = var.helpers_email
repository = var.helpers_repository
team = var.helpers_team
}

# Helm Release
# https://registry.terraform.io/providers/hashicorp/helm/latest/docs/resources/release

Expand Down
25 changes: 25 additions & 0 deletions regional/istio-csr/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,31 @@ variable "chart_repository" {
default = "https://charts.jetstack.io"
}

variable "helpers_cost_center" {
description = "The cost center the resources will be billed to, must start with 'x' followed by three or four digits"
type = string
}

variable "helpers_data_classification" {
description = "The data classification of the resources can be public, internal, or confidential"
type = string
}

variable "helpers_email" {
description = "The email address of the team responsible for the resources"
type = string
}
Comment on lines +37 to +40
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add email format validation.

The email variable should validate the input format.

Add a validation block:

 variable "helpers_email" {
   description = "The email address of the team responsible for the resources"
   type        = string
+  validation {
+    condition     = can(regex("^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$", var.helpers_email))
+    error_message = "Please provide a valid email address."
+  }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
variable "helpers_email" {
description = "The email address of the team responsible for the resources"
type = string
}
variable "helpers_email" {
description = "The email address of the team responsible for the resources"
type = string
validation {
condition = can(regex("^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$", var.helpers_email))
error_message = "Please provide a valid email address."
}
}


variable "helpers_repository" {
description = "The repository name (should be in the format 'owner/repo' containing only lowercase alphanumeric characters or hyphens)"
type = string
}

variable "helpers_team" {
description = "The team name (should contain only lowercase alphanumeric characters and hyphens)"
type = string
}
Comment on lines +42 to +50
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add format validation for repository and team variables.

Both variables require specific format constraints that should be enforced.

Add validation blocks:

 variable "helpers_repository" {
   description = "The repository name (should be in the format 'owner/repo' containing only lowercase alphanumeric characters or hyphens)"
   type        = string
+  validation {
+    condition     = can(regex("^[a-z0-9-]+/[a-z0-9-]+$", var.helpers_repository))
+    error_message = "Repository must be in format 'owner/repo' using only lowercase alphanumeric characters and hyphens."
+  }
 }

 variable "helpers_team" {
   description = "The team name (should contain only lowercase alphanumeric characters and hyphens)"
   type        = string
+  validation {
+    condition     = can(regex("^[a-z0-9-]+$", var.helpers_team))
+    error_message = "Team name must contain only lowercase alphanumeric characters and hyphens."
+  }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
variable "helpers_repository" {
description = "The repository name (should be in the format 'owner/repo' containing only lowercase alphanumeric characters or hyphens)"
type = string
}
variable "helpers_team" {
description = "The team name (should contain only lowercase alphanumeric characters and hyphens)"
type = string
}
variable "helpers_repository" {
description = "The repository name (should be in the format 'owner/repo' containing only lowercase alphanumeric characters or hyphens)"
type = string
validation {
condition = can(regex("^[a-z0-9-]+/[a-z0-9-]+$", var.helpers_repository))
error_message = "Repository must be in format 'owner/repo' using only lowercase alphanumeric characters and hyphens."
}
}
variable "helpers_team" {
description = "The team name (should contain only lowercase alphanumeric characters and hyphens)"
type = string
validation {
condition = can(regex("^[a-z0-9-]+$", var.helpers_team))
error_message = "Team name must contain only lowercase alphanumeric characters and hyphens."
}
}


variable "resources_limits_cpu" {
description = "The CPU limit for the Istio CSR container"
type = string
Expand Down
16 changes: 1 addition & 15 deletions regional/locals.tf
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,14 @@
# https://www.terraform.io/docs/language/values/locals.html

locals {
env = lookup(local.env_map, local.environment, "none")

environment = (
terraform.workspace == "default" ?
"mock-environment" :
regex(".*-(?P<environment>[^-]+)$", terraform.workspace)["environment"]
)

env_map = {
"non-production" = "nonprod"
"production" = "prod"
"sandbox" = "sb"
}

helm_values = {
"cainjector.podLabels.tags\\.datadoghq\\.com/service" = "cert-manager-cainjector"
"cainjector.resources.limits.cpu" = var.cain_injector_resources_limits_cpu
"cainjector.resources.limits.memory" = var.cain_injector_resources_limits_memory
"cainjector.resources.requests.cpu" = var.cain_injector_resources_requests_cpu
"cainjector.resources.requests.memory" = var.cain_injector_resources_requests_memory
"cainjector.replicaCount" = var.cain_injector_replicas
"global.commonLabels.tags\\.datadoghq\\.com/env" = local.environment
"global.commonLabels.tags\\.datadoghq\\.com/env" = module.helpers.environment
"global.commonLabels.tags\\.datadoghq\\.com/version" = var.cert_manager_version
"podLabels.tags\\.datadoghq\\.com/service" = "cert-manager"
"resources.limits.cpu" = var.resources_limits_cpu
Expand Down
13 changes: 13 additions & 0 deletions regional/main.tf
Original file line number Diff line number Diff line change
@@ -1,3 +1,16 @@
# Terraform Core Helpers Module (osinfra.io)
# https://github.com/osinfra-io/terraform-core-helpers

module "helpers" {
source = "github.com/osinfra-io/terraform-core-helpers?ref=v0.1.0"

cost_center = var.helpers_cost_center
data_classification = var.helpers_data_classification
email = var.helpers_email
repository = var.helpers_repository
team = var.helpers_team
}

# Helm Release
# https://registry.terraform.io/providers/hashicorp/helm/latest/docs/resources/release

Expand Down
25 changes: 25 additions & 0 deletions regional/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,31 @@ variable "chart_repository" {
default = "https://charts.jetstack.io"
}

variable "helpers_cost_center" {
description = "The cost center the resources will be billed to, must start with 'x' followed by three or four digits"
type = string
}

variable "helpers_data_classification" {
description = "The data classification of the resources can be public, internal, or confidential"
type = string
}

variable "helpers_email" {
description = "The email address of the team responsible for the resources"
type = string
}

variable "helpers_repository" {
description = "The repository name (should be in the format 'owner/repo' containing only lowercase alphanumeric characters or hyphens)"
type = string
}

variable "helpers_team" {
description = "The team name (should contain only lowercase alphanumeric characters and hyphens)"
type = string
}

variable "replicas" {
description = "The number of replicas to run"
type = number
Expand Down
8 changes: 8 additions & 0 deletions tests/default.tftest.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,11 @@ run "default_regional_istio_csr" {
source = "./tests/fixtures/default/regional/istio-csr"
}
}

variables {
helpers_cost_center = "mock-cost-center"
helpers_data_classification = "mock-data-classification"
helpers_email = "mock-team@osinfra.io"
helpers_repository = "mock-owner/mock-repository"
helpers_team = "mock-team"
Comment on lines +29 to +33
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider using more distinctive mock values.

To prevent any potential confusion or accidental use in production, consider using more obviously fake values.

-  helpers_cost_center         = "mock-cost-center"
-  helpers_data_classification = "mock-data-classification"
-  helpers_email               = "mock-team@osinfra.io"
-  helpers_repository          = "mock-owner/mock-repository"
-  helpers_team                = "mock-team"
+  helpers_cost_center         = "TEST-MOCK-COST-CENTER-000"
+  helpers_data_classification = "TEST-MOCK-DATA-CLASS-000"
+  helpers_email               = "test-mock-team@example.com"
+  helpers_repository          = "test-mock-owner/test-mock-repo"
+  helpers_team                = "TEST-MOCK-TEAM-000"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
helpers_cost_center = "mock-cost-center"
helpers_data_classification = "mock-data-classification"
helpers_email = "mock-team@osinfra.io"
helpers_repository = "mock-owner/mock-repository"
helpers_team = "mock-team"
helpers_cost_center = "TEST-MOCK-COST-CENTER-000"
helpers_data_classification = "TEST-MOCK-DATA-CLASS-000"
helpers_email = "test-mock-team@example.com"
helpers_repository = "test-mock-owner/test-mock-repo"
helpers_team = "TEST-MOCK-TEAM-000"

}
9 changes: 7 additions & 2 deletions tests/fixtures/default/regional/istio-csr/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ data "terraform_remote_state" "regional" {
module "test" {
source = "../../../../../regional/istio-csr"

artifact_registry = "mock-docker.pkg.dev/mock-project/mock-virtual"
cluster_prefix = "mock-cluster"
artifact_registry = "mock-docker.pkg.dev/mock-project/mock-virtual"
cluster_prefix = "mock-cluster"
helpers_cost_center = var.helpers_cost_center
helpers_data_classification = var.helpers_data_classification
helpers_email = var.helpers_email
helpers_repository = var.helpers_repository
helpers_team = var.helpers_team
}
22 changes: 22 additions & 0 deletions tests/fixtures/default/regional/istio-csr/variables.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Input Variables
# https://www.terraform.io/language/values/variables

variable "helpers_cost_center" {
type = string
}

variable "helpers_data_classification" {
type = string
}

variable "helpers_email" {
type = string
}

variable "helpers_repository" {
type = string
}

variable "helpers_team" {
type = string
}
7 changes: 6 additions & 1 deletion tests/fixtures/default/regional/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -63,5 +63,10 @@ data "terraform_remote_state" "regional" {
module "test" {
source = "../../../../regional"

artifact_registry = "mock-docker.pkg.dev/mock-project/mock-virtual"
artifact_registry = "mock-docker.pkg.dev/mock-project/mock-virtual"
helpers_cost_center = var.helpers_cost_center
helpers_data_classification = var.helpers_data_classification
helpers_email = var.helpers_email
helpers_repository = var.helpers_repository
helpers_team = var.helpers_team
Comment on lines +21 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding variable validation for helper values.

While the mock values are appropriate for testing, consider adding variable validation blocks in the actual module to ensure these helper values meet your organization's standards (e.g., specific email formats, repository naming conventions).

Example validation block to add in the main module:

variable "helpers_email" {
  description = "Contact email for the resource owner"
  type        = string
  
  validation {
    condition     = can(regex("^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$", var.helpers_email))
    error_message = "The helpers_email value must be a valid email address."
  }
}

}
22 changes: 22 additions & 0 deletions tests/fixtures/default/regional/variables.tf
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Input Variables
# https://www.terraform.io/language/values/variables

variable "helpers_cost_center" {
type = string
}

variable "helpers_data_classification" {
type = string
}

variable "helpers_email" {
type = string
}

variable "helpers_repository" {
type = string
}

variable "helpers_team" {
type = string
}