terraform-google-modules/terraform-google-scheduled-function

Module broken

ocervell opened this issue · 6 comments

Last change from #26 (changing variable scheduler_job from any to object({name=string}) has broken the module.

I'm getting the error:

Error: Inconsistent conditional result types

  on .terraform/modules/slo-app-search-latency-64ms.slo_cloud_function/terraform-google-modules-terraform-google-scheduled-function-7aacc62/outputs.tf line 23, in output "scheduler_job":
  23:   value       = var.scheduler_job == null ? google_cloud_scheduler_job.job : var.scheduler_job
    |----------------
    | google_cloud_scheduler_job.job is tuple with 1 element
    | var.scheduler_job is null

The true and false result expressions must have consistent types. The given
expressions are tuple and object, respectively.

I don't think changing the type of var.scheduler_job is relevant to that error. The issue appears to be that we're outputting google_cloud_scheduler_job.job which is tuple because that resource has a count. We should be outputting google_cloud_scheduler_job.job[0].

I am doing some testing, but it seems I'm getting the same error with google_cloud_scheduler_job.job.0 (the outcome is: "google_cloud_scheduler_job.job.0 is an object with 12 elements" error)

We should reconsider outputting a variable as it does not impact the dependency graph in a meaningful way and removing it from the output value would avoid this problem.

The goal of the PR was to use this output variable to allow sharing a Cloud Scheduler with other modules, so this is needed.

I understand the intent of the new feature; it's just that there is no value in conditionally outputting the input variable. All modules which need to use the same job should use the job output from the module which creates it. Passing that output in and out of each chained module does not enforce a meaningful dependency, and creates problems as we see with the type.

Ok, let's default to null if no scheduler is created for the output and enforce a strict type checking.