hashicorp / terraform-provider-google

Terraform Provider for Google Cloud Platform
https://registry.terraform.io/providers/hashicorp/google/latest/docs
Mozilla Public License 2.0
2.29k stars 1.72k forks source link

google_cloud_run_v2_service should validate that scaling.min_instance_count is less than scaling.max_instance_count #18969

Open minimice opened 1 month ago

minimice commented 1 month ago

Community Note

Description

With regards to this page

https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/cloud_run_v2_service

For the scaling block, update the documentation to state that max_instance_count is required to be present when min_instance_count is specified and vice versa. Failure to do so would result in the following error (here I did not specify max_instance_count only min_instance_count):

Error 400: Violation in UpdateServiceRequest.service.template.scaling.max_instance_count: must be greater or equal than min_instance_count.
β”‚ Details:
β”‚ [
β”‚   {
β”‚     "@type": "type.googleapis.com/google.rpc.BadRequest",
β”‚     "fieldViolations": [
β”‚       {
β”‚         "description": "must be greater or equal than min_instance_count.",
β”‚         "field": "Violation in UpdateServiceRequest.service.template.scaling.max_instance_count"
β”‚       }
β”‚     ]
β”‚   }
β”‚ ]

New or Affected Resource(s)

Potential Terraform Configuration

References

No response

b/357621141

melinath commented 1 month ago

From triage: Updated the title to reflect that not only should the docs be updated, we should also mark the fields as required_with each other. (Note: this is not currently documented but functions similarly to exactly_one_of - that is, all fields must be listed.) This shouldn't be a breaking change as long as the API always returns errors for invalid configurations.

c2thorn commented 1 month ago

https://github.com/GoogleCloudPlatform/magic-modules/blob/main/mmv1/templates/terraform/examples/cloudrunv2_service_sql.tf.erb#L8 is a passing example of max_instance_count without min_instance_count. So maybe this is only true for min_instance_count ?

charan-kumar-137 commented 1 month ago

max_instance_count and min_instance_count are actually NOT required with each other. The default for max_instance_count is 100 as mentioned in https://cloud.google.com/run/docs/configuring/max-instances#limits.

The above error might be due to setting min_instance_count greater than 100 (default).

melinath commented 1 month ago

Indeed - the error just says that max_instance_count needs to be greater than min_instance_count. Thanks for the catch!

charan-kumar-137 commented 3 weeks ago

@melinath, Not just Cloud Run, Other Services where there is Range Arguments (min & max) would need this kind of validation.

Spanner - https://cloud.google.com/spanner/docs/reference/rest/v1/projects.instances#autoscalinglimits Cloud Functions - https://cloud.google.com/functions/docs/reference/rest/v2/projects.locations.functions#serviceconfig

Since ValidateFunc is supported Only for Primitive Types. is there any way we can add Validation for Nested Objects.

melinath commented 3 weeks ago

you can do cross-field validation using a CustomizeDiff function. For MMv1 resources, this can be set up using the custom_diff top-level resource property (which takes a list of function names) and the functions can be defined using custom_code.constants.