Cloud IoT module #4
Conversation
main.tf
Outdated
| @@ -19,14 +19,15 @@ locals { | |||
| } | |||
|
|
|||
| resource "google_pubsub_topic" "topic" { | |||
| count = "${var.topic == "" ? 0 : 1}" | |||
There was a problem hiding this comment.
This is a measure for the IoT side to not be able to call module conditionally.
|
Hi @namusyaka -- thanks for your contribution! It looks like there have been significant changes to this module since you filed this PR; would you mind rebasing your changes on |
|
Sure, will do |
This change is to allow submodule to use pubsub module as optional.
Now the validator behaves as if this were `false`. See: hashicorp/terraform#21408 (comment) However, as a result of my investigation, the validator uses the source module to check whether the syntax is valid and types/values are being used correctly. To react with the changes, this commit adds PWD=directory as an temporary environment variable on the command execution, because validate command seems to refer to the current directory.
This commit also contains a workaround for an issue that terraform-docs unsupport 0.12.0. The workaround avoids the failure of terraform-docs by passing variables.tf and outputs.tf instead of the directory name. ref: terraform-docs/terraform-docs#62
|
@Jberlinsky Updated. PTAL |
|
Seems conflicted again. |
test/fixtures/pubsub/main.tf
Outdated
| project_id = var.project_id | ||
| topic_name = "ci-int-topic-${random_id.random_suffix.hex}" | ||
| topic_name = var.topic_name |
There was a problem hiding this comment.
If the count argument depends on the resource like random_id, the error happens:
The "count" value depends on resource attributes that cannot be determined
There was a problem hiding this comment.
Oh that's a big issue, I will add a note above.
modules/cloudiot/main.tf
Outdated
| event_notification_configs { | ||
| pubsub_topic_name = "projects/${var.project_id}/topics/${module.event_notification_topic.topic}" | ||
| } | ||
| state_notification_config = { |
There was a problem hiding this comment.
I don't think so, this is a map type. IIUC I can't use dynamic for the map type, right?
But I've tried to change these config blocks to be optional: 236d904
There was a problem hiding this comment.
Hmm, can you try dynamic? I think it might work.
There was a problem hiding this comment.
I've already tried it but got an error like: Blocks of type "xxxx" are not expected here.
In fact, i think we're using dynamic only for typeList, i couldn't find the case for typeMap.
If there is any thing I overlooked, let me know.
| default = [] | ||
| } | ||
|
|
||
| variable "event_notification_config" { |
test/fixtures/pubsub/main.tf
Outdated
| project_id = var.project_id | ||
| topic_name = "ci-int-topic-${random_id.random_suffix.hex}" | ||
| topic_name = var.topic_name |
There was a problem hiding this comment.
Oh that's a big issue, I will add a note above.
modules/cloudiot/main.tf
Outdated
| event_notification_configs { | ||
| pubsub_topic_name = "projects/${var.project_id}/topics/${module.event_notification_topic.topic}" | ||
| } | ||
| state_notification_config = { |
There was a problem hiding this comment.
I don't think so, this is a map type. IIUC I can't use dynamic for the map type, right?
But I've tried to change these config blocks to be optional: 236d904
| default = {} | ||
| } | ||
|
|
||
| variable "state_notification_config" { |
modules/cloudiot/main.tf
Outdated
| event_notification_configs { | ||
| pubsub_topic_name = "projects/${var.project_id}/topics/${module.event_notification_topic.topic}" | ||
| } | ||
| state_notification_config = { |
There was a problem hiding this comment.
Hmm, can you try dynamic? I think it might work.
This is a new Cloud IoT module implementation. Just combining existing the pubsub module with
google_cloudiot_registry.In addition, the module has strongly depended on the features introduced in terraform v0.12.
In order to use the new features, I needed to upgrade a few things such as terraform validate and terraform-docs.
Basically the changes are separated into multiple commits, so please review those based for each commit.
After getting merged, I think we can work on the upgrading main pubsub module to 0.12.
@morgante @aaron-lane