From 1e6dc72ffc51a192856f16760c09ac7fbde38cc5 Mon Sep 17 00:00:00 2001 From: Alex Fenlon Date: Wed, 15 May 2024 12:27:07 +0100 Subject: [PATCH 1/3] Add AppProtectVersion to Telemetry --- cmd/nginx-ingress/main.go | 3 +- docs/content/overview/product-telemetry.md | 1 + internal/k8s/controller.go | 2 + internal/telemetry/cluster.go | 8 ++ internal/telemetry/collector.go | 8 ++ internal/telemetry/collector_test.go | 113 ++++++++++++++++++ internal/telemetry/data.avdl | 3 + internal/telemetry/exporter.go | 2 + .../nicresourcecounts_attributes_generated.go | 1 + 9 files changed, 140 insertions(+), 1 deletion(-) diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index b0e8796b7c..1eb4b8c1bd 100644 --- a/cmd/nginx-ingress/main.go +++ b/cmd/nginx-ingress/main.go @@ -51,7 +51,7 @@ const ( versionLabel = "app.kubernetes.io/version" appProtectVersionLabel = "appprotect.f5.com/version" agentVersionLabel = "app.nginx.org/agent-version" - appProtectVersionPath = "/opt/app_protect/VERSION" + appProtectVersionPath = "/opt/app_protect/RELEASE" ) func main() { @@ -184,6 +184,7 @@ func main() { DefaultServerSecret: *defaultServerSecret, AppProtectEnabled: *appProtect, AppProtectDosEnabled: *appProtectDos, + AppProtectVersion: appProtectVersion, IsNginxPlus: *nginxPlus, IngressClass: *ingressClass, ExternalServiceName: *externalService, diff --git a/docs/content/overview/product-telemetry.md b/docs/content/overview/product-telemetry.md index 5d0f29234a..1592d69a6e 100644 --- a/docs/content/overview/product-telemetry.md +++ b/docs/content/overview/product-telemetry.md @@ -47,6 +47,7 @@ These are the data points collected and reported by NGINX Ingress Controller: - **OIDCPolicies** Number of OIDC policies. - **WAFPolicies** Number of WAF policies. - **GlobalConfiguration** Represents the use of a GlobalConfiguration resource. +- **AppProtectVersion** The Version of AppProtect. ## Opt out diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 114ffbc4be..c87aa1d0d6 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -185,6 +185,7 @@ type NewLoadBalancerControllerInput struct { DefaultServerSecret string AppProtectEnabled bool AppProtectDosEnabled bool + AppProtectVersion string IsNginxPlus bool IngressClass string ExternalServiceName string @@ -364,6 +365,7 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc Period: 24 * time.Hour, K8sClientReader: input.KubeClient, Version: input.NICVersion, + AppProtectVersion: input.AppProtectVersion, GlobalConfiguration: lbc.watchGlobalConfiguration, Configurator: lbc.configurator, SecretStore: lbc.secretStore, diff --git a/internal/telemetry/cluster.go b/internal/telemetry/cluster.go index 896ee7e2cb..9888aab339 100644 --- a/internal/telemetry/cluster.go +++ b/internal/telemetry/cluster.go @@ -192,6 +192,14 @@ func (c *Collector) PolicyCount() map[string]int { return policyCounters } +// AppProtectVersion returns the AppProtect Version +func (c *Collector) AppProtectVersion() string { + if c.Config.AppProtectVersion == "" { + return "" + } + return c.Config.AppProtectVersion +} + // lookupPlatform takes a string representing a K8s PlatformID // retrieved from a cluster node and returns a string // representing the platform name. diff --git a/internal/telemetry/collector.go b/internal/telemetry/collector.go index 21a5bf98ea..0c88ef0d41 100644 --- a/internal/telemetry/collector.go +++ b/internal/telemetry/collector.go @@ -71,6 +71,9 @@ type CollectorConfig struct { // Policies gets all policies Policies func() []*conf_v1.Policy + + // AppProtectVersion represents the version of App Protect. + AppProtectVersion string } // NewCollector takes 0 or more options and creates a new TraceReporter. @@ -133,6 +136,7 @@ func (c *Collector) Collect(ctx context.Context) { WAFPolicies: int64(report.WAFCount), GlobalConfiguration: report.GlobalConfiguration, IngressAnnotations: report.IngressAnnotations, + AppProtectVersion: report.AppProtectVersion, }, } @@ -173,6 +177,7 @@ type Report struct { WAFCount int GlobalConfiguration bool IngressAnnotations []string + AppProtectVersion string } // BuildReport takes context, collects telemetry data and builds the report. @@ -241,6 +246,8 @@ func (c *Collector) BuildReport(ctx context.Context) (Report, error) { ingressAnnotations := c.IngressAnnotations() + appProtectVersion := c.AppProtectVersion() + return Report{ Name: "NIC", Version: c.Config.Version, @@ -268,5 +275,6 @@ func (c *Collector) BuildReport(ctx context.Context) (Report, error) { WAFCount: wafCount, GlobalConfiguration: c.Config.GlobalConfiguration, IngressAnnotations: ingressAnnotations, + AppProtectVersion: appProtectVersion, }, err } diff --git a/internal/telemetry/collector_test.go b/internal/telemetry/collector_test.go index 55b0275ff9..7fb5bd7f70 100644 --- a/internal/telemetry/collector_test.go +++ b/internal/telemetry/collector_test.go @@ -665,6 +665,119 @@ func TestIngressCountReportsNumberOfDeployedIngresses(t *testing.T) { } } +func TestCollectAppProtectVersion(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + appProtectVersion string + wantVersion string + }{ + { + name: "AppProtect 4.8", + appProtectVersion: "4.8.1", + wantVersion: "4.8.1", + }, + { + name: "AppProtect 4.9", + appProtectVersion: "4.9", + wantVersion: "4.9", + }, + { + name: "AppProtect 5.1", + appProtectVersion: "5.1", + wantVersion: "5.1", + }, + { + name: "No AppProtect Installed", + appProtectVersion: "0", + wantVersion: "0", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + buf := &bytes.Buffer{} + exp := &telemetry.StdoutExporter{Endpoint: buf} + + configurator := newConfiguratorWithIngress(t) + + cfg := telemetry.CollectorConfig{ + Configurator: configurator, + K8sClientReader: newTestClientset(node1, kubeNS), + Version: telemetryNICData.ProjectVersion, + AppProtectVersion: tc.appProtectVersion, + } + + c, err := telemetry.NewCollector(cfg, telemetry.WithExporter(exp)) + if err != nil { + t.Fatal(err) + } + c.Collect(context.Background()) + + ver := c.AppProtectVersion() + + if tc.wantVersion != ver { + t.Errorf("want: %s, got: %s", tc.wantVersion, ver) + } + }) + } +} + +func TestCollectInvalidAppProtectVersion(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + appProtectVersion string + wantVersion string + }{ + { + name: "AppProtect Not Installed", + appProtectVersion: "0", + wantVersion: "4.8.1", + }, + { + name: "Cant Find AppProtect 4.9", + appProtectVersion: "4.9", + wantVersion: "0", + }, + { + name: "Found Different AppProtect Version", + appProtectVersion: "5.1", + wantVersion: "4.9", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + buf := &bytes.Buffer{} + exp := &telemetry.StdoutExporter{Endpoint: buf} + + configurator := newConfiguratorWithIngress(t) + + cfg := telemetry.CollectorConfig{ + Configurator: configurator, + K8sClientReader: newTestClientset(node1, kubeNS), + Version: telemetryNICData.ProjectVersion, + AppProtectVersion: tc.appProtectVersion, + } + + c, err := telemetry.NewCollector(cfg, telemetry.WithExporter(exp)) + if err != nil { + t.Fatal(err) + } + c.Collect(context.Background()) + + ver := c.AppProtectVersion() + + if tc.wantVersion == ver { + t.Errorf("want: %s, got: %s", tc.wantVersion, ver) + } + }) + } +} + func TestCountVirtualServers(t *testing.T) { t.Parallel() diff --git a/internal/telemetry/data.avdl b/internal/telemetry/data.avdl index d70633f21a..a7a105186a 100644 --- a/internal/telemetry/data.avdl +++ b/internal/telemetry/data.avdl @@ -87,5 +87,8 @@ It is the UID of the `kube-system` Namespace. */ /** IngressAnnotations is the list of annotations resources managed by NGINX Ingress Controller */ union {null, array} IngressAnnotations = null; + /** AppProtectVersion represents the version of AppProtect. */ + string? AppProtectVersion = null; + } } diff --git a/internal/telemetry/exporter.go b/internal/telemetry/exporter.go index 05b687009f..db158dd875 100644 --- a/internal/telemetry/exporter.go +++ b/internal/telemetry/exporter.go @@ -101,4 +101,6 @@ type NICResourceCounts struct { GlobalConfiguration bool // IngressAnnotations is the list of annotations resources managed by NGINX Ingress Controller IngressAnnotations []string + // AppProtectVersion represents the version of AppProtect. + AppProtectVersion string } diff --git a/internal/telemetry/nicresourcecounts_attributes_generated.go b/internal/telemetry/nicresourcecounts_attributes_generated.go index 896180518a..75f8eb905d 100644 --- a/internal/telemetry/nicresourcecounts_attributes_generated.go +++ b/internal/telemetry/nicresourcecounts_attributes_generated.go @@ -31,6 +31,7 @@ func (d *NICResourceCounts) Attributes() []attribute.KeyValue { attrs = append(attrs, attribute.Int64("WAFPolicies", d.WAFPolicies)) attrs = append(attrs, attribute.Bool("GlobalConfiguration", d.GlobalConfiguration)) attrs = append(attrs, attribute.StringSlice("IngressAnnotations", d.IngressAnnotations)) + attrs = append(attrs, attribute.String("AppProtectVersion", d.AppProtectVersion)) return attrs } From c443d1144ecf54b3464c065defeaacdb0ca8022a Mon Sep 17 00:00:00 2001 From: Alex Fenlon Date: Wed, 15 May 2024 15:28:06 +0100 Subject: [PATCH 2/3] Update to allow for empty string --- internal/telemetry/cluster.go | 3 --- internal/telemetry/collector_test.go | 8 ++++---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/internal/telemetry/cluster.go b/internal/telemetry/cluster.go index 9888aab339..ebc2c7a760 100644 --- a/internal/telemetry/cluster.go +++ b/internal/telemetry/cluster.go @@ -194,9 +194,6 @@ func (c *Collector) PolicyCount() map[string]int { // AppProtectVersion returns the AppProtect Version func (c *Collector) AppProtectVersion() string { - if c.Config.AppProtectVersion == "" { - return "" - } return c.Config.AppProtectVersion } diff --git a/internal/telemetry/collector_test.go b/internal/telemetry/collector_test.go index 7fb5bd7f70..b1867f1cdb 100644 --- a/internal/telemetry/collector_test.go +++ b/internal/telemetry/collector_test.go @@ -690,8 +690,8 @@ func TestCollectAppProtectVersion(t *testing.T) { }, { name: "No AppProtect Installed", - appProtectVersion: "0", - wantVersion: "0", + appProtectVersion: "", + wantVersion: "", }, } @@ -734,13 +734,13 @@ func TestCollectInvalidAppProtectVersion(t *testing.T) { }{ { name: "AppProtect Not Installed", - appProtectVersion: "0", + appProtectVersion: "", wantVersion: "4.8.1", }, { name: "Cant Find AppProtect 4.9", appProtectVersion: "4.9", - wantVersion: "0", + wantVersion: "", }, { name: "Found Different AppProtect Version", From 0533185f86b8beff73b130503784ac0b5aa5e6fa Mon Sep 17 00:00:00 2001 From: AlexFenlon Date: Thu, 16 May 2024 13:29:07 +0100 Subject: [PATCH 3/3] Update docs/content/overview/product-telemetry.md Co-authored-by: Alan Dooley Signed-off-by: AlexFenlon --- docs/content/overview/product-telemetry.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/overview/product-telemetry.md b/docs/content/overview/product-telemetry.md index 1592d69a6e..387421f619 100644 --- a/docs/content/overview/product-telemetry.md +++ b/docs/content/overview/product-telemetry.md @@ -47,7 +47,7 @@ These are the data points collected and reported by NGINX Ingress Controller: - **OIDCPolicies** Number of OIDC policies. - **WAFPolicies** Number of WAF policies. - **GlobalConfiguration** Represents the use of a GlobalConfiguration resource. -- **AppProtectVersion** The Version of AppProtect. +- **AppProtectVersion** The AppProtect version ## Opt out