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.

Sign up for GitHub

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

Fix media uploads for Docker in rootless mode #50608

Closed
wants to merge 6 commits into from
Closed

Fix media uploads for Docker in rootless mode #50608

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bd29266
Fix media uploads for rootless docker
May 13, 2023
53b1693
Try adding the parents option to mkdir
May 19, 2023
c2f58fc
Try using install instead of mkdir and chown
May 22, 2023
19d2f57
Remove trailing slash
May 22, 2023
fe62240
Update unit test
May 22, 2023
c44b630
Rename user-uploads to wordpress-uploads
May 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions packages/env/lib/build-docker-compose-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ function getMounts(
? `user-home:/home/${ hostUsername }`
: `tests-user-home:/home/${ hostUsername }`;

const userUploadsMount =
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if there is a mappings entry for /var/www/html/wp-content/uploads already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry, I don't really know that much about docker so I may be misunderstanding. This function creates a new Set() which, by definition, doesn't contain duplicates, so I don't think there could already be an entry for it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's take, for example, a wp-env.json file containing this:

{
  "mappings": {
    "/var/www/html/wp-content/uploads": "./env/uploads"
  }
}

This would translate to a unique entry in the Set, but, be a duplicate mapping for /var/www/html/wp-content/upload.

wordpressDefault === 'wordpress'
? `user-uploads:/var/www/html/wp-content/uploads`
ajlende marked this conversation as resolved.
Show resolved Hide resolved
: `tests-user-uploads:/var/www/html/wp-content/uploads`;

const corePHPUnitMount = `${ path.join(
workDirectoryPath,
wordpressDefault === 'wordpress'
Expand All @@ -72,6 +77,7 @@ function getMounts(
coreMount, // Must be first because of some operations later that expect it to be!
corePHPUnitMount,
userHomeMount,
userUploadsMount,
...directoryMounts,
...pluginMounts,
...themeMounts,
Expand Down Expand Up @@ -269,6 +275,8 @@ module.exports = function buildDockerComposeConfig( config ) {
'mysql-test': {},
'user-home': {},
'tests-user-home': {},
'user-uploads': {},
'tests-user-uploads': {},
},
};
};
3 changes: 3 additions & 0 deletions packages/env/lib/init-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ ARG HOST_GID
RUN groupadd -g $HOST_GID $HOST_USERNAME || true
RUN useradd -m -u $HOST_UID -g $HOST_GID $HOST_USERNAME || true

# Make media uploads writeable by the host user.
RUN install -d -o $HOST_UID -g $HOST_GID /var/www/html/wp-content/uploads

# Install any dependencies we need in the container.
${ installDependencies( 'wordpress', env, config ) }`;
}
Expand Down
1 change: 1 addition & 0 deletions packages/env/lib/test/build-docker-compose-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ describe( 'buildDockerComposeConfig', () => {
'wordpress:/var/www/html', // WordPress root.
'/path/WordPress-PHPUnit/tests/phpunit:/wordpress-phpunit', // WordPress test library,
'user-home:/home/test',
'user-uploads:/var/www/html/wp-content/uploads',
'/path/to/wp-plugins:/var/www/html/wp-content/plugins', // Mapped plugins root.
'/path/to/local/plugin:/var/www/html/wp-content/plugins/test-name', // Mapped plugin.
] );
Expand Down