Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/new deployment hook #2790

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

moreamazingnick
Copy link
Contributor

@moreamazingnick moreamazingnick commented Aug 24, 2023

refs #2788

@Thomas-Gelf @lippserd
I would like to propose a new deploymenthook function to make changes to the files written to disk.

In addition I would like to propose a BeforeStoreIcingaObjectHook, which allows changes to an IcingaObject before written to the Database.

Please reach out to me if any changes are necessary.
I think linuxfabrik could make use of the deploymenthook function as well.

PullRequest will be created soon.

Best Regards
Nicolas

@cla-bot cla-bot bot added the cla/signed label Aug 24, 2023
@Thomas-Gelf
Copy link
Contributor

Hi @moreamazingnick,

I have some problems with this PR, and cannot merge it as is. Main problems:

  • tweaking config files AFTER their checksums have been calculated and persisted is a very bad idea. Please explain your motivation behind this. If this is about shipping additional configuration, not controlled by the Director, then the ShipConfigFilesHook could be what you're looking for. If you want to have an influence on rendering behaviour, a more specific Hook should be found
  • allowing to modify every object after all validation took place also doesn't make me feel comfortable. Of course it gives your implementation super powers, and this sounds promising. But it makes future development and major changes incredibly difficult, as we can no longer trust in our objects being stored as given. Lots of modules might implement black magic, and they might even conflict, depending on execution order.

This all also might result in undesired side-effects related to caching mechanisms, Apply Rule calculation and more. If you want to tackle this, please provide related real-world examples, which led to your requirements. I'm sure we'll find a better solution, to address them.

Cheers,
Thomas

@Thomas-Gelf Thomas-Gelf self-requested a review October 10, 2023 09:27
Copy link
Contributor

@Thomas-Gelf Thomas-Gelf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot be merged as-is, please address the points in my comment

@moreamazingnick
Copy link
Contributor Author

Hi @Thomas-Gelf

Thanks for the reply.

With these hooks I created something like this in a non prod environment, just a proove of concept:

  • Create a Field that has the ability to wrap a passsword. PREENC()
  • Since fieldnames are not unique, I don't know, which field is an encrypted field so I need something like this "modify every object"-hook to encrypt everything that matches PREENC(.+?)
  • Encrypt the fieldcontent and change it to ENC(xxx)
  • Since icinga2 cannot make use of my decryption feature I need to write it to disk unencryted so I need the "tweaking config files AFTER"-hook and replace the ENC(xxx) with the clear text password.

Why I think it is a good idea:

  • It provides the possibility to use PREENC(secretxy) in api too.
  • no passwords are leaked in any way using director web gui
  • passwords are "protected" in director database, backups, dumps
    • since the director writes configfiles in plaintext to the db this has to be done after checksum creation so the secrets are still protected in the database

It sadly moves the problem to the icinga2 infrastructure but the director database is somehow secured.

here a module that makes use of these hooks:
https://github.com/moreamazingnick/directorencrypter

other ideas where the"modify every object"-hook would help:

Best Regards Nicolas

@lippserd lippserd added the dev-call Issues and Pull Requests to be discussed at the Dev Call. label Aug 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla/signed dev-call Issues and Pull Requests to be discussed at the Dev Call.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants