From 2c5e7105e3c3089edece342261c34ec2e45c5890 Mon Sep 17 00:00:00 2001 From: Ludo Date: Fri, 12 Apr 2024 10:56:15 +0200 Subject: [PATCH 01/15] reorder tfdoc methods --- tools/tfdoc.py | 298 ++++++++++++++++++++++++------------------------- 1 file changed, 146 insertions(+), 152 deletions(-) diff --git a/tools/tfdoc.py b/tools/tfdoc.py index 9bcfa30fdb..7db145ad2f 100755 --- a/tools/tfdoc.py +++ b/tools/tfdoc.py @@ -49,16 +49,15 @@ import click import marko -# manipulate path to import COUNT_TEST_RE from tests/examples/test_plan.py -REPO_ROOT = os.path.dirname(os.path.dirname(__file__)) -sys.path.append(os.path.join(REPO_ROOT, 'tests')) - -from examples.test_plan import COUNT_TEST_RE - __version__ = '2.1.0' +# COUNT_TEST_RE copied from tests/examples/test_plan.py +COUNT_TEST_RE = re.compile( + r'# tftest +modules=(?P\d+) +resources=(?P\d+)' + + r'(?: +files=(?P[\w@,_-]+))?' + + r'(?: +fixtures=(?P[\w@,_/.-]+))?' + + r'(?: +inventory=(?P[\w\-.]+))?') # TODO(ludomagno): decide if we want to support variables*.tf and outputs*.tf - FILE_DESC_DEFAULTS = { 'main.tf': 'Module-level locals and resources.', 'outputs.tf': 'Module outputs.', @@ -87,6 +86,7 @@ (?:^(.*?)$) ''') OUT_TEMPLATE = ('description', 'value', 'sensitive') +REPO_ROOT = os.path.dirname(os.path.dirname(__file__)) TAG_RE = re.compile(r'(?sm)^\s*#\stfdoc:([^:]+:\S+)\s+(.*?)\s*$') TOC_BEGIN = '' TOC_END = '' @@ -116,7 +116,11 @@ Variable = collections.namedtuple( 'Variable', 'name description type default required nullable source file line') -# parsing functions + + +def _escape(s): + 'Basic, minimal HTML escaping' + return ''.join(c if c in UNESCAPED else ('&#%s;' % ord(c)) for c in s) def _extract_tags(body): @@ -158,107 +162,40 @@ def _parse(body, enum=VAR_ENUM, re=VAR_RE, template=VAR_TEMPLATE): item[context].append(data) -def parse_files(basepath, exclude_files=None): - 'Return a list of File named tuples in root module at basepath.' - exclude_files = exclude_files or [] - for name in glob.glob(os.path.join(basepath, '*tf')): - if os.path.islink(name): - continue - shortname = os.path.basename(name) - if shortname in exclude_files: - continue - try: - with open(name, encoding='utf-8') as file: - body = file.read() - except (IOError, OSError) as e: - raise SystemExit(f'Cannot read file {name}: {e}') - tags = _extract_tags(body) - description = tags.get('file:description', - FILE_DESC_DEFAULTS.get(shortname)) - modules = set( - os.path.basename(urllib.parse.urlparse(m).path) - for m in FILE_RE_MODULES.findall(body)) - resources = set(FILE_RE_RESOURCES.findall(body)) - yield File(shortname, description, modules, resources) - - -def parse_outputs(basepath, exclude_files=None): - 'Return a list of Output named tuples for root module outputs*.tf.' - exclude_files = exclude_files or [] - names = glob.glob(os.path.join(basepath, 'outputs*tf')) - names += glob.glob(os.path.join(basepath, 'local-*outputs*tf')) - for name in names: - shortname = os.path.basename(name) - if shortname in exclude_files: - continue - try: - with open(name, encoding='utf-8') as file: - body = file.read() - except (IOError, OSError) as e: - raise SystemExit(f'Cannot open outputs file {shortname}.') - for item in _parse(body, enum=OUT_ENUM, re=OUT_RE, template=OUT_TEMPLATE): - description = ''.join(item['description']) - sensitive = item['sensitive'] != [] - consumers = item['tags'].get('output:consumers', '') - yield Output(name=item['name'], description=description, - sensitive=sensitive, consumers=consumers, file=shortname, - line=item['line']) - - -def parse_variables(basepath, exclude_files=None): - 'Return a list of Variable named tuples for root module variables*.tf.' - exclude_files = exclude_files or [] - names = glob.glob(os.path.join(basepath, 'variables*tf')) - names += glob.glob(os.path.join(basepath, 'local-*variables*tf')) - for name in names: - shortname = os.path.basename(name) - if shortname in exclude_files: - continue - try: - with open(name, encoding='utf-8') as file: - body = file.read() - except (IOError, OSError) as e: - raise SystemExit(f'Cannot open variables file {shortname}.') - for item in _parse(body): - description = (''.join(item['description'])).replace('|', '\\|') - vtype = '\n'.join(item['type']) - default = HEREDOC_RE.sub(r'\1', '\n'.join(item['default'])) - required = not item['default'] - nullable = item.get('nullable') != ['false'] - source = item['tags'].get('variable:source', '') - if not required and default != 'null' and vtype == 'string': - default = f'"{default}"' - - yield Variable(name=item['name'], description=description, type=vtype, - default=default, required=required, source=source, - file=shortname, line=item['line'], nullable=nullable) - - -def parse_fixtures(basepath, readme): - 'Return a list of file paths of all the unique fixtures used in the module.' +def create_toc(readme): + 'Create a Markdown table of contents a for README.' doc = marko.parse(readme) - used_fixtures = set() - for child in doc.children: - if isinstance(child, marko.block.FencedCode): - if child.lang == 'hcl': - code = child.children[0].children - if match := COUNT_TEST_RE.search(code): - if fixtures := match.group('fixtures'): - for fixture in fixtures.split(','): - fixture_full = os.path.join(REPO_ROOT, 'tests', fixture) - if not os.path.exists(fixture_full): - raise SystemExit(f'Unknown fixture: {fixture}') - fixture_relative = os.path.relpath(fixture_full, basepath) - used_fixtures.add(fixture_relative) - yield from sorted(used_fixtures) - - -# formatting functions + lines = [] + headings = [x for x in doc.children if x.get_type() == 'Heading'] + for h in headings[1:]: + title = h.children[0].children + slug = title.lower().strip() + slug = re.sub(r'[^\w\s-]', '', slug) + slug = re.sub(r'[-\s]+', '-', slug) + link = f'- [{title}](#{slug})' + indent = ' ' * (h.level - 2) + lines.append(f'{indent}{link}') + return "\n".join(lines) -def _escape(s): - 'Basic, minimal HTML escaping' - return ''.join(c if c in UNESCAPED else ('&#%s;' % ord(c)) for c in s) +def create_tfref(module_path, files=False, show_extra=False, exclude_files=None, + readme=None): + 'Return tfdoc mark and generated content.' + if readme: + # check for overrides in doc + opts = get_tfref_opts(readme) + files = opts.get('files', files) + show_extra = opts.get('show_extra', show_extra) + try: + mod_files = list(parse_files(module_path, exclude_files)) if files else [] + mod_variables = list(parse_variables(module_path, exclude_files)) + mod_outputs = list(parse_outputs(module_path, exclude_files)) + mod_fixtures = list(parse_fixtures(module_path, readme)) + except (IOError, OSError) as e: + raise SystemExit(e) + doc = format_tfref(mod_outputs, mod_variables, mod_files, mod_fixtures, + show_extra) + return Document(doc, mod_files, mod_variables, mod_outputs) def format_tfref(outputs, variables, files, fixtures, show_extra=False): @@ -301,6 +238,12 @@ def format_tfref_files(items): f' {resources} |' if num_resources else '') +def format_tfref_fixtures(items): + 'Format fixtures table.' + for x in items: + yield f"- [{os.path.basename(x)}]({x})" + + def format_tfref_outputs(items, show_extra=True): 'Format outputs table.' if not items: @@ -352,29 +295,12 @@ def format_tfref_variables(items, show_extra=True): yield format -def format_tfref_fixtures(items): - 'Format fixtures table.' - for x in items: - yield f"- [{os.path.basename(x)}]({x})" - - -def create_toc(readme): - 'Create a Markdown table of contents a for README.' - doc = marko.parse(readme) - lines = [] - headings = [x for x in doc.children if x.get_type() == 'Heading'] - for h in headings[1:]: - title = h.children[0].children - slug = title.lower().strip() - slug = re.sub(r'[^\w\s-]', '', slug) - slug = re.sub(r'[-\s]+', '-', slug) - link = f'- [{title}](#{slug})' - indent = ' ' * (h.level - 2) - lines.append(f'{indent}{link}') - return "\n".join(lines) - - -# replace functions +def get_readme(readme_path): + 'Open and return README.md in module.' + try: + return open(readme_path, "r", encoding="utf-8").read() + except (IOError, OSError) as e: + raise SystemExit(f'Error opening README {readme_path}: {e}') def get_tfref_parts(readme): @@ -408,31 +334,99 @@ def get_tfref_opts(readme): return opts -def create_tfref(module_path, files=False, show_extra=False, exclude_files=None, - readme=None): - if readme: - # check for overrides in doc - opts = get_tfref_opts(readme) - files = opts.get('files', files) - show_extra = opts.get('show_extra', show_extra) - try: - mod_files = list(parse_files(module_path, exclude_files)) if files else [] - mod_variables = list(parse_variables(module_path, exclude_files)) - mod_outputs = list(parse_outputs(module_path, exclude_files)) - mod_fixtures = list(parse_fixtures(module_path, readme)) - except (IOError, OSError) as e: - raise SystemExit(e) - doc = format_tfref(mod_outputs, mod_variables, mod_files, mod_fixtures, - show_extra) - return Document(doc, mod_files, mod_variables, mod_outputs) +def parse_files(basepath, exclude_files=None): + 'Return a list of File named tuples in root module at basepath.' + exclude_files = exclude_files or [] + for name in glob.glob(os.path.join(basepath, '*tf')): + if os.path.islink(name): + continue + shortname = os.path.basename(name) + if shortname in exclude_files: + continue + try: + with open(name, encoding='utf-8') as file: + body = file.read() + except (IOError, OSError) as e: + raise SystemExit(f'Cannot read file {name}: {e}') + tags = _extract_tags(body) + description = tags.get('file:description', + FILE_DESC_DEFAULTS.get(shortname)) + modules = set( + os.path.basename(urllib.parse.urlparse(m).path) + for m in FILE_RE_MODULES.findall(body)) + resources = set(FILE_RE_RESOURCES.findall(body)) + yield File(shortname, description, modules, resources) -def get_readme(readme_path): - 'Open and return README.md in module.' - try: - return open(readme_path, "r", encoding="utf-8").read() - except (IOError, OSError) as e: - raise SystemExit(f'Error opening README {readme_path}: {e}') +def parse_fixtures(basepath, readme): + 'Return a list of file paths of all the unique fixtures used in the module.' + doc = marko.parse(readme) + used_fixtures = set() + for child in doc.children: + if isinstance(child, marko.block.FencedCode): + if child.lang == 'hcl': + code = child.children[0].children + if match := COUNT_TEST_RE.search(code): + if fixtures := match.group('fixtures'): + for fixture in fixtures.split(','): + fixture_full = os.path.join(REPO_ROOT, 'tests', fixture) + if not os.path.exists(fixture_full): + raise SystemExit(f'Unknown fixture: {fixture}') + fixture_relative = os.path.relpath(fixture_full, basepath) + used_fixtures.add(fixture_relative) + yield from sorted(used_fixtures) + + +def parse_outputs(basepath, exclude_files=None): + 'Return a list of Output named tuples for root module outputs*.tf.' + exclude_files = exclude_files or [] + names = glob.glob(os.path.join(basepath, 'outputs*tf')) + names += glob.glob(os.path.join(basepath, 'local-*outputs*tf')) + for name in names: + shortname = os.path.basename(name) + if shortname in exclude_files: + continue + try: + with open(name, encoding='utf-8') as file: + body = file.read() + except (IOError, OSError) as e: + raise SystemExit(f'Cannot open outputs file {shortname}.') + for item in _parse(body, enum=OUT_ENUM, re=OUT_RE, template=OUT_TEMPLATE): + description = ''.join(item['description']) + sensitive = item['sensitive'] != [] + consumers = item['tags'].get('output:consumers', '') + yield Output(name=item['name'], description=description, + sensitive=sensitive, consumers=consumers, file=shortname, + line=item['line']) + + +def parse_variables(basepath, exclude_files=None): + 'Return a list of Variable named tuples for root module variables*.tf.' + exclude_files = exclude_files or [] + names = glob.glob(os.path.join(basepath, 'variables*tf')) + names += glob.glob(os.path.join(basepath, 'local-*variables*tf')) + for name in names: + shortname = os.path.basename(name) + if shortname in exclude_files: + continue + try: + with open(name, encoding='utf-8') as file: + body = file.read() + except (IOError, OSError) as e: + raise SystemExit(f'Cannot open variables file {shortname}.') + for item in _parse(body): + description = (''.join(item['description'])).replace('|', '\\|') + vtype = '\n'.join(item['type']) + default = HEREDOC_RE.sub(r'\1', '\n'.join(item['default'])) + required = not item['default'] + nullable = item.get('nullable') != ['false'] + source = item['tags'].get('variable:source', '') + if not required and default != 'null' and vtype == 'string': + default = f'"{default}"' + + yield Variable(name=item['name'], description=description, type=vtype, + default=default, required=required, source=source, + file=shortname, line=item['line'], nullable=nullable) def render_tfref(readme, doc): From ad6ee8e5a20c99b6fd5c47bf101130d1d0b34119 Mon Sep 17 00:00:00 2001 From: Ludo Date: Fri, 12 Apr 2024 13:44:18 +0200 Subject: [PATCH 02/15] add support for recipes to tfdoc --- tools/tfdoc.py | 57 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 10 deletions(-) diff --git a/tools/tfdoc.py b/tools/tfdoc.py index 7db145ad2f..fae6bc244a 100755 --- a/tools/tfdoc.py +++ b/tools/tfdoc.py @@ -86,7 +86,9 @@ (?:^(.*?)$) ''') OUT_TEMPLATE = ('description', 'value', 'sensitive') +RECIPE_RE = re.compile(r'(?sm)^#\s*(.*?)$') REPO_ROOT = os.path.dirname(os.path.dirname(__file__)) +REPO_URL = 'https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/blob/master/' TAG_RE = re.compile(r'(?sm)^\s*#\stfdoc:([^:]+:\S+)\s+(.*?)\s*$') TOC_BEGIN = '' TOC_END = '' @@ -113,6 +115,7 @@ File = collections.namedtuple('File', 'name description modules resources') Output = collections.namedtuple( 'Output', 'name description sensitive consumers file line') +Recipe = collections.namedtuple('Recipe', 'path title') Variable = collections.namedtuple( 'Variable', 'name description type default required nullable source file line') @@ -186,7 +189,13 @@ def create_tfref(module_path, files=False, show_extra=False, exclude_files=None, opts = get_tfref_opts(readme) files = opts.get('files', files) show_extra = opts.get('show_extra', show_extra) + abspath = os.path.abspath(module_path) try: + if os.path.dirname(abspath).endswith('/modules'): + mod_recipes = parse_recipes( + module_path, f'{REPO_URL}/modules/{os.path.basename(abspath)}') + else: + mod_recipes = None mod_files = list(parse_files(module_path, exclude_files)) if files else [] mod_variables = list(parse_variables(module_path, exclude_files)) mod_outputs = list(parse_outputs(module_path, exclude_files)) @@ -194,13 +203,17 @@ def create_tfref(module_path, files=False, show_extra=False, exclude_files=None, except (IOError, OSError) as e: raise SystemExit(e) doc = format_tfref(mod_outputs, mod_variables, mod_files, mod_fixtures, - show_extra) + mod_recipes, show_extra) return Document(doc, mod_files, mod_variables, mod_outputs) -def format_tfref(outputs, variables, files, fixtures, show_extra=False): +def format_tfref(outputs, variables, files, fixtures, recipes=None, + show_extra=False): 'Return formatted document.' buffer = [] + if recipes: + buffer += ['', '## Recipes', ''] + buffer += list(format_tfref_recipes(recipes)) if files: buffer += ['', '## Files', ''] buffer += list(format_tfref_files(files)) @@ -262,6 +275,14 @@ def format_tfref_outputs(items, show_extra=True): yield format +def format_tfref_recipes(recipes): + 'Format recipes list.' + if not recipes: + return + for r in recipes: + yield f'- [{r.title}]({r.path})' + + def format_tfref_variables(items, show_extra=True): 'Format variables table.' if not items: @@ -311,14 +332,6 @@ def get_tfref_parts(readme): return {'doc': m.group(1).strip(), 'start': m.start(), 'end': m.end()} -def get_toc_parts(readme): - 'Check if README file is marked, and return current toc.' - t = re.search('(?sm)%s(.*)%s' % (TOC_BEGIN, TOC_END), readme) - if not t: - return - return {'toc': t.group(1).strip(), 'start': t.start(), 'end': t.end()} - - def get_tfref_opts(readme): 'Check if README file is setting options via a mark, and return options.' m = MARK_OPTS_RE.search(readme) @@ -334,6 +347,14 @@ def get_tfref_opts(readme): return opts +def get_toc_parts(readme): + 'Check if README file is marked, and return current toc.' + t = re.search('(?sm)%s(.*)%s' % (TOC_BEGIN, TOC_END), readme) + if not t: + return + return {'toc': t.group(1).strip(), 'start': t.start(), 'end': t.end()} + + def parse_files(basepath, exclude_files=None): 'Return a list of File named tuples in root module at basepath.' exclude_files = exclude_files or [] @@ -400,6 +421,22 @@ def parse_outputs(basepath, exclude_files=None): line=item['line']) +def parse_recipes(module_path, module_url): + 'Find and return module recipes.' + for dirpath, dirnames, filenames in os.walk(module_path): + name = os.path.basename(dirpath) + if name.startswith('recipe-') and 'README.md' in filenames: + try: + with open(os.path.join(dirpath, 'README.md'), encoding='utf-8') as f: + match = RECIPE_RE.search(f.read()) + if match: + yield Recipe(f'{module_url}/{name}', match.group(1)) + else: + raise SystemExit(f'No title for recipe {dirpath}') + except (IOError, OSError) as e: + raise SystemExit(f'Error opening recipe {dirpath}') + + def parse_variables(basepath, exclude_files=None): 'Return a list of Variable named tuples for root module variables*.tf.' exclude_files = exclude_files or [] From 034a9f4216c7874747ccf9fd9bc2b4dcae2be371 Mon Sep 17 00:00:00 2001 From: Ludo Date: Fri, 12 Apr 2024 18:08:12 +0200 Subject: [PATCH 03/15] fix repo url in tfdoc --- tools/tfdoc.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/tfdoc.py b/tools/tfdoc.py index fae6bc244a..1b0536d438 100755 --- a/tools/tfdoc.py +++ b/tools/tfdoc.py @@ -88,7 +88,7 @@ OUT_TEMPLATE = ('description', 'value', 'sensitive') RECIPE_RE = re.compile(r'(?sm)^#\s*(.*?)$') REPO_ROOT = os.path.dirname(os.path.dirname(__file__)) -REPO_URL = 'https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/blob/master/' +REPO_URL = 'https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/blob/master' TAG_RE = re.compile(r'(?sm)^\s*#\stfdoc:([^:]+:\S+)\s+(.*?)\s*$') TOC_BEGIN = '' TOC_END = '' From cc96246363b24460c2b3963dc8aeb798ec646b22 Mon Sep 17 00:00:00 2001 From: Ludo Date: Fri, 12 Apr 2024 18:08:32 +0200 Subject: [PATCH 04/15] update module README --- modules/net-lb-app-int-cross-region/README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/modules/net-lb-app-int-cross-region/README.md b/modules/net-lb-app-int-cross-region/README.md index 003861e63b..e49fa2bd28 100644 --- a/modules/net-lb-app-int-cross-region/README.md +++ b/modules/net-lb-app-int-cross-region/README.md @@ -19,6 +19,7 @@ Due to the complexity of the underlying resources, changes to the configuration - [Private Service Connect NEG creation](#private-service-connect-neg-creation) - [URL Map](#url-map) - [Complex example](#complex-example) +- [Recipes](#recipes) - [Files](#files) - [Variables](#variables) - [Outputs](#outputs) @@ -721,6 +722,10 @@ module "ilb-l7" { +## Recipes + +- [Cross-region internal Application Load Balancer with VM instance group backends](https://github.com/GoogleCloudPlatform/cloud-foundation-fabric/blob/master/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns) + ## Files | name | description | resources | From 23d9503c35a56527ab58aa637fad2ecc1f29a043 Mon Sep 17 00:00:00 2001 From: Ludo Date: Fri, 12 Apr 2024 18:09:18 +0200 Subject: [PATCH 05/15] validated untested recipe --- .../README.md | 40 +++++ .../main.tf | 151 ++++++++++++++++++ .../nginx-cloud-config.yaml | 79 +++++++++ .../outputs.tf | 16 ++ .../region_shortnames.tf | 41 +++++ .../variables.tf | 94 +++++++++++ modules/net-lb-app-int/README.md | 4 + 7 files changed, 425 insertions(+) create mode 100644 modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md create mode 100644 modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/main.tf create mode 100644 modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/nginx-cloud-config.yaml create mode 100644 modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/outputs.tf create mode 100644 modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/region_shortnames.tf create mode 100644 modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/variables.tf diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md new file mode 100644 index 0000000000..ec397d49cb --- /dev/null +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md @@ -0,0 +1,40 @@ +# Cross-region internal Application Load Balancer with VM instance group backends + +This recipe shows an actual usage scenario for the [cross-region internal application load balancer](../README.md) by implementing the [example provided in the GCP documentation](https://cloud.google.com/load-balancing/docs/l7-internal/setting-up-l7-cross-reg-internal). + +

+ Scenario diagram. +

+ +## Prerequisites + +### Proxy-only global subnets + +The load balancer needs one proxy-only global subnet in each of its regions. If you are using the [`net-vpc`](../../net-vpc/) module with the subnet factory enabled, this is an example on how to create one of the subnets. + +```yaml +# data/subnets/proxy-global-ew1.yaml +region: europe-west1 +ip_cidr_range: 172.16.192.0/24 +proxy_only: true +global: true +description: Terraform-managed proxy-only subnet for Regional HTTPS or Internal HTTPS LB. +``` + +### Firewall rules + +## Variable configuration + +## Testing + +## Variables + +| name | description | type | required | default | +|---|---|:---:|:---:|:---:| +| [project_id](variables.tf#L50) | Project used to create resources. | string | ✓ | | +| [vpc_config](variables.tf#L70) | VPC configuration for load balancer and instances. | object({…}) | ✓ | | +| [dns_config](variables.tf#L17) | DNS configuration. | object({…}) | | {} | +| [instances_config](variables.tf#L28) | Configuration for instances. | object({…}) | | {} | +| [prefix](variables.tf#L43) | Prefix used for or in resource names. | string | | "lb-xr-00" | +| [regions](variables.tf#L56) | Regions used for the compute resources, use shortnames as keys. | map(string) | | {…} | + diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/main.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/main.tf new file mode 100644 index 0000000000..10f785a87e --- /dev/null +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/main.tf @@ -0,0 +1,151 @@ +/** + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +locals { + _instance_subnets = ( + var.vpc_config.instance_subnets == null + ? var.vpc_config.load_balancer_subnets + : var.vpc_config.instance_subnets + ) + _instance_regions = { + for v in var.vpc_config.instance_subnets : + regex("/regions/([^/]+)/", v) => v + } + _lb_subnets = [ + for v in var.vpc_config.load_balancer_subnets : merge( + regex("/regions/(?P[^/]+)/subnetworks/(?P[^/]+)$"), + { subnet = v } + ) + ] + instances = flatten([ + for region, subnet in local._instance_regions : [ + for zone in var.instances_config.zones : [ + for num in range(var.instances_config.count) : { + key = join("-", [var.prefix, region, zone, num]) + subnet = subnet + zone = "${region}-${zone}" + } + ] + ] + ]) + lb_regions = { + for v in var.vpc_config.load_balancer_subnets : + regex("/regions/([^/]+)/", v) => v + } + lb_subnets = { + for v in local._lb_subnets : + "${local.region_shortnames[v.region]}-${v.name}" => v.subnet + } +} + +module "instance-sa" { + source = "../../iam-service-account" + project_id = var.project_id + name = "vm-default" + prefix = var.prefix + display_name = "Cross-region LB instances service account" + iam_project_roles = { + (var.project_id) = [ + "roles/logging.logWriter", + "roles/monitoring.metricWriter" + ] + } +} + +module "instances" { + source = "../../compute-vm" + for_each = { for v in local.instances : v.key => v } + project_id = var.project_id + zone = each.value.zone + name = each.key + instance_type = var.instances_config.machine_type + boot_disk = { + initialize_params = { + image = "projects/cos-cloud/global/images/family/cos-stable" + } + } + network_interfaces = [{ + network = var.vpc_config.network + subnetwork = each.value.subnet + }] + tags = [ + "${var.prefix}-ssh", "${var.prefix}-http-server", "${var.prefix}-proxy" + ] + metadata = { + user-data = file("nginx-cloud-config.yaml") + } + service_account = { + email = module.instance-sa.email + } + group = { + named_ports = { + http = 80 + https = 443 + } + } +} + +module "load-balancer" { + source = "../../net-lb-app-int-cross-region" + name = var.prefix + project_id = var.project_id + backend_service_configs = { + default = { + port_name = "http" + backends = [ + for k, v in module.instances : { group = v.group.id } + ] + } + } + vpc_config = { + network = var.vpc_config.network + subnetworks = local.lb_subnets + } +} + +module "dns" { + source = "../../dns" + project_id = var.project_id + name = var.prefix + zone_config = { + domain = var.dns_config.domain + private = { + client_networks = ( + var.dns_config.client_networks != null + ? var.dns_config.client_networks + : [var.vpc_config.network] + ) + } + } + recordsets = { + "A ${coalesce(var.dns_config.hostname, var.prefix)}" = { + geo_routing = [ + for k, v in local.lb_regions : { + location = k + health_checked_targets = [ + { + load_balancer_type = "globalL7ilb" + ip_address = module.load-balancer.addresses[k] + port = "80" + ip_protocol = "tcp" + network_url = var.vpc_config.network + project = var.project_id + } + ] } + ] + } + } +} diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/nginx-cloud-config.yaml b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/nginx-cloud-config.yaml new file mode 100644 index 0000000000..7817eb606b --- /dev/null +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/nginx-cloud-config.yaml @@ -0,0 +1,79 @@ +#cloud-config + +# Copyright 2022 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# https://hub.docker.com/r/nginx/nginx/ +# https://nginx.io/manual/toc/#installation + +users: + - name: nginx + uid: 2000 + +write_files: + - path: /var/lib/docker/daemon.json + permissions: 0644 + owner: root + content: | + { + "live-restore": true, + "storage-driver": "overlay2", + "log-opts": { + "max-size": "1024m" + } + } + + - path: /etc/nginx/nginx.conf + permissions: 0644 + owner: root + content: | + server { + listen 80; + listen 8080; + listen [::]:80; + listen [::]:8080; + + location / { + default_type text/plain; + expires -1; + return 200 'Server address: $server_addr:$server_port\nRemote address: $remote_addr\nServer name: $hostname\nDate: $time_local\nURI: $request_uri\nRequest ID: $request_id\n'; + } + } + + # nginx container service + - path: /etc/systemd/system/nginx.service + permissions: 0644 + owner: root + content: | + [Unit] + Description=Start nginx container + After=gcr-online.target docker.socket + Wants=gcr-online.target docker.socket docker-events-collector.service + [Service] + Environment="HOME=/home/nginx" + ExecStartPre=/usr/bin/docker-credential-gcr configure-docker + ExecStart=/usr/bin/docker run --rm --name=nginx \ + --network host \ + -v /etc/nginx:/etc/nginx/conf.d \ + nginxdemos/hello:plain-text + ExecStop=/usr/bin/docker stop nginx + +bootcmd: + - systemctl start node-problem-detector + +runcmd: + - iptables -I INPUT 1 -p tcp -m tcp --dport 80 -m state --state NEW,ESTABLISHED -j ACCEPT + - iptables -I INPUT 1 -p tcp -m tcp --dport 8080 -m state --state NEW,ESTABLISHED -j ACCEPT + - systemctl daemon-reload + - systemctl start nginx diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/outputs.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/outputs.tf new file mode 100644 index 0000000000..35828ec07e --- /dev/null +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/outputs.tf @@ -0,0 +1,16 @@ +/** + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/region_shortnames.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/region_shortnames.tf new file mode 100644 index 0000000000..50e0df421b --- /dev/null +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/region_shortnames.tf @@ -0,0 +1,41 @@ +/** + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +# borrowed from FAST networking stages + +locals { + # used when the first character would not work + _region_cardinal = { + southeast = "se" + } + _region_geo = { + australia = "o" + } + # split in [geo, cardinal, number] tokens + _region_tokens = { + for v in local.lb_regions : v => regexall("(?:[a-z]+)|(?:[0-9]+)", v) + } + region_shortnames = { + for k, v in local._region_tokens : k => join("", [ + # first token via geo alias map or first character + lookup(local._region_geo, v.0, substr(v.0, 0, 1)), + # first token via cardinal alias map or first character + lookup(local._region_cardinal, v.1, substr(v.1, 0, 1)), + # region number as is + v.2 + ]) + } +} diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/variables.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/variables.tf new file mode 100644 index 0000000000..9d2a4470c3 --- /dev/null +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/variables.tf @@ -0,0 +1,94 @@ +/** + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +variable "dns_config" { + description = "DNS configuration." + type = object({ + client_networks = optional(list(string)) + domain = optional(string, "gce.example.") + hostname = optional(string) + }) + nullable = false + default = {} +} + +variable "instances_config" { + description = "Configuration for instances." + type = object({ + count = optional(number, 1) + machine_type = optional(string, "e2-micro") + zones = optional(list(string), ["b"]) + }) + nullable = false + default = {} + validation { + condition = length(var.instances_config.zones) > 0 + error_message = "At least one zone is required for instances." + } +} + +variable "prefix" { + description = "Prefix used for or in resource names." + type = string + nullable = false + default = "lb-xr-00" +} + +variable "project_id" { + description = "Project used to create resources." + type = string + nullable = false +} + +variable "regions" { + description = "Regions used for the compute resources, use shortnames as keys." + type = map(string) + nullable = false + default = { + ew1 = "europe-west1" + ew3 = "europe-west3" + } + validation { + condition = length(var.regions) >= 2 + error_message = "At least two regions are required." + } +} + +variable "vpc_config" { + description = "VPC configuration for load balancer and instances." + type = object({ + load_balancer_subnets = map(string) + network = string + instance_subnets = optional(map(string)) + }) + nullable = false + validation { + condition = ( + toset([ + for s in var.vpc_config.load_balancer_subnets : + regex("/regions/([^/]+)/", s) + ]) + == + toset([ + for s in coalesce( + var.vpc_config.instance_subnets, + var.vpc_config.load_balancer_subnets + ) : regex("/regions/([^/]+)/", s) + ]) + ) + error_message = "Instance subnet regions must match load balancer regions." + } +} diff --git a/modules/net-lb-app-int/README.md b/modules/net-lb-app-int/README.md index 6d4ebda0f0..81da5131b9 100644 --- a/modules/net-lb-app-int/README.md +++ b/modules/net-lb-app-int/README.md @@ -21,6 +21,7 @@ Due to the complexity of the underlying resources, changes to the configuration - [SSL Certificates](#ssl-certificates) - [PSC service attachment](#psc-service-attachment) - [Complex example](#complex-example) +- [Recipes](#recipes) - [Files](#files) - [Variables](#variables) - [Outputs](#outputs) @@ -671,6 +672,9 @@ module "ilb-l7" { +## Recipes + + ## Files | name | description | resources | From 8ba3ecb71064c8f6a6485427649485cc50cb2e7d Mon Sep 17 00:00:00 2001 From: Ludo Date: Sat, 13 Apr 2024 09:27:35 +0200 Subject: [PATCH 06/15] validated untested refactored recipe --- .../README.md | 10 +- .../instances.tf | 75 ++++++++++ .../main.tf | 129 +++++++----------- .../region_shortnames.tf | 4 +- .../variables.tf | 31 ++--- 5 files changed, 142 insertions(+), 107 deletions(-) create mode 100644 modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/instances.tf diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md index ec397d49cb..b352ef6b2c 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md @@ -31,10 +31,10 @@ description: Terraform-managed proxy-only subnet for Regional HTTPS or Internal | name | description | type | required | default | |---|---|:---:|:---:|:---:| -| [project_id](variables.tf#L50) | Project used to create resources. | string | ✓ | | -| [vpc_config](variables.tf#L70) | VPC configuration for load balancer and instances. | object({…}) | ✓ | | +| [project_id](variables.tf#L49) | Project used to create resources. | string | ✓ | | +| [vpc_config](variables.tf#L69) | VPC configuration for load balancer and instances. Subnets are keyed by region. | object({…}) | ✓ | | | [dns_config](variables.tf#L17) | DNS configuration. | object({…}) | | {} | -| [instances_config](variables.tf#L28) | Configuration for instances. | object({…}) | | {} | -| [prefix](variables.tf#L43) | Prefix used for or in resource names. | string | | "lb-xr-00" | -| [regions](variables.tf#L56) | Regions used for the compute resources, use shortnames as keys. | map(string) | | {…} | +| [instances_config](variables.tf#L28) | Configuration for instances. | object({…}) | | {} | +| [prefix](variables.tf#L42) | Prefix used for or in resource names. | string | | "lb-xr-00" | +| [regions](variables.tf#L55) | Regions used for the compute resources, use shortnames as keys. | map(string) | | {…} | diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/instances.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/instances.tf new file mode 100644 index 0000000000..b98cd9bc8e --- /dev/null +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/instances.tf @@ -0,0 +1,75 @@ +/** + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +locals { + # use lb subnets for instances if instance subnets are not defined + subnets_instances = coalesce( + var.vpc_config.subnets_instances, var.vpc_config.subnets + ) + # derive instance names/attributes from permutation of regions and zones + instances = { + for t in setproduct(local.regions, var.instances_config.zones) : + "${var.prefix}-${local.region_shortnames[t.0]}-${t.1}" => { + region = t.0 + zone = t.1 + } + } +} + +module "instance-sa" { + source = "../../iam-service-account" + project_id = var.project_id + name = "vm-default" + prefix = var.prefix + display_name = "Cross-region LB instances service account" + iam_project_roles = { + (var.project_id) = [ + "roles/logging.logWriter", + "roles/monitoring.metricWriter" + ] + } +} + +module "instances" { + source = "../../compute-vm" + for_each = local.instances + project_id = var.project_id + zone = each.value.zone + name = each.key + instance_type = var.instances_config.machine_type + boot_disk = { + initialize_params = { + image = "projects/cos-cloud/global/images/family/cos-stable" + } + } + network_interfaces = [{ + network = var.vpc_config.network + subnetwork = local.subnets_instances[each.value.region] + }] + tags = [var.prefix] + metadata = { + user-data = file("nginx-cloud-config.yaml") + } + service_account = { + email = module.instance-sa.email + } + group = { + named_ports = { + http = 80 + https = 443 + } + } +} diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/main.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/main.tf index 10f785a87e..5033ec4b7a 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/main.tf +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/main.tf @@ -15,87 +15,8 @@ */ locals { - _instance_subnets = ( - var.vpc_config.instance_subnets == null - ? var.vpc_config.load_balancer_subnets - : var.vpc_config.instance_subnets - ) - _instance_regions = { - for v in var.vpc_config.instance_subnets : - regex("/regions/([^/]+)/", v) => v - } - _lb_subnets = [ - for v in var.vpc_config.load_balancer_subnets : merge( - regex("/regions/(?P[^/]+)/subnetworks/(?P[^/]+)$"), - { subnet = v } - ) - ] - instances = flatten([ - for region, subnet in local._instance_regions : [ - for zone in var.instances_config.zones : [ - for num in range(var.instances_config.count) : { - key = join("-", [var.prefix, region, zone, num]) - subnet = subnet - zone = "${region}-${zone}" - } - ] - ] - ]) - lb_regions = { - for v in var.vpc_config.load_balancer_subnets : - regex("/regions/([^/]+)/", v) => v - } - lb_subnets = { - for v in local._lb_subnets : - "${local.region_shortnames[v.region]}-${v.name}" => v.subnet - } -} - -module "instance-sa" { - source = "../../iam-service-account" - project_id = var.project_id - name = "vm-default" - prefix = var.prefix - display_name = "Cross-region LB instances service account" - iam_project_roles = { - (var.project_id) = [ - "roles/logging.logWriter", - "roles/monitoring.metricWriter" - ] - } -} - -module "instances" { - source = "../../compute-vm" - for_each = { for v in local.instances : v.key => v } - project_id = var.project_id - zone = each.value.zone - name = each.key - instance_type = var.instances_config.machine_type - boot_disk = { - initialize_params = { - image = "projects/cos-cloud/global/images/family/cos-stable" - } - } - network_interfaces = [{ - network = var.vpc_config.network - subnetwork = each.value.subnet - }] - tags = [ - "${var.prefix}-ssh", "${var.prefix}-http-server", "${var.prefix}-proxy" - ] - metadata = { - user-data = file("nginx-cloud-config.yaml") - } - service_account = { - email = module.instance-sa.email - } - group = { - named_ports = { - http = 80 - https = 443 - } - } + # define regions for both instances and lb for easy access + regions = keys(var.vpc_config.subnets) } module "load-balancer" { @@ -112,7 +33,7 @@ module "load-balancer" { } vpc_config = { network = var.vpc_config.network - subnetworks = local.lb_subnets + subnetworks = var.vpc_config.subnets } } @@ -133,7 +54,7 @@ module "dns" { recordsets = { "A ${coalesce(var.dns_config.hostname, var.prefix)}" = { geo_routing = [ - for k, v in local.lb_regions : { + for k, v in local.regions : { location = k health_checked_targets = [ { @@ -149,3 +70,45 @@ module "dns" { } } } + +module "firewall" { + source = "../../net-vpc-firewall" + count = var.vpc_config.firewall_config == null ? 0 : 1 + project_id = var.project_id + network = var.vpc_config.network + default_rules_config = { disabled = true } + ingress_rules = merge( + { + "ingress-${var.prefix}-proxies" = { + description = "Allow load balancer proxy traffic to instances." + source_ranges = var.vpc_config.firewall_config.proxy_subnet_ranges + targets = [var.prefix] + rules = [{ protocol = "tcp", ports = [80, 443] }] + } + }, + var.vpc_config.firewall_config.client_allowed_ranges == null ? {} : { + "ingress-${var.prefix}-http-clients" = { + description = "Allow client HTTP traffic to instances." + source_ranges = var.vpc_config.firewall_config.client_allowed_ranges + targets = [var.prefix] + rules = [{ protocol = "tcp", ports = [80, 443] }] + } + }, + var.vpc_config.firewall_config.enable_health_check != true ? {} : { + "ingress-${var.prefix}-health-checks" = { + description = "Allow health check traffic to instances." + source_ranges = ["35.191.0.0/16", "130.211.0.0/22"] + targets = [var.prefix] + rules = [{ protocol = "tcp", ports = [80, 443] }] + } + }, + var.vpc_config.firewall_config.enable_iap_ssh != true ? {} : { + "ingress-${var.prefix}-iap-ssh" = { + description = "Allow SSH traffic to instances via IAP." + source_ranges = ["35.235.240.0/20"] + targets = [var.prefix] + rules = [{ protocol = "tcp", ports = [22] }] + } + } + ) +} diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/region_shortnames.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/region_shortnames.tf index 50e0df421b..7f1df48c9d 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/region_shortnames.tf +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/region_shortnames.tf @@ -14,7 +14,7 @@ * limitations under the License. */ -# borrowed from FAST networking stages +# adapted from FAST networking stages locals { # used when the first character would not work @@ -26,7 +26,7 @@ locals { } # split in [geo, cardinal, number] tokens _region_tokens = { - for v in local.lb_regions : v => regexall("(?:[a-z]+)|(?:[0-9]+)", v) + for v in local.regions : v => regexall("(?:[a-z]+)|(?:[0-9]+)", v) } region_shortnames = { for k, v in local._region_tokens : k => join("", [ diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/variables.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/variables.tf index 9d2a4470c3..156a6e60be 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/variables.tf +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/variables.tf @@ -28,7 +28,6 @@ variable "dns_config" { variable "instances_config" { description = "Configuration for instances." type = object({ - count = optional(number, 1) machine_type = optional(string, "e2-micro") zones = optional(list(string), ["b"]) }) @@ -68,27 +67,25 @@ variable "regions" { } variable "vpc_config" { - description = "VPC configuration for load balancer and instances." + description = "VPC configuration for load balancer and instances. Subnets are keyed by region." type = object({ - load_balancer_subnets = map(string) - network = string - instance_subnets = optional(map(string)) + network = string + subnets = map(string) + subnets_instances = optional(map(string)) + firewall_config = optional(object({ + proxy_subnet_ranges = list(string) + client_allowed_ranges = optional(list(string)) + enable_health_check = optional(bool, true) + enable_iap_ssh = optional(bool, false) + })) }) nullable = false validation { condition = ( - toset([ - for s in var.vpc_config.load_balancer_subnets : - regex("/regions/([^/]+)/", s) - ]) - == - toset([ - for s in coalesce( - var.vpc_config.instance_subnets, - var.vpc_config.load_balancer_subnets - ) : regex("/regions/([^/]+)/", s) - ]) + var.vpc_config.subnets_instances == null + || + keys(var.vpc_config.subnets) == keys(var.vpc_config.subnets_instances) ) - error_message = "Instance subnet regions must match load balancer regions." + error_message = "Instance subnet regions must match load balancer regions if defined." } } From 00bd4ee046fa0016f594da9726f25bd840281a28 Mon Sep 17 00:00:00 2001 From: Ludo Date: Sat, 13 Apr 2024 13:50:13 +0200 Subject: [PATCH 07/15] add optional proxy subnet creation, outputs, fixes --- .../README.md | 24 +++++++++++++++--- .../instances.tf | 2 ++ .../main.tf | 19 ++++++++++++++ .../outputs.tf | 23 +++++++++++++++++ .../region_shortnames.tf | 2 ++ .../variables.tf | 25 ++++++++----------- 6 files changed, 77 insertions(+), 18 deletions(-) diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md index b352ef6b2c..319f0d50d2 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md @@ -26,15 +26,33 @@ description: Terraform-managed proxy-only subnet for Regional HTTPS or Internal ## Variable configuration ## Testing + + +## Files + +| name | description | modules | +|---|---|---| +| [instances.tf](./instances.tf) | Instance-related locals and resources. | compute-vm · iam-service-account | +| [main.tf](./main.tf) | Load balancer and VPC resources. | dns · net-lb-app-int-cross-region · net-vpc · net-vpc-firewall | +| [outputs.tf](./outputs.tf) | Module outputs. | | +| [region_shortnames.tf](./region_shortnames.tf) | Region shortnames via locals. | | +| [variables.tf](./variables.tf) | Module variables. | | + ## Variables | name | description | type | required | default | |---|---|:---:|:---:|:---:| | [project_id](variables.tf#L49) | Project used to create resources. | string | ✓ | | -| [vpc_config](variables.tf#L69) | VPC configuration for load balancer and instances. Subnets are keyed by region. | object({…}) | ✓ | | +| [vpc_config](variables.tf#L55) | VPC configuration for load balancer and instances. Subnets are keyed by region. | object({…}) | ✓ | | | [dns_config](variables.tf#L17) | DNS configuration. | object({…}) | | {} | | [instances_config](variables.tf#L28) | Configuration for instances. | object({…}) | | {} | -| [prefix](variables.tf#L42) | Prefix used for or in resource names. | string | | "lb-xr-00" | -| [regions](variables.tf#L55) | Regions used for the compute resources, use shortnames as keys. | map(string) | | {…} | +| [prefix](variables.tf#L42) | Prefix used for resource names. | string | | "lb-xr-00" | + +## Outputs + +| name | description | sensitive | +|---|---|:---:| +| [instances](outputs.tf#L17) | Instances details. | | +| [lb](outputs.tf#L34) | Load balancer details. | | diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/instances.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/instances.tf index b98cd9bc8e..734d1f9f54 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/instances.tf +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/instances.tf @@ -14,6 +14,8 @@ * limitations under the License. */ +# tfdoc:file:description Instance-related locals and resources. + locals { # use lb subnets for instances if instance subnets are not defined subnets_instances = coalesce( diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/main.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/main.tf index 5033ec4b7a..2ae6f22668 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/main.tf +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/main.tf @@ -14,11 +14,30 @@ * limitations under the License. */ +# tfdoc:file:description Load balancer and VPC resources. + locals { # define regions for both instances and lb for easy access regions = keys(var.vpc_config.subnets) } +module "vpc" { + source = "../../net-vpc" + count = var.vpc_config.proxy_subnets_config == null ? 0 : 1 + project_id = var.project_id + name = regex("global/networks/([^/]+$)", var.vpc_config.network) + vpc_create = false + subnets_proxy_only = [ + for k, v in var.vpc_config.proxy_subnets_config : { + ip_cidr_range = v + name = "{var.prefix}-proxy-${local.region_shortnames[k]}" + region = k + active = true + global = true + } + ] +} + module "load-balancer" { source = "../../net-lb-app-int-cross-region" name = var.prefix diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/outputs.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/outputs.tf index 35828ec07e..f1bdb8cb13 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/outputs.tf +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/outputs.tf @@ -14,3 +14,26 @@ * limitations under the License. */ +output "instances" { + description = "Instances details." + value = { + addresses = { + for k, v in module.instances : k => v.internal_ip + } + commands = { + for k, v in module.instances : k => ( + "gssh ${nonsensitive(v.instance.name)} --project ${var.project_id} --zone ${nonsensitive(v.instance.zone)}" + ) + } + groups = { + for k, v in module.instances : k => v.group.id + } + } +} + +output "lb" { + description = "Load balancer details." + value = { + addresses = module.load-balancer.addresses[k] + } +} diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/region_shortnames.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/region_shortnames.tf index 7f1df48c9d..0d7e4ad112 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/region_shortnames.tf +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/region_shortnames.tf @@ -14,6 +14,8 @@ * limitations under the License. */ +# tfdoc:file:description Region shortnames via locals. + # adapted from FAST networking stages locals { diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/variables.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/variables.tf index 156a6e60be..1b967c2966 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/variables.tf +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/variables.tf @@ -40,7 +40,7 @@ variable "instances_config" { } variable "prefix" { - description = "Prefix used for or in resource names." + description = "Prefix used for resource names." type = string nullable = false default = "lb-xr-00" @@ -52,20 +52,6 @@ variable "project_id" { nullable = false } -variable "regions" { - description = "Regions used for the compute resources, use shortnames as keys." - type = map(string) - nullable = false - default = { - ew1 = "europe-west1" - ew3 = "europe-west3" - } - validation { - condition = length(var.regions) >= 2 - error_message = "At least two regions are required." - } -} - variable "vpc_config" { description = "VPC configuration for load balancer and instances. Subnets are keyed by region." type = object({ @@ -78,6 +64,7 @@ variable "vpc_config" { enable_health_check = optional(bool, true) enable_iap_ssh = optional(bool, false) })) + proxy_subnets_config = optional(map(string)) }) nullable = false validation { @@ -88,4 +75,12 @@ variable "vpc_config" { ) error_message = "Instance subnet regions must match load balancer regions if defined." } + validation { + condition = ( + var.vpc_config.proxy_subnets_config == null + || + keys(var.vpc_config.subnets) == keys(var.vpc_config.proxy_subnets_config) + ) + error_message = "Proxy subnet regions must match load balancer regions if defined." + } } From 72cc87fe037c62256b67fcc8efe87e876bad6095 Mon Sep 17 00:00:00 2001 From: Ludo Date: Sat, 13 Apr 2024 14:48:14 +0200 Subject: [PATCH 08/15] tested --- .../README.md | 131 ++++++++++++++++-- .../instances.tf | 2 +- .../main.tf | 9 +- .../outputs.tf | 2 +- .../variables.tf | 21 ++- 5 files changed, 147 insertions(+), 18 deletions(-) diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md index 319f0d50d2..e9162e765f 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md @@ -6,27 +6,138 @@ This recipe shows an actual usage scenario for the [cross-region internal applic Scenario diagram.

+ +- [Prerequisites](#prerequisites) + - [Proxy-only global subnets](#proxy-only-global-subnets) + - [Firewall rules](#firewall-rules) +- [Variable configuration](#variable-configuration) + - [VPC configuration options](#vpc-configuration-options) + - [Instance configuration options](#instance-configuration-options) + - [DNS configuration](#dns-configuration) +- [Testing](#testing) +- [Files](#files) +- [Variables](#variables) +- [Outputs](#outputs) + + ## Prerequisites -### Proxy-only global subnets +To run this recipe, you need -The load balancer needs one proxy-only global subnet in each of its regions. If you are using the [`net-vpc`](../../net-vpc/) module with the subnet factory enabled, this is an example on how to create one of the subnets. +- an existing GCP project with the `compute` API enabled +- the `roles/compute.admin` role or equivalent (e.g. `roles/editor`) assigned on the project +- an existing VPC in the same project +- one regular subnet per region where you want to deploy the load balancer in the same VPC +- an organization policy configuration that allows creation of internal application load balancer (the default configuration is fine) +- access to the Docker Registry from the instances -```yaml -# data/subnets/proxy-global-ew1.yaml -region: europe-west1 -ip_cidr_range: 172.16.192.0/24 -proxy_only: true -global: true -description: Terraform-managed proxy-only subnet for Regional HTTPS or Internal HTTPS LB. -``` +### Proxy-only global subnets + +The load balancer needs one proxy-only global subnet in each of its regions. If the subnets already exist the load balancer will consume them. If you need to create them, either do that manually or configure the module to do it for you as explained in the [Variable configuration](#variable-configuration) section below. ### Firewall rules +For the load balancer to work you need to allow ingress to the instances from the health check ranges, and from the load balancer proxy ranges. You can configure firewall rules manually or configure the module to do it for you as explained in the [Variable configuration](#variable-configuration) section below. + ## Variable configuration +With all the requirements in place, the only variables that are needed are those that configure the project and VPC details. Note that you need to use ids or self link in the VPC configuration, not names which also allows supporting Shared VPC. + +This is a simple minimal configuration: + +```tfvars +project_id = "my-project" +vpc_config = { + network = "projects/my-project/global/networks/test" + subnets = { + europe-west1 = "projects/my-project/regions/europe-west1/subnetworks/default" + europe-west8 = "projects/my-project/regions/europe-west8/subnetworks/default", + } +} +``` + +### VPC configuration options + +The VPC configuration also allows creating instances in different subnets, and auto-creation of proxy subnets and firewall rules. This is a complete configuration with all options. + +```tfvars +project_id = "my-project" +vpc_config = { + network = "projects/my-project/global/networks/test" + subnets = { + europe-west1 = "projects/my-project/regions/europe-west1/subnetworks/default" + europe-west8 = "projects/my-project/regions/europe-west8/subnetworks/default", + } + # only specify this to use different subnets for instances + subnets_instances = { + europe-west1 = "projects/my-project/regions/europe-west1/subnetworks/vms" + europe-west8 = "projects/my-project/regions/europe-west8/subnetworks/vms", + } + # create proxy subnets + proxy_subnets_config = { + europe-west1 = "172.16.193.0/24" + europe-west8 = "172.16.192.0/24" + } + # create firewall rules + firewall_config = { + proxy_subnet_ranges = [ + "172.16.193.0/24", + "172.16.192.0/24" + ] + enable_health_check = true + enable_iap_ssh = true + } +} +``` + +### Instance configuration options + +Instance type and the number of zones can be configure via the `instances_config` variable: + +```hcl +project_id = "my-project" +vpc_config = { + network = "projects/my-project/global/networks/test" + subnets = { + europe-west1 = "projects/my-project/regions/europe-west1/subnetworks/default" + europe-west8 = "projects/my-project/regions/europe-west8/subnetworks/default", + } + instances_config = { + # both attributes are optional + machine_type = "e2-small" + zones = ["b", "c"] + } +} +``` + +### DNS configuration + +The DNS zone used for the load balancer record can be configured via the `dns_config` variable: + +```hcl +project_id = "my-project" +vpc_config = { + network = "projects/my-project/global/networks/test" + subnets = { + europe-west1 = "projects/my-project/regions/europe-west1/subnetworks/default" + europe-west8 = "projects/my-project/regions/europe-west8/subnetworks/default", + } + dns_config = { + # all attributes are optional + client_networks = [ + "projects/my-project/global/networks/test", + "projects/my-other-project/global/networks/test" + ] + domain = "foo.example." + hostname = "lb-test + } +} +``` + ## Testing +To test the load balancer behaviour, you can simply disable the service on the backend instances by connecting via SSH and issuing the `sudo systemctl stop nginx` command. + ## Files diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/instances.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/instances.tf index 734d1f9f54..2118ef12c4 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/instances.tf +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/instances.tf @@ -26,7 +26,7 @@ locals { for t in setproduct(local.regions, var.instances_config.zones) : "${var.prefix}-${local.region_shortnames[t.0]}-${t.1}" => { region = t.0 - zone = t.1 + zone = "${t.0}-${t.1}" } } } diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/main.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/main.tf index 2ae6f22668..237eb606bc 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/main.tf +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/main.tf @@ -24,13 +24,13 @@ locals { module "vpc" { source = "../../net-vpc" count = var.vpc_config.proxy_subnets_config == null ? 0 : 1 - project_id = var.project_id - name = regex("global/networks/([^/]+$)", var.vpc_config.network) + project_id = regex("projects/([^/]+)/", var.vpc_config.network)[0] + name = regex("global/networks/([^/]+)$", var.vpc_config.network)[0] vpc_create = false subnets_proxy_only = [ for k, v in var.vpc_config.proxy_subnets_config : { ip_cidr_range = v - name = "{var.prefix}-proxy-${local.region_shortnames[k]}" + name = "${var.prefix}-proxy-${local.region_shortnames[k]}" region = k active = true global = true @@ -54,6 +54,7 @@ module "load-balancer" { network = var.vpc_config.network subnetworks = var.vpc_config.subnets } + depends_on = [module.vpc] } module "dns" { @@ -73,7 +74,7 @@ module "dns" { recordsets = { "A ${coalesce(var.dns_config.hostname, var.prefix)}" = { geo_routing = [ - for k, v in local.regions : { + for k in local.regions : { location = k health_checked_targets = [ { diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/outputs.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/outputs.tf index f1bdb8cb13..f0f70880a8 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/outputs.tf +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/outputs.tf @@ -34,6 +34,6 @@ output "instances" { output "lb" { description = "Load balancer details." value = { - addresses = module.load-balancer.addresses[k] + addresses = module.load-balancer.addresses } } diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/variables.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/variables.tf index 1b967c2966..e9cef8d68b 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/variables.tf +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/variables.tf @@ -67,11 +67,21 @@ variable "vpc_config" { proxy_subnets_config = optional(map(string)) }) nullable = false + validation { + condition = try(regex("/", var.vpc_config.network), null) != null + error_message = "Network must be a network id or self link, not a name." + } + validation { + condition = alltrue([ + for k, v in var.vpc_config.subnets : try(regex("/", v), null) != null + ]) + error_message = "Subnet values must be ids or self links, not names." + } validation { condition = ( var.vpc_config.subnets_instances == null || - keys(var.vpc_config.subnets) == keys(var.vpc_config.subnets_instances) + keys(var.vpc_config.subnets) == keys(coalesce(var.vpc_config.subnets_instances, {})) ) error_message = "Instance subnet regions must match load balancer regions if defined." } @@ -79,8 +89,15 @@ variable "vpc_config" { condition = ( var.vpc_config.proxy_subnets_config == null || - keys(var.vpc_config.subnets) == keys(var.vpc_config.proxy_subnets_config) + keys(var.vpc_config.subnets) == keys(coalesce(var.vpc_config.proxy_subnets_config, {})) ) error_message = "Proxy subnet regions must match load balancer regions if defined." } + validation { + condition = alltrue([ + for k, v in coalesce(var.vpc_config.subnets_instances, {}) : + try(regex("/", v), null) != null + ]) + error_message = "Instance subnet values must be ids or self links, not names." + } } From 6d458e0ac057c227929e751d43824960c928bd72 Mon Sep 17 00:00:00 2001 From: Ludo Date: Sat, 13 Apr 2024 14:53:24 +0200 Subject: [PATCH 09/15] tfdoc fix --- tools/tfdoc.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/tfdoc.py b/tools/tfdoc.py index 1b0536d438..e240f05b3f 100755 --- a/tools/tfdoc.py +++ b/tools/tfdoc.py @@ -192,8 +192,9 @@ def create_tfref(module_path, files=False, show_extra=False, exclude_files=None, abspath = os.path.abspath(module_path) try: if os.path.dirname(abspath).endswith('/modules'): - mod_recipes = parse_recipes( - module_path, f'{REPO_URL}/modules/{os.path.basename(abspath)}') + mod_recipes = list( + parse_recipes(module_path, + f'{REPO_URL}/modules/{os.path.basename(abspath)}')) else: mod_recipes = None mod_files = list(parse_files(module_path, exclude_files)) if files else [] From 24c542143abc17a37d2995ab85f0bb1c21c9dfd0 Mon Sep 17 00:00:00 2001 From: Ludo Date: Sat, 13 Apr 2024 15:07:12 +0200 Subject: [PATCH 10/15] fix README --- modules/net-lb-app-int/README.md | 4 ---- tools/tfdoc.py | 6 ++++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/modules/net-lb-app-int/README.md b/modules/net-lb-app-int/README.md index 81da5131b9..6d4ebda0f0 100644 --- a/modules/net-lb-app-int/README.md +++ b/modules/net-lb-app-int/README.md @@ -21,7 +21,6 @@ Due to the complexity of the underlying resources, changes to the configuration - [SSL Certificates](#ssl-certificates) - [PSC service attachment](#psc-service-attachment) - [Complex example](#complex-example) -- [Recipes](#recipes) - [Files](#files) - [Variables](#variables) - [Outputs](#outputs) @@ -672,9 +671,6 @@ module "ilb-l7" { -## Recipes - - ## Files | name | description | resources | diff --git a/tools/tfdoc.py b/tools/tfdoc.py index e240f05b3f..742260b12b 100755 --- a/tools/tfdoc.py +++ b/tools/tfdoc.py @@ -111,7 +111,9 @@ VAR_RE_TYPE = re.compile(r'([\(\{\}\)])') VAR_TEMPLATE = ('default', 'description', 'type', 'nullable') -Document = collections.namedtuple('Document', 'content files variables outputs') +Document = collections.namedtuple('Document', + 'content files variables outputs recipes', + defaults=[None]) File = collections.namedtuple('File', 'name description modules resources') Output = collections.namedtuple( 'Output', 'name description sensitive consumers file line') @@ -205,7 +207,7 @@ def create_tfref(module_path, files=False, show_extra=False, exclude_files=None, raise SystemExit(e) doc = format_tfref(mod_outputs, mod_variables, mod_files, mod_fixtures, mod_recipes, show_extra) - return Document(doc, mod_files, mod_variables, mod_outputs) + return Document(doc, mod_files, mod_variables, mod_outputs, mod_recipes) def format_tfref(outputs, variables, files, fixtures, recipes=None, From f7b0842089e9434d4a8d81301225f877de8d455e Mon Sep 17 00:00:00 2001 From: Ludo Date: Sat, 13 Apr 2024 15:10:25 +0200 Subject: [PATCH 11/15] exclude examples from test collector --- .../recipe-cross-reg-int-app-lb-vm-dns/README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md index e9162e765f..16c199e05e 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md @@ -54,6 +54,7 @@ vpc_config = { europe-west8 = "projects/my-project/regions/europe-west8/subnetworks/default", } } +# tftest skip ``` ### VPC configuration options @@ -88,6 +89,7 @@ vpc_config = { enable_iap_ssh = true } } +# tftest skip ``` ### Instance configuration options @@ -108,6 +110,7 @@ vpc_config = { zones = ["b", "c"] } } +# tftest skip ``` ### DNS configuration @@ -132,6 +135,7 @@ vpc_config = { hostname = "lb-test } } +# tftest skip ``` ## Testing From 7e3f30fdeaa405a149be6c0a408aa72fa6b746ec Mon Sep 17 00:00:00 2001 From: Ludo Date: Sat, 13 Apr 2024 19:43:56 +0200 Subject: [PATCH 12/15] README typos --- .../recipe-cross-reg-int-app-lb-vm-dns/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md index 16c199e05e..c2bb86f914 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md @@ -37,11 +37,11 @@ The load balancer needs one proxy-only global subnet in each of its regions. If ### Firewall rules -For the load balancer to work you need to allow ingress to the instances from the health check ranges, and from the load balancer proxy ranges. You can configure firewall rules manually or configure the module to do it for you as explained in the [Variable configuration](#variable-configuration) section below. +For the load balancer to work you need to allow ingress to the instances from the health check ranges, and from the load balancer proxy ranges. You can create firewall rules manually or configure the module to do it for you as explained in the [Variable configuration](#variable-configuration) section below. ## Variable configuration -With all the requirements in place, the only variables that are needed are those that configure the project and VPC details. Note that you need to use ids or self link in the VPC configuration, not names which also allows supporting Shared VPC. +With all the requirements in place, the only variables that are needed are those that configure the project and VPC details. Note that you need to use ids or self links in the VPC configuration not names (Shared VPC configurations are also supported). This is a simple minimal configuration: From daf9dae9c2d558a8c90ed7cf8d1394f212d4be4d Mon Sep 17 00:00:00 2001 From: Ludo Date: Sat, 13 Apr 2024 19:47:20 +0200 Subject: [PATCH 13/15] README fixes --- .../recipe-cross-reg-int-app-lb-vm-dns/README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md index c2bb86f914..5da6cd48be 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md @@ -29,7 +29,7 @@ To run this recipe, you need - an existing VPC in the same project - one regular subnet per region where you want to deploy the load balancer in the same VPC - an organization policy configuration that allows creation of internal application load balancer (the default configuration is fine) -- access to the Docker Registry from the instances +- access to the Docker Registry from the instances (e.g. via Cloud NAT) ### Proxy-only global subnets @@ -94,7 +94,7 @@ vpc_config = { ### Instance configuration options -Instance type and the number of zones can be configure via the `instances_config` variable: +The instance type and the number of zones can be configured via the `instances_config` variable: ```hcl project_id = "my-project" @@ -142,6 +142,8 @@ vpc_config = { To test the load balancer behaviour, you can simply disable the service on the backend instances by connecting via SSH and issuing the `sudo systemctl stop nginx` command. +If the backends are unhealthy and the necessary firewall rules are in place, check that the Docker containers have started successfully on the instances by connecting via SSH and issuing the `sudo systemctl status nginx` command. + ## Files From d3ebfe1e711b4ac58deb2e4618cbcffd2819e29b Mon Sep 17 00:00:00 2001 From: Ludo Date: Sun, 14 Apr 2024 10:37:13 +0200 Subject: [PATCH 14/15] add test and fix cloud init path --- .../README.md | 17 +++++++++++++++++ .../instances.tf | 2 +- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md index 5da6cd48be..391f95d308 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md @@ -173,3 +173,20 @@ If the backends are unhealthy and the necessary firewall rules are in place, che | [instances](outputs.tf#L17) | Instances details. | | | [lb](outputs.tf#L34) | Load balancer details. | | + +## Tests + +```hcl +module "recipe" { + source = "./fabric/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns" + project_id = "my-project" + vpc_config = { + network = "projects/my-project/global/networks/test" + subnets = { + europe-west1 = "projects/my-project/regions/europe-west1/subnetworks/default" + europe-west8 = "projects/my-project/regions/europe-west8/subnetworks/default", + } + } +} +# tftest modules=6 resources=15 +``` diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/instances.tf b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/instances.tf index 2118ef12c4..df1f794798 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/instances.tf +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/instances.tf @@ -63,7 +63,7 @@ module "instances" { }] tags = [var.prefix] metadata = { - user-data = file("nginx-cloud-config.yaml") + user-data = file("${path.module}/nginx-cloud-config.yaml") } service_account = { email = module.instance-sa.email From c916d510bddb0e237be7031ba2f01347889b81a1 Mon Sep 17 00:00:00 2001 From: Julio Castillo Date: Sun, 14 Apr 2024 14:30:13 +0200 Subject: [PATCH 15/15] Add support for tests to run from tfvars files too --- .../README.md | 29 ++----- tests/examples/conftest.py | 12 ++- tests/examples/test_plan.py | 84 +++++++++++-------- tests/fixtures.py | 46 ++++------ 4 files changed, 78 insertions(+), 93 deletions(-) diff --git a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md index 391f95d308..1bdd5a9a63 100644 --- a/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md +++ b/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns/README.md @@ -54,7 +54,7 @@ vpc_config = { europe-west8 = "projects/my-project/regions/europe-west8/subnetworks/default", } } -# tftest skip +# tftest modules=5 resources=15 ``` ### VPC configuration options @@ -96,7 +96,7 @@ vpc_config = { The instance type and the number of zones can be configured via the `instances_config` variable: -```hcl +```tfvars project_id = "my-project" vpc_config = { network = "projects/my-project/global/networks/test" @@ -110,14 +110,14 @@ vpc_config = { zones = ["b", "c"] } } -# tftest skip +# tftest modules=5 resources=15 ``` ### DNS configuration The DNS zone used for the load balancer record can be configured via the `dns_config` variable: -```hcl +```tfvars project_id = "my-project" vpc_config = { network = "projects/my-project/global/networks/test" @@ -132,10 +132,10 @@ vpc_config = { "projects/my-other-project/global/networks/test" ] domain = "foo.example." - hostname = "lb-test + hostname = "lb-test" } } -# tftest skip +# tftest modules=5 resources=15 ``` ## Testing @@ -173,20 +173,3 @@ If the backends are unhealthy and the necessary firewall rules are in place, che | [instances](outputs.tf#L17) | Instances details. | | | [lb](outputs.tf#L34) | Load balancer details. | | - -## Tests - -```hcl -module "recipe" { - source = "./fabric/modules/net-lb-app-int-cross-region/recipe-cross-reg-int-app-lb-vm-dns" - project_id = "my-project" - vpc_config = { - network = "projects/my-project/global/networks/test" - subnets = { - europe-west1 = "projects/my-project/regions/europe-west1/subnetworks/default" - europe-west8 = "projects/my-project/regions/europe-west8/subnetworks/default", - } - } -} -# tftest modules=6 resources=15 -``` diff --git a/tests/examples/conftest.py b/tests/examples/conftest.py index bc6834249a..5833f9c6a9 100644 --- a/tests/examples/conftest.py +++ b/tests/examples/conftest.py @@ -1,4 +1,4 @@ -# Copyright 2023 Google LLC +# Copyright 2024 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -25,7 +25,8 @@ FILE_TEST_RE = re.compile(r'# tftest-file +id=([\w_.-]+) +path=([\S]+)') FIXTURE_TEST_RE = re.compile(r'# tftest-fixture +id=([\w_.-]+)') -Example = collections.namedtuple('Example', 'name code module files fixtures') +Example = collections.namedtuple('Example', + 'name code module files fixtures type') File = collections.namedtuple('File', 'path content') @@ -74,9 +75,11 @@ def pytest_generate_tests(metafunc, test_group='example', index += 1 code = child.children[0].children tftest_tag = get_tftest_directive(code) + if tftest_tag is None: + continue if tftest_tag and not filter_tests(tftest_tag): continue - if child.lang == 'hcl': + if child.lang in ('hcl', 'tfvars'): path = module.relative_to(FABRIC_ROOT) name = f'{path}:{last_header}' if index > 1: @@ -88,7 +91,8 @@ def pytest_generate_tests(metafunc, test_group='example', # see: https://pytest-xdist.readthedocs.io/en/latest/distribution.html marks = [pytest.mark.xdist_group("serial") ] if 'serial' in tftest_tag else [] - example = Example(name, code, path, files[last_header], fixtures) + example = Example(name, code, path, files[last_header], fixtures, + child.lang) examples.append(pytest.param(example, marks=marks)) elif isinstance(child, marko.block.Heading): last_header = child.children[0].children diff --git a/tests/examples/test_plan.py b/tests/examples/test_plan.py index 345d155094..f618d7c549 100644 --- a/tests/examples/test_plan.py +++ b/tests/examples/test_plan.py @@ -1,4 +1,4 @@ -# Copyright 2023 Google LLC +# Copyright 2024 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -15,6 +15,8 @@ import re import subprocess import yaml +import shutil +import tempfile from pathlib import Path BASE_PATH = Path(__file__).parent @@ -47,49 +49,59 @@ def prepare_files(example, test_path, files, fixtures): destination.write_text(example.fixtures[f]) -def test_example(plan_validator, tmp_path, example): +def test_example(plan_validator, example): if match := COUNT_TEST_RE.search(example.code): - (tmp_path / 'fabric').symlink_to(BASE_PATH.parents[1]) - (tmp_path / 'variables.tf').symlink_to(BASE_PATH / 'variables.tf') - (tmp_path / 'main.tf').write_text(example.code) - assets_path = BASE_PATH.parent / str(example.module).replace('-', - '_') / 'assets' - if assets_path.exists(): - (tmp_path / 'assets').symlink_to(assets_path) + # for tfvars-based tests, create the temporary directory with the + # same parent as the original module + directory = example.module.parent if example.type == 'tfvars' else None + prefix = f'pytest-{example.module.name}' + with tempfile.TemporaryDirectory(prefix=prefix, dir=directory) as tmp_path: + tmp_path = Path(tmp_path) + tf_var_files = [] + if example.type == 'hcl': + (tmp_path / 'fabric').symlink_to(BASE_PATH.parents[1]) + (tmp_path / 'variables.tf').symlink_to(BASE_PATH / 'variables.tf') + (tmp_path / 'main.tf').write_text(example.code) + assets_path = (BASE_PATH.parent / + str(example.module).replace('-', '_') / 'assets') + if assets_path.exists(): + (tmp_path / 'assets').symlink_to(assets_path) - expected_modules = int(match.group('modules')) - expected_resources = int(match.group('resources')) + prepare_files(example, tmp_path, match.group('files'), + match.group('fixtures')) + elif example.type == 'tfvars': + (tmp_path / 'terraform.auto.tfvars').write_text(example.code) + shutil.copytree(example.module, tmp_path, dirs_exist_ok=True) + tf_var_files = [(tmp_path / 'terraform.auto.tfvars').resolve()] - prepare_files(example, tmp_path, match.group('files'), - match.group('fixtures')) + expected_modules = int(match.group('modules')) + expected_resources = int(match.group('resources')) - inventory = [] - if match.group('inventory') is not None: - python_test_path = str(example.module).replace('-', '_') - inventory = BASE_PATH.parent / python_test_path / 'examples' - inventory = inventory / match.group('inventory') + inventory = [] + if match.group('inventory') is not None: + python_test_path = str(example.module).replace('-', '_') + inventory = BASE_PATH.parent / python_test_path / 'examples' + inventory = inventory / match.group('inventory') - # TODO: force plan_validator to never copy files (we're already - # running from a temp dir) - summary = plan_validator(module_path=tmp_path, inventory_paths=inventory, - tf_var_files=[]) + summary = plan_validator(module_path=tmp_path, inventory_paths=inventory, + tf_var_files=tf_var_files) - print('\n') - print(yaml.dump({'values': summary.values})) - print(yaml.dump({'counts': summary.counts})) - print(yaml.dump({'outputs': summary.outputs})) + print('\n') + print(yaml.dump({'values': summary.values})) + print(yaml.dump({'counts': summary.counts})) + print(yaml.dump({'outputs': summary.outputs})) - counts = summary.counts - num_modules, num_resources = counts['modules'], counts['resources'] - assert expected_modules == num_modules, 'wrong number of modules' - assert expected_resources == num_resources, 'wrong number of resources' + counts = summary.counts + num_modules, num_resources = counts['modules'], counts['resources'] + assert expected_modules == num_modules, 'wrong number of modules' + assert expected_resources == num_resources, 'wrong number of resources' - # TODO(jccb): this should probably be done in check_documentation - # but we already have all the data here. - result = subprocess.run( - 'terraform fmt -check -diff -no-color main.tf'.split(), cwd=tmp_path, - stdout=subprocess.PIPE, encoding='utf-8') - assert result.returncode == 0, f'terraform code not formatted correctly\n{result.stdout}' + # TODO(jccb): this should probably be done in check_documentation + # but we already have all the data here. + result = subprocess.run( + 'terraform fmt -check -diff -no-color main.tf'.split(), cwd=tmp_path, + stdout=subprocess.PIPE, encoding='utf-8') + assert result.returncode == 0, f'terraform code not formatted correctly\n{result.stdout}' else: assert False, "can't find tftest directive" diff --git a/tests/fixtures.py b/tests/fixtures.py index 93b853c3f0..50b6d8ad6a 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -1,4 +1,4 @@ -# Copyright 2023 Google LLC +# Copyright 2024 Google LLC # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -34,10 +34,9 @@ def _prepare_root_module(path): """Context manager to prepare a terraform module to be tested. - If the TFTEST_COPY environment variable is set, `path` is copied to - a temporary directory and a few terraform files (e.g. - terraform.tfvars) are deleted to ensure a clean test environment. - Otherwise, `path` is simply returned untouched. + `path` is copied to a temporary directory and a few terraform files + (e.g. terraform.tfvars) are deleted to ensure a clean test + environment. """ # if we're copying the module, we might as well ignore files and # directories that are automatically read by terraform. Useful @@ -50,31 +49,18 @@ def _prepare_root_module(path): '.terraform.lock.hcl', 'terraform.tfvars', '.terraform') - if os.environ.get('TFTEST_COPY'): - # if the TFTEST_COPY is set, create temp dir and copy the root - # module there - with tempfile.TemporaryDirectory(dir=path.parent) as tmp_path: - tmp_path = Path(tmp_path) - - # Running tests in a copy made with symlinks=True makes them run - # ~20% slower than when run in a copy made with symlinks=False. - shutil.copytree(path, tmp_path, dirs_exist_ok=True, symlinks=False, - ignore=ignore_patterns) - lockfile = _REPO_ROOT / 'tools' / 'lockfile' / '.terraform.lock.hcl' - if lockfile.exists(): - shutil.copy(lockfile, tmp_path / '.terraform.lock.hcl') - - yield tmp_path - else: - # check if any ignore_patterns files are present in path - if unwanted_files := ignore_patterns(path, os.listdir(path=path)): - # prevent shooting yourself in the foot (unexpected test results) when ignored files are present - raise RuntimeError( - f'Test in path {path} contains {", ".join(unwanted_files)} which may affect ' - f'test results. Please run tests with TFTEST_COPY=1 environment variable' - ) - # if TFTEST_COPY is not set, just return the same path - yield path + with tempfile.TemporaryDirectory(dir=path.parent) as tmp_path: + tmp_path = Path(tmp_path) + + # Running tests in a copy made with symlinks=True makes them run + # ~20% slower than when run in a copy made with symlinks=False. + shutil.copytree(path, tmp_path, dirs_exist_ok=True, symlinks=False, + ignore=ignore_patterns) + lockfile = _REPO_ROOT / 'tools' / 'lockfile' / '.terraform.lock.hcl' + if lockfile.exists(): + shutil.copy(lockfile, tmp_path / '.terraform.lock.hcl') + + yield tmp_path def plan_summary(module_path, basedir, tf_var_files=None, extra_files=None,