-
Notifications
You must be signed in to change notification settings - Fork 15
fix: use host for dict key in tunnel - hostname is not a dict key #236
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
base: main
Are you sure you want to change the base?
fix: use host for dict key in tunnel - hostname is not a dict key #236
Conversation
Cause: The template code was using the hostname field as a dict key into the tunnel configuration when a hostname was given in the configuration. By default, the host field is used which works correctly. Consequence: The host field is the key, not the hostname field. This caused an error to be reported that the hostname value is not a attribute of the tunnel dict. Fix: Use the correct host field value as the tunnel dict key even if the hostname field is given. Result: The tunnel is configured correctly with no errors. Signed-off-by: Rich Megginson <rmeggins@redhat.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts the libreswan host-to-host tunnel Jinja2 template to consistently use the dictionary key (host) for lookups instead of the hostname value, and extends the subnet-to-subnet test to cover multiple hosts with hostnames and subnets. ER diagram for tunnel and hosts configuration structureerDiagram
TUNNEL {
string name
string host
string hostname
}
HOST_ENTRY {
string host_key
string hostname
string subnets_list
}
SUBNET {
string cidr
}
TUNNEL ||--o{ HOST_ENTRY : has_hosts
HOST_ENTRY ||--o{ SUBNET : has_subnets
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
fixes #230 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've left some high level feedback:
- In
libreswan-host-to-host.conf.j2,tunnel.hosts[thishost]is now used only forsubnetswhile other lookups still usehost; consider standardizing on a single key variable to avoid inconsistent behavior whenhostandhostnamediffer. - The change of
__vpn_num_hostsfrom 1 to 2 intests_subnet_to_subnet.ymlhappens alongside adding three hosts (mainhost.local,host01.local,host02.local); double-check that this count still reflects the intended semantics of the variable to avoid future confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `libreswan-host-to-host.conf.j2`, `tunnel.hosts[thishost]` is now used only for `subnets` while other lookups still use `host`; consider standardizing on a single key variable to avoid inconsistent behavior when `host` and `hostname` differ.
- The change of `__vpn_num_hosts` from 1 to 2 in `tests_subnet_to_subnet.yml` happens alongside adding three hosts (`mainhost.local`, `host01.local`, `host02.local`); double-check that this count still reflects the intended semantics of the variable to avoid future confusion.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[citest] |
Cause: The template code was using the hostname field as a dict key into the tunnel configuration
when a hostname was given in the configuration. By default, the host field is used which works
correctly.
Consequence: The host field is the key, not the hostname field. This caused an error to be reported
that the hostname value is not a attribute of the tunnel dict.
Fix: Use the correct host field value as the tunnel dict key even if the hostname field is given.
Result: The tunnel is configured correctly with no errors.
Signed-off-by: Rich Megginson rmeggins@redhat.com
Summary by Sourcery
Fix tunnel template to use the correct host key when looking up host configuration and expand subnet-to-subnet test coverage to multiple hosts.
Bug Fixes:
Tests: