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

CloudSQL PSC Endpoints support #2242

Merged
merged 7 commits into from
May 12, 2024
Merged

Conversation

wiktorn
Copy link
Collaborator

@wiktorn wiktorn commented Apr 27, 2024

Add PSC Endpoints to net-address. Simplifies creation of the PSC endpoint for any PSC supporting resource.

Additional changes:

  • Added message how to generate inventories for YAML based tests.
  • Added E2E tests for CloudSQL

Checklist

I applicable, I acknowledge that I have:

  • Read the contributing guide
  • Ran terraform fmt on all modified files
  • Regenerated the relevant README.md files using tools/tfdoc.py
  • Made sure all relevant tests pass

@juliocc
Copy link
Collaborator

juliocc commented Apr 29, 2024

decide what to do with network_config.connectivity.psc_allowed_consumer_projects (either remove or merge var.psc_consumers into var.network_config.connectivity)

I'm not so sure I like having the creation of the PSC endpoints in this module. Are we going to add PSC endpoint management to every product that supports it (e.g. GKE, Apigee, etc)?

Personally I'd prefer keeping this module just setting the allowed PSC consumer projects and leaving the creation of the endpoints to the consumer. Having said that, PSC creation is becoming common enough that I can see the use of a net-psc-endpoint module that creates the google_compute_address and the google_compute_forwarding_rule.

tests/collectors.py Outdated Show resolved Hide resolved
@ludoo
Copy link
Collaborator

ludoo commented Apr 29, 2024

I'm not so sure I like having the creation of the PSC endpoints in this module. Are we going to add PSC endpoint management to every product that supports it (e.g. GKE, Apigee, etc)?

Personally I'd prefer keeping this module just setting the allowed PSC consumer projects and leaving the creation of the endpoints to the consumer. Having said that, PSC creation is becoming common enough that I can see the use of a net-psc-endpoint module that creates the google_compute_address and the google_compute_forwarding_rule.

100% agree, endpoints are conceptually part of the net- family of modules, as they materialize an LB and consume an address. I would explore either integration into net-address or a new net-xxxx module for them.

Plus, this would avoid integrating them everywhere as more more products start offering support, like Julio noted above.

@wiktorn wiktorn force-pushed the wiktorn/cloudsql-psc-endpoints branch from 8b597f3 to 843f43a Compare May 11, 2024 12:39
@wiktorn wiktorn force-pushed the wiktorn/cloudsql-psc-endpoints branch from 843f43a to d72254b Compare May 11, 2024 12:41
@wiktorn
Copy link
Collaborator Author

wiktorn commented May 11, 2024

Personally I'd prefer keeping this module just setting the allowed PSC consumer projects and leaving the creation of the endpoints to the consumer. Having said that, PSC creation is becoming common enough that I can see the use of a net-psc-endpoint module that creates the google_compute_address and the google_compute_forwarding_rule.

100% agree, endpoints are conceptually part of the net- family of modules, as they materialize an LB and consume an address. I would explore either integration into net-address or a new net-xxxx module for them.

Plus, this would avoid integrating them everywhere as more more products start offering support, like Julio noted above.

I really liked the idea of adding that to net-address, which already has some PSC logic, so I followed this path.

This PR is now a bit split-brained, between E2E tests in cloudsql-instance and changes in net-address, but I hope that you don't mind that.

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.

two nits, great PR

@wiktorn wiktorn force-pushed the wiktorn/cloudsql-psc-endpoints branch from 446ea07 to a2f69b8 Compare May 11, 2024 14:54
@wiktorn wiktorn force-pushed the wiktorn/cloudsql-psc-endpoints branch from 1e5c1d4 to 3f8ebdc Compare May 12, 2024 05:26
@wiktorn wiktorn merged commit 6a3c7fe into master May 12, 2024
13 checks passed
@wiktorn wiktorn deleted the wiktorn/cloudsql-psc-endpoints branch May 12, 2024 10:00
sruffilli pushed a commit that referenced this pull request May 15, 2024
* Add PSC endpoints consumers to net-address
* Cloud SQL E2E tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants