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

Add api specs for prometheus endpoints #295

Merged
merged 2 commits into from
Jan 23, 2018
Merged
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
86 changes: 75 additions & 11 deletions spec/requests/providers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
"port" => 18_443,
"hostname" => "sample-openshift.provider.com",
"ipaddress" => "100.200.300.3",
"security_protocol" => "something",
"security_protocol" => "ssl-without-validation",
"certificate_authority" => certificate_authority,
}
end
Expand All @@ -93,7 +93,7 @@
"role" => "default",
"hostname" => "sample-openshift-multi-end-point.provider.com",
"port" => 18_443,
"security_protocol" => "something",
"security_protocol" => "ssl-without-validation",
"certificate_authority" => certificate_authority,
},
"authentication" => {
Expand All @@ -108,7 +108,7 @@
"role" => "default",
"hostname" => "sample-openshift-multi-end-point.provider.com",
"port" => "28443",
"security_protocol" => "something else",
"security_protocol" => "ssl-without-validation",
"certificate_authority" => certificate_authority,
},
"authentication" => {
Expand All @@ -123,7 +123,7 @@
"role" => "hawkular",
"hostname" => "sample-openshift-multi-end-point.provider.com",
"port" => 1_443,
"security_protocol" => "something",
"security_protocol" => "ssl-without-validation",
"certificate_authority" => certificate_authority,
},
"authentication" => {
Expand All @@ -132,13 +132,48 @@
}
}
end
let(:sample_containers_multi_end_point) do
let(:prometheus_connection) do
{
"endpoint" => {
"role" => "prometheus",
"hostname" => "prometheus.example.com",
"port" => 443,
"security_protocol" => "ssl-without-validation"
},
"authentication" => {
"role" => "prometheus",
"auth_key" => SecureRandom.hex
}
}
end
let(:prometheus_alerts_connection) do
{
"endpoint" => {
"role" => "prometheus_alerts",
"hostname" => "prometheus.example.com",
"port" => 443,
"security_protocol" => "ssl-without-validation"
},
"authentication" => {
"role" => "prometheus_alerts",
"auth_key" => SecureRandom.hex
}
}
end
let(:sample_containers_multi_end_point_with_hawkular) do
{
"type" => containers_class,
"name" => "sample containers provider with multiple endpoints",
"name" => "sample containers provider with multiple endpoints and hawkular",
"connection_configurations" => [default_connection, hawkular_connection]
}
end
let(:sample_containers_multi_end_point_with_prometheus) do
{
"type" => containers_class,
"name" => "sample containers provider with multiple endpoints and prometheus",
"connection_configurations" => [prometheus_alerts_connection, default_connection, prometheus_connection]
}
end

def have_endpoint_attributes(expected_hash)
h = expected_hash.slice(*ENDPOINT_ATTRS)
Expand Down Expand Up @@ -573,15 +608,15 @@ def token(connection)
connection["authentication"]["auth_key"]
end

it "supports provider with multiple endpoints creation" do
it "supports provider with multiple endpoints creation with hawkular" do
api_basic_authorize collection_action_identifier(:providers, :create)

post(api_providers_url, :params => gen_request(:create, sample_containers_multi_end_point))
post(api_providers_url, :params => gen_request(:create, sample_containers_multi_end_point_with_hawkular))

expect(response).to have_http_status(:ok)
expected = {"id" => a_kind_of(String),
"type" => containers_class,
"name" => "sample containers provider with multiple endpoints"}
"name" => "sample containers provider with multiple endpoints and hawkular"}

results = response.parsed_body["results"]
expect(results.first).to include(expected)
Expand All @@ -596,6 +631,35 @@ def token(connection)
)
expect(provider.authentication_token(:hawkular)).to eq(token(hawkular_connection))
end

it "supports provider with multiple endpoints creation and prometheus" do
api_basic_authorize collection_action_identifier(:providers, :create)

post(api_providers_url, :params => gen_request(:create, sample_containers_multi_end_point_with_prometheus))

expect(response).to have_http_status(:ok)
expected = {"id" => a_kind_of(String),
"type" => containers_class,
"name" => "sample containers provider with multiple endpoints and prometheus"}

results = response.parsed_body["results"]
expect(results.first).to include(expected)

provider_id = results.first["id"]
provider = ExtManagementSystem.find(provider_id)
expect(provider).to have_endpoint_attributes(default_connection["endpoint"])
expect(provider.authentication_token).to eq(token(default_connection))

expect(provider.connection_configurations.prometheus.endpoint).to have_endpoint_attributes(
prometheus_connection["endpoint"]
)

expect(provider.connection_configurations.prometheus_alerts.endpoint).to have_endpoint_attributes(
prometheus_alerts_connection["endpoint"]
)
expect(provider.authentication_token(:prometheus)).to eq(token(prometheus_connection))
expect(provider.authentication_token(:prometheus_alerts)).to eq(token(prometheus_alerts_connection))
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 not sure this tested what we wanted to test.
AFAICT it only checks parent endpoints got created correctly; we mainly want to test child manager got created (and can see the endpoint it needs).

Consider also adding test for deletion of prometheus endpoint and maybe of whole provider resulting in child manager deletion.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cben testing the child manager got created should probably not be done in the API tests. If I understand correctly, API tests are supposed to test the interface, not the implementation, and the child manager is definitely an implementation detail.

Copy link
Author

Choose a reason for hiding this comment

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

I was thinking of it doesnt blow up type tests

Should the sub-manager logic go away(which i feel it might) then we will need to change these spec tests so endpoint creation is enough IMO.

Copy link
Contributor

Choose a reason for hiding this comment

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

API tests are supposed to test the interface, not the implementation, and the child manager is definitely an implementation detail.

Well, I really want an integration test that implementation reacted correctly to API somewhere.
Do you see a better place/way to test it?
OK, in principle, if we have a test in our repo that directly creating parent endpoint creates the child (do we?), and test that API creates endpoints, then in theory we're covered. I have low trust for theory in this case though :-)

I was thinking of it doesnt blow up type tests

Hmm, yes, for this specific failure it was blowing up, that's enough.

end
end
end
end
Expand Down Expand Up @@ -683,7 +747,7 @@ def token(connection)
it "does not schedule a new credentials check if endpoint does not change" do
api_basic_authorize collection_action_identifier(:providers, :edit)

provider = FactoryGirl.create(:ext_management_system, sample_containers_multi_end_point)
provider = FactoryGirl.create(:ext_management_system, sample_containers_multi_end_point_with_hawkular)
MiqQueue.where(:method_name => "authentication_check_types",
:class_name => "ExtManagementSystem",
:instance_id => provider.id).delete_all
Expand All @@ -702,7 +766,7 @@ def token(connection)
it "schedules a new credentials check if endpoint change" do
api_basic_authorize collection_action_identifier(:providers, :edit)

provider = FactoryGirl.create(:ext_management_system, sample_containers_multi_end_point)
provider = FactoryGirl.create(:ext_management_system, sample_containers_multi_end_point_with_hawkular)
MiqQueue.where(:method_name => "authentication_check_types",
:class_name => "ExtManagementSystem",
:instance_id => provider.id).delete_all
Expand Down