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

Redis datasource not returning reserved_ip_range information #14154

Open
jawnsy opened this issue Mar 30, 2023 · 5 comments
Open

Redis datasource not returning reserved_ip_range information #14154

jawnsy opened this issue Mar 30, 2023 · 5 comments

Comments

@jawnsy
Copy link

jawnsy commented Mar 30, 2023

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request.
  • Please do not leave +1 or me too comments, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.
  • If an issue is assigned to the modular-magician user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If an issue is assigned to a user, that user is claiming responsibility for the issue. If an issue is assigned to hashibot, a community member has claimed the issue already.

Terraform Version

Working version:

Terraform v1.4.2
on darwin_arm64
+ provider registry.terraform.io/hashicorp/google v4.57.0
+ provider registry.terraform.io/hashicorp/google-beta v4.57.0

Version with bug:

Terraform v1.4.2
on darwin_arm64
+ provider registry.terraform.io/hashicorp/google v4.59.0
+ provider registry.terraform.io/hashicorp/google-beta v4.59.0

Affected Resource(s)

  • Data Source data.google_redis_instance no longer returning IP address range

Terraform Configuration Files

We have this logic (this is the approach that @jamiezieziula shared in terraform-google-modules/terraform-google-memorystore#89):

  reserved_ip_range = {
    # this is a little janky - but on initial apply of this component, terraform expects a "named" subnet range to hand to the redis instance,
    ## but on subsequent applies, providing the named range will force a replacement of the instance -- which we do not want.  This logic
    ## tells terraform to lookup (using a data source, under data.tf) the cloud2-redis-$ENV and return the reserved_ip_range its using.  If the
    ## data source returns a string (the CIDR range its allocated for the master instance), then our logic will supply that to this parameter
    ## otherwise, it will supply the named subnet range.
    legacy = (
      data.google_redis_instance.redis["legacy"].reserved_ip_range != null ?
      data.google_redis_instance.redis["legacy"].reserved_ip_range :
      data.terraform_remote_state.network.outputs.private_service_address_name
    )
    work = (
      data.google_redis_instance.redis["work"].reserved_ip_range != null ?
      data.google_redis_instance.redis["work"].reserved_ip_range :
      data.terraform_remote_state.network.outputs.private_service_address_name
    )
  }

For debugging purposes, we added this code:

output "legacy_reserved_range" {
  value = {
    range = data.google_redis_instance.redis["legacy"].reserved_ip_range
    service = data.terraform_remote_state.network.outputs.private_service_address_name
  }
}

output "work_reserved_range" {
  value = {
    range = data.google_redis_instance.redis["work"].reserved_ip_range
    service = data.terraform_remote_state.network.outputs.private_service_address_name
  }
}

Debug Output

Panic Output

Expected Behavior

Due to a limitation of the Memorystore API, we need to pass a network name as a string to the Memorystore module during initial creation, and then we need to use an IP address for subsequent invocations to avoid a drift problem. We described our approach here, and it has been working pretty well for us: terraform-google-modules/terraform-google-memorystore#89

Using 4.57.0, this code works well for us. However, the returned reserved_ip_range is null in 4.58.0 and 4.59.0. We believe that it may be related to the pull request that attempted to fix the issue: #13958

In 4.57.0, the outputs show this data:

Changes to Outputs:
  + legacy_reserved_range        = {
      + range   = "1.1.1.0/29"
      + service = "ga-dev-shared-base-hub-vpc-peering-internal"
    }
  + work_reserved_range          = {
      + range   = "1.1.1.8/29"
      + service = "ga-dev-shared-base-hub-vpc-peering-internal"
    }

Actual Behavior

In 4.58.0 and 4.59.0, the outputs show this data:

Changes to Outputs:
  + legacy_reserved_range        = {
      + range   = null
      + service = "ga-dev-shared-base-hub-vpc-peering-internal"
    }
  ~ redis_legacy_auth_string     = (sensitive value)
  ~ redis_legacy_connection_host = "1.1.1.3" -> (known after apply)
  ~ redis_work_auth_string       = (sensitive value)
  ~ redis_work_connection_host   = "1.1.1.11" -> (known after apply)
  + work_reserved_range          = {
      + range   = null
      + service = "ga-dev-shared-base-hub-vpc-peering-internal"
    }

Steps to Reproduce

I expect that you should be able to recreate this behavior by using the data source on any existing Redis instance. You should see that the reserved_ip_range variable is null.

Important Factoids

References

b/299600768

@jawnsy jawnsy added the bug label Mar 30, 2023
@edwardmedia edwardmedia self-assigned this Mar 30, 2023
@edwardmedia
Copy link
Contributor

The resource version of google_redis_instance provides both region and location_id (zonal). When users provide location_id, the resource will be created in a zone

The data version of it only accepts region. In this case, there is no way to retrieve its data

@edwardmedia edwardmedia assigned zli82016 and unassigned edwardmedia Mar 30, 2023
@zli82016
Copy link
Collaborator

It looks like it is because of the PR GoogleCloudPlatform/magic-modules#7429, reserved_ip_range variable returns null value.

Instead of adding ignore read on reserved ip ranges, a diff suppress function could be a better option.

@zli82016
Copy link
Collaborator

zli82016 commented Apr 3, 2023

Unassigning myself as I wasn't able to work on the diff suppression function last week

@zli82016 zli82016 removed their assignment Apr 3, 2023
@rileykarson rileykarson added persistent-bug Hard to diagnose or long lived bugs for which resolutions are more like feature work than bug work bug and removed bug persistent-bug Hard to diagnose or long lived bugs for which resolutions are more like feature work than bug work labels Apr 3, 2023
@rileykarson
Copy link
Collaborator

+1 that this was caused by GoogleCloudPlatform/magic-modules#7429

The value returned from the API was different than the input. After a discussion, the Redis API team recommended dropping this from diff calculation and future calls. This had the unintended effect of removing it from the datasource & import, because we're now exclusively relying on the clientside value. That was a miss on our part. This isn't easy to resolve

We could add a second field that displays the real value to add this back to the datasource. We can copy from the second field into the first field in the handwritten DS implementation, as well.

@rileykarson rileykarson self-assigned this Apr 3, 2023
@github-actions github-actions bot added forward/review In review; remove label to forward service/redis-instance labels Aug 17, 2023
@melinath melinath removed the forward/review In review; remove label to forward label Sep 7, 2023
@rileykarson rileykarson removed their assignment Sep 18, 2023
@adaminato42
Copy link

Is there any progress on this? At the moment we have resorted to ignoring reserved_ip_ranges in our Terraform, but this seems like a clunky workaround

+  lifecycle {
+    ignore_changes = [ reserved_ip_range ]
+  }

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

No branches or pull requests

7 participants