Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.The description of the profile lock options reads:
Locking this platform will not delete or disable it or its sites, but will
prevent any new sites from being created on it. This may be useful when you have sites that
cannot be migrated onto a newer platform, but you wish to prevent other users or clients
from continuing to provision on this platform. The platform can be unlocked later.
Thus after locking a platform it should no longer show up as an option on the 'Create Site' form (maybe for admins, but certainly not for clients).
This however is not happening and a client is able to create a site on a locked platform.
The radios in the create site form are populated by hosting_get_profile_platforms(), which never checks the platform_status to see if the platform is locked:
function hosting_get_profile_platforms($profile, $check_old_short_name = FALSE) {
$defaults = array('default', 'standard', 'minimal', 'testing');
$platforms = array();
$instances = hosting_package_instances_load(array(
'i.package_id' => $profile,
'n.status' => 1,
'r.status' => 1,
'r.type' => 'platform',
));
if ($check_old_short_name) {
$instances = array_merge($instances, hosting_package_instances_load(array(
'p.old_short_name' => $instances[key($instances)]->short_name,
'n.status' => 1,
'r.status' => 1,
'r.type' => 'platform',
)));
}
foreach ($instances as $iid => $instance) {
$platform = node_load($instance->rid);
// this is one of the default profiles
if (in_array($instance->short_name, $defaults) &&
sizeof(array_diff(array_values($platform->profiles), $defaults)) &&
variable_get('hosting_ignore_default_profiles', FALSE)) {
// there are other profiles available on this platform. skip this.
continue;
}
if ($platform->platform_status != HOSTING_PLATFORM_DELETED) {
$platforms[$instance->rid] = $platform->title;
}
}
return $platforms;
}
| Comment | File | Size | Author |
|---|---|---|---|
| #32 | clients_can_select-2821401-32.patch | 1.29 KB | helmo |
| #20 | clients_can_select-2821401-20.patch | 659 bytes | Neograph734 |
| #4 | clients_can_create-2821401-4.patch | 810 bytes | Neograph734 |
Comments
Comment #2
Neograph734Comment #3
Neograph734Comment #4
Neograph734Okay... It seems the client cannot actually create the site (lowering priority), the form validation catches it. Yet it is very strange the client is offered an option he cannot use.
There is the
hosting_site_available_options()function that performs several checks for the available form options (this is what the validation uses as well). Attached patch uses this to also limit the displayed platforms.Comment #6
helmo CreditAttribution: helmo as a volunteer and at Initfour websolutions for Aegir Cooperative commentedThanks, committed after a quick review on https://github.com/aegir-project/hosting/pull/8
Comment #7
memtkmcc CreditAttribution: memtkmcc at Omega8.cc commentedThis patch actually breaks things for all platforms with installation profiles other than those by default included by D7 core.
When you click on any such installation profile, you no longer can choose corresponding platform, even when you have all access rights (uid=1)
See the screenshots attached.
Comment #8
memtkmcc CreditAttribution: memtkmcc at Omega8.cc commentedAnd here is how it looks after reverting this patch.
Comment #9
Jon PughThis looks critical to me, now that it is in a released version...
helmo: how do you want to proceed? revert completely or try to fix it?
Comment #10
Neograph734memtkmcc are you sure those platforms are not restricted (locked, assigned to certain clients, etc) somehow? The listing has become stricter as result of this patch.
I cannot reproduce this error:
Comment #11
memtkmcc CreditAttribution: memtkmcc at Omega8.cc commentedHmmm.. no, there are no restrictions used at all. The platforms are not locked either. I can reproduce the problem using Hosting head version. I have also checked all remaining BOA specific modifications we have had in place, and there is nothing which could affect this form behaviour.
Note that to reproduce this you need to go to /node/add/site form directly, and not via Add site link on the platform node -- which still works, because it redirects to /node/add/site?platform=4090 type URL.
As for the settings, we don't use "Lock platforms by default on initial creation" and "Hide platforms with non-default profiles" is also left disabled, while "standard" is set for Default profile.
Comment #12
Neograph734I just spun up a new virtual machine, and installed
Hosting 7.x-3.8+5-dev (2016-Nov-11)(as part ofHostmaster (Aegir) 7.x-3.8+1-dev (2016-Nov-11)). This contains all recent patches.I have built Open Atrium from their make file, but placed the include inline, so that it now looks like this (you can verify the contents of the included file in their repository):
And it showed up nicely on node/add/site.
Perhaps any of the aegir dev's can give a second opinion?
Comment #13
memtkmcc CreditAttribution: memtkmcc at Omega8.cc commentedWhat happens when you delete minimal & standard profiles (which are useless for platforms with distro-specific profile anyway), so there will be only open atrium profile in the OA platform? Can you still see the OA platform in this form then?
Comment #14
memtkmcc CreditAttribution: memtkmcc at Omega8.cc commentedI have just tested this, and I can confirm that if you delete those default core profiles, you no longer can see the platform in this form, while the other platform with those default core profiles still existing, can be displayed w/o problems. This is not how it should work, though.
Comment #15
Neograph734How can I delete these profiles? I can only delete the Drupal base platform (which caused no errors).
Comment #16
memtkmcc CreditAttribution: memtkmcc at Omega8.cc commentedYou delete not used profiles via SSH/SFTP from /profiles/ in the OA platform, and then re-verify the platform, so it will have only its single, distro specific profile. We delete all those useless profiles in BOA automatically for years, because their presence is known to cause a high level of confusion for users.
Comment #17
Jon PughAdding Needs Tests tag, this is a great candidate for a Behat test. Access control issues are very easy to create and very difficult for a human to test.
See http://github.com/aegir-project/tests
Comment #18
Jon Pughmemtkmcc: That does sound like a good feature...
Comment #19
Neograph734@memtkmcc: yes, confirmed. It appears to be coming from
hosting_site_available_options($node), that for a new node defaults to the first platform it encounters. It can be overridden per profile, so replacing the lines from the patch with something like this works:I'll tidy it up and build a new patch. Perhaps it would be better to change the logic in
hosting_site_available_options, but that depends on how and where it is used.Comment #20
Neograph734Whoa, it is way easier. We just have to make the node aware of the selected profile.
This basically concludes to adding this before the lines of the patch.
So the entire patch looks like:
Comment #21
Neograph734Comment #22
Neograph734#16 and #18, follow-up issue #2828256: Enable / disable profiles within platforms.
Comment #23
Neograph734It appears this still needs some work, currently the 'add site' tab on a platform node is showing the form, but not with the selected platform.
Basically the wrong profile is pre-selected, and as such the chosen platform might not available.
I am not sure if this is caused by the patch, or by manually deleting several profiles. (I suppose the latter because all platforms used to be available for the standard profile.)
For now, please review the proposed solution and then revert this to 'needs work'.
Comment #24
Neograph734@helmo, since #4 has been committed, can you add #20to it or revert #4 so that dev functions again? From there we can start looking at tests (I'd like some examples before I proceed with those)
Comment #26
helmo CreditAttribution: helmo as a volunteer and at Initfour websolutions for Aegir Cooperative commented#20 is now committed.
Comment #27
Neograph734Thanks helmo.
@memtkmcc, could you check if your issue is solved?
@helmo, or Jon Pugh, could you provide me with some example of a test?
Comment #28
helmo CreditAttribution: helmo as a volunteer and at Initfour websolutions for Aegir Cooperative commented@Jon Pugh is the best source about behat ... but I can get you a link to the feature files.
Comment #29
memtkmcc CreditAttribution: memtkmcc at Omega8.cc commentedI can confirm that it is now fixed without any side effects. Thanks!
Comment #30
Neograph734Thanks for checking.
Reopening in order to add a test for this. Behat is still a lot of magic for me, but I'll take a stab at it later this week.
Comment #31
memtkmcc CreditAttribution: memtkmcc at Omega8.cc commentedThis patch breaks things totally now, albeit this time on site's node edit (like when you add an alias, etc) -- any site with custom/non-default profile will lose association with its real profile, replaced in the hostmaster database with default profile (which either doesn't exist in that site's platform, or has no codebase expected), which breaks drushrc.php for that site, all its tasks, and the site itself is now completely broken, because Drupal can't find the site's modules.
Please revert all four lines until something truly working will be proposed.
Comment #32
helmo CreditAttribution: helmo as a volunteer and at Initfour websolutions for Aegir Cooperative commentedPlease check this new patch ... it reverts both previous commits and 'fixes' the hosting_get_profile_platforms() function.
It changes
platform_status != HOSTING_PLATFORM_DELETEDtoplatform_status == HOSTING_PLATFORM_ENABLEDas I could not find a use for the function to return platforms with status HOSTING_PLATFORM_QUEUED or HOSTING_PLATFORM_DELETEDComment #33
Neograph734This patch seems to work partially;
Working:
- Locked platforms to not show up.
Not working:
- Clients are able to see and create sites on the hostmaster platform (could be solved by #2821402: Lock the hostmaster platform by default?)
- Clients are able to see but NOT create sites on platforms where exclusive access was granted to another client.
Comment #35
helmo CreditAttribution: helmo as a volunteer and at Initfour websolutions for Aegir Cooperative commentedI've committed #32 to get rid of the regression mentioned in #31 and to get in into the 3.10 Aegir release.
Setting this back to Major to find a proper solution.