Remove spurious SGID bit on directories #9447

smortex · 2023-08-19T03:33:04Z

Description

While working on improving OpenSearch packaging in opensearch-project/opensearch-build#3898, we saw that some unexpected SGID bit was set on some directories and that we had to remove it to match the default packages behavior of packages on Debian.

The SGID bit can be tracked to rjernst/elasticsearch@a5c18f1 where it was added so that on first start, the opensearch service has access to the keystore initialized by the root user. However, when deploying OpenSearch today, the keystore is created by the opensearch user and the SGID bit is not required anymore.

This PR remove the code that sets these SGID bits so that we do not need to clean them anymore when packaging.

Related Issues

Check List

Commits are signed per the DCO using --signoff

github-actions · 2023-08-19T03:38:51Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/23055/
CommitID: bc09ed5
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2023-08-19T04:40:41Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/23057/
CommitID: 3300b4d
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

dbwiddis

LGTM. Do we need a change log entry for this?

reta · 2023-08-21T17:34:39Z

LGTM. Do we need a change log entry for this?

Yes, I think we need

opensearch-trigger-bot · 2023-08-21T19:00:05Z

Compatibility status:

Checks if related components are compatible with change 61c5f17

Incompatible components

Incompatible components: [https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/security-analytics.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

opensearch-trigger-bot · 2023-08-21T19:00:42Z

Compatibility status:

Checks if related components are compatible with change 61c5f17

Incompatible components

Incompatible components: [https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/security-analytics.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

github-actions · 2023-08-21T19:35:40Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      2 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.classMethod
      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testScrollWithConcurrentIndexAndSearch

URL: https://build.ci.opensearch.org/job/gradle-check/23137/
CommitID: 84d5215
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

codecov · 2023-08-21T19:36:23Z

Codecov Report

Merging #9447 (efaf798) into main (8807d7a) will increase coverage by 0.07%.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #9447      +/-   ##
============================================
+ Coverage     71.04%   71.12%   +0.07%     
- Complexity    58167    58206      +39     
============================================
  Files          4830     4830              
  Lines        274506   274505       -1     
  Branches      40002    40002              
============================================
+ Hits         195018   195234     +216     
+ Misses        63153    62921     -232     
- Partials      16335    16350      +15

Files	Coverage Δ
...arch/analysis/common/MappingCharFilterFactory.java	`50.00% <100.00%> (ø)`
...n/java/org/opensearch/index/analysis/Analysis.java	`84.21% <100.00%> (-0.12%)`	⬇️

... and 465 files with indirect coverage changes

distribution/packages/build.gradle

opensearch-trigger-bot · 2023-09-27T15:20:40Z

This PR is stalled because it has been open for 30 days with no activity.

Setting the SGID bit on directories is maybe something some users will want to use, but setting it by default for all users does not really make sense and when packaging OpenSearch, we need to remove this customization when building packges. This was added to ElasticSearch to make it possible to manage the keystore as root while the service runs as an unprivileged user. Without the SGID trick, the generated keystore was owned by root and ElasticSearch could not access it. It is preferable to manage the keystore with non-root privileges, and this hack is not required in this case. Stick to the default permissions and remove this personalization. Signed-off-by: Romain Tartière <[email protected]>

smortex · 2023-09-28T00:15:44Z

Rebased on top of main to fix conflict and adjusted the commit message.

github-actions · 2023-09-28T00:33:19Z

Compatibility status:

Checks if related components are compatible with change efaf798

Incompatible components

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/custom-codecs.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/neural-search.git]

github-actions · 2023-09-28T00:52:21Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/26415/
CommitID: 2214f89
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2023-09-28T01:00:40Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.search.SearchWeightedRoutingIT.testSearchAggregationWithNetworkDisruption_FailOpenEnabled

URL: https://build.ci.opensearch.org/job/gradle-check/26416/
CommitID: efaf798
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

Setting the SGID bit on directories is maybe something some users will want to use, but setting it by default for all users does not really make sense and when packaging OpenSearch, we need to remove this customization when building packges. This was added to ElasticSearch to make it possible to manage the keystore as root while the service runs as an unprivileged user. Without the SGID trick, the generated keystore was owned by root and ElasticSearch could not access it. It is preferable to manage the keystore with non-root privileges, and this hack is not required in this case. Stick to the default permissions and remove this personalization. Signed-off-by: Romain Tartière <[email protected]> (cherry picked from commit 9d0db5e) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Setting the SGID bit on directories is maybe something some users will want to use, but setting it by default for all users does not really make sense and when packaging OpenSearch, we need to remove this customization when building packges. This was added to ElasticSearch to make it possible to manage the keystore as root while the service runs as an unprivileged user. Without the SGID trick, the generated keystore was owned by root and ElasticSearch could not access it. It is preferable to manage the keystore with non-root privileges, and this hack is not required in this case. Stick to the default permissions and remove this personalization. (cherry picked from commit 9d0db5e) Signed-off-by: Romain Tartière <[email protected]> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Setting the SGID bit on directories is maybe something some users will want to use, but setting it by default for all users does not really make sense and when packaging OpenSearch, we need to remove this customization when building packges. This was added to ElasticSearch to make it possible to manage the keystore as root while the service runs as an unprivileged user. Without the SGID trick, the generated keystore was owned by root and ElasticSearch could not access it. It is preferable to manage the keystore with non-root privileges, and this hack is not required in this case. Stick to the default permissions and remove this personalization. Signed-off-by: Romain Tartière <[email protected]>

Setting the SGID bit on directories is maybe something some users will want to use, but setting it by default for all users does not really make sense and when packaging OpenSearch, we need to remove this customization when building packges. This was added to ElasticSearch to make it possible to manage the keystore as root while the service runs as an unprivileged user. Without the SGID trick, the generated keystore was owned by root and ElasticSearch could not access it. It is preferable to manage the keystore with non-root privileges, and this hack is not required in this case. Stick to the default permissions and remove this personalization. Signed-off-by: Romain Tartière <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>

smortex force-pushed the drop-sgid branch from bc09ed5 to 3300b4d Compare August 19, 2023 03:57

smortex mentioned this pull request Aug 19, 2023

Defaut config permission too relaxed in deb and rpm packages opensearch-project/opensearch-build#3815

Closed

dbwiddis approved these changes Aug 19, 2023

View reviewed changes

peterzhuamazon mentioned this pull request Aug 21, 2023

Improve ownership and permissions of files in OpenSearch deb and rpm packages opensearch-project/opensearch-build#3898

Merged

reta reviewed Aug 21, 2023

View reviewed changes

distribution/packages/build.gradle Show resolved Hide resolved

smortex requested a review from reta August 23, 2023 18:09

opensearch-trigger-bot bot added the stalled Issues that have stalled label Sep 27, 2023

smortex force-pushed the drop-sgid branch from 84d5215 to 2214f89 Compare September 28, 2023 00:04

smortex requested review from peternied and msfroh as code owners September 28, 2023 00:04

smortex force-pushed the drop-sgid branch from 2214f89 to efaf798 Compare September 28, 2023 00:08

opensearch-trigger-bot bot removed the stalled Issues that have stalled label Sep 28, 2023

smortex mentioned this pull request Sep 29, 2023

[Meta] Improve Deb/Rpm OpenSearch packaging in 2.x Releases opensearch-project/opensearch-build#4087

Closed

24 tasks

reta approved these changes Oct 2, 2023

View reviewed changes

dblock approved these changes Oct 2, 2023

View reviewed changes

dblock merged commit 9d0db5e into opensearch-project:main Oct 2, 2023
14 checks passed

dblock added the backport 2.x Backport to 2.x branch label Oct 2, 2023

opensearch-trigger-bot bot mentioned this pull request Oct 2, 2023

[Backport 2.x] Remove spurious SGID bit on directories #10313

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove spurious SGID bit on directories #9447

Remove spurious SGID bit on directories #9447

smortex commented Aug 19, 2023 •

edited

Loading

github-actions bot commented Aug 19, 2023

github-actions bot commented Aug 19, 2023

dbwiddis left a comment

reta commented Aug 21, 2023

opensearch-trigger-bot bot commented Aug 21, 2023

opensearch-trigger-bot bot commented Aug 21, 2023

github-actions bot commented Aug 21, 2023

codecov bot commented Aug 21, 2023 •

edited

Loading

opensearch-trigger-bot bot commented Sep 27, 2023

smortex commented Sep 28, 2023

github-actions bot commented Sep 28, 2023 •

edited

Loading

github-actions bot commented Sep 28, 2023

github-actions bot commented Sep 28, 2023

Remove spurious SGID bit on directories #9447

Remove spurious SGID bit on directories #9447

Conversation

smortex commented Aug 19, 2023 • edited Loading

Description

Related Issues

Check List

github-actions bot commented Aug 19, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Aug 19, 2023

Gradle Check (Jenkins) Run Completed with:

dbwiddis left a comment

Choose a reason for hiding this comment

reta commented Aug 21, 2023

opensearch-trigger-bot bot commented Aug 21, 2023

Compatibility status:

Incompatible components

Skipped components

Compatible components

opensearch-trigger-bot bot commented Aug 21, 2023

Compatibility status:

Incompatible components

Skipped components

Compatible components

github-actions bot commented Aug 21, 2023

Gradle Check (Jenkins) Run Completed with:

codecov bot commented Aug 21, 2023 • edited Loading

Codecov Report

opensearch-trigger-bot bot commented Sep 27, 2023

smortex commented Sep 28, 2023

github-actions bot commented Sep 28, 2023 • edited Loading

Compatibility status:

Incompatible components

Skipped components

Compatible components

github-actions bot commented Sep 28, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Sep 28, 2023

Gradle Check (Jenkins) Run Completed with:

smortex commented Aug 19, 2023 •

edited

Loading

codecov bot commented Aug 21, 2023 •

edited

Loading

github-actions bot commented Sep 28, 2023 •

edited

Loading