gitea/helm-gitea
Fork 169
Code Issues 54 Pull Requests 11 Actions Releases 103 Activity

Fix regression for creating repositories in root-based containers #172

Merged
luhahn merged 2 commits from justusbunsi/helm-chart:issue/171-repo-api into master 2021-06-09 14:35:51 +00:00
Conversation 9 Commits 2 Files Changed 3 +10 -1
justusbunsi commented 2021-06-09 06:17:45 +00:00
Contributor
Copy Link

Due to #160 it was no longer possible to create repositories in root-based containers. This was caused by the missing /tmp/gitea directory in that image. It was dynamically created by Gitea internal functionality with less privileges than necessary.

Explicitly creating the directory and set proper permissions fix this.

Fixes: #171

Due to #160 it was no longer possible to create repositories in root-based containers. This was caused by the missing `/tmp/gitea` directory in that image. It was dynamically created by Gitea internal functionality with less privileges than necessary. Explicitly creating the directory and set proper permissions fix this. Fixes: #171
justusbunsi commented 2021-06-09 06:19:13 +00:00
Author
Contributor
Copy Link

@luhahn Does the increase of Chart.yaml app version does the trick to indicate the correct version for the chart? As mentioned in issue #171, this was incorrect due to the previous PR #160.

@luhahn Does the increase of Chart.yaml app version does the trick to indicate the correct version for the chart? As mentioned in issue #171, this was incorrect due to the previous PR #160.
luhahn commented 2021-06-09 07:23:22 +00:00
Member
Copy Link

@luhahn Does the increase of Chart.yaml app version does the trick to indicate the correct version for the chart? As mentioned in issue #171, this was incorrect due to the previous PR #160.

Yes, missed it in your PR, sorry :D.

> @luhahn Does the increase of Chart.yaml app version does the trick to indicate the correct version for the chart? As mentioned in issue #171, this was incorrect due to the previous PR #160. Yes, missed it in your PR, sorry :D.
luhahn requested changes 2021-06-09 07:34:52 +00:00
templates/gitea/init.yaml Outdated
@@ -24,0 +25,4 @@
mkdir -p "${GITEA_TEMP}"
chown 1000:1000 "${GITEA_TEMP}"
chmod ug+rwx "${GITEA_TEMP}"
luhahn commented 2021-06-09 07:34:46 +00:00
Member
Copy Link

Since we need this only on the root image, should we rebase your branch on my
Pr gitea/helm-chart#165 so we can include your checks into the rootless if condition?

Something like this:

{{- if not .Values.image.rootless }}  chown 1000:1000 /data  mkdir -p "${GITEA_TEMP}"  chown 1000:1000 "${GITEA_TEMP}"  chmod ug+rwx "${GITEA_TEMP}" {{- end }}
Since we need this only on the root image, should we rebase your branch on my Pr https://gitea.com/gitea/helm-chart/pulls/165 so we can include your checks into the rootless if condition? Something like this: ```yaml {{- if not .Values.image.rootless }} chown 1000:1000 /data mkdir -p "${GITEA_TEMP}" chown 1000:1000 "${GITEA_TEMP}" chmod ug+rwx "${GITEA_TEMP}" {{- end }} ```
justusbunsi commented 2021-06-09 09:15:29 +00:00
Author
Contributor
Copy Link

It wouldn't cause any issues when running for both image variants. In the root image this is necessary. For the rootless image this would fix permissions in case they are too restrictive. At least that's what I've tested so far.

Actually, I don't think that the chown 1000:1000 /data is necessary since the fsGroup already declares the group as 1000. It should work without the chown-in.

I leave it up to you to decide whether I should rebase or not. ?

Will update the readme.

It wouldn't cause any issues when running for both image variants. In the root image this is necessary. For the rootless image this would fix permissions in case they are too restrictive. At least that's what I've tested so far. Actually, I don't think that the `chown 1000:1000 /data` is necessary since the `fsGroup` already declares the group as 1000. It _should_ work without the chown-in. I leave it up to you to decide whether I should rebase or not. ? Will update the readme.
luhahn commented 2021-06-09 11:52:44 +00:00
Member
Copy Link

Yea, I agree, it SHOULD :D. But it doesn't we had an issue (gitea/helm-chart#135) and i could only resolve it by applying the chown.

However, i just tested your changes and yes, the chown doesn't cause any error messages :)

You'll still need to update your branch with the latest master, so we can merge

Yea, I agree, it SHOULD :D. But it doesn't we had an issue (https://gitea.com/gitea/helm-chart/issues/135) and i could only resolve it by applying the chown. However, i just tested your changes and yes, the chown doesn't cause any error messages :) You'll still need to update your branch with the latest master, so we can merge
6543 marked this conversation as resolved
luhahn commented 2021-06-09 07:39:57 +00:00
Member
Copy Link

Please also add the image version into the readmes value description

Please also add the image version into the readmes value description
justusbunsi commented 2021-06-09 10:14:34 +00:00
Author
Contributor
Copy Link

Please also add the image version into the readmes value description

Seems to be already done in 031b58c90e.

> Please also add the image version into the readmes value description Seems to be already done in 031b58c90ea3b18eb8964fab5b1f214e78fca79b.
luhahn commented 2021-06-09 11:47:11 +00:00
Member
Copy Link

Please also add the image version into the readmes value description

Seems to be already done in 031b58c90e.

right, missed that one, sorry

> > Please also add the image version into the readmes value description > > Seems to be already done in 031b58c90ea3b18eb8964fab5b1f214e78fca79b. right, missed that one, sorry
luhahn approved these changes 2021-06-09 11:53:06 +00:00
justusbunsi force-pushed issue/171-repo-api from 109b89ec5f to 9a7f67b406 2021-06-09 12:14:56 +00:00 Compare
justusbunsi commented 2021-06-09 12:18:19 +00:00
Author
Contributor
Copy Link

Branch up-to-date. Hopefully these changes do not introduce other side-effects. ?

Branch up-to-date. Hopefully these changes do not introduce other side-effects. ?
6543 approved these changes 2021-06-09 13:39:02 +00:00
luhahn merged commit 6e841e6e26 into master 2021-06-09 14:35:51 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
luhahn
6543
Labels
Clear labels
has
backport
in progress
invalid
kind
breaking
kind
bug
kind
build
kind
dependency
kind
deployment
kind
docs
kind
enhancement
kind
feature
kind
lint
kind
proposal
kind
question
kind
refactor
kind
security
kind
testing
kind
translation
kind
ui
need
backport
priority
critical
priority
low
priority
maybe
priority
medium
reviewed
duplicate
reviewed
invalid
reviewed
wontfix
skip-changelog
status
blocked
status
needs-feedback
status
needs-reviews
status
wip
upstream
gitea
upstream
other
workaround
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
42wim 6543 a1012112796 Akkuman appleboy axifive betacat0 blumia bwko caicandong ChristopherHX DaanSelen davidsvantesson delvh denyskon fuxiaohei gary-kim garymoon giteabot guillep2k HesterG hiifong jaqra jolheiser jonasfranz jpraet kdumontnu kemzeb kerwin612 khmarbaise KN4CK3R kolaente lafriks lng2020 lotherk luhahn lunny mrsdizzie noerw pat-s puni9869 renovate-bot richmahn rossigee sapk screenshots sillyguodong silverwind smartclip_tim strk tboerger techknowlogick TheFox0x7 tobiasbp typeless volker.raschek wolfogre wxiaoguang yardenshoham yp05327 zeripath Zettat123
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: gitea/helm-gitea#172
No description provided.
Delete Branch "justusbunsi/helm-chart:issue/171-repo-api"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?