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

Implement runWithUser/fsGroup within entrypoint #21

Merged
merged 3 commits into from
May 16, 2022

Conversation

raxod502-plaid
Copy link
Contributor

@raxod502-plaid raxod502-plaid commented Dec 21, 2020

Addresses #14
See also: runatlantis/atlantis#1326

@lkysow
Copy link
Member

lkysow commented Jan 13, 2021

Hi Radon, won't removing this cause issues for folks though? There's a reason it was added IIRC.

@raxod502-plaid
Copy link
Contributor Author

If I understand correctly, runatlantis/atlantis#342 is the reason why it was added, and I am fixing that issue in a different way at runatlantis/atlantis#1326.

@lkysow
Copy link
Member

lkysow commented Jan 18, 2021

Ahh sorry I didn't see it was coupled with an Atlantis core PR.

spirosoik pushed a commit to spirosoik/helm-charts that referenced this pull request Aug 3, 2021
* [stable/prometheus-aws-health-exporter] add chart

* add readme

* add readme
@ShpilerX
Copy link

ShpilerX commented Apr 9, 2022

Hello @lkysow, would that be possible to revive this effort? That would really help our 25-minute restarts 🎉
Thanks in advance!

@jamengual
Copy link
Contributor

@raxod502-plaid can you resolve the conflicts?

@raxod502-plaid
Copy link
Contributor Author

Resolved.

@jamengual jamengual merged commit a107789 into runatlantis:main May 16, 2022
@nokernel
Copy link

nokernel commented May 26, 2022

I believe that this change is causing issues, atlantis is not able to start for me using the chart anymore.

I had to revert this change so that atlantis starts, or it crashloop with permission denied when doing mkdir in /atlantis-data.

Using

chart atlantis-4.0.1 app version v0.19.3

@raxod502-plaid raxod502-plaid deleted the you-just-got-chowned branch May 26, 2022 15:09
@raxod502-plaid
Copy link
Contributor Author

Are you using a custom securityContext?

@nokernel
Copy link

Are you using a custom securityContext?

I used the default values.yaml which I believe set an empty securityContext.

Are you suggesting that I remove that empty securityContext from the values.yaml?

@raxod502-plaid
Copy link
Contributor Author

Nah, was just trying to brainstorm why this might fail. Unfortunately I no longer have a setup on hand to test this, but you're not the only person who's reported a problem when setting up a new volume, so looks like there's something further that needs to be changed in the script.

@luozhaoyu
Copy link

luozhaoyu commented Jun 9, 2022

Hi @raxod502-plaid I met the same issue as above with chart 4.0.3 and atlantis v0.19.5:

docker-entrypoint.sh: detected /atlantis-data wrong filesystem permissions
currently owned by root:atlantis, changing to atlantis:atlantis...
chown: /atlantis-data/lost+found: Operation not permitted
chown: /atlantis-data/lost+found: Operation not permitted
chown: /atlantis-data: Operation not permitted
chown: /atlantis-data: Operation not permitted

I did not set securityContext and my values.yaml is bare minimum: I am starting from beginning, I only configured the gitconfig / ingress etc

From Dockerfile, I did not see it change user, so it should be root by default: https://github.com/runatlantis/atlantis/blob/v0.19.4/docker-entrypoint.sh#L6-L12

I also did not see any issue with the k8s manifest:

    volumeMounts:
    - mountPath: /atlantis-data
      name: atlantis-data

So I don't know why root could not do the chown command above?

However, I succeed with chart 4.0.3 and atlantis v0.19.3

@raxod502-plaid
Copy link
Contributor Author

That's super weird, on the deployed pod manifest can you see any security context getting applied by default somehow?

@luozhaoyu
Copy link

luozhaoyu commented Jun 9, 2022

@raxod502-plaid oh I found the issue! I am doing local development, so I did not remove the old version 3 Chart!

So after using chart 4.0.3, it succeed with v0.19.5 !! Sorry to bring up the confusion!

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.

6 participants