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

Rules are not applied to implicit created directories #85

Closed
moedan opened this issue Nov 29, 2023 · 9 comments
Closed

Rules are not applied to implicit created directories #85

moedan opened this issue Nov 29, 2023 · 9 comments
Labels

Comments

@moedan
Copy link
Contributor

moedan commented Nov 29, 2023

Hello,

I have noticed that rules are not applied to implicitly created directories. For example, when a file is created in a target directory that does not yet exist, no rules are applied to these directories. For explicitly created directories, the rules are applied (see example below).

I am not sure if this is a bug or the expected behavior. I would have expected that the rules are applied to all applicable intermediate directories in a path as well.

Can you please clarify that? Thanks in advance!


Example

To reproduce the described problem, I have attached a simplified MWE (Used rpm-bilder-Plugin 1.11.0): mwe.zip

Ruleset:

<ruleset>
    <id>my-default</id>
    <rules>
        <rule>
            <when>
                <prefix>/opt/mycompany</prefix>
            </when>
            <user>myuser</user>
            <group>mygroup</group>
        </rule>
        <rule>
            <when>
                <prefix>/opt/mycompany</prefix>
                <type>directory</type>
            </when>
            <mode>0777</mode>
        </rule>
    </rules>
</ruleset>

Entries:

<entries>
    <entry>
        <!-- implicitly create directories "mycompany/mwe/a/b/c/" in "/opt" -->
        <name>/opt/mycompany/mwe/a/b/c/someFile.txt</name>
        <file>${project.basedir}/src/main/resources/someFile.txt</file>
        <ruleset>my-default</ruleset>
    </entry>
    <entry>
        <!-- explicitly create directory "mwe/" in "/opt/mycompany" -->
        <name>/opt/mycompany/mwe</name>
        <directory>true</directory>
        <ruleset>my-default</ruleset>
    </entry>
</entries>

Output of created files/directroies with permissions after installation of built rpm from MWE:

[/]$ ls -alpR /opt
/opt:
total 20
drwxr-xr-x. 1 root root 4096 Nov 29 10:28 ./
dr-xr-xr-x. 1 root root 4096 Nov 29 10:28 ../
drwxr-xr-x. 3 root root 4096 Nov 29 10:28 mycompany/

/opt/mycompany:
total 16
drwxr-xr-x. 3 root   root    4096 Nov 29 10:28 ./
drwxr-xr-x. 1 root   root    4096 Nov 29 10:28 ../
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:14 mwe/

/opt/mycompany/mwe:
total 12
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:14 ./
drwxr-xr-x. 3 root   root    4096 Nov 29 10:28 ../
drwxr-xr-x. 3 root   root    4096 Nov 29 10:28 a/

/opt/mycompany/mwe/a:
total 12
drwxr-xr-x. 3 root   root    4096 Nov 29 10:28 ./
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:14 ../
drwxr-xr-x. 3 root   root    4096 Nov 29 10:28 b/

/opt/mycompany/mwe/a/b:
total 12
drwxr-xr-x. 3 root root 4096 Nov 29 10:28 ./
drwxr-xr-x. 3 root root 4096 Nov 29 10:28 ../
drwxr-xr-x. 2 root root 4096 Nov 29 10:28 c/

/opt/mycompany/mwe/a/b/c:
total 12
drwxr-xr-x. 2 root   root    4096 Nov 29 10:28 ./
drwxr-xr-x. 3 root   root    4096 Nov 29 10:28 ../
-rw-r--r--. 1 myuser mygroup   11 Nov 29 10:14 someFile.txt

Expected output of created files/directroies:

[/]$ ls -alpR /opt
/opt:
total 20
drwxr-xr-x. 1 root   root    4096 Nov 29 10:28 ./
dr-xr-xr-x. 1 root   root    4096 Nov 29 10:28 ../
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 mycompany/

/opt/mycompany:
total 16
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 ./
drwxr-xr-x. 1 root   root    4096 Nov 29 10:28 ../
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:14 mwe/

/opt/mycompany/mwe:
total 12
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:14 ./
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 ../
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 a/

/opt/mycompany/mwe/a:
total 12
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 ./
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:14 ../
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 b/

/opt/mycompany/mwe/a/b:
total 12
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 ./
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 ../
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 c/

/opt/mycompany/mwe/a/b/c:
total 12
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 ./
drwxrwxrwx. 3 myuser mygroup 4096 Nov 29 10:28 ../
-rw-r--r--. 1 myuser mygroup   11 Nov 29 10:14 someFile.txt
@ctron
Copy link
Owner

ctron commented Nov 29, 2023

I am not sure if this is a bug or the expected behavior. I would have expected that the rules are applied to all applicable intermediate directories in a path as well.

