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

BUG: permissions of falco output #67

Closed
wants to merge 1 commit into from

Conversation

DriesSchaumont
Copy link
Contributor

@DriesSchaumont DriesSchaumont commented Jun 26, 2024

Description

Falco, by default, creates the output folder in mode 700 only. See https://github.com/smithlabcode/falco/blob/3e4f0ad4bee3fa84280c179cc4320bf103c59262/src/falco.cpp#L635

This means that using this component with docker creates files as root, which are not usable by the user and causes problems when publishing. There are two solutions for this:

  • in the script, use chmod g+rx,o+rx on the output folder
  • use docker.fixOwnership = true. It think this option is preferred because it just changes the owner of the files, not the mode and the fix is only applied when using docker.

Why is this not a problem for other components? The behavior occurs when:

  1. There is an output argument of type file
  2. This output will be created by the script of the component and its a directory.
  3. The created directory missing read and execute permissions for 'others'.

Most other components create files, for which the permissions are based on the umask. The umask is by default 022, so most of the times the files are created with permissions 0644. This includes read permissions for groups and others.

Issue ticket number

Closes #xxxx

Checklist before requesting a review

  • I have performed a self-review of my code

  • Conforms to the Contributing guidelines

  • Proposed changes are described in the CHANGELOG.md

  • I have tested my code with viash ns test --parallel -q <name or namespace>

  • Check the correct box. Does this PR contain:

    • Breaking changes
    • New functionality
    • Major changes
    • Minor changes
    • Documentation
    • Bug fixes

@DriesSchaumont
Copy link
Contributor Author

Ah, the proposed solution to add fixOwnership to only this component does not work when this component is used as a dependency. The fixOwnership gets added to the nextflow.config for this component, but this config file that needs to be added manually with each run

@DriesSchaumont
Copy link
Contributor Author

Closing in favour of viash-io/viash#727

@rcannood rcannood deleted the bug/falco_permissions branch July 4, 2024 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant