Add cloud quota preferences by jacobstr · Pull Request #761 · crossplane-contrib/provider-upjet-gcp

jacobstr · 2025-03-21T07:36:32Z

Description of your changes

Addresses #550. Bulk of the content was this diff in the first commit, everything else was code generation.

Fixes #550

I have:

Read and followed Crossplane's [contribution process].
Run make reviewable to ensure this PR is ready for review.

How has this code been tested

I intend to respond with a comment as outlined here https://github.com/crossplane/upjet/blob/main/docs/adding-new-resource.md#automated-tests---uptest

jacobstr · 2025-03-21T07:37:42Z

 	"google_tags_tag_value": config.IdentifierFromProvider,
+
+	// cloudquotas
+	"google_cloud_quotas_quota_preference": config.TemplatedStringAsIdentifier("name", "{{ .parameters.parent }}/locations/global/quotaPreferences/{{ .external_name }}"),


I think I got this right? https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/cloud_quotas_quota_preference#name-4 looks like a pretty cut and dry version of https://github.com/crossplane/upjet/blob/main/docs/adding-new-resource.md#gcp

Yes, it seems okay, but since we are using the name parameter in the external name here, it would be a good idea to mark it as required. Here is an example configuration:

provider-upjet-gcp/config/redis/config.go

Line 28 in ae12d12

r.MarkAsRequired("region")

Also, it would be good to move the cloudquotas group to the lines with cloud groups in alphabetical order.

Done. I believe :) See 190ed4b.

I was anticipating that adding this would affect the code generation and, in turn, mark it as required in the openapi schema at https://github.com/jacobstr/provider-upjet-gcp/blob/1db2f529d96c09b7de407c4775a9d1c3a8440a78/package/crds/cloudquotas.gcp.upbound.io_quotapreferences.yaml#L232

jacobstr · 2025-03-21T07:38:19Z


 	"google_essential_contacts_contact$": ReplaceGroupWords("", 2),
+
+	"google_cloud_quotas.+$": ReplaceGroupWords("", 2),


This little bit is underdocumented in the https://github.com/crossplane/upjet/blob/main/docs/adding-new-resource.md#gcp guide fwiw, but adding this induced the code generation for a new provider as well.

When we don't add this, which group does it add the resource to?

cloud.gcp.upbound.io/v1beta1

jacobstr · 2025-03-21T07:39:40Z

/test-examples="examples/cloudquotas/v1beta1/quotapreference.yaml"

turkenf · 2025-03-21T13:14:27Z

/test-examples="examples/cloudquotas/v1beta1/quotapreference.yaml"

jacobstr · 2025-03-21T15:58:18Z

+    dimensions:
+      region: us-east1
+    parent: projects/my-project-name
+    quotaConfig:


The code gen was producing an array here and that was leading to test failures. I inspected the generated code and the QuotaConfigParameters property is not a slice.

Seems like something to nip in the bud lest the code gen want to clobber my bespoke tuning?

This is a missing/not handled issue in our code generation pipeline. I opened an issue for this on Upjet after you came across this, feel free to leave a comment there.

However, from what I see here, you have removed the generated example. Changing the generated examples is not recommended and will cause errors in the CI pipeline. After running make generate, please leave the generated example as is and make the corrections in the copied one.

Pardon the laaate response here. The generated examples are the ones in examples-generated? So leave those intact and modify examples.

Double checking because we're commenting on examples folder. I had not committed the generated example. Just committed a moment ago.

turkenf

Welcome, Jacob Straszynski! 😊

Thanks so much for your first contribution with this PR — I appreciate your effort and patience. I've left a few comments for you to consider.

turkenf · 2025-04-10T19:02:26Z

 	"google_tags_tag_value": config.IdentifierFromProvider,
+
+	// cloudquotas
+	"google_cloud_quotas_quota_preference": config.TemplatedStringAsIdentifier("name", "{{ .parameters.parent }}/locations/global/quotaPreferences/{{ .external_name }}"),


Yes, it seems okay, but since we are using the name parameter in the external name here, it would be a good idea to mark it as required. Here is an example configuration:

provider-upjet-gcp/config/redis/config.go

Line 28 in ae12d12

r.MarkAsRequired("region")

Also, it would be good to move the cloudquotas group to the lines with cloud groups in alphabetical order.

turkenf · 2025-04-10T19:03:58Z


 	"google_essential_contacts_contact$": ReplaceGroupWords("", 2),
+
+	"google_cloud_quotas.+$": ReplaceGroupWords("", 2),


When we don't add this, which group does it add the resource to?

turkenf · 2025-04-10T19:39:17Z

+    dimensions:
+      region: us-east1
+    parent: projects/my-project-name
+    quotaConfig:


This is a missing/not handled issue in our code generation pipeline. I opened an issue for this on Upjet after you came across this, feel free to leave a comment there.

However, from what I see here, you have removed the generated example. Changing the generated examples is not recommended and will cause errors in the CI pipeline. After running make generate, please leave the generated example as is and make the corrections in the copied one.

github-actions · 2025-07-10T02:51:46Z

This provider repo does not have enough maintainers to address every pull request. Since there has been no activity in the last 90 days it is now marked as stale. It will be closed in 14 days if no further activity occurs. Leaving a comment starting with /fresh will mark this issue as not stale.

jacobstr · 2025-07-10T22:04:40Z

/fresh

turkenf · 2025-07-22T18:53:01Z

/test-examples="examples/cloudquotas/v1beta1/quotapreference.yaml"

jacobstr · 2025-07-22T21:20:58Z

Hrm I see the job failed:

case.go:363: QuotaPreference.cloudquotas.gcp.upbound.io "preference" is invalid: [spec.forProvider.quotaConfig.preferredValue: Invalid value: "integer": spec.forProvider.quotaConfig.preferredValue in body must be of type string: "integer", <nil>: Invalid value: "null": some validation rules were not checked because the object was invalid; correct the existing errors to complete validation]

The file example does have a quotaConfig.preferredValue https://github.com/crossplane-contrib/provider-upjet-gcp/blob/koobz/cloud-quotas/examples/cloudquotas/v1beta1/quotapreference.yaml#L16

jeanduplessis · 2025-08-14T12:05:10Z

@jacobstr, now that the v2 version is out, we're keen to include this PR in the next release of this provider. Can you please rebase it on the latest code from the main? If you can't continue contributing to this one, we'd be happy to take it over for you.

Initial steps to get the code gen goodness going. I'm aiming to produce a cloudquotas provider for the family as well. Signed-off-by: Jacob Straszynski <jacob.straszynski@planet.com>

Signed-off-by: Jacob Straszynski <jacob.straszynski@planet.com>

Unsure why the code generation made this example a list instead of an object. I suppose terraform allows multiple blocks for some resources, but it doesn't look like a good fit here. Signed-off-by: Jacob Straszynski <jacob.straszynski@planet.com>

Signed-off-by: Jacob Straszynski <jacob.straszynski@planet.com>

@turkenf

Based on feedback from @turkenf since we are using the name parameter in the external name here, it would be a good idea to mark it as required. Signed-off-by: Jacob Straszynski <jacob.straszynski@planet.com>

Signed-off-by: Jacob Straszynski <jacob.straszynski@planet.com> Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

sergenyalcin · 2025-10-22T10:41:23Z

Hİ @jacobstr, thanks for your work on this PR. We'd like to get this merged and make sure you get credit for it. We'll take it from here by updating it to resolve conflicts with main and fixing the tests. Appreciate your contribution!

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

sergenyalcin · 2025-10-22T10:47:44Z

/test-examples="examples/cloudquotas/cluster/v1beta1/quotapreference.yaml"

jacobstr requested review from jastang, sergenyalcin and turkenf as code owners March 21, 2025 07:36

jacobstr changed the title ~~Koobz/cloud quotas~~ Add cloud quota preferences Mar 21, 2025

jacobstr commented Mar 21, 2025

View reviewed changes

jacobstr force-pushed the koobz/cloud-quotas branch from cd1f919 to 7be31c1 Compare March 21, 2025 15:55

jacobstr commented Mar 21, 2025

View reviewed changes

turkenf reviewed Apr 10, 2025

View reviewed changes

github-actions Bot added the stale label Jul 10, 2025

github-actions Bot removed the stale label Jul 11, 2025

jacobstr added 6 commits October 22, 2025 13:01

configure cloud quotas code generation

94ba4ad

Initial steps to get the code gen goodness going. I'm aiming to produce a cloudquotas provider for the family as well. Signed-off-by: Jacob Straszynski <jacob.straszynski@planet.com>

generate cloudquotas provider and preferences resource

51f4d76

Signed-off-by: Jacob Straszynski <jacob.straszynski@planet.com>

convert quotaconfigparameter to object

4f5f681

Unsure why the code generation made this example a list instead of an object. I suppose terraform allows multiple blocks for some resources, but it doesn't look like a good fit here. Signed-off-by: Jacob Straszynski <jacob.straszynski@planet.com>

adding generate cloudquotas example

1ada3e5

Signed-off-by: Jacob Straszynski <jacob.straszynski@planet.com>

mark name required for google_cloud_quotas_quota_preference

10b004f

Based on feedback from @turkenf since we are using the name parameter in the external name here, it would be a good idea to mark it as required. Signed-off-by: Jacob Straszynski <jacob.straszynski@planet.com>

move cloudquotas configuration block to alphabetical location

a862264

Signed-off-by: Jacob Straszynski <jacob.straszynski@planet.com> Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

sergenyalcin force-pushed the koobz/cloud-quotas branch from 1db2f52 to 0792efe Compare October 22, 2025 10:42

Fix example manifest

ad3899e

Signed-off-by: Sergen Yalçın <yalcinsergen97@gmail.com>

sergenyalcin force-pushed the koobz/cloud-quotas branch from 0792efe to ad3899e Compare October 22, 2025 10:46

jeanduplessis approved these changes Oct 22, 2025

View reviewed changes

sergenyalcin merged commit 0022ac4 into crossplane-contrib:main Oct 22, 2025
9 checks passed


		"google_essential_contacts_contact$": ReplaceGroupWords("", 2),

		"google_cloud_quotas.+$": ReplaceGroupWords("", 2),

Conversation

jacobstr commented Mar 21, 2025

Description of your changes

How has this code been tested

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jacobstr commented Mar 21, 2025

Uh oh!

turkenf commented Mar 21, 2025

Uh oh!

jacobstr Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

turkenf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 10, 2025

Uh oh!

jacobstr commented Jul 10, 2025

Uh oh!

turkenf commented Jul 22, 2025

Uh oh!

jacobstr commented Jul 22, 2025

Uh oh!

jeanduplessis commented Aug 14, 2025

Uh oh!

sergenyalcin commented Oct 22, 2025

Uh oh!

sergenyalcin commented Oct 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jacobstr Mar 21, 2025 •

edited

Loading