Skip to content
This repository was archived by the owner on Jun 2, 2021. It is now read-only.

Commit 0e8a6e6

Browse files
monamohebbiBrian Cunnie
andcommitted
V3: POST /v3/routes on TCP domains: no host/path
TCP domains don't accept `host` or `path` in the body of the request. `path` is an HTTP attribute, and thus makes no sense for a TCP domain. Although you could make the argument that `host` is not an HTTP attribute, its purpose is to enable [Name-based Virtual Host Support](https://httpd.apache.org/docs/2.4/vhosts/name-based.html), which again is HTTP-thing. This is usually accomplished with a wildcard DNS entry (e.g. `*.domain.tld). Many Cloud Foundry installations have a TCP domain, with an IP address/load balancer distinct from the shared app domain. Often the DNS entry's hostname is `tcp`, as in `tcp.domain.tld`. The convention we are using to identify a TCP domain is the following: - if it's running on k8s, it's an "http" domain (this will change) - if `router_group.guid` is populated, it's a "tcp" domain. - The route controller has an awkward method, `unprocessable_non_http_protocols(message, domain)`; its sole purpose is to appease RuboCop's `Metrics/CyclomaticComplexity` requirements. Yes, we are not happy about it either. - We yanked the `protocols` method (which returns the domain's protocols, currently "http" and "tcp") out of the presenter and thrust it into the model, which is a more appropriate home. We also backfilled tests for it. [finishes #169636853] Co-authored-by: Mona Mohebbi <mmohebbi@pivotal.io> Co-authored-by: Brian Cunnie <bcunnie@pivotal.io>
1 parent 5f0c5c4 commit 0e8a6e6

3 files changed

Lines changed: 112 additions & 1 deletion

File tree

app/controllers/v3/routes_controller.rb

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,10 @@ def create
6767
unprocessable_space! unless space
6868
unprocessable_domain! unless domain
6969
unauthorized! unless permission_queryer.can_write_to_space?(space.guid)
70-
unprocessable_wildcard! if domain.shared? && message.wildcard? && !permission_queryer.can_write_globally?
70+
if domain.shared?
71+
unprocessable_wildcard! if message.wildcard? && !permission_queryer.can_write_globally?
72+
unprocessable_non_http_protocols(message, domain)
73+
end
7174

7275
route = RouteCreate.new(user_audit_info).create(message: message, space: space, domain: domain)
7376

@@ -222,6 +225,14 @@ def unprocessable_domain!
222225
unprocessable!('Invalid domain. Ensure that the domain exists and you have access to it.')
223226
end
224227

228+
def unprocessable_protocol_host!
229+
unprocessable!('Hosts are not supported for TCP routes.')
230+
end
231+
232+
def unprocessable_protocol_path!
233+
unprocessable!('Paths are not supported for TCP routes.')
234+
end
235+
225236
def validate_app_guids!(apps_hash, desired_app_guids)
226237
existing_app_guids = apps_hash.keys
227238

@@ -239,4 +250,15 @@ def validate_app_spaces!(apps_hash, route)
239250
def app_not_found!
240251
resource_not_found!(:app)
241252
end
253+
254+
def non_http_protocols_present?(domain)
255+
!(domain.protocols - ['http']).empty?
256+
end
257+
258+
def unprocessable_non_http_protocols(message, domain)
259+
if non_http_protocols_present?(domain)
260+
unprocessable_protocol_host! if message.host
261+
unprocessable_protocol_path! if message.path
262+
end
263+
end
242264
end

spec/request/routes_spec.rb

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -938,6 +938,61 @@
938938
end
939939

940940
describe 'POST /v3/routes' do
941+
context 'when creating a route in a tcp domain' do
942+
let(:domain) { VCAP::CloudController::SharedDomain.make(router_group_guid: 'some-router-group', router_group_type: 'tcp') }
943+
before do
944+
token = { token_type: 'Bearer', access_token: 'my-favourite-access-token' }
945+
stub_request(:post, 'https://uaa.service.cf.internal/oauth/token').
946+
to_return(status: 200, body: token.to_json, headers: { 'Content-Type' => 'application/json' })
947+
stub_request(:get, 'http://localhost:3000/routing/v1/router_groups').
948+
to_return(status: 200, body: '[{"guid":"some-router-group","name":"Robby Router","type":"tcp","reservable_ports":"25555"}]', headers: {})
949+
end
950+
951+
context 'and the route has a host' do
952+
let(:params) do
953+
{
954+
host: 'my-host',
955+
relationships: {
956+
space: {
957+
data: { guid: space.guid }
958+
},
959+
domain: {
960+
data: { guid: domain.guid }
961+
},
962+
}
963+
}
964+
end
965+
966+
it 'returns a 422 and a helpful error message' do
967+
post '/v3/routes', params.to_json, admin_header
968+
expect(last_response.status).to eq(422)
969+
expect(last_response).to have_error_message('Hosts are not supported for TCP routes.')
970+
end
971+
end
972+
973+
context 'and the route has a path' do
974+
let(:params) do
975+
{
976+
path: '/cgi-bin',
977+
relationships: {
978+
space: {
979+
data: { guid: space.guid }
980+
},
981+
domain: {
982+
data: { guid: domain.guid }
983+
},
984+
}
985+
}
986+
end
987+
988+
it 'returns a 422 and a helpful error message' do
989+
post '/v3/routes', params.to_json, admin_header
990+
expect(last_response.status).to eq(422)
991+
expect(last_response).to have_error_message('Paths are not supported for TCP routes.')
992+
end
993+
end
994+
end
995+
941996
context 'when creating a route in a scoped domain' do
942997
let(:domain) { VCAP::CloudController::PrivateDomain.make(owning_organization: space.organization) }
943998

spec/unit/models/runtime/domain_spec.rb

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,40 @@ module VCAP::CloudController
115115
end
116116
end
117117

118+
describe 'protocols' do
119+
let(:domain_with_router_group) { Domain.make(router_group_guid: 'some-guid') }
120+
before do
121+
allow(VCAP::CloudController::Config).to receive_message_chain(:config, :get).with(:kubernetes, :host_url).and_return k8s_enabled
122+
end
123+
124+
context 'kubernetes is enabled (which implies istio)' do
125+
let(:k8s_enabled) { 'k8s' }
126+
context "there's a router group (this should never happen)" do
127+
it 'will return http' do
128+
expect(domain_with_router_group.protocols).to eq(['http'])
129+
end
130+
end
131+
context "there's no router group" do
132+
it 'will still return http' do
133+
expect(subject.protocols).to eq(['http'])
134+
end
135+
end
136+
end
137+
context "kubernetes isn't enabled; it's the canonical routing back-end" do
138+
let(:k8s_enabled) { '' }
139+
context "there's a router group" do
140+
it 'will return tcp' do
141+
expect(domain_with_router_group.protocols).to eq(['tcp'])
142+
end
143+
end
144+
context "there's no router group" do
145+
it 'will return http' do
146+
expect(subject.protocols).to eq(['http'])
147+
end
148+
end
149+
end
150+
end
151+
118152
describe '#spaces_sti_eager_load (eager loading)' do
119153
before { SharedDomain.dataset.destroy }
120154

0 commit comments

Comments
 (0)