[fix] Make parameter tls_cipher an array #349

[okraits]

Fixes #349

Checklist

• [X] I have read the OpenWISP Contributing Guidelines.
• [X] I have manually tested the changes proposed in this pull request.
• [x] I have written new test cases for new code and/or updated existing tests for changes to existing code.
• [ ] I have updated the documentation.

Reference to Existing Issue

Closes #349.

Description of Changes

Made the parameter tls_cipher an array and updated the documentation accordingly. There were no tests to update.

[coveralls]

coverage: 99.179% (+0.001%) from 99.178% when pulling d2077a6735c3ddfb5bf0535e15aaf7e3c84c7620 on tls-cipher-list into 777ddfbd2285041f74b72431341b2cccb26111e8 on master.

[okraits]

@okraits this change would be backward incompatible. Why is it needed?

Can you provide an example of a value that you can't supply now and you'd be able to supply with the list format?

I gave an example and the reasoning in the related issue.

[okraits]

What about avoiding the schema change and change the code internally to convert the string to a list with 1 element so that it's rendered as a list?

I think this would be an appropriate solution as well.

Is the problem just the rendering of UCI option vs UCI list? Or do we actually need to allow multiple lines with different values?

Rendering the parameter as an UCI list is required for the parameter to work. In the LuCI OpenVPN app it's possible to create multiple list items with different values but I think for most usecases of netjsonconfig it would be sufficient to have one list item.

[nemesifier]

What about avoiding the schema change and change the code internally to convert the string to a list with 1 element so that it's rendered as a list?

I think this would be an appropriate solution as well.

We can do this here: https://github.com/openwisp/netjsonconfig/blob/master/netjsonconfig/backends/openwrt/converters/openvpn.py

We need two tests:

• classic conversion to NetJSON to UCI, probably modifying existing tests is going to be enough
• backward conversion from UCI to NetJSON, not sure if we can modify an existing one or we need to add a new one

Is the problem just the rendering of UCI option vs UCI list? Or do we actually need to allow multiple lines with different values?

Rendering the parameter as an UCI list is required for the parameter to work. In the LuCI OpenVPN app it's possible to create multiple list items with different values but I think for most usecases of netjsonconfig it would be sufficient to have one list item.

Ok so it sounds to methat handling this internally it's the best option as it's just an output issue.

[okraits]

@nemesifier I implemented the change as suggested. Do you think we need more or other tests?

[okraits]

@nemesifier Any opinion on this?