Commit 1204723f authored by Valery Sizov's avatar Valery Sizov Committed by Douglas Barbosa Alexandre

Geo: Support syncing over non-publicly accessible URLs

This change allows to use some alternative URL for primary node.
This URL will be used only by secondary node to reach the primary
parent 3f7836b2
......@@ -1362,6 +1362,7 @@ ActiveRecord::Schema.define(version: 20190403161806) do
t.integer "verification_max_capacity", default: 100, null: false
t.integer "minimum_reverification_interval", default: 7, null: false
t.string "alternate_url"
t.string "internal_url"
t.index ["access_key"], name: "index_geo_nodes_on_access_key", using: :btree
t.index ["primary"], name: "index_geo_nodes_on_primary", using: :btree
t.index ["url"], name: "index_geo_nodes_on_url", unique: true, using: :btree
......
......@@ -20,6 +20,8 @@ Example response:
{
"id": 1,
"url": "https://primary.example.com/",
"alternate_url": null,
"internal_url": "https://internal.example.com/",
"primary": true,
"enabled": true,
"current": true,
......@@ -32,6 +34,7 @@ Example response:
"id": 2,
"url": "https://secondary.example.com/",
"alternate_url": "https://alternate.example.com/",
"internal_url": "https://secondary.example.com/",
"primary": false,
"enabled": true,
"current": false,
......@@ -59,6 +62,8 @@ Example response:
{
"id": 1,
"url": "https://primary.example.com/",
"alternate_url": null,
"internal_url": "https://primary.example.com/",
"primary": true,
"enabled": true,
"current": true,
......@@ -84,7 +89,8 @@ PUT /geo_nodes/:id
| `id` | integer | yes | The ID of the Geo node. |
| `enabled` | boolean | no | Flag indicating if the Geo node is enabled. |
| `url` | string | no | The URL to connect to the Geo node. |
| `alternate_url` | string | no | Allows users to log in to the secondary at an alternate URL (required for OAuth) |
| `alternate_url` | string | no | Allows users to log in to the secondary node at an alternate URL (required for OAuth). |
| `internal_url` | string | no | The URL defined on the primary node that secondary nodes should use to contact it. Returns `url` if not set.|
| `files_max_capacity` | integer | no | Control the maximum concurrency of LFS/attachment backfill for this secondary node. |
| `repos_max_capacity` | integer | no | Control the maximum concurrency of repository backfill for this secondary node. |
| `verification_max_capacity` | integer | no | Control the maximum concurrency of verification for this node. |
......@@ -96,6 +102,7 @@ Example response:
"id": 1,
"url": "https://secondary.example.com/",
"alternate_url": "https://alternate.example.com/",
"internal_url": "https://secondary.example.com/",
"primary": false,
"enabled": true,
"current": true,
......@@ -137,6 +144,8 @@ Example response:
{
"id": 1,
"url": "https://primary.example.com/",
"alternate_url": null,
"internal_url": "https://primary.example.com/",
"primary": true,
"enabled": true,
"current": true,
......
......@@ -56,3 +56,13 @@ this URL must be specified as the "Alternate URL" on every secondary behind it.
Additionally, the load balancer should use sticky sessions, since users must
authenticate each first request to each secondary.
## Using a different URL for synchronization
The **primary** node's Internal URL is used by **secondary** nodes to contact it
(to sync repositories, for example). The name Internal URL distinguishes it from
[External URL](https://docs.gitlab.com/omnibus/settings/configuration.html#configuring-the-external-url-for-gitlab)
which is used by users. Internal URL does not need to be a private address.
Internal URL defaults to External URL, but you can customize it under
**Admin area > Geo Nodes**.
......@@ -121,7 +121,7 @@ export default {
</script>
<template>
<div class="node-detail-item">
<div class="prepend-top-15 prepend-left-10 node-detail-item">
<div class="node-detail-title">
<span> {{ itemTitle }} </span>
<icon
......
......@@ -58,6 +58,15 @@ export default {
});
}
if (this.node.internalUrl) {
primaryNodeDetailItems.push({
itemTitle: s__('GeoNodes|Internal URL'),
itemValue: this.node.internalUrl,
itemValueType: VALUE_TYPE.PLAIN,
cssClass: 'node-detail-value-bold',
});
}
return primaryNodeDetailItems;
}
......@@ -91,14 +100,6 @@ export default {
? `${cssClass} node-detail-value-error`
: cssClass;
},
sectionItemsContainerClasses() {
const { nodeTypePrimary, showSectionItems } = this;
return {
'col-md-6 prepend-left-15': nodeTypePrimary,
'row col-md-12 prepend-left-10': !nodeTypePrimary,
'd-flex': showSectionItems && !nodeTypePrimary,
};
},
},
methods: {
handleSectionToggle(toggleState) {
......@@ -118,13 +119,11 @@ export default {
</div>
<div
v-show="showSectionItems"
:class="sectionItemsContainerClasses"
class="prepend-top-10 section-items-container"
class="col-md-6 prepend-left-15 prepend-top-10 section-items-container"
>
<geo-node-detail-item
v-for="(nodeDetailItem, index) in nodeDetailItems"
:key="index"
:class="{ 'prepend-top-15 prepend-left-10': nodeTypePrimary, 'col-sm-3': !nodeTypePrimary }"
:css-class="nodeDetailItem.cssClass"
:item-title="nodeDetailItem.itemTitle"
:item-value="nodeDetailItem.itemValue"
......
......@@ -141,7 +141,6 @@ export default {
:item-value-stale-tooltip="statusInfoStaleMessage"
:custom-type="nodeDetailItem.customType"
:event-type-log-status="nodeDetailItem.eventTypeLogStatus"
class="prepend-top-15 prepend-left-10"
/>
</div>
</div>
......
......@@ -136,7 +136,6 @@ export default {
:failure-label="nodeDetailItem.failureLabel"
:custom-type="nodeDetailItem.customType"
:help-info="nodeDetailItem.helpInfo"
class="prepend-top-15 prepend-left-10"
/>
</div>
</template>
......
......@@ -49,6 +49,7 @@ export default class GeoNodesStore {
current,
enabled,
alternateUrl: rawNode.alternate_url || '',
internalUrl: rawNode.internal_url || '',
nodeActionActive: false,
basePath: rawNode._links.self,
repairPath: rawNode._links.repair,
......
......@@ -5,8 +5,12 @@ import Api from '~/api';
const onPrimaryCheckboxChange = function onPrimaryCheckboxChange(e, $namespaces, $reverification) {
const $namespacesSelect = $('.select2', $namespaces);
const $alternateUrl = $('.js-alternate-url');
const $internalUrl = $('.js-internal-url');
$namespacesSelect.select2('data', null);
$alternateUrl.toggleClass('hidden', e.currentTarget.checked);
$internalUrl.toggleClass('hidden', !e.currentTarget.checked);
$namespaces.toggleClass('hidden', e.currentTarget.checked);
$reverification.toggleClass('hidden', !e.currentTarget.checked);
};
......
......@@ -51,6 +51,7 @@ class Admin::Geo::NodesController < Admin::ApplicationController
params.require(:geo_node).permit(
:url,
:alternate_url,
:internal_url,
:primary,
:selective_sync_type,
:namespace_ids,
......
......@@ -155,7 +155,7 @@ module EE
end
def primary_full_url
File.join(::Gitlab::Geo.primary_node.url, request_fullpath_for_primary)
::Gitlab::Utils.append_path(::Gitlab::Geo.primary_node.internal_url, request_fullpath_for_primary)
end
def redirect?
......
......@@ -18,9 +18,9 @@ class GeoNode < ApplicationRecord
default_values url: ->(record) { record.class.current_node_url },
primary: false
validates :url, presence: true, uniqueness: { case_sensitive: false }
validate :url_is_http
validate :alternate_url_is_http
validates :url, presence: true, uniqueness: { case_sensitive: false }, url: true
validates :alternate_url, url: true, allow_blank: true, allow_nil: true
validates :internal_url, url: true, allow_blank: true, allow_nil: true
validates :primary, uniqueness: { message: 'node already exists' }, if: :primary
validates :enabled, if: :primary, acceptance: { message: 'Geo primary node cannot be disabled' }
......@@ -142,6 +142,16 @@ class GeoNode < ApplicationRecord
@alternate_uri = nil
end
def internal_url
read_with_ending_slash(:internal_url).presence || read_with_ending_slash(:url)
end
def internal_url=(value)
value = add_ending_slash(value) != url ? value : nil
write_with_ending_slash(:internal_url, value)
@internal_uri = nil
end
def uri
@uri ||= URI.parse(url) if url.present?
end
......@@ -150,6 +160,10 @@ class GeoNode < ApplicationRecord
@alternate_uri ||= URI.parse(alternate_url) if alternate_url.present?
end
def internal_uri
@internal_uri ||= URI.parse(internal_url) if internal_url.present?
end
def geo_transfers_url(file_type, file_id)
geo_api_url("transfers/#{file_type}/#{file_id}")
end
......@@ -249,7 +263,7 @@ class GeoNode < ApplicationRecord
end
def api_url(suffix)
Gitlab::Utils.append_path(uri.to_s, "api/#{API::API.version}/#{suffix}")
Gitlab::Utils.append_path(internal_uri.to_s, "api/#{API::API.version}/#{suffix}")
end
def ensure_access_keys!
......@@ -289,24 +303,6 @@ class GeoNode < ApplicationRecord
end
end
def url_is_http
url_is_http_for(:url, uri)
end
def alternate_url_is_http
url_is_http_for(:alternate_url, alternate_uri)
end
def url_is_http_for(attribute, uri_value)
return unless uri_value
unless %w[http https].include?(uri_value.scheme)
errors.add(attribute, 'scheme must be http or https')
end
rescue URI::InvalidURIError
errors.add(attribute, 'is not a valid URI')
end
def update_clone_url
self.clone_url_prefix = Gitlab.config.gitlab_shell.ssh_path_prefix
end
......
......@@ -108,7 +108,7 @@ module Geo
end
def remote_url
Gitlab::Geo.primary_node.url + repository.full_path + '.git'
Gitlab::Utils.append_path(Gitlab::Geo.primary_node.internal_url, "#{repository.full_path}.git")
end
# Use snapshotting for redownloads *only* when enabled.
......
......@@ -4,13 +4,19 @@
= form.label :url, s_('Geo|URL'), class: 'font-weight-bold'
= form.text_field :url, class: 'form-control'
.form-group.col-sm-6.js-hide-if-geo-primary{ class: ('hidden' unless geo_node.secondary?) }
.form-group.col-sm-6.js-alternate-url{ class: ('hidden' unless geo_node.secondary?) }
= form.label :alternate_url, s_('Geo|Alternate URL'), class: 'font-weight-bold'
= form.text_field :alternate_url, class: 'form-control'
.form-text.text-muted= s_('Geo|To support OAuth logins to this node at a different domain than URL')
.form-row.form-group
.col-sm-12
.form-group.col-sm-6.js-internal-url{ class: ('hidden' unless geo_node.primary?) }
= form.label :internal_url, s_('Geo|Internal URL'), class: 'font-weight-bold'
= form.text_field :internal_url, class: 'form-control'
.form-text.text-muted= s_('Geo|The URL defined on the primary node that secondary nodes should use to contact it. Returns `url` if not set')
.form-group.row
.col-sm-10
.form-check
= form.check_box :primary, class: 'form-check-input'
= form.label :primary, class: 'form-check-label' do
......
......@@ -11,3 +11,4 @@
= render partial: 'form', locals: { form: f, geo_node: @node }
.form-actions
= f.submit 'Add Node', class: 'btn btn-success'
= link_to 'Cancel', admin_geo_nodes_path, class: 'btn btn-cancel'
---
title: 'Geo: Support syncing over non-publicly accessible URLs'
merge_request: 9634
author:
type: added
# frozen_string_literal: true
class AddInternalUrlToGeoNodes < ActiveRecord::Migration[5.0]
def change
add_column :geo_nodes, :internal_url, :string
end
end
......@@ -144,7 +144,8 @@ module API
params do
optional :enabled, type: Boolean, desc: 'Flag indicating if the Geo node is enabled'
optional :url, type: String, desc: 'The URL to connect to the Geo node'
optional :alternate_url, type: String, desc: 'An alternate URL to allow OAuth logins to this secondary node'
optional :alternate_url, type: String, desc: 'The alternate URL to allow OAuth logins to this secondary node'
optional :internal_url, type: String, desc: 'The URL defined on the primary node that secondary nodes should use to contact it. Defaults to url'
optional :files_max_capacity, type: Integer, desc: 'Control the maximum concurrency of LFS/attachment backfill for this secondary node'
optional :repos_max_capacity, type: Integer, desc: 'Control the maximum concurrency of repository backfill for this secondary node'
optional :verification_max_capacity, type: Integer, desc: 'Control the maximum concurrency of repository verification for this node'
......
......@@ -463,6 +463,7 @@ module EE
expose :id
expose :url
expose :alternate_url
expose :internal_url
expose :primary?, as: :primary
expose :enabled
expose :current?, as: :current
......
......@@ -36,10 +36,14 @@ module Gitlab
oauth_application&.secret,
site: Gitlab::Geo.primary_node.url,
authorize_url: oauth_authorization_path,
token_url: oauth_token_path
token_url: token_url
)
end
end
def token_url
Gitlab::Utils.append_path(Gitlab::Geo.primary_node.internal_url, oauth_token_path)
end
end
end
end
......
......@@ -33,8 +33,8 @@ module SystemCheck
private
def check_gitlab_geo_node(node)
response = Net::HTTP.start(node.uri.host, node.uri.port, use_ssl: (node.uri.scheme == 'https')) do |http|
http.request(Net::HTTP::Get.new(node.uri))
response = Net::HTTP.start(node.internal_uri.host, node.internal_uri.port, use_ssl: (node.internal_uri.scheme == 'https')) do |http|
http.request(Net::HTTP::Get.new(node.internal_uri))
end
if response.code_type == Net::HTTPFound
......@@ -48,7 +48,7 @@ module SystemCheck
try_fixing_it(
'Check if the machine is online and GitLab is running',
'Check your firewall rules and make sure this machine can reach the target machine',
"Make sure port and protocol are correct: '#{node.url}', or change it in Admin > Geo Nodes"
"Make sure port and protocol are correct: '#{node.internal_url}', or change it in Admin > Geo Nodes"
)
rescue SocketError => e
display_exception(e)
......@@ -56,7 +56,7 @@ module SystemCheck
if e.cause && e.cause.message.starts_with?('getaddrinfo')
try_fixing_it(
'Check if your machine can connect to a DNS server',
"Check if your machine can resolve DNS for: '#{node.uri.host}'",
"Check if your machine can resolve DNS for: '#{node.internal_uri.host}'",
'If machine host is incorrect, change it in Admin > Geo Nodes'
)
end
......
......@@ -110,7 +110,14 @@ describe Admin::Geo::NodesController, :postgresql do
end
describe '#update' do
let(:geo_node_attributes) { { url: 'http://example.com', alternate_url: 'http://anotherexample.com', selective_sync_shards: %w[foo bar] } }
let(:geo_node_attributes) do
{
url: 'http://example.com',
alternate_url: 'http://anotherexample.com',
internal_url: 'http://internal-url.com',
selective_sync_shards: %w[foo bar]
}
end
let(:geo_node) { create(:geo_node) }
......@@ -138,6 +145,7 @@ describe Admin::Geo::NodesController, :postgresql do
geo_node.reload
expect(geo_node.url.chomp('/')).to eq(geo_node_attributes[:url])
expect(geo_node.alternate_url.chomp('/')).to eq(geo_node_attributes[:alternate_url])
expect(geo_node.internal_url.chomp('/')).to eq(geo_node_attributes[:internal_url])
expect(geo_node.selective_sync_shards).to eq(%w[foo bar])
end
......
......@@ -114,16 +114,18 @@ describe 'admin Geo Nodes', :js do
end
describe 'update an existing Geo Node' do
before do
it 'updates an existing Geo Node' do
geo_node.update(primary: true)
visit admin_geo_nodes_path
wait_for_requests
page.within(find('.geo-node-actions', match: :first)) do
page.click_link('Edit')
end
end
it 'updates an existing Geo Node' do
fill_in 'URL', with: 'http://newsite.com'
fill_in 'Internal URL', with: 'http://internal-url.com'
check 'This is a primary node'
click_button 'Save changes'
......@@ -133,10 +135,23 @@ describe 'admin Geo Nodes', :js do
page.within(find('.geo-node-item', match: :first)) do
expect(page).to have_content('http://newsite.com')
expect(page).to have_content('Primary')
click_button 'Other information'
page.within(find('.other-section')) do
expect(page).to have_content('http://internal-url.com')
end
end
end
it "allows the admin to update a secondary node's alternate URL" do
visit admin_geo_nodes_path
wait_for_requests
page.within(find('.geo-node-actions', match: :first)) do
page.click_link('Edit')
end
fill_in 'Alternate URL', with: 'http://someloadbalancer.com'
click_button 'Save changes'
......
......@@ -18,6 +18,7 @@
"url": { "type": ["string", "null"] },
"alternate_url": { "type": ["string", "null"] },
"primary": { "type": "boolean" },
"internal_url": { "type": ["string", "null"] },
"enabled": { "type": "boolean" },
"current": { "type": "boolean" },
"files_max_capacity": { "type": "integer" },
......
......@@ -43,11 +43,13 @@ describe('NodeDetailsSectionOther', () => {
const items = vmNodePrimary.nodeDetailItems;
expect(items.length).toBe(2);
expect(items.length).toBe(3);
expect(items[0].itemTitle).toBe('Replication slots');
expect(items[0].itemValue).toBe(mockNodeDetails.replicationSlots);
expect(items[1].itemTitle).toBe('Replication slot WAL');
expect(items[1].itemValue).toBe(numberToHumanSize(mockNodeDetails.replicationSlotWAL));
expect(items[2].itemTitle).toBe('Internal URL');
expect(items[2].itemValue).toBe(mockNode.internalUrl);
vmNodePrimary.$destroy();
});
......
......@@ -45,6 +45,7 @@ export const mockNode = {
id: 1,
url: 'http://127.0.0.1:3001/',
alternateUrl: '',
internalUrl: 'http://127.0.0.1:3001/',
primary: true,
current: true,
enabled: true,
......
......@@ -16,12 +16,12 @@ describe Gitlab::Geo::Oauth::Session do
describe '#authorized_url' do
it 'returns a valid url to the primary node' do
expect(subject.authorize_url).to start_with(primary_node.url)
expect(subject.authorize_url).to start_with(primary_node.internal_url)
end
end
describe '#authenticate' do
let(:api_url) { "#{primary_node.url.chomp('/')}/api/v4/user" }
let(:api_url) { "#{primary_node.internal_url.chomp('/')}/api/v4/user" }
let(:user_json) { ActiveSupport::JSON.encode({ id: 555, email: 'user@example.com' }.as_json) }
context 'on success' do
......
......@@ -81,6 +81,28 @@ describe GeoNode, type: :model do
it { is_expected.not_to be_valid }
end
end
context 'when validating internal_url' do
subject { build(:geo_node, internal_url: internal_url) }
context 'when internal_url is http' do
let(:internal_url) { 'http://foo' }
it { is_expected.to be_valid }
end
context 'when internal_url is https' do
let(:internal_url) { 'https://foo' }
it { is_expected.to be_valid }
end
context 'when internal_url is not http or https' do
let(:internal_url) { 'nothttp://foo' }
it { is_expected.not_to be_valid }
end
end
end
context 'default values' do
......@@ -491,7 +513,7 @@ describe GeoNode, type: :model do
end
describe '#alternate_url=' do
subject { GeoNode.new(alternate_url: 'https://foo:3003/bar') }
subject { described_class.new(alternate_url: 'https://foo:3003/bar') }
it 'sets schema field based on url' do
expect(subject.alternate_uri.scheme).to eq('https')
......@@ -527,6 +549,70 @@ describe GeoNode, type: :model do
end
end
describe '#internal_url' do
let(:internal_url) { 'https://foo:3003/bar' }
let(:node) { create(:geo_node, url: dummy_url, internal_url: internal_url) }
it 'returns a string' do
expect(node.internal_url).to be_a String
end
it 'includes schema home port and relative_url with a terminating /' do
expect(node.internal_url).to eq("#{internal_url}/")
end
it 'falls back to url' do
empty_node.url = dummy_url
empty_node.internal_url = nil
expect(empty_node.internal_url).to eq "#{dummy_url}/"
end
it 'resets internal_url if it matches #url' do
empty_node.url = dummy_url
empty_node.internal_url = dummy_url
expect(empty_node.attributes[:internal_url]).to be_nil
end
end
describe '#internal_url=' do
subject { described_class.new(internal_url: 'https://foo:3003/bar') }
it 'sets schema field based on url' do
expect(subject.internal_uri.scheme).to eq('https')
end
it 'sets host field based on url' do
expect(subject.internal_uri.host).to eq('foo')
end
it 'sets port field based on specified by url' do
expect(subject.internal_uri.port).to eq(3003)
end
context 'when using unspecified ports' do
let(:dummy_http) { 'http://example.com/' }
let(:dummy_https) { 'https://example.com/' }
context 'when schema is http' do
it 'sets port 80' do
subject.internal_url = dummy_http
expect(subject.internal_uri.port).to eq(80)
end
end
context 'when schema is https' do
it 'sets port 443' do
subject.internal_url = dummy_https
expect(subject.internal_uri.port).to eq(443)
end
end
end
end
describe '#geo_transfers_url' do
let(:transfers_url) { "https://localhost:3000/gitlab/api/#{api_version}/geo/transfers/lfs/1" }
......
......@@ -213,6 +213,7 @@ describe API::GeoNodes, :geo, :prometheus, api: true do
enabled: false,
url: 'https://updated.example.com/',
alternate_url: 'https://alternate.example.com/',
internal_url: 'https://internal-com.com/',
files_max_capacity: 33,
repos_max_capacity: 44,
verification_max_capacity: 55
......
......@@ -4824,6 +4824,9 @@ msgstr ""
msgid "GeoNodes|Health status"
msgstr ""
msgid "GeoNodes|Internal URL"
msgstr ""
msgid "GeoNodes|Last event ID processed by cursor"
msgstr ""
......@@ -5010,6 +5013,9 @@ msgstr ""
msgid "Geo|In sync"
msgstr ""
msgid "Geo|Internal URL"
msgstr ""
msgid "Geo|Last repository check run"
msgstr ""
......@@ -5097,6 +5103,9 @@ msgstr ""
msgid "Geo|Synchronization failed - %{error}"
msgstr ""
msgid "Geo|The URL defined on the primary node that secondary nodes should use to contact it. Returns `url` if not set"
msgstr ""
msgid "Geo|This is a primary node"
msgstr ""
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment