Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[universal] - Issue universal config change for non-root default codespace user and installing google chrome browser reuse sandbox to run puppeteer cli in universal image #1287

Conversation

Kaniska244
Copy link
Contributor

@Kaniska244 Kaniska244 commented Jan 22, 2025

Ref: https://github.com/devcontainers/internal/issues/247
https://github.com/devcontainers/internal/issues/249

Dev container name:

Universal

Description:

We can determine UID of host machine via id -u & id -g and pass it to docker while building the image. Modified devcontainer.json as below resolved the issue.

Changelog:

For Universal UID permission issue

  • Updated GitHub workflows to determine UID of host machine via id -u & id -g and pass it to GITHUB_ENV & updated devcontainer.json file to read the UID & GID from localEnv.

For puppeteer library issue

  • Updated setup-user local feature to install chrome & also changed the devcontainer.json to populate a specific env variable in the remoteEnv, required for puppeteer library.

Checklist:

  • Checked that applied changes work as expected.

@Kaniska244 Kaniska244 marked this pull request as ready for review January 22, 2025 16:21
@Kaniska244 Kaniska244 requested a review from a team as a code owner January 22, 2025 16:21
@Kaniska244
Copy link
Contributor Author

Hi @ddoyle2017 ,

Tested another workaround for this instead of forcing the UID 1001 as that was increasing the size of the image considerably.

  • Setting "updateRemoteUserUID" flag to false so that UID & GID aren't replaced, so that the UID & GID of codespace user remains 1000.

  • Adding "postStartCommand" to change the ownership of /workspaces/images folder to the same as of the codespace user, not using "postCreateCommand" option as its already used in other sub-features in universal image. This folder is directly mounted from the local machine containing the images repo sources.

  • Tests are fine with this. Also tested locally in a codespace along with the devcontainer CI pipeline.

Would you kindly review the same. Please let me know in case of any concern.

With Regards,
Kaniska

Copy link
Contributor

@ddoyle2017 ddoyle2017 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there! Can we revert the puppeteer --no-sandbox changes and rerun the Actions?

@Kaniska244
Copy link
Contributor Author

Kaniska244 commented Jan 28, 2025

Hi @ddoyle2017 ,

I have tested one fix for the puppeteer cli issue. Uploaded the fix. Would you kindly review the same & let me know in case of any concern.
Please note the change is only in the test script as the purpose of the test is to demonstrate that its possible to run a puppeteer node app in the universal container.

PFB the details:

  • The actual issue was that there was no sandbox available to launch the browser when runner image is on ubuntu-24.04.
  • Downloaded google chrome amd64-deb variant latest stable version & installed the same in the container.
  • Chrome is shipped with a default sandbox, copied the same & populated this CHROME_DEVEL_SANDBOX variable which is used by the puppeteer by default.
  • All tests are fine with this & this appears to be the safest approach compared to the other approaches which would require to launch the browser directly in the container without the sandbox or modifying the kernel to allow unprivileged access.

Ref - https://github.com/puppeteer/puppeteer/blob/88ef09c6ccc27d024972725c728a9db50f010f36/docs/troubleshooting.md#setting-up-chrome-linux-sandbox

With Regards,
Kaniska Sengupta

Copy link
Contributor

@ddoyle2017 ddoyle2017 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a reasonable work around for the Puppeteer issues with Ubuntu 24.04! Can you add a comment above both your changes to explain what issue they're solving?

@ddoyle2017
Copy link
Contributor

hey @Kaniska244, so @eljog and I had a discussion about how to move forward with the release and I think we need to reorganize your changes for Puppeteer.

Currently, your Chrome workaround works for our CI test, but Puppeteer will still be broken for users of this image. We should apply your updates to the image as a whole (not just in test.sh), then see if that fixes the issue 👍

@Kaniska244
Copy link
Contributor Author

hey @Kaniska244, so @eljog and I had a discussion about how to move forward with the release and I think we need to reorganize your changes for Puppeteer.

Currently, your Chrome workaround works for our CI test, but Puppeteer will still be broken for users of this image. We should apply your updates to the image as a whole (not just in test.sh), then see if that fixes the issue 👍

Hi @ddoyle2017 ,

I have made the changes according to the comments & all tests are fine with this. Would you kindly review the latest change.
PFB the details of the change.

  • Changed the postStartCommand option to postCreateCommand as per the review comments.
  • Made change in Dockerfile to install three more libraries which are required to install chrome.
  • Made change in install.sh of the local feature setup-user to install chrome & placing the sandbox in the desired location. Please let me know if I should introduce a new local feature for this instead.
  • Made change in devcontainer.json to configure "remoteEnv" tag for the CHROME_DEVEL_SANDBOX variable with the sandbox location which is used by the puppeteer library while launching the browser.
  • Size of the image has now increased considerably due to the chrome installation which is still within the configured threshold but only by about 64 MB. With the previous change the gap was about 422 MB.
  • Added comments on top of the changes for more clarity.

With Regards,
Kaniska

@Kaniska244 Kaniska244 changed the title [universal] - Issue universal config change for non-root default codespace user in universal image [universal] - Issue universal config change for non-root default codespace user and installing google chrome browser reuse sandboc to run puppeteer cli in universal image Feb 4, 2025
@Kaniska244
Copy link
Contributor Author

Hi @ddoyle2017 , Hi @eljog ,

I have made further changes on the solution of the UID issue after consulting with @Mathiyarasy in order to achieve more clean, error-proof solution. Would you kindly review the same.

PFB the details of the changes:

  • Changed the GitHub action workflows(universal smoke test & all the build and push workflows) to copy the UID & GID of the host & set it as environment variable.

  • Changed the devcontainer.json to read the environment variables for UID & GID instead of hardcoding them to 1000.

  • In case if the environment variables aren't set, the UID & GID are defaulted to 'automatic' option in common-utils feature which creates the user & group with the default available UID & GID as 1000. The build doesn't fail in that case as I tested in my local where the build was manually triggered, not by workflow.

  • Made changes in all of the build & push workflows to fetch the UID/ GID and set the environment variables. However not able to test them independently in the forked repo so far. Same change works fine for smoke test universal workflow.

  • The image size is also back within the configured threshold with this change.

  • Also kindly note that jekyll smoke test is failing but I see it has already been failing with the same, so not related to this.

Please let me know in case of further concerns with this latest change.

With Regards,
Kaniska

ddoyle2017
ddoyle2017 previously approved these changes Feb 11, 2025
Copy link
Contributor

@ddoyle2017 ddoyle2017 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good 🚀 I think this approach makes sense, based on Option 3 of Chromium's recommended AppArmor workarounds

@Kaniska244 Kaniska244 changed the title [universal] - Issue universal config change for non-root default codespace user and installing google chrome browser reuse sandboc to run puppeteer cli in universal image [universal] - Issue universal config change for non-root default codespace user and installing google chrome browser reuse sandbox to run puppeteer cli in universal image Feb 13, 2025
Copy link
Contributor

@chrmarti chrmarti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few questions.

@Kaniska244 Kaniska244 requested a review from chrmarti February 24, 2025 04:48
Copy link
Contributor

@chrmarti chrmarti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants