From 145e66be9c7f022558c9feb69c0648acae673ed4 Mon Sep 17 00:00:00 2001
From: Danny Seymour <seymourd@google.com>
Date: Fri, 22 Feb 2019 10:34:24 -0800
Subject: [PATCH] Move App Engine components into its own submodule to avoid
 duplicating module interfaces

---
 CHANGELOG.md                              |  9 +-------
 docs/upgrading_to_project_factory_v2.0.md | 15 +++++++-------
 main.tf                                   |  5 -----
 modules/app_engine/main.tf                |  5 +----
 modules/app_engine/variables.tf           |  5 -----
 modules/core_project_factory/main.tf      | 12 -----------
 test/fixtures/full/main.tf                | 10 +++++----
 variables.tf                              | 25 -----------------------
 8 files changed, 15 insertions(+), 71 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8436f32a5..9a3ab1994 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,14 +11,7 @@ Extending the adopted spec, each change should have a link to its corresponding
 
 ### ADDED
 
-- Added separate App Engine module. [#144]
-
-### REMOVED
-
-- Removed `app_engine` argument (config block).
-
-## [1.1.0] - 2019-02-22
-### ADDED
+- Added separate App Engine module. [#134]
 - Preconditions script checks billing account format. [#117]
 - Add project_services submodule. [#133]
 
diff --git a/docs/upgrading_to_project_factory_v2.0.md b/docs/upgrading_to_project_factory_v2.0.md
index 31a721107..5a43b3ea5 100644
--- a/docs/upgrading_to_project_factory_v2.0.md
+++ b/docs/upgrading_to_project_factory_v2.0.md
@@ -30,18 +30,17 @@ module "project-factory" {
 }
 ```
 
-The new version of project factory uses granular fields prefixed by `app_engine_`. There is also an additional `app_engine_enabled` argument that needs to be set to true.
+The new version of project factory uses a new module named `app_engine`. It accepts
 
 ```hcl
 /// @file main.tf
 
-module "project-factory" {
-  ...
-  app_engine_enabled     = true
-  app_engine_location_id = "${var.region}"
-  app_engine_auth_domain = "${var.domain}"
+module "app-engine" {
+  project     = "${var.project_id}
+  location_id = "${var.region}"
+  auth_domain = "${var.domain}"
 
-  app_engine_feature_settings = [
+  feature_settings = [
     {
       split_health_checks = true
     },
@@ -54,7 +53,7 @@ module "project-factory" {
 The new implementation uses the `google_app_engine_application` resource which needs to be imported into the current state (make sure to replace `$YOUR_PROJECT_ID`):
 
 ```sh
-terraform import module.project-factory.module.project-factory.module.app-engine.google_app_engine_application.app $YOUR_PROJECT_ID
+terraform import module.app-engine.google_app_engine_application.app $YOUR_PROJECT_ID
 ```
 
 After importing, run `terraform` `plan` and `apply`.
diff --git a/main.tf b/main.tf
index f8046e44c..1acbe9f60 100755
--- a/main.tf
+++ b/main.tf
@@ -49,9 +49,4 @@ module "project-factory" {
   bucket_name                 = "${var.bucket_name}"
   auto_create_network         = "${var.auto_create_network}"
   disable_services_on_destroy = "${var.disable_services_on_destroy}"
-  app_engine_enabled          = "${var.app_engine_enabled}"
-  app_engine_location_id      = "${var.app_engine_location_id}"
-  app_engine_auth_domain      = "${var.app_engine_auth_domain}"
-  app_engine_serving_status   = "${var.app_engine_serving_status}"
-  app_engine_feature_settings = "${var.app_engine_feature_settings}"
 }
diff --git a/modules/app_engine/main.tf b/modules/app_engine/main.tf
index 7c63e5907..338c018be 100644
--- a/modules/app_engine/main.tf
+++ b/modules/app_engine/main.tf
@@ -15,10 +15,7 @@
  */
 
 resource "google_app_engine_application" "app" {
-  count = "${var.enabled ? 1 : 0}"
-
-  project = "${var.project_id}"
-
+  project          = "${var.project_id}"
   location_id      = "${var.location_id}"
   auth_domain      = "${var.auth_domain}"
   serving_status   = "${var.serving_status}"
diff --git a/modules/app_engine/variables.tf b/modules/app_engine/variables.tf
index 2ee1bf8c7..16d1dd667 100644
--- a/modules/app_engine/variables.tf
+++ b/modules/app_engine/variables.tf
@@ -14,11 +14,6 @@
  * limitations under the License.
  */
 
-variable "enabled" {
-  description = "Enable App Engine."
-  default     = true
-}
-
 variable "project_id" {
   description = "The project to enable app engine on."
 }
diff --git a/modules/core_project_factory/main.tf b/modules/core_project_factory/main.tf
index 9cf9ad08c..61586fb64 100644
--- a/modules/core_project_factory/main.tf
+++ b/modules/core_project_factory/main.tf
@@ -87,18 +87,6 @@ resource "google_project" "main" {
   depends_on = ["null_resource.preconditions"]
 }
 
-module "app-engine" {
-  source = "../app_engine"
-
-  enabled = "${var.app_engine_enabled}"
-
-  project_id       = "${google_project.main.project_id}"
-  location_id      = "${var.app_engine_location_id}"
-  auth_domain      = "${var.app_engine_auth_domain}"
-  serving_status   = "${var.app_engine_serving_status}"
-  feature_settings = "${var.app_engine_feature_settings}"
-}
-
 /******************************************
   Project lien
  *****************************************/
diff --git a/test/fixtures/full/main.tf b/test/fixtures/full/main.tf
index a138f8bcc..a75da073a 100644
--- a/test/fixtures/full/main.tf
+++ b/test/fixtures/full/main.tf
@@ -101,12 +101,14 @@ module "project-factory" {
   ]
 
   disable_services_on_destroy = "false"
+}
 
-  app_engine_enabled     = true
-  app_engine_location_id = "${var.region}"
-  app_engine_auth_domain = "${var.domain}"
+module "app-engine" {
+  project     = "${module.project-factory.project_id}"
+  location_id = "${var.region}"
+  auth_domain = "${var.domain}"
 
-  app_engine_feature_settings = [
+  feature_settings = [
     {
       split_health_checks = true
     },
diff --git a/variables.tf b/variables.tf
index a7d7018da..704380a18 100755
--- a/variables.tf
+++ b/variables.tf
@@ -119,28 +119,3 @@ variable "disable_services_on_destroy" {
   default     = "true"
   type        = "string"
 }
-
-variable "app_engine_enabled" {
-  description = "Enable App Engine on the project."
-  default     = false
-}
-
-variable "app_engine_location_id" {
-  description = "The location to serve the app from."
-  default     = ""
-}
-
-variable "app_engine_auth_domain" {
-  description = "The domain to authenticate users with when using App Engine's User API."
-  default     = ""
-}
-
-variable "app_engine_serving_status" {
-  description = "The serving status of the App Engine application."
-  default     = "SERVING"
-}
-
-variable "app_engine_feature_settings" {
-  description = "A block of optional settings to configure specific App Engine features."
-  default     = []
-}