jednoprsak / PuppetNetworkManagerModule

Puppet module which installs, configures, and controls NetworkManager
MIT License
2 stars 7 forks source link

no additional configuration when method disabled #19

Closed kbucheli closed 6 months ago

kbucheli commented 6 months ago

With v1.0.0-rc1 I get some unnecessary configuration if I disable an IP family.

Resource definition:

networkmanager::ifc::connection { 'sniffer':
  mac_address => '00:50:56:9d:24:78',
  ipv4_method => 'disabled',
  ipv6_method => 'disabled',
}

gets now following diff compared to commit a538619:

@@ -10,6 +10,10 @@

 [ipv4]
 method=disabled
+may-fail=true

 [ipv6]
 method=disabled
+addr-gen-mode=0
+ip6-privacy=0
+may-fail=true

which is not so nice as no specific detail configuration is needed or has any effect with disabled. With this PR there is no diff any more for this case.

samuraiii commented 6 months ago

Hi, I understand that rest of the config is filling the interface config and that it is not looking so good (it is bothering me only slightly). But then you have a code which gets more complex for this only cosmetic problem. "ignore" or "disabled" means to switch the ipv config down and ignore the rest of the data under corresponding ipv section. I would suggest to either rework the PR to include all other interfaces (now it is work half done) or to reject it. Cheers S

kbucheli commented 6 months ago

@samuraiii : what are the "other interfaces" which needs to be included into the PR?

samuraiii commented 6 months ago

Every manifest that use $ipv4_method or $ipv6_method (exhaustive list): networkmanager::ifc::bond networkmanager::ifc::bridge networkmanager::ifc::connection

kbucheli commented 6 months ago

I see, thanks. I will then create a new PR.

samuraiii commented 6 months ago

May I suggest to employ intermediate call of deep_merge()? Something like:

# This is just an example!!!
$ipv4_config = $ipv4_method ? {
  disabled => { ipv4 => { method => disabled } },
  default => deep_merge($all_ipv4_config),
}

then include this result into the interface config?