Conversation
4463589 to
312b0c5
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the Terraform cluster and node-pool modules to use stronger-typed map(object(...)) configurations (moving away from comma-separated strings), and updates quickstarts/tests accordingly. It also removes the KinD starter/module support and its related CI/image build paths.
Changes:
- Migrate cluster/node-pool module inputs from
map(map(string))+ comma-separated strings tomap(object(...))+list(string)/set(string)where appropriate. - Update quickstart and test configurations to use list values (e.g., AZs, instance types, node locations).
- Remove KinD starter/module artifacts and exclude KinD from CI matrices and OCI image variants.
Reviewed changes
Copilot reviewed 68 out of 68 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/gke_zero_cluster.tf | Update cluster_node_locations to a list. |
| tests/eks_zero_node_pools.tf | Update instance_types / availability_zones to lists. |
| tests/eks_zero_cluster.tf | Update EKS instance types / AZs to list forms. |
| quickstart/src/configurations/multi-cloud/gke_zero_cluster.tf | Switch example cluster_node_locations to list; remove legacy comments/locals. |
| quickstart/src/configurations/multi-cloud/eks_zero_cluster.tf | Switch instance/AZ inputs to list forms; adjust defaults. |
| quickstart/src/configurations/multi-cloud/aks_zero_cluster.tf | Remove unused loc env stub. |
| quickstart/src/configurations/kind/versions.tf | Removed KinD starter Terraform versions file. |
| quickstart/src/configurations/kind/kind_zero_providers.tf | Removed KinD starter provider wiring. |
| quickstart/src/configurations/kind/kind_zero_cluster.tf | Removed KinD starter cluster module usage. |
| quickstart/src/configurations/kind/README.md | Removed KinD starter README pointer. |
| quickstart/src/configurations/kind/Dockerfile | Removed KinD starter Dockerfile template. |
| quickstart/src/configurations/kind/.user | Removed KinD starter user template link. |
| quickstart/src/configurations/kind/.gitignore | Removed KinD starter gitignore template link. |
| quickstart/src/configurations/gke/gke_zero_cluster.tf | Switch example cluster_node_locations to list; remove loc env. |
| quickstart/src/configurations/eks/eks_zero_cluster.tf | Switch EKS instance/AZ inputs to list; remove loc env. |
| quickstart/src/configurations/aks/aks_zero_cluster.tf | Update commented network params to list examples; remove loc env. |
| oci/Dockerfile | Remove KinD build stages/variant. |
| kind/cluster/versions.tf | Removed KinD cluster wrapper module file. |
| kind/cluster/variables.tf | Removed KinD cluster wrapper module file. |
| kind/cluster/outputs.tf | Removed KinD cluster wrapper module file. |
| kind/cluster/main.tf | Removed KinD cluster wrapper module file. |
| kind/cluster/configuration.tf | Removed KinD cluster wrapper module file. |
| kind/_modules/kind/versions.tf | Removed KinD implementation module file. |
| kind/_modules/kind/variables.tf | Removed KinD implementation module file. |
| kind/_modules/kind/outputs.tf | Removed KinD implementation module file. |
| kind/_modules/kind/main.tf | Removed KinD implementation module file. |
| google/cluster/variables.tf | Change cluster configuration type to map(object(...)) with list-based fields. |
| google/cluster/node-pool/main.tf | Inline configuration merging and use cfg directly for module inputs. |
| google/cluster/node-pool/configuration.tf | Remove separate configuration locals file (logic moved to main.tf). |
| google/cluster/main.tf | Inline configuration merging and use cfg directly for module inputs. |
| google/cluster/configuration.tf | Remove separate configuration locals file (logic moved to main.tf). |
| google/cluster-local/versions.tf | Removed cluster-local wrapper (no longer supported here). |
| google/cluster-local/variables.tf | Removed cluster-local wrapper (no longer supported here). |
| google/cluster-local/outputs.tf | Removed cluster-local wrapper (no longer supported here). |
| google/cluster-local/main.tf | Removed cluster-local wrapper (no longer supported here). |
| google/cluster-local/configuration.tf | Removed cluster-local wrapper (no longer supported here). |
| google/_modules/gke/variables.tf | Update router advertisement config type (map → list) and minor formatting. |
| google/_modules/gke/network.tf | Update router advertisement for_each and modernize splat syntax. |
| azurerm/cluster/variables.tf | Change AKS cluster configuration type to map(object(...)) with list-based fields. |
| azurerm/cluster/node-pool/variables.tf | Tighten types for node pool counts to numbers. |
| azurerm/cluster/node-pool/main.tf | Inline configuration merging and use cfg directly for module inputs. |
| azurerm/cluster/node-pool/configuration.tf | Remove separate configuration locals file (logic moved to main.tf). |
| azurerm/cluster/main.tf | Inline configuration merging and use cfg directly for module inputs. |
| azurerm/cluster/configuration.tf | Remove separate configuration locals file (logic moved to main.tf). |
| azurerm/cluster-local/versions.tf | Removed cluster-local wrapper (no longer supported here). |
| azurerm/cluster-local/variables.tf | Removed cluster-local wrapper (no longer supported here). |
| azurerm/cluster-local/outputs.tf | Removed cluster-local wrapper (no longer supported here). |
| azurerm/cluster-local/main.tf | Removed cluster-local wrapper (no longer supported here). |
| azurerm/cluster-local/configuration.tf | Removed cluster-local wrapper (no longer supported here). |
| aws/cluster/variables.tf | Change EKS cluster configuration type to map(object(...)) with list-based fields. |
| aws/cluster/node-pool/vpc.tf | Switch AZ handling to list-based config access and modernize index math. |
| aws/cluster/node-pool/variables.tf | Change node pool inputs to list-based fields (instance_types, availability_zones, vpc_subnet_ids). |
| aws/cluster/node-pool/main.tf | Inline configuration merging and use cfg directly for module inputs. |
| aws/cluster/node-pool/data_sources.tf | Update AZ handling to list-based config access. |
| aws/cluster/node-pool/configuration.tf | Remove separate configuration locals file (logic moved to main.tf). |
| aws/cluster/main.tf | Inline configuration merging and use cfg directly for module inputs. |
| aws/cluster/configuration.tf | Remove separate configuration locals file (logic moved to main.tf). |
| aws/cluster-local/versions.tf | Removed cluster-local wrapper (no longer supported here). |
| aws/cluster-local/variables.tf | Removed cluster-local wrapper (no longer supported here). |
| aws/cluster-local/outputs.tf | Removed cluster-local wrapper (no longer supported here). |
| aws/cluster-local/main.tf | Removed cluster-local wrapper (no longer supported here). |
| aws/cluster-local/elb-dns/versions.tf | Removed elb-dns wrapper module file. |
| aws/cluster-local/elb-dns/variables.tf | Removed elb-dns wrapper module file. |
| aws/cluster-local/configuration.tf | Removed cluster-local wrapper (no longer supported here). |
| aws/_modules/eks/node_pool/variables.tf | Change subnet_ids type to set(string). |
| aws/_modules/eks/node_pool.tf | Update splat syntax for subnet IDs. |
| aws/_modules/eks/master.tf | Update splat syntax for subnet IDs. |
| .github/workflows/main.yml | Remove kind starter from build/test/publish matrices; update sed replacements for new list syntax. |
Comments suppressed due to low confidence (1)
google/_modules/gke/network.tf:50
- After changing
router_advertise_config.ip_rangesto a list/set, the dynamicadvertised_ip_rangesblock still treats the iterator like a map (range.key/range.value), and setsdescriptionto the CIDR. Consider iterating a list of strings and settingrange = range.valueanddescription = null(or accept a map again if descriptions are needed).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a57293e to
6e4f6b0
Compare
16b048f to
960111f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 104 out of 104 changed files in this pull request and generated 14 comments.
Comments suppressed due to low confidence (2)
google/_modules/gke/variables.tf:214
- Missing default value: The
enable_gcs_fuse_csi_drivervariable lacks a default value and is not marked as nullable. Since this is an optional feature flag, adddefault = nullordefault = falseto make it optional.
azurerm/_modules/aks/variables.tf:159 - Missing default value: The
kubernetes_versionvariable (lines 156-159) lacks a default value and is not marked as nullable. However, the description says "If not specified, latest version will be used," which implies it should be optional. Adddefault = nullto match the intended behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 105 out of 105 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (1)
google/_modules/gke/network.tf:50
router_advertise_config.ip_rangeshas been changed to aset(string), but this dynamic block still treats the iteration element as a key/value pair (range.key+range.value) and assignsdescription = range.value. For a set,keyandvalueare the same element, so descriptions are lost/misleading. Either keepip_rangesas amap(string)(range => description) or update the block to userange = range.valueand setdescription = null/omit it.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 105 out of 105 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
oci/Dockerfile:250
- This PR removes the KinD image build stages/targets from the main OCI Dockerfile, which changes the published image variants. If this is intended deprecation, please ensure all references (docs, release notes, CI expectations) are updated; otherwise consider keeping the KinD target or splitting this into a separate change from the configuration type migration.
google/_modules/gke/network.tf:51 - The
for_eachconditional mixes list ([]) and set (var.router_advertise_config.ip_ranges), which can trigger an "inconsistent conditional result types" error. Alsoip_rangesis now aset(string)but the block still setsdescription = range.value(no description available with a set). Use a consistent collection type (e.g.,toset([])andtoset(var.router_advertise_config.ip_ranges)), or keepip_rangesas a map if descriptions are required.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 142 out of 151 changed files in this pull request and generated 12 comments.
Comments suppressed due to low confidence (1)
aws/cluster/aws_iam_authenticator.tf:15
- The interpolated
mapRolessnippet is preceded by a leading space (" ${local.aws_auth_map_roles...}"), which will indent the first line of any custom role mappings and can break the YAML structure foraws-auth(role list items must start at column 0). Remove the leading whitespace before the interpolation so inserted- rolearn:lines remain properly aligned.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 142 out of 151 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5f900ab to
20acd79
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 144 out of 153 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 140 out of 149 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* refactor code to not rely on _modules anymore removing one level of module nesting for easier maintainability * provide move blocks for all affected modules and resources * introduce mandatory cluster output on cluster modules and corresponding input on node pool modulesto reduce dependence on data sources causing issues when using the node pool module for the clsuter provisioned default node pool * unify node pool module inputs between providers using new cluster module output * reduce use of locals to improve DX for contributions and maintenance work * added support for merging of nested maps and objects to inheritance module and updated previous string vars to more specific types
No description provided.