Skip to content

Conversation

@rhariady
Copy link
Contributor

@rhariady rhariady commented Apr 16, 2025

This PR add support for MD5 authentication in BGP session in the interface submodule (include reference in interconnect_attachment module). Needs min provider version 5.12

BEGIN_COMMIT_OVERRIDE
feat!: MD5 authentication for BGP support
END_COMMIT_OVERRIDE

@rhariady rhariady requested review from a team and imrannayer as code owners April 16, 2025 08:31
@imrannayer
Copy link
Collaborator

@rhariady thanks foe the PR. Left some comments.

@imrannayer imrannayer self-assigned this Apr 22, 2025
@rhariady
Copy link
Contributor Author

rhariady commented Apr 23, 2025

Hi, thanks for the review @imrannayer!

Regarding lookup, i was trying to keep it consistent with other arguments (eg. advertised_route_priority and bfd) that is also optional. But i agree that using try is probably better than lookup, so i think I can keep it consistent still by updating the other arguments to use try as well.

But then, i realize that since all these variables had already been optional, probably we can omit the lookup or try altogether, as it already default-ed to null and make lookup or try redundant. What do you think? I will be fine to implement either way.

@rhariady rhariady requested a review from imrannayer April 23, 2025 04:30
@rhariady rhariady force-pushed the bgp_md5_authentication branch from c21abf4 to 48a86f9 Compare April 23, 2025 04:43
@imrannayer
Copy link
Collaborator

/gcbrun

1 similar comment
@imrannayer
Copy link
Collaborator

/gcbrun

@imrannayer imrannayer merged commit e6f8b6c into terraform-google-modules:main Apr 24, 2025
4 checks passed
@imrannayer imrannayer changed the title feat: MD5 authentication for BGP support feat!: MD5 authentication for BGP support Apr 24, 2025
@imrannayer imrannayer added the release-please:force-run Force release-please to check for changes. label Apr 24, 2025
@release-please release-please bot removed the release-please:force-run Force release-please to check for changes. label Apr 24, 2025
@imrannayer imrannayer added the release-please:force-run Force release-please to check for changes. label Apr 24, 2025
@release-please release-please bot removed the release-please:force-run Force release-please to check for changes. label Apr 24, 2025
@imrannayer
Copy link
Collaborator

@rhariady I merged this PR. Can you plz submit another PR to change min provider version in versions.tf file for interface sub-module to 5.12? MD5 authentication was part of that version.

@rhariady
Copy link
Contributor Author

@rhariady I merged this PR. Can you plz submit another PR to change min provider version in versions.tf file for interface sub-module to 5.12? MD5 authentication was part of that version.

Thanks @imrannayer! And sorry i missed to update the min provider version, but i have open another MR for that as you suggested in #153.

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.

2 participants