-
Notifications
You must be signed in to change notification settings - Fork 29
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
Initial commit for multi-tenant-configuration #37
base: master
Are you sure you want to change the base?
Conversation
mheyen
commented
Feb 19, 2021
- created initial structure
- added main script
- added config file
- added environment folder
- added configurations folder
- added requirements.txt
- created initial structure - added main script - added config file - added environment folder - added configurations folder - added requirements.txt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this isn't ready for merging, how about you make this a draft PR for now? :)
multi-tenant-configuration/config.py
Outdated
#If you have multiple tenants use something like | ||
#url_pattern = "https://{}.example.org" | ||
#otherwise, url_pattern should be the same as the url variable above | ||
url_pattern = "http://{}:8080" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say the url pattern could be blank if it's a single-tenant-system.
For the future it might be nice to have the alternative possibility to list all tenant urls here in case there's no pattern that matches all of them. So something like tenant_urls = ["http://tenant1.com", "http://tenant2.de"] But that's not really a high priority.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So in the example where I have 'http://tenant1:8080' and 'http://tenant2:8080' as tenants, I would still keep the pattern_url
?
Or do you mean that the script would be more general and also applicable to single-tenant-systems if the pattern is {} ?
But in this case you would just use the global admin node url, right?
I thought the pattern was exactly for the case, where there are multiple tenants with similar URLs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was talking about this comment:
otherwise, url_pattern should be the same as the url variable above
IMO, if you only have one tenant, url_pattern doesn't even need to be defined. You just use url for everything. The script should be able to deal with that.
If you have tenants URLs that match a pattern, define url_pattern. (Maybe rename to tenant_url_pattern to be clear?)
If they don't match a pattern, define something like tenant_urls instead. The script should use whatever's available. (And if a user defines both... idk, pick one or explode with errors. :D)
And in general, the script should treat 'config key undefined' and 'value is blank' the same.
I hope this is clear? If not, just ask. :D
PS: I just realized that tenant_urls might need to be a map, to know which url belongs to which tenant. But like I said, you could ignore this option for now and add it later. It's not really that important...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I got it :D
Just for clarification:
'config key undefined' -> the user commented out, i.e.
# tenant_url_pattern =
'value is blank' -> the user inserted an empty string, i.e.
tenant_url_pattern = ""
--- | ||
|
||
opencast_organizations: | ||
- id: dummy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This structure is quite complex and if I remember correctly, not all of it is used by the user scripts. I might have to think about whether it makes sense to have a separate file for this, even if we then have some duplication...
- username: player | ||
name: Player System User | ||
email: [email protected] | ||
password: 34dchG6nbhmhnG |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case anyone is wondering - these are not real passwords, just me smashing on a keyboard. ;)
- added dictionary for tenant_urls - updated documentation
…onfiguration most functions are now in utils.py . can be shifted to other libs if they are ready. tenant_urls can now be configured manually or are automatically generated by the script based on the tenant_url_pattern.
Added update_group and create_group function using the External API. Added more User Interactions to group checks. and overall code improvements: - fixed verbose flag - added usage of get_tenants function while parsing the config - added function get_user to configure_users.py
To improve the usability,the User can now store his answers by adding a 't' or 'a' to his answers. The question for that action will then not be asked again.
Improved User questions included for member and roles checks.
If the script is started without specifying a tenant_id it will now perform all checks for all tenants.
Added configurations and digest login as global variables. Now they don't have to be passed onto every function.
Added configurations and digest login as global variables in configure_users.py .
It is now checked if the argument 'check' has a valid value.
The script can now store permissions for creating accounts if the user wants this. imrpoved readability and structure of the code. The basic structure is now similar to the group checks. More checks need to be added.
One can now add individual headers to a get request.
Permissions are now stored in a dictionary via a combined key of 'action' + 'tenant' + 'target' target-specific permissions overwrite tenant-specific permissions.
The script now checks the API acess and the user roles. A new function to update a user is also added.
The group types (closed or open) are not used in the script anymore. The were formerly used to allow certain members to be added without asking the user again. Now, as user permissions can be stored, this is not neccesary anymore. The user is asked for every member, if no permissions has been specified before.
removed anything which is not used by the script.
Moved or deleted functions from parsing_configuration.py Created minimal Logger class and included the logger in all scripts
Is this going to move forward still? We now have https://github.com/opencast/community-integrations/tree/main/multi-tenant, which is similar to what this is supposed to do - although this looks to do a bunch of extra things too. |
*insert Gandalf's I have no memory of this place meme here* If I remember correctly, this is supposed to make the configuration of multi-tenant-systems easier, so it's not quite what's in the integrations repo. But also this has been open for ages and Malte isn't working for elan currently, so this isn't getting finished any time soon. Feel free to close if you want. |
I'll leave this open. If nothing else it might get finished since a significant chunk of work has been done. If not, leaving it open until it's hopelessly out of date doesn't hurt. |