Pantsbuild: Introduce dependency rules (aka "visibility") #6133

CHANGELOG.rst

@@ -19,7 +19,7 @@ Added
 * Continue introducing `pants <https://www.pantsbuild.org/docs>`_ to improve DX (Developer Experience)
   working on StackStorm, improve our security posture, and improve CI reliability thanks in part
   to pants' use of PEX lockfiles. This is not a user-facing addition.
-  #6118 #6141
+  #6118 #6141 #6133
   Contributed by @cognifloyd
 
 3.8.1 - December 13, 2023

contrib/chatops/tests/BUILD

@@ -1,3 +1,6 @@
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))
+
 files(
     name="fixtures",
     sources=["fixtures/*.json"],

contrib/core/tests/BUILD

@@ -1,3 +1,6 @@
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))
+
 python_tests(
     skip_pylint=True,
     overrides={

contrib/examples/tests/BUILD

@@ -1,3 +1,6 @@
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))
+
 python_tests(
     skip_pylint=True,
 )

contrib/linux/tests/BUILD

@@ -1,3 +1,6 @@
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))
+
 python_tests(
     skip_pylint=True,
 )

contrib/packs/tests/BUILD

@@ -1,3 +1,6 @@
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))
+
 python_tests(
     skip_pylint=True,
 )

contrib/runners/action_chain_runner/tests/BUILD

@@ -3,3 +3,6 @@ __defaults__(
         skip_pylint=True,
     )
 )
+
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))

contrib/runners/announcement_runner/tests/BUILD

@@ -3,3 +3,6 @@ __defaults__(
         skip_pylint=True,
     )
 )
+
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))

contrib/runners/http_runner/tests/BUILD

@@ -3,3 +3,6 @@ __defaults__(
         skip_pylint=True,
     )
 )
+
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))

contrib/runners/inquirer_runner/tests/BUILD

@@ -3,3 +3,6 @@ __defaults__(
         skip_pylint=True,
     )
 )
+
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))

contrib/runners/local_runner/tests/BUILD

@@ -3,3 +3,6 @@ __defaults__(
         skip_pylint=True,
     )
 )
+
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))

contrib/runners/noop_runner/tests/BUILD

@@ -3,3 +3,6 @@ __defaults__(
         skip_pylint=True,
     )
 )
+
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))

contrib/runners/orquesta_runner/tests/BUILD

@@ -3,3 +3,6 @@ __defaults__(
         skip_pylint=True,
     )
 )
+
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))

contrib/runners/python_runner/tests/BUILD

@@ -3,3 +3,6 @@ __defaults__(
         skip_pylint=True,
     )
 )
+
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))

contrib/runners/remote_runner/tests/BUILD

@@ -3,3 +3,6 @@ __defaults__(
         skip_pylint=True,
     )
 )
+
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))

contrib/runners/winrm_runner/BUILD

@@ -1,3 +1,16 @@
+__dependents_rules__(
+    (
+        # winrm
+        {"path": "/", "name": "winrm"},
+        # can ONLY be used by sources in the winrm_runner
+        "/**",
+        # and nothing else
+        "!*",
+    ),
+    # everything else is not restricted
+    ("*", "*"),
+)
+
 python_requirement(
     name="winrm",
     requirements=["pywinrm"],

contrib/runners/winrm_runner/tests/BUILD

@@ -3,3 +3,6 @@ __defaults__(
         skip_pylint=True,
     )
 )
+
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))

lint-configs/regex-lint.yaml

@@ -12,11 +12,12 @@ required_matches:
 
   # TODO: In the future pants should get `visibility` and possibly other
   #       features to restrict imports for dependees or dependencies.
-  #       - https://github.com/pantsbuild/pants/issues/13393
-  #       - https://github.com/pantsbuild/pants/pull/15803
-  #       - https://github.com/pantsbuild/pants/pull/15836
-  #       When that happens, we can add that target metadata,
-  #       and remove these regex based dependency checks.
+  #       We now have the rules. We just need the lint backend to check regularly.
+  #       - https://github.com/pantsbuild/pants/discussions/17389 dep rules
+  #       - https://github.com/pantsbuild/pants/issues/17634 visibility stabilization
+  #       - https://www.pantsbuild.org/v2.16/docs/validating-dependencies
+  #       We can use the visibility lint backend once we upgrade to pants 2.18:
+  #       https://www.pantsbuild.org/blog/2023/11/14/pants-2.18.0-is-released#more-visible-visibility
 
   # st2client-dependencies-check
   st2client:

pants-plugins/pack_metadata/target_types.py

@@ -38,6 +38,9 @@ class PackMetadataSourcesField(ResourcesGeneratingSourcesField):
         # "requirements*.txt",  # including this causes target conflicts
         # "README.md",
         # "HISTORY.md",
+        # exclude yaml files under tests
+        "!tests/**/*.yml",
+        "!tests/**/*.yaml",
     )
 
 

pants.toml

@@ -10,6 +10,9 @@ pants_version = "2.17.1"
 pythonpath = ["%(buildroot)s/pants-plugins"]
 build_file_prelude_globs = ["pants-plugins/macros.py"]
 backend_packages = [
+  # https://www.pantsbuild.org/2.19/docs/using-pants/validating-dependencies
+  "pants.backend.experimental.visibility",
+
   # python
   "pants.backend.python",
   "pants.backend.experimental.python", # activates twine `publish` support

st2actions/tests/BUILD

@@ -3,3 +3,6 @@ __defaults__(
         skip_pylint=True,
     )
 )
+
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))

st2api/tests/BUILD

@@ -3,3 +3,6 @@ __defaults__(
         skip_pylint=True,
     )
 )
+
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))

st2auth/tests/BUILD

@@ -4,6 +4,9 @@ __defaults__(
     )
 )
 
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))
+
 python_test_utils(
     sources=["*.py"],
 )

st2client/st2client/BUILD

@@ -1 +1,17 @@
+# rules on what st2client can depend on
+__dependencies_rules__(
+    (
+        # All sources in st2client
+        "*",
+        (
+            # may depend on 3rd party dependencies,
+            "//reqs#*",
+            # and on anything in this diretory,
+            "/**",
+            # but nothing else (eg not st2common, st2*, runners, ...).
+            "!*",
+        ),
+    ),
+)
+
 python_sources()

st2client/tests/BUILD

@@ -4,6 +4,9 @@ __defaults__(
     )
 )
 
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))
+
 python_test_utils(
     sources=["*.py"],
 )

st2common/st2common/BUILD

@@ -1,3 +1,46 @@
+_ST2COMMON_DEPENDENCIES_RULE = (
+    # All selected code (see the actual rule):
+    # may depend on 3rd party dependencies
+    "//reqs#*",
+    # and on st2client
+    "//st2client/st2client/**",
+    # and on packs, runners, etc
+    "//contrib/**",
+    # and on conf files
+    "//conf/**",
+    # and on anything in this directory,
+    "/**",
+    # but nothing else (eg not st2api, st2auth, tools, ...)
+    "!*",
+)
+
+# rules on what st2commonn can depend on
+__dependencies_rules__(
+    (
+        # Only the inquiry service
+        "/services/inquiry.py",
+        # may depend on st2actions.containers.base ('?' makes this a WARNING),
+        (
+            # TODO: refactor inquiry.py to not import from st2actions
+            "?//st2actions/st2actions/container/base.py",
+        ),
+        # and may depend on code according to these rules.
+        _ST2COMMON_DEPENDENCIES_RULE,
+    ),
+    (
+        # All remaining sources in st2common
+        "*",
+        # may depend on code according to these rules
+        _ST2COMMON_DEPENDENCIES_RULE,
+    ),
+)
+
+# rules on what can depend on st2commonn
+__dependents_rules__(
+    # All sources in st2common may be a dependency of anything except st2client
+    ("*", "!//st2client/st2client/**", "*"),
+)
+
 python_sources(
     dependencies=[
         ":openapi_spec",

st2common/tests/BUILD

@@ -3,3 +3,6 @@ __defaults__(
         skip_pylint=True,
     )
 )
+
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))

st2reactor/tests/BUILD

@@ -3,3 +3,6 @@ __defaults__(
         skip_pylint=True,
     )
 )
+
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))

st2stream/tests/BUILD

@@ -3,3 +3,6 @@ __defaults__(
         skip_pylint=True,
     )
 )
+
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))

st2tests/tests/BUILD

@@ -3,3 +3,6 @@ __defaults__(
         skip_pylint=True,
     )
 )
+
+# tests can only be dependencies of other tests in this directory
+__dependents_rules__(("*", "/**", "!*"))

tools/BUILD

@@ -1,3 +1,19 @@
+__dependents_rules__(
+    (
+        # graphviz and pika
+        (
+            {"path": "/", "name": "graphviz"},
+            {"path": "/", "name": "pika"},
+        ),
+        # can used by tools in this directory
+        "/**",
+        # and nothing else
+        "!*",
+    ),
+    # everything else is not restricted
+    ("*", "*"),
+)
+
 python_requirement(
     name="graphviz",
     requirements=["graphviz"],