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

NETOBSERV-1621: fix error messages #523

Merged
merged 6 commits into from
Jun 6, 2024
Merged
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
1 change: 0 additions & 1 deletion config/sample-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ loki:
tenantID: netobserv
useMocks: false
prometheus:
url: http://prometheus:9090
timeout: 30s
metrics:
- enabled: false
Expand Down
43 changes: 43 additions & 0 deletions pkg/kubernetes/auth/check_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ const (
AuthHeader = "Authorization"
CheckAuthenticated CheckType = "authenticated"
CheckAdmin CheckType = "admin"
CheckDenyAll CheckType = "denyAll"
CheckNone CheckType = "none"
)

type Checker interface {
CheckAuth(ctx context.Context, header http.Header) error
CheckAdmin(ctx context.Context, header http.Header) error
}

func NewChecker(typez CheckType, apiProvider client.APIProvider) (Checker, error) {
Expand All @@ -36,6 +38,9 @@ func NewChecker(typez CheckType, apiProvider client.APIProvider) (Checker, error
return &BearerTokenChecker{apiProvider: apiProvider, predicates: []authPredicate{mustBeAuthenticated}}, nil
case CheckAdmin:
return &BearerTokenChecker{apiProvider: apiProvider, predicates: []authPredicate{mustBeAuthenticated, mustBeClusterAdmin}}, nil
case CheckDenyAll:
return &DenyAllChecker{}, nil

}
return nil, fmt.Errorf("auth checker type unknown: %s. Must be one of %s, %s, %s", typez, CheckAdmin, CheckAuthenticated, CheckNone)
}
Expand All @@ -49,6 +54,25 @@ func (b *NoopChecker) CheckAuth(_ context.Context, _ http.Header) error {
return nil
}

func (b *NoopChecker) CheckAdmin(_ context.Context, _ http.Header) error {
hlog.Debug("noop auth checker: ignore auth")
return nil
}

type DenyAllChecker struct {
Checker
}

func (b *DenyAllChecker) CheckAuth(_ context.Context, _ http.Header) error {
hlog.Debug("deny all auth checker: deny auth")
return errors.New("deny all auth mode selected")
}

func (b *DenyAllChecker) CheckAdmin(_ context.Context, _ http.Header) error {
hlog.Debug("deny all auth checker: deny auth")
return errors.New("deny all auth mode selected")
}

func getUserToken(header http.Header) (string, error) {
authValue := header.Get(AuthHeader)
if authValue != "" {
Expand Down Expand Up @@ -118,3 +142,22 @@ func (c *BearerTokenChecker) CheckAuth(ctx context.Context, header http.Header)
hlog.Debug("Checking auth: passed")
return nil
}

func (c *BearerTokenChecker) CheckAdmin(ctx context.Context, header http.Header) error {
hlog.Debug("Checking admin user")
token, err := getUserToken(header)
if err != nil {
return err
}
hlog.Debug("Checking admin: token found")
client, err := c.apiProvider()
if err != nil {
return err
}
if err = mustBeClusterAdmin(ctx, client, token); err != nil {
return err
}

hlog.Debug("Checking admin: passed")
return nil
}
8 changes: 8 additions & 0 deletions pkg/kubernetes/auth/check_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@ func TestCheckAuth_NoAuth(t *testing.T) {
// No header => success
err = checkerNoop.CheckAuth(context.TODO(), http.Header{})
require.NoError(t, err)

// Deny All mode
checkerDenyAll := DenyAllChecker{}

// No header => fail
err = checkerDenyAll.CheckAuth(context.TODO(), http.Header{})
require.Error(t, err)
require.Equal(t, "deny all auth mode selected", err.Error())
}

func TestCheckAuth_NormalUser(t *testing.T) {
Expand Down
23 changes: 19 additions & 4 deletions pkg/server/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,13 @@ func setupRoutes(ctx context.Context, cfg *config.Config, authChecker auth.Check
orig.ServeHTTP(w, r)
})
})

api.HandleFunc("/status", handler.Status)
api.HandleFunc("/loki/ready", h.LokiReady())
api.HandleFunc("/loki/metrics", h.LokiMetrics())
api.HandleFunc("/loki/buildinfo", h.LokiBuildInfos())
api.HandleFunc("/loki/config/limits", h.LokiConfig("limits_config"))
api.HandleFunc("/loki/config/ingester/max_chunk_age", h.IngesterMaxChunkAge())
api.HandleFunc("/loki/metrics", forceCheckAdmin(authChecker, h.LokiMetrics()))
api.HandleFunc("/loki/buildinfo", forceCheckAdmin(authChecker, h.LokiBuildInfos()))
api.HandleFunc("/loki/config/limits", forceCheckAdmin(authChecker, h.LokiConfig("limits_config")))
api.HandleFunc("/loki/config/ingester/max_chunk_age", forceCheckAdmin(authChecker, h.IngesterMaxChunkAge()))
api.HandleFunc("/loki/flow/records", h.GetFlows(ctx))
api.HandleFunc("/loki/flow/metrics", h.GetTopology(ctx))
api.HandleFunc("/loki/export", h.ExportFlows(ctx))
Expand All @@ -55,3 +56,17 @@ func setupRoutes(ctx context.Context, cfg *config.Config, authChecker auth.Check
r.PathPrefix("/").Handler(http.FileServer(http.Dir("./web/dist/")))
return r
}

func forceCheckAdmin(authChecker auth.Checker, handle func(http.ResponseWriter, *http.Request)) func(http.ResponseWriter, *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
if err := authChecker.CheckAdmin(context.TODO(), r.Header); err != nil {
w.WriteHeader(http.StatusUnauthorized)
_, err2 := w.Write([]byte(err.Error()))
if err2 != nil {
logrus.Errorf("Error while responding an error: %v (initial was: %v)", err2, err)
}
return
}
handle(w, r)
}
}
6 changes: 6 additions & 0 deletions pkg/server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,8 +513,14 @@ func (a *authMock) CheckAuth(ctx context.Context, header http.Header) error {
return args.Error(0)
}

func (a *authMock) CheckAdmin(ctx context.Context, header http.Header) error {
args := a.Called(ctx, header)
return args.Error(0)
}

func (a *authMock) MockGranted() {
a.On("CheckAuth", mock.Anything, mock.Anything).Return(nil)
a.On("CheckAdmin", mock.Anything, mock.Anything).Return(nil)
}

