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

helm chart minio bug + extension upload bug #2067

Closed
hm2075 opened this issue Oct 26, 2020 · 20 comments · Fixed by #2072
Closed

helm chart minio bug + extension upload bug #2067

hm2075 opened this issue Oct 26, 2020 · 20 comments · Fixed by #2072
Labels
self-hosted type: bug Something isn't working

Comments

@hm2075
Copy link

hm2075 commented Oct 26, 2020

Describe the bug

when a user sets up their own instance of minio, gitpod server deployment continues to use hard coded values

update chart/templates/server-deployment.yaml

{{- if eq .Values.components.wsDaemon.remoteStorage.kind "minio" }}
        - name: GITPOD_STORAGE_CLIENT
          value: minio
        - name: MINIO_END_POINT
          value: minio.{{ .Release.Namespace }}.svc.cluster.local
        - name: MINIO_PORT
          value: "9000"
        - name: MINIO_ACCESS_KEY
          value: {{ .Values.minio.accessKey }}
        - name: MINIO_SECRET_KEY
          value: {{ .Values.minio.secretKey }}
        - name: MINIO_REGION
          value: {{ .Values.minio.region }}
    {{- end }}

replacing these hard codes values with those from values.yaml

with a linked issue on uploading extensions, users using minio pod or external minio are unable to upload extensions

@corneliusludmann
Copy link
Contributor

Thanks for investigating this, @hm2075! 👍

gitpod/chart/values.yaml

Lines 381 to 388 in 92c9494

remoteStorage:
kind: minio
minio:
endpoint: minio:9000
accessKey: EXAMPLEvalue
secretKey: Someone.Should/ReallyChangeThisKey!!
tmpdir: /tmp
region: local

@hm2075
Copy link
Author

hm2075 commented Oct 26, 2020

we will get there, i think there is a knock on effect with creating / saving plugins

SignatureDoesNotMatch
The request signature we calculated does not match the signature you provided. Check your key and signing method.

@arleif-dfactory
Copy link

@hm2075 yes, those two are indeed the root causes. Seems like some special characters are double encoded while generating the signed URL.

I was able to work around this by writing a little service to create the signature correctly, and then calling it from the nginx lua script, right between the call to the server and minio. Was just too hackish to contribute the fix into the GitPod repo 😄

@hm2075
Copy link
Author

hm2075 commented Oct 27, 2020

Yes, I noticed that I had to decode the url

@corneliusludmann the first fix is easy but I couldn't see how the url is generated for the second issue

@hm2075
Copy link
Author

hm2075 commented Oct 27, 2020

ran a complete trace on this from mc admin tools

08:32:28.290 [200 OK] s3.HeadBucket nexusdevops.com:9000/gitpod-nexusdevops--com-plugins 10.244.0.203 136µs ↑ 88 B ↓ 218 B 08:32:28.330 [400 Bad Request] s3.PutObject nexusdevops.com:9000/gitpod-nexusdevops--com-plugins/cc325b04-e306-451f-9c88-f87538e4715d.plugin.f6f5690a-84d0-49f2-ab8d-ba2b5b64a863.dbaeumer.vscode-eslint~2.1.8?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=minio%252F20201027%252Fus-east-1%252Fs3%252Faws4_request&X-Amz-Date=20201027T083228Z&X-Amz-Expires=604800&X-Amz-SignedHeaders=host&X-Amz-Signature=e50271c7d1ba68c23e8b6124c9d60529d48952312882b55bb845f357d95d6241 10.244.0.207 287µs ↑ 44 B ↓ 815 B

this is what is causing the failure with uploading extensions

Credential=minio%252F20201027%252Fus-east-1%252Fs3%252Faws4_request&

f

@jgallucci32 download https://docs.min.io/docs/minio-client-quickstart-guide.html minio client, windows binaries available
can see exactly what is happening here

hunting down the code on gitpod that is generating the url

as @arleif-dfactory said, double encoded credential ?
I would guess

` if ($action = preflight) {
set $targetUrl "no-url";
access_by_lua_block {
url = "/plugins-preflight?type=" .. ngx.var.type .. "&" .. ngx.var.api_key_encoded .. "&" .. ngx.var.qs;
response = ngx.location.capture(url);
if response.status == ngx.HTTP_OK then
ngx.var.targetUrl = response.body;
else
ngx.log(ngx.ERR, "Bad Request: /plugins/preflight returned with code " .. response.status)
return ngx.exit(400)
end
}

log_by_lua_block {
    ngx.log(ngx.ERR, " proxy_pass to " .. ngx.var.targetUrl)
}

proxy_pass $targetUrl;

}`

@hm2075 hm2075 changed the title helm chart minio bug helm chart minio bug + extension upload bug Oct 27, 2020
@hm2075
Copy link
Author

hm2075 commented Oct 27, 2020

C:\Development>mc admin trace -v local
nexusdevops.com [REQUEST s3.HeadBucket] 12:12:59.603
nexusdevops.com HEAD /gitpod-nexusdevops--com-plugins
nexusdevops.com Proto: HTTP/1.1
nexusdevops.com Host: nexusdevops.com:9000
nexusdevops.com Authorization: AWS4-HMAC-SHA256 Credential=minio/20201027/us-east-1/s3/aws4_request, SignedHeaders=host;x-amz-content-sha256;x-amz-date, Signature=7e1dab3cc7714f70e011720b945671fc7a360e7801d83c3e9662dc3b0391c38c
nexusdevops.com Connection: close
nexusdevops.com Content-Length: 0
nexusdevops.com User-Agent: MinIO (linux; x64) minio-js/7.0.12
nexusdevops.com X-Amz-Content-Sha256: e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
nexusdevops.com X-Amz-Date: 20201027T121259Z
nexusdevops.com
nexusdevops.com [RESPONSE] [12:12:59.603] [ Duration 208µs ↑ 88 B ↓ 218 B ]
nexusdevops.com 200 OK
nexusdevops.com Accept-Ranges: bytes
nexusdevops.com Content-Length: 0
nexusdevops.com Content-Security-Policy: block-all-mixed-content
nexusdevops.com Server: MinIO/RELEASE.2020-09-26T03-44-56Z
nexusdevops.com Vary: Origin
nexusdevops.com X-Amz-Request-Id: 1641D84FCEC0D80A
nexusdevops.com X-Xss-Protection: 1; mode=block
nexusdevops.com
nexusdevops.com
nexusdevops.com [REQUEST s3.PutObject] 12:12:59.607
nexusdevops.com PUT /gitpod-nexusdevops--com-plugins/cc325b04-e306-451f-9c88-f87538e4715d.plugin.bab6f0d9-d6a4-4f24-8820-d8157fe52c5d.dbaeumer.vscode-eslint~2.1.8?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=minio%252F20201027%252Fus-east-1%252Fs3%252Faws4_request&X-Amz-Date=20201027T121259Z&X-Amz-Expires=604800&X-Amz-SignedHeaders=host&X-Amz-Signature=61db87baad4d957bd1ad5bad853ab8239c6595d734704a04f6405e52f53e8fa7
nexusdevops.com Proto: HTTP/1.0
nexusdevops.com Host: nexusdevops.com:9000
nexusdevops.com Connection: close
nexusdevops.com Content-Length: 137252
nexusdevops.com Content-Type: /
nexusdevops.com
nexusdevops.com [RESPONSE] [12:12:59.608] [ Duration 188µs ↑ 44 B ↓ 815 B ]
nexusdevops.com 400 Bad Request

@hm2075
Copy link
Author

hm2075 commented Oct 27, 2020

the get request is fine,

and then on the put request we see the credentials are encoded and signature is probably wrong too

@jgallucci32
Copy link
Contributor

@hm2075 I was able to get a valid request on the PUT by replacing the %252F characters with / in the X-Amz-Credential URI Query Parameter. It seems the request is not properly encoding the URL. I used cURL to test and verify.

@hm2075
Copy link
Author

hm2075 commented Oct 27, 2020

can we log this

ngx.var.targetUrl = response.body;

ngx.var.targetUrl = response.body;

and see what the response.body is?

looks to me it is doing a preflight check without issue, then using the response body for the put request
might just be a simple decode on that response.body

@jgallucci32
Copy link
Contributor

I'll see if I can get that to log.

Side note, I also replaced %252F with %2F which is the actual encoded character for a / and the request works as well. Something is using the wrong character replacement for the / in the request.

@arleif-dfactory
Copy link

@jgallucci32 @hm2075
I logged it while debugging this and you will find it look something like this
http://minio.default.svc.cluster.local:9000/gitpod-xxx-plugins/4fb826c7-dcba-4827-a6bb-be93af4895cc.plugin.0e311ac5-fca1-438e-9f87-5c3bc1314501.eamodio.gitlens~10.2.1?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=xxxxxxxxxxxx%252F20201027%252Fus-east-1%252Fs3%252Faws4_request&X-Amz-Date=20201027T125215Z&X-Amz-Expires=604800&X-Amz-SignedHeaders=host&X-Amz-Signature=xxxxxxxxxxxxxxxxxxx

@jgallucci32
Copy link
Contributor

Thanks @arleif-dfactory I confirm that is exactly the same request I see in wireshark capture of the request.

It looks like the character %252F is a double encoded slash while %2F is a single encoded slash. This seems to be a common problem while using a proxy so I am going through the online articles now to see how to fix this.

@jgallucci32
Copy link
Contributor

From this page https://www.npmjs.com/package/aws4 I noticed the following

headers['Content-Type'] (will use 'application/x-www-form-urlencoded; charset=utf-8' if not given and there is a body

Looking at gitpod-plugin-deployer.js it appears this is being changed for fix an issue with gcloud-storage

568                                    // Content-Type has to match exactly with the `getSignedUrl` request from gcloud-storage-client.ts
569                                    // otherwise the upload will fail with a "signature does not match" error.
570                                    'Content-Type': '*/*',
571                                    'Content-Length': stat.size

So it is quite possible the changing of the Content-Type and removing the application/x-www-form-urlencoded is what is breaking this.

@hm2075
Copy link
Author

hm2075 commented Oct 27, 2020

Explains why cloud gcp works but selfhosted doesnt

What would happen if we patch the response.body and do a replace in the conf file ?

@hm2075
Copy link
Author

hm2075 commented Oct 27, 2020

here's my hack job

image

edit chart\config\proxy\lib.gitpod-plugins.conf

response.body = response.body:gsub("%%252F", "%%2F");

needs a double % to escape otherwise lua thinks we are looking at a regex

@corneliusludmann

I dont think this is a valid fix, more like a patch job on the helm templates
what do you think

@jgallucci32
Copy link
Contributor

jgallucci32 commented Oct 27, 2020

@hm2075 You were on the right track. It looks like the correct function is ngx.unescape_uri from the Lua Documentation which will correctly unescape the %252F characters to %2F. Because the response body from the first request is being used to generate a new URL escaping the encoding here makes most sense.

I created #2072 which escapes the URI using the nginx function.

@hm2075
Copy link
Author

hm2075 commented Oct 27, 2020

Excellent ,

We still need to fix the first issue though , whereby server deployment has hard coded minio values.

@jgallucci32
Copy link
Contributor

@hm2075 I'm unable to reproduce the initial issue. I deployed Gitpod v0.5.0 self-hosted into a native-Kubernetes deployment using Rancher and it worked with both the default Minio values as well as overwritten values for s3gateway functionality.

I do think I see what the issue might be. When I configured Minio to be an s3gateway I had to set the minio values in two places. It has to be defined at the high-level as well as in the wsSync: component. So you may need to configure values.yaml to be something like this:

  components:
    wsSync:
      remoteStorage:
        minio:
          accessKey: "XXXXXXXXXXXXXXXXXXX"
          secretKey: "yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy"
  minio:
    accessKey: "XXXXXXXXXXXXXXXXXXXX"
    secretKey: "yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy"

NOTE: X and y must match exactly in the two sections. I think this is the hard-coded part which needs to be fixed as it should only need to be referenced once.

@corneliusludmann
Copy link
Contributor

Thank you very much for investigating this, @jgallucci32 @hm2075 @arleif-dfactory! Awesome contribution!

And thanks for the PR #2072, @jgallucci32. I'll take care that this one will be reviewed soon!

I'll also have a look at the initial issue of whether we need to fix the server-deployment.yaml template for external minio instances.

@jimmybrancaccio
Copy link

Glad to see this is getting worked on 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
self-hosted type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants