Do not run commands as other user as we lose env variables

Created on 9 November 2023, over 1 year ago

Problem/Motivation

Env variables are not passed when running commands via sudo, and we need to declare them explicitly. It'd be great if all the variables that are set are available when running the tests commands, which are run as "www-data" user via "sudo -u" commands.

Steps to reproduce

See #3400218: [GitlabCI] run-tests.sh does not pass on SYMFONY_DEPRECATIONS_HELPER environment variable value to spawned processes β†’ where "SYMFONY_DEPRECATIONS_HELPER" was not being recognized by "run-tests.sh".

Proposed resolution

As suggested by @longwave in the parent issue in #4 option 3:

Stop using sudo entirely, we are in throwaway containers so does it matter if the tests run as root?

After this is done here, the "Drupal core" gitlabci files could use the same changes.

Remaining tasks

MR

User interface changes

API changes

Data model changes

✨ Feature request
Status

Active

Component

gitlab-ci

Created by

πŸ‡ͺπŸ‡ΈSpain fjgarlin

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @fjgarlin
  • πŸ‡ΊπŸ‡ΈUnited States cmlara

    Running as root could silently hide a few disk access related flaws:

    $BLANK='';
    mkdir("$BLANK/test_dir");
    

    would succeed if running as root, but (likely) fail as www-data.

    https://www.drupal.org/docs/automated-testing/phpunit-in-drupal/running-... β†’ calls for using the -E flag.

  • πŸ‡ͺπŸ‡ΈSpain fjgarlin

    That's a good point and a great argument to keep using www-data user.

    Note that we tried on the mentioned issue the use of -E but that did not work (see this comment β†’ ).

    That's exactly why I created this issue as follow-up. I did the quick fix for the other one and then decide on approach and investigation on this one.

    I'm happy with whatever makes the most sense. The -E seems to be the less intrusive, but we need to make it work without requiring the sudo password.

  • πŸ‡ΊπŸ‡ΈUnited States cmlara
    Test run started:
      Wednesday, November 8, 2023 - 19:35
    Test summary
    ------------
    We trust you have received the usual lecture from the local System
    Administrator. It usually boils down to these three things:
        #1) Respect the privacy of others.
    

    To me this looks likely the script might have been ran (successfully) as www-data and inside it tried to sudo again since we don't appear to output any of that text as part of the Gitlab templates.

    The 36 counts of sudo: a password is required implies it happened for each and every "Test to be run" listed and not just the parent run.

  • πŸ‡ͺπŸ‡ΈSpain fjgarlin

    Good catch. So we might need to also alter something inside "run-tests.sh" to get this to work.

  • πŸ‡ͺπŸ‡ΈSpain fjgarlin

    Trying again the -E flag in πŸ› BROWSERTEST_OUTPUT_VERBOSE is not passed to run-tests.sh when running with it Active didn't work. Then an alternative solution was suggested. I suggest that we leave that fix as it will solve the env variables issue and that we maybe use this issue to have it behind a flag that can be turn on/off as needed .

  • πŸ‡ͺπŸ‡ΈSpain fjgarlin

    I'll close this in favour of the other one.

Production build 0.71.5 2024