Refactor ansible #78

frankduncan · 2020-11-04T17:11:55Z

Wow, 6100 lines of code removed! This fixes #59, #62, and #64, in a formal way, in various commits in the chain.

Go ahead and review as you like, though I'll be merging in the next few days before I upgrade mediawiki and push this all to production.

Because roles are getting centralized, the documentation needs to be updated with how and how to configure it.

For centralizing roles, Climate2030 being the most recent competition was the best candidate to base the refactoring on. The refactoring took the form of splitting out the main.yml from the mediawiki task into many main.yml files in different roles.

A lot of the configuration was inline blocks that were dumped directly into LocalSettings.php. This change separates those configurations out into their own files for easier maintenance and documentation. Issue #59: Change MediaWiki configurations to have multiple files

Clean it up so things are a bit more declarative, as well as moving the simpelsaml group config code over to the permissions, so that the separation of concerns is a bit more straightforward. Issue #62: Create security model for torque-sites Issue #64: Pluralize the names of the abstract user groups

With the ansible configuration centralized into a top level torque-sites directory (roles), the majority of the tasks main.yml and supporting files can be removed with the roles being added to the top level playbook. This required a few changes to the base pipeline: * Add composer to mediawiki install * Composer version update * Add postgres base package * Add LFC Evaluators as a known SAML group * Add pre-req installation for mediawiki Then removing all the old stuff from 100Change2017 and updating the yml file to reference the general stuff.

With the ansible configuration centralized into a top level torque-sites directory (roles), the majority of the tasks main.yml and supporting files can be removed with the roles being added to the top level playbook. This required a change to the base roles: * torquedataconnect_sheet_name can be different from the competition

With the ansible configuration centralized into a top level torque-sites directory (roles), the majority of the tasks main.yml and supporting files can be removed with the roles being added to the top level playbook. This required a change to the base roles: * Board Members from Okta are Decision Makers

With the ansible configuration centralized into a top level torque-sites directory (roles), the majority of the tasks main.yml and supporting files can be removed with the roles being added to the top level playbook.

This removes a lot of formatting that hurts printing out pages, especially those that have been heavily styled.

With the ansible configuration centralized into a top level torque-sites directory (roles), the majority of the tasks main.yml and supporting files can be removed with the roles being added to the top level playbook.

This brings it in line with the other competitions naming scheme.

We added this for instances where the competition name does not match the sheet, but it needed to be backported to previously done competitions.

With the ansible configuration centralized into a top level torque-sites directory (roles), the majority of the tasks main.yml and supporting files can be removed with the roles being added to the top level playbook. This required a change to the base roles: * Add rss as a base role for the RSS extension * Add api roles to ase permissions * Fill out some roles (like psuedo board member) * Add board members to okta groups we knwo about * Add login override * Respect local login flag when rebuilding the login page

kfogel · 2020-11-04T17:41:56Z

100Change2017Partners/ansible/inv/local/group_vars/all.tmpl

+# this is different when either the competition name doesn't work as
+# a sheet name (for instance, if it starts with a number), or if
+# this wiki should act on a different sheet.
+torquedataconnect_sheet_name: LFC100Change2017


I paused for a moment on that sheet_name, of course, but now that I think about it it makes sense. Whew.

kfogel · 2020-11-04T17:43:36Z

100Change2020/ansible/100Change2020.yml

+    - embed_video
+    - permissions
+    - simplefavorites
+    # PickSome has to come after collection so the left menu bar is correct


Is there some way that we could have PickSome notice whether Collection has already been loaded, so that PickSome can just raise an error if that requirement hasn't been met? It's better if the code can protect us against these things automagically, rather than us having to remember via comments, if possible.

I can add something to our Collection config that will error if PickSome is already installed. I'll put it on my todo list, because I don't think it needs to happen in this PR.

kfogel · 2020-11-04T17:44:01Z

100Change2020/ansible/inv/dev/group_vars/all/base

@@ -1,4 +1,6 @@
 ---
+competition_name: lfc


What's up with this .../all/base that has variables whose values get overridden later?

base is the configuration that can get checked in for production, so this doesn't get overridden later, as this is IS the overriding configuration. "secret" is passwords and whatnot that shouldn't be. See https://github.com/OpenTechStrategies/torque-sites/blob/main/INSTALL.md#production-installs-for-ots-only for more information.

kfogel · 2020-11-04T17:46:35Z

100Change2020/ansible/inv/dev/group_vars/all/base

@@ -1,4 +1,6 @@
 ---
+competition_name: lfc
+torquedataconnect_sheet_name: proposals


kfogel · 2020-11-04T17:47:02Z

100Change2020/ansible/inv/prod/group_vars/all/base

@@ -1,4 +1,6 @@
 ---
+competition_name: lfc


Same question...

kfogel · 2020-11-04T18:05:04Z

roles/picksome/files/picksome.php

+
+$wgLFCPickSomeEligiblePage = "TorqueConfig:ValidProposals";
+
+# This may not be performant and require caching if number of users/page hits


"This may not be performant, and it may require caching if number of users/page hits becomes large."

kfogel · 2020-11-04T18:05:57Z

roles/picksome/files/picksome.php

+  return $text1 > $text2;
+};
+
+# These are defaults for competitions, and probably will be overridden at some point


When you say "probably will be overridden at some point", you mean "probably will be overridden by competition-specific settings within a given competition", right?

Yes, exactly. Updated to be more clear.

kfogel · 2020-11-04T18:06:34Z

roles/simplefavorites/tasks/main.yml

@@ -0,0 +1,12 @@
+---
+
+- name: Checkout SimpleFavorites


Hill. Dying on.

kfogel · 2020-11-04T18:07:15Z

roles/simplesaml/templates/simplesaml.php.j2

+<?php
+
+# When including this file, a competition needs to override $wgPluggableAuth_EnableLocalLogin
+# before this file is loaded.  That's because the file takes care of the local login code


"because this file"

kfogel · 2020-11-04T18:08:01Z

roles/teamcomments/tasks/main.yml

@@ -0,0 +1,19 @@
+---
+
+- name: Checkout TeamComments


Not saying anything more about hills. Maybe I'm just receiving medical treatment on them, and there is still hope.

I'm really tempted to leave this one, just because you were less committed to it than the others.

See #78 for the conversation. This is mostly to clear up some confusing comments and fix typos/grammattical errors.

frankduncan · 2020-11-08T23:52:54Z

Thanks so much for the review! I updated many things and am merging, but feel free to ping me about anything that's still unclear. (I need to merge so I can get the next competition loaded in)

Part of the code review for #78: refactoring ansible. See #78 (comment) for more information.

See #78 for the conversation. This is mostly to clear up some confusing comments and fix typos/grammattical errors.

Part of the code review for #78: refactoring ansible. See #78 (comment) for more information.

Frank Duncan added 15 commits October 28, 2020 09:56

Create documentation about upcoming ansible refactor

5e632f0

Because roles are getting centralized, the documentation needs to be updated with how and how to configure it.

Use the new refactored ansible with EO2020

a699c32

With the ansible configuration centralized into a top level torque-sites directory (roles), the majority of the tasks main.yml and supporting files can be removed with the roles being added to the top level playbook.

Remove accidentally checked in retry file

00dbadc

Update Common.css to have printing improvements

cec6f41

This removes a lot of formatting that hurts printing out pages, especially those that have been heavily styled.

Use the new refactored ansible with LLIIA2020

43da257

With the ansible configuration centralized into a top level torque-sites directory (roles), the majority of the tasks main.yml and supporting files can be removed with the roles being added to the top level playbook.

Use the new refactored ansible with LoneStar2020

2fce530

With the ansible configuration centralized into a top level torque-sites directory (roles), the majority of the tasks main.yml and supporting files can be removed with the roles being added to the top level playbook.

Capitalize climate2030 ansible role directory

1bfaca5

This brings it in line with the other competitions naming scheme.

Add torquedataconnect_sheet_name to already refactored competisions

2cb945f

We added this for instances where the competition name does not match the sheet, but it needed to be backported to previously done competitions.

frankduncan requested a review from kfogel November 4, 2020 17:11

kfogel approved these changes Nov 4, 2020

View reviewed changes

kfogel force-pushed the main branch 4 times, most recently from 9fb3000 to c26b58e Compare November 5, 2020 15:22

Updates to documentation based on code review

2b92fd3

See #78 for the conversation. This is mostly to clear up some confusing comments and fix typos/grammattical errors.

frankduncan merged commit 33ca52b into main Nov 8, 2020

frankduncan pushed a commit that referenced this pull request Nov 11, 2020

Check in code that picksome/collection are loaded in the right order

4515776

Part of the code review for #78: refactoring ansible. See #78 (comment) for more information.

frankduncan deleted the refactor-ansible branch November 18, 2020 12:52

frankduncan pushed a commit that referenced this pull request Apr 7, 2021

Updates to documentation based on code review

8e70e5e

See #78 for the conversation. This is mostly to clear up some confusing comments and fix typos/grammattical errors.

frankduncan pushed a commit that referenced this pull request Apr 7, 2021

Check in code that picksome/collection are loaded in the right order

6f8229e

Part of the code review for #78: refactoring ansible. See #78 (comment) for more information.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor ansible #78

Refactor ansible #78

frankduncan commented Nov 4, 2020

kfogel Nov 4, 2020

kfogel Nov 4, 2020

frankduncan Nov 8, 2020

kfogel Nov 4, 2020

frankduncan Nov 8, 2020

kfogel Nov 4, 2020

kfogel Nov 4, 2020

kfogel Nov 4, 2020

kfogel Nov 4, 2020

frankduncan Nov 8, 2020

kfogel Nov 4, 2020

kfogel Nov 4, 2020

kfogel Nov 4, 2020

frankduncan Nov 8, 2020 •

edited

Loading

frankduncan commented Nov 8, 2020


		$wgLFCPickSomeEligiblePage = "TorqueConfig:ValidProposals";

		# This may not be performant and require caching if number of users/page hits

Refactor ansible #78

Refactor ansible #78

Conversation

frankduncan commented Nov 4, 2020

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frankduncan Nov 8, 2020 • edited Loading

Choose a reason for hiding this comment

frankduncan commented Nov 8, 2020

frankduncan Nov 8, 2020 •

edited

Loading