Documentation and example clound function for shared vpcs. #2674
Conversation
|
/gcbrun |
| - group: primary | ||
| modules: | ||
| - source: modules/network/pre-existing-subnetwork | ||
| kind: terraform |
There was a problem hiding this comment.
Delete kind: terraform. Default kind is terraform.
| subnetwork_self_link: https://compute.googleapis.com/compute/v1/projects/{project}/regions/{region}/subnetworks/{subnetwork} | ||
| name = name-of-subnet (optional - not used when subnet_self_link is defined) | ||
| region = name-of-region (optional - not used when subnet_self_link is defined) | ||
| project = name-of-project (optional - not used when subnet_self_link is defined) |
There was a problem hiding this comment.
I suggest the following alternative format for L18-20 as it represents the valid yaml syntax and also being commented out will prevent issues when people copy paste this block.
# name: name-of-subnet (optional - not used when subnet_self_link is defined)
# region: name-of-region (optional - not used when subnet_self_link is defined)
# project: name-of-project (optional - not used when subnet_self_link is defined)
|
|
||
| The module is located in `modules/network/pre-existing-subnetwork`. | ||
|
|
||
| The extension is build to support subnet level permissions. |
There was a problem hiding this comment.
I recommend instead:
The
pre-existing-subnetworkmodule was created to support subnet level permissions.
There was a problem hiding this comment.
Given the title of this readme, I think there should be some reference to the other shared vpc example.
Maybe start by saying
"Here is an example of using shared vpc with the pre-existing-vpc module. If service projects only have permissions to access subnets then you can use the pre-existing-subnetwork module instead."
If instead you think the pre-existing-subnetwork module should always be used for shared VPC then maybe we should consider updating that example.
|
|
||
| If subnetwork_self_link is provided then name,region,project is ignored. | ||
|
|
||
| Since using the HPC toolkit creates a new service account for the cluster, the cluster service accounts needs roles/compute.networkUser on the subnet on shared VPC. |
There was a problem hiding this comment.
This is only true when using the service account module in a blueprint.
I would instead say:
When using the HPC toolkit creates a new service account
There was a problem hiding this comment.
If I understand correctly this terraform would essentially be a pre-step that would set up the shared vpc and cloud function to give appropriate permissions before deploying a cluster in a service project using a blueprint.
This submission would be the first of its kind. The HPC Toolkit repo does not host terraform that is not Toolkit compatible. It seems to me the way this terraform is written is meant to be a self contained deployment (aka no inputs/outputs and not modular). While the example is informative, I believe it strays from the purpose of the Toolkit to build composable modules.
Several ideas on a path forward.
- Modularize the code and create a blueprint that could be used with the Toolkit. This would be a non trival amount of work but would reuse existing modules and produce new modules that could be reused in future.
- network/vpc module - exists
- project/iam-custom-role module - needs development
- project/service-account module - exists
- pubsub/topic module - exists but expand to include log sink functionality
- file-system/cloud-storage-bucket module - exists
- cloud-function/serviceaccount-audit-logs-watcher module - needs development
Possible alternative breakdown could be:
- network/vpc module - exists
- project/service-account module - exists
- file-system/cloud-storage-bucket module - exists
- cloud-function/serviceaccount-audit-logs-watcher - new module that takes in bucket, vpc, and service account and contains rest of functionality.
- Host this example in some other repo and refer to it from the shared vpc documentation in this PR.
- Maybe there is a deeper level we could place this code that is more appropriate such as /community/examples/shared-vpc-setup-helper/
I will confer with the team and decide what options are reasonable.
Documentation and example clound function for shared vpcs.