Setup BWC repos, Install BWC and Setup RBAC #126

lakshmi-kannan · 2017-03-07T21:52:02Z

To test, I changed stackstorm.yml to include following:

    - name: bwc_repos
      vars:
        license: <REDACTED>

    - name: bwc
      vars:
        ldap: 
          config:
            bind_dn: foo
            ...
        rbac:
          roles:
              - foo
          assignments:
              - bar

TODO:

Rotate license keys after ensuring we don't log anymore
Idempotency fails in U14 and U16 https://gist.github.com/lakshmi-kannan/581bf90b3a192ae7303b662ad197bc13
bwc smoke tests

arm4b · 2017-03-08T18:12:36Z

What do you think about moving bwc_rbac role logic inside the bwc?

RBAC comes tightly connected with bwc-enterprise package/dependencies in any way. Looks a bit more simpler:

  roles:
    - name: Install and Configure Brocade Workflow Composer
      role: bwc
      vars:
        bwc_version: latest
        bwc_rbac_enable: yes
        bwc_rbac_roles:
          ...
        bwc_rbac_assignments:
          ...

which is similar to st2 role with st2_auth_enable: yes var.

Related question to that. Do you think we'll have more roles related to bwc in nearest future? Just to have better understanding, since it could be a game changer how to distribute everything later (you previous valid concern about: roles/galaxy/repos).

arm4b · 2017-03-08T18:24:16Z

roles/bwc_rbac/defaults/main.yml

@@ -0,0 +1,3 @@
+---
+
+st2_config_file_path: /etc/st2/st2.conf


I think we can hardcode it without any big worries. st2.conf path is pretty much a constant everywhere, bundled in packaging.

Also that's what we do in st2 role (st2.conf path is hardcoded).

arm4b · 2017-03-08T18:24:59Z

Just a note: We should think about better approaches for setting RBAC configs in this or maybe in future bwc role versions.

Vars like:

bwc_superuser: st2admin
rbac_setup_default_assignments: true

seems to provide not that much flexibility in terms of configuration. But yeah, it's a good start.

arm4b · 2017-03-08T19:39:02Z

@lakshmi-kannan Returning to your original concern about roles/galaxy/repos/distribution. Here is a more detailed write-up about our options:

1) `BWC` role in separated repo

Long-term (?), more effort, polishing, time

As you noted, we can use packagecloud-ansible-role as external dependency for bwc
- but it's not idempotent
- in future we need contribute PRs to improve it
Publish bwc on Ansible-Galaxy later
- important requirements (!): bwc should work as a single role (with depenencies if needed), very different approach comparing to ansible-st2
Blog Post it later (marketing ++)
Harder to test the role, since we need st2 installed first
Needs some CI configuration
Long term solution, harder to do/more to polish/more time

2) `BWC` in `ansible-st2` repo

KISS, faster, easier for short-term

I don't like packagecloud-ansible-role code and how we re-use it in ansible-st2, needs much more polishing.
Easy to test, since all roles are in one repo and CI is configured: https://travis-ci.org/StackStorm/ansible-st2
short-term solution, easier to do if we need it faster

I'll let you to implement/polish the main "core" logic (since it's needed in any way) and later when it's finished I think we should throw all pros/cons to decide what works best based on improved data/arguments/ideas/priorities we have.
The reasoning about rising these questions, - I have opinions and my requests to change this or that piece of code depends on the approach we choose.

cc @dzimine about short-term vs long-term approaches.

PS. @lakshmi-kannan Also take a look at related: #45 to have better context.

lakshmi-kannan · 2017-03-10T02:15:53Z

roles/bwc/tasks/ldap.yml

+    dest: "{{ st2_config_file_path }}"
+    section: auth
+    option: backend_kwargs
+    value: "{{ ldap.config | to_nice_json }}"


This prints out a multi-line string in the ini file and our config parser pukes. I tried to_json. Still the same.

arm4b

One important request is to define all used vars in defaults/main.yml and include them in README.md #Variables with descriptons.

Another issue is idempotence. Packagecloud logic needs a good cleaning and refactoring.

