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

Major refactor for GCP modules and examples #57

Open
horiagunica opened this issue Jan 15, 2025 · 0 comments
Open

Major refactor for GCP modules and examples #57

horiagunica opened this issue Jan 15, 2025 · 0 comments
Labels
enhancement New feature or request

Comments

@horiagunica
Copy link
Contributor

horiagunica commented Jan 15, 2025

Hub issue for major refactor of examples and modules for GCP SWFW.

Introduction

  • Ensure all the cloud resources get created in GCP doesn't have any hardcoded suffixes/prefixes. The name values should be configurable via variables.
  • define types for all variables, even complex ones, use optional keyword
  • bump min supported TF version to 1.5
  • migrate to new documentation format:
    • README in document layout
    • introduction, purpose, usage should be moved to .header.md
    • every variable should have a description, where the 1st sentence should contain a summary
    • when documenting complex types, follow the format described below
    • keep the variables in variables.tf in order that makes sense (see below)
    • keep the description of variables that repeat between example the same, like name or tags (see below for a copy&paste pattern to follow)
  • get rid of lookup/try statements where possible -> rely on default values (for complex variables as well)
  • get rid of boolean variables that do not add any value, like enable_zones - use smart defaults instead
  • adjust all examples to have the same main.tf for groups of deployments e.g. VM-Series , autoscaling, Cloud NGFW, Panorama or others if required
  • adjust all modules & examples to be IPv6 ready
  • be bold, if you see some old code, or you think something can be done better, DO IT (this will be a breaking change anyway)
  • when you work on a module, do fix all examples using that module. Make sure the code is the same across all examples
  • inside module, add a comment with a link to Terraform Registry before a resource block, like this:
    # https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/...
    resource "aws_vpc" "example" {
    name                = "example-vpc"
    ...
  • use a # EXAMPLE \n comment format to indicate a start of a section
  • Remove sets from each modules (ex. natgw, transit_gateway_attachment, etc.)
  • Adjust examples to use map of objects instead of single object
  • Added comments under each locals

Issues per module

(many issues have already been merged to main refactor branch in the old archived repo)

  • TBD

Issues per example

  • TBD

Module Refactor Considerations

  • Reconsider the implementation of set concept in the current modules.
  • Avoid rigid variable construction concepts (for example, implicit specification of variables which means the first time users need to have some domain specific knowledge in order to use the example. For more context fix(examples): Refactor examples for reference architectures terraform-aws-swfw-modules#49)
  • Collapse multiple modules into one where it makes sense
  • Keep the module separated as is if the complexity going to increase
  • Restructure the naming of the outputs (align input and output names where applicable and also keep consistency in the noun usage across modules). This will improve the readability of the code for users

Variables ordering

Some basic principles:

  • ensure the variable descriptions are consistent across all the examples (and modules if applicable)
  • unless a module is constructed in a different way, variables.tf should be sorted based on logical hierarchical order:
    • name
    • region
  • keep vars related to the same type of resource next to each other, i.e. create_subnets should be next to subnets
  • order the variables, either:
    • based on decision steps when designing infrastructure, i.e. create_subnets 1st, then subnets definition (you have to decide if you create subnets, or do you source them, then you provide the specs, which will differ based on your decision)
    • based on dependencies between resources, i.e. you define SGs and RTs 1st, then Subnets, as in subnets variable we point to already defined SGs and RTs
    • if no other criteria matches, based on importance or usage frequency
  • group the variables into more complex objects where it makes sense and improves module's readability, e.g. variable interfaces grouping all network interface related settings

Keep in mind that his order will be retained in a README.md.

Description format

Follow the example below:

description = <<-EOF
A short, one sentence description of the variable.

Some more detailed description, can be multiple lines.

List of either required or important properties:

- `name`                   - (`string`, required) name of the Network Security Group.
- `some_optional_value`    - (`string`, optional, defaults to `null`) some description.
- `some_complex_property`  - (`map`, optional) A list of objects representing something.
  - `name`                    - (`string`, required) name of the something.
  - `some_number`             - (`number`, optional, defaults to `5`) numeric value.
  - `some_value_1`            - (`string`, required, mutually exclusive with `some_value_2`) some description.
  - `some_value_2`            - (`string`, required, mutually exclusive with `some_value_1`) some description.
  - `some_optional_value`     - (`bool`, optional, defaults to `false`) some description.

List of other, optional properties:

- `less_important_1`    - (`string`, optional, defaults to `null`) some description.
- `less_important_2`    - (`string`, optional, defaults Azure defaults) some description.
- `less_important_3`    - (`string`, optional, defaults to `""`) some description.
- `less_important_4`    - (`list(string)`, optional, defaults to `[]`) some description.

Example:
```hcl
{
  "object_1" = {
    name = "name of object 1"
    .....
  }
}
```
EOF

Common variables

Replace the following variables with these definitions:

variable "name" {
  description = "The name of the Virtual Private Network."
  type        = string
}

variable "tags" {
  description = "The map of tags to assign to all created resources."
  default     = {}
  type        = map(string)
}

Bootstrap options for examples

The new examples will support the default bootstrap method (user-data) and may be ability to use other bootstrap methods as options (by commenting out certain variables)

@horiagunica horiagunica added the enhancement New feature or request label Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant