[PATCH v3 1/2] image.bbclass: add prohibited-paths QA test

[PATCH v3 1/2] image.bbclass: add prohibited-paths QA test

[Martyn Welch]

Sometimes we wish to ensure that files or directories are not installed
somewhere that may prove detrimental to the operation of the system. For
example, this may be the case if files are placed in a directory that is
utilised as a mount point at run time, thus making them inaccessible once
when the mount point is being utilised.

Implement the prohibited paths QA test, which enables such locations to be
specified in a "PROHIBITED_PATHS" variable. This implementation allows for
a colon separated list of paths to be provided. Shell style wildcards can
be used.

Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
Changes since v1:
 - Correcting author and SOB.

Changes since v2:
 - Reimplemented as image rather than package level QA test.
 - Changed variable from PROHIBITED_PATH to PROHIBITED_PATHS to better
   reflect its use.

 meta/classes/image.bbclass | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index d93de02..bebb363 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -296,6 +296,26 @@ python do_image_complete_setscene () {
 }
 addtask do_image_complete_setscene
 
+python image_check_prohibited_paths () {
+    import glob
+    from oe.utils import ImageQAFailed
+
+    rootfs = d.getVar('IMAGE_ROOTFS')
+
+    path = d.getVar('PROHIBITED_PATHS')
+    if path != None and path != "":
+        for p in path.split(':'):
+            if p[0] != '/':
+                raise ImageQAFailed("PROHIBITED_PATHS \"%s\" must be an absolute path" % p, image_check_prohibited_paths)
+
+            match = glob.glob("%s%s" % (rootfs, p))
+            if match:
+                loc = ", ".join(item.replace(rootfs, '') for item in match)
+                raise ImageQAFailed("Match(es) for PROHIBITED_PATHS \"%s\": %s" % (p, loc), image_check_prohibited_paths)
+}
+
+IMAGE_QA_COMMANDS += "image_check_prohibited_paths"
+
 # Add image-level QA/sanity checks to IMAGE_QA_COMMANDS
 #
 # IMAGE_QA_COMMANDS += " \
-- 
2.1.4

[PATCH v3 2/2] ref-manual: Add documentation for prohibited-path QA test

[Martyn Welch]

Add documentation for the PROHIBITED_PATHS variable and associated
prohibited-path QA test

Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
---
Changes since v1:
 - Correcting author and SOB.

Changes since v2:
 - Reimplemented as image rather than package level QA test, altering
   documentation to suit.
 - Changed variable from PROHIBITED_PATH to PROHIBITED_PATHS to better
   reflect its use.

 documentation/ref-manual/ref-variables.xml | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/documentation/ref-manual/ref-variables.xml b/documentation/ref-manual/ref-variables.xml
index e31aa21..1fb8a86 100644
--- a/documentation/ref-manual/ref-variables.xml
+++ b/documentation/ref-manual/ref-variables.xml
@@ -10820,6 +10820,30 @@ recipes-graphics/xorg-font/font-alias_1.0.3.bb:PR = "${INC_PR}.3"
             </glossdef>
         </glossentry>
 
+        <glossentry id='var-PROHIBITED_PATHS'><glossterm>PROHIBITED_PATHS</glossterm>
+            <info>
+                PROHIBITED_PATHS[doc] = "A colon separated list of paths in which recipes are prohibited from installing."
+            </info>
+            <glossdef>
+                <para role="glossdeffirst">
+<!--                <para role="glossdeffirst"><imagedata fileref="figures/define-generic.png" /> -->
+                    A colon separated list of paths in which recipes are
+                    prohibited from installing.
+                    Shell-style wildcards can be used in paths. All paths need
+                    to be absolute paths.
+                </para>
+
+                <para>
+                    For example, the following
+                    <filename>PROHIBITED_PATHS</filename> ensures nothing is
+                    installed under <filename>/mnt</filename>:
+                    <literallayout class='monospaced'>
+     PROHIBITED_PATHS += "/mnt/*"
+                    </literallayout>
+                </para>
+            </glossdef>
+        </glossentry>
+
         <glossentry id='var-PROVIDES'><glossterm>PROVIDES</glossterm>
             <info>
                 PROVIDES[doc] = "A list of aliases that a recipe also provides. These aliases are useful for satisfying dependencies of other recipes during the build as specified by DEPENDS."
-- 
2.1.4

✗ patchtest: failure for "[v3] image.bbclass: add prohib..." and 1 more

[Patchwork]

== Series Details ==

Series: "[v3] image.bbclass: add prohib..." and 1 more
Revision: 1
URL   : https://patchwork.openembedded.org/series/9805/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Issue             Series sent to the wrong mailing list or some patches from the series correspond to different mailing lists [test_target_mailing_list] 
  Suggested fix    Send the series again to the correct mailing list (ML)
  Suggested ML     yocto@yoctoproject.org [http://git.yoctoproject.org/cgit/cgit.cgi/yocto-docs/]
  Patch's path:    documentation/ref-manual/ref-variables.xml

* Issue             Series does not apply on top of target branch [test_series_merge_on_head] 
  Suggested fix    Rebase your series on top of targeted branch
  Targeted branch  master (currently at a17f3ec910)



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Guidelines:     https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe

Re: [PATCH v3 1/2] image.bbclass: add prohibited-paths QA test

[Otavio Salvador]

On Wed, Nov 15, 2017 at 1:10 PM, Martyn Welch
<martyn.welch@collabora.co.uk> wrote:
> Sometimes we wish to ensure that files or directories are not installed
> somewhere that may prove detrimental to the operation of the system. For
> example, this may be the case if files are placed in a directory that is
> utilised as a mount point at run time, thus making them inaccessible once
> when the mount point is being utilised.
>
> Implement the prohibited paths QA test, which enables such locations to be
> specified in a "PROHIBITED_PATHS" variable. This implementation allows for
> a colon separated list of paths to be provided. Shell style wildcards can
> be used.
>
> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
> ---
> Changes since v1:
>  - Correcting author and SOB.
>
> Changes since v2:
>  - Reimplemented as image rather than package level QA test.
>  - Changed variable from PROHIBITED_PATH to PROHIBITED_PATHS to better
>    reflect its use.
>
>  meta/classes/image.bbclass | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index d93de02..bebb363 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -296,6 +296,26 @@ python do_image_complete_setscene () {
>  }
>  addtask do_image_complete_setscene
>
> +python image_check_prohibited_paths () {
> +    import glob
> +    from oe.utils import ImageQAFailed
> +
> +    rootfs = d.getVar('IMAGE_ROOTFS')
> +
> +    path = d.getVar('PROHIBITED_PATHS')

path = (d.getVar('PROHIBITED_PATHS') or "")

I'd use IMAGE_QA_PROHIBITED_PATHS as variable name. It makes easier to
know what it relates to.

> +    if path != None and path != "":

If can die.

> +        for p in path.split(':'):
> +            if p[0] != '/':

if not p.startswith('/'):

> +                raise ImageQAFailed("PROHIBITED_PATHS \"%s\" must be an absolute path" % p, image_check_prohibited_paths)
> +
> +            match = glob.glob("%s%s" % (rootfs, p))
> +            if match:

I'd use:

if glob.glob(...):

It is a single use so not sure it is worth the extra variable.

> +                loc = ", ".join(item.replace(rootfs, '') for item in match)
> +                raise ImageQAFailed("Match(es) for PROHIBITED_PATHS \"%s\": %s" % (p, loc), image_check_prohibited_paths)
> +}

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH v3 1/2] image.bbclass: add prohibited-paths QA test

[Martyn Welch]

On Wed, 2017-11-15 at 18:46 -0200, Otavio Salvador wrote:
> On Wed, Nov 15, 2017 at 1:10 PM, Martyn Welch
> <martyn.welch@collabora.co.uk> wrote:
> > Sometimes we wish to ensure that files or directories are not installed
> > somewhere that may prove detrimental to the operation of the system. For
> > example, this may be the case if files are placed in a directory that is
> > utilised as a mount point at run time, thus making them inaccessible once
> > when the mount point is being utilised.
> >
> > Implement the prohibited paths QA test, which enables such locations to be
> > specified in a "PROHIBITED_PATHS" variable. This implementation allows for
> > a colon separated list of paths to be provided. Shell style wildcards can
> > be used.
> >
> > Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> > Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
> > ---
> > Changes since v1:
> >  - Correcting author and SOB.
> >
> > Changes since v2:
> >  - Reimplemented as image rather than package level QA test.
> >  - Changed variable from PROHIBITED_PATH to PROHIBITED_PATHS to better
> >    reflect its use.
> >
> >  meta/classes/image.bbclass | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> > index d93de02..bebb363 100644
> > --- a/meta/classes/image.bbclass
> > +++ b/meta/classes/image.bbclass
> > @@ -296,6 +296,26 @@ python do_image_complete_setscene () {
> >  }
> >  addtask do_image_complete_setscene
> >
> > +python image_check_prohibited_paths () {
> > +    import glob
> > +    from oe.utils import ImageQAFailed
> > +
> > +    rootfs = d.getVar('IMAGE_ROOTFS')
> > +
> > +    path = d.getVar('PROHIBITED_PATHS')
> 
> path = (d.getVar('PROHIBITED_PATHS') or "")
> 
> I'd use IMAGE_QA_PROHIBITED_PATHS as variable name. It makes easier to
> know what it relates to.
> 
> > +    if path != None and path != "":
> 
> If can die.
> 
> > +        for p in path.split(':'):
> > +            if p[0] != '/':
> 
> if not p.startswith('/'):
> 
> > +                raise ImageQAFailed("PROHIBITED_PATHS \"%s\" must be an absolute path" % p, image_check_prohibited_paths)
> > +
> > +            match = glob.glob("%s%s" % (rootfs, p))
> > +            if match:
> 
> I'd use:
> 
> if glob.glob(...):
> 
> It is a single use so not sure it is worth the extra variable.
> 
> > +                loc = ", ".join(item.replace(rootfs, '') for item in match)

match is used here.

> > +                raise ImageQAFailed("Match(es) for PROHIBITED_PATHS \"%s\": %s" % (p, loc), image_check_prohibited_paths)
> > +}
> 

