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-1284: implement metrics white-listing #447

Merged
merged 1 commit into from
Nov 8, 2023
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
16 changes: 16 additions & 0 deletions api/v1alpha1/flowcollector_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/netobserv/network-observability-operator/api/v1beta2"
utilconversion "github.com/netobserv/network-observability-operator/pkg/conversion"
"github.com/netobserv/network-observability-operator/pkg/helper"
"github.com/netobserv/network-observability-operator/pkg/metrics"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
apiconversion "k8s.io/apimachinery/pkg/conversion"
"sigs.k8s.io/controller-runtime/pkg/conversion"
Expand Down Expand Up @@ -73,6 +74,12 @@ func (r *FlowCollector) ConvertTo(dstRaw conversion.Hub) error {
// Loki
dst.Spec.Loki.Enable = restored.Spec.Loki.Enable

if restored.Spec.Processor.Metrics.IncludeList != nil {
list := make([]string, len(*restored.Spec.Processor.Metrics.IncludeList))
copy(list, *restored.Spec.Processor.Metrics.IncludeList)
dst.Spec.Processor.Metrics.IncludeList = &list
}

return nil
}

Expand Down Expand Up @@ -172,3 +179,12 @@ func Convert_v1beta2_FlowCollectorEBPF_To_v1alpha1_FlowCollectorEBPF(in *v1beta2
func Convert_v1beta2_ServerTLS_To_v1alpha1_ServerTLS(in *v1beta2.ServerTLS, out *ServerTLS, s apiconversion.Scope) error {
return autoConvert_v1beta2_ServerTLS_To_v1alpha1_ServerTLS(in, out, s)
}

// This function need to be manually created because conversion-gen not able to create it intentionally because
// we have new defined fields in v1beta2 not in v1beta1
// nolint:golint,stylecheck,revive
func Convert_v1alpha1_FLPMetrics_To_v1beta2_FLPMetrics(in *FLPMetrics, out *v1beta2.FLPMetrics, s apiconversion.Scope) error {
includeList := metrics.GetEnabledNames(in.IgnoreTags, nil)
out.IncludeList = &includeList
return autoConvert_v1alpha1_FLPMetrics_To_v1beta2_FLPMetrics(in, out, s)
}
19 changes: 7 additions & 12 deletions api/v1alpha1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 14 additions & 3 deletions api/v1beta1/flowcollector_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,13 +344,24 @@ type FLPMetrics struct {
// +optional
Server MetricsServerConfig `json:"server,omitempty"`

// `ignoreTags` is a list of tags to specify which metrics to ignore. Each metric is associated with a list of tags. More details in https://github.com/netobserv/network-observability-operator/tree/main/controllers/flowlogspipeline/metrics_definitions .
// `ignoreTags` [deprecated (*)] is a list of tags to specify which metrics to ignore. Each metric is associated with a list of tags. More details in https://github.com/netobserv/network-observability-operator/tree/main/controllers/flowlogspipeline/metrics_definitions .
// Available tags are: `egress`, `ingress`, `flows`, `bytes`, `packets`, `namespaces`, `nodes`, `workloads`, `nodes-flows`, `namespaces-flows`, `workloads-flows`.
// Namespace-based metrics are covered by both `workloads` and `namespaces` tags, hence it is recommended to always ignore one of them (`workloads` offering a finer granularity).
//+kubebuilder:default:={"egress","packets","nodes-flows","namespaces-flows","workloads-flows","namespaces"}
// Namespace-based metrics are covered by both `workloads` and `namespaces` tags, hence it is recommended to always ignore one of them (`workloads` offering a finer granularity).<br>
// Deprecation notice: use `includeList` instead.
// +kubebuilder:default:={"egress","packets","nodes-flows","namespaces-flows","workloads-flows","namespaces"}
// +optional
IgnoreTags []string `json:"ignoreTags"`

// `includeList` is a list of metric names to specify which metrics to generate.
// The names correspond to the name in Prometheus, without the prefix. For example,
// `namespace_egress_packets_total` will show up as `netobserv_namespace_egress_packets_total` in Prometheus.
// Available names are: `namespace_egress_bytes_total`, `namespace_egress_packets_total`, `namespace_ingress_bytes_total`,
// `namespace_ingress_packets_total`, `namespace_flows_total`, `node_egress_bytes_total`, `node_egress_packets_total`,
// `node_ingress_bytes_total`, `node_ingress_packets_total`, `node_flows_total`, `workload_egress_bytes_total`,
// `workload_egress_packets_total`, `workload_ingress_bytes_total`, `workload_ingress_packets_total`, `workload_flows_total`.
// +optional
IncludeList *[]string `json:"includeList,omitempty"`
Copy link
Member Author

Choose a reason for hiding this comment

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

cc @OlivierCazade can you tell me again why we didn't use pointers on slices? That would be an easy way to tell: this setting was/wasn't explicitly configured.
Here, I'm not providing any default in CRD, but defaults are provided in code

Copy link
Contributor

Choose a reason for hiding this comment

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

Using pointer in CRD to enable/disable a feature does not work, if at some point the pointer is not nil, it is not possible to set the pointer to nil again, applying an yaml without the slice section does not modify it.

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm I just tired that and it worked.
ie. I've set a includeList => saved => checked results, then removed includeList => saved => check results , this worked as intended


// `disableAlerts` is a list of alerts that should be disabled.
// Possible values are:<br>
// `NetObservNoFlows`, which is triggered when no flows are being observed for a certain period.<br>
Expand Down
28 changes: 21 additions & 7 deletions api/v1beta1/flowcollector_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/netobserv/network-observability-operator/api/v1beta2"
utilconversion "github.com/netobserv/network-observability-operator/pkg/conversion"
"github.com/netobserv/network-observability-operator/pkg/helper"
"github.com/netobserv/network-observability-operator/pkg/metrics"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
apiconversion "k8s.io/apimachinery/pkg/conversion"
"sigs.k8s.io/controller-runtime/pkg/conversion"
Expand Down Expand Up @@ -79,13 +80,6 @@ func Convert_v1beta2_FlowCollectorFLP_To_v1beta1_FlowCollectorFLP(in *v1beta2.Fl
return autoConvert_v1beta2_FlowCollectorFLP_To_v1beta1_FlowCollectorFLP(in, out, s)
}

// This function need to be manually created because conversion-gen not able to create it intentionally because
// we have new defined fields in v1beta2 not in v1beta1
// nolint:golint,stylecheck,revive
func Convert_v1beta2_FLPMetrics_To_v1beta1_FLPMetrics(in *v1beta2.FLPMetrics, out *FLPMetrics, s apiconversion.Scope) error {
return autoConvert_v1beta2_FLPMetrics_To_v1beta1_FLPMetrics(in, out, s)
}

// This function need to be manually created because conversion-gen not able to create it intentionally because
// we have new defined fields in v1beta2 not in v1beta1
// nolint:golint,stylecheck,revive
Expand Down Expand Up @@ -129,3 +123,23 @@ func Convert_v1beta1_FlowCollectorLoki_To_v1beta2_FlowCollectorLoki(in *FlowColl
}
return autoConvert_v1beta1_FlowCollectorLoki_To_v1beta2_FlowCollectorLoki(in, out, s)
}

// This function need to be manually created because conversion-gen not able to create it intentionally because
// we have new defined fields in v1beta2 not in v1beta1
// nolint:golint,stylecheck,revive
func Convert_v1beta2_FLPMetrics_To_v1beta1_FLPMetrics(in *v1beta2.FLPMetrics, out *FLPMetrics, s apiconversion.Scope) error {
return autoConvert_v1beta2_FLPMetrics_To_v1beta1_FLPMetrics(in, out, s)
}

// This function need to be manually created because conversion-gen not able to create it intentionally because
// we have new defined fields in v1beta2 not in v1beta1
// nolint:golint,stylecheck,revive
func Convert_v1beta1_FLPMetrics_To_v1beta2_FLPMetrics(in *FLPMetrics, out *v1beta2.FLPMetrics, s apiconversion.Scope) error {
err := autoConvert_v1beta1_FLPMetrics_To_v1beta2_FLPMetrics(in, out, s)
if err != nil {
return err
}
includeList := metrics.GetEnabledNames(in.IgnoreTags, in.IncludeList)
out.IncludeList = &includeList
return nil
}
160 changes: 160 additions & 0 deletions api/v1beta1/flowcollector_webhook_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
package v1beta1

import (
"testing"

"github.com/netobserv/network-observability-operator/api/v1beta2"
"github.com/stretchr/testify/assert"
"k8s.io/utils/ptr"
)

func TestBeta1ConversionRoundtrip_Loki(t *testing.T) {
// Testing beta1 -> beta2 -> beta1
assert := assert.New(t)

initial := FlowCollector{
Spec: FlowCollectorSpec{
Loki: FlowCollectorLoki{
Enable: ptr.To(true),
URL: "http://loki",
StatusURL: "http://loki/status",
QuerierURL: "http://loki/querier",
TenantID: "tenant",
AuthToken: LokiAuthForwardUserToken,
TLS: ClientTLS{
Enable: true,
InsecureSkipVerify: true,
},
StatusTLS: ClientTLS{
Enable: true,
InsecureSkipVerify: true,
},
BatchSize: 1000,
},
},
}

var converted v1beta2.FlowCollector
err := initial.ConvertTo(&converted)
assert.NoError(err)

assert.Equal(v1beta2.LokiModeManual, converted.Spec.Loki.Mode)
assert.True(*converted.Spec.Loki.Enable)
assert.Equal("http://loki", converted.Spec.Loki.Manual.IngesterURL)
assert.Equal("http://loki/status", converted.Spec.Loki.Manual.StatusURL)
assert.Equal("http://loki/querier", converted.Spec.Loki.Manual.QuerierURL)
assert.Equal("tenant", converted.Spec.Loki.Manual.TenantID)
assert.Equal(LokiAuthForwardUserToken, converted.Spec.Loki.Manual.AuthToken)
assert.True(converted.Spec.Loki.Manual.TLS.Enable)
assert.True(converted.Spec.Loki.Manual.TLS.InsecureSkipVerify)
assert.True(converted.Spec.Loki.Manual.StatusTLS.Enable)
assert.True(converted.Spec.Loki.Manual.StatusTLS.InsecureSkipVerify)

// Other way
var back FlowCollector
err = back.ConvertFrom(&converted)
assert.NoError(err)
assert.Equal(initial.Spec.Loki, back.Spec.Loki)
}

func TestBeta2ConversionRoundtrip_Loki(t *testing.T) {
// Testing beta2 -> beta1 -> beta2
assert := assert.New(t)

initial := v1beta2.FlowCollector{
Spec: v1beta2.FlowCollectorSpec{
Loki: v1beta2.FlowCollectorLoki{
Enable: ptr.To(true),
Mode: v1beta2.LokiModeLokiStack,
LokiStack: v1beta2.LokiStackRef{
Name: "lokiii",
Namespace: "lokins",
},
BatchSize: 1000,
},
},
}

var converted FlowCollector
err := converted.ConvertFrom(&initial)
assert.NoError(err)

assert.True(*converted.Spec.Loki.Enable)
assert.Equal("https://lokiii-gateway-http.lokins.svc:8080/api/logs/v1/network/", converted.Spec.Loki.URL)
assert.Equal("https://lokiii-query-frontend-http.lokins.svc:3100/", converted.Spec.Loki.StatusURL)
assert.Equal("https://lokiii-gateway-http.lokins.svc:8080/api/logs/v1/network/", converted.Spec.Loki.QuerierURL)
assert.Equal("network", converted.Spec.Loki.TenantID)
assert.Equal(LokiAuthForwardUserToken, converted.Spec.Loki.AuthToken)
assert.True(converted.Spec.Loki.TLS.Enable)
assert.False(converted.Spec.Loki.TLS.InsecureSkipVerify)
assert.True(converted.Spec.Loki.StatusTLS.Enable)
assert.False(converted.Spec.Loki.StatusTLS.InsecureSkipVerify)

// Other way
var back v1beta2.FlowCollector
err = converted.ConvertTo(&back)
assert.NoError(err)
assert.Equal(initial.Spec.Loki, back.Spec.Loki)
}

func TestBeta1ConversionRoundtrip_Metrics(t *testing.T) {
// Testing beta1 -> beta2 -> beta1
assert := assert.New(t)

initial := FlowCollector{
Spec: FlowCollectorSpec{
Processor: FlowCollectorFLP{
Metrics: FLPMetrics{
DisableAlerts: []FLPAlert{AlertLokiError},
IgnoreTags: []string{"nodes", "workloads", "bytes", "ingress"},
},
},
},
}

var converted v1beta2.FlowCollector
err := initial.ConvertTo(&converted)
assert.NoError(err)

assert.Equal([]v1beta2.FLPAlert{v1beta2.AlertLokiError}, converted.Spec.Processor.Metrics.DisableAlerts)
assert.NotNil(converted.Spec.Processor.Metrics.IncludeList)
assert.Equal([]string{"namespace_egress_packets_total", "namespace_flows_total"}, *converted.Spec.Processor.Metrics.IncludeList)

// Other way
var back FlowCollector
err = back.ConvertFrom(&converted)
assert.NoError(err)
// Here, includeList is preserved; it takes precedence over ignoreTags
assert.Equal([]string{"namespace_egress_packets_total", "namespace_flows_total"}, *back.Spec.Processor.Metrics.IncludeList)
assert.Equal(initial.Spec.Processor.Metrics.DisableAlerts, back.Spec.Processor.Metrics.DisableAlerts)
assert.Equal(initial.Spec.Processor.Metrics.Server, back.Spec.Processor.Metrics.Server)
}

func TestBeta2ConversionRoundtrip_Metrics(t *testing.T) {
// Testing beta2 -> beta1 -> beta2
assert := assert.New(t)

initial := v1beta2.FlowCollector{
Spec: v1beta2.FlowCollectorSpec{
Processor: v1beta2.FlowCollectorFLP{
Metrics: v1beta2.FLPMetrics{
DisableAlerts: []v1beta2.FLPAlert{v1beta2.AlertLokiError},
IncludeList: &[]string{"namespace_egress_packets_total", "namespace_flows_total"},
},
},
},
}

var converted FlowCollector
err := converted.ConvertFrom(&initial)
assert.NoError(err)

assert.Equal([]FLPAlert{AlertLokiError}, converted.Spec.Processor.Metrics.DisableAlerts)
assert.NotNil(converted.Spec.Processor.Metrics.IncludeList)
assert.Equal([]string{"namespace_egress_packets_total", "namespace_flows_total"}, *converted.Spec.Processor.Metrics.IncludeList)

var back v1beta2.FlowCollector
err = converted.ConvertTo(&back)
assert.NoError(err)
assert.Equal(initial.Spec.Processor.Metrics, back.Spec.Processor.Metrics)
}
20 changes: 8 additions & 12 deletions api/v1beta1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading