search

apigee / terraform-modules

Terraform modules and ready to use end-to-end examples for Apigee.
Apache License 2.0
52 stars 56 forks source link

feat: adding a host parameter to the target server to be able to use DNS #152

Closed rs1986x closed 4 months ago

rs1986x commented 4 months ago

What's changed, or what was fixed?

I have added the possibility of specifying a host parameter for target server. i will use this in combination with https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/google_service_networking_peered_dns_domain to access load balancer with a url mask type serverless backend with one single psc

Fixes: #issue

danistrebel commented 4 months ago

Hi @rs1986x, thanks for the PR.

I'm wondering what's the motivation for using the sb-psc module in the first place if you already have a host that you want to use. wouldn't you be better off by just using the google_apigee_target_server resource directly?

The creation of the other two resources looks wasteful at first glance. Maybe I'm missing something here.

rs1986x commented 4 months ago

Very good question @danistrebel: i run all my terraform with terragrunt, which has many advantages, with the exception that it really works well if you put all of your code in module form. i was faced with 2 choices here:

  1. what i did
  2. write a different module to manage the target servers independently.

honestly either approach is fine, with method 1 i manage all in one module, with method 2 i have the advantage of easier migration for the target servers (if i need to change the endpoint attachement for a particular target server)

tl;dr - the code is sound, the "is this a good change for this module" is totally up for debate

danistrebel commented 4 months ago

Thanks for the background.

Since the module is really meant to provide SB PSC functionality, using it for creating only an Apigee target server seems a bit of an overly complex approach. Given this is literally just about creating a module wrapper around a single resource I'd vote to keep the module as is.

Maybe it's better for your use case to fork this and have an Apigee southbound connection module that supports for PSC and direct host names.

danistrebel commented 4 months ago

Closing this, appreciate the suggestion and the constructive discussion here!