Netcentric / accesscontroltool

Rights and roles management for AEM made easy
Eclipse Public License 1.0
149 stars 92 forks source link

Execute Startup Hook always after Pipeline Deployment #748

Open kwin opened 1 month ago

kwin commented 1 month ago

Currently there is a simple logic in the Startup Hook which prevents it from being executed with each start of the Kubernetes pod

To avoid overhead for the case a configuration has already been applied, an MD5 checksum is created over all configuration files and the configuration is only applied for the case the checksum has changed.

(https://github.com/Netcentric/accesscontroltool/blob/develop/docs/ApplyConfig.md#startup-hook)

Unfortunately that does also prevent reinstalling/cleaning up the relevant ACLs/authorizables from the YAML config after each Cloud Manager deployment

The installation incorrectly does not occur in case the YAML configuration was not touched but

  1. content has been modified which is being looped over with the YAML configuration or
  2. ACEs have been manually modified/deleted/created which affect authorizables managed by ACTool
  3. Authorizables have been manually modified/deleted which are managed by ACTool
  4. Newer version of ACTool has been installed which treats config files differently
  5. OSGi configuration changed which has an impact on ACTool execution (e.g. configuration PID biz.netcentric.cq.tools.actool.ims.IMSUserManagement or biz.netcentric.cq.tools.actool.impl.AcInstallationServiceImpl)

Instead of leveraging a hash over the config files to check if reinstallation is necessary one should rely on something else.

kwin commented 1 month ago

The question is how to reliably detect if the last execution of the ACTool was based on a different Cloud Manager build or the same. Potential candidates are:

  1. An Environment variable (not documented by Adobe, therefore may change in the future)
  2. A check over all bundles to get the most recent modification date (Unclear if those dates change when a pod is restarted without a build)
  3. The POM version modified by the pipeline build (Unclear how to identify a bundle having that version from the POM)
kwin commented 1 month ago

Probably just checking if the previous log was a triggered via startup_hook_image_build is enough.

ghenzler commented 1 month ago

Regarding the reasons to run the tool automatically on mutable content after each deployment:

  1. content has been modified which is being looped over with the YAML configuration or

this can happen, the question is if the deployment event is the best time to react on such changes (conceivable is also to create listeners for children-changes of all content-derived loops and then automatically run the AC tool without deployment). But then again, usually the most straight forward is not to have content-derived loops but rather define the root folders as master in the AC tool with initialContent

  1. ACEs have been manually modified/deleted/created which affect authorizables managed by ACTool
  2. Authorizables have been manually modified/deleted which are managed by ACTool

I would assume that if manual interaction is allowed (it normally should not be allowed), then it's also ok to "reset to AC Tool config" by running the AC Tool manually

Probably just checking if the previous log was a triggered via startup_hook_image_build is enough.

yes that could be an option if we want to change the behaviour - downside is that usually there is no change to be applied if the config files don't change (so the AC tool will run often in vain, especially for established platforms where the yaml files and root folders rarely change)

kwin commented 1 month ago

if the deployment event is the best time to react on such changes

It happens like this with AEM 6.5, therefore I would first like to align both versions before coming up with a more sophisticated option (listeners always mean quite some overhead)

then it's also ok to "reset to AC Tool config" by running the AC Tool manually

It is not so much about the additional manual step but about the principle of least surprise which is IMHO violated here.