Config: Make person and project services configurable per tenant

[ValentinFutterer]

Description

Refactoring / Config

What does this PR do?

Dont be scared by the amount of files touched, most are just adjustments made after config changes.

The heart of the PR is a change to config that allows for configuring persons and project services (and fields / title) per tenant and not once for the whole application. To do this, I grouped all config fields in the application.yaml under "tenant-aware" (if you have better naming suggestions, do tell). Then I used Quarkus @ConfigMapping to load the whole config as one config object. This allows for switching config objects by checking tenant ID at runtime. @ConfigProperty would not work, since they are loaded statically at runtime.

Multitenancy configuraton has now changed. You need to supply the tenants under the tenants key and supply ALL tenant-aware configs per tenant. These are then loaded and switched between dynamically per request.

Sadly, Quarkus has some strict opinions on how to load these configs, so I had to axe all environment variables that could load configs via json. This wasnt an easy decision, but Quarkus gotta be Quarkus. It actually parsed the json correctly, but then the validator complains...

Anyways this has lead to some more changes. The person services list now has to passed like this as env variables:

DAMAP_TENANT_AWARE_PERSON_SERVICES_0__DISPLAY_TEXT: University
DAMAP_TENANT_AWARE_PERSON_SERVICES_0__QUERY_VALUE: UNIVERSITY
DAMAP_TENANT_AWARE_PERSON_SERVICES_0__CLASS_NAME:org.damap.base.integration.mock.MockUniversityPersonServiceImpl

The _0__ is the list index, so for a second list element, you would increment to _1__.

DAMAP_TENANT_AWARE_ELSEVIER_PURE_CONTRIBUTOR_ROLE_CLASSIFICATIONS used to be a map with the pure role /dk/atira/pure/member as key. Since we cnanot parse json anymore, I tried to load it as env variables, but failed. Since the / are turned to _ as env variables, but _ could also be [ or ] or ., Quarkus cannot go from the env variable to the correct config element. Therefore I had to do an ugly workaround and also turn it into a list like this:

DAMAP_TENANT_AWARE_ELSEVIER_PURE_CONTRIBUTOR_ROLE_CLASSIFICATIONS_0__PURE_ROLE_URI: /dk/atira/pure/member
DAMAP_TENANT_AWARE_ELSEVIER_PURE_CONTRIBUTOR_ROLE_CLASSIFICATIONS_0__CONTRIBUTOR_ROLE: PROJECT_MEMBER

Please check in the code if the list elements are used correctly.

All of this of course broke the tests, since now they had to load the config differently, so I did a bunch of refactoring there.

Also, since we switch dynamically between tenants, I couldnt use a static Pure Client anymore. So I reused the PureApiFactory to generate dynamic PureClients per HTTP request based on the calling tenant + some caching to make the generation efficient. Please check this whole process also by using the console. E.g. give some bogus URL unique for each tenant and then check the error messages in the console.

I also added a mode for disabling project services, since some Unis wont connect any. The frontend gets the info in the config, but cant use it yet. (Will create an issue for that).

Documentation will follow on the Website at a later date.

I tried to update the Helm Chart too but I ran out of time. I did some stuff, but I am sure there is somehting wrong / missing. E.g. the loading of person services. Sorry in advance.

Breaking changes

Configstuff, very much breaking

Code review focus

Naming, did everything make sense. Does Pure still work? Testing on Kubernetes please.

closes GH-472

[SotosTsepe]

leftovers from dev? this shouldn't be commented out

[ValentinFutterer]

Yes good catch

[SotosTsepe]

this duplicate key naming can cause confusion. Maybe rename it to tenants.items?

[ValentinFutterer]

i renamed it to tenant-list

[SotosTsepe]

Solid effort on a first glance.
Main focus:

• The helm chart definitely needs a more thorough inspection, as the structure has changed quite a bit. There is additional nesting in the multitenancy config as well as several configuration naming changes, so we have to double-check that everything is still mapped correctly.
• Regarding the tenant-aware section: does introducing this level remove support for the previous single-tenant configuration (e.g. damap.title, damap.fields.*, etc.)?
  If so, this would also be a breaking change for non-multitenant deployments, as existing configs would need to be migrated to damap.tenant-aware.* .
• Not sure if the person services list now enforces each subitem to be read separately. In the Helm chart it was previously possible to pass the entire list as YAML and let the application parse it directly. If the new approach is mandatory, there may be scalability and maintainability concerns, as this could become cumbersome when the list grows.

[ValentinFutterer]

• The helm chart definitely needs a more thorough inspection, as the structure has changed quite a bit. There is additional nesting in the multitenancy config as well as several configuration naming changes, so we have to double-check that everything is still mapped correctly.

Yes for sure, as I wrote in my internal message, I am pretty sure that the Helm Chart doesnt work correctly anymore. I tried to fix as much as possible, but I only had limited time and knowledge, so I have to ask you to fix my mess for me 😃

• Regarding the tenant-aware section: does introducing this level remove support for the previous single-tenant configuration (e.g. damap.title, damap.fields.*, etc.)?
  If so, this would also be a breaking change for non-multitenant deployments, as existing configs would need to be migrated to damap.tenant-aware.* .

Its a breaking change, but v5.0.0 is going to be breaking anyway. Now its just even more so. We already renamed many config properties and added new ones, now they also need to prepend the tenant-aware, i think its going to be manageable.

• Not sure if the person services list now enforces each subitem to be read separately. In the Helm chart it was previously possible to pass the entire list as YAML and let the application parse it directly. If the new approach is mandatory, there may be scalability and maintainability concerns, as this could become cumbersome when the list grows.

What are you referring to with passing as YAML. I thought we passed the list as JSON. But dont worry, the list will realistically never grow beyond 5 services (and even that is hyperbolical). In Almost every case, 3 will be the maxium and most will have two.