There are 3 settings in Foreman that affect a lot of code for very little value IMHO:
locations_enabled
organizations_enabled
login
These settings allow running a “simplified” Foreman, but come at a significant cost with respect to code complexity:
every method touching taxonomies or users has to check the value of the relevant setting
installer and puppet modules need to support setting them
some methods related to taxonomies need to account for the possibility of just organizations or just locations enabled
tests need to check all possible setting combinations as well
Going forward, I suggest being more opinionated and removing these settings.
This should be done in several stages:
Informing users these settings are going to be removed in version X (1.21? 1.22?), perhaps using notifications.
Creating a default organization and/or location and assigning all objects to it for users who have the matching taxonomy disabled. (perhaps not needed and we can set the “all” option on the taxonomy?)
setting all the settings to true during upgrade to X or X-1
removing a whole lot of code
TBH, I would be happy to get rid of the unattended setting as well, but it seems that it has more users and use-cases and would be harder to remove.
If you think there is reason to keep these settings, please shout out. Otherwise, if there is agreement, I would want to deprecate the settings in 1.20 already so by 1.22 we can get to step #4 above.
ACK! In general, I agree completely that removing the matrix of configuration and being opinionated here will make code and deployments easier. Speaking from a Katello point of view that requires both taxonomies to be enabled this would reduce a headache with adding Katello to an existing Foreman.
Let’s play the devils advocate. Can we keep supporting locations_enabled and organizations_enabled without the code overhead? Would it be possible to use locations and organizations but only hide it in the UI? That would mean always using a single location + organization and hiding the selectors. It would make it possible to change it from a pre-install setting (because of seeding) to a runtime setting.
I don’t care much about the login setting because a critical part of infra like Foreman should IMHO always be protected by a login.
I suspect unattended could be done more on the UI level as well. At least keeping the same class signatures and changing return values would improve the situation.
As I mentioned in the original post, I didn’t include unattended on purpose, since that also changes a lot of the behavior of the application. For example, the whole host build and orchestration workflow is different when enabled or disabled, not only in the ui but also behind the scenes.
For location and organizations, we could just hide the relevant bits in the UI, but that would require more complexity in the ui - e.g. assign the default taxonomies when hiding the elements etc. If there is much push-back that could be a viable option, but let’s see first if there is any such.
Regarding the login I agree, this should definitely be enforced (and if anyone doesn’t like typing their password often they can change the session timeout setting)
On the other hand, is it really big deal to have a single org/loc and having extra two fields for each resource?
I’d rather see investing dev time into fixing UX around taxonomy so for example org/loc is always set automatically according to default selection so uses can completely forget about it and simply ignore the setting rather than doing extra work of “hiding” the stuff. That can easily generate more pain (e.g. hidden element preventing a form from being submitted).
personal experience of rolling out foreman in a number of different use cases I’ve found that using foreman on it’s only normally only warrants one location/org, I’ve rarely hit a use case that required the complex org/locations configuration, so normally disable them by default. Having them hardcoded to enabled but all resources under a default location / org actually makes things significantly better.
When using complex orgs/location layouts I’ve found Katello normally is a better fit due to the size of the estate that warrants this complexity and the functionality the estate requires, so foreman on it’s own normally doesn’t need that level of complexity.
It’s a good idea, if we are able to tweak some of the usability of taxonomies. I agree there’s very little difference to the user if there’s 1 location and 1 organization and they always do things in that context. All the UI fields get populated based on that context. Hammer now has the default taxonomy options too, so it’s not too painful.
But, you can still create organizationless/locationless objects and that does cause issues. It would be nice if we could avoid letting users do that, and stopped do it for auto populated hosts (e.g. puppet reports). Also smart proxies are always registered without a taxonomy.
As a user of both location (I use this to identify EC2 instances in particular AWS regions) and organization (I use this to identify instances used by a particular business unit), what’s going to change from the user’s perspective? Are these going away completely or being accomplished in some other way?
We’re removing the settings to disable them. For the end user this means they’re always enabled which means we developers can remove conditionals in our code. Since you have both enabled, nothing will change for you.
do we need to add code to auto include every attribute into the default
location/org? I think the current user flow (a user that had those disabled
before) will need to assign all of his resources to the org?
