Skip to content

rgw: fix luarocks directory permissions#62227

Merged
yuvalif merged 2 commits intoceph:mainfrom
atta:fix-lua-mkdtemp-permissions
Mar 16, 2026
Merged

rgw: fix luarocks directory permissions#62227
yuvalif merged 2 commits intoceph:mainfrom
atta:fix-lua-mkdtemp-permissions

Conversation

@atta
Copy link
Copy Markdown
Contributor

@atta atta commented Mar 11, 2025

rgw: fix permissions on mudules installed by luarocks

mkdtemp is using 0700 as umask by the time the rgw starts RGW is running as user root, in order to allow acces for user ceph we need to change the umask to 755

Fixes: https://tracker.ceph.com/issues/70447

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands

@atta atta requested a review from a team as a code owner March 11, 2025 13:07
@github-actions github-actions bot added the rgw label Mar 11, 2025
ldpp_dout(dpp, 1) << "Lua ERROR: failed to create temporary directory from template: " <<
tmp_path_template << ". error: " << rc << dendl;
return rc;
} else {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

making the direcory works ok. it is the call in line 268 is failing.
also, luarocks needs write access to that directory

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the luarocks install is also working fine as it is still running as user root:

tree /tmp/rgw_luarocks/0SejpX/
/tmp/rgw_luarocks/0SejpX/
|-- lib
|   |-- lua
|   |   `-- 5.3
|   |       |-- mime
|   |       |   `-- core.so
|   |       `-- socket
|   |           |-- core.so
|   |           |-- serial.so
|   |           `-- unix.so
|   `-- luarocks

but as soon as the rgw drops to the user ceph in normal operration access to the luarocks files is no longer possible

Copy link
Copy Markdown
Member

@batrick batrick left a comment

Choose a reason for hiding this comment

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

Please title your commits according to the conventions of this project. This is required for your PR to be merged.

See this article on GitHub on how to amend commits and update your pull request.

Copy link
Copy Markdown
Member

@batrick batrick left a comment

Choose a reason for hiding this comment

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

Please sign your commits. This is required for your PR to be merged.

See this article on GitHub on how to amend commits and update your pull request.

@atta atta changed the title as the rgw can start as root and later drop the rights to user ceph w… rgw: fix luarocks directory permissions Mar 12, 2025
@atta atta force-pushed the fix-lua-mkdtemp-permissions branch from 95e55ae to 1c75bc6 Compare March 12, 2025 09:36
@atta
Copy link
Copy Markdown
Contributor Author

atta commented Mar 12, 2025

change the commit-message and singed it

@atta atta requested a review from batrick March 12, 2025 09:52
@batrick batrick dismissed their stale review March 12, 2025 19:46

addressed

@yuvalif yuvalif self-requested a review March 13, 2025 09:58
@atta atta force-pushed the fix-lua-mkdtemp-permissions branch from 1c75bc6 to 0b914bb Compare March 19, 2025 15:58
@ivancich ivancich added the wip-eric-testing-1 for ivancich testing label Mar 19, 2025
@ivancich
Copy link
Copy Markdown
Member

@atta Please fill out the checklist and create a tracker if necessary.

@atta
Copy link
Copy Markdown
Contributor Author

atta commented Mar 19, 2025

@atta Please fill out the checklist and create a tracker if necessary.

@ivancich the tracker issue, i do not know if i need to link it somehow to github?

Copy link
Copy Markdown
Member

@ivancich ivancich left a comment

Choose a reason for hiding this comment

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

This was not ready for a qa run.

return rc;
} else {
// rgw starts as root and will later drop to user ceph
umask(tmp_luarocks_path, 0755)
Copy link
Copy Markdown
Member

@ivancich ivancich Mar 19, 2025

Choose a reason for hiding this comment

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

Missing semicolon prevents this from compiling. In general it's best to compile and do your own basic testing before adding the needs-qa label or getting reviews.

@ivancich ivancich removed needs-qa wip-eric-testing-1 for ivancich testing labels Mar 19, 2025
@ivancich
Copy link
Copy Markdown
Member

@atta Please fill out the checklist and create a tracker if necessary.

@ivancich the tracker issue, i do not know if i need to link it somehow to github?

Yeah, in the main comment we usually add "Fixes: ". I'll add it here.

@atta atta force-pushed the fix-lua-mkdtemp-permissions branch from 0b914bb to 21f6597 Compare March 21, 2025 06:28
@cbodley
Copy link
Copy Markdown
Contributor

cbodley commented Mar 21, 2025

from the failling checks:

src/rgw/rgw_lua.cc:243:5: error: no matching function for call to 'umask'
  243 |     umask(tmp_luarocks_path, 0755);
      |     ^~~~~
/usr/include/x86_64-linux-gnu/sys/stat.h:380:17: note: candidate function not viable: requires single argument '__mask', but 2 arguments were provided
  380 | extern __mode_t umask (__mode_t __mask) __THROW;
      |                 ^      ~~~~~~~~~~~~~~~

maybe you want chmod instead?

@yuvalif
Copy link
Copy Markdown
Contributor

yuvalif commented Mar 24, 2025

to make it more OS agnostic, maybe we can use boost::filesystem.
combine unique_path and create_directory().
according to do: https://www.boost.org/doc/libs/1_87_0/libs/filesystem/doc/reference.html#create_directory
directory is created with S_IRWXU|S_IRWXG|S_IRWXO

mkdtemp creates the temporary directory with 0700 permissions by default, which restricts access to the user that created it. Since RGW can start as root and later drop privileges to the ceph user, the installed Lua code needs to be accessible by other users.

This commit explicitly sets the umask (chmod) to ensure the temporary directory has appropriate permissions (755), allowing access to all users.

Signed-off-by: Ansgar Jazdzewski <ansgar.jazdzewski@hetzner-cloud.de>
@atta atta force-pushed the fix-lua-mkdtemp-permissions branch from 21f6597 to 827de0d Compare May 13, 2025 08:24
@github-actions
Copy link
Copy Markdown

Config Diff Tool Output

+ added: crimson_poll_mode (crimson.yaml.in)
+ added: osd_pool_default_flag_ec_optimizations (global.yaml.in)
+ added: ceph_assert_supresssions (global.yaml.in)
+ added: bluestore_debug_extent_map_encode_check (global.yaml.in)
+ added: ec_pdw_write_mode (global.yaml.in)
+ added: ec_extent_cache_size (global.yaml.in)
+ added: bluestore_onode_segment_size (global.yaml.in)
+ added: bluestore_debug_onode_segmentation_random (global.yaml.in)
+ added: bluefs_wal_envelope_mode (global.yaml.in)
+ added: bluestore_allocator_lookup_policy (global.yaml.in)
+ added: breakpad (global.yaml.in)
+ added: bluestore_recompression_min_gain (global.yaml.in)
+ added: client_file_blockdiff_max_concurrent_object_scans (mds-client.yaml.in)
+ added: osd_recovery_sleep_degraded (osd.yaml.in)
+ added: osd_recovery_sleep_degraded_hybrid (osd.yaml.in)
+ added: osd_recovery_sleep_degraded_hdd (osd.yaml.in)
+ added: osd_recovery_sleep_degraded_ssd (osd.yaml.in)
+ added: rgw_max_control_aio (rgw.yaml.in)
+ added: d4n_writecache_enabled (rgw.yaml.in)
+ added: rgw_d4n_l1_write_open_flags (rgw.yaml.in)
+ added: rgw_bucket_eexist_override (rgw.yaml.in)
+ added: rgw_d4n_cache_cleaning_interval (rgw.yaml.in)
+ added: rgw_d4n_backend_address (rgw.yaml.in)
+ added: mds_file_blockdiff_max_concurrent_object_scans (mds.yaml.in)
+ added: enable_availability_tracking (mon.yaml.in)
+ added: mon_nvmeofgw_skip_failovers_interval (mon.yaml.in)
+ added: mon_nvmeofgw_wrong_map_ignore_sec (mon.yaml.in)
! changed: mgr_data: old: Filesystem path to the ceph-mgr data directory, used to contain keyring. (mgr.yaml.in)
! changed: mgr_data: new: Filesystem path to the Manager's data directory, which contains keyrings and other data (mgr.yaml.in)
! changed: seastore_cache_lru_size: old: Size in bytes of extents to keep in cache. (crimson.yaml.in)
! changed: seastore_cache_lru_size: new: Size in bytes of extents to keep in cache (per reactor). (crimson.yaml.in)
! changed: seastore_cache_lru_size: old: 64_M (crimson.yaml.in)
! changed: seastore_cache_lru_size: new: 2_G (crimson.yaml.in)
! changed: seastore_max_concurrent_transactions: old: maximum concurrent transactions that seastore allows (crimson.yaml.in)
! changed: seastore_max_concurrent_transactions: new: maximum concurrent transactions that seastore allows (per reactor) (crimson.yaml.in)
! changed: seastore_max_concurrent_transactions: old: 8 (crimson.yaml.in)
! changed: seastore_max_concurrent_transactions: new: 128 (crimson.yaml.in)
! changed: bluestore_write_v2_random: old: For testing purposes. If true, value of bluestore_write_v2 is randomly selected. (global.yaml.in)
! changed: bluestore_write_v2_random: new: For testing purposes. If true, value of bluestore_write_v2 is randomly selected on each mount. (global.yaml.in)
! changed: mon_warn_pg_not_deep_scrubbed_ratio: old: Percentage of the deep scrub interval past the deep scrub interval to warn (global.yaml.in)
! changed: mon_warn_pg_not_deep_scrubbed_ratio: new: Percentage of the deep scrub interval past the deep scrub interval to warn - Set this configurable with the command "ceph config set mgr mon_warn_pg_not_deep_scrubbed_ratio <ratio_value>" (global.yaml.in)
! changed: bluestore_rocksdb_cf: old: #ifdef WITH_SEASTAR
// This is necessary as the Seastar's allocator imposes restrictions
// on the number of threads that entered malloc/free/*. Unfortunately,
// RocksDB sharding in BlueStore dramatically lifted the number of
// threads spawn during RocksDB's init.
.set_validator([](std::string *value, std::string *error_message) {
  if (const bool parsed_value = strict_strtob(value->c_str(), error_message);
    error_message->empty() && parsed_value) {
    *error_message = "invalid BlueStore sharding configuration."
                     " Be aware any change takes effect only on mkfs!";
    return -EINVAL;
  } else {
    return 0;
  }
})
#endif
 (global.yaml.in)
! changed: bluestore_rocksdb_cf: new: #ifdef WITH_CRIMSON
// This is necessary as the Seastar's allocator imposes restrictions
// on the number of threads that entered malloc/free/*. Unfortunately,
// RocksDB sharding in BlueStore dramatically lifted the number of
// threads spawn during RocksDB's init.
.set_validator([](std::string *value, std::string *error_message) {
  if (const bool parsed_value = strict_strtob(value->c_str(), error_message);
    error_message->empty() && parsed_value) {
    *error_message = "invalid BlueStore sharding configuration."
                     " Be aware any change takes effect only on mkfs!";
    return -EINVAL;
  } else {
    return 0;
  }
})
#endif
 (global.yaml.in)
! changed: mon_warn_pg_not_scrubbed_ratio: old: Percentage of the scrub max interval past the scrub max interval to warn (global.yaml.in)
! changed: mon_warn_pg_not_scrubbed_ratio: new: Raise a health warning when shallow scrubs are delayed by this percentage of the shallow scrub interval. Note that this must be set at mgr or with global scope. Set this configurable with the command "ceph config set mgr mon_warn_pg_not_scrubbed_ratio <ratio_value>". (global.yaml.in)
! changed: osd_max_scrubs: old: This setting is ignored when the mClock scheduler is used. (osd.yaml.in)
! changed: osd_max_scrubs: new:  (osd.yaml.in)
! changed: osd_class_load_list: old: cephfs hello journal lock log numops otp rbd refcount rgw rgw_gc timeindex user version cas cmpomap queue 2pc_queue fifo (osd.yaml.in)
! changed: osd_class_load_list: new: cephfs hello journal lock log numops otp rbd refcount rgw rgw_gc timeindex user version cas cmpomap queue 2pc_queue fifo sem_set (osd.yaml.in)
! changed: osd_deep_scrub_interval: old: Deep scrub each PG (i.e., verify data checksums) at least this often (osd.yaml.in)
! changed: osd_deep_scrub_interval: new: Deep scrub each PG (i.e., verify data checksums) at least this often. Note that this option must be set at ``global`` scope, or for both ``mgr`` and``osd``. (osd.yaml.in)
! changed: osd_deep_scrub_interval_cv: old: determining the amount of variation in the deep scrub interval (osd.yaml.in)
! changed: osd_deep_scrub_interval_cv: new: Determines the amount of variation in the deep scrub interval (osd.yaml.in)
! changed: osd_deep_scrub_interval_cv: old: The coefficient of variation for the deep scrub interval, specified as a ratio. On average, the next deep scrub for a PG is scheduled osd_deep_scrub_interval after the last deep scrub . The actual time is randomized to a normal distribution with a standard deviation of osd_deep_scrub_interval * osd_deep_scrub_interval_cv (clamped to within 2 standard deviations). The default value guarantees that 95% of the deep scrubs will be scheduled in the range [0.8 * osd_deep_scrub_interval, 1.2 * osd_deep_scrub_interval]. (osd.yaml.in)
! changed: osd_deep_scrub_interval_cv: new: The coefficient of variation for the deep scrub interval, specified as a ratio. On average, the next deep scrub for a PG is scheduled osd_deep_scrub_interval after the last deep scrub . The actual time is randomized to a normal distribution with a standard deviation of osd_deep_scrub_interval * osd_deep_scrub_interval_cv (clamped to within 2 standard deviations). The default value guarantees that 95% of deep scrubs will be scheduled in the range [0.8 * osd_deep_scrub_interval, 1.2 * osd_deep_scrub_interval]. (osd.yaml.in)
! changed: osd_deep_scrub_interval_cv: old: deep scrub intervals are varied by a random amount to prevent stampedes. This parameter determines the amount of variation. Technically - osd_deep_scrub_interval_cv is the coefficient of variation for the deep scrub interval. (osd.yaml.in)
! changed: osd_deep_scrub_interval_cv: new: Deep scrub intervals are varied by a random amount to prevent stampedes. This parameter determines the amount of variation. Technically ``osd_deep_scrub_interval_cv`` is the coefficient of variation for the deep scrub interval. (osd.yaml.in)
! changed: osd_scrub_min_interval: old: The desired interval between scrubs of a specific PG. (osd.yaml.in)
! changed: osd_scrub_min_interval: new: The desired interval between scrubs of a specific PG. Note that this option must be set at ``global`` scope, or for both ``mgr`` and``osd``. (osd.yaml.in)
! changed: osd_class_default_list: old: cephfs hello journal lock log numops otp rbd refcount rgw rgw_gc timeindex user version cas cmpomap queue 2pc_queue fifo (osd.yaml.in)
! changed: osd_class_default_list: new: cephfs hello journal lock log numops otp rbd refcount rgw rgw_gc timeindex user version cas cmpomap queue 2pc_queue fifo sem_set (osd.yaml.in)
! changed: osd_scrub_max_interval: old: Scrub each PG no less often than this interval (osd.yaml.in)
! changed: osd_scrub_max_interval: new: Scrub each PG no less often than this interval. Note that this option must be set at ``global`` scope, or for both ``mgr`` and``osd``. (osd.yaml.in)
! changed: rgw_admin_entry: old:  (rgw.yaml.in)
! changed: rgw_admin_entry: new: Note that multisite replication requires the value admin, so this option must be left at the default in such deployments. (rgw.yaml.in)
! changed: rgw_num_control_oids: old:  (rgw.yaml.in)
! changed: rgw_num_control_oids: new: ['rgw_cache_enabled', 'rgw_max_control_aio'] (rgw.yaml.in)
! changed: rgw_lfuda_sync_frequency: old:  (rgw.yaml.in)
! changed: rgw_lfuda_sync_frequency: new: ['startup'] (rgw.yaml.in)
! changed: rgw_sts_key: old: Key used for encrypting/ decrypting session token. (rgw.yaml.in)
! changed: rgw_sts_key: new: Key used for encrypting/ decrypting role session tokens. This key must consist of 16 hexadecimal characters, which can be generated by the command 'openssl rand -hex 16'. All radosgw instances in a zone should use the same key. In multisite configurations, all zones in a realm should use the same key. (rgw.yaml.in)

The above configuration changes are found in the PR. Please update the relevant release documentation if necessary.
Ignore this comment if docs are already updated. To make the "Check ceph config changes" CI check pass, please comment /config check ok and re-run the test.

@github-actions
Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added the stale label Aug 18, 2025
@yuvalif
Copy link
Copy Markdown
Contributor

yuvalif commented Aug 25, 2025

unstale please

@github-actions github-actions bot removed the stale label Aug 25, 2025
@github-actions
Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added the stale label Oct 24, 2025
@yuvalif
Copy link
Copy Markdown
Contributor

yuvalif commented Oct 26, 2025

unstale please

@github-actions github-actions bot removed the stale label Oct 26, 2025
@github-actions
Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days.
If you are a maintainer or core committer, please follow-up on this pull request to identify what steps should be taken by the author to move this proposed change forward.
If you are the author of this pull request, thank you for your proposed contribution. If you believe this change is still appropriate, please ensure that any feedback has been addressed and ask for a code review.

@github-actions github-actions bot added the stale label Dec 25, 2025
@ceph-jenkins
Copy link
Copy Markdown
Collaborator

Can one of the admins verify this patch?

@yuvalif
Copy link
Copy Markdown
Contributor

yuvalif commented Feb 11, 2026

/config check ok

@ivancich
Copy link
Copy Markdown
Member

The QA run for this PR included two others. But they're showing a lot of crash related errors.

There are six of these:

"2026-02-25T05:26:33.173487+0000 mon.a (mon.0) 181 : cluster [WRN] Health check failed: 2 daemons have recently crashed (RECENT_CRASH)" in cluster log

And there are nine of these, mostly dealing with the upgrade tests.

Found coredumps on ubuntu@trial096.front.sepia.ceph.com

None of the PRs jumps out at me as a likely culprit, so I'm asking you. I'm also having a re-run done.

Here's the full run: https://pulpito.ceph.com/anuchaithra-2026-02-25_05:07:15-rgw-wip-anrao1-testing-2026-02-23-1551-distro-default-trial/

Thanks!

@anrao19
Copy link
Copy Markdown
Contributor

anrao19 commented Mar 16, 2026

Tracker approved by : @ivancich, details are in https://tracker.ceph.com/issues/75121
@atta , if no further testing is needed, Please let us know if we can merge

@ceph-jenkins
Copy link
Copy Markdown
Collaborator

Thank you for your contribution. Since you, the author, are not a member of the Ceph GitHub Org yet, our CI will not automatically run. Any member of the Ceph Org may comment "ok to test" to allow the Jenkins jobs to run.

@yuvalif
Copy link
Copy Markdown
Contributor

yuvalif commented Mar 16, 2026

Tracker approved by : @ivancich, details are in https://tracker.ceph.com/issues/75121 @atta , if no further testing is needed, Please let us know if we can merge

the lua test was passing (https://qa-proxy.ceph.com/teuthology/anuchaithra-2026-03-05_15:00:47-rgw-wip-anrao1-testing-2026-03-05-1405-distro-default-trial/88115/teuthology.log), so, if there is not regression in other tests we can merge

@yuvalif yuvalif merged commit 824865d into ceph:main Mar 16, 2026
17 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants