Skip to content

Fix #187: in openshift fix redis-session.ini permission denied #717

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

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

antoinetran
Copy link

@antoinetran antoinetran commented Mar 26, 2025

Description of the change

Benefits

Possible drawbacks

Applicable issues

Additional information

Checklist

@lindhe
Copy link
Contributor

lindhe commented Mar 27, 2025

I suggest changing the PR title. This has nothing to do with OpenShift.

- Fix #187: in openshift fix redis-session.ini permission denied
+ Fix redis-session.ini permission denied

And if you mention Fixes #187 in the PR body and/or the commit body (not the title), then that will link this PR to that issue.

@antoinetran
Copy link
Author

antoinetran commented Mar 27, 2025

I suggest changing the PR title. This has nothing to do with OpenShift.

- Fix #187: in openshift fix redis-session.ini permission denied
+ Fix redis-session.ini permission denied

This is an issue that happens 99% of case in OpenShift environment, because generally in any other Kubernetes deployment, we can run as root. It could happen to people who try to secure their non OpenShift Kubernetes, but otherway, for me it is very linked to OpenShift. However, I have no issue with removing openshit label in commit!

And if you mention Fixes #187 in the PR body and/or the commit body (not the title), then that will link this PR to that issue.

I don't know what is the "standard way" in nextcloud, but I looked at other commit in this repository, and it seems mostly the issue are in title. And the PR is linked in the related issue. But if the best practice in this repository is to put in commit body, ok for me!

@lindhe
Copy link
Contributor

lindhe commented Mar 27, 2025

I don't know what is the "standard way" in nextcloud, but I looked at other commit in this repository, and it seems mostly the issue are in title. And the PR is linked in the related issue. But if the best practice in this repository is to put in commit body, ok for me!

It's just how GitHub works, not particular to this repo. But Nextcloud seem to embrace it too, since they indicate it in the PR template that you missed filling in:

image

Here's docs for how Github handles keywords in PRs: https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests

@lindhe
Copy link
Contributor

lindhe commented Mar 27, 2025

This is an issue that happens 99% of case in OpenShift environment, because generally in any other Kubernetes deployment, we can run as root. It could happen to people who try to secure their non OpenShift Kubernetes, but otherway, for me it is very linked to OpenShift.

As you say; this is only an issue if one tries to run the Nextcloud deployment as non-root. So anyone that wants to run Nextcloud as non-root in Kubernetes will run into the issue, not just if they use OpenShift. Any well-configured Kubernetes environment may have PSA/PSS in place enforcing this, and any security minded individual may choose to deploy rootless anyway. So it's not at all specific to OpenShift, even if it obviously applies there too.

@antoinetran antoinetran force-pushed the feature/issue187-fixredispermission branch from 3cf2ae7 to 73a5127 Compare April 3, 2025 08:03
@antoinetran
Copy link
Author

I suggest changing the PR title. This has nothing to do with OpenShift.

- Fix #187: in openshift fix redis-session.ini permission denied
+ Fix redis-session.ini permission denied

And if you mention Fixes #187 in the PR body and/or the commit body (not the title), then that will link this PR to that issue.

done

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.

3 participants