Icinga / icingaweb2-module-director

The Director aims to be your new favourite Icinga config deployment tool. Director is designed for those who want to automate their configuration deployment and those who want to grant their “point & click” users easy access to the configuration.
https://icinga.com/docs/director/latest
GNU General Public License v2.0
413 stars 203 forks source link

[dev.icinga.com #12441] Use Icinga 2 API /v1/variables for fetching constants #402

Open icinga-migration opened 8 years ago

icinga-migration commented 8 years ago

This issue has been migrated from Redmine: https://dev.icinga.com/issues/12441

Created by mfriedrich on 2016-08-15 13:48:38 +00:00

Assignee: tgelf Status: Assigned Target Version: (none) Last Update: 2016-08-30 14:41:55 +00:00 (in Redmine)


Currently the director requires the API permission for "execute-script" in order to fetch global variables in getConstants().

Icinga 2 v2.5 adds the API endpoint /v1/variables in #11955 which allows to fetch all available constants.

Please consider changing this in the next release.


Relations:

icinga-migration commented 8 years ago

Updated by mfriedrich on 2016-08-15 13:48:51 +00:00

icinga-migration commented 8 years ago

Updated by tgelf on 2016-08-23 15:32:22 +00:00

Requires some testing and compatibility checks of course, as we can not immediately drop support for v2.4.x. But sure, why not. Out of curiosity, is there anything wrong with "execute-script"? Director Documentation explicitly asks for wildcard ("*") permissions. Further restrictions do not make any sense as long as an ApiUser is allowed to deploy config packages. Those would always allow one to work around restrictions by deploying an additional ApiUser object.

Please be aware that basing Directors CoreApi::getConstants() on /v1/variables would not lead to a removal of ::runConsoleCommand(), being the Directors "abstraction" for console/execute-script. There is a bunch of other features that should be based on this. So in case you have any objections here please let me know.

Cheers, Thomas

icinga-migration commented 8 years ago

Updated by tgelf on 2016-08-23 16:05:00 +00:00

icinga-migration commented 8 years ago

Updated by mfriedrich on 2016-08-24 13:57:11 +00:00

Granting permissions for anything might cause trouble when they are not properly handled through the Director interface itself. E.g. if you are putting it into public domain and one could run his/her own console commands (not only how variables are fetched). At first glance the Director only required the config package permissions but requiring "*" is similar to granting root to your monitoring core. I'm not sure if that opens possible security holes (we're human beings and make mistakes).

Afaik this topic also came up with the project Sebastian/Achim are currently building. Hardening the things external users might do with your Icinga 2 core through a web application mainly. Probably best to ask them directly..

Nevertheless the original issue is about using the API endpoint /v1/variables as stable interface.

icinga-migration commented 8 years ago

Updated by tgelf on 2016-08-26 14:29:01 +00:00

dnsmichi wrote:

Granting permissions for anything might cause trouble when they are not properly handled through the Director interface itself.

As discussed elsewhere this won't change much as with config/package permissions one could easily extend it's own permissions.

Nevertheless the original issue is about using the API endpoint /v1/variables as stable interface.

Absolutely, I just wanted to clarify this as you explicitly named the permission as a motivation for this change request. There is a blocking issue attached that requires some love, I'll postpone this unless that one is has been implemented.

icinga-migration commented 8 years ago

Updated by mfriedrich on 2016-08-30 14:29:40 +00:00

As discussed offline, the main issue is to use the /v1/variables endpoint when the version detection is implemented.

icinga-migration commented 8 years ago

Updated by tgelf on 2016-08-30 14:41:55 +00:00

Thanks! This needs to be implemented with no urgency as users wouldn't notice the difference and is currently blocked by #12520. That one requires some love and should be implemented before Icinga v2.6.

dnsmichi commented 6 years ago

During code analysis in icinga/icinga2#6428, I've stumbled of the code parts again which require console permissions for fetching constants. Would be nice if such could be moved to just use /v1/variables since Director now officially supports >= 2.6. cc @lazyfrosch