[PATCH v2] bitbake-worker: remove the network flag

[PATCH v2] bitbake-worker: remove the network flag

[rs]

From: Randolph Sapp <rs@ti.com>

The network flag does not currently respect proxies by default as the
fetch stage does [1]. As such it only works for some users at the whim
of the recipe writer.

This introduces an issue downstream as the build results are now
dependent on the network architecture of the builder, the recipe in
question, and the combination of proxy variables they choose to export
in addition to the normal host variables that affect builds (host tool
version, build environment, etc).

This is far more indeterminate than a build system that properly exports
proxy variables. If abuse of the network tag is an issue, then the
network tag should be removed in it's current state and networking
should depend entirely on whether the current task is 'do_fetch'.

[1] https://lists.openembedded.org/g/bitbake-devel/topic/100924096

Signed-off-by: Randolph Sapp <rs@ti.com>
---

What I feel would be the proper solution to the aforementioned issues.

Examples of layers that currently use the network flag outside of the do_fetch
stage:

	oe-core (icecc.bbclass)
	meta-openembedded (go recipes)
	meta-flutter (all flutter recipes and classes)

This is not an attempt to shame maintainers. I'm just curious why the network
task expects every maintainer to handle proxy variables (something pretty much
required if you are actually using the network in that task) independently. It
just results in needless debugging and a bunch of patches attempting to export
variables in their own way. Sorry if this comes off as antagonistic in any way.

 bin/bitbake-worker                                       | 2 +-
 doc/bitbake-user-manual/bitbake-user-manual-metadata.rst | 6 ------
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/bin/bitbake-worker b/bin/bitbake-worker
index 451e6926..f25880c1 100755
--- a/bin/bitbake-worker
+++ b/bin/bitbake-worker
@@ -270,7 +270,7 @@ def fork_off_task(cfg, data, databuilder, workerdata, extraconfigdata, runtask):
 
                 bb.utils.set_process_name("%s:%s" % (the_data.getVar("PN"), taskname.replace("do_", "")))
 
-                if not bb.utils.to_boolean(the_data.getVarFlag(taskname, 'network')):
+                if taskname != 'do_fetch':
                     if bb.utils.is_local_uid(uid):
                         logger.debug("Attempting to disable network for %s" % taskname)
                         bb.utils.disable_network(uid, gid)
diff --git a/doc/bitbake-user-manual/bitbake-user-manual-metadata.rst b/doc/bitbake-user-manual/bitbake-user-manual-metadata.rst
index 58975f4c..b35c332c 100644
--- a/doc/bitbake-user-manual/bitbake-user-manual-metadata.rst
+++ b/doc/bitbake-user-manual/bitbake-user-manual-metadata.rst
@@ -1519,12 +1519,6 @@ functionality of the task:
    released. You can use this variable flag to accomplish mutual
    exclusion.
 
--  ``[network]``: When set to "1", allows a task to access the network. By
-   default, only the ``do_fetch`` task is granted network access. Recipes
-   shouldn't access the network outside of ``do_fetch`` as it usually
-   undermines fetcher source mirroring, image and licence manifests, software
-   auditing and supply chain security.
-
 -  ``[noexec]``: When set to "1", marks the task as being empty, with
    no execution required. You can use the ``[noexec]`` flag to set up
    tasks as dependency placeholders, or to disable tasks defined
-- 
2.42.0

Re: [PATCH v2] bitbake-worker: remove the network flag

[Alexander Kanavin]

Please stop sending the patch. You are just winding everyone up. If
you want to have a discussion, open a thread with the comment about
icecc and friends, and we'll take it from there, otherwise any further
submission from you will be treated with suspicion and mistrust, which
is not the outcome you want.

Alex