Indeed, that feels like a bug.

@moedan
Copy link
Contributor Author

moedan commented Dec 13, 2023

Thank you for your feedback!

I thought about the problem and played around a bit.

I have no experience in packaging RPMs, but from my point of view, the following points should be considered for fixing the error:

  • Intermediate directories may only be added once all entries have been processed.
  • Only the rules specified in the entry may be applied to the intermediate directories, but not the explicit information of the entry.
  • The order of the entries must be taken into account. E.g.: An intermediate directory from a second entry may not overwrite the intermediate directory from the first entry.
  • Intermediate directories that are part of the defined prefixes should not be added.

Taking these aspects into account, I created a pull request as a first draft. This could be a starting point for a potential fix of the problem. I have not adapted the integration tests. Currently, some of them fail because the implicit intermediate folders are now part of the output and the hash sums are no longer correct.

At the moment, my bugfix would be a breaking change (Prefixes should be configured correctly and not ignored). If a breaking change is not an option, perhaps an extra flag should be added for the feature of generating intermediate directories automatically.

Sample-Output of rpm generated by mwe:

$ rpm -q --dump -p mwe.rpm
/opt/mycompany/mwe 0 1702461307 0000000000000000000000000000000000000000000000000000000000000000 040777 myuser mygroup 0 0 0 X
/opt/mycompany/mwe/a 0 1702461307 0000000000000000000000000000000000000000000000000000000000000000 040777 myuser mygroup 0 0 0 X
/opt/mycompany/mwe/a/b 0 1702461307 0000000000000000000000000000000000000000000000000000000000000000 040777 myuser mygroup 0 0 0 X
/opt/mycompany/mwe/a/b/c 0 1702461307 0000000000000000000000000000000000000000000000000000000000000000 040777 myuser mygroup 0 0 0 X
/opt/mycompany/mwe/a/b/c/someFile.txt 11 1702461307 6f6b40bbcd7bf26ef883167642d8117b58973c145da7d7818173a282eda0903c 0100644 myuser mygroup 0 0 0 X

@ctron
Copy link
Owner

ctron commented Dec 13, 2023

That looks awesome. I think those four assumptions are correct. Especially the last one, directories like /usr should not end up on the RPM. Maybe directories should only be processed starting with point where the scanner starts.

Changes of hashes are expected when the tool changes. And a breaking change would also work for me. But as always when adding some new feature, it might good to have a way to disable it.

@moedan
Copy link
Contributor Author

moedan commented Dec 13, 2023

At the moment I (mis)use the prefixes property to exclude directories like /usr. If this has unwanted side effects, we'll have to think of a new property.

In the current version I've added the new configuration property 'generateIntermediateDirectories' to disable the feature. At default, the property is enabled.
I would be happy to receive suggestions for improving the naming and description of this property. The same applies to the source code.

@moedan
Copy link
Contributor Author

moedan commented Jan 10, 2024

Are there any news on this topic?
Have you had time to take a closer look at my bugfix?

@RolandRodenbeck
Copy link

Hello, I also look for the changes to this maven plugin. So I am also interested in this issue and pull request. What is the actual status, what is missing to get this feature merged?

@ctron
Copy link
Owner

ctron commented Feb 21, 2024

Sorry I lost track of the PR. Thanks @RolandRodenbeck for pinging!

@moedan
Copy link
Contributor Author

moedan commented Mar 6, 2024

Many thanks for the quick release!

Unfortunately, I encountered an error when trying to install the generated RPM package using the new feature.

$ dnf install example.rpm
...
error: unpacking of archive failed on file /etc/mycompany/myapp: cpio: mkdir
error: example.noarch: install failed

It looks like the order of unpacking files with cpio depends on the inode index. The inode index is set and incremented when adding entries to the RpmBuilder. Regretfully, I did not take this into account when adding the intermediate folders. In the current implementation I'm adding the missing directories after all entries have been processed. This then leads to the error mentioned above. :-/

The output of the predefined rpm query commands like rpm -q --dump example.rpm was misleading here, as it is sorted automatically by path(?).

The output of the command rpm -q --queryformat "[%2{FILEINODES} %{FILENAMES}\n]" example.rpm | sort -n displays the ordering correctly.

I'll think about the problem again and hope that I can come up with a better solution.

@moedan
Copy link
Contributor Author

moedan commented Aug 12, 2024

I think the ticket can be closed as the main problem has been fixed.

Unfortunately, I have not yet found time for the improvement mentioned in the PR to make the build process fail if an invalid file structure is used (wrong ordering of added files / directories). I haven't forgotten about it, and it's still on my list.

@moedan moedan closed this as completed Aug 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants