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

Add default googleapi route creation to net-vpc #1400

Merged
merged 7 commits into from May 26, 2023

Conversation

juliocc
Copy link
Collaborator

@juliocc juliocc commented May 26, 2023

No description provided.

modules/net-vpc/routes.tf Outdated Show resolved Hide resolved
modules/net-vpc/routes.tf Outdated Show resolved Hide resolved
@drebes
Copy link
Member

drebes commented May 26, 2023

All good just maybe change the var name? These are not default routes (in the networking sense).

@drebes
Copy link
Member

drebes commented May 26, 2023

One more point, a bit more controversial. I see today we don't default to true delete_default_routes_on_create. We could consider also adding a TF managed "default" route in addition to these in the module (create_special_routes), or at least do that just for FAST.

@juliocc
Copy link
Collaborator Author

juliocc commented May 26, 2023

One more point, a bit more controversial. I see today we don't default to true delete_default_routes_on_create. We could consider also adding a TF managed "default" route in addition to these in the module (create_special_routes), or at least do that just for FAST.

Good point. Can you open an issue for this?

@juliocc
Copy link
Collaborator Author

juliocc commented May 26, 2023

All good just maybe change the var name? These are not default routes (in the networking sense).

You're right, "default routes" is a terrible name. Any suggestions?

@juliocc juliocc marked this pull request as ready for review May 26, 2023 15:33
@juliocc juliocc merged commit b1ea36b into master May 26, 2023
9 checks passed
@juliocc juliocc deleted the jccb/default-vpc-routes branch May 26, 2023 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants