Fix #1100: Fix steps and add a 'jbi lint' command to make sure we don't fall into this anymore (#1104)

* Add 'jbi lint' command

* Lock dependencies

* Run lint on all envs by default

* Check order of steps

* Factorize status and resolution maps

* Check that status and resolution maps are provided

* Do not require settings environment vars to just run linting

Author: leplatrem

bin/lint.sh

@@ -18,6 +18,8 @@ MYPY_CMD="$POETRY_RUN mypy jbi"
 
 YAMLLINT_CMD="$POETRY_RUN yamllint -c .yamllint config/*.yaml"
 
+ACTIONS_LINT_CMD="$POETRY_RUN jbi lint"
+
 all () {
   echo "running bandit"
   $BANDIT_CMD
@@ -31,6 +33,8 @@ all () {
   $MYPY_CMD
   echo "running yamllint"
   $YAMLLINT_CMD
+  echo "running actions lint"
+  $ACTIONS_LINT_CMD
 }
 
 usage () {
@@ -43,6 +47,7 @@ usage () {
   echo "  lint"
   echo "  mypy"
   echo "  yamllint"
+  echo "  actions"
 }
 
 if [ -z "$1" ]; then
@@ -78,6 +83,9 @@ else
     "detect-secrets")
       $DETECT_SECRETS_CMD
       ;;
+    "actions")
+      $ACTIONS_LINT_CMD
+      ;;
     *)
       usage
       ;;

config/config.nonprod.yaml

@@ -14,14 +14,12 @@
         - add_link_to_bugzilla
         - add_link_to_jira
         - maybe_assign_jira_user
-        - maybe_update_issue_status
         - maybe_update_issue_severity
         - sync_whiteboard_labels
         - sync_keywords_labels
       existing:
         - update_issue_summary
         - maybe_assign_jira_user
-        - maybe_update_issue_status
         - maybe_update_issue_severity
         - sync_whiteboard_labels
         - sync_keywords_labels

config/config.prod.yaml

@@ -150,21 +150,21 @@
         - add_link_to_jira
         - maybe_assign_jira_user
         - maybe_update_components
-        - maybe_update_issue_resolution
         - maybe_update_issue_status
+        - maybe_update_issue_resolution
         - sync_whiteboard_labels
         - sync_keywords_labels
       existing:
         - update_issue_summary
         - maybe_update_components
         - maybe_assign_jira_user
-        - maybe_update_issue_resolution
         - maybe_update_issue_status
+        - maybe_update_issue_resolution
         - sync_whiteboard_labels
         - sync_keywords_labels
       comment:
         - create_comment
-    status_map:
+    status_map: &basic-status-map
       UNCONFIRMED: Backlog
       NEW: Backlog
       ASSIGNED: In Progress
@@ -179,7 +179,7 @@
       WORKSFORME: Done
       INCOMPLETE: Done
       MOVED: Done
-    resolution_map:
+    resolution_map: &basic-resolution-map
       FIXED: Done
       INVALID: Invalid
       WONTFIX: "Won't Do"
@@ -337,30 +337,8 @@
         - sync_keywords_labels
       comment:
         - create_comment
-    status_map:
-      UNCONFIRMED: Backlog
-      NEW: Backlog
-      ASSIGNED: In Progress
-      REOPENED: In Progress
-      RESOLVED: Done
-      VERIFIED: Done
-      FIXED: Done
-      INVALID: Done
-      WONTFIX: Done
-      INACTIVE: Done
-      DUPLICATE: Done
-      WORKSFORME: Done
-      INCOMPLETE: Done
-      MOVED: Done
-    resolution_map:
-      FIXED: Done
-      INVALID: Invalid
-      WONTFIX: "Won't Do"
-      INACTIVE: Inactive
-      DUPLICATE: Duplicate
-      WORKSFORME: "Cannot Reproduce"
-      INCOMPLETE: Incomplete
-      MOVED: Moved
+    status_map: *basic-status-map
+    resolution_map: *basic-resolution-map
 
 - whiteboard_tag: proton
   bugzilla_user_id: tbd
@@ -488,16 +466,16 @@
         - add_link_to_jira
         - maybe_assign_jira_user
         - maybe_update_components
-        - maybe_update_issue_resolution
         - maybe_update_issue_status
+        - maybe_update_issue_resolution
         - sync_whiteboard_labels
         - sync_keywords_labels
       existing:
         - update_issue_summary
         - maybe_assign_jira_user
         - maybe_update_components
-        - maybe_update_issue_resolution
         - maybe_update_issue_status
+        - maybe_update_issue_resolution
         - sync_whiteboard_labels
         - sync_keywords_labels
       comment:
@@ -517,15 +495,7 @@
       WORKSFORME: Done
       INCOMPLETE: Done
       MOVED: Done
-    resolution_map:
-      FIXED: Done
-      INVALID: Invalid
-      WONTFIX: "Won't Do"
-      INACTIVE: Inactive
-      DUPLICATE: Duplicate
-      WORKSFORME: "Cannot Reproduce"
-      INCOMPLETE: Incomplete
-      MOVED: Moved
+    resolution_map: *basic-resolution-map
 
 - whiteboard_tag: dataplatform
   bugzilla_user_id: tbd
@@ -550,21 +520,7 @@
         - maybe_update_issue_status
       comment:
         - create_comment
-    status_map:
-      UNCONFIRMED: Backlog
-      NEW: Backlog
-      ASSIGNED: In Progress
-      REOPENED: In Progress
-      RESOLVED: Done
-      VERIFIED: Done
-      FIXED: Done
-      INVALID: Done
-      WONTFIX: Done
-      INACTIVE: Done
-      DUPLICATE: Done
-      WORKSFORME: Done
-      INCOMPLETE: Done
-      MOVED: Done
+    status_map: *basic-status-map
 
 - whiteboard_tag: dataquality
   bugzilla_user_id: tbd
@@ -582,12 +538,10 @@
         - add_link_to_bugzilla
         - add_link_to_jira
         - maybe_assign_jira_user
-        - maybe_update_issue_resolution
         - maybe_update_issue_status
       existing:
         - update_issue_summary
         - maybe_assign_jira_user
-        - maybe_update_issue_resolution
         - maybe_update_issue_status
       comment:
         - create_comment
@@ -703,18 +657,4 @@
       enhancement: Story
       task: Task
       defect: Bug
-    status_map:
-      UNCONFIRMED: Backlog
-      NEW: Backlog
-      ASSIGNED: In Progress
-      REOPENED: In Progress
-      RESOLVED: Done
-      VERIFIED: Done
-      FIXED: Done
-      INVALID: Done
-      WONTFIX: Done
-      INACTIVE: Done
-      DUPLICATE: Done
-      WORKSFORME: Done
-      INCOMPLETE: Done
-      MOVED: Done
+    status_map: *basic-status-map

jbi/__main__.py

@@ -0,0 +1,27 @@
+import click
+
+from jbi.configuration import get_actions
+
+
+@click.group()
+def cli():
+    pass
+
+
+@cli.command()
+@click.argument("env", default="all")
+def lint(env):
+    click.echo(f"Linting: {env} action configuration")
+
+    if env == "all":
+        envs = ["local", "nonprod", "prod"]
+    else:
+        envs = [env]
+
+    for env in envs:
+        get_actions(env)
+        click.secho(f"No issues found for {env}.", fg="green")
+
+
+if __name__ == "__main__":
+    cli()

jbi/app.py

@@ -23,14 +23,15 @@
 
 import jbi
 import jbi.queue
-from jbi.configuration import ACTIONS
+from jbi.configuration import get_actions
 from jbi.environment import get_settings
 from jbi.log import CONFIG
 from jbi.router import router
 
 SRC_DIR = Path(__file__).parent
 APP_DIR = Path(__file__).parents[1]
 
+ACTIONS = get_actions()
 settings = get_settings()
 version_info: dict[str, str] = get_version(APP_DIR)
 VERSION: str = version_info["version"]
@@ -60,8 +61,8 @@ def traces_sampler(sampling_context: dict[str, Any]) -> float:
 # https://github.com/tiangolo/fastapi/discussions/9241
 @asynccontextmanager
 async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]:
-    jira_service = jbi.jira.get_service()
-    bugzilla_service = jbi.bugzilla.get_service()
+    jira_service = jbi.jira.service.get_service()
+    bugzilla_service = jbi.bugzilla.service.get_service()
     queue = jbi.queue.get_dl_queue()
 
     checks.register(bugzilla_service.check_bugzilla_connection, name="bugzilla.up")

jbi/bugzilla/__init__.py

@@ -1,7 +0,0 @@
-from .models import (
-    Bug,
-    BugId,
-    WebhookEvent,
-    WebhookRequest,
-)
-from .service import BugzillaService, get_service

jbi/configuration.py

@@ -10,7 +10,6 @@
 from jbi import environment
 from jbi.models import Actions
 
-settings = environment.get_settings()
 logger = logging.getLogger(__name__)
 
 
@@ -30,9 +29,9 @@ def get_actions_from_file(jbi_config_file: str) -> Actions:
         raise ConfigError("Errors exist.") from exception
 
 
-def get_actions(env=settings.env):
+def get_actions(env=None) -> Actions:
     """Load actions from file determined by ENV name"""
+    if env is None:
+        settings = environment.get_settings()
+        env = settings.env
     return get_actions_from_file(f"config/config.{env}.yaml")
-
-
-ACTIONS = get_actions()

jbi/jira/service.py

@@ -14,7 +14,8 @@
 from dockerflow import checks
 from requests import exceptions as requests_exceptions
 
-from jbi import Operation, bugzilla, environment
+from jbi import Operation, environment
+from jbi.bugzilla import models as bugzilla_models
 from jbi.jira.utils import markdown_to_jira
 from jbi.models import ActionContext
 
@@ -184,7 +185,7 @@ def add_jira_comments_for_changes(self, context: ActionContext):
         return jira_response_comments
 
     def delete_jira_issue_if_duplicate(
-        self, context: ActionContext, latest_bug: bugzilla.Bug
+        self, context: ActionContext, latest_bug: bugzilla_models.Bug
     ):
         """Rollback the Jira issue creation if there is already a linked Jira issue
         on the Bugzilla ticket"""

jbi/models.py

@@ -18,7 +18,7 @@
 )
 
 from jbi import Operation, steps
-from jbi.bugzilla import Bug, BugId, WebhookEvent
+from jbi.bugzilla.models import Bug, BugId, WebhookEvent
 
 logger = logging.getLogger(__name__)
 
@@ -51,14 +51,25 @@ class ActionSteps(BaseModel, frozen=True):
     @classmethod
     def validate_steps(cls, function_names: list[str]):
         """Validate that all configure step functions exist in the steps module"""
-
         invalid_functions = [
             func_name for func_name in function_names if not hasattr(steps, func_name)
         ]
         if invalid_functions:
             raise ValueError(
                 f"The following functions are not available in the `steps` module: {', '.join(invalid_functions)}"
             )
+
+        # Make sure `maybe_update_resolution` comes after `maybe_update_status`.
+        try:
+            idx_resolution = function_names.index("maybe_update_issue_resolution")
+            idx_status = function_names.index("maybe_update_issue_status")
+            assert idx_resolution > idx_status, (
+                "Step `maybe_update_resolution` should be put after `maybe_update_issue_status`"
+            )
+        except ValueError:
+            # One of these 2 steps not listed.
+            pass
+
         return function_names
 
 
@@ -191,6 +202,16 @@ def validate_actions(cls, actions: list[Action]):
                     f"Provide bugzilla_user_id data for `{action.whiteboard_tag}` action."
                 )
 
+            assert action.parameters.status_map or (
+                "maybe_update_issue_status" not in action.parameters.steps.new
+                and "maybe_update_issue_status" not in action.parameters.steps.existing
+            ), "`maybe_update_issue_status` was used without `status_map`"
+            assert action.parameters.resolution_map or (
+                "maybe_update_issue_resolution" not in action.parameters.steps.new
+                and "maybe_update_issue_resolution"
+                not in action.parameters.steps.existing
+            ), "`maybe_update_issue_resolution` was used without `resolution_map`"
+
         return actions
 
     model_config = ConfigDict(ignored_types=(functools.cached_property,))