Subdomains #1786

murny · 2020-07-29T03:54:24Z

This PR wraps all current routes under an "ERA" subdomain. Which sets up the foundation for allowing multiple front doors going forward.

murny · 2020-07-29T03:56:04Z

test/controllers/aip/v1/aip_collections_controller_test.rb


 class Aip::V1::CollectionsControllerTest < ActionDispatch::IntegrationTest

  include AipHelper

-  def setup
+  setup do


Slight difference here. setup do hooks into all setup blocks in the inheritance where as def setup doesn't.

As a result, this was flaking if this test ran first as our host configuration never ran yet.

murny · 2020-07-29T03:56:56Z

test/controllers/aip/v1/aip_collections_controller_test.rb

@@ -1,11 +1,11 @@
 require 'test_helper'
-require Rails.root.join('test/support/aip_helper')
+require 'support/aip_helper'


use a relative require here instead (no different then require 'test_helper')

I tried that so many times with no luck ¯_(ツ)_/¯

Haha yeah not sure. Seems to work on my local and CI likes it 🤷

murny · 2020-07-29T04:00:29Z

test/fixtures/files/n3/2107bfb6-2670-4ffc-94a1-aeb4f8c1fd81-prev-embargoed.n3

@@ -6,11 +6,11 @@
 @prefix rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#> .
 @prefix xsd: <http://www.w3.org/2001/XMLSchema#> .

-<http://localhost/aip/v1/items/2107bfb6-2670-4ffc-94a1-aeb4f8c1fd81> a pcdm:Object;
+<<%= url %>/aip/v1/items/2107bfb6-2670-4ffc-94a1-aeb4f8c1fd81> a pcdm:Object;


url is no longer http://localhost, so need to override this url in for all these files

lagoan

Nice clean changes. Just a question on documentation.

lagoan · 2020-08-05T21:52:16Z

README.md

@@ -159,7 +170,7 @@ Now everything should be up and running!
 ## Step 3: Open and view Jupiter!
 Now everything is ready, you can go and view Jupiter! Just open your favorite browser and go to the following url:

-[localhost:3000](http://localhost:3000)
+[era.ualberta.localhost:3000](http://era.ualberta.localhost:3000)

 (Note: ip address may be different if you are using `docker-machine`)


Is this comment regarding different ip address when using docker-machine still valid now that we are using subdomains?

That's a really good point. If someone's using docker-machine they would probably have to add/edit their own /etc/hosts to map to their ip address that docker-machine is providing.

But this is probably old instructions and I doubt anyone is using docker-machine? Since docker has come along way since v1.12 and Docker Desktop is the best way to run docker on Mac/Windows now.

We probably should just delete this note 🤔

pgwillia

Works great! Just a few questions.

pgwillia · 2020-08-05T22:38:12Z

README.md

@@ -211,7 +222,7 @@ modifying the config files, set the environment variable `RUN_FITS_CHARACTERIZAT

 * Update `secrets.yml` (and maybe `omniauth.rb`) for the SAML implementation (you may need to generate a certificate/key for certain environments)
 * Give IST's Identity Provider (uat-login or login) the metadata for our service provider
-  * Quick way to view this metadata is to the start the Rails server and navigate to `http://localhost:3000/auth/saml/metadata` (feel free to edit this metadata accordingly for example adding Organization and ContactPerson metadata)
+  * Quick way to view this metadata is to the start the Rails server and navigate to `http://era.ualberta.localhost:3000/auth/saml/metadata` (feel free to edit this metadata accordingly for example adding Organization and ContactPerson metadata)


Will subdomains impact the metadata that we've already sent to IST for authentication?

It shouldn't. URL for production/staging is unchanged, so should be no impact.

This is just instructions on retrieving the metadata on your local development environment which we need to update as everything in jupiter now is nested under "era" subdomain

pgwillia · 2020-08-05T22:43:31Z

config/environments/development.rb

@@ -67,4 +67,6 @@

  # Action on unpermitted parameters
  config.action_controller.action_on_unpermitted_parameters = :raise
+
+  config.hosts << 'era.lvh.me'


Is this related to

jupiter/config/environments/development.rb

Line 1 in 44df063

Rails.application.routes.default_url_options = { host: 'localhost' }

This line is for whitelisting this host. As documented in the spike:

#1707 (comment) under the "Whitelisting hosts"

Without this, era.lvh.me:3000 will end up with a 500 error exception.

But you raise a very interesting question about our default_url_options in environments and might answer some of my troubles I was having in our test suite 🤔. Will look into maybe a better way of doing things now that I've seen this, so thank you for raising that point 👍

pgwillia · 2020-08-05T22:50:13Z

test/test_helper.rb

+
+  setup do
+    host = URI(Jupiter::TEST_URL).host
+    host! host


What does this line do?

This sets the host that our tests run on to be era.ualberta.localhost. So when we use get '/' the @request.host is being set as era.ualberta.localhost. This allows our code to work as the constraints(subdomain: 'era') do returns true, allowing traffic to flow into our routes.

The next line, configures our url_helpers to set our host properly (so root_url returns era.ualberta.localhost). But as you highlighted in your review, we are setting this in environment/test.rb, so may not need this anymore if I just set it properly in test.rb 🎉 . Will play around with this

pgwillia · 2020-08-05T22:53:35Z

config/routes.rb

-    mount Sidekiq::Web => '/sidekiq', constraints: AdminConstraint.new
-  end
+    post '/logout_as_user', to: 'sessions#logout_as_user'
+    match '/auth/:provider/callback', to: 'sessions#create', via: [:get, :post], as: :login


We could use the login_url helper elsewhere now too! Clever 👍

jupiter/app/views/application/_navbar.html.erb

Lines 95 to 105 in 44df063

<% else %>

<li role="presentation" class="nav-item">

<%= link_to t('.links.login'), '/auth/saml', class: 'nav-item nav-link text-nowrap', method: :post %>

</li>

<% if Rails.env.development? || Rails.env.uat? %>

<%# TODO: Temporary login form for local development or UAT %>

<li role="presentation" class="nav-item">

<%= link_to t('.links.login_as_dev'), '/auth/developer', class: 'nav-item nav-link text-nowrap', method: :post %>

</li>

<% end %>

<% end %>

Yeah absolutely. Not sure why we never named it before 😆. But should allow us to reuse this elsewhere instead of hardcoding the path

pgwillia · 2020-08-05T22:55:27Z

config/secrets.yml

@@ -32,6 +32,7 @@ shared:
  fits_path: <%= ENV['FITS_PATH'] || 'fits.sh' %>
  rack_attack_safelisted_ips: <%= ENV['RACK_ATTACK_SAFELISTED_IPS'] || '""' %>
  system_user_api_key: <%= ENV['SYSTEM_USER_API_KEY'] %>
+  tld_length: <%= ENV['TLD_LENGTH'] || 1 %>


Should we set a value for this in https://github.com/ualbertalib/jupiter/blob/integration_postmigration/.env_deployment_sample

This will only need to be overridden when we have host/domains that have multiple subdomains.

So UAT may need it? I assume UAT might be era.uat.library.ualberta.ca in which case this value needs to be set for 3 as we have 3 subdomains.

We will need this for production especially since our url is era.library.ualberta.ca and we will need to set this value to be 2

pgwillia · 2020-08-05T23:01:29Z

lib/jupiter.rb

@@ -1,3 +1,4 @@
 module Jupiter
  PRODUCTION_URL = 'https://era.library.ualberta.ca'.freeze
+  TEST_URL = 'http://era.ualberta.localhost'.freeze


Is this module just for tests? Is it related to Rails.application.routes.default_url_options?

This module is a generic place to store global configuration.

Examples of other apps in the wild with similar modules:
https://github.com/gitlabhq/gitlabhq/blob/master/lib/gitlab.rb
https://github.com/discourse/discourse/blob/master/lib/discourse.rb

I just need a place to stick this TEST_URL constant without having to copy and paste it all around the codebase. This TEST_URL is used in various places in our test suite to configure the tests to run on this url (so code works as code is now under an "era" subdomain)

Is there a better place for this? Maybe 🤔

…ubdomains

pgwillia

mbarnett

murny requested review from ConnorSheremeta, lagoan, mbarnett and pgwillia as code owners July 29, 2020 03:54

murny commented Jul 29, 2020

View reviewed changes

murny changed the title ~~Subdomains~~ WIP: Subdomains Jul 29, 2020

murny commented Jul 29, 2020

View reviewed changes

murny added 7 commits August 4, 2020 19:07

Add current routes under era subdomain

74fd493

Fix n3 graph urls to include subdomain within aip tests

1a2c65f

Point to oaisys with subdomain fixes

75ce3be

Fix rubocop error

b7cf112

Cleanup host code, fix flakey test in AIP tests

cbe4a00

Add changelog entry

1bf2504

Add configuration for production url and developer instructions

fd5a969

murny force-pushed the subdomains branch from 7e10a01 to fd5a969 Compare August 5, 2020 01:09

Fix rubocop and tests after merge conflict

1593466

murny changed the title ~~WIP: Subdomains~~ Subdomains Aug 5, 2020

lagoan reviewed Aug 5, 2020

View reviewed changes

pgwillia reviewed Aug 5, 2020

View reviewed changes

Remove docker-machine instructions. Cleanup default url helpers for s…

89fbf3f

…ubdomains

pgwillia previously approved these changes Aug 10, 2020

View reviewed changes

mbarnett previously approved these changes Aug 10, 2020

View reviewed changes

Merge latest changes into subdomain branch

68c7b77

murny dismissed stale reviews from mbarnett and pgwillia via 68c7b77 September 3, 2020 17:51

mbarnett added the hacktoberfest-accepted label Oct 5, 2020

mbarnett closed this Oct 7, 2020

mbarnett mentioned this pull request Oct 7, 2020

Subdomains #1922

Closed

murny mentioned this pull request Oct 7, 2020

Subdomains #1928

Merged

pgwillia mentioned this pull request Sep 22, 2021

Review Changes with Stakeholders before 2.1 deploy #2524

Closed

23 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Subdomains #1786

Subdomains #1786

murny commented Jul 29, 2020 •

edited

Loading

murny Jul 29, 2020 •

edited

Loading

murny Jul 29, 2020

lagoan Aug 5, 2020

murny Aug 5, 2020

murny Jul 29, 2020

lagoan left a comment

lagoan Aug 5, 2020

murny Aug 5, 2020 •

edited

Loading

pgwillia left a comment

pgwillia Aug 5, 2020

murny Aug 5, 2020

pgwillia Aug 5, 2020

murny Aug 6, 2020 •

edited

Loading

pgwillia Aug 5, 2020

murny Aug 6, 2020 •

edited

Loading

pgwillia Aug 5, 2020

murny Aug 6, 2020

pgwillia Aug 5, 2020

murny Aug 6, 2020

pgwillia Aug 5, 2020

murny Aug 6, 2020

pgwillia left a comment

mbarnett left a comment •

edited

Loading

	<% else %>
	<li role="presentation" class="nav-item">
	<%= link_to t('.links.login'), '/auth/saml', class: 'nav-item nav-link text-nowrap', method: :post %>
	</li>
	<% if Rails.env.development? \|\| Rails.env.uat? %>
	<%# TODO: Temporary login form for local development or UAT %>
	<li role="presentation" class="nav-item">
	<%= link_to t('.links.login_as_dev'), '/auth/developer', class: 'nav-item nav-link text-nowrap', method: :post %>
	</li>
	<% end %>
	<% end %>

Subdomains #1786

Subdomains #1786

Conversation

murny commented Jul 29, 2020 • edited Loading

murny Jul 29, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lagoan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

murny Aug 5, 2020 • edited Loading

Choose a reason for hiding this comment

pgwillia left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

murny Aug 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

murny Aug 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgwillia left a comment

Choose a reason for hiding this comment

mbarnett left a comment • edited Loading

Choose a reason for hiding this comment

murny commented Jul 29, 2020 •

edited

Loading

murny Jul 29, 2020 •

edited

Loading

murny Aug 5, 2020 •

edited

Loading

murny Aug 6, 2020 •

edited

Loading

murny Aug 6, 2020 •

edited

Loading

mbarnett left a comment •

edited

Loading