ansible/tacacs_plus

Accounting implementation

crisidev opened this issue · 2 comments

Hello,
I have worked on a patch to implement the accounting part of TAC+ protocol and I would like to submit the PR. So far I have implemented only the Start part of accounting and tested against Shrubbery TAC+. I am planning to work also on Continue and Stop.

I have some questions for you:

  1. Code structure: the code is in a big single file, making the development and testing quite complex. Is there a reason for it? The first PR I would like to submit would be to split the project and the tests into a modular fashion like:
tac_plus/__init__.py - version and global constanst
tac_plus/flags.py - all the protocol flags
tac_plus/packet.py - packet object and helpers
tac_plus/authentication.py - authentication object and helpers
tac_plus/authorisation.py - authorisation object and helpers
tac_plus/accounting.py - accounting object and helpers
tac_plus/aaa.py - send and receive methods, using object from above files
tests/test_tac_plus.py - tests, to be compliant with pytest and usable with python setup.py test
  1. Accounting patch: after the code is more structured, I would like to post a PR for the accounting part, to avoid a too big PR where we can miss things.

Thoughts?

@crisidev seems pretty reasonable to me; the reason this all ended up in one single module is because it originally was only the authentication implementation.

@ryanpetrello cool! I will send out the first PRs later next week. I don't think there anything else to discuss here. Fell free to close the issue.