Additionally, as noted before https://stackstorm.slack.com/archives/stackstorm/p1489014359911089?thread_ts=1489011496.584237&cid=C029K4CBU we need all these roles to run in TravisCI. It will show you more errors. See #125 (comment) and #125 (comment) in @humblearner's PR how to mask the secrets in Ansible/Travis debug logs.
If we don't run these Ansible roles in CI, - it's not real.

What I liked: dicts and all the logic to configure advanced things like rbac settings. Looks pretty flexible from the ops PR.

In general, - I like the high-level interface to interact with bwc (again, according to ops PR), - we'll just need to polish low-level tasks more.

For ansible-st2 repo, I think it's a good idea to set these rbac var defaults so they conifgure the same settings as bwc installer. Good for consistency.

Finally, if you know how to check bwc basic functionality by adding bwc-smoketests, similar to st2smoketests, - that would be very nice.

arm4b · 2017-03-10T14:15:56Z

roles/bwc/defaults/main.yml

+# used only if 'bwc_version' is numeric
+bwc_revision: 1
+
+st2_config_file_path: /etc/st2/st2.conf


As noted before, don't forget to hardcode st2.conf path.
We don't need it in vars (#126 (comment) reasons why).

arm4b · 2017-03-10T14:19:18Z

roles/bwc/tasks/ldap.yml

+    section: auth
+    option: backend
+    value: ldap
+    backup: yes


Notify st2 restart or reload handler (if enough) trigger when changing the st2.conf file.
Example: https://github.com/StackStorm/ansible-st2/blob/master/roles/st2/tasks/auth.yml#L34

Yeah, we also have to set the backend_kwargs before restarting. See next step. I did notify.

Include notify for every task where its needed logically (ex: changing the conf file), its less error prone since "next task" could be removed, adjusted or never ran because of some conditional.

It doesn't matter how many times we call Ansible handlers, - they will be executed in the end of entire play run in any way.

See "Handlers: Running Operations On Change": http://docs.ansible.com/ansible/playbooks_intro.html with better explanation.

Ahh, good to know about that.

arm4b · 2017-03-10T14:22:17Z

roles/bwc/tasks/main.yml

+    - bwc
+    - st2 enterprise
+
+- name: Setup RBAC and setup roles and assignments if enable_rbac is defined


What is enable_rbac?

arm4b · 2017-03-10T14:25:21Z

roles/bwc/tasks/rbac.yml

+      owner: st2
+      group: st2
+    with_items: "{{ rbac.roles }}"
+    when: rbac_roles is defined


Can't find any occurrence of rbac_roles var. Is it used anywhere?

arm4b · 2017-03-10T14:25:36Z

roles/bwc/tasks/rbac.yml

+      owner: st2
+      group: st2
+    with_items: "{{ rbac.assignments }}"
+    when: rbac_assignments is defined


Can't find any occurrence of rbac_assignments var. Is it used anywhere?

arm4b · 2017-03-10T14:45:50Z

roles/bwc_repos/tasks/bwc_repos_yum.yml

+
+- name: Update yum package cache
+  become: yes
+  shell: yum -q makecache -y --disablerepo='*' --enablerepo='StackStorm_{{ bwc_pkg_repo|replace("/", "_") }}'


Any idea why do we need this?
Also it's not idempotent.

arm4b · 2017-03-10T14:46:53Z

roles/bwc_repos/tasks/bwc_repos_yum.yml

+- name: Update yum package cache BWC enterprise source repo
+  become: yes
+  shell: yum -q makecache -y --disablerepo='*' --enablerepo='StackStorm_{{ bwc_pkg_repo|replace("/", "_") }}-source'
+  when: ansible_os_family == "RedHat" and added_bwc_rpm_repository|success


when: ansible_os_family == "RedHat"

we're already in os-family specific yum task.

arm4b · 2017-03-10T14:48:18Z

roles/bwc/tasks/main.yml

+
+- name: Setup RBAC and setup roles and assignments if enable_rbac is defined
+  include: "rbac.yml"
+  when: rbac is defined


Please name vars accordingly to role, eg: bwc_rbac.

arm4b · 2017-03-10T14:48:38Z

roles/bwc/tasks/main.yml

+
+- name: Setup LDAP and set up LDAP configuration
+  include: "ldap.yml"
+  when: ldap is defined


