[VRP] Add constant for checking allowed platforms - #5450
Conversation
dylanjew
left a comment
There was a problem hiding this comment.
Is this only a temporary problem? Will all of the Testcases scheduled for Mac/windows eventually be trusted=True?
| allowed_platforms = [ | ||
| p.strip().lower() | ||
| for p in enable_untrusted_flag.string_value.split(',') | ||
| if p.strip() |
There was a problem hiding this comment.
does this handle if there is just one platform? do you stil need to lower() the input allowed_platforms?
There was a problem hiding this comment.
Yes, this handles cases where string_value is, for example, 'MAC', resulting in allowed_platforms = ['mac']. The lower() call is included as a safety measure in case the value was incorrectly set using uppercase letters.
I guess what I'm getting at is that the point of having the Do we need to go through and backfill the Testcases.trusted field from fuzzers that are enabled on windows and mac? |
Just to highlight that the issue is that there is no untrusted worker logic for windows/mac/android platforms. All testcases and fuzzers for these platforms must be "trusted" in the sense of being able to run on long-lived privileged bots (that's the current state). I think testcases uploaded to clusterfuzz are marked as untrusted. And there are codepaths that are blocked by the testcase not being marked as trusted which are not really related to bot execution, so we should keep the trusted=false even though they can still run on bots for that platform. If we remove this, we should understand the impact for most use cases. @PauloVLB may add more details about that |
| SWARMING_MAX_PENDING_TASKS = 'swarming_max_pending_tasks' | ||
|
|
||
| ENABLE_FUZZ_FOR_BOTS = 'enable_fuzz_for_bots' | ||
| ENABLE_UNTRUSTED_TESTCASES_FOR_BOTS = 'enable_untrusted_testcases_for_bots' |
There was a problem hiding this comment.
I think this name is not very clear, without context it is difficult to understand. It should have something related to platforms that support the trusted/untrusted model.
There was a problem hiding this comment.
Also, I think we should set it as a global var instead of a feature flag. This will be true for android/mac/windows until there are significant architectural changes and not a feature that we can simply enable/disable. Wdyt?
There was a problem hiding this comment.
Agreed. I changed the name to PLATFORMS_SUPPORTING_UNTRUSTED_WORKLOADS and removed the feature flag.
Exactly. Since we are not yet able to run test cases for non-Linux platforms in ephemeral environments, we are obligated to run them on long-lived bots.
Yes. In
Indeed: clusterfuzz/src/clusterfuzz/_internal/bot/tasks/utasks/progression_task.py Lines 430 to 431 in d937566 I agree that we should not backfill non-Linux testcases as |
|
|
||
|
|
||
| def is_untrusted_testcase_allowed_on_bot(testcase) -> bool: | ||
| """Returns True if the untrusted testcase is on a platform that does not |
There was a problem hiding this comment.
nit: go/pystyle#function-docs - although this is not nearly close to be enforced on clusterfuzz legacy code, usually it is better to have a one-liner docstring followed by a more detailed description after a blank line.
| def is_untrusted_testcase_allowed_on_bot(testcase) -> bool: | ||
| """Returns True if the untrusted testcase is on a platform that does not | ||
| support the untrusted execution model (so it must run on a long-lived bot).""" | ||
| job = data_types.Job.query(data_types.Job.name == testcase.job_type).get() |
There was a problem hiding this comment.
Why not use testcase.platform()? Can you check if this field is filled in the database?
b/556677120
Problem
Long-lived bots currently exit with a fatal error whenever they encounter an untrusted testcase (
trusted=False). However, there are existing testcases in Datastore marked as untrusted that belong to non-Linux jobs (e.g. Windows, Mac). Since uworker execution only runs in Linux containers, non-Linux tasks have no other way to execute and are completely blocked.This was not caught during dev testing due to the lack of non-Linux VMs in the dev environment. This change does not affect the VRP security model because untrusted external fuzzers are strictly restricted to Linux jobs.
Proposed Solution
PLATFORMS_SUPPORTING_UNTRUSTED_WORKLOADS = {'linux'}inuworker_io.py.is_untrusted_testcase_allowed_on_bot(testcase)inuworker_io.pyto check if the testcase's job platform is outsidePLATFORMS_SUPPORTING_UNTRUSTED_WORKLOADS.check_handling_testcase_safeinuworker_io.pyto allow execution on long-lived bots for non-Linux jobs.Testing
uworker_io_test.pyverifying that untrusted testcases on non-Linux platforms (Windows, Mac, Android) pass on long-lived bots, while Linux jobs raiseSystemExit.python butler.py py_unittest -t core -p uworker_io_test.pypython butler.py lint