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 #17960 by adding 6 more tunnel encap options #18097

Merged
merged 3 commits into from
Dec 9, 2024

Conversation

jmcguir
Copy link
Contributor

@jmcguir jmcguir commented Nov 25, 2024

Fixes: #17960

I included all 6 types suggested in the issue. I did not include an "other" because it seems like that's against house style (there are a limited number of tunnel encap types, we could get them all in netbox).

They should now show as an option as a tunnel encap. I wasn't sure what the proper style was for the order of the choices so I sorted them by what I thought was most common. Happy to change the PR to some other sort.

@netravnen
Copy link

netravnen commented Nov 29, 2024

I would not recommend simply adding the OpenVPN and Wireguard to the choice field.

From comment #14683 (comment) a set of fields are necessary to specify the crypto information for WireGuard tunnels. I will expect the same to be necessary for OpenVPN tunnels using Client & Server Certificates.

@jmcguir
Copy link
Contributor Author

jmcguir commented Dec 2, 2024

I don't have any feelings on OpenVPN or WireGuard. I'm happy to remove them from the commit but I don't currently use or understand them so if more work is needed on the crypto side modeling, I'm not the guy unfortunately.

@jeremystretch
Copy link
Member

@jmcguir please see my comment here regarding the proposed additions.

Copy link
Member

@jeremystretch jeremystretch left a comment

Choose a reason for hiding this comment

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

Thanks @jmcguir!

@jeremystretch jeremystretch merged commit 21962b3 into netbox-community:develop Dec 9, 2024
3 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.

Add additional tunnel encapsulation choices: PPTP, L2TP, EoIP, SSTP
3 participants