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

feat: adding support for automatic creation of psc consumer #613

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

sid-occrp
Copy link

When using psc you can optionally create psc consumer for main and read replica in the specified network and subnet.

Copy link

google-cla bot commented Jul 4, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Contributor

@g-awmalik g-awmalik left a comment

Choose a reason for hiding this comment

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

Looks good for the most part. Please take a look at one comment.

modules/postgresql/variables.tf Show resolved Hide resolved
@g-awmalik g-awmalik self-assigned this Jul 10, 2024
@sid-occrp
Copy link
Author

@imrannayer @isaurabhuttam how is my PR looking ?

@imrannayer
Copy link
Collaborator

/gcbrun

@imrannayer
Copy link
Collaborator

/gcbrun

@imrannayer
Copy link
Collaborator

@sid-occrp integration test failed

Error: "name" ("your_network_name") doesn't match regexp "^(?:[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?)$"

  with module.network-auto-psc.module.vpc.google_compute_network.network,
  on .terraform/modules/network-auto-psc/modules/vpc/main.tf line 21, in resource "google_compute_network" "network":
  21:   name                                      = var.network_name
}

@@ -320,15 +325,62 @@ resource "google_sql_user" "iam_account" {
deletion_policy = var.user_deletion_policy
}


resource "google_compute_address" "psc_ilb_consumer_address" {
Copy link
Member

Choose a reason for hiding this comment

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

For each resource that supports project argument let's explicitly provide it and not depend on inheriting from provider.

version = "~> 9.0"

project_id = var.project_id
network_name = "your_network_name"
Copy link
Member

Choose a reason for hiding this comment

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

Looks like test failure was due to name constraints Returning due to fatal error: FatalError{Underlying: error while running command: exit status 1; Error: "name" ("your_network_name") doesn't match regexp "^(?:[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?)$"

Suggested change
network_name = "your_network_name"
network_name = "sample-network"

Comment on lines +349 to +371
resource "google_dns_managed_zone" "psc_dns_zone" {
for_each = local.psc_consumers
name = each.value.name
dns_name = each.value.dns_name
visibility = "private"
private_visibility_config {
networks {
network_url = var.psc_consumer.network_id
}
}
}

resource "google_dns_record_set" "a" {
for_each = local.psc_consumers
name = each.value.dns_name
managed_zone = google_dns_managed_zone.psc_dns_zone[each.value.name].name
type = "A"
ttl = 300
rrdatas = [google_compute_address.psc_ilb_consumer_address[each.value.name].address]
}



Copy link
Member

Choose a reason for hiding this comment

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

@srinandan suggested we make this optional with default false

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.

4 participants