Re: [PATCH v3 1/2] image.bbclass: add prohibited-paths QA test

[Martyn Welch]

On Wed, 2017-11-15 at 18:46 -0200, Otavio Salvador wrote:
> On Wed, Nov 15, 2017 at 1:10 PM, Martyn Welch
> <martyn.welch@collabora.co.uk> wrote:
> > Sometimes we wish to ensure that files or directories are not installed
> > somewhere that may prove detrimental to the operation of the system. For
> > example, this may be the case if files are placed in a directory that is
> > utilised as a mount point at run time, thus making them inaccessible once
> > when the mount point is being utilised.
> >
> > Implement the prohibited paths QA test, which enables such locations to be
> > specified in a "PROHIBITED_PATHS" variable. This implementation allows for
> > a colon separated list of paths to be provided. Shell style wildcards can
> > be used.
> >
> > Signed-off-by: Fabien Lahoudere <fabien.lahoudere@collabora.co.uk>
> > Signed-off-by: Martyn Welch <martyn.welch@collabora.co.uk>
> > ---
> > Changes since v1:
> >  - Correcting author and SOB.
> >
> > Changes since v2:
> >  - Reimplemented as image rather than package level QA test.
> >  - Changed variable from PROHIBITED_PATH to PROHIBITED_PATHS to better
> >    reflect its use.
> >
> >  meta/classes/image.bbclass | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> > index d93de02..bebb363 100644
> > --- a/meta/classes/image.bbclass
> > +++ b/meta/classes/image.bbclass
> > @@ -296,6 +296,26 @@ python do_image_complete_setscene () {
> >  }
> >  addtask do_image_complete_setscene
> >
> > +python image_check_prohibited_paths () {
> > +    import glob
> > +    from oe.utils import ImageQAFailed
> > +
> > +    rootfs = d.getVar('IMAGE_ROOTFS')
> > +
> > +    path = d.getVar('PROHIBITED_PATHS')
> 
> path = (d.getVar('PROHIBITED_PATHS') or "")
> 
> I'd use IMAGE_QA_PROHIBITED_PATHS as variable name. It makes easier to
> know what it relates to.
> 
> > +    if path != None and path != "":
> 
> If can die.
> 

OK, if is still needed, else the zero length path triggers the "not
p.startswith('/')" error condition.

> > +        for p in path.split(':'):
> > +            if p[0] != '/':
> 
> if not p.startswith('/'):
> 
> > +                raise ImageQAFailed("PROHIBITED_PATHS \"%s\" must be an absolute path" % p, image_check_prohibited_paths)
> > +
> > +            match = glob.glob("%s%s" % (rootfs, p))
> > +            if match:
> 
> I'd use:
> 
> if glob.glob(...):
> 
> It is a single use so not sure it is worth the extra variable.
> 
> > +                loc = ", ".join(item.replace(rootfs, '') for item in match)
> > +                raise ImageQAFailed("Match(es) for PROHIBITED_PATHS \"%s\": %s" % (p, loc), image_check_prohibited_paths)
> > +}
> 

Re: [PATCH v3 1/2] image.bbclass: add prohibited-paths QA test

[Alexander Kanavin]

On 11/15/2017 05:10 PM, Martyn Welch wrote:
> Sometimes we wish to ensure that files or directories are not installed
> somewhere that may prove detrimental to the operation of the system. For
> example, this may be the case if files are placed in a directory that is
> utilised as a mount point at run time, thus making them inaccessible once
> when the mount point is being utilised.
> 
> Implement the prohibited paths QA test, which enables such locations to be
> specified in a "PROHIBITED_PATHS" variable. This implementation allows for
> a colon separated list of paths to be provided. Shell style wildcards can
> be used.

The test does nothing if PROHIBITED_PATHS is not set. Can you set it in 
core-image.bbclass to some reasonable default common to reference 
images? Something like /mnt /media /tmp /run /var/run /var/tmp (not sure 
at the moment what the mount points in those images are).

Alex