Add api_info and api_modify modules #91

felixfontein · 2022-05-14T21:48:55Z

SUMMARY

This adds two new modules, api_info and api_modify, which support querying and modifying certain paths.

The modules provide a high-level interface and only support some paths. The API information included is based on what I gathered from RouterOS 6.49. I'd like to have a better way to gather this data, but haven't been able to look into that yet.

The api_modify module should be able to at least solve some of the problems mentioned in #33.

ISSUE TYPE

New Module Pull Request

COMPONENT NAME

api_info
api_modify

codecov · 2022-05-14T21:50:18Z

Codecov Report

Merging #91 (f783df2) into main (7973f58) will increase coverage by 2.70%.
The diff coverage is 90.79%.

❗ Current head f783df2 differs from pull request most recent head 6353f21. Consider uploading reports for the commit 6353f21 to get more accurate results

@@            Coverage Diff             @@
##             main      #91      +/-   ##
==========================================
+ Coverage   83.78%   86.48%   +2.70%     
==========================================
  Files          25       29       +4     
  Lines        2491     3507    +1016     
  Branches      432      656     +224     
==========================================
+ Hits         2087     3033     +946     
- Misses        315      348      +33     
- Partials       89      126      +37

Impacted Files	Coverage Δ
plugins/modules/api.py	`77.82% <ø> (ø)`
plugins/modules/api_facts.py	`77.52% <ø> (ø)`
plugins/modules/api_find_and_modify.py	`91.42% <ø> (ø)`
plugins/modules/api_modify.py	`79.57% <79.57%> (ø)`
plugins/modules/api_info.py	`88.88% <88.88%> (ø)`
plugins/module_utils/_api_data.py	`100.00% <100.00%> (+4.08%)`	⬆️
tests/unit/plugins/modules/fake_api.py	`81.92% <100.00%> (+13.63%)`	⬆️
tests/unit/plugins/modules/test_api_info.py	`100.00% <100.00%> (ø)`
tests/unit/plugins/modules/test_api_modify.py	`100.00% <100.00%> (ø)`

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 7973f58...6353f21. Read the comment docs.

felixfontein · 2022-05-14T21:50:21Z

Example:

- community.routeros.api_modify:
    path: ip dns static
    data:
      - address: 192.168.88.1
        name: router
      - address: 192.168.88.2
        name: server1
      - address: 192.168.88.3
        name: server2
    handle_absent_entries: remove
    handle_entries_content: remove
    ensure_order: true

This will ensure that /ip dns static has extractly three entries in the specified order of exactly the specified format.

NikolayDachev · 2022-05-22T21:34:51Z

I'm not sure why choices: list must be in docs ? Is there anything specifics for those paths ? what I miss ?

NikolayDachev · 2022-05-22T21:44:38Z

As general I absolutely like it :D

More questions

This NAT to WAN example .. make no sense to me, Also I get error for "msg": "Unknown key \"name\" at index 1." which is ok since I'm not sure if ip firewall nat .. have attribute name ?!

The example from this PR make more sense (about DNS static entries )

From code prospective I think is ok

felixfontein · 2022-05-23T10:20:22Z

It should have been comment, not name in that example... I've updated it and added another example.

choices: is kind of needed since the modules only support some pathes, namely the ones they "know how they work" and for which they have information in fields that exist, default values, etc.

I have a one note here.

Please test it with RouterOS 7 ! Some paths are changed

Also if this affect " /interface/wireless" for example with routeros pkg wifiwave2 (if is installed) this path is changed to '/interface/wifiwave2'

It is not a big issue just for information

Overall I think we can merge this request !

NikolayDachev

Look ok to be merged in main

NikolayDachev · 2022-05-23T11:52:03Z

note: Probably rebase will be needed today I merge "extended-query"

felixfontein · 2022-05-23T16:02:06Z

Things this PR is still missing:

A way to get hold of the API information. Right now it's manually compiling everything, but that's not really efficient and will likely cause problems already with RouterOS 7. Also there are quite some paths that aren't fully implemented yet due to me not knowing how to correctly support them, how exactly they work, etc.
Having better data, like some options only work when other options have certain values, etc. Might also be related to the above point.
Ordering for paths with stratified keys seems to have problems (maybe also for paths with primary keys?). For some firewall rules I had to run the module several times until they finally had the correct order. I'll need to debug this more to figure out what causes this and how to fix it.
Not related directly to this PR, but a common problem for all api* modules: using non-ASCII characters. For example I had an ü in a comment for my home router, and the module crashed when trying to send that comment to RouterOS since it didn't like that letter. I was able to set it in the web UI though.

NikolayDachev · 2022-05-24T20:24:15Z

For firewall order (not directly related) from ros prospective

Let say we have

 0    chain=forward action=accept log=no log-prefix="" 
 1    chain=forward action=accept log=no log-prefix="" 
 2    chain=forward action=accept log=no log-prefix=""

If you want to change rule number 1 to action=drop set can be used

 /ip/firewall/filter/set action=drop numbers=1

result

 0    chain=forward action=accept log=no log-prefix="" 
 1    chain=forward action=drop log=no log-prefix="" 
 2    chain=forward action=accept log=no log-prefix=""

@felixfontein Not sure if this example can help, probably if you provide me more info ... ?

or with api module cmd

    - name: add ip address test
      community.routeros.api:
        hostname: "10.20.36.253"
        ssl: "false"
        password: "SUPERSECRETPWD"
        username: "admin"
        encoding: "utf-8"
        path: "ip firewall filter"
        cmd: "set action=drop numbers=1"

NikolayDachev · 2022-05-24T20:33:08Z

For " Having better data, like some options only work when other options have certain values, etc. Might also be related to the above point."

I guess you know some routeros paths haves sub path, so is normal some attributes to depend on other (specially with sub paths) .. if I understand correctly the problem ?

felixfontein · 2022-05-24T20:36:25Z

No, I meant more that for some values in one path some attributes only make sense in some case, like in /ip service, certificate and tls-version make sense for www-ssl and api-ssl, but not for www, api, ftp, ... There are more cases like this in other paths.

NikolayDachev · 2022-05-24T20:38:11Z

"Also there are quite some paths that aren't fully implemented yet due to me not knowing how to correctly support them, how exactly they work, etc."

Let's clear the problems her first on ROS level then we can see what we can do with API, let me know if you have a issues with something specific, probably I can help

Yes ROS7 don't have long term support version yet, we cannot predict every change so simple "disclaimer" in doc's will do the job for now , for the rest we can relay on module users feedback !

NikolayDachev · 2022-05-24T20:45:52Z

No, I meant more that for some values in one path some attributes only make sense in some case, like in /ip service, certificate and tls-version make sense for www-ssl and api-ssl, but not for www, api, ftp, ... There are more cases like this in other paths.

I think I understand the problem, unfortunately no clear solution,

"hardcoded check per case"
somehow routeros trap errors to be tracked .. probably
no idea really

note: I think rebase is needed ..

NikolayDachev · 2022-05-27T11:03:38Z

@felixfontein let me know if you need a help with something here

felixfontein · 2022-06-12T16:44:28Z

@felixfontein let me know if you need a help with something here

I was looking at how to automatically export information on the API structure from RouterOS recently (like which fields are supported for which path etc.), but I couldn't really find anything.

Some information seems to be available from the HTTP interface, like https://192.168.88.1/webfig/roteros.jg (base system) or https://192.168.88.1/webfig/wlan6.jg (there seems to be one such file per package that can be installed). This is unfortunately not JSON, but something a JavaScript interpreter understands, and I suspect it contains some information needed to render the web UI. But it doesn't seem to be everything, it probably works together with other internal data.

That's probably the most important thing left...

NikolayDachev · 2022-06-15T04:42:08Z

Ros7 have REST api support, if you need something specific probably can look it with postman, I guess rest and routeros api are similar .

Not sure if api structure can be dump in both cases

github-actions · 2022-07-18T21:32:49Z

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

felixfontein · 2022-07-19T05:55:18Z

@NikolayDachev I haven't been able to really work on this, but I also don't see how to actually solve the remaining problems. What do you think of simply declaring this as "finished" for now and merging it and releasing it, so folks can start trying it out and maybe even extending/improving/... it for the use cases they have?

NikolayDachev · 2022-07-19T06:03:27Z

No problems, just we should mark somehow what is working and what not in doc.

felixfontein · 2022-07-19T06:08:29Z

I'll update the docs and mark this as ready once I've pushed that. Might not happen today, but very soon :)

felixfontein · 2022-07-29T09:47:56Z

@NikolayDachev what do you think of the formulation I pushed?

NikolayDachev · 2022-07-29T18:39:48Z

I will try to find a time during the weekend

NikolayDachev · 2022-07-31T07:00:20Z

Looking good, just check my suggestion in code review .. I don't think is something really special.
Depend on your decision .. overall we can merge if no future changes are needed.

felixfontein · 2022-07-31T09:12:08Z

@NikolayDachev in case you had a suggestion, you didn't add it to the review :)

docs/docsite/rst/api-guide.rst

NikolayDachev · 2022-07-31T09:19:36Z

@NikolayDachev in case you had a suggestion, you didn't add it to the review :)

sorry was in pending state

felixfontein · 2022-07-31T10:50:09Z

Ok, ready for another round of review :)

NikolayDachev

look good to go

felixfontein · 2022-07-31T20:06:54Z

Then let's do this! Thanks a lot for reviewing and testing this, @NikolayDachev!
I'll create a new release probably tomorrow, so folks can more easily try this out.

felixfontein mentioned this pull request May 14, 2022

New task module for configuring #33

Open

felixfontein force-pushed the api_info-api_modify branch 4 times, most recently from edf4b84 to 59b7b9f Compare May 15, 2022 13:48

felixfontein mentioned this pull request May 15, 2022

Improve unit test performance #92

Merged

felixfontein force-pushed the api_info-api_modify branch 2 times, most recently from 67b6e07 to 067003f Compare May 22, 2022 09:59

felixfontein mentioned this pull request May 22, 2022

Add api_find_and_modify module #93

Merged

NikolayDachev approved these changes May 23, 2022

View reviewed changes

felixfontein force-pushed the api_info-api_modify branch from 0fa1438 to 4c2ef41 Compare May 23, 2022 15:54

felixfontein mentioned this pull request May 24, 2022

Add encoding option to API modules #95

Merged

felixfontein force-pushed the api_info-api_modify branch 3 times, most recently from 2443a48 to cf7876b Compare May 24, 2022 20:20

felixfontein force-pushed the api_info-api_modify branch from cf7876b to 84d9a48 Compare May 25, 2022 04:48

felixfontein force-pushed the api_info-api_modify branch from 33f232a to cc296e4 Compare June 12, 2022 15:22

felixfontein added 6 commits July 18, 2022 23:17

Add api_info and api_modify modules.

7fb35cb

Add documentation, and cross-references for the modules.

4316b45

Bump version added for new modules.

594a769

Ignore dynamic entries. Allow _info module to return them.

78521a2

Document behavior for dynamic entries.

42c0097

Update license headers.

9ca249d

felixfontein force-pushed the api_info-api_modify branch from 9f8ff6e to 9ca249d Compare July 18, 2022 21:18

Add disclaimer.

0c4cae8

felixfontein changed the title ~~[WIP] Add api_info and api_modify modules~~ Add api_info and api_modify modules Jul 31, 2022

NikolayDachev reviewed Jul 31, 2022

View reviewed changes

docs/docsite/rst/api-guide.rst Outdated Show resolved Hide resolved

felixfontein added 3 commits July 31, 2022 11:43

Improve formulation.

6c1b859

Auto-generate path list in DOCUMENTATION.

8b3d074

Add test to make sure lists have been generated automatically.

6353f21

felixfontein force-pushed the api_info-api_modify branch from f783df2 to 6353f21 Compare July 31, 2022 10:49

NikolayDachev approved these changes Jul 31, 2022

View reviewed changes

felixfontein merged commit 2911710 into ansible-collections:main Jul 31, 2022

felixfontein deleted the api_info-api_modify branch July 31, 2022 20:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add api_info and api_modify modules #91

Add api_info and api_modify modules #91

felixfontein commented May 14, 2022

codecov bot commented May 14, 2022 •

edited

Loading

felixfontein commented May 14, 2022

NikolayDachev commented May 22, 2022

NikolayDachev commented May 22, 2022

felixfontein commented May 23, 2022 •

edited by NikolayDachev

Loading

NikolayDachev left a comment

NikolayDachev commented May 23, 2022

felixfontein commented May 23, 2022 •

edited

Loading

NikolayDachev commented May 24, 2022 •

edited

Loading

NikolayDachev commented May 24, 2022

felixfontein commented May 24, 2022

NikolayDachev commented May 24, 2022

NikolayDachev commented May 24, 2022 •

edited

Loading

NikolayDachev commented May 27, 2022

felixfontein commented Jun 12, 2022

NikolayDachev commented Jun 15, 2022

github-actions bot commented Jul 18, 2022 •

edited

Loading

felixfontein commented Jul 19, 2022

NikolayDachev commented Jul 19, 2022

felixfontein commented Jul 19, 2022

felixfontein commented Jul 29, 2022

NikolayDachev commented Jul 29, 2022

NikolayDachev commented Jul 31, 2022

felixfontein commented Jul 31, 2022

NikolayDachev commented Jul 31, 2022

felixfontein commented Jul 31, 2022

NikolayDachev left a comment

felixfontein commented Jul 31, 2022

Add api_info and api_modify modules #91

Add api_info and api_modify modules #91

Conversation

felixfontein commented May 14, 2022

SUMMARY

ISSUE TYPE

COMPONENT NAME

codecov bot commented May 14, 2022 • edited Loading

Codecov Report

felixfontein commented May 14, 2022

NikolayDachev commented May 22, 2022

NikolayDachev commented May 22, 2022

felixfontein commented May 23, 2022 • edited by NikolayDachev Loading

NikolayDachev left a comment

Choose a reason for hiding this comment

NikolayDachev commented May 23, 2022

felixfontein commented May 23, 2022 • edited Loading

NikolayDachev commented May 24, 2022 • edited Loading

NikolayDachev commented May 24, 2022

felixfontein commented May 24, 2022

NikolayDachev commented May 24, 2022

NikolayDachev commented May 24, 2022 • edited Loading

NikolayDachev commented May 27, 2022

felixfontein commented Jun 12, 2022

NikolayDachev commented Jun 15, 2022

github-actions bot commented Jul 18, 2022 • edited Loading

Docs Build 📝

felixfontein commented Jul 19, 2022

NikolayDachev commented Jul 19, 2022

felixfontein commented Jul 19, 2022

felixfontein commented Jul 29, 2022

NikolayDachev commented Jul 29, 2022

NikolayDachev commented Jul 31, 2022

felixfontein commented Jul 31, 2022

NikolayDachev commented Jul 31, 2022

felixfontein commented Jul 31, 2022

NikolayDachev left a comment

Choose a reason for hiding this comment

felixfontein commented Jul 31, 2022

codecov bot commented May 14, 2022 •

edited

Loading

felixfontein commented May 23, 2022 •

edited by NikolayDachev

Loading

felixfontein commented May 23, 2022 •

edited

Loading

NikolayDachev commented May 24, 2022 •

edited

Loading

NikolayDachev commented May 24, 2022 •

edited

Loading

github-actions bot commented Jul 18, 2022 •

edited

Loading