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
Optionally block enabling certain feature flags via Management UI/API #10682
Conversation
Useful to avoid accidentally enabling for example experiemental feature flags from the Management UI on sensitive clusters.
@gomoripeti I'd highly recommend consulting with someone on the core team before you spend time on something that's not obviously a bug fix. For example, this PR further complicates the matrix of feature flags, deprecated features, all that. Now we'd potentially have "blocked" feature flags. The risk of not being accepted for such semantically complex PRs is usually much higher than average :( |
you are right with the consulting. I'd like to mention that exactly because of the complexity of the feature flags subsystem, I did not touch that. This is rather part of the restrictions subsystem of the management plugin only, that I think fits well to the other restrictions. I am open to changing naming in any way - another alternative that matches more the naming of the other restrictions, |
|
||
%% Block enabling certain feature flags over API | ||
|
||
{mapping, "management.restrictions.feature_flag_blocked.$name", "rabbitmq_management.restrictions.feature_flag_blocked", [ |
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.
What do you think about renaming this key to management.restrictions.blocked_feature_flags.$name
(in plural with the adjective coming first, as it would in a piece of prose in English)?
I did not mean the technical complexity of feature flags. If we accept a new concept, like "blocked feature flags" or "restricted feature flags", we must be able to explain it to the user with as little as possible confusion with related concepts. Deprecated features and feature flags have very little in common technically but they seem like the same "on and off switch" idea for features, which is not correct. Adding blocked/restricted feature flags does not make that any easier, and as more concepts are added, the complexity grows exponentially. In any case, I've asked the rest of the team if they'd accept such a feature in general. |
@gomoripeti someone has suggested that a UI confirmation (of the yes/no type) for experimental feature flags would suffice, and would not introduce any new concepts to document and explain. |
@gomoripeti another idea, sadly a more painful one to implement given the age of our JS codebase, but still: similarly to how you must type in the name of the repository you attempt to delete (or transfer) on GitHub, we can ask for the name of an experimental feature flag before enabling it. This should be a lot more "fat-finger-proof" than a simple Ok/Cancel confirmation. |
it seems to be important to have a naming that clarifies that this is not a new concept There are already
But I realise these proposals (including the PR in the current form) don't have any added value. Just blocking certain API endpoints can be implemented in a firewall or proxy and doesn't need any more rabbitmq-specific information. It would have added value if the UI would be changed as you described with some dialogue, but I would like to skip that more painful part. But it was a good discussion and I learnt about management restrictions while implementing it, so not fully wasted effort :) |
With a solution like this, we'd have to explain this "concept" or "feature set" of feature flags on top of everything else. Whether that's technically a new concept or not does not change that fact. But I think that any confirmation solution for experimental FFs would be very welcome, I can see how clicking a button and enabling Khepri by accident is fairly easy. |
In RabbitMQ-as-a-service scenarios, blocking an endpoint might work. We can always wait and see how often people would enable Khepri (or a future experimental feature flag) by accident and complain. |
Proposed Changes
Useful to avoid accidentally enabling for example experimental feature flags from the Management UI on sensitive clusters,
even for users who have administrator access to the Mgmt UI/API, but still don't have server access. After testing an experimental feature flag in a test/staging env, the feature flag can be unblocked by operators on the sensitive production clusters.
I intentionally kept this configuration outside of the core feature flags framework, and only a management plugin feature.
I am open to suggestions if this should be configured or implemented differently.
Types of Changes
What types of changes does your code introduce to this project?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply.You can also fill these out after creating the PR.
If you're unsure about any of them, don't hesitate to ask on the mailing list.
We're here to help!
This is simply a reminder of what we are going to look for before merging your code.
CONTRIBUTING.md
documentFurther Comments