Skip to content

Commit

Permalink
Firestore: Fix tests
Browse files Browse the repository at this point in the history
There's a whole bunch of different constraints that have to be met
for these tests to pass consistently.

* Backup schedules need to be run in a project with billing enabled,
  and the database needs a deletion policy to ensure it is properly swept.
* The default-database examples were marked as skip-test.
  They're valuable as examples but not really as tests - there's nothing
  Terraform-specific about them worth testing - and the Field tests
  will fail if default database creation doesn't work in Terraform
* The Document tests need to run against the (default) database due
  to an existing bug
  (hashicorp/terraform-provider-google#15550).
  So they will now create their own project and their own (default)
  database.
* The basic Field example is also creating its own project and its
  own database now for test isolation purposes. It might be sufficient
  to just create a database the same way as for backup schedules;
  we can evaluate that once the tests have been passing for a while.
* The other two Field examples are following the same pattern as
  the backup schedule examples.
* The Firestore-native index example is using the inband project
  creation method, because then we can use the (default) database,
  which means we can use the Document resource (see the above-mentioned bug),
  which means that we don't need to use a manually-created project (because creating
  a Document implicitly creates its collection).
* The Datastore mode Index example can't take that same approach, because it doesn't
  seem like the Document resource works on datastore-mode databases. So we will need
  to use the manually-created FIRESTORE_PROJECT_NAME project for that one, and it will
  need to have a Datastore mode database manually baked into it.

 This exercise has identified a fair number of places for improvement to the Firestore
 Terraform story, but as a stopgap to get the tests passing again, this will have to do.
  • Loading branch information
rwhogg committed Jan 18, 2024
1 parent dab48df commit 7714b66
Show file tree
Hide file tree
Showing 21 changed files with 341 additions and 92 deletions.
22 changes: 15 additions & 7 deletions mmv1/products/firestore/BackupSchedule.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,24 @@ docs: !ruby/object:Provider::Terraform::Docs
examples:
- !ruby/object:Provider::Terraform::Examples
name: 'firestore_backup_schedule_daily'
primary_resource_id:
'daily-backup'
primary_resource_id: 'daily-backup'
vars:
database_id: 'database-id'
delete_protection_state: "DELETE_PROTECTION_ENABLED"
test_env_vars:
project_id: :FIRESTORE_PROJECT_NAME
project_id: :PROJECT_NAME
test_vars_overrides:
delete_protection_state: '"DELETE_PROTECTION_DISABLED"'
- !ruby/object:Provider::Terraform::Examples
name: 'firestore_backup_schedule_weekly'
primary_resource_id:
'weekly-backup'
primary_resource_id: 'weekly-backup'
vars:
database_id: 'database-id'
delete_protection_state: "DELETE_PROTECTION_ENABLED"
test_env_vars:
project_id: :FIRESTORE_PROJECT_NAME
project_id: :PROJECT_NAME
test_vars_overrides:
delete_protection_state: '"DELETE_PROTECTION_DISABLED"'
parameters:
- !ruby/object:Api::Type::String
name: database
Expand All @@ -64,7 +72,7 @@ properties:
output: true
description: |
The unique backup schedule identifier across all locations and databases for the given project. Format:
`projects/{{project}}/databases/{{database}}/backupSchedules/{{backupSchedule}}
`projects/{{project}}/databases/{{database}}/backupSchedules/{{backupSchedule}}`
immutable: true
custom_flatten: templates/terraform/custom_flatten/name_from_self_link.erb
- !ruby/object:Api::Type::String
Expand Down
14 changes: 3 additions & 11 deletions mmv1/products/firestore/Database.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -57,22 +57,18 @@ examples:
- !ruby/object:Provider::Terraform::Examples
name: 'firestore_default_database'
primary_resource_id: 'database'
pull_external: true
vars:
delete_protection_state: "DELETE_PROTECTION_ENABLED"
test_env_vars:
project_id: :PROJECT_NAME
test_vars_overrides:
delete_protection_state: '"DELETE_PROTECTION_DISABLED"'
ignore_read_extra:
- project
- etag
- deletion_policy
skip_test: true
- !ruby/object:Provider::Terraform::Examples
name: 'firestore_database'
primary_resource_id: 'database'
vars:
name: "example-database-id"
database_id: "database-id"
delete_protection_state: "DELETE_PROTECTION_ENABLED"
test_env_vars:
project_id: :PROJECT_NAME
Expand All @@ -85,12 +81,8 @@ examples:
- !ruby/object:Provider::Terraform::Examples
name: 'firestore_default_database_in_datastore_mode'
primary_resource_id: 'datastore_mode_database'
vars:
delete_protection_state: "DELETE_PROTECTION_ENABLED"
test_env_vars:
project_id: :PROJECT_NAME
test_vars_overrides:
delete_protection_state: '"DELETE_PROTECTION_DISABLED"'
ignore_read_extra:
- project
- etag
Expand All @@ -100,7 +92,7 @@ examples:
name: 'firestore_database_in_datastore_mode'
primary_resource_id: 'datastore_mode_database'
vars:
name: "example-database-id"
database_id: "database-id"
delete_protection_state: "DELETE_PROTECTION_ENABLED"
test_env_vars:
project_id: :PROJECT_NAME
Expand Down
8 changes: 6 additions & 2 deletions mmv1/products/firestore/Document.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,20 @@ examples:
name: 'firestore_document_basic'
primary_resource_id: 'mydoc'
test_env_vars:
project_id: :FIRESTORE_PROJECT_NAME
org_id: :ORG_ID
vars:
document_id: 'my-doc-id'
project_id: 'project-id'
pull_external: true
- !ruby/object:Provider::Terraform::Examples
name: 'firestore_document_nested_document'
primary_resource_id: 'mydoc'
test_env_vars:
project_id: :FIRESTORE_PROJECT_NAME
org_id: :ORG_ID
vars:
document_id: 'my-doc-id'
project_id: 'project-id'
pull_external: true
custom_code: !ruby/object:Provider::Terraform::CustomCode
custom_import: templates/terraform/custom_import/firestore_document.go.erb
decoder: templates/terraform/decoders/firestore_document.go.erb
Expand Down
21 changes: 18 additions & 3 deletions mmv1/products/firestore/Field.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,33 @@ examples:
- !ruby/object:Provider::Terraform::Examples
name: 'firestore_field_basic'
primary_resource_id: 'basic'
vars:
project_id: "project-id"
test_env_vars:
project_id: :FIRESTORE_PROJECT_NAME
org_id: :ORG_ID
pull_external: true
- !ruby/object:Provider::Terraform::Examples
name: 'firestore_field_timestamp'
primary_resource_id: 'timestamp'
vars:
database_id: 'database-id'
delete_protection_state: "DELETE_PROTECTION_ENABLED"
test_env_vars:
project_id: :FIRESTORE_PROJECT_NAME
project_id: :PROJECT_NAME
test_vars_overrides:
delete_protection_state: '"DELETE_PROTECTION_DISABLED"'
pull_external: true
- !ruby/object:Provider::Terraform::Examples
name: 'firestore_field_match_override'
primary_resource_id: 'match_override'
vars:
database_id: 'database-id'
delete_protection_state: "DELETE_PROTECTION_ENABLED"
test_env_vars:
project_id: :FIRESTORE_PROJECT_NAME
project_id: :PROJECT_NAME
test_vars_overrides:
delete_protection_state: '"DELETE_PROTECTION_DISABLED"'
pull_external: true
custom_code: !ruby/object:Provider::Terraform::CustomCode
custom_import: templates/terraform/custom_import/index_self_link_as_name_set_project.go.erb
encoder: templates/terraform/encoders/firestore_field.go.erb
Expand Down
14 changes: 7 additions & 7 deletions mmv1/products/firestore/Index.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,19 @@ examples:
name: 'firestore_index_basic'
primary_resource_id:
'my-index'
# This example relies on a well-known collection having been created and
# Firestore being enabled on the project. We can't do that automatically
# so we need a pre-created project.
vars:
project_id: "project-id"
test_env_vars:
project_id: :FIRESTORE_PROJECT_NAME
org_id: :ORG_ID
pull_external: true
- !ruby/object:Provider::Terraform::Examples
name: 'firestore_index_datastore_mode'
primary_resource_id:
'my-datastore-mode-index'
'my-index'
test_env_vars:
# This example relies on a well-known collection having been created and
# Firestore being enabled on the project. We can't do that automatically
# so we need a pre-created project.
test_env_vars:
# for Datastore mode so we need a pre-created project.
project_id: :FIRESTORE_PROJECT_NAME
custom_code: !ruby/object:Provider::Terraform::CustomCode
custom_import: templates/terraform/custom_import/index_self_link_as_name_set_project.go.erb
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
resource "google_firestore_database" "database" {
project = "<%= ctx[:test_env_vars]['project_id'] %>"
name = "<%= ctx[:vars]['database_id'] %>"
location_id = "nam5"
type = "FIRESTORE_NATIVE"

delete_protection_state = "<%= ctx[:vars]['delete_protection_state'] %>"
deletion_policy = "DELETE"
}

resource "google_firestore_backup_schedule" "<%= ctx[:primary_resource_id] %>" {
project = "<%= ctx[:test_env_vars]['project_id'] %>"
project = "<%= ctx[:test_env_vars]['project_id'] %>"
database = google_firestore_database.database.name

retention = "604800s" // 7 days (maximum possible value for daily backups)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,16 @@
resource "google_firestore_database" "database" {
project = "<%= ctx[:test_env_vars]['project_id'] %>"
name = "<%= ctx[:vars]['database_id'] %>"
location_id = "nam5"
type = "FIRESTORE_NATIVE"

delete_protection_state = "<%= ctx[:vars]['delete_protection_state'] %>"
deletion_policy = "DELETE"
}

resource "google_firestore_backup_schedule" "<%= ctx[:primary_resource_id] %>" {
project = "<%= ctx[:test_env_vars]['project_id'] %>"
database = "(default)"
database = google_firestore_database.database.name

retention = "8467200s" // 14 weeks (maximum possible value for weekly backups)

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
resource "google_firestore_database" "<%= ctx[:primary_resource_id] %>" {
project = "<%= ctx[:test_env_vars]['project_id'] %>"
name = "<%= ctx[:vars]['name']%>"
name = "<%= ctx[:vars]['database_id']%>"
location_id = "nam5"
type = "FIRESTORE_NATIVE"
concurrency_mode = "OPTIMISTIC"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
resource "google_firestore_database" "<%= ctx[:primary_resource_id] %>" {
project = "<%= ctx[:test_env_vars]['project_id'] %>"
name = "<%= ctx[:vars]['name']%>"
name = "<%= ctx[:vars]['database_id']%>"
location_id = "nam5"
type = "DATASTORE_MODE"
concurrency_mode = "OPTIMISTIC"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
resource "google_firestore_database" "<%= ctx[:primary_resource_id] %>" {
project = "<%= ctx[:test_env_vars]['project_id'] %>"
name = "(default)"
location_id = "nam5"
type = "FIRESTORE_NATIVE"
delete_protection_state = "<%= ctx[:vars]['delete_protection_state'] %>"
deletion_policy = "DELETE"
project = "<%= ctx[:test_env_vars]['project_id'] %>"
name = "(default)"
location_id = "nam5"
type = "FIRESTORE_NATIVE"
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
resource "google_firestore_database" "<%= ctx[:primary_resource_id] %>" {
project = "<%= ctx[:test_env_vars]['project_id'] %>"
name = "(default)"
location_id = "nam5"
type = "DATASTORE_MODE"
delete_protection_state = "<%= ctx[:vars]['delete_protection_state'] %>"
deletion_policy = "DELETE"
project = "<%= ctx[:test_env_vars]['project_id'] %>"
name = "(default)"
location_id = "nam5"
type = "DATASTORE_MODE"
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,35 @@
resource "google_project" "project" {
project_id = "<%= ctx[:vars]['project_id'] %>"
name = "<%= ctx[:vars]['project_id'] %>"
org_id = "<%= ctx[:test_env_vars]['org_id'] %>"
}

resource "time_sleep" "wait_60_seconds" {
depends_on = [google_project.project]

create_duration = "60s"
}

resource "google_project_service" "firestore" {
project = google_project.project.project_id
service = "firestore.googleapis.com"

# Needed for CI tests for permissions to propagate, should not be needed for actual usage
depends_on = [time_sleep.wait_60_seconds]
}

resource "google_firestore_database" "database" {
project = google_project.project.project_id
name = "(default)"
location_id = "nam5"
type = "FIRESTORE_NATIVE"

depends_on = [google_project_service.firestore]
}

resource "google_firestore_document" "<%= ctx[:primary_resource_id] %>" {
project = "<%= ctx[:test_env_vars]['project_id'] %>"
project = google_project.project.project_id
database = google_firestore_database.database.name
collection = "somenewcollection"
document_id = "<%= ctx[:vars]['document_id'] %>"
fields = "{\"something\":{\"mapValue\":{\"fields\":{\"akey\":{\"stringValue\":\"avalue\"}}}}}"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,51 @@
resource "google_project" "project" {
project_id = "<%= ctx[:vars]['project_id'] %>"
name = "<%= ctx[:vars]['project_id'] %>"
org_id = "<%= ctx[:test_env_vars]['org_id'] %>"
}

resource "time_sleep" "wait_60_seconds" {
depends_on = [google_project.project]

create_duration = "60s"
}

resource "google_project_service" "firestore" {
project = google_project.project.project_id
service = "firestore.googleapis.com"

# Needed for CI tests for permissions to propagate, should not be needed for actual usage
depends_on = [time_sleep.wait_60_seconds]
}

resource "google_firestore_database" "database" {
project = google_project.project.project_id
name = "(default)"
location_id = "nam5"
type = "FIRESTORE_NATIVE"

depends_on = [google_project_service.firestore]
}

resource "google_firestore_document" "<%= ctx[:primary_resource_id] %>" {
project = "<%= ctx[:test_env_vars]['project_id'] %>"
project = google_project.project.project_id
database = google_firestore_database.database.name
collection = "somenewcollection"
document_id = "<%= ctx[:vars]['document_id'] %>"
fields = "{\"something\":{\"mapValue\":{\"fields\":{\"akey\":{\"stringValue\":\"avalue\"}}}}}"
}

resource "google_firestore_document" "sub_document" {
project = "<%= ctx[:test_env_vars]['project_id'] %>"
project = google_project.project.project_id
database = google_firestore_database.database.name
collection = "${google_firestore_document.<%= ctx[:primary_resource_id] %>.path}/subdocs"
document_id = "bitcoinkey"
fields = "{\"something\":{\"mapValue\":{\"fields\":{\"ayo\":{\"stringValue\":\"val2\"}}}}}"
}

resource "google_firestore_document" "sub_sub_document" {
project = "<%= ctx[:test_env_vars]['project_id'] %>"
project = google_project.project.project_id
database = google_firestore_database.database.name
collection = "${google_firestore_document.sub_document.path}/subsubdocs"
document_id = "asecret"
fields = "{\"something\":{\"mapValue\":{\"fields\":{\"secret\":{\"stringValue\":\"hithere\"}}}}}"
Expand Down
35 changes: 32 additions & 3 deletions mmv1/templates/terraform/examples/firestore_field_basic.tf.erb
Original file line number Diff line number Diff line change
@@ -1,8 +1,37 @@
resource "google_project" "project" {
project_id = "<%= ctx[:vars]['project_id'] %>"
name = "<%= ctx[:vars]['project_id'] %>"
org_id = "<%= ctx[:test_env_vars]['org_id'] %>"
}

resource "time_sleep" "wait_60_seconds" {
depends_on = [google_project.project]

create_duration = "60s"
}

resource "google_project_service" "firestore" {
project = google_project.project.project_id
service = "firestore.googleapis.com"

# Needed for CI tests for permissions to propagate, should not be needed for actual usage
depends_on = [time_sleep.wait_60_seconds]
}

resource "google_firestore_database" "database" {
project = google_project.project.project_id
name = "(default)"
location_id = "nam5"
type = "FIRESTORE_NATIVE"

depends_on = [google_project_service.firestore]
}

resource "google_firestore_field" "<%= ctx[:primary_resource_id] %>" {
project = "<%= ctx[:test_env_vars]['project_id'] %>"
database = "(default)"
project = google_project.project.project_id
database = google_firestore_database.database.name
collection = "chatrooms_%{random_suffix}"
field = "basic"
field = "basic"

index_config {
indexes {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
resource "google_firestore_field" "<%= ctx[:primary_resource_id] %>" {
project = "<%= ctx[:test_env_vars]['project_id'] %>"
project = "<%= ctx[:test_env_vars]['project_id'] %>"
collection = "chatrooms_%{random_suffix}"
field = "`*`"
field = "`*`"

index_config {
indexes {
Expand Down
Loading

0 comments on commit 7714b66

Please sign in to comment.