func prepareServerAssets(t *testing.T) string {
Expand Down
23 changes: 15 additions & 8 deletions web/locales/en/plugin__netobserv-plugin.json
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@
"Configuration limits": "Configuration limits",
"Metrics": "Metrics",
"You may consider the following changes to avoid this error:": "You may consider the following changes to avoid this error:",
"Add missing metrics to prometheus using FlowMetric API": "Add missing metrics to prometheus using FlowMetric API",
"Enable Loki in FlowCollector API": "Enable Loki in FlowCollector API",
"Reduce the Query Options -> limit to reduce the number of results": "Reduce the Query Options -> limit to reduce the number of results",
"Increase Loki \"max_entries_limit_per_query\" entry in configuration file": "Increase Loki \"max_entries_limit_per_query\" entry in configuration file",
"Add Namespace, Owner or Resource filters (which use indexed fields) to improve the query performance": "Add Namespace, Owner or Resource filters (which use indexed fields) to improve the query performance",
Expand All @@ -180,9 +182,16 @@
"This error is generally seen when cluster admin groups are not properly configured.": "This error is generally seen when cluster admin groups are not properly configured.",
"Click the link below for more help.": "Click the link below for more help.",
"More information": "More information",
"Loki '/ready' endpoint returned the following error": "Loki '/ready' endpoint returned the following error",
"Check if Loki is running correctly. '/ready' endpoint should respond \"ready\"": "Check if Loki is running correctly. '/ready' endpoint should respond \"ready\"",
"Configuring Grafana Loki": "Configuring Grafana Loki",
"Check your connectivity with cluster / console plugin pod": "Check your connectivity with cluster / console plugin pod",
"Check current user permissions": "Check current user permissions",
"For LokiStack, your user must either:": "For LokiStack, your user must either:",
"have the 'netobserv-reader' cluster role, which allows multi-tenancy": "have the 'netobserv-reader' cluster role, which allows multi-tenancy",
"or be in the 'cluster-admin' group (not the same as the 'cluster-admin' role)": "or be in the 'cluster-admin' group (not the same as the 'cluster-admin' role)",
"or LokiStack spec.tenants.openshift.adminGroups must be configured with a group this user belongs to": "or LokiStack spec.tenants.openshift.adminGroups must be configured with a group this user belongs to",
"For other configurations, refer to FlowCollector spec.loki.manual.authToken": "For other configurations, refer to FlowCollector spec.loki.manual.authToken",
"Configuring the Loki Operator": "Configuring the Loki Operator",
"Configuring Grafana Loki (community)": "Configuring Grafana Loki (community)",
"Show metrics": "Show metrics",
"Show build info": "Show build info",
"Show configuration limits": "Show configuration limits",
Expand Down Expand Up @@ -245,7 +254,6 @@
"Show total": "Show total",
"Show total dropped": "Show total dropped",
"Show overall": "Show overall",
"Unable to get overview": "Unable to get overview",
"Graph type": "Graph type",
"Donut": "Donut",
"Bars": "Bars",
Expand Down Expand Up @@ -284,8 +292,8 @@
"More info": "More info",
"Raw": "Raw",
"JSON": "JSON",
"Unable to get {{item}}": "Unable to get {{item}}",
"Kind not managed": "Kind not managed",
"Unable to get flows": "Unable to get flows",
"Step into this {{name}}": "Step into this {{name}}",
"Filter by source or destination {{name}}": "Filter by source or destination {{name}}",
"Filter by {{name}}": "Filter by {{name}}",
Expand All @@ -310,8 +318,10 @@
"Cluster name": "Cluster name",
"Edge": "Edge",
"Drops": "Drops",
"Unable to get topology": "Unable to get topology",
"Query is slow": "Query is slow",
"When in \"Match any\" mode, try using only Namespace, Owner or Resource filters (which use indexed fields), or decrease limit / range, to improve the query performance": "When in \"Match any\" mode, try using only Namespace, Owner or Resource filters (which use indexed fields), or decrease limit / range, to improve the query performance",
"Add Namespace, Owner or Resource filters (which use indexed fields), or decrease limit / range, to improve the query performance": "Add Namespace, Owner or Resource filters (which use indexed fields), or decrease limit / range, to improve the query performance",
"Add more filters or decrease limit / range to improve the query performance": "Add more filters or decrease limit / range to improve the query performance",
"Overview": "Overview",
"Traffic flows": "Traffic flows",
"Topology": "Topology",
Expand All @@ -326,9 +336,6 @@
"More options": "More options",
"Time range": "Time range",
"Refresh interval": "Refresh interval",
"When in \"Match any\" mode, try using only Namespace, Owner or Resource filters (which use indexed fields), or decrease limit / range, to improve the query performance": "When in \"Match any\" mode, try using only Namespace, Owner or Resource filters (which use indexed fields), or decrease limit / range, to improve the query performance",
"Add Namespace, Owner or Resource filters (which use indexed fields), or decrease limit / range, to improve the query performance": "Add Namespace, Owner or Resource filters (which use indexed fields), or decrease limit / range, to improve the query performance",
"Add more filters or decrease limit / range to improve the query performance": "Add more filters or decrease limit / range to improve the query performance",
"Network Traffic": "Network Traffic",
"Hide histogram": "Hide histogram",
"Show histogram": "Show histogram",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#loki-error-container {
#netobserv-error-container {
overflow: auto;
height: 100%;
}
Expand All @@ -10,8 +10,8 @@
justify-content: center;
}

.loki-error-icon,
.loki-error-message {
.netobserv-error-icon,
.netobserv-error-message {
color: #A30000 !important;
margin-bottom: 1em;
}
Expand All @@ -22,5 +22,10 @@
}

.error-text-content>blockquote {
width: 50%;
}
width: 60%;
}

.error-text-content ul {
margin-top: 0.5em;
text-align: initial;
}
Loading
Loading