Please name vars accordingly to role, eg: bwc_ldap.

arm4b · 2017-03-10T14:50:13Z

roles/bwc/defaults/main.yml

+bwc_version: latest
+# used only if 'bwc_version' is numeric
+bwc_revision: 1
+


Please define all vars used in this role with comments, as well as include them in README.md.

arm4b · 2017-03-10T15:03:10Z

When all finished/merged, we'll need Ansible st2docs update with separated section to install/configure bwc.

arm4b · 2017-03-13T14:32:35Z

Just a note: sync-up with master to include the recent build failure fix.

arm4b · 2017-03-13T17:14:34Z

roles/bwc/tasks/bwc_repos_yum.yml

+  tags: skip_ansible_lint
+
+# See: https://github.com/docker-library/docs/tree/master/centos#package-documentation
+# We ship `nginx.conf` via `st2` package doc files, for example


This block could be removed, was really critical for st2 package.

* master: Include memo to revert the version pinning later Fix idempotence test in kitchen-ansible

arm4b · 2017-03-15T18:26:34Z

roles/bwc/tasks/bwc_repos_apt.yml

+---
+
+- name: Assert that master_token is specified
+  fail: msg="License key must be supplied for BWC enterprise installation."


Not needed in this file, since you include that check in previous step bwc_repos_setup.yml

arm4b · 2017-03-15T18:31:09Z

roles/bwc/tasks/bwc_repos_setup.yml

+  no_log: yes
+  uri:
+    url: https://{{ bwc_license }}:@packagecloud.io/install/repositories/StackStorm/{{ bwc_pkg_repo }}/tokens.text
+    creates: "/etc/packagecloud/StackStorm_{{ bwc_pkg_repo }}_read_token.txt"  # Don't download if file already exists


Following our previous case when we rotated {{ bwc_license }}.

What will happen if user changes {{ bwc_license }}? Seems like read token will remain from the old license.

Eg. if user changes the license - Ansible should update apt/yum repository too with new read token.

Yep. Unfortunately, if I generate a token every time, it would cause idempotency failures. If I do what I did, then installation with fail if they swapped the license. But I think this is fine. It fails out with an error.

Another approach is to ask user for {{ bwc_read_token }} which I am -1 to.

Looks like I could force changed_when: no. That sounds hacky to me.

What we can do is to additionally generate the hash from the {{ bwc_license }} via http://docs.ansible.com/ansible/playbooks_filters.html#hashing-filters and store hash somewhere in file.

If hash changed (eg. user provided new license), - then we request new {{ bwc_read_token }} and update apt/yum repo config.

I think we can address this in a different PR. I am not too keen on adding more scope to this PR.

Deal. Feel free to create an issue for this then so maybe someone else could pick it up.

arm4b · 2017-03-15T19:11:21Z

roles/bwc/tasks/ldap.yml

+    value: ldap
+    backup: yes
+  # Don't even setup LDAP if backend_kwargs is not defined
+  with_dict: "{{ bwc_ldap.backend_kwargs | default({}) }}"


I see with_dict here but don't see {{ item }}. That thing is used only for loops, see http://docs.ansible.com/ansible/playbooks_loops.html#looping-over-hashes.

So you probably want something like:

when: bwc_ldap.backend_kwargs is defined and bwc_ldap.backend_kwargs|length > 0

Also @lakshmi-kannan length for dict worked in yaml:

TASK [bwc : debug] ************************************************************* task path: /vagrant/roles/bwc/tasks/ldap.yml:6 ok: [ubuntu16] => { "{}|length": "0" }

TASK [bwc : debug] ************************************************************* task path: /vagrant/roles/bwc/tasks/ldap.yml:6 ok: [ubuntu16] => { "{'key':'val','a':'b'}|length": "2" }

Just interesting fact.

arm4b · 2017-03-15T19:12:56Z

roles/bwc/tasks/ldap.yml

+    option: backend_kwargs
+    value: "{{ bwc_ldap.backend_kwargs | to_json | string }}"
+    backup: yes
+  with_dict: "{{ bwc_ldap.backend_kwargs | default({}) }}"


Same here, with_dict is used only for loops, just:

when: bwc_ldap.backend_kwargs is defined and bwc_ldap.backend_kwargs|length > 0

arm4b · 2017-03-15T19:31:51Z

roles/bwc/tasks/main.yml

+    name: bwc-enterprise
+    state: latest
+  register: bwc_installed
+  when: bwc_repo_added|success and bwc_version == "latest"


Don't need bwc_repo_added|success, since ansible will stop on first failure

arm4b · 2017-03-15T19:32:16Z

roles/bwc/tasks/main.yml

+
+- name: Setup RBAC and setup roles and assignments if bwc_rbac is defined
+  include: "rbac.yml"
+  when: bwc_installed|success and  bwc_rbac is defined


Don't need bwc_installed|success since Ansible will stop on first failure

arm4b · 2017-03-15T19:33:22Z

roles/bwc/tasks/main.yml

+
+- name: Setup LDAP and set up LDAP configuration
+  include: "ldap.yml"
+  when: bwc_installed|success and bwc_ldap is defined


Same here to avoid unecessary check bwc_installed|success

arm4b · 2017-03-15T19:44:28Z

roles/bwc/tasks/rbac.yml

+    become: yes
+    template:
+      src: rbac_roles/roles.yml.j2
+      dest: /opt/stackstorm/rbac/roles/{{ item.name }}.yaml


Changing any file in /rbac/ dir should notify reload bwc_rbac trigger.

Make sure we run that thing for other tasks in this file.

arm4b · 2017-03-15T20:57:28Z

roles/bwc/templates/rbac_assignments/assignments.yml.j2

+
+  username: {{ item.name }}
+  roles:
+    {{ item.roles | to_nice_yaml }}


Since we found indentation fix for https://github.com/StackStorm/ansible-st2/pull/126/files#r106033276 this must be the right way:
{{ item.roles | to_nice_yaml(2) | indent(2) }}

in case if someday item.roles will contain more complex data.

arm4b · 2017-03-17T18:24:37Z

roles/bwc_smoketests/templates/rbac_assignments/assignments.yml.j2

+
+  username: {{ item.name }}
+  roles:
+    {{ item.roles | to_nice_yaml(2) | indent(2) }}


indentation should be 2 spaces here, same as in roles.yml.j2

arm4b · 2017-03-17T18:25:01Z

roles/bwc_smoketests/templates/rbac_assignments/assignments.yml.j2

+---
+
+  username: {{ item.name }}
+  roles:


and 0 spaces here.

Otherwise {{ item.roles | to_nice_yaml(2) | indent(2) }} won't work correctly if we add some complex data there

arm4b

👍 🎉

arm4b · 2017-03-21T21:41:03Z

Thanks for being serious about the quality! 💯

After merging, what we'll need also is respective st2docs change to highlight bwc installation (as said in: #126 (comment))

* master: (28 commits) Add check for hubot_token Changed to right variable Move st2chatops smoke tests to seperate yml file Add tags and update failure message Update test for slack adapter Remove notify handler in favor of changed_when:no Fix for idempotency Errors only Travis can catch fix for ansible-lint update kitchen.yml for adapter smoketests for st2chatops Add matteruser as one of the supported adapters. Make adapter update idempotent Support for adapter update notify restart for shell Fix lint error Update README(2) and meta/main.yml Add support for shell and assert for supported adapter and its settings Add shell and mattermost in supported_adapters Making Travis happy ...

Lakshmi Kannan added 3 commits March 7, 2017 13:50

Initial commit

ab143a0

First round of fixes after U16 testing

6d05514

Remove Debug for GPG_URL

dca13a3

arm4b reviewed Mar 8, 2017

View reviewed changes

Lakshmi Kannan added 9 commits March 8, 2017 12:07

Fixes based on centos7 testing

3098871

Fix typo in variable name

b850d58

Remove support for user supplied RBAC roles&assignments

c287cc3

Let's support roles and assignments via dicts

b3772fc

Rename files -> templates

8548acc

use to_nice_yaml

a09cb88

use yaml as file ext and not yml

ac4cd06

Move back RBAC to same role as BWC

69fd550

Remove separate RBAC role

1aa9d35

lakshmi-kannan commented Mar 10, 2017

