Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add database_type to google_app_engine_application #6598

Closed
wants to merge 1 commit into from
Closed

Add database_type to google_app_engine_application #6598

wants to merge 1 commit into from

Conversation

salrashid123
Copy link
Contributor

fixes #3657

Adds the database_type enum as a parameter that allows creation of firestore backend

provider "google" {}

resource "google_app_engine_application" "app" {
  project = "mineral-minutia-820"
  location_id = "us-central"
  database_type = "DATABASE_TYPE_UNSPECIFIED"
}

ref: https://cloud.google.com/appengine/docs/admin-api/reference/rest/v1beta/apps#DatabaseType


do not merge yet: i had to bump the google api dependency version

@ghost ghost added the size/xs label Jun 15, 2020
@ghost ghost requested a review from c2thorn June 15, 2020 20:10
@ghost ghost added the documentation label Jun 15, 2020
@c2thorn
Copy link
Collaborator

c2thorn commented Jun 15, 2020

Hi @salrashid123, thanks for doing this. The changes looks fine, however they will need to be upstreamed to https://github.com/GoogleCloudPlatform/magic-modules which generates changes for this provider and google-beta.
Essentially, this PR can be reduced to bumping the api dependency version and modifying the upstream files (resource, test, web) will do the rest. Also the beta provider will need the api version to be bumped as well.

If you don't have time to do this, I should be able to get to it early tomorrow.

@salrashid123
Copy link
Contributor Author

hi @c2thorn

please do merge upstream when you can.
(i have a pretty remarkable trackrecord of goofing PRs at the last min. (squash+merge, etc)

@@ -102,6 +102,7 @@ resource "google_app_engine_application" "acceptance" {
auth_domain = "hashicorptest.com"
location_id = "us-central"
serving_status = "SERVING"
database_type = "DATABASE_TYPE_UNSPECIFIED"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can DATABASE_TYPE_UNSPECIFIED actually be set in the API? This test is failing because the API returns CLOUD_DATASTORE_COMPATIBILITY and causes a diff.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried setting to CLOUD_DATASTORE but got An internal error occurred while provisioning Datastore.. Were you able to try out this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think the GAE Admin api isn't returning the "already exists" error or something.

for example, i can mutate the databaseType after i set it and it still returns a valid response. It should return some error,


I tested it several times, by using the DATABASE_TYPE_UNSPECIFIED (which does use CLOUD_DATASTORE_COMPATIBLITY on its own unilaterally on the serverside (i.,e it looks to be the default unspecified type)

  • dstest1-280522

On a clean project, start off with DATABASE_TYPE_UNSPECIFIED

resource "google_app_engine_application" "app" {
  project = "dstest1-280522"
  location_id = "us-central"
  database_type = "DATABASE_TYPE_UNSPECIFIED"
}
$ terraform apply

# google_app_engine_application.app will be created
+ resource "google_app_engine_application" "app" {
    + app_id            = (known after apply)
    + auth_domain       = (known after apply)
    + code_bucket       = (known after apply)
    + database_type     = "DATABASE_TYPE_UNSPECIFIED"
    + default_bucket    = (known after apply)
    + default_hostname  = (known after apply)
    + gcr_domain        = (known after apply)
    + id                = (known after apply)
    + location_id       = "us-central"
    + name              = (known after apply)
    + project           = "dstest1-280522"
    + serving_status    = (known after apply)
    + url_dispatch_rule = (known after apply)

    + feature_settings {
        + split_health_checks = (known after apply)
      }
  }

the storage type is changed from
DATABASE_TYPE_UNSPECIFIED --> CLOUD_DATASTORE_COMPATIBILITY

$ gcloud app describe --project dstest1-280522 --format="value(databaseType)"
CLOUD_DATASTORE_COMPATIBILITY

then try change to firestore

resource "google_app_engine_application" "app" {
  project = "dstest1-280522"
  location_id = "us-central"
  database_type = "CLOUD_FIRESTORE"
}
# google_app_engine_application.app will be updated in-place
~ resource "google_app_engine_application" "app" {
      app_id            = "dstest1-280522"
      auth_domain       = "gmail.com"
      code_bucket       = "staging.dstest1-280522.appspot.com"
    ~ database_type     = "CLOUD_DATASTORE_COMPATIBILITY" -> "CLOUD_FIRESTORE"
      default_bucket    = "dstest1-280522.appspot.com"
      default_hostname  = "dstest1-280522.uc.r.appspot.com"
      gcr_domain        = "us.gcr.io"
      id                = "dstest1-280522"
      location_id       = "us-central"
      name              = "apps/dstest1-280522"
      project           = "dstest1-280522"
      serving_status    = "SERVING"
      url_dispatch_rule = []

      feature_settings {
          split_health_checks = true
      }
  }

No errors

but its still CLOUD_DATASTORE_COMPATIBILITY

  $ gcloud app describe --project dstest1-280522 --format="value(databaseType)"
  CLOUD_DATASTORE_COMPATIBILITY

Try again on a clean project

  • dstest2-280523
$ gcloud app describe --project dstest2-280523  --format="value(databaseType)"
ERROR: (gcloud.app.describe) The current Google Cloud project [dstest2-280523] does not contain an App Engine application. Use `gcloud app create` to initialize an App Engine application within the project.

First set to CLOUD_FIRESTORE

# google_app_engine_application.app will be created
+ resource "google_app_engine_application" "app" {
    + app_id            = (known after apply)
    + auth_domain       = (known after apply)
    + code_bucket       = (known after apply)
    + database_type     = "CLOUD_FIRESTORE"
    + default_bucket    = (known after apply)
    + default_hostname  = (known after apply)
    + gcr_domain        = (known after apply)
    + id                = (known after apply)
    + location_id       = "us-central"
    + name              = (known after apply)
    + project           = "dstest2-280523"
    + serving_status    = (known after apply)
    + url_dispatch_rule = (known after apply)

    + feature_settings {
        + split_health_checks = (known after apply)
      }
  }

That succeeds:

$ gcloud app describe --project dstest2-280523  --format="value(databaseType)"
CLOUD_FIRESTORE

Try to change to CLOUD_DATASTORE (this should fail)

# google_app_engine_application.app will be updated in-place
~ resource "google_app_engine_application" "app" {
      app_id            = "dstest2-280523"
      auth_domain       = "gmail.com"
      code_bucket       = "staging.dstest2-280523.appspot.com"
    ~ database_type     = "CLOUD_FIRESTORE" -> "CLOUD_DATASTORE"
      default_bucket    = "dstest2-280523.appspot.com"
      default_hostname  = "dstest2-280523.uc.r.appspot.com"
      gcr_domain        = "us.gcr.io"
      id                = "dstest2-280523"
      location_id       = "us-central"
      name              = "apps/dstest2-280523"
      project           = "dstest2-280523"
      serving_status    = "SERVING"
      url_dispatch_rule = []

      feature_settings {
          split_health_checks = true
      }
  }

but the api seems to suceed but its still CLOUD_FIRESTORE

  $ gcloud app describe --project dstest2-280523  --format="value(databaseType)"
  CLOUD_FIRESTORE

Type: schema.TypeString,
Optional: true,
ValidateFunc: validation.StringInSlice([]string{
"DATABASE_TYPE_UNSPECIFIED",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment below, we can probably remove this value.

@c2thorn
Copy link
Collaborator

c2thorn commented Jun 17, 2020

After internal discussion, closing this in favor of GoogleCloudPlatform/magic-modules#3646

@c2thorn c2thorn closed this Jun 17, 2020
@ghost
Copy link

ghost commented Jul 18, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@hashicorp hashicorp locked and limited conversation to collaborators Jul 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow configuring Firestore on a project with Terraform
2 participants