Repository: ansible/ansible
Pull request: ansible-galaxy - fix listing specific role and role description
##### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
If the role was not in the first search path, it was reported as not found.

Get the role description from within `galaxy_info` and fall back to a top level `description` key.

Fix logic around reporting whether or not a role exists when displaying info.

Add integration tests.

Fixes #60167
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Bugfix Pull Request


##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below -->
`lib/ansible/galaxy/role.py`

Problem statement:
galaxy-info --offline option doesn't work
##### SUMMARY
Running `ansible-galaxy info` command using --offline option does not give the "description" information from role_name/meta/main.yml. 

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
roles

##### ANSIBLE VERSION
```
2.8
```
##### STEPS TO REPRODUCE
```
ansible-galaxy init my-role
- my-role was created successfully
```

Executing the following command, you can't see the "description" information.
```
ansible-galaxy info --offline --roles-path=/home/user my-role

Role: my-role
        description: 
        dependencies: []
        galaxy_info:
                author: your name
                company: your company (optional)
                galaxy_tags: []
                license: license (GPL-2.0-or-later, MIT, etc)
                min_ansible_version: 2.4
        path: (u'/home/user', u'/home/user/.ansible/roles', u'/usr/share/ansible/roles', u'/etc/ansible/roles')
```
##### EXPECTED RESULTS
```
Role: my-role
        dependencies: []
        galaxy_info:
                author: your name
                description: your description >> description goes here
                company: your company (optional)
                galaxy_tags: []
                license: license (GPL-2.0-or-later, MIT, etc)
                min_ansible_version: 2.4
        path: (u'/home/user', u'/home/user/.ansible/roles', u'/usr/share/ansible/roles', u'/etc/ansible/roles')
```

Changing the `meta/main.yml` to the following code, it works:
```
description: your description
galaxy_info:
  author: your name
  company: your company (optional)
  license: license (GPL-2.0-or-later, MIT, etc)
  min_ansible_version: 2.4
  galaxy_tags: []
dependencies: []
```

Command/output:
```
ansible-galaxy info --offline --roles-path=/home/user my-role

Role: my-role
        description: your description
        dependencies: []
        galaxy_info:
                author: your name
                company: your company (optional)
                galaxy_tags: []
                license: license (GPL-2.0-or-later, MIT, etc)
                min_ansible_version: 2.4
        path: (u'/home/user', u'/home/user/.ansible/roles', u'/usr/share/ansible/roles', u'/etc/ansible/roles')
```

Hints from issue discussion:
Files identified in the description:
* [`lib/ansible/galaxy`](https://github.com/ansible/ansible/blob/devel/lib/ansible/galaxy)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
It looks like there a few problems that can contribute to this:

1. We set `GalaxyRole.path` to a potentially wrong value:

    ```
            # use the first path by default
            self.path = os.path.join(galaxy.roles_paths[0], self.name)
    ```
2. We assume that `description` should be top level, and not under `galaxy_info`. I'm not sure which is correct, but seeing as though init has been creating it this way for so long, maybe we want to check both locations

With (1), even if we don't "use the first path by default", there are paths missing that ansible will use to search for a role, such a missing path would be `playbook_dir/role-name`.
Files identified in the description:
* [`lib/ansible/playbook/role`](https://github.com/ansible/ansible/blob/devel/lib/ansible/playbook/role)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
Related to discussion around #60167, we do indeed have a bug in that code as described above. Listing a specific role doesn't work if it's not in the first path defined in `ANSIBLE_ROLES_PATH`. Here's my simple reproducer:

```
[root@d6895cd7a73f ~]# ansible-galaxy init testrole
- Role testrole was created successfully
[root@d6895cd7a73f ~]# ansible-galaxy list
# /usr/share/ansible/roles
# /etc/ansible/roles
[WARNING]: - the configured path /root/.ansible/roles does not exist.

[root@d6895cd7a73f ~]# ls
anaconda-ks.cfg  testrole
[root@d6895cd7a73f ~]# ANSIBLE_ROLES_PATH=/usr/share/ansible/roles:$(pwd) ansible-galaxy list testrole
- the role testrole was not found
[root@d6895cd7a73f ~]# ANSIBLE_ROLES_PATH=$(pwd):/usr/share/ansible/roles ansible-galaxy list testrole
# /root
- testrole, (unknown version)
[root@d6895cd7a73f ~]#
```

Commit message: BSD and macOS ruining all the fun

Patch to review:
diff --git a/changelogs/fragments/ansible-galaxy-role-list-specific-fix.yml b/changelogs/fragments/ansible-galaxy-role-list-specific-fix.yml
new file mode 100644
index 00000000000000..eaef1d59523e8c
--- /dev/null
+++ b/changelogs/fragments/ansible-galaxy-role-list-specific-fix.yml
@@ -0,0 +1,6 @@
+bugfixes:
+  - >
+    ansible-galaxy - fix a bug where listing a specific role if it was not in the first
+    path failed to find the role
+
+  - ansible-galaxy - properly show the role description when running offline (https://github.com/ansible/ansible/issues/60167)
diff --git a/lib/ansible/cli/galaxy.py b/lib/ansible/cli/galaxy.py
index a5558e278459a0..f66bc353c80b3c 100644
--- a/lib/ansible/cli/galaxy.py
+++ b/lib/ansible/cli/galaxy.py
@@ -580,7 +580,11 @@ def exit_without_ignore(rc=1):
     def _display_role_info(role_info):
 
         text = [u"", u"Role: %s" % to_text(role_info['name'])]
-        text.append(u"\tdescription: %s" % role_info.get('description', ''))
+
+        # Get description from galaxy_info['galaxy_info']['description'] first, falling back to top-level 'description'.
+        galaxy_info = role_info.get('galaxy_info', {})
+        description = galaxy_info.get('description', role_info.get('description', ''))
+        text.append(u"\tdescription: %s" % description)
 
         for k in sorted(role_info.keys()):
 
@@ -854,6 +858,9 @@ def execute_info(self):
 
             role_info = {'path': roles_path}
             gr = GalaxyRole(self.galaxy, self.api, role)
+            if not gr._exists:
+                data = u"- the role %s was not found" % role
+                break
 
             install_info = gr.install_info
             if install_info:
@@ -878,10 +885,6 @@ def execute_info(self):
                 role_info.update(role_spec)
 
             data = self._display_role_info(role_info)
-            # FIXME: This is broken in both 1.9 and 2.0 as
-            # _display_role_info() always returns something
-            if not data:
-                data = u"\n- the role %s was not found" % role
 
         self.pager(data)
 
diff --git a/lib/ansible/galaxy/role.py b/lib/ansible/galaxy/role.py
index 8e1ca72fd42a46..ba12d1d4c28057 100644
--- a/lib/ansible/galaxy/role.py
+++ b/lib/ansible/galaxy/role.py
@@ -64,6 +64,7 @@ def __init__(self, galaxy, api, name, src=None, version=None, scm=None, path=Non
         self.version = version
         self.src = src or name
         self.scm = scm
+        self.paths = [os.path.join(x, self.name) for x in galaxy.roles_paths]
 
         if path is not None:
             if not path.endswith(os.path.join(os.path.sep, self.name)):
@@ -82,9 +83,6 @@ def __init__(self, galaxy, api, name, src=None, version=None, scm=None, path=Non
         else:
             # use the first path by default
             self.path = os.path.join(galaxy.roles_paths[0], self.name)
-            # create list of possible paths
-            self.paths = [x for x in galaxy.roles_paths]
-            self.paths = [os.path.join(x, self.name) for x in self.paths]
 
     def __repr__(self):
         """
@@ -105,17 +103,17 @@ def metadata(self):
         Returns role metadata
         """
         if self._metadata is None:
-            for meta_main in self.META_MAIN:
-                meta_path = os.path.join(self.path, meta_main)
-                if os.path.isfile(meta_path):
-                    try:
-                        f = open(meta_path, 'r')
-                        self._metadata = yaml.safe_load(f)
-                    except Exception:
-                        display.vvvvv("Unable to load metadata for %s" % self.name)
-                        return False
-                    finally:
-                        f.close()
+            for path in self.paths:
+                for meta_main in self.META_MAIN:
+                    meta_path = os.path.join(path, meta_main)
+                    if os.path.isfile(meta_path):
+                        try:
+                            with open(meta_path, 'r') as f:
+                                self._metadata = yaml.safe_load(f)
+                        except Exception:
+                            display.vvvvv("Unable to load metadata for %s" % self.name)
+                            return False
+                        break
 
         return self._metadata
 
@@ -138,6 +136,14 @@ def install_info(self):
                     f.close()
         return self._install_info
 
+    @property
+    def _exists(self):
+        for path in self.paths:
+            if os.path.isdir(path):
+                return True
+
+        return False
+
     def _write_galaxy_install_info(self):
         """
         Writes a YAML-formatted file to the role's meta/ directory
diff --git a/test/integration/targets/ansible-galaxy/runme.sh b/test/integration/targets/ansible-galaxy/runme.sh
index 2f735fb172bc48..258fdfa9cfb937 100755
--- a/test/integration/targets/ansible-galaxy/runme.sh
+++ b/test/integration/targets/ansible-galaxy/runme.sh
@@ -120,6 +120,71 @@ f_ansible_galaxy_status \
 
     [[ $(grep -c '^- test-role' out.txt ) -eq 2 ]]
 
+# Galaxy role test case
+#
+# Test listing a specific role that is not in the first path in ANSIBLE_ROLES_PATH.
+# https://github.com/ansible/ansible/issues/60167#issuecomment-585460706
+
+f_ansible_galaxy_status \
+    "list specific role not in the first path in ANSIBLE_ROLES_PATHS"
+
+role_testdir=$(mktemp -d)
+pushd "${role_testdir}"
+
+    mkdir testroles
+    ansible-galaxy role init --init-path ./local-roles quark
+    ANSIBLE_ROLES_PATH=./local-roles:${HOME}/.ansible/roles ansible-galaxy role list quark | tee out.txt
+
+    [[ $(grep -c 'not found' out.txt) -eq 0 ]]
+
+    ANSIBLE_ROLES_PATH=${HOME}/.ansible/roles:./local-roles ansible-galaxy role list quark | tee out.txt
+
+    [[ $(grep -c 'not found' out.txt) -eq 0 ]]
+
+popd # ${role_testdir}
+rm -fr "${role_testdir}"
+
+
+# Galaxy role info tests
+
+f_ansible_galaxy_status \
+    "role info non-existant role"
+
+role_testdir=$(mktemp -d)
+pushd "${role_testdir}"
+
+    ansible-galaxy role info notaroll | tee out.txt
+
+    grep -- '- the role notaroll was not found' out.txt
+
+f_ansible_galaxy_status \
+    "role info description offline"
+
+    mkdir testroles
+    ansible-galaxy role init testdesc --init-path ./testroles
+
+    # Only galaxy_info['description'] exists in file
+    sed -i -e 's#[[:space:]]\{1,\}description:.*$#  description: Description in galaxy_info#' ./testroles/testdesc/meta/main.yml
+    ansible-galaxy role info -p ./testroles --offline testdesc | tee out.txt
+    grep 'description: Description in galaxy_info' out.txt
+
+    # Both top level 'description' and galaxy_info['description'] exist in file
+    # Use shell-fu instead of sed to prepend a line to a file because BSD
+    # and macOS sed don't work the same as GNU sed.
+    echo 'description: Top level' | \
+        cat - ./testroles/testdesc/meta/main.yml > tmp.yml && \
+        mv tmp.yml ./testroles/testdesc/meta/main.yml
+    ansible-galaxy role info -p ./testroles --offline testdesc | tee out.txt
+    grep 'description: Description in galaxy_info' out.txt
+
+    # Only top level 'description'exists in file
+    sed -i -e '/^[[:space:]]\+description: Description in galaxy_info/d' ./testroles/testdesc/meta/main.yml
+    ansible-galaxy role info -p ./testroles --offline testdesc | tee out.txt
+    grep 'description: Top level' out.txt
+
+popd # ${role_testdir}
+rm -fr "${role_testdir}"
+
 
 # Properly list roles when the role name is a subset of the path, or the role
 # name is the same name as the parent directory of the role. Issue #67365


Code review comments:
diff_hunk: @@ -580,7 +580,11 @@ def exit_without_ignore(rc=1):
     def _display_role_info(role_info):
 
         text = [u"", u"Role: %s" % to_text(role_info['name'])]
-        text.append(u"\tdescription: %s" % role_info.get('description', ''))
+
+        # Get description from galaxy_info['galaxy_info']['description'] first, falling back to top-level 'description'.
+        galaxy_info = role_info.get('galaxy_info', {})
+        description = galaxy_info.get('description', role_info.get('description', ''))
path: lib/ansible/cli/galaxy.py
text: @user1:
Just in case people updated roles to contain a top level 'description' key, maybe that should take precedence so the behavior doesn't change.

@user1:
I got a +1 from sivel to update the precedence, so I did that. I forgot to update the comment too - fixed in the last commit. This should be passing tests now and looks good to merge to me.