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

Enable/disable charts feature #2147

Merged
merged 7 commits into from
Jan 9, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
16 changes: 16 additions & 0 deletions assets/js/common/ChartDisabledBox/ChartDisabledBox.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import React from 'react';

function ChartDisabledBox() {
CDimonaco marked this conversation as resolved.
Show resolved Hide resolved
return (
<div
className="mt-4 bg-white shadow rounded-lg py-4 px-8 mx-auto"
data-testid="chart-disabled-box"
>
<h2 className="font-bold text-center text-xl">
Charts are disabled, please check documentation for further details
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe Charts are disabled. Please check the documentation for further details.
Could we point the user to that documentation with a link?
Otherwise it doesn't look like that helpful

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but we don't have the docs yet, so atm I think we could have as is

</h2>
</div>
);
}

export default ChartDisabledBox;
10 changes: 10 additions & 0 deletions assets/js/common/ChartDisabledBox/ChartDisabledBox.stories.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import ChartDisabledBox from '.';

export default {
title: 'Components/ChartDisabledBox',
component: ChartDisabledBox,
};

export const Default = {
args: {},
};
3 changes: 3 additions & 0 deletions assets/js/common/ChartDisabledBox/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import ChartDisabledBox from './ChartDisabledBox';

export default ChartDisabledBox;
10 changes: 10 additions & 0 deletions assets/js/common/ChartFeatureWrapper/ChartFeatureWrapper.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import React from 'react';
import ChartDisabledBox from '@common/ChartDisabledBox';

function ChartFeatureWrapper({ children }) {
CDimonaco marked this conversation as resolved.
Show resolved Hide resolved
// eslint-disable-next-line no-undef
if (!config.chartsEnabled) return <ChartDisabledBox />;
return children;
}

export default ChartFeatureWrapper;
31 changes: 31 additions & 0 deletions assets/js/common/ChartFeatureWrapper/ChartFeatureWrapper.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import React from 'react';
import { render, screen } from '@testing-library/react';
import '@testing-library/jest-dom';
import ChartFeatureWrapper from './ChartFeatureWrapper';

describe('ChartFeatureWrapper', () => {
afterEach(() => {
global.config = { chartsEnabled: false };
});

it('should render ChartDisabledBox if the chart feature is disabled', () => {
global.config = { chartsEnabled: false };

render(<ChartFeatureWrapper />);
expect(screen.getByTestId('chart-disabled-box')).toHaveTextContent(
'disabled'
);
});

it('should render children if the chart feature is enabled', () => {
global.config = { chartsEnabled: true };

render(
<ChartFeatureWrapper>
{' '}
<div data-testid="child">child</div>{' '}
</ChartFeatureWrapper>
);
expect(screen.getByTestId('child')).toHaveTextContent('child');
});
});
3 changes: 3 additions & 0 deletions assets/js/common/ChartFeatureWrapper/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import ChartFeatureWrapper from './ChartFeatureWrapper';

export default ChartFeatureWrapper;
40 changes: 22 additions & 18 deletions assets/js/pages/HostDetailsPage/HostDetails.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import PageHeader from '@common/PageHeader';
import Table from '@common/Table';
import Tooltip from '@common/Tooltip';
import WarningBanner from '@common/Banners/WarningBanner';
import ChartFeatureWrapper from '@common/ChartFeatureWrapper/ChartFeatureWrapper';

import { subHours } from 'date-fns';

import SuseLogo from '@static/suse_logo.svg';
Expand Down Expand Up @@ -209,24 +211,26 @@ function HostDetails({
/>
</div>
</div>
<div>
<HostChart
hostId={hostID}
chartId="cpu"
chartTitle="CPU"
yAxisFormatter={(value) => `${value}%`}
startInterval={subHours(timeNow, 3)}
/>
</div>
<div>
<HostChart
hostId={hostID}
chartId="memory"
chartTitle="Memory"
startInterval={subHours(timeNow, 3)}
yAxisFormatter={(value) => formatBytes(value, 3)}
/>
</div>
<ChartFeatureWrapper>
<div>
<HostChart
hostId={hostID}
chartId="cpu"
chartTitle="CPU"
yAxisFormatter={(value) => `${value}%`}
startInterval={subHours(timeNow, 3)}
/>
</div>
<div>
<HostChart
hostId={hostID}
chartId="memory"
chartTitle="Memory"
startInterval={subHours(timeNow, 3)}
yAxisFormatter={(value) => formatBytes(value, 3)}
/>
</div>
</ChartFeatureWrapper>
<div className="mt-16">
<div className="mb-4">
<h2 className="text-2xl font-bold">Provider details</h2>
Expand Down
4 changes: 3 additions & 1 deletion config/config.exs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,9 @@ config :trento, Trento.Infrastructure.Prometheus,

config :trento, Trento.Infrastructure.Prometheus.PrometheusApi, url: "http://localhost:9090"

config :trento, Trento.Charts, host_data_fetcher: Trento.Infrastructure.Prometheus.PrometheusApi
config :trento, Trento.Charts,
enabled: true,
host_data_fetcher: Trento.Infrastructure.Prometheus.PrometheusApi

config :trento,
uuid_namespace: "fb92284e-aa5e-47f6-a883-bf9469e7a0dc",
Expand Down
2 changes: 2 additions & 0 deletions config/runtime.exs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ if config_env() in [:prod, :demo] do
config :trento, Trento.Infrastructure.Prometheus.PrometheusApi,
url: System.get_env("PROMETHEUS_URL") || "http://localhost:9090"

config :trento, Trento.Charts, enabled: System.get_env("CHARTS_ENABLED", "true") == "true"

# ## Using releases
#
# If you are doing OTP releases, you need to instruct Phoenix
Expand Down
3 changes: 2 additions & 1 deletion lib/trento_web/controllers/page_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ defmodule TrentoWeb.PageController do

def index(conn, _params) do
check_service_base_url = Application.fetch_env!(:trento, :checks_service)[:base_url]

charts_enabled = Application.fetch_env!(:trento, Trento.Charts)[:enabled]
deregistration_debounce = Application.fetch_env!(:trento, :deregistration_debounce)

render(conn, "index.html",
check_service_base_url: check_service_base_url,
charts_enabled: charts_enabled,
deregistration_debounce: deregistration_debounce
)
end
Expand Down
33 changes: 33 additions & 0 deletions lib/trento_web/plugs/charts_disabled_plug.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
defmodule TrentoWeb.Plugs.ChartsDisabledPlug do
@moduledoc """
This plug will act as a barrier for the charts endpoint, will return 501 for all the requests.
CDimonaco marked this conversation as resolved.
Show resolved Hide resolved

The endpoints will be accessible only if the ":trento, Trento.Charts, enabled" configuration entry is properly set.

The plug itself is mounted only when the charts are disabled in the configuration.
"""
@behaviour Plug

alias TrentoWeb.ErrorView

import Plug.Conn

@impl true
def init(opts), do: opts

@impl true
@spec call(Plug.Conn.t(), any) :: Plug.Conn.t()
def call(conn, _) do
conn
|> put_resp_content_type("application/json")
|> resp(
501,
Jason.encode!(
ErrorView.render("501.json", %{
reason: "Charts endpoints are disabled, check the documentation for further details"
})
)
)
|> halt()
end
end
8 changes: 8 additions & 0 deletions lib/trento_web/plugs/unplug_charts_enabled_predicate.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
defmodule TrentoWeb.Plugs.UnplugChartsEnabledPredicate do
CDimonaco marked this conversation as resolved.
Show resolved Hide resolved
@moduledoc false

@behaviour Unplug.Predicate

@impl true
def call(_, _), do: !Application.fetch_env!(:trento, Trento.Charts)[:enabled]
end
14 changes: 12 additions & 2 deletions lib/trento_web/router.ex
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ defmodule TrentoWeb.Router do
error_handler: TrentoWeb.Plugs.ApiAuthErrorHandler}
end

pipeline :charts_feature do
plug Unplug,
if: TrentoWeb.Plugs.UnplugChartsEnabledPredicate,
do: TrentoWeb.Plugs.ChartsDisabledPlug
end

scope "/" do
pipe_through :browser

Expand Down Expand Up @@ -140,8 +146,12 @@ defmodule TrentoWeb.Router do

get "/hosts/:id/exporters_status", PrometheusController, :exporters_status

get "/charts/hosts/:id/cpu", ChartController, :host_cpu
get "/charts/hosts/:id/memory", ChartController, :host_memory
scope "/charts" do
pipe_through :charts_feature

get "/hosts/:id/cpu", ChartController, :host_cpu
get "/hosts/:id/memory", ChartController, :host_memory
end
end

scope "/v2", TrentoWeb.V2 do
Expand Down
1 change: 1 addition & 0 deletions lib/trento_web/templates/page/index.html.heex
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
const config = {
checksServiceBaseUrl: '<%= @check_service_base_url %>',
deregistrationDebounce: <%= @deregistration_debounce %>,
chartsEnabled: <%= @charts_enabled %>,
};
</script>

Expand Down
11 changes: 11 additions & 0 deletions lib/trento_web/views/error_view.ex
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,17 @@ defmodule TrentoWeb.ErrorView do
}
end

def render("501.json", %{reason: reason}) do
%{
errors: [
%{
title: "Not implemented",
detail: reason
}
]
}
end

def template_not_found(template, _assigns) do
%{
errors: [
Expand Down
22 changes: 22 additions & 0 deletions test/trento_web/plugs/charts_disabled_plug_test.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
defmodule TrentoWeb.Plugs.ChartsDisabledPlugTest do
use TrentoWeb.ConnCase, async: true
use Plug.Test

alias TrentoWeb.Plugs.ChartsDisabledPlug

test "should return 501 when called" do
conn = conn(:get, "/foo")

conn = ChartsDisabledPlug.call(conn, [])

assert %{
"errors" => [
%{
"detail" =>
"Charts endpoints are disabled, check the documentation for further details",
"title" => "Not implemented"
}
]
} = json_response(conn, 501)
end
end