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

[WIP] Move home directory check to PUN #7

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
1 participant
@ericfranz
Copy link
Contributor

ericfranz commented Nov 1, 2018

Now if the user's home directory does not exist, an error page
will be displayed for any request to the dashboard app

this error page will provide directions to attempt to ssh to the default
ssh host configured for the shell app in order to auto-create this directory

it will also provide a link to restart the app

See https://discourse.osc.edu/t/launching-ondemand-when-home-directory-does-not-exist/53 for screenshots

move home directory check to pun
now if the user's home directory does not exist, an error page
will be displayed for any request to the dashboard app

this error page will provide directions to attempt to ssh to the default
ssh host configured for the shell app in order to auto-create this directory

it will also provide a link to restart the app

@ericfranz ericfranz changed the title Move home directory check to PUN [WIP] Move home directory check to PUN Nov 1, 2018

@ericfranz

This comment has been minimized.

Copy link
Contributor Author

ericfranz commented Nov 1, 2018

So this doesn't provide the ability to customize the template. The problem is that the error page is mostly relevant only for sites that generate home directories using pam_mkhomedir.so.

A quick hack to do that is in this PR: #8

Another solution would be to have a simple Passenger app (that is not in Ruby) that displays the desired look and feel and then there are lots of options for configuration.

@ericfranz

This comment has been minimized.

Copy link
Contributor Author

ericfranz commented Nov 2, 2018

I propose we do not do this: #8

Rather, we should encourage and enable setting these env vars in the PUN:

OOD_BRAND_BG_COLOR
OOD_BRAND_LINK_ACTIVE_BG_COLOR
OOD_DASHBOARD_TITLE
OOD_DASHBOARD_URL

Then we can update the error pages in nginx_stage to have the same banner at the top, using the same brand.

Also, we should probably provide a custom app to serve these error pages and use NGINX's internal redirects to serve responses. This custom app could be in node.js, since launching Ruby is problematic with rubygems that is not 3.0.0.

These things will be easier to implement with a monorepo.

@ericfranz

This comment has been minimized.

Copy link
Contributor Author

ericfranz commented Nov 2, 2018

To clarify, instead of #8 we would:

  1. enable the simple branding options we support in the dashboard to work across all OOD apps (portal title, brand active link color, brand background color)
  2. add a second missing home directory error page that says nothing of auto-creating home directories on login
  3. add a new nginx_stage configuration option to support logging in

And even without a monorepo, we could do this today by adding an empty apps directory to this repo and adding a new error handling app to this directory.

@ericfranz

This comment has been minimized.

Copy link
Contributor Author

ericfranz commented Nov 2, 2018

The one unfortunate aspect of having any variable set in the PUN is that then this affects 100% of passenger apps started by that PUN - thus dev and shared apps as well as prod apps. So it makes our developer work slightly more difficult.

@ericfranz ericfranz added the duplicate label Jan 11, 2019

@ericfranz

This comment has been minimized.

Copy link
Contributor Author

ericfranz commented Jan 11, 2019

This is now in OnDemand 1.4. Closing this PR.

@ericfranz ericfranz closed this Jan 11, 2019

@ericfranz ericfranz deleted the fix_missing_home_dir branch Jan 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment