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

fix: lookup default value for soft delete policy #318

Merged
merged 2 commits into from
Jul 3, 2024

Conversation

alexandre-abrioux
Copy link
Contributor

@alexandre-abrioux alexandre-abrioux commented May 24, 2024

This fix is a follow up to #309

Issue

On a multi-bucket configuration, adding a soft_delete_policy configuration block like so:

  soft_delete_policy = {
    "bucket1" = {
      // Disable protection on this bucket
      retention_duration_seconds = 0
    }
    "bucket2" = {
      // Increase protection to the max value (90 days)
      retention_duration_seconds = 7776000
    }
  }

; results in the following error:

│ Error: Invalid function argument
│ 
│   on .terraform/modules/gcs.cloud_storage/main.tf line 160, in resource "google_storage_bucket" "buckets":
│  160:     for_each = [lookup(var.soft_delete_policy, each.value, {})]
│     ├────────────────
│     │ while calling lookup(inputMap, key, default...)
│ 
│ Invalid value for "default" parameter: the default value must have the same
│ type as the map elements.

Fix

Instead of using an empty object {} as the default value of the lookup, we need to specify a complete object with every default value. The issue of not being able to use an empty object is described here: hashicorp/terraform#35027

@alexandre-abrioux alexandre-abrioux requested a review from a team as a code owner May 24, 2024 12:39
Copy link

google-cla bot commented May 24, 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.

@alexandre-abrioux
Copy link
Contributor Author

Hi @bharathkkb, maybe you could review this one? It's a fix related to your previous approval of #309. Thanks a lot 🙏

@jonathan-anyword
Copy link

Any update on this PR? need it also :)

@vdcadena553
Copy link

any updates here?

@flecno
Copy link

flecno commented Jul 3, 2024

Would be great if this is fixed. As GCP will start billing the soft delete in September I really want to disable it for some buckets, created by this module.

@alexandre-abrioux
Copy link
Contributor Author

Maybe @apeabody could help run CI and approve?

@apeabody
Copy link
Contributor

apeabody commented Jul 3, 2024

/gcbrun

Copy link
Contributor

@apeabody apeabody left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @alexandre-abrioux!

@apeabody apeabody merged commit aa7c57e into terraform-google-modules:master Jul 3, 2024
4 checks passed
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.

None yet

5 participants