From 1337361882519dcef4ef5778955af0c13553bf0b Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Tue, 5 Mar 2024 16:12:44 -0500 Subject: [PATCH 1/7] Add specs covering K8s `.verify_credentials` --- .../kubernetes/container_manager_spec.rb | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb b/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb index ac5fac23ec..a39dd4e38a 100644 --- a/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb +++ b/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb @@ -277,6 +277,72 @@ end end + describe ".verify_credentials" do + let(:zone) { EvmSpecHelper.create_guid_miq_server_zone.last } + let(:params) { {"name" => "k8s", "zone" => zone, "endpoints" => endpoints, "authentications" => authentications} } + let(:endpoints) { {} } + let(:authentications) { {} } + + context "with a default endpoint" do + require "kubeclient" + + let(:kubeclient_client) { double("Kubeclient::Client") } + let(:endpoints) { {"default" => {"role" => "default", "hostname" => "kubernetes.local", "port" => 6443, "security_protocol" => "ssl-with-validation"}} } + let(:authentications) { {"bearer" => {"authtype" => "bearer", "auth_key" => "super secret"}} } + + before do + allow(kubeclient_client).to receive(:discover) + allow(Kubeclient::Client).to receive(:new).with(instance_of(URI::HTTPS), "v1", anything).and_return(kubeclient_client) + end + + it "returns true if the parameters are valid" do + expect(kubeclient_client).to receive(:api_valid?).and_return(true) + expect(kubeclient_client).to receive(:get_namespaces).with(:limit => 1).and_return([Kubeclient::Resource.new]) + expect(described_class.verify_credentials(params)).to be_truthy + end + + it "returns false if the parameters aren't valid" do + expect(kubeclient_client).to receive(:api_valid?).and_return(false) + expect(described_class.verify_credentials(params)).to be_falsey + end + end + + context "with a metrics endpoint" do + require "prometheus/api_client" + + let(:endpoints) { {"prometheus" => {"role" => "prometheus", "hostname" => "prometheus.kubernetes.local", "port" => 443, "security_protocol" => "ssl-with-validation"}} } + let(:authentications) { {"bearer" => {"authtype" => "bearer", "auth_key" => "super secret"}} } + let(:prometheus_api_client) { double("Prometheus::ApiClient") } + + before do + allow(Prometheus::ApiClient).to receive(:client).with(anything).and_return(prometheus_api_client) + end + + it "returns true if the parameters are valid" do + expect(prometheus_api_client).to receive(:query).with(:query => "ALL").and_return({}) + expect(described_class.verify_credentials(params)).to be_truthy + end + + it "returns false if the parameters aren't valid" do + expect(prometheus_api_client).to receive(:query).with(:query => "ALL").and_return(nil) + expect(described_class.verify_credentials(params)).to be_falsey + end + end + + context "with a virtualization endpoint" do + let(:endpoints) { {"kubevirt" => {"role" => "kubevirt", "hostname" => "kubevirt.kubernetes.local", "port" => 443, "security_protocol" => "ssl-with-validation"}} } + let(:authentications) { {"bearer" => {"authtype" => "bearer", "auth_key" => "super secret"}} } + + it "returns true if the parameters are valid" do + expect(ManageIQ::Providers::Kubevirt::InfraManager) + .to receive(:verify_credentials) + .with("endpoints" => {"default" => {"server" => "kubevirt.kubernetes.local", "port" => 443, "token" => "super secret"}}) + .and_return(true) + expect(described_class.verify_credentials(params)).to be_truthy + end + end + end + describe "hostname_uniqueness_valid?" do it "allows duplicate hostname with different ports" do FactoryBot.create(:ems_kubernetes, :hostname => "k8s.local", :port => 6443) From 7835013578409a52917ed9a11237c1c5fa2334a1 Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Tue, 5 Mar 2024 16:30:11 -0500 Subject: [PATCH 2/7] Add checks for default endpoint ssl verify options --- .../kubernetes/container_manager_spec.rb | 65 ++++++++++++++++--- 1 file changed, 57 insertions(+), 8 deletions(-) diff --git a/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb b/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb index a39dd4e38a..267ac0bbf9 100644 --- a/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb +++ b/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb @@ -287,23 +287,72 @@ require "kubeclient" let(:kubeclient_client) { double("Kubeclient::Client") } - let(:endpoints) { {"default" => {"role" => "default", "hostname" => "kubernetes.local", "port" => 6443, "security_protocol" => "ssl-with-validation"}} } + let(:endpoints) { {"default" => {"role" => "default", "hostname" => "kubernetes.local", "port" => 6443, "security_protocol" => security_protocol}} } let(:authentications) { {"bearer" => {"authtype" => "bearer", "auth_key" => "super secret"}} } + let(:security_protocol) { "ssl-with-validation" } before do allow(kubeclient_client).to receive(:discover) allow(Kubeclient::Client).to receive(:new).with(instance_of(URI::HTTPS), "v1", anything).and_return(kubeclient_client) end - it "returns true if the parameters are valid" do - expect(kubeclient_client).to receive(:api_valid?).and_return(true) - expect(kubeclient_client).to receive(:get_namespaces).with(:limit => 1).and_return([Kubeclient::Resource.new]) - expect(described_class.verify_credentials(params)).to be_truthy + context "with valid parameters" do + before do + expect(kubeclient_client).to receive(:api_valid?).and_return(true) + expect(kubeclient_client).to receive(:get_namespaces).with(:limit => 1).and_return([Kubeclient::Resource.new]) + end + + it "returns true" do + expect(described_class.verify_credentials(params)).to be_truthy + end + + it "verifies ssl" do + expect(Kubeclient::Client).to receive(:new).with(instance_of(URI::HTTPS), "v1", hash_including(:ssl_options => hash_including(:verify_ssl => OpenSSL::SSL::VERIFY_PEER))).and_return(kubeclient_client) + + described_class.verify_credentials(params) + end + + context "with security_protocol=ssl-without-validation" do + let(:security_protocol) { "ssl-without-validation" } + + it "doesn't verify ssl" do + expect(Kubeclient::Client).to receive(:new).with(instance_of(URI::HTTPS), "v1", hash_including(:ssl_options => hash_including(:verify_ssl => OpenSSL::SSL::VERIFY_NONE))).and_return(kubeclient_client) + + described_class.verify_credentials(params) + end + end + + context "with security_protocol=sssl-with-validation-custom-ca" do + let(:security_protocol) { "ssl-with-validation-custom-ca" } + let(:endpoints) { {"default" => {"role" => "default", "hostname" => "kubernetes.local", "port" => 6443, "security_protocol" => security_protocol, "certificate_authority" => certificate_authority}} } + let(:certificate_authority) { "-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----" } + + it "verifies ssl with a custom certificate_authority" do + expect(Kubeclient::Client) + .to receive(:new) + .with( + instance_of(URI::HTTPS), + "v1", + hash_including(:ssl_options => hash_including(:verify_ssl => OpenSSL::SSL::VERIFY_PEER, :ca_file => certificate_authority)) + ) + .and_return(kubeclient_client) + + described_class.verify_credentials(params) + end + end end - it "returns false if the parameters aren't valid" do - expect(kubeclient_client).to receive(:api_valid?).and_return(false) - expect(described_class.verify_credentials(params)).to be_falsey + context "with invalid parameters" do + it "returns false if the api isn't valid" do + expect(kubeclient_client).to receive(:api_valid?).and_return(false) + expect(described_class.verify_credentials(params)).to be_falsey + end + + it "returns false if no namespaces can be retrieved" do + expect(kubeclient_client).to receive(:api_valid?).and_return(true) + expect(kubeclient_client).to receive(:get_namespaces).with(:limit => 1).and_return(nil) + expect(described_class.verify_credentials(params)).to be_falsey + end end end From 603307f10ae381f768fb9f0aa39c0843513076bd Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Wed, 6 Mar 2024 12:24:44 -0500 Subject: [PATCH 3/7] Add default endpoint proxy specs --- .../kubernetes/container_manager_spec.rb | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb b/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb index 267ac0bbf9..2efaab6059 100644 --- a/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb +++ b/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb @@ -340,6 +340,30 @@ described_class.verify_credentials(params) end end + + context "with no http_proxy set in Settings" do + before do + stub_settings_merge(:http_proxy => {:host => nil}) + end + + it "doesn't pass a proxy to Kubeclient" do + expect(Kubeclient::Client).to receive(:new).with(instance_of(URI::HTTPS), "v1", hash_including(:http_proxy_uri => nil)).and_return(kubeclient_client) + + described_class.verify_credentials(params) + end + end + + context "with http_proxy set in Settings" do + before do + stub_settings_merge(:http_proxy => {:host => "proxy.local"}) + end + + it "passes the proxy info to Kubeclient" do + expect(Kubeclient::Client).to receive(:new).with(instance_of(URI::HTTPS), "v1", hash_including(:http_proxy_uri => instance_of(URI::Generic))).and_return(kubeclient_client) + + described_class.verify_credentials(params) + end + end end context "with invalid parameters" do From 0adac1d170c352a033c165178c8e7831493c538b Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Wed, 6 Mar 2024 12:31:45 -0500 Subject: [PATCH 4/7] Add specs for http_proxy for metrics endpoint --- .../kubernetes/container_manager_spec.rb | 73 +++++++++++++++++-- 1 file changed, 67 insertions(+), 6 deletions(-) diff --git a/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb b/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb index 2efaab6059..94cbf9b328 100644 --- a/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb +++ b/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb @@ -391,14 +391,75 @@ allow(Prometheus::ApiClient).to receive(:client).with(anything).and_return(prometheus_api_client) end - it "returns true if the parameters are valid" do - expect(prometheus_api_client).to receive(:query).with(:query => "ALL").and_return({}) - expect(described_class.verify_credentials(params)).to be_truthy + context "with valid parameters" do + before do + expect(prometheus_api_client).to receive(:query).with(:query => "ALL").and_return({}) + end + + it "returns true" do + expect(described_class.verify_credentials(params)).to be_truthy + end + + it "verifies ssl" do + expect(Prometheus::ApiClient) + .to receive(:client) + .with( + :url => "https://prometheus.kubernetes.local", + :credentials => {:token => "super secret"}, + :options => hash_including(:verify_ssl => OpenSSL::SSL::VERIFY_PEER) + ) + .and_return(prometheus_api_client) + + described_class.verify_credentials(params) + end + + context "with no http_proxy set in Settings" do + before do + stub_settings_merge(:http_proxy => {:host => nil}) + end + + it "doesn't set an http_proxy_uri" do + expect(Prometheus::ApiClient) + .to receive(:client) + .with( + :url => "https://prometheus.kubernetes.local", + :credentials => {:token => "super secret"}, + :options => hash_including(:http_proxy_uri => nil) + ) + .and_return(prometheus_api_client) + + described_class.verify_credentials(params) + end + end + + context "with an http_proxy set in Settings" do + before do + stub_settings_merge(:http_proxy => {:host => "proxy.local"}) + end + + it "sets an http_proxy_uri" do + expect(Prometheus::ApiClient) + .to receive(:client) + .with( + :url => "https://prometheus.kubernetes.local", + :credentials => {:token => "super secret"}, + :options => hash_including(:http_proxy_uri => "http://proxy.local") + ) + .and_return(prometheus_api_client) + + described_class.verify_credentials(params) + end + end end - it "returns false if the parameters aren't valid" do - expect(prometheus_api_client).to receive(:query).with(:query => "ALL").and_return(nil) - expect(described_class.verify_credentials(params)).to be_falsey + context "with invalid parameters" do + before do + expect(prometheus_api_client).to receive(:query).with(:query => "ALL").and_return(nil) + end + + it "returns false" do + expect(described_class.verify_credentials(params)).to be_falsey + end end end From e3a69ee9f9f3b64a04cb1bbbb437e9209e3b6882 Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Wed, 6 Mar 2024 12:36:21 -0500 Subject: [PATCH 5/7] Add ssl specs for metrics endpoint --- .../kubernetes/container_manager_spec.rb | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb b/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb index 94cbf9b328..122c0b638c 100644 --- a/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb +++ b/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb @@ -383,9 +383,10 @@ context "with a metrics endpoint" do require "prometheus/api_client" - let(:endpoints) { {"prometheus" => {"role" => "prometheus", "hostname" => "prometheus.kubernetes.local", "port" => 443, "security_protocol" => "ssl-with-validation"}} } + let(:endpoints) { {"prometheus" => {"role" => "prometheus", "hostname" => "prometheus.kubernetes.local", "port" => 443, "security_protocol" => security_protocol}} } let(:authentications) { {"bearer" => {"authtype" => "bearer", "auth_key" => "super secret"}} } let(:prometheus_api_client) { double("Prometheus::ApiClient") } + let(:security_protocol) { "ssl-with-validation" } before do allow(Prometheus::ApiClient).to receive(:client).with(anything).and_return(prometheus_api_client) @@ -413,6 +414,45 @@ described_class.verify_credentials(params) end + context "with security_protocol=ssl-without-validation" do + let(:security_protocol) { "ssl-without-validation" } + + it "doesn't verify ssl" do + expect(Prometheus::ApiClient) + .to receive(:client) + .with( + :url => "https://prometheus.kubernetes.local", + :credentials => {:token => "super secret"}, + :options => hash_including(:verify_ssl => OpenSSL::SSL::VERIFY_NONE) + ) + .and_return(prometheus_api_client) + + described_class.verify_credentials(params) + end + end + + context "with security_protocol=sssl-with-validation-custom-ca" do + let(:security_protocol) { "ssl-with-validation-custom-ca" } + let(:endpoints) { {"prometheus" => {"role" => "prometheus", "hostname" => "prometheus.kubernetes.local", "port" => 443, "security_protocol" => security_protocol, "certificate_authority" => certificate_authority}} } + let(:certificate_authority) { "-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----" } + + it "verifies ssl with a custom certificate_authority" do + expect(Prometheus::ApiClient) + .to receive(:client) + .with( + :url => "https://prometheus.kubernetes.local", + :credentials => {:token => "super secret"}, + :options => hash_including( + :verify_ssl => OpenSSL::SSL::VERIFY_PEER, + :ssl_cert_store => certificate_authority + ) + ) + .and_return(prometheus_api_client) + + described_class.verify_credentials(params) + end + end + context "with no http_proxy set in Settings" do before do stub_settings_merge(:http_proxy => {:host => nil}) From dba6a4890a66f08e89b1d8d508551701a8fe4f54 Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Wed, 6 Mar 2024 12:40:13 -0500 Subject: [PATCH 6/7] Add invalid params specs for kubevirt endpoint --- .../providers/kubernetes/container_manager_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb b/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb index 122c0b638c..0b80f5480f 100644 --- a/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb +++ b/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb @@ -514,6 +514,14 @@ .and_return(true) expect(described_class.verify_credentials(params)).to be_truthy end + + it "returns false if the parameters are invalid" do + expect(ManageIQ::Providers::Kubevirt::InfraManager) + .to receive(:verify_credentials) + .with("endpoints" => {"default" => {"server" => "kubevirt.kubernetes.local", "port" => 443, "token" => "super secret"}}) + .and_return(false) + expect(described_class.verify_credentials(params)).to be_falsey + end end end From 5997bee994ba424f9b190c152f6c73b9c5a7e70c Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Wed, 6 Mar 2024 14:01:54 -0500 Subject: [PATCH 7/7] Replace instance_of(URI::HTTPS) with URI::HTTPS.build() --- .../kubernetes/container_manager_spec.rb | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb b/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb index 0b80f5480f..443c61b46a 100644 --- a/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb +++ b/spec/models/manageiq/providers/kubernetes/container_manager_spec.rb @@ -292,8 +292,10 @@ let(:security_protocol) { "ssl-with-validation" } before do + expected_uri = URI::HTTPS.build(:host => "kubernetes.local", :port => 6443) + allow(kubeclient_client).to receive(:discover) - allow(Kubeclient::Client).to receive(:new).with(instance_of(URI::HTTPS), "v1", anything).and_return(kubeclient_client) + allow(Kubeclient::Client).to receive(:new).with(expected_uri, "v1", anything).and_return(kubeclient_client) end context "with valid parameters" do @@ -307,7 +309,8 @@ end it "verifies ssl" do - expect(Kubeclient::Client).to receive(:new).with(instance_of(URI::HTTPS), "v1", hash_including(:ssl_options => hash_including(:verify_ssl => OpenSSL::SSL::VERIFY_PEER))).and_return(kubeclient_client) + expected_uri = URI::HTTPS.build(:host => "kubernetes.local", :port => 6443) + expect(Kubeclient::Client).to receive(:new).with(expected_uri, "v1", hash_including(:ssl_options => hash_including(:verify_ssl => OpenSSL::SSL::VERIFY_PEER))).and_return(kubeclient_client) described_class.verify_credentials(params) end @@ -316,7 +319,8 @@ let(:security_protocol) { "ssl-without-validation" } it "doesn't verify ssl" do - expect(Kubeclient::Client).to receive(:new).with(instance_of(URI::HTTPS), "v1", hash_including(:ssl_options => hash_including(:verify_ssl => OpenSSL::SSL::VERIFY_NONE))).and_return(kubeclient_client) + expected_uri = URI::HTTPS.build(:host => "kubernetes.local", :port => 6443) + expect(Kubeclient::Client).to receive(:new).with(expected_uri, "v1", hash_including(:ssl_options => hash_including(:verify_ssl => OpenSSL::SSL::VERIFY_NONE))).and_return(kubeclient_client) described_class.verify_credentials(params) end @@ -328,10 +332,12 @@ let(:certificate_authority) { "-----BEGIN CERTIFICATE-----\n-----END CERTIFICATE-----" } it "verifies ssl with a custom certificate_authority" do + expected_uri = URI::HTTPS.build(:host => "kubernetes.local", :port => 6443) + expect(Kubeclient::Client) .to receive(:new) .with( - instance_of(URI::HTTPS), + expected_uri, "v1", hash_including(:ssl_options => hash_including(:verify_ssl => OpenSSL::SSL::VERIFY_PEER, :ca_file => certificate_authority)) ) @@ -347,7 +353,8 @@ end it "doesn't pass a proxy to Kubeclient" do - expect(Kubeclient::Client).to receive(:new).with(instance_of(URI::HTTPS), "v1", hash_including(:http_proxy_uri => nil)).and_return(kubeclient_client) + expected_uri = URI::HTTPS.build(:host => "kubernetes.local", :port => 6443) + expect(Kubeclient::Client).to receive(:new).with(expected_uri, "v1", hash_including(:http_proxy_uri => nil)).and_return(kubeclient_client) described_class.verify_credentials(params) end @@ -359,7 +366,9 @@ end it "passes the proxy info to Kubeclient" do - expect(Kubeclient::Client).to receive(:new).with(instance_of(URI::HTTPS), "v1", hash_including(:http_proxy_uri => instance_of(URI::Generic))).and_return(kubeclient_client) + expected_uri = URI::HTTPS.build(:host => "kubernetes.local", :port => 6443) + expected_proxy_uri = URI::Generic.build(:scheme => "http", :host => "proxy.local") + expect(Kubeclient::Client).to receive(:new).with(expected_uri, "v1", hash_including(:http_proxy_uri => expected_proxy_uri)).and_return(kubeclient_client) described_class.verify_credentials(params) end