View reviewed changes

arm4b suggested changes Mar 10, 2017

View reviewed changes

Lakshmi Kannan added 7 commits March 10, 2017 10:46

Coalesce bwc_repos into bwc

ea34e3c

Fixes after testing

22f86a1

Fix variable names

1c1ebd0

Fix st2.conf file path

bd198f2

@ me baby one more time

17d7045

Setup CI

aaccdab

Lint fix

18da115

arm4b reviewed Mar 13, 2017

View reviewed changes

Lakshmi Kannan added 2 commits March 13, 2017 12:59

Variable name goofups (fix)

a5f2478

Merge branch 'master' into bwc_install_role

b7d84e7

* master: Include memo to revert the version pinning later Fix idempotence test in kitchen-ansible

YAML cleanup

2fd65e8

arm4b reviewed Mar 15, 2017

View reviewed changes

Fix 'no_logs' -> 'no_log' blocking the Ansible run

56d0587

arm4b reviewed Mar 15, 2017

View reviewed changes

armab and others added 7 commits March 16, 2017 00:28

Fix 'st2web' vs 'bwc-ui' conflict when enterprise is installed

c1d98a9

Fix st2web yaml

8837fff

Remove all safety checks because arma says so

48dcd6b

Use to_nice_yaml

b4843f3

Add smoke tests for BWC RBAC

76084dd

Add bwc_smoketests to stackstorm.yml

15ac7e6

lets see if this makes ansible-lint happy

9891ed2

arm4b reviewed Mar 17, 2017

View reviewed changes

Lakshmi Kannan added 3 commits March 20, 2017 12:08

Smoke tests need not be idempotent

9b5ff73

Use hash to save license info and handle when license changed

a729179

Use hash as opposed to password_hash

51ff517

lakshmi-kannan changed the title ~~WIP: Setup BWC repos, Install BWC and Setup RBAC~~ Setup BWC repos, Install BWC and Setup RBAC Mar 21, 2017

arm4b approved these changes Mar 21, 2017

View reviewed changes

lakshmi-kannan merged commit 7b1485b into master Mar 22, 2017

lakshmi-kannan deleted the bwc_install_role branch March 22, 2017 03:06

arm4b mentioned this pull request May 2, 2017

Docs update, related to Ansible-st2 0.7.0 release StackStorm/st2docs#472

Merged

9 tasks

Setup BWC repos, Install BWC and Setup RBAC #126

Setup BWC repos, Install BWC and Setup RBAC #126

Conversation

lakshmi-kannan commented Mar 7, 2017 • edited Loading

arm4b commented Mar 8, 2017

arm4b Mar 8, 2017 • edited Loading

Choose a reason for hiding this comment

arm4b commented Mar 8, 2017

arm4b commented Mar 8, 2017 • edited Loading

1) BWC role in separated repo

2) BWC in ansible-st2 repo

Choose a reason for hiding this comment

arm4b left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arm4b Mar 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arm4b commented Mar 10, 2017

arm4b commented Mar 13, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arm4b Mar 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arm4b Mar 15, 2017 • edited Loading

Choose a reason for hiding this comment

arm4b Mar 15, 2017 • edited Loading

Choose a reason for hiding this comment

arm4b Mar 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arm4b Mar 15, 2017 • edited Loading

Choose a reason for hiding this comment

arm4b Mar 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arm4b left a comment

Choose a reason for hiding this comment

arm4b commented Mar 21, 2017

lakshmi-kannan commented Mar 7, 2017 •

edited

Loading

arm4b Mar 8, 2017 •

edited

Loading

arm4b commented Mar 8, 2017 •

edited

Loading

1) `BWC` role in separated repo

2) `BWC` in `ansible-st2` repo

arm4b left a comment •

edited

Loading

arm4b Mar 10, 2017 •

edited

Loading

arm4b Mar 16, 2017 •

edited

Loading

arm4b Mar 15, 2017 •

edited

Loading

arm4b Mar 15, 2017 •

edited

Loading

arm4b Mar 15, 2017 •

edited

Loading

arm4b Mar 15, 2017 •

edited

Loading

arm4b Mar 17, 2017 •

edited

Loading