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

[content-service] enable public read on gitpod repo root folder #10229

Merged
merged 1 commit into from Jun 1, 2022

Conversation

sagor999
Copy link
Contributor

@sagor999 sagor999 commented May 24, 2022

Description

This fixes permissions on gitpod repo to ensure it has public read enabled (breaks some workflows that relies on that).

Related Issue(s)

Fixes gitpod-io/workspace-images#640

How to test

Start a new workspace in current production.
do ls -la /workspace and notice that permissions on gitpod repo are 0750.

Start a workspace from preview env of this PR
do ls -la /workspace and notice that permissions on gitpod repo are now set to correct 0755.

Release Notes

Ensure correct permissions are set on gitpod repo inside the workspace.

Documentation

@sagor999 sagor999 changed the title wip [supervisor] ensure correct permissions on gitpod repo root folder May 25, 2022
@sagor999 sagor999 requested a review from a team May 25, 2022 00:21
@github-actions github-actions bot added the team: workspace Issue belongs to the Workspace team label May 25, 2022
@sagor999 sagor999 marked this pull request as ready for review May 25, 2022 00:23
@sagor999 sagor999 requested a review from a team May 25, 2022 00:23
@sagor999
Copy link
Contributor Author

/hold
holding to make sure someone from workspace team can review this changes as well, in case if there is a better way to implement this.

Copy link
Contributor

@mustard-mh mustard-mh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected.

When we checkout git repo, we correctly set permissions to 0755, but it is done in ws-daemon via git initializer.

But why not move it to where it create?

@sagor999
Copy link
Contributor Author

But why not move it to where it create?

@mustard-mh
we already create folder with 0755 permissions, but that folder is created from ws-daemon. I think when we shift permissions inside container, we somehow lose public r+x permissions on that folder. So this fix is a workaround to this.

@mustard-mh
Copy link
Contributor

if err := os.MkdirAll(ws.Location, 0770); err != nil {

err = os.MkdirAll(c.Location, 0755)

As we can see lines above, we create folder in line 73, then exec creation in line 316, so only 73 will works if it create success(c and ws is same object).

/workspace/<repo> folder should be 0770, but no idea why golang cannot set the write flag in group permission, see screen capture below.

Screen Capture
image

@mustard-mh
Copy link
Contributor

So change this to 0755 should works

if err := os.MkdirAll(ws.Location, 0770); err != nil {

@sagor999
Copy link
Contributor Author

/hold

@sagor999
Copy link
Contributor Author

@mustard-mh you are 💯 right! I thought I tried that change and for some reason it was not working, but I guess I messed something up when testing that exact fix. 🤦‍♂️
Thank you for double checking! Updated PR.

@sagor999 sagor999 changed the title [supervisor] ensure correct permissions on gitpod repo root folder [content-service] enable public read on gitpod repo root folder May 25, 2022
@sagor999
Copy link
Contributor Author

/unhold

Copy link
Contributor

@mustard-mh mustard-mh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kylos101
Copy link
Contributor

kylos101 commented May 31, 2022

/werft run

👍 started the job as gitpod-build-pavel-images-640.9
(with .werft/ from main)

@kylos101
Copy link
Contributor

@sagor999 what do you mean by breaks some workflows that relies on that? For example, will we need a communication plan for this, for workflows that we break?

@kylos101
Copy link
Contributor

kylos101 commented May 31, 2022

/werft run

👍 started the job as gitpod-build-pavel-images-640.10
(with .werft/ from main)

@kylos101
Copy link
Contributor

kylos101 commented Jun 1, 2022

/werft run

👍 started the job as gitpod-build-pavel-images-640.11
(with .werft/ from main)

@kylos101
Copy link
Contributor

kylos101 commented Jun 1, 2022

/werft run

👍 started the job as gitpod-build-pavel-images-640.12
(with .werft/ from main)

Copy link
Contributor

@kylos101 kylos101 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebased with main (preview environment would not build, was getting install config validation errors), and tested with the working preview environment, and the following worked like a champ:

touch private.env
docker-compose up db

cc: @alanwilter

Nice work, @sagor999 ! 💪

@roboquat roboquat merged commit 2605461 into main Jun 1, 2022
@roboquat roboquat deleted the pavel/images-640 branch June 1, 2022 01:42
@roboquat roboquat added the deployed: IDE IDE change is running in production label Jun 9, 2022
@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: IDE IDE change is running in production deployed: workspace Workspace team change is running in production deployed Change is completely running in production release-note size/XS team: IDE team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Project folder permissions won't let docker postgres work
4 participants