Refactor all rego (#141)

* Refactor long lines; refactor GetTopLevelOU function * Unify comment style * Remove 'no domain' helper rules * Change ServiceName to SettingName * Refactor test cases * Change lists to sets where possible * Chat change to for in syntax * Chat change to rule contains syntax * Refactor GetTopLevelOU function * Change lists to sets where possible chat * Remove trailing whitespace * Change ServiceName to SettingName * Standardize comment syntax chat * Refactor chat unit tests * Change to for in syntax * Change to rule contains syntax classroom * Change ServiceName to SettingName * Classroom change lists to sets where possible * Classroom standardize comments * Refactor GetTopLevelOU function Classroom * Refactor Classroom unit tests * Remove stray blank lines classroom * Refactor common functions Drive * Drive change to some in syntax * Remove trailing whitespace * Remove more trailing whitespace * Change to rule contains syntax Drive * Refactor long lines Drive * Drive standardize comments * Refactor Drive unit tests * Refactor drive 2 long lines * Refactor groups unit tests * Groups standardize comments * Rego refactor meet * Refactor 'or' Drive * Refactor sites rego * Refactor utils rego * Move OUsWithEvents rule to the utils file * Calendar fill in topLevelOU in test cases with multiple OU as it cannot be blank in those cases * Chat fill in empty topLevelOU in test cases where needed * Classroom fill in empty topLevelOU in test cases where needed * Drive unit tests fill in topLevelOU where required * Gmail test cases add topLevelOU when needed * groups test cases add topLevelOU when needed * Meet test cases add topLevelOU when needed * Move TopLevelOU rule to utils file * Remove print statements * Move GetLastEvent function to utils file * Moved all shared rego code to the utils file * Import utils package instead of individual rules * push unsaved change * Remove duplicates from list * Change SettingChangeEvents back to filter by log name for efficiency * Refactor groups 7 * Ensure test names are unique * Add regal config file * Add github workflow for regal * Add missing ifs * Bump the version of the regal exe * Trim trailing whitespace rego/Utils.rego Co-authored-by: David Bui <[email protected]> * Trim trailing whitespace rego/Utils.rego Co-authored-by: David Bui <[email protected]> * Correct the ID used in comment rego/Chat.rego Co-authored-by: David Bui <[email protected]> * Move TopLevelOU comments for consistency * Remove trailing whitespace rego/Utils.rego Co-authored-by: David Bui <[email protected]> * Add new line for readability and consistency * Sites change lists to sets * Remove leading whitespace rego/Groups.rego Co-authored-by: David Bui <[email protected]> * Align whitespace rego/Classroom.rego Co-authored-by: David Bui <[email protected]> * Groups align whitespace * Refactor 'ignore ous without events' comment * Remove trailing whitespace from Rego unit tests --------- Co-authored-by: David Bui <[email protected]>
cisagov · Jan 15, 2024 · fa4433c · fa4433c
1 parent 699f906
commit fa4433c
Show file tree

Hide file tree

Showing 64 changed files with 3,865 additions and 4,460 deletions.
diff --git a/.github/workflows/run_opa_tests.yml b/.github/workflows/run_opa_tests.yml
@@ -32,3 +32,11 @@ jobs:
 
     - name: Run OPA Tests
       run: opa test rego/*.rego Testing/RegoTests/**/*.rego -v
+
+    - name: Setup Regal
+      uses: StyraInc/[email protected]
+      with:
+        version: v0.15.0
+
+    - name: Run Regal Lint
+      run: regal lint --format github rego Testing
diff --git a/.regal/config.yaml b/.regal/config.yaml
@@ -0,0 +1,60 @@
+# Regal linter configuration
+# All rules documented here:
+# https://docs.styra.com/regal/category/rules
+rules:
+  custom:
+    naming-convention:
+      # Codifying the conventions used in this project.
+      # By all means, change this to be less restrictive
+      # if you wish.
+      level: error
+      conventions:
+        - pattern: '^[A-Z]+[a-zA-Z0-9_]+$|^tests$|^test_|^check_'
+          targets:
+            - rule
+        - pattern: '^[A-Z]+[a-zA-Z0-9_]+$'
+          targets:
+            - function
+        - pattern: '^[a-z]+$'
+          targets:
+            - package
+  idiomatic:
+    no-defined-entrypoint:
+      # This is a good practice for documentation, and
+      # compilation to Wasm/IR, but not a requirement.
+      # Safe to ignore.
+      level: ignore
+  imports:
+    implicit-future-keywords:
+      # This rule is on it's way out anyway, as future
+      # versions of OPA will make these keywords standard.
+      level: ignore
+  style:
+    external-reference:
+      # This rule is quite opinionated / style preference
+      # safe to ignore.
+      level: ignore
+    file-length:
+      level: ignore
+    no-whitespace-comment:
+      # This repo is actually good about this, but frequently
+      # uses '#--' as a delimeter of sorts. That should be OK,
+      # and the next version of Regal will allow for exceptions
+      # like this: https://github.com/StyraInc/regal/issues/379
+      level: ignore
+    opa-fmt:
+      level: ignore
+    prefer-snake-case:
+      # This is the default style preference for Rego, but since
+      # the style of this project better matches the domain it is
+      # modeling, we'll ignore this rule in favor of the custom
+      # naming-convention rule defined above.
+      level: ignore
+    rule-length:
+      level: ignore
+    todo-comment:
+      level: ignore
+  testing:
+    test-outside-test-package:
+      # This is just a style preference
+      level: ignore
diff --git a/Testing/RegoTests/calendar/calendar01_test.rego b/Testing/RegoTests/calendar/calendar01_test.rego
@@ -26,7 +26,7 @@ test_ExtSharingPrimaryCal_Correct_V1 if {
         }
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     RuleOutput[0].RequirementMet
     not RuleOutput[0].NoSuchEvent
@@ -66,7 +66,7 @@ test_ExtSharingPrimaryCal_Correct_V2 if {
         }
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     RuleOutput[0].RequirementMet
     not RuleOutput[0].NoSuchEvent
@@ -102,11 +102,11 @@ test_ExtSharingPrimaryCal_Correct_V3 if {
             }
         ]},
         "tenant_info": {
-            "topLevelOU": ""
+            "topLevelOU": "Test Top-Level OU"
         }
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     RuleOutput[0].RequirementMet
     not RuleOutput[0].NoSuchEvent
@@ -153,18 +153,18 @@ test_ExtSharingPrimaryCal_Correct_V4 if {
             }
         ]},
         "tenant_info": {
-            "topLevelOU": ""
+            "topLevelOU": "Test Top-Level OU"
         }
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     RuleOutput[0].RequirementMet
     not RuleOutput[0].NoSuchEvent
     RuleOutput[0].ReportDetails == "Requirement met in all OUs."
 }
 
-test_ExtSharingPrimaryCal_Correct_V4 if {
+test_ExtSharingPrimaryCal_Correct_V5 if {
     # Test external sharing for primary, inherit from parent
     PolicyId := "GWS.CALENDAR.1.1v0.1"
     Output := tests with input as {
@@ -204,11 +204,11 @@ test_ExtSharingPrimaryCal_Correct_V4 if {
             }
         ]},
         "tenant_info": {
-            "topLevelOU": ""
+            "topLevelOU": "Test Top-Level OU"
         }
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     RuleOutput[0].RequirementMet
     not RuleOutput[0].NoSuchEvent
@@ -237,11 +237,15 @@ test_ExtSharingPrimaryCal_Incorrect_V1 if {
         }
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     not RuleOutput[0].RequirementMet
     RuleOutput[0].NoSuchEvent
-    RuleOutput[0].ReportDetails == "No relevant event in the current logs for the top-level OU, Test Top-Level OU. While we are unable to determine the state from the logs, the default setting is non-compliant; manual check recommended."
+    RuleOutput[0].ReportDetails == concat("", [
+        "No relevant event in the current logs for the top-level OU, Test Top-Level OU. ",
+        "While we are unable to determine the state from the logs, the default setting ",
+        "is non-compliant; manual check recommended."
+    ])
 }
 
 test_ExtSharingPrimaryCal_Incorrect_V2 if {
@@ -266,7 +270,7 @@ test_ExtSharingPrimaryCal_Incorrect_V2 if {
         }
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     not RuleOutput[0].RequirementMet
     not RuleOutput[0].NoSuchEvent
@@ -306,15 +310,16 @@ test_ExtSharingPrimaryCal_Incorrect_V3 if {
         },
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     not RuleOutput[0].RequirementMet
     not RuleOutput[0].NoSuchEvent
     RuleOutput[0].ReportDetails == "Requirement failed in Test Top-Level OU."
 }
 
 test_ExtSharingPrimaryCal_Incorrect_V4 if {
-    # Test external sharing for primary calendars when there is no event for the Top-level OU but there is one for a different OU
+    # Test external sharing for primary calendars when there is no event for the Top-level OU
+    # but there is one for a different OU
     PolicyId := "GWS.CALENDAR.1.1v0.1"
     Output := tests with input as {
         "calendar_logs": {"items": [
@@ -335,15 +340,20 @@ test_ExtSharingPrimaryCal_Incorrect_V4 if {
         },
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     not RuleOutput[0].RequirementMet
     RuleOutput[0].NoSuchEvent
-    RuleOutput[0].ReportDetails == "No relevant event in the current logs for the top-level OU, Test Top-Level OU. While we are unable to determine the state from the logs, the default setting is non-compliant; manual check recommended."
+    RuleOutput[0].ReportDetails == concat("", [
+        "No relevant event in the current logs for the top-level OU, Test Top-Level OU. ",
+        "While we are unable to determine the state from the logs, the default setting ",
+        "is non-compliant; manual check recommended."
+    ])
 }
 
 test_ExtSharingPrimaryCal_Incorrect_V5 if {
-    # Test external sharing for primary calendars when the Top-Level OU is compliant, but a secondary OU is non-compliant
+    # Test external sharing for primary calendars when the Top-Level OU is compliant,
+    # but a secondary OU is non-compliant
     PolicyId := "GWS.CALENDAR.1.1v0.1"
     Output := tests with input as {
         "calendar_logs": {"items": [
@@ -375,13 +385,11 @@ test_ExtSharingPrimaryCal_Incorrect_V5 if {
         },
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     not RuleOutput[0].RequirementMet
     not RuleOutput[0].NoSuchEvent
-    print(RuleOutput[0].ReportDetails)
     RuleOutput[0].ReportDetails == "Requirement failed in Secondary OU."
-
 }
 
 #
@@ -399,7 +407,7 @@ test_External_Sharing_Options_V1 if {
         }
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     not RuleOutput[0].RequirementMet
     RuleOutput[0].NoSuchEvent

diff --git a/Testing/RegoTests/calendar/calendar02_test.rego b/Testing/RegoTests/calendar/calendar02_test.rego
@@ -26,7 +26,7 @@ test_ExtInvitationsWarning_Correct_V1 if {
         }
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     RuleOutput[0].RequirementMet
     not RuleOutput[0].NoSuchEvent
@@ -66,7 +66,7 @@ test_ExtInvitationsWarning_Correct_V2 if {
         }
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     RuleOutput[0].RequirementMet
     not RuleOutput[0].NoSuchEvent
@@ -102,11 +102,11 @@ test_ExtInvitationsWarning_Correct_V3 if {
             }
         ]},
         "tenant_info": {
-            "topLevelOU": ""
+            "topLevelOU": "Test Top-Level OU"
         }
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     RuleOutput[0].RequirementMet
     not RuleOutput[0].NoSuchEvent
@@ -153,11 +153,11 @@ test_ExtInvitationsWarning_Correct_V4 if {
             }
         ]},
         "tenant_info": {
-            "topLevelOU": ""
+            "topLevelOU": "Test Top-Level OU"
         }
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     RuleOutput[0].RequirementMet
     not RuleOutput[0].NoSuchEvent
@@ -186,11 +186,15 @@ test_ExtInvitationsWarning_Incorrect_V1 if {
         }
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     not RuleOutput[0].RequirementMet
     RuleOutput[0].NoSuchEvent
-    RuleOutput[0].ReportDetails == "No relevant event in the current logs for the top-level OU, Test Top-Level OU. While we are unable to determine the state from the logs, the default setting is non-compliant; manual check recommended."
+    RuleOutput[0].ReportDetails == concat("", [
+        "No relevant event in the current logs for the top-level OU, Test Top-Level OU. ",
+        "While we are unable to determine the state from the logs, the default setting ",
+        "is non-compliant; manual check recommended."
+    ])
 }
 
 test_ExtInvitationsWarning_Incorrect_V2 if {
@@ -215,7 +219,7 @@ test_ExtInvitationsWarning_Incorrect_V2 if {
         }
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     not RuleOutput[0].RequirementMet
     not RuleOutput[0].NoSuchEvent
@@ -255,7 +259,7 @@ test_ExtInvitationsWarning_Incorrect_V3 if {
         },
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     not RuleOutput[0].RequirementMet
     not RuleOutput[0].NoSuchEvent
@@ -284,11 +288,15 @@ test_ExtInvitationsWarning_Incorrect_V4 if {
         },
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     not RuleOutput[0].RequirementMet
     RuleOutput[0].NoSuchEvent
-    RuleOutput[0].ReportDetails == "No relevant event in the current logs for the top-level OU, Test Top-Level OU. While we are unable to determine the state from the logs, the default setting is non-compliant; manual check recommended."
+    RuleOutput[0].ReportDetails == concat("", [
+        "No relevant event in the current logs for the top-level OU, Test Top-Level OU. ",
+        "While we are unable to determine the state from the logs, the default setting ",
+        "is non-compliant; manual check recommended."
+    ])
 }
 
 test_ExtInvitationsWarning_Incorrect_V5 if {
@@ -324,11 +332,10 @@ test_ExtInvitationsWarning_Incorrect_V5 if {
         },
     }
 
-    RuleOutput := [Result | Result = Output[_]; Result.PolicyId == PolicyId]
+    RuleOutput := [Result | some Result in Output; Result.PolicyId == PolicyId]
     count(RuleOutput) == 1
     not RuleOutput[0].RequirementMet
     not RuleOutput[0].NoSuchEvent
     RuleOutput[0].ReportDetails == "Requirement failed in Secondary OU."
 }
-
 #--