On Sun, 10 Sept 2023 at 21:54, <rs@ti.com> wrote:
>
> From: Randolph Sapp <rs@ti.com>
>
> The network flag does not currently respect proxies by default as the
> fetch stage does [1]. As such it only works for some users at the whim
> of the recipe writer.
>
> This introduces an issue downstream as the build results are now
> dependent on the network architecture of the builder, the recipe in
> question, and the combination of proxy variables they choose to export
> in addition to the normal host variables that affect builds (host tool
> version, build environment, etc).
>
> This is far more indeterminate than a build system that properly exports
> proxy variables. If abuse of the network tag is an issue, then the
> network tag should be removed in it's current state and networking
> should depend entirely on whether the current task is 'do_fetch'.
>
> [1] https://lists.openembedded.org/g/bitbake-devel/topic/100924096
>
> Signed-off-by: Randolph Sapp <rs@ti.com>
> ---
>
> What I feel would be the proper solution to the aforementioned issues.
>
> Examples of layers that currently use the network flag outside of the do_fetch
> stage:
>
>         oe-core (icecc.bbclass)
>         meta-openembedded (go recipes)
>         meta-flutter (all flutter recipes and classes)
>
> This is not an attempt to shame maintainers. I'm just curious why the network
> task expects every maintainer to handle proxy variables (something pretty much
> required if you are actually using the network in that task) independently. It
> just results in needless debugging and a bunch of patches attempting to export
> variables in their own way. Sorry if this comes off as antagonistic in any way.
>
>  bin/bitbake-worker                                       | 2 +-
>  doc/bitbake-user-manual/bitbake-user-manual-metadata.rst | 6 ------
>  2 files changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/bin/bitbake-worker b/bin/bitbake-worker
> index 451e6926..f25880c1 100755
> --- a/bin/bitbake-worker
> +++ b/bin/bitbake-worker
> @@ -270,7 +270,7 @@ def fork_off_task(cfg, data, databuilder, workerdata, extraconfigdata, runtask):
>
>                  bb.utils.set_process_name("%s:%s" % (the_data.getVar("PN"), taskname.replace("do_", "")))
>
> -                if not bb.utils.to_boolean(the_data.getVarFlag(taskname, 'network')):
> +                if taskname != 'do_fetch':
>                      if bb.utils.is_local_uid(uid):
>                          logger.debug("Attempting to disable network for %s" % taskname)
>                          bb.utils.disable_network(uid, gid)
> diff --git a/doc/bitbake-user-manual/bitbake-user-manual-metadata.rst b/doc/bitbake-user-manual/bitbake-user-manual-metadata.rst
> index 58975f4c..b35c332c 100644
> --- a/doc/bitbake-user-manual/bitbake-user-manual-metadata.rst
> +++ b/doc/bitbake-user-manual/bitbake-user-manual-metadata.rst
> @@ -1519,12 +1519,6 @@ functionality of the task:
>     released. You can use this variable flag to accomplish mutual
>     exclusion.
>
> --  ``[network]``: When set to "1", allows a task to access the network. By
> -   default, only the ``do_fetch`` task is granted network access. Recipes
> -   shouldn't access the network outside of ``do_fetch`` as it usually
> -   undermines fetcher source mirroring, image and licence manifests, software
> -   auditing and supply chain security.
> -
>  -  ``[noexec]``: When set to "1", marks the task as being empty, with
>     no execution required. You can use the ``[noexec]`` flag to set up
>     tasks as dependency placeholders, or to disable tasks defined
> --
> 2.42.0
>

Re: [EXTERNAL] Re: [PATCH v2] bitbake-worker: remove the network flag

[Randolph Sapp]

On 9/10/23 14:57, Alexander Kanavin wrote:
> Please stop sending the patch. You are just winding everyone up. If
> you want to have a discussion, open a thread with the comment about
> icecc and friends, and we'll take it from there, otherwise any further
> submission from you will be treated with suspicion and mistrust, which
> is not the outcome you want.
> 
> Alex
> [snip]
Fair enough. It was my intent to actually submit a usable patch with 
this discussion but the first revision was a bit too hastily sent. Rest 
assured, this will be the last revision.

I'm not here to push the matter. I just figured this was the next step 
if the network flag didn't behave the same for all users.

Seems like people like the current behavior though. That's fine. I just 
think this should be documented somewhere if that's the case. I can 
submit a patch for that documentation update instead, if that's preferred.

Re: [EXTERNAL] Re: [PATCH v2] bitbake-worker: remove the network flag

[Alexander Kanavin]

On Sun, 10 Sept 2023 at 22:11, Randolph Sapp <rs@ti.com> wrote:
> Seems like people like the current behavior though. That's fine. I just
> think this should be documented somewhere if that's the case. I can
> submit a patch for that documentation update instead, if that's preferred.

I've briefly looked at meta-flutter meanwhile. It's generally full of
horrible things such as AUTOREV, and does abuse the network flag, but
the issue is it's a badly written layer, and so you should take it up
with the layer maintainer:
https://github.com/jwinarske
https://github.com/meta-flutter/meta-flutter/issues/new

Go recipes (not sure which ones because I didn't find any in meta-oe
master but nvm) need to be fixed in the same way we fixed rust
recipes: by listing all items in SRC_URI, and having tooling in place
to update the list automatically from source trees. Patches welcome.

icecc is probably okay, as that's the kind of thing generally
accessible without having to go through a proxy.

Alex

Re: [PATCH v2] bitbake-worker: remove the network flag

[Richard Purdie]

On Sun, 2023-09-10 at 14:46 -0500, rs@ti.com wrote:
> What I feel would be the proper solution to the aforementioned issues.
> 
> Examples of layers that currently use the network flag outside of the do_fetch
> stage:
> 
> 	oe-core (icecc.bbclass)
> 	meta-openembedded (go recipes)
> 	meta-flutter (all flutter recipes and classes)
> 
> This is not an attempt to shame maintainers. I'm just curious why the network
> task expects every maintainer to handle proxy variables (something pretty much
> required if you are actually using the network in that task) independently. It
> just results in needless debugging and a bunch of patches attempting to export
> variables in their own way. Sorry if this comes off as antagonistic in any way.

You are doing yourself no favours at all by posting this patch again.
My instinctive reaction would be to filter all your mails to /dev/null
from now on.

You have to keep in mind that bitbake/OE allow you to do pretty much
anything from metadata so it is hard to put boundaries and any that we
do put in place can and will get bypassed. We've gone backwards and
forwards on these issues for a long time.

The "Yocto Project Compatible" status was born out of a desire to at
least make it possible to highlight layers doing things which could be
considered anti-social. We haven't quite caught up with network
enable/disable there yet but we should and likely will (please file a
bug for that). Sadly the work in maintaining that standard is more
complex than people realise, e.g. a new revision of the criteria has
been held up by a YP website refresh for over a year <snip long painful
story> :(.

We added the network flag since before that, code was doing pretty much
what it wanted anywhere. We now at least can tell exactly where the
accesses are being enabled, which is a start to fixing some of these
issues.

icecc in OE-Core enables it since by its very nature, distributed
compiling needs the network. It is unlikely to need/want proxy info
though as it will likely be local network only.

go needs someone to work on the fetcher side of things for it. We've
solutions with rust/crates which might help provide a model for how
that could work in the future. For better or worse, someone does need
to spend time on it.

I've no experience with flutter to comment there. I will note that most
of these "new" things start with basics and things like proxy support
have to follow later as they're not top of people's minds when they're
trying to get anything working.

I am frustrated in general about a great many areas of the project
where things don't get "completed" to the level everyone would like.
Over time we do have a good track record of sorting them out though, I
just wish people could work on them proactively rather than reactively.
We do have some proactive work happening at least for the first time in
a while with the RFQs.

I do understand your frustration but I think you need to work out a
more productive way to move forward. The network flag did at least let
us mark up and know where the issues are so in that sense we did
improve on what went before.

Cheers,

Richard

Re: [EXTERNAL] Re: [PATCH v2] bitbake-worker: remove the network flag

[Randolph Sapp]

On 9/10/23 15:17, Richard Purdie wrote:
> On Sun, 2023-09-10 at 14:46 -0500, rs@ti.com wrote:
>> What I feel would be the proper solution to the aforementioned issues.
>>
>> Examples of layers that currently use the network flag outside of the do_fetch
>> stage:
>>
>> 	oe-core (icecc.bbclass)
>> 	meta-openembedded (go recipes)
>> 	meta-flutter (all flutter recipes and classes)
>>
>> This is not an attempt to shame maintainers. I'm just curious why the network
>> task expects every maintainer to handle proxy variables (something pretty much
>> required if you are actually using the network in that task) independently. It
>> just results in needless debugging and a bunch of patches attempting to export
>> variables in their own way. Sorry if this comes off as antagonistic in any way.
> 
> You are doing yourself no favours at all by posting this patch again.
> My instinctive reaction would be to filter all your mails to /dev/null
> from now on.
> 
> You have to keep in mind that bitbake/OE allow you to do pretty much
> anything from metadata so it is hard to put boundaries and any that we
> do put in place can and will get bypassed. We've gone backwards and
> forwards on these issues for a long time.
> 
> The "Yocto Project Compatible" status was born out of a desire to at
> least make it possible to highlight layers doing things which could be
> considered anti-social. We haven't quite caught up with network
> enable/disable there yet but we should and likely will (please file a
> bug for that). Sadly the work in maintaining that standard is more
> complex than people realise, e.g. a new revision of the criteria has
> been held up by a YP website refresh for over a year <snip long painful
> story> :(.
> 
> We added the network flag since before that, code was doing pretty much
> what it wanted anywhere. We now at least can tell exactly where the
> accesses are being enabled, which is a start to fixing some of these
> issues.
> 
> icecc in OE-Core enables it since by its very nature, distributed
> compiling needs the network. It is unlikely to need/want proxy info
> though as it will likely be local network only.
> 
> go needs someone to work on the fetcher side of things for it. We've
> solutions with rust/crates which might help provide a model for how
> that could work in the future. For better or worse, someone does need
> to spend time on it.
> 
> I've no experience with flutter to comment there. I will note that most
> of these "new" things start with basics and things like proxy support
> have to follow later as they're not top of people's minds when they're
> trying to get anything working.
> 
> I am frustrated in general about a great many areas of the project
> where things don't get "completed" to the level everyone would like.
> Over time we do have a good track record of sorting them out though, I
> just wish people could work on them proactively rather than reactively.
> We do have some proactive work happening at least for the first time in
> a while with the RFQs.
> 
> I do understand your frustration but I think you need to work out a
> more productive way to move forward. The network flag did at least let
> us mark up and know where the issues are so in that sense we did
> improve on what went before.
> 
> Cheers,
> 
> Richard

Hey Richard,

Thanks for the background. I won't push this issue anymore. The current 
method does seem to be pretty good at passively catching offending tasks.

Distributed compiling itself is an interesting cornercase to both this 
patch and my previous proxy related one that shows that the current 
approach is reasonable and doesn't necessarily warrant either of these 
changes.

Sorry to bother you all on a Sunday.

Regards,
Randolph

Re: [bitbake-devel] [PATCH v2] bitbake-worker: remove the network flag

[Alexandre Belloni]

On 10/09/2023 15:50:22-0500, Randolph Sapp via lists.openembedded.org wrote:
> Thanks for the background. I won't push this issue anymore. The current
> method does seem to be pretty good at passively catching offending tasks.
> 

This is certainly way more important for your customers than you
realize. The proxy issue is not actually an issue because it is very
likely that anyway they would have to set up a local mirror to ensure
they are able to rebuild without relying on a third party. You should
rather teach them to do that instead of working around the networking
issues they are creating for themselves.

This is even more important as the companies having those network issue
are probably the one that care the most about the correctness of spdx.

Really, the proper fix is in the fetcher.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [EXTERNAL] Re: [bitbake-devel] [PATCH v2] bitbake-worker: remove the network flag

[Randolph Sapp]

On 9/10/23 16:16, Alexandre Belloni wrote:
> On 10/09/2023 15:50:22-0500, Randolph Sapp via lists.openembedded.org wrote:
>> Thanks for the background. I won't push this issue anymore. The current
>> method does seem to be pretty good at passively catching offending tasks.
>>
> 
> This is certainly way more important for your customers than you
> realize. The proxy issue is not actually an issue because it is very
> likely that anyway they would have to set up a local mirror to ensure
> they are able to rebuild without relying on a third party. You should
> rather teach them to do that instead of working around the networking
> issues they are creating for themselves.
> 
> This is even more important as the companies having those network issue
> are probably the one that care the most about the correctness of spdx.
> 
> Really, the proper fix is in the fetcher.
> 

Right, I suppose I'm just pessimistic in assuming that users will tend 
to flock to the easier solution of enabling the network flag and not 
circling back to proper method of implementing the fetcher. (I know 
that's what I did for my first layer.)

Education and community involvement ideally will solve this though. I 
just have my reservations about allowing multiple methods of solving an 
issue to exist in parallel. Makes debugging things interesting.

Regardless, thanks Alexandre. :)