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 triggers to Cloud Functions v2 #965

Merged

Conversation

wiktorn
Copy link
Collaborator

@wiktorn wiktorn commented Nov 10, 2022

  • (incompatibile change) change structure for trigger_config and create substructures for v1 Cloud Functions and v2
  • (incompatibile change) rename bucket_config.lifecycle_delete_age to bucket_config,lifecycle_delete_age_days
  • (incompatibile change) rename function_config.instances to function_config.instance_count
  • (incompatibile change) rename function_config.memory to function_config.memory_mb
  • (incompatibile change) rename function_config.timeout to function_config.timeout_seconds
  • add optional for objects in variables.tf
  • make examples in README runable as tests
  • add example for Cloud Function v2
  • add example for trigger for Cloud Function v2
  • remove optional variables from examples with null value
  • assign IAM bindings to function based on version
  • bump default Python version to 3.10

Closes #973

@ludoo ludoo added incompatible change Pull request that breaks compatibility with previous version on:modules labels Nov 10, 2022
Copy link
Collaborator

@ludoo ludoo left a comment

Choose a reason for hiding this comment

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

A couple minor comments and a doubt which can be ignored, but good to go many thanks!

modules/cloud-function/README.md Outdated Show resolved Hide resolved
modules/cloud-function/variables.tf Outdated Show resolved Hide resolved
modules/cloud-function/variables.tf Outdated Show resolved Hide resolved
modules/cloud-function/variables.tf Outdated Show resolved Hide resolved
@wiktorn
Copy link
Collaborator Author

wiktorn commented Nov 12, 2022

@juliocc @ludoo the only feature I also consider for this, is to also support creation of trigger service accounts. As you can notice in example - caller needs to specify service account for trigger by themselves.

I can add create_service_account = optional(bool, true) in v2 trigger_config and grant roles/run.invoker by default. This will help network-dashboard blueprint (that's my guess, why it's not working with V2 CF and requires ALLOW_ALL).

The other thing that I guess we could improve here is to provide option to specify roles for Cloud Function service account. Though this kind of overlaps with iam attribute. Any thoughts on that?

@wiktorn wiktorn force-pushed the cloud_functions_trigger_v2_optionals branch 2 times, most recently from 363bb03 to 483650e Compare November 12, 2022 13:13
@ludoo
Copy link
Collaborator

ludoo commented Nov 13, 2022

That looks like a good idea, up to you if you want to add it here or in a separate PR.

Julio was also thinking about splitting the module into a v1 and a v2 module, which kind of makes sense. Again, feel free to merge this and we can discuss it later, or split it now if it makes more sense to you.

BTW, you should see a merge button already, if that's not the case let us know.

@juliocc
Copy link
Collaborator

juliocc commented Nov 14, 2022

Julio was also thinking about splitting the module into a v1 and a v2 module, which kind of makes sense. Again, feel free to merge this and we can discuss it later, or split it now if it makes more sense to you.

Lets's discuss it. I've been thinking about it for a few days but tbh I don't know if it's a good idea.

modules/cloud-function/variables.tf Show resolved Hide resolved
modules/cloud-function/variables.tf Outdated Show resolved Hide resolved
modules/cloud-function/main.tf Outdated Show resolved Hide resolved
modules/cloud-function/variables.tf Outdated Show resolved Hide resolved
modules/cloud-function/main.tf Outdated Show resolved Hide resolved
modules/cloud-function/main.tf Outdated Show resolved Hide resolved
* add `trigger_config_v2` for v2 functions
* add optional for objects in variables.tf
* make examples in README runnable
* add example for Cloud Function v2
* add exapmle for trigger for Cloud Function v2
* remove optional variables from examples with `null` value
* Refactor trigger_config and trigger_config_v2 into one structure
* bump default python version to 3.10
* typo fixes
@wiktorn wiktorn force-pushed the cloud_functions_trigger_v2_optionals branch from 6c194a0 to 0121806 Compare November 16, 2022 15:46
@wiktorn wiktorn merged commit 4d4bcb2 into GoogleCloudPlatform:master Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
incompatible change Pull request that breaks compatibility with previous version on:modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cloud Functions bindings for v2
3 participants