canvas-rce-api
canvas-rce-api copied to clipboard
Added the RCE_API_HOST environment variable
Hi,
We run our Canvas instances on servers not directly exposed to the internet and without publicly routable addresses. They are behind a load balancer/nat appliance.
The canvas-rce-api proxy was getting 127.0.0.1 when calling request.get("host")
which meant that the flies browser was getting unusable JSON from app/api/folders.js
and unable to work.
To get around the problem, we added an environment variable RCE_API_HOST
that allows the sysadmin to force the public URL to a setting of their choosing. If the variable is not set, the previous behavior (using request.get("host")
) is preserved.
Best, Jorge
@jrherskovic-mda It seems that this patchset doesn't pass the test suite. I'm getting the following two errors:
272 passing (615ms)
2 failing
1) Link API response handlers
successful canvas responses
bookmark construction
keeps the request's protocol and host:
AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value:
assert.ok(bookmark.match("^http://rce.example"))
at Context.it (test/service/api/linksResponseHandler.test.js:103:16)
2) packageBookmark
forms corect url:
AssertionError [ERR_ASSERTION]: 'http://false/a/b/c?id=47&bookmark=canvasurl' == 'http://somehost:3000/a/b/c?id=47&bookmark=canvasurl'
+ expected - actual
-http://false/a/b/c?id=47&bookmark=canvasurl
+http://somehost:3000/a/b/c?id=47&bookmark=canvasurl
at Context.it (test/service/api/packageBookmark.test.js:27:5)
Would you take a look at those and make adjustments as needed? You should be able to run the test suite by running npm test
within the repo.
Fixed the issue. Sorry about that - node isn't my strong suit. That said, the test suite behaves differently on my machine. Could you please verify again?
@jherskovic I'm just auditing stuff that's outstanding here. There hasn't been any response since I last reviewed on June 3. Are you still interested in the getting this merged in?
@jherskovic I'm just auditing stuff that's outstanding here. There hasn't been any response since I last reviewed on June 3. Are you still interested in the getting this merged in?
Yes, definitely. Sorry - I didn't realize that your comments on the test were meant for me. I can certainly attempt to fix the test, if that's what you need.
Ended up ignoring casing of the Host variable - please LMK if this is acceptable.
Hi, as @claydiffrient said in https://github.com/instructure/canvas-rce-api/pull/5#issuecomment-531869048, is this change going to be applied?, we're working Apache with a proxy to serve the RCE API and the problem is the same, so we need to be able to pass the RCE_API_HOST in the .env
This PR is great. I tested in out production environment. I suggest this PR be accepted except "package-lock.json"