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

Allows any tenant to be at the root #13958

Merged
merged 5 commits into from Jul 19, 2023
Merged

Allows any tenant to be at the root #13958

merged 5 commits into from Jul 19, 2023

Conversation

jtkech
Copy link
Member

@jtkech jtkech commented Jul 7, 2023

Fixes #13952

Meaning to AutoSetup the Default tenant but with a non empty RequestUrlPrefix. And then also allows to Autosetup a non Default tenant being at the root, meaning having a null/empty RequestUrlPrefix.

Todo: Also allow the same from the admin UI or API controller, as it was before.

Okay done by updating the TenantValidator but we still check if 2 tenants (including the Default) have both the same RequestUrlPrefix and a shared RequestUrlHost.

@jtkech jtkech changed the title Allows to Autosetup the Default tenant but not at the root Allows any tenant to be at the root Jul 7, 2023
@jtkech
Copy link
Member Author

jtkech commented Jul 7, 2023

Has been tried by @devlife, see #13952 (comment)

@Skrypt I let you review it when you will have time, and approve if you think that's okay.

// The 'Default' shell is first created by the infrastructure,
// so the following 'Autosetup' options need to be passed.
shellSettings.RequestUrlHost = options.RequestUrlHost;
shellSettings.RequestUrlPrefix = options.RequestUrlPrefix;
Copy link
Member

Choose a reason for hiding this comment

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

If it's not the default tenant how the URL prefix will be set?!!

Copy link
Member Author

Choose a reason for hiding this comment

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

Just before there is a CreateTenantSettings() which is called for each tenant to pre-create them with the Autosetup options, unless for the Default tenant which is still created by the shellHost initialization on the first request.

@Skrypt
Copy link
Contributor

Skrypt commented Jul 7, 2023

Don't merge it yet. I want to have some time to test it. Need to see if we can automate some tests with this.

@jtkech
Copy link
Member Author

jtkech commented Jul 7, 2023

Okay, let me know ;)

@jtkech jtkech merged commit d5cd01b into main Jul 19, 2023
3 checks passed
@jtkech jtkech deleted the jtkech/autosetup branch July 19, 2023 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AutoSetup issue with "/" RequestUrlPrefix
3 participants