Skip to content

feat: Complex variable types for module zones #111

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

Closed

Conversation

kevcube
Copy link

@kevcube kevcube commented Sep 16, 2024

use descriptive variable types.

versions.tf already specifies higher than 1.3.0 so no change needed there.

@kevcube kevcube changed the title feat(modules/zone): complex variable types feat(modules/zone): Complex variable types Sep 16, 2024
Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale label Oct 17, 2024
@kevcube
Copy link
Author

kevcube commented Oct 17, 2024

Not stale

@github-actions github-actions bot removed the stale label Oct 18, 2024
Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale label Nov 17, 2024
@BeyondEvil
Copy link

This is really nice!

Any chance of getting your input here @antonbabenko ?

@antonbabenko
Copy link
Member

This is really nice!

Any chance of getting your input here @antonbabenko ?

Yes, I plan to get back to this and many other PRs in terraform-aws-modules after AWS reinvent.

@github-actions github-actions bot removed the stale label Nov 28, 2024
Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale label Dec 29, 2024
@BeyondEvil
Copy link

How was re:invent @antonbabenko ? 😅

Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

These changes certainly look like an improvement, but it is essential to keep backward compatibility in mind and don't introduce breaking changes.

Please make sure that the code in examples/complete works as expected (now I see the error: each.value.vpc is null Cannot use a null value in for_each.).

@antonbabenko
Copy link
Member

How was re:invent @antonbabenko ? 😅

@BeyondEvil Thanks for asking, it went well - https://www.youtube.com/watch?v=fX7c2GGqTWs 😅

@antonbabenko antonbabenko changed the title feat(modules/zone): Complex variable types feat: Complex variable types for module zones Jan 7, 2025
@github-actions github-actions bot removed the stale label Jan 8, 2025
@BeyondEvil
Copy link

How was re:invent @antonbabenko ? 😅

@BeyondEvil Thanks for asking, it went well - https://www.youtube.com/watch?v=fX7c2GGqTWs 😅

Oh nice! I'll have a look. 👍

Copy link

github-actions bot commented Feb 8, 2025

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale label Feb 8, 2025
@BeyondEvil
Copy link

ping @kevcube

@BeyondEvil
Copy link

not stale

@github-actions github-actions bot removed the stale label Feb 9, 2025
Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale label Mar 12, 2025
@BeyondEvil
Copy link

BeyondEvil commented Mar 12, 2025

not stale

I might be able to fix the issue in a couple of weeks if @kevcube doesn't respond.

@github-actions github-actions bot removed the stale label Mar 13, 2025
name = lookup(each.value, "domain_name", each.key)
comment = lookup(each.value, "comment", null)
force_destroy = lookup(each.value, "force_destroy", false)
name = coalesce(each.value.domain_name, each.key)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name = coalesce(each.value.domain_name, each.key)
name = try(each.value.domain_name, each.key)

type = map(object({
domain_name = optional(string)
comment = optional(string)
force_destroy = optional(bool)
Copy link
Member

Choose a reason for hiding this comment

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

to maintain current behavior:

Suggested change
force_destroy = optional(bool)
force_destroy = optional(bool, false)


dynamic "vpc" {
for_each = try(tolist(lookup(each.value, "vpc", [])), [lookup(each.value, "vpc", {})])
for_each = each.value.vpc
Copy link
Member

Choose a reason for hiding this comment

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

not quite, I think we need more guards/checks

Suggested change
for_each = each.value.vpc
for_each = each.value.vpc != null ? each.value.vpc : []

Copy link

This PR has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this PR will be closed in 10 days

@github-actions github-actions bot added the stale label Apr 29, 2025
Copy link

github-actions bot commented May 9, 2025

This PR was automatically closed because of stale in 10 days

@github-actions github-actions bot closed this May 9, 2025
Copy link

github-actions bot commented Jun 8, 2025

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 8, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants