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: Fix type for NAT subnetwork attributes #85

Conversation

peikk0
Copy link
Contributor

@peikk0 peikk0 commented Aug 24, 2023

  • fix type for NAT subnetwork attributes
  • remove unnecessary lookup() calls with optional attributes

Fixes #84

@peikk0 peikk0 requested review from imrannayer and a team as code owners August 24, 2023 04:43
@google-cla
Copy link

google-cla bot commented Aug 24, 2023

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.

@peikk0 peikk0 changed the title Fix type for NAT subnetwork attributes fix: Fix type for NAT subnetwork attributes Aug 24, 2023
variables.tf Outdated Show resolved Hide resolved
variables.tf Outdated Show resolved Hide resolved
nat.tf Show resolved Hide resolved
nat.tf Show resolved Hide resolved
@imrannayer
Copy link
Collaborator

@peikk0 thanks for the PR. few change requested

@peikk0 peikk0 force-pushed the pguinoiseau/fix-nat-subnetwork-attr-types branch from 71b2fd1 to ad538c5 Compare August 28, 2023 06:02
@peikk0 peikk0 requested a review from imrannayer August 28, 2023 06:13
@imrannayer
Copy link
Collaborator

/gcbrun

@imrannayer
Copy link
Collaborator

@peikk0 can you plz follow contribution guide to fix issues with lint test.

Thanks

@peikk0 peikk0 force-pushed the pguinoiseau/fix-nat-subnetwork-attr-types branch from ad538c5 to ea52554 Compare August 29, 2023 05:10
@peikk0
Copy link
Contributor Author

peikk0 commented Aug 29, 2023

@imrannayer I've fixed the doc linting issue. I was certain I already did it the time before. 🙈

@imrannayer
Copy link
Collaborator

/gcbrun

@imrannayer
Copy link
Collaborator

@peikk0 thx for fixing the issue. If you are getting error with type list then you can switch it to set. Also can you add a subnet and use that subnet in nats block in this example. This will help us with testing this part of code.

Thanks

@ebeltramo96
Copy link

any update? we cannot merge the update for the same reason

@peikk0 peikk0 force-pushed the pguinoiseau/fix-nat-subnetwork-attr-types branch from e0c3223 to 7a4b996 Compare August 31, 2023 10:55
@peikk0
Copy link
Contributor Author

peikk0 commented Aug 31, 2023

@imrannayer I've updated the NAT example, it applies cleanly for me.

@imrannayer
Copy link
Collaborator

/gcbrun

1 similar comment
@imrannayer
Copy link
Collaborator

/gcbrun

@imrannayer imrannayer merged commit 1498a8c into terraform-google-modules:master Aug 31, 2023
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.

source_ip_ranges_to_nat and secondary_ip_range_names should be sets of strings
3 participants