fix(api): fixed a bug when splitting the argument string

[akimrx]

SUMMARY

Closes #44

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

routeros_api

[akimrx]

All tests passed in my repo, but you can check again

[codecov]

Codecov Report

Merging #45 (0f8ce99) into main (48c7920) will decrease coverage by 0.06%.
The diff coverage is 80.45%.

@@            Coverage Diff             @@
##             main      #45      +/-   ##
==========================================
- Coverage   80.13%   80.06%   -0.07%     
==========================================
  Files          11       11              
  Lines        1223     1304      +81     
  Branches      161      181      +20     
==========================================
+ Hits          980     1044      +64     
- Misses        184      193       +9     
- Partials       59       67       +8     

Impacted Files	Coverage Δ
plugins/modules/api.py	71.87% <77.02%> (+1.13%)	⬆️
tests/unit/plugins/modules/test_api.py	98.61% <100.00%> (+0.08%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 48c7920...0f8ce99. Read the comment docs.

[felixfontein]

Related PR: #47

[heuels]

Looks good to me!

Although I haven't touched Python in a while now, @felixfontein may be a better judge here 🙂

[NikolayDachev]

@akimrx Thanks a lot for the fix

Initial testing look ok, however I want to do more deep testing with arbitary cmd, query etc..

[felixfontein]

Before discussing on whether this should we merged, we should really discuss the underlying questions in #44 which need to be answered before we can decide whether this PR actually provides a correct fix.

[NikolayDachev]

@akimrx can you please merge after #47 is committed.

[felixfontein]

@akimrx I've rebased your branch.

[felixfontein]

I've added #44 (comment) + a changelog fragment to this PR in 3bc446c.

[NikolayDachev]

Thanks again for the work here,

note: We should update module documentation about strings with '""' for users which define it as variables , no sure how to define it but

ros_ip_addr_comment: '"my long string"' -> correct
ros_ip_addr_comment: "'my long string'" -> wrong
ros_ip_addr_comment: "my long string" -> wrong
ros_ip_addr_comment: 'my long string' -> wrong

[felixfontein]

It's probably a good idea to simplify this in some ways:

1. Provide Jinja2 filters to quote and unquote parameters.
2. Allow to pass in parameters as a dictionary to the api module, so you don't need to encode everything as a string first.

[NikolayDachev]

Ideally we should be able to use | as this example without escape symbols

{{ ros_backup_script_withes }} - work (tested with current code pull/45/head )
{{ ros_backup_script_noes }} - we want to work as well (if is possible ! )

I'm not sure how correctly to utilize

ros_backup_script_noes: |
  {%raw%} 
  .. 
  {%endraw%}

  hosts: ros_testing
  gather_facts: no
  connection: local

  tasks:
  - name: add script
    vars:
      ros_backup_script_withes: '"
        :local host value=[/system identity get name];
        :local date value=[/system clock get date];
        :local day [ :pick $date 4 6 ];
        :local month [ :pick $date 0 3 ];
        :local year [ :pick $date 7 11 ];
        :local name value=($host.\"-\".$day.\"-\".$month.\"-\".$year);
        /system backup save name=$name;
        /export file=$name;
        /tool fetch address=192.168.1.1 user=ros password=\"PASSWORD\" mode=ftp dst-path=(\"/mikrotik/rsc/\".$name.\".rsc\") src-path=($name.\".rsc\") upload=yes;
        /tool fetch address=192.168.1.1 user=ros password=\"PASSWORD\" mode=ftp dst-path=(\"/mikrotik/backup/\".$name.\".backup\") src-path=($name.\".backup\") upload=yes;
        "'
      
      ros_backup_script_noes: |
        :local host value=[/system identity get name];
        :local date value=[/system clock get date];
        :local day [ :pick $date 4 6 ];
        :local month [ :pick $date 0 3 ];
        :local year [ :pick $date 7 11 ];
        :local name value=($host."-".$day."-".$month."-".$year);
        /system backup save name=$name;
        /export file=$name;
        /tool fetch address=192.168.1.1 user=ros password="PASSWORD" mode=ftp dst-path=("/mikrotik/rsc/".$name.".rsc") src-path=($name.".rsc") upload=yes;
        /tool fetch address=192.168.1.1 user=ros password="PASSWORD" mode=ftp dst-path=("/mikrotik/backup/".$name.".backup") src-path=($name.".backup") upload=yes;

    community.routeros.api:
      hostname: "{{ inventory_hostname }}"
      ssl: "false"
      password: "{{ vault_common_pwd }}"
      username: "admin"
      path: "system script"
      add:
        name="test"
        source={{ ros_backup_script_withes }}
    register: scriptout
    delegate_to: localhost
  
  - name: result ip address
    debug:
      msg: '{{ scriptout }}'

[NikolayDachev]

2. Allow to pass in parameters as a dictionary to the api module, so you don't need to encode everything as a string first.

well the idea is to be close as ros cmd, but both cases are good idea as well !

[NikolayDachev]

I believe we can submit the code (if there is no other objections )

For dictionary arguments will solve some problems which we have, however will open others, so I suggest to open separate pr for it.

[felixfontein]

I agree, let's merge this and then work on improvements in further PRs.

[felixfontein]

@akimrx thanks for your contribution!
@heuels @NikolayDachev thanks for reviewing!

[felixfontein]

@NikolayDachev with #53, you can do

  - community.routeros.api:
      hostname: "{{ inventory_hostname }}"
      ssl: "false"
      password: "{{ vault_common_pwd }}"
      username: "admin"
      path: "system script"
      add:
        name="test"
        source={{ ros_backup_script_noes | community.routeros.quote_argument_value }}
    register: scriptout
    delegate_to: localhost