forseti-security / terraform-google-forseti

A Terraform module for installing Forseti on GCP
Apache License 2.0
132 stars 127 forks source link

Update Module to Support GKE #169

Closed kevensen closed 4 years ago

kevensen commented 5 years ago

In order to support deploying forseti-security on GKE, this module should deploy a GKE cluster. It should have the following features.

  1. Deploy forseti-security via Helm
  2. If the user already has a GKE cluster they want to use, don't deploy a new GKE cluster, and don't deploy a server VM
  3. If the user doesn't have a GKE cluster, deploy a GKE cluster with hardening best practices, and don't deploy a server VM
  4. If the user doesn't want to deploy on GKE, deploy the server VM

    • [x] Terra-form sub-module - For Helm to function on GKE (BETA)
    • [x] Terra-form changes for optional deployment of Server VM (GA)
    • [ ] Terra-form changes for optional deployment of Client VM (GA)
morgante commented 5 years ago

If we pursue this, it should be done as a submodule.

However, I'm not quite sure what value does having Terraform orchestrate the creation provide over simply deploying the Helm chart?

kevensen commented 5 years ago

After further consideration, it may be enough to provide an alternative "main.tf". Saying if you want to deploy this on gke, use the GKE CFT module. Then add the Helm Release resource to your main.tf

Still, if the GKE route is taken, it would be nice to not deploy the Server VM.

RexBelli commented 5 years ago

I've been talking with @kevensen, I'd like to take this on. If someone can assign this issue to me, that'd be great.

kevensen commented 5 years ago

@morgante Terraform would create the GKE cluster. The Helm chart will deploy forseti-security on GKE.

morgante commented 5 years ago

Definitely, if you're deploying to GKE you don't need a server VM. I can see the value in an alternative submodule which doesn't include that but does include the Helm Release.

kevensen commented 5 years ago

Gotcha. I need to understand submodules better.

aaron-lane commented 5 years ago

@kevensen They're also referred to as "nested modules" in the Standard Module Structure article. Their behaviour is completely arbitrary, but they're generally used provide composable units of related resources or alternative implementations of the root module.

kevensen commented 5 years ago

@aaron-lane Thanks for the info! I'll look into it.

kevensen commented 5 years ago

The approach we've chosen is to place the necessary module invocations in a single main.tf. Please let me know if there better practice.

If we were to place it in a nested module, would we create a PR to add the nested module to terraform-google-forseti? Or would we expect it to be incumbent on the user to create a local sub-module in their Terraform work space?.

morgante commented 5 years ago

You could create a PR to provide a nested module, we certainly wouldn't require users to create a local sub-module.

kevensen commented 5 years ago

@morgante I am still learning Terraform best practices so I appreciate your guidance! @RexBelli, do you think we could refactor the work you've done to be a submodule?

RexBelli commented 5 years ago

@kevensen I originally attempted to do this and ran into an issue: I could not get the GKE cluster to be conditional.

My approach was to use the count parameter on any resource that needed to be conditional. I found Terraform modules do not support this parameter, so I then moved to adding it to individual resources. In this context the big ones would be the GCE VMs in the server submodule, and the GKE cluster in the new gke submodule I was building. This seemed like it was going to work until I ran into this: I could not get the count parameter to work on the google_container_cluster resource.

I don't know enough about the inner workings of Terraform to say whether the issue is a bug or user error.

kevensen commented 4 years ago

Addressed by #284