Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove vip usage #419

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Remove vip usage #419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions jobs/cloud_controller_ng/spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1235,11 +1235,7 @@ properties:

cc.internal_route_vip_range:
default: "127.128.0.0/9"
description: "The IPv4 CIDR range of virtual IP addresses to be assigned to routes on internal domains.
WARNING: Changing this range is not supported, and has undefined behaviors.
It is recommended to leave this value as the default.
If this range is changed, it is likely the routes on the internal service mesh domain
will need to be recreated."
description: "This is only here cause cf networking needs it, we should coordinate a delete with them"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ameowlia @geofffranks why does cf-networking-release use this?
https://github.com/search?q=repo%3Acloudfoundry%2Fcf-networking-release%20internal_route_vip_range&type=code

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh this was only needed for istio release stuff which was never GAed. rip it out.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when you say rip it out, do you mean make prs to cf networking to remove internal_route_vip_range, followed by removing internal_route_vip_range here? Or do you mean merge this PR as is and some day in the future remove from internal_route_vip_range from cf networking and then from here?


cc.log_audit_events:
default: true
Expand Down
10 changes: 0 additions & 10 deletions jobs/cloud_controller_ng/templates/cloud_controller_ng.yml.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,16 +546,6 @@ perm:
max_labels_per_resource: <%= p("cc.max_labels_per_resource") %>
max_annotations_per_resource: <%= p("cc.max_annotations_per_resource") %>

<%
internal_vip_range = p("cc.internal_route_vip_range")
raise StandardError.new("invalid cc.internal_route_vip_range: #{internal_vip_range}") unless internal_vip_range =~ /\A (?:\d{1,3}\.){3} \d{1,3} \/ \d{1,3} \z/x

parts = internal_vip_range.split(/[\.\/]/).map(&:to_i)
raise StandardError.new("invalid cc.internal_route_vip_range: #{internal_vip_range}") if parts[0..3].any? {|x| x > 255} || parts[4] > 32
%>

internal_route_vip_range: <%= internal_vip_range %>

threadpool_size: <%= p("cc.experimental.thin_server.thread_pool_size") %>

default_app_lifecycle: buildpack
Expand Down
6 changes: 1 addition & 5 deletions jobs/cloud_controller_worker/spec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,11 +542,7 @@ properties:

cc.internal_route_vip_range:
default: "127.128.0.0/9"
description: "The IPv4 CIDR range of virtual IP addresses to be assigned to routes on internal domains.
WARNING: Changing this range is not supported, and has undefined behaviors.
It is recommended to leave this value as the default.
If this range is changed, it is likely the routes on the internal service mesh domain
will need to be recreated."
description: "This is only here cause cf networking needs it, we should coordinate a delete with them"

cc.loggregator.internal_url:
description: "Internal URL used to communicate with traffic_controller"
Expand Down
10 changes: 0 additions & 10 deletions jobs/cloud_controller_worker/templates/cloud_controller_ng.yml.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,16 +332,6 @@ perform_blob_cleanup: <%= p("cc.perform_blob_cleanup") %>
system_domain: <%= p("system_domain") %>
system_hostnames: <%= link("cloud_controller_internal").p("cc.system_hostnames") %>

<%
internal_vip_range = p("cc.internal_route_vip_range")
raise StandardError.new("invalid cc.internal_route_vip_range: #{internal_vip_range}") unless internal_vip_range =~ /\A (?:\d{1,3}\.){3} \d{1,3} \/ \d{1,3} \z/x

parts = internal_vip_range.split(/[\.\/]/).map(&:to_i)
raise StandardError.new("invalid cc.internal_route_vip_range: #{internal_vip_range}") if parts[0..3].any? {|x| x > 255} || parts[4] > 32
%>

internal_route_vip_range: <%= internal_vip_range %>

disable_private_domain_cross_space_context_path_route_sharing: <%= link("cloud_controller_internal").p("cc.disable_private_domain_cross_space_context_path_route_sharing") %>

max_labels_per_resource: <%= link("cloud_controller_internal").p("cc.max_labels_per_resource") %>
Expand Down
29 changes: 0 additions & 29 deletions spec/cloud_controller_ng/cloud_controller_ng_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,35 +195,6 @@ module Test
end
end

describe 'internal route vip range' do
it 'has a default range' do
rendered_hash = YAML.safe_load(template.render(merged_manifest_properties, consumes: links))
expect(rendered_hash['internal_route_vip_range']).to eq('127.128.0.0/9')
end

describe 'when a range is specified in manifest properties' do
it 'validates they are valid CIDRs' do
merged_manifest_properties['cc']['internal_route_vip_range'] = '10.16.255.0/777'
expect do
YAML.safe_load(template.render(merged_manifest_properties, consumes: links))
end.to raise_error(StandardError, 'invalid cc.internal_route_vip_range: 10.16.255.0/777')
end

it 'does not allow ipv6 addresses' do
merged_manifest_properties['cc']['internal_route_vip_range'] = '2001:0db8:85a3:0000:0000:8a2e:0370:7334/21'
expect do
YAML.safe_load(template.render(merged_manifest_properties, consumes: links))
end.to raise_error(StandardError, 'invalid cc.internal_route_vip_range: 2001:0db8:85a3:0000:0000:8a2e:0370:7334/21')
end

it 'renders valid CIDRs' do
merged_manifest_properties['cc']['internal_route_vip_range'] = '10.16.255.0/24'
rendered_hash = YAML.safe_load(template.render(merged_manifest_properties, consumes: links))
expect(rendered_hash['internal_route_vip_range']).to eq('10.16.255.0/24')
end
end
end

describe 'database_encryption block' do
context 'when the database_encryption block is not present' do
before do
Expand Down