Skip to content

Comments

Only apply NatDNS if it´s an inline network#7035

Merged
nqb merged 1 commit intodevelfrom
fix/shuffle_dns
Jun 28, 2022
Merged

Only apply NatDNS if it´s an inline network#7035
nqb merged 1 commit intodevelfrom
fix/shuffle_dns

Conversation

@fdurand
Copy link
Member

@fdurand fdurand commented Jun 20, 2022

Description

(REQUIRED)
Describe what the new code is used for.
ie: Allow PacketFence to trigger the coffee machine on wireless EAP 802.1X connection to brew a fresh espresso.

Impacts

pfdhcp

Delete branch after merge

YES

Checklist

  • Document the feature
  • Add unit tests
  • Add acceptance tests (TestLink)

NEWS file entries

Bug Fixes

  • Fixed ShuffleDNS logic (only apply to inline network)

@nqb
Copy link
Contributor

nqb commented Jun 20, 2022

This PR will fix following issue: in a cluster environment with dns_on_vip_only enabled, pfdhcp returns IP address of current server in cluster in place of VIP.

@nqb
Copy link
Contributor

nqb commented Jun 20, 2022

I wonder if we should backport that fix because it's has the potential to cause issues:

  • before fix: DNS server was IP address of each cluster member
  • after fix: DNS server will be VIP only

If somebody has put firewall rules which doesn't allow DNS requests on VIP, DNS inside registration VLAN will not work anymore.
I doubt that is the case because if you enabled dns_on_vip_only, I doesn't make sense to restrict DNS traffic to VIP.

@nqb
Copy link
Contributor

nqb commented Jun 21, 2022

@fdurand,

FYI, on a setup with L3 registration VLANs, it looks like nat_dns is enabled by default which mean for these setups, we already return DNS VIP in DHCP response.

@nqb nqb merged commit c571ce9 into devel Jun 28, 2022
@nqb nqb deleted the fix/shuffle_dns branch June 28, 2022 05:26
nqb added a commit that referenced this pull request Jun 29, 2022
nqb added a commit that referenced this pull request Jun 29, 2022
nqb added a commit that referenced this pull request Jun 30, 2022
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.

2 participants