[role-name] Handle string role dependencies (#4054)

Co-authored-by: Sorin Sbarnea <ssbarnea@redhat.com>
ansible · Mar 8, 2024 · 6d247c1 · 6d247c1
1 parent d65aeed
commit 6d247c1
Show file tree

Hide file tree

Showing 6 changed files with 99 additions and 7 deletions.
diff --git a/examples/roles/role_with_deps_paths/meta/main.yml b/examples/roles/role_with_deps_paths/meta/main.yml
@@ -7,3 +7,4 @@ dependencies:
     vars:
       param: baz
   - role: subfolder/2nd_role
+  - subfolder/3rd_role
diff --git a/src/ansiblelint/rules/role_name.py b/src/ansiblelint/rules/role_name.py
@@ -97,13 +97,19 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
 
         if file.kind == "meta":
             for role in file.data.get("dependencies", []):
-                role_name = role["role"]
+                if isinstance(role, dict):
+                    role_name = role["role"]
+                elif isinstance(role, str):
+                    role_name = role
+                else:
+                    msg = "Role dependency has unexpected type."
+                    raise RuntimeError(msg)
                 if "/" in role_name:
                     result.append(
                         self.create_matcherror(
                             f"Avoid using paths when importing roles. ({role_name})",
                             filename=file,
-                            lineno=role["__line__"],
+                            lineno=role_name.ansible_pos[1],
                             tag=f"{self.id}[path]",
                         ),
                     )
@@ -187,7 +193,7 @@ def test_role_name_path(
 
     @pytest.mark.parametrize(
         ("test_file", "failure"),
-        (pytest.param("examples/roles/role_with_deps_paths", 2, id="fail"),),
+        (pytest.param("examples/roles/role_with_deps_paths", 3, id="fail"),),
     )
     def test_role_deps_path_names(
         default_rules_collection: RulesCollection,
@@ -202,7 +208,9 @@ def test_role_deps_path_names(
         expected_errors = (
             ("role-name[path]", 3),
             ("role-name[path]", 9),
+            ("role-name[path]", 10),
         )
+        assert len(expected_errors) == failure
         for idx, result in enumerate(results):
             assert result.tag == expected_errors[idx][0]
             assert result.lineno == expected_errors[idx][1]

diff --git a/src/ansiblelint/schemas/meta.json b/src/ansiblelint/schemas/meta.json
@@ -1520,7 +1520,14 @@
     },
     "dependencies": {
       "items": {
-        "$ref": "#/$defs/DependencyModel"
+        "anyOf": [
+          {
+            "type": "string"
+          },
+          {
+            "$ref": "#/$defs/DependencyModel"
+          }
+        ]
       },
       "title": "Dependencies",
       "type": "array"

diff --git a/test/schemas/negative_test/roles/role_with_bad_deps_in_meta/meta/main.yml b/test/schemas/negative_test/roles/role_with_bad_deps_in_meta/meta/main.yml
@@ -11,3 +11,4 @@ galaxy_info:
 
 dependencies:
   - version: foo # invalid, should have at least name, role or src properties
+  - 1234 # invalid, needs to be a string
diff --git a/test/schemas/negative_test/roles/role_with_bad_deps_in_meta/meta/main.yml.md b/test/schemas/negative_test/roles/role_with_bad_deps_in_meta/meta/main.yml.md
@@ -2,6 +2,15 @@
 
 ```json
 [
+  {
+    "instancePath": "/dependencies/0",
+    "keyword": "type",
+    "message": "must be string",
+    "params": {
+      "type": "string"
+    },
+    "schemaPath": "#/properties/dependencies/items/anyOf/0/type"
+  },
   {
     "instancePath": "/dependencies/0",
     "keyword": "required",
@@ -36,6 +45,38 @@
     "params": {},
     "schemaPath": "#/anyOf"
   },
+  {
+    "instancePath": "/dependencies/0",
+    "keyword": "anyOf",
+    "message": "must match a schema in anyOf",
+    "params": {},
+    "schemaPath": "#/properties/dependencies/items/anyOf"
+  },
+  {
+    "instancePath": "/dependencies/1",
+    "keyword": "type",
+    "message": "must be string",
+    "params": {
+      "type": "string"
+    },
+    "schemaPath": "#/properties/dependencies/items/anyOf/0/type"
+  },
+  {
+    "instancePath": "/dependencies/1",
+    "keyword": "type",
+    "message": "must be object",
+    "params": {
+      "type": "object"
+    },
+    "schemaPath": "#/type"
+  },
+  {
+    "instancePath": "/dependencies/1",
+    "keyword": "anyOf",
+    "message": "must match a schema in anyOf",
+    "params": {},
+    "schemaPath": "#/properties/dependencies/items/anyOf"
+  },
   {
     "instancePath": "/galaxy_info",
     "keyword": "required",
@@ -73,14 +114,22 @@ stdout:
       "has_sub_errors": true,
       "best_match": {
         "path": "$.dependencies[0]",
-        "message": "'role' is a required property"
+        "message": "{'version': 'foo'} is not of type 'string'"
       },
       "best_deep_match": {
         "path": "$.dependencies[0]",
-        "message": "'role' is a required property"
+        "message": "{'version': 'foo'} is not of type 'string'"
       },
-      "num_sub_errors": 2,
+      "num_sub_errors": 4,
       "sub_errors": [
+        {
+          "path": "$.dependencies[0]",
+          "message": "{'version': 'foo'} is not of type 'string'"
+        },
+        {
+          "path": "$.dependencies[0]",
+          "message": "{'version': 'foo'} is not valid under any of the given schemas"
+        },
         {
           "path": "$.dependencies[0]",
           "message": "'role' is a required property"
@@ -95,6 +144,31 @@ stdout:
         }
       ]
     },
+    {
+      "filename": "negative_test/roles/role_with_bad_deps_in_meta/meta/main.yml",
+      "path": "$.dependencies[1]",
+      "message": "1234 is not valid under any of the given schemas",
+      "has_sub_errors": true,
+      "best_match": {
+        "path": "$.dependencies[1]",
+        "message": "1234 is not of type 'string'"
+      },
+      "best_deep_match": {
+        "path": "$.dependencies[1]",
+        "message": "1234 is not of type 'string'"
+      },
+      "num_sub_errors": 1,
+      "sub_errors": [
+        {
+          "path": "$.dependencies[1]",
+          "message": "1234 is not of type 'string'"
+        },
+        {
+          "path": "$.dependencies[1]",
+          "message": "1234 is not of type 'object'"
+        }
+      ]
+    },
     {
       "filename": "negative_test/roles/role_with_bad_deps_in_meta/meta/main.yml",
       "path": "$.galaxy_info",

diff --git a/test/schemas/test/roles/foo/meta/main.yml b/test/schemas/test/roles/foo/meta/main.yml
@@ -5,6 +5,7 @@ dependencies:
     version: "1.0"
   - name: ansible-role-bar
     version: "1.0"
+  - ansible-role-baz
   # from Bitbucket
   - src: git+http://bitbucket.org/willthames/git-ansible-galaxy
     version: v1.4