Skip to content

Conversation

@Kami
Copy link
Member

@Kami Kami commented Mar 26, 2019

This pull request contains initial implementation of our proprietary and private RBAC backend code.

The goal is to move our proprietary RBAC code from open-source repo to a private one. To be able to do that I introduced new "RBAC" backends concept which follow a similar adapter / driver concept we already use for runners, auth backends and metrics backends.

There are multiple reasons for that:

  1. Foundation efforts - repo which will we move to foundation shouldn't contain proprietary bits (see recent ElasticSearch drama, etc.).
  2. Prevent free loading - right now it's too easy to enable RBAC without paying for enterprise edition (eventually we plan to only ship obfuscated .pyc files and not raw .py files to make that even harder, etc).
  3. Ability to ship RBAC bug fixes independently of StackStorm packages itself.

Due to the nature of the code in StackStorm/st2 repo, I can't move 100% of the RBAC related code here. We simply don't have plugin / adapter paradigm for a lot of code parts (API endpoints, API and DB models, services, etc.).

And, TBH, it doesn't even make sense - making all of those code parts pluggable would mean a lot of work, vastly increase the code complexity and maintenance costs. In theory, having "everything pluggable" sounds kinda nice, but in practice it's a mess and not something I would ever want to do.

The "good" news is that RBAC already follows quite a nice abstraction, so it was relatively easy to move the most important parts to this repo:

  • RBAC Permission resolver - that's where the main permission resolving business logic lives
  • RBAC definitions loader and syncer class
  • st2-apply-rbac-definitions script

Things which are left as part of the open source repo:

  • DB, API models
  • RBAC service code (code for talking and reading to the database)
  • RBAC utility code and constants
  • Config options

NOTE 1: StackStorm/st2 PR will be separate.

To avoid drawing too much attention to that PR, I propose temporarily forking StackStorm/st2 to a private repo so we can review code there in private. Once we are happy with it we can merge it and push it directly to StackStorm/st2 master branch. Smart people can of course still figure out this change, but that's not the point.

NOTE 2: To be able to run the tests, we will need to clone StackStorm/st2 repo since a lot of tests rely on code from there (that's a similar pattern we already use in a lot of other repos and packs).

NOTE 3: Once this is ready for review, please only focus on new code which was added as part of the backend concept. 90% of the code and tests is the existing code just moved from StackStorm/st2 repo and adapted to use backend concept.

TODO

  • Migrate RBAC related code from code from StackStorm/st2 repo (where possible and it makes sense)
  • Migrate RBAC related tests from StackStorm/st2 repo
  • Update license headers in the source files - they should indicate those files are proprietary and Copyright of Extreme
  • Add and hook up Circle CI builds
    • Lint and checks (Python 2.7, Python 3.6)
    • Unit tests (Python 2.7, Python 3.6)
    • Integration tests (Python 2.7, Python 3.6)
    • Run build job nightly
    • Make sure each StackStorm/st2 master merge triggers a build in this repo. Not all RBAC code is 100% self sustained and a lot of it depends on code in StackStorm/st2 repo so we need to ensure that a change in StackStorm/st2 repo doesn't regression or break behavior in this repo.
    • Debian and RPM package builds
  • Add package (deb, rpm) related metadata
  • Install and configure this package as part of bwc-enterprise package

@Kami
Copy link
Member Author

Kami commented Mar 26, 2019

WIP StackStorm/st2 change is located here - https://github.com/StackStorm/st2-private/pull/1.

@@ -1 +1,60 @@
# StackStorm Enterprise RBAC Backend for StackStorm Enterprise Edition

Copy link
Member

Choose a reason for hiding this comment

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

For God's sake, please add the CircleCI build badge !!

Copy link
Member

Choose a reason for hiding this comment

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

This one ^^ is from recent #community pearls 😈

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for reminder, was on my TODO but forgot to do it :)

@Kami
Copy link
Member Author

Kami commented Mar 28, 2019

Talking about the postinst postrm scriptlets for enabling/disabling rbac in st2.conf,
I think this deb/rpm package should add/remove [rbac] settings once it's installed or uninstalled, not the enterprise package, just because you can force-remove deb/rpm even if it's a dependency or not.

Yeah, I agree that this makes more sense now that we have this package.

Only "problem" is upgrade, although I assume this should be handled automatically on bwc-enterprise upgrade because it depends on st2-rbac-backend which will be auto installed when upgrading bwc-enterprise, right?

@Kami
Copy link
Member Author

Kami commented Mar 28, 2019

Added steps to automatically add / remove config entries on install / removal.

Per discussion with @armab with slack, we won't automatically restart services on install / upgrade / removal. We will document this in st2docs - user needs to restart services for RBAC to start working.

Copy link
Contributor

@m4dcoder m4dcoder left a comment

Choose a reason for hiding this comment

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

This is hard to review. I assume most of this is just moving code from the main st2 repo. If there are any changes, it would be addition for CircleCI, make, and packaging. I defer review to build and packaging to @armab. Otherwise, this LGTM.

# Proprietary and confidential.
# See the LICENSE file included with this work for details.

__version__ = '3.0dev'
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a reminder here to add steps in st2 release process to update this version.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the reminder. Here we go - StackStorm/st2cd@2f643d4

On a related note - those release workflows have a lot of duplication and copy and paste. Once with-items stuff in Orquesta is working correctly without performance penalties we should look at refactoring some of those workflows to use with-items instead.

@Kami Kami merged commit cb3ef50 into master Apr 4, 2019
@Kami Kami deleted the add_initial_implementation branch April 4, 2019 11:06
Kami added a commit that referenced this pull request May 7, 2019
73fe88f Fix merge conflict.
72d3b7c Merge pull request #1 from StackStorm/update_lint_configs
2709c47 Update lint config with changes from StackStorm/st2#4657.

git-subtree-dir: lint-configs
git-subtree-split: 73fe88f2da7403d69bc5ebd7c6d22d12860c3744
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.

4 participants