Repository navigation
pkg_rpm: add rpm_own_tree_dirs attribute to own directories in TreeArtifacts - #1089
Open
chouquette wants to merge 1 commit into
Open
chouquette wants to merge 1 commit into
chouquette wants to merge 1 commit into
Conversation
gh-worker-dd-mergequeue-cf854d Bot
pushed a commit
to DataDog/datadog-agent
that referenced
this pull request
Oct 22, 2026
…8466) <!--Please give us some feedback on your experience writing this PR ! https://app.datadoghq.com/forms/43db4c02-6837-400c-8083-692e141b1b88 !--> ### What does this PR do? Fix a gap in the RPM packages that was leaving some directories as not owned by our bazel generated packages. There are 2 paths that we need to cover. * All directories that are automatically collected through `dd_cc_packaged` and installed in `//:install_dir` should be owned by our RPM package. This is implemented by a new rule, `dd_pkg_owned_dirs`, which is called from a new wrapper around `pkg_rpm`: `dd_pkg_rpm`. * Tree artifacts collected by `package_license` are marked as owned. This is done by: * A patch to rules_pkg (sent upstream as bazelbuild/rules_pkg#1089, which we use directly from this PR for now) * A small modification to the rule invocation on our side so that the attribute is passed for anything related to the collected licenses. Debian doesn't have the same concept of directory ownership, so debian packages aren't touched by this PR. ### Motivation Since these folders are not owned, they aren't removed on uninstall, which causes some E2E tests to fail: ``` Messages: should not find anything in install folder, found 14 dir entries. Content: /opt/datadog-agent/, /opt/datadog-agent/LICENSES, /opt/datadog-agent/bin, /opt/datadog-agent/bin/agent, /opt/datadog-agent/bin/agent/dist, /opt/datadog-agent/bin/agent/dist/views, /opt/datadog-agent/bin/agent/dist/views/css, /opt/datadog-agent/bin/agent/dist/views/fonts, /opt/datadog-agent/bin/agent/dist/views/images, /opt/datadog-agent/bin/agent/dist/views/js, /opt/datadog-agent/embedded, /opt/datadog-agent/sources, /opt/datadog-agent/sources/systemd ``` With these changes, the directories are correctly owned and therefore removed upon uninstall. ### Describe how you validated your changes Tested in the WIP PR to create the iot-agent packages with bazel, the E2E install tests now pass: https://gitlab.ddbuild.io/DataDog/datadog-agent/-/jobs/2155467243 ### Additional Notes Co-authored-by: hugo.beauzee <hugo.beauzee@datadoghq.com>
Collaborator
|
The rpm_own_tree_dirs=True behavior is obviously the correct behavior. But it would have consequences for people Since the next release has to be version 2.x anyway, why don't we make the default behavior the True case. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a TreeArtifact is packaged with
pkg_rpm, its files are added to the%fileslist but its directories are not. The package doesn't own them, so they are left behind on uninstall.Owning those directories unconditionally isn't safe, since a TreeArtifact may be installed into a directory shared with other packages (e.g.
/usr/lib). Owning such a directory would let the package override its mode and ownership.This PR adds an opt-in
rpm_own_tree_dirsattribute, set throughpkg_attributeson the mapping rule. When it's set,pkg_rpmalso adds a%direntry for the TreeArtifact destination and every directory below it. The default behavior is unchanged.