strongswan / strongMan

Management UI for strongSwan
https://www.strongswan.org/
Other
114 stars 39 forks source link

Can't set many DNS attribute values for a Pool #149

Closed theko2fi closed 9 months ago

theko2fi commented 10 months ago

Hi everyone !

First, thank you for this nice project and all the efforts you put regarding the maintenance. I noticed something like a bug today while setting a Pool and I decided to report it here.

image

Comma separated values aren't supported.

By the way, I also think we should have the ability to set multiple attributes for a pool. What do you think? I mean, it should be possible to set both the DNS and DHCP (for example) attributes for the same pool.

Thanks

tobiasbrunner commented 10 months ago

Yeah, as you noticed, this is definitely limited. I think comma-separated values could be supported by adding something like this:

diff --git a/strongMan/apps/pools/models/pools.py b/strongMan/apps/pools/models/pools.py
index b02f4a963cd5..6550bcb5c91c 100644
--- a/strongMan/apps/pools/models/pools.py
+++ b/strongMan/apps/pools/models/pools.py
@@ -31,7 +31,7 @@ class Pool(models.Model):
         elif self.attribute is None:
             pools = {self.poolname: {'addrs': self.addresses}}
         else:
-            pools = {self.poolname: {'addrs': self.addresses, self.attribute: [self.attributevalues]}}
+            pools = {self.poolname: {'addrs': self.addresses, self.attribute: self.attributevalues.split(',')}}
         return pools

     def __str__(self):

Supporting multiple attribute types per pool would require a lot more changes. If you need that, you have to use swanctl.conf or vici directly.

theko2fi commented 10 months ago

Hi dear @tobiasbrunner ,

Thanks for taking time to suggest a workaround. Unfortunately, it didn't work. Still have the same error message.

image

tobiasbrunner commented 10 months ago

I see. Yeah, those students really seemed to like duplicating code. You'll need this patch too:

diff --git a/strongMan/apps/pools/forms.py b/strongMan/apps/pools/forms.py
index 3e89ae56bf44..0eb9fb0189af 100644
--- a/strongMan/apps/pools/forms.py
+++ b/strongMan/apps/pools/forms.py
@@ -26,7 +26,6 @@ class AddOrEditForm(forms.Form):
         else:
             pool.attribute = self.cleaned_data['attribute']
             pool.attributevalues = self.cleaned_data['attributevalues']
-        pool.save()

     def is_valid(self):
         valid = super(AddOrEditForm, self).is_valid()
diff --git a/strongMan/apps/pools/views/AddHandler.py b/strongMan/apps/pools/views/AddHandler.py
index 70009df6dcf3..da3ab0e978a7 100644
--- a/strongMan/apps/pools/views/AddHandler.py
+++ b/strongMan/apps/pools/views/AddHandler.py
@@ -45,7 +45,6 @@ class AddHandler(object):
                                          'Can\'t add pool: Attribute values unclear for Attribute "None"')
                     return self._render(self.form)
                 pool = Pool(poolname=self.form.my_poolname, addresses=self.form.my_addresses)
-                vici_pool = {self.form.my_poolname: {'addrs': self.form.my_addresses}}
             else:
                 if self.form.my_attributevalues == "":
                     messages.add_message(self.request, messages.ERROR,
@@ -55,13 +54,11 @@ class AddHandler(object):
                 pool = Pool(poolname=self.form.my_poolname, addresses=self.form.my_addresses,
                             attribute=attr,
                             attributevalues=self.form.my_attributevalues)
-                vici_pool = {self.form.my_poolname: {'addrs': self.form.my_addresses,
-                                                     attr: [self.form.my_attributevalues]}}
         try:
             pool.clean()
-            pool.save()
             vici = ViciWrapper()
-            vici.load_pool(vici_pool)
+            vici.load_pool(pool.dict())
+            pool.save()

         except ViciException as e:
             messages.add_message(self.request, messages.ERROR, str(e))
diff --git a/strongMan/apps/pools/views/EditHandler.py b/strongMan/apps/pools/views/EditHandler.py
index a56fe365e9f6..34560d16d3ab 100644
--- a/strongMan/apps/pools/views/EditHandler.py
+++ b/strongMan/apps/pools/views/EditHandler.py
@@ -42,20 +42,17 @@ class EditHandler(object):
                     messages.add_message(self.request, messages.ERROR,
                                          'Won\'t update: Attribute values unclear for Attribute "None"')
                     return render(self.request, 'pools/edit.html', {"form": self.form})
-                vici_pool = {self.form.my_poolname: {'addrs': self.form.my_addresses}}
-
             else:
                 if self.form.my_attributevalues == "":
                     messages.add_message(self.request, messages.ERROR,
                                          'Won\'t update: Attribute values mandatory if attribute is set.')
                     return render(self.request, 'pools/edit.html', {"form": self.form})
-                vici_pool = {self.form.my_poolname: {'addrs': self.form.my_addresses,
-                                                     self.form.my_attribute: [self.form.my_attributevalues]}}
             msg = 'Successfully updated pool'

             try:
-                vici.load_pool(vici_pool)
                 self.form.update_pool(self.pool)
+                self.pool.clean()
+                vici.load_pool(self.pool.dict())
                 self.pool.save()
                 messages.add_message(self.request, messages.SUCCESS, msg)
             except ViciException as e:
theko2fi commented 9 months ago

Hey @tobiasbrunner sorry I completely forgot to make feedback here.

Your solution works like a charm. Thanks sooo much. This issue can be closed as completed.

image

tobiasbrunner commented 9 months ago

I've pushed my changes to master.