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

Apigee X: Southbound PSC to connect to internal Cloud Run services in GCP #116

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JoelGauci
Copy link

@JoelGauci JoelGauci commented Apr 28, 2023

In this sample, the following elements are provided:

  • PSC southbound from Apigee X

  • Internal Cloud Run apps

  • Internal HTTPS Load Balancer with Serverless NEG based on URL Masking to target Cloud Run services

  • Cloud DNS

  • I have run all the tests locally and they all pass.

  • I have followed the relevant style guide for my changes.

Copy link
Collaborator

@danistrebel danistrebel left a comment

Choose a reason for hiding this comment

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

Hi @JoelGauci thanks for this sample. I agree this pattern would definitely be a great addition.

A few changes that I'd like to propose:

  • Can we please add the apigee-x-core module and to make this sample standalone with the Apigee Instance included.
  • It seems like the Cloud Run Services are very similar. How about you generalize it to a single mock service and supply the path and mock body as env variable instead? That would help with the maintainability of the whole thing.
  • Can we consider using the cloud foundation fabric module for Cloud Run instead of GoogleCloudPlatform/cloud-run/google. Ideally we'd like to keep the number of independently versioned artifacts limited?
  • Move some hardcoded hostnames, ips and CIDRs to variables with default values?
  • Export the DNS names in the outputs.tf
  • Align the DNS names in the diagram with the values in the code (iloveapis vs example)
  • Maybe move the DNS part to its own reusable module

Happy to have a chat and discuss all of the above points and challenge if they make sense. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants