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.

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

Added support for falling back to native ingress if not on Openshift #394

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
134 changes: 123 additions & 11 deletions app/services/integration/kubernetes_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ def initialize(integration, namespace: self.class.namespace)
@client = K8s::Client.autoconfig(namespace: namespace).extend(MergePatch)
end

class << self
attr_accessor :use_openshift_route
end

def use_openshift_route?
return self.class.use_openshift_route unless self.class.use_openshift_route.nil?

self.class.use_openshift_route = !Rails.application.config.integrations.fetch(:kubernetes_force_native_ingress, false) && client.api_groups.include?('route.openshift.io/v1')
end

module MergePatch
# @param resource [K8s::Resource]
# @param attrs [Hash]
Expand Down Expand Up @@ -131,6 +141,14 @@ def initialize(attributes, **options)
end
end

class Ingress < K8s::Resource
def initialize(attributes, **options)
super attributes.with_indifferent_access
.merge(apiVersion: 'networking.k8s.io/v1', kind: 'Ingress')
.reverse_merge(metadata: {}), **options
end
end

class RouteSpec < K8s::Resource
def initialize(url, service, port)
uri = URI(url)
Expand All @@ -150,13 +168,55 @@ def initialize(url, service, port)
end
end

class IngressSpec < K8s::Resource
def initialize(url, service, port, tenant_id)
uri = URI(url)
host = uri.host || uri.path

tls_options = [{
hosts: [host],
secretName: service + '-tls-' + tenant_id
}] if uri.class == URI::HTTPS || uri.scheme.blank?

super({
ingressClassName: Rails.application.config.integrations.fetch(:kubernetes_ingress_class, 'nginx'),
rules: [{
host: host,
http: {
paths: [{
path: '/',
pathType: 'Prefix',
backend: {
service: {
name: service,
port: {
name: port
}
}
}
}]
}
}]
}.merge(tls: tls_options))
end
end

def build_proxy_routes(entry)
build_routes('zync-3scale-api-', [
RouteSpec.new(entry.data.fetch('endpoint'), 'apicast-production', 'gateway'),
RouteSpec.new(entry.data.fetch('sandbox_endpoint'), 'apicast-staging', 'gateway')
], labels: labels_for_proxy(entry), annotations: annotations_for(entry))
end

def build_proxy_ingresses(entry)
data = entry.data
tenant_id = String(entry.tenant_id)
build_ingresses('zync-3scale-api-', [
IngressSpec.new(data.fetch('endpoint'), 'apicast-production', 'gateway', tenant_id),
IngressSpec.new(data.fetch('sandbox_endpoint'), 'apicast-staging', 'gateway', tenant_id)
], labels: labels_for_proxy(entry), annotations: annotations_for(entry))
end

def build_routes(name, specs = [], owner: get_owner, **metadata)
specs.map do |spec|
Route.new(
Expand All @@ -178,6 +238,27 @@ def build_routes(name, specs = [], owner: get_owner, **metadata)
end
end

def build_ingresses(name, specs = [], owner: get_owner, **metadata)
specs.map do |spec|
Ingress.new(
metadata: {
generateName: name,
namespace: namespace,
labels: owner.metadata.labels,
ownerReferences: [as_reference(owner)]
}.deep_merge(metadata.deep_merge(
labels: {
'zync.3scale.net/route-to': spec.to_h.dig(:rules, 0, :http, :paths, 0, :backend, :service, :name),
},
annotations: {
'zync.3scale.net/host': spec.host,
}
)),
spec: spec
)
end
end

def build_provider_routes(entry)
data = entry.data
domain, admin_domain = data.values_at('domain', 'admin_domain')
Expand All @@ -195,6 +276,24 @@ def build_provider_routes(entry)
end
end

def build_provider_ingresses(entry)
data = entry.data
domain, admin_domain = data.values_at('domain', 'admin_domain')
metadata = { labels: labels_for_provider(entry), annotations: annotations_for(entry) }
tenant_id = String(entry.tenant_id)

if admin_domain == domain # master account
build_ingresses('zync-3scale-master-', [
IngressSpec.new(data.fetch('domain'), 'system-master', 'http', tenant_id)
], **metadata)
else
build_ingresses('zync-3scale-provider-', [
IngressSpec.new(data.fetch('domain'), 'system-developer', 'http', tenant_id),
IngressSpec.new(data.fetch('admin_domain'), 'system-provider', 'http', tenant_id)
], **metadata)
end
end

def cleanup_but(list, label_selector)
client
.client_for_resource(list.first, namespace: namespace)
Expand All @@ -219,7 +318,6 @@ def extract_route_patch(resource)
existing = client
.client_for_resource(resource, namespace: namespace)
.list(labelSelector: label_selector_from(resource))

client.get_resource case existing.size
when 0
client.create_resource(resource)
Expand Down Expand Up @@ -279,26 +377,40 @@ def update_resource(existing, resource)
end

def persist_proxy(entry)
routes = build_proxy_routes(entry)

cleanup_routes persist_resources(routes)
if use_openshift_route?
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we could create two separate subclasses: one for OpenShift service, and another one for plain Kubernetes, and override the methods where the if use_openshift_route? check is currently done.
Then maybe we could select the correct service in DiscoverIntegrationService...

Choose a reason for hiding this comment

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

I agree with @mayorova.
Adding this use_openshift_route option forced us to add ifs on multiple methods that that now have to have a different behaviour depending on this new option.
Rather than doing that, since there are multiple changes to be done, we could define a new class with all the behaviour we expect when we talk about OpenShift routes and leave this one to care about Kubernetes.
With this, we'd have different points in code responsible for each scenario.

Copy link
Author

Choose a reason for hiding this comment

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

I had originally intended to do it this way but it required me to touch more of the code base than I was comfortable with. @mayorova Could you maybe elaborate or give an example of how I would select Openshift vs Kubernetes inside DiscoverIntegrationService?

routes = build_proxy_routes(entry)
cleanup_routes persist_resources(routes)
else
ingresses = build_proxy_ingresses(entry)
persist_resources(ingresses)
end
end

def delete_proxy(entry)
label_selector = labels_for_proxy(entry)

cleanup_but([Route.new({})], label_selector)
if use_openshift_route?
cleanup_but([Route.new({})], label_selector)
else
cleanup_but([Ingress.new({})], label_selector)
end
end

def persist_provider(entry)
routes = build_provider_routes(entry)

cleanup_routes persist_resources(routes)
if use_openshift_route?
routes = build_provider_routes(entry)
cleanup_routes persist_resources(routes)
else
ingresses = build_provider_ingresses(entry)
persist_resources(ingresses)
end
end

def delete_provider(entry)
label_selector = labels_for_provider(entry)

cleanup_but([Route.new({})], label_selector)
if use_openshift_route?
cleanup_but([Route.new({})], label_selector)
else
cleanup_but([Ingress.new({})], label_selector)
end
end
end
3 changes: 3 additions & 0 deletions config/integrations.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
shared: &shared
kubernetes: <%= ENV.fetch('DISABLE_K8S_ROUTES_CREATION', '0') == '0' %>
kubernetes_force_native_ingress: <%= ActiveModel::Type::Boolean.new.cast(ENV['FORCE_NATIVE_INGRESS']) %>
kubernetes_ingress_class: <%= ENV.fetch('INGRESS_CLASS', 'nginx') %>

production:
<<: *shared
Expand All @@ -9,3 +11,4 @@ development:

test:
<<: *shared

142 changes: 140 additions & 2 deletions test/services/kubernetes_service_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ def after_teardown
end

setup do
Integration::KubernetesService.use_openshift_route = nil
Rails.application.config.integrations[:kubernetes_force_native_ingress] = false

ENV['KUBERNETES_NAMESPACE'] = 'zync'
ENV['KUBE_TOKEN'] = strict_encode64('token')
ENV['KUBE_SERVER'] = 'http://localhost'
Expand All @@ -33,12 +36,27 @@ def after_teardown
-----END CERTIFICATE-----
CERTIFICATE

@service = Integration::KubernetesService.new(nil)
stub_request(:get, 'http://localhost/apis').
with(
headers: {
'Accept'=>'application/json',
'Authorization'=>'Bearer token',
}).
to_return(status: 200, body: {
kind: "APIGroupList",
apiVersion: "v1",
groups: [
{ name: "networking.k8s.io", versions: [{ groupVersion: "networking.k8s.io/v1", version: "v1" }], preferredVersion: { groupVersion: "networking.k8s.io/v1", version: "v1" } },
{ name: "route.openshift.io", versions: [{ groupVersion: "route.openshift.io/v1", version: "v1" }], preferredVersion: { groupVersion: "route.openshift.io/v1", version: "v1" } },
]
}.to_json, headers: { 'Content-Type' => 'application/json' })
akostadinov marked this conversation as resolved.
Show resolved Hide resolved

@service = Integration::KubernetesService.new(nil).freeze
end

attr_reader :service

test 'create ingress' do
test 'create openshift route' do
proxy = entries(:proxy)

stub_request(:get, 'http://localhost/apis/route.openshift.io/v1').
Expand Down Expand Up @@ -302,6 +320,103 @@ class RouteSpec < ActiveSupport::TestCase
end
end

test 'create native ingress if not on openshift' do
stub_request(:get, 'http://localhost/apis').
with(headers: request_headers).
to_return(status: 200, body: {
kind: "APIGroupList",
apiVersion: "v1",
groups: [
{ name: "networking.k8s.io", versions: [{ groupVersion: "networking.k8s.io/v1", version: "v1" }], preferredVersion: { groupVersion: "networking.k8s.io/v1", version: "v1" } },
]
}.to_json, headers: { 'Content-Type' => 'application/json' })

proxy = entries(:proxy)

stub_request(:get, 'http://localhost/apis/networking.k8s.io/v1').
with(headers: request_headers).
to_return(status: 200, body: k8s_networking_resource_list_response, headers: { 'Content-Type' => 'application/json' })

stub_request(:get, 'http://localhost/apis/networking.k8s.io/v1/namespaces/zync/ingresses?labelSelector=3scale.net/created-by=zync,3scale.net/tenant_id=298486374,zync.3scale.net/record=Z2lkOi8venluYy9Qcm94eS8yOTg0ODYzNzQ,zync.3scale.net/ingress=proxy,3scale.net/service_id=2').
with(headers: request_headers).
to_return(status: 200, body: k8s_ingress_list_response, headers: { 'Content-Type' => 'application/json' })

service.call(proxy)
end

test 'create native ingress if kubernetes_force_native_ingress' do
Rails.application.config.integrations[:kubernetes_force_native_ingress] = true

proxy = entries(:proxy)

stub_request(:get, 'http://localhost/apis/networking.k8s.io/v1').
with(headers: request_headers).
to_return(status: 200, body: k8s_networking_resource_list_response, headers: { 'Content-Type' => 'application/json' })

stub_request(:get, 'http://localhost/apis/networking.k8s.io/v1/namespaces/zync/ingresses?labelSelector=3scale.net/created-by=zync,3scale.net/tenant_id=298486374,zync.3scale.net/record=Z2lkOi8venluYy9Qcm94eS8yOTg0ODYzNzQ,zync.3scale.net/ingress=proxy,3scale.net/service_id=2').
with(headers: request_headers).
to_return(status: 200, body: k8s_ingress_list_response, headers: { 'Content-Type' => 'application/json' })

service.call(proxy)
end

class IngressSpec < ActiveSupport::TestCase
test 'secure ingresses' do
url = 'https://my-api.example.com'
service_name = 'My API'
port = 7443
tenant_id = '2'
spec = Integration::KubernetesService::IngressSpec.new(url, service_name, port, tenant_id)
json = {
ingressClassName: "nginx",
rules: [{
host: "my-api.example.com",
http: {
paths: [{ path: '/', pathType: 'Prefix', backend: { service: { name: service_name, port: { name: port } } } }]
}
}],
tls: [{hosts: ["my-api.example.com"], secretName: "My API-tls-2"}]
}
assert_equal json, spec.to_hash

url = 'http://my-api.example.com'
service_name = 'My API'
port = 7780
tenant_id = '2'
spec = Integration::KubernetesService::IngressSpec.new(url, service_name, port, tenant_id)
json = {
ingressClassName: "nginx",
rules: [{
host: "my-api.example.com",
http: {
paths: [{ path: '/', pathType: 'Prefix', backend: { service: { name: service_name, port: { name: port } } } }]
}
}],
tls: nil
}
assert_equal json, spec.to_hash
end

test 'defaults to https when scheme is missing' do
url = 'my-api.example.com'
service_name = 'My API'
port = 7443
tenant_id = '2'
spec = Integration::KubernetesService::IngressSpec.new(url, service_name, port, tenant_id)
json = {
ingressClassName: "nginx",
rules: [{
host: "my-api.example.com",
http: {
paths: [{ path: '/', pathType: 'Prefix', backend: { service: { name: service_name, port: { name: port } } } }]
}
}],
tls: [{hosts: ["my-api.example.com"], secretName: "My API-tls-2"}]
}
assert_equal json, spec.to_hash
end
end

protected

def request_headers
Expand All @@ -315,4 +430,27 @@ def request_headers
def response_headers
{ 'Content-Type' => 'application/json' }
end

def k8s_networking_resource_list_response
{
kind: "APIResourceList",
apiVersion: "v1",
groupVersion: "networking.k8s.io/v1",
resources: [
{ name: "ingressclasses", singularName: "", namespaced: false, kind: "IngressClass", verbs: ["create","delete","deletecollection","get","list","patch","update","watch"], storageVersionHash: "6upRfBq0FOI=" },
{ name: "ingresses", singularName: "", namespaced: true, kind: "Ingress", verbs: ["create","delete","deletecollection","get","list","patch","update","watch"], shortNames: ["ing"], storageVersionHash: "ZOAfGflaKd0=" },
{ name: "ingresses/status", singularName: "", namespaced:true, kind: "Ingress", verbs: ["get","patch","update"] },
{ name: "networkpolicies", singularName: "", namespaced: true, kind: "NetworkPolicy", verbs: ["create","delete","deletecollection","get","list","patch","update","watch"], shortNames: ["netpol"], storageVersionHash: "YpfwF18m1G8=" },
]
}.to_json
end

def k8s_ingress_list_response
{
kind: 'IngressList',
apiVersion: 'networking.k8s.io/v1',
metadata: { selfLink: '/apis/networking.k8s.io/v1/namespaces/zync/ingresses', resourceVersion: '651341' },
items: []
}.to_json
end
end