sighupio / furyagent

Apache License 2.0
9 stars 2 forks source link

implemented list flag for getting list of openvpn certs #18

Closed lzecca78 closed 4 years ago

lzecca78 commented 4 years ago

As per requested Trello card: https://trello.com/c/hvFUappl/116-furyagent-list-current-vpn-certificates i've implemented it:

go run main.go --config=ssh/furyagent.yml configure openvpn-client --list
2020-03-19 17:09:00.727031 I | storage.go:146: Item pki/vpn-client/revoked/luca.zecca.crt found [size: 1103]
2020-03-19 17:09:00.727195 I | storage.go:147: Saving item pki/vpn-client/revoked/luca.zecca.crt ...
2020-03-19 17:09:00.830450 I | storage.go:146: Item pki/vpn-client/simone.messina.crt found [size: 1107]
2020-03-19 17:09:00.830470 I | storage.go:147: Saving item pki/vpn-client/simone.messina.crt ...
2020-03-19 17:09:00.948095 I | storage.go:146: Item pki/vpn/ca.crl found [size: 597]
2020-03-19 17:09:00.948113 I | storage.go:147: Saving item pki/vpn/ca.crl ...
2020-03-19 17:09:01.046877 I | storage.go:146: Item pki/vpn/ca.crl found [size: 597]
2020-03-19 17:09:01.046893 I | storage.go:147: Saving item pki/vpn/ca.crl ...
+----------------+------------+------------+---------+--------------------------------+
|      USER      | VALID FROM |  VALID TO  | EXPIRED |            REVOKED             |
+----------------+------------+------------+---------+--------------------------------+
| luca.zecca     | 2020-03-19 | 2021-03-19 | false   | true 2020-03-19 14:47:40 +0000 |
|                |            |            |         | UTC                            |
+----------------+------------+------------+---------+--------------------------------+
| simone.messina | 2020-03-19 | 2021-03-19 | false   | false 0001-01-01 00:00:00      |
|                |            |            |         | +0000 UTC                      |
+----------------+------------+------------+---------+--------------------------------+

This is the output of the terminal. Let me know your feedback! 🙏

lzecca78 commented 4 years ago

@ralgozino you're right: here the implementation:

 go run main.go --config=ssh/furyagent.yml configure openvpn-client --list --output=json
2020-03-19 18:31:55.722874 I | storage.go:146: Item pki/vpn-client/revoked/luca.zecca.crt found [size: 1103]
2020-03-19 18:31:55.723021 I | storage.go:147: Saving item pki/vpn-client/revoked/luca.zecca.crt ...
2020-03-19 18:31:55.815336 I | storage.go:146: Item pki/vpn-client/simone.messina.crt found [size: 1107]
2020-03-19 18:31:55.815359 I | storage.go:147: Saving item pki/vpn-client/simone.messina.crt ...
2020-03-19 18:31:55.939013 I | storage.go:146: Item pki/vpn/ca.crl found [size: 597]
2020-03-19 18:31:55.939038 I | storage.go:147: Saving item pki/vpn/ca.crl ...
2020-03-19 18:31:56.035563 I | storage.go:146: Item pki/vpn/ca.crl found [size: 597]
2020-03-19 18:31:56.035588 I | storage.go:147: Saving item pki/vpn/ca.crl ...
{"User":"simone.messina","Valid_from":"2020-03-19","Valid_to":"2021-03-19","Expired":false,"Revoked":{"Revoked":false,"RevokeTime":"0001-01-01T00:00:00Z"}}
lzecca78 commented 4 years ago

sorry, errata corrige :

go run main.go --config=ssh/furyagent.yml configure openvpn-client --list --output=json
2020-03-19 18:37:25.204840 I | storage.go:146: Item pki/vpn-client/revoked/luca.zecca.crt found [size: 1103]
2020-03-19 18:37:25.204988 I | storage.go:147: Saving item pki/vpn-client/revoked/luca.zecca.crt ...
2020-03-19 18:37:25.314691 I | storage.go:146: Item pki/vpn-client/simone.messina.crt found [size: 1107]
2020-03-19 18:37:25.314715 I | storage.go:147: Saving item pki/vpn-client/simone.messina.crt ...
2020-03-19 18:37:25.432634 I | storage.go:146: Item pki/vpn/ca.crl found [size: 597]
2020-03-19 18:37:25.432655 I | storage.go:147: Saving item pki/vpn/ca.crl ...
2020-03-19 18:37:25.537314 I | storage.go:146: Item pki/vpn/ca.crl found [size: 597]
2020-03-19 18:37:25.537341 I | storage.go:147: Saving item pki/vpn/ca.crl ...
[{"User":"luca.zecca","Valid_from":"2020-03-19","Valid_to":"2021-03-19","Expired":false,"Revoked":{"Revoked":true,"RevokeTime":"2020-03-19T14:47:40Z"}},{"User":"simone.messina","Valid_from":"2020-03-19","Valid_to":"2021-03-19","Expired":false,"Revoked":{"Revoked":false,"RevokeTime":"0001-01-01T00:00:00Z"}}]
ralgozino commented 4 years ago

Awesome! Please don't forget to update the README :)

angelbarrera92 commented 4 years ago

The implementation looks awesome, both internals and user experience. To finalize the implementation, please modify this repo README.md document and i will copy paste it to the documentation site.

Once done, we can proceed with the release. Again, awesome work.

lzecca78 commented 4 years ago

@lnovara i didn't get your previous request

lzecca78 commented 4 years ago

@lnovara , you are right was missing annotations :), added them

lzecca78 commented 4 years ago

@lnovara for what i dig, i used Flag and not PersistedFlag, so it is related to the --list. For the rest of the parameters, is not something related to this pull request imho, is something that need to be addressed elsewhere.

lzecca78 commented 4 years ago

openvpn related task has really more problems than mix of flag parameters: you must have openvpn binary installed, no documentation at all on how use it( i personally have a lot of difficulties to understand how to setup my working environment and furyagent.yml), a lot of certificate complexity and so on. I guess that these should the priorities compare to the mixing flag problems of a command line that - at the end - we use only.

lnovara commented 4 years ago

Feel free to open a PR to add what you've found to the documentation.

angelbarrera92 commented 4 years ago

Ok, what if we open a new trello card to address pending points (refactor or whatever) meanwhile we deliver new features to customers?

I think this is an awesome feature that has to be delivered asap ;)

lzecca78 commented 4 years ago

can this pr be merged? Or are we still waiting for some kind of review/ bugfix?

ralgozino commented 4 years ago

This can't be merged until at least the readme has been updated with the changes introduced by the PR. As you have pointed out, it is not good to have undocumented stuff, we must at least don't add even more undocumented features.

OTOH, @angel is doing a great job with the docs site, have you taken a look to the CLI section? If not please do and help us improve it.

lzecca78 commented 4 years ago

@ralgozino you're right. Even if adding 1 documentation flag of the thousand that furyagent has i don't think that change the situation. I am only adding a flag to something that was already done and merged without any documentation. Dura lex sed lex. And this basically means that if this rule (that you are imposing and could be reasonable, i don't say that) exists, should be a standard for anyone has made a PR previous than mine. And this is not case. I will add documentation. Let's see what will happen afterwards :)

lzecca78 commented 4 years ago

added documentation of certificate list feature @ralgozino

angelbarrera92 commented 4 years ago

Merging!