From 1dbe45af396c0baf3f2eff033d015883f6f52803 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Thu, 22 Feb 2024 08:16:20 -0700 Subject: [PATCH] Add flag to opt out of product telemetry (#1605) Problem: As a user of NGF I want an easy option to opt out of recording and sending of all telemetry data So that if I am not comfortable sending that information, I can still continue to use NGF Solution: Added a new configurable flag that can disable telemetry. On by default. Also turns off any RBAC if not needed (for N+ usage or telemetry) --- cmd/gateway/commands.go | 59 +++++++++++-------- deploy/helm-chart/README.md | 3 + deploy/helm-chart/templates/deployment.yaml | 3 + deploy/helm-chart/templates/rbac.yaml | 27 ++++++--- deploy/helm-chart/values.yaml | 4 ++ .../manifests/nginx-gateway-experimental.yaml | 15 ++--- deploy/manifests/nginx-gateway.yaml | 15 ++--- .../nginx-plus-gateway-experimental.yaml | 21 ++++--- deploy/manifests/nginx-plus-gateway.yaml | 21 ++++--- internal/mode/static/config/config.go | 12 +++- internal/mode/static/manager.go | 32 +++++----- site/content/reference/cli-help.md | 1 + 12 files changed, 128 insertions(+), 85 deletions(-) diff --git a/cmd/gateway/commands.go b/cmd/gateway/commands.go index e721e697a..32e819c21 100644 --- a/cmd/gateway/commands.go +++ b/cmd/gateway/commands.go @@ -44,23 +44,24 @@ func createRootCommand() *cobra.Command { func createStaticModeCommand() *cobra.Command { // flag names const ( - gatewayFlag = "gateway" - configFlag = "config" - serviceFlag = "service" - updateGCStatusFlag = "update-gatewayclass-status" - metricsDisableFlag = "metrics-disable" - metricsSecureFlag = "metrics-secure-serving" - metricsPortFlag = "metrics-port" - healthDisableFlag = "health-disable" - healthPortFlag = "health-port" - leaderElectionDisableFlag = "leader-election-disable" - leaderElectionLockNameFlag = "leader-election-lock-name" - plusFlag = "nginx-plus" - gwAPIExperimentalFlag = "gateway-api-experimental-features" - usageReportSecretFlag = "usage-report-secret" - usageReportServerURLFlag = "usage-report-server-url" - usageReportSkipVerifyFlag = "usage-report-skip-verify" - usageReportClusterNameFlag = "usage-report-cluster-name" + gatewayFlag = "gateway" + configFlag = "config" + serviceFlag = "service" + updateGCStatusFlag = "update-gatewayclass-status" + metricsDisableFlag = "metrics-disable" + metricsSecureFlag = "metrics-secure-serving" + metricsPortFlag = "metrics-port" + healthDisableFlag = "health-disable" + healthPortFlag = "health-port" + leaderElectionDisableFlag = "leader-election-disable" + leaderElectionLockNameFlag = "leader-election-lock-name" + productTelemetryDisableFlag = "product-telemetry-disable" + plusFlag = "nginx-plus" + gwAPIExperimentalFlag = "gateway-api-experimental-features" + usageReportSecretFlag = "usage-report-secret" + usageReportServerURLFlag = "usage-report-server-url" + usageReportSkipVerifyFlag = "usage-report-skip-verify" + usageReportClusterNameFlag = "usage-report-cluster-name" ) // flag values @@ -101,6 +102,8 @@ func createStaticModeCommand() *cobra.Command { gwExperimentalFeatures bool + disableProductTelemetry bool + plus bool usageReportSkipVerify bool usageReportClusterName = stringValidatingValue{ @@ -203,12 +206,15 @@ func createStaticModeCommand() *cobra.Command { LockName: leaderElectionLockName.String(), Identity: podName, }, - UsageReportConfig: usageReportConfig, - Plus: plus, - TelemetryReportPeriod: period, - Version: version, - ExperimentalFeatures: gwExperimentalFeatures, - ImageSource: imageSource, + UsageReportConfig: usageReportConfig, + ProductTelemetryConfig: config.ProductTelemetryConfig{ + TelemetryReportPeriod: period, + Enabled: !disableProductTelemetry, + }, + Plus: plus, + Version: version, + ExperimentalFeatures: gwExperimentalFeatures, + ImageSource: imageSource, } if err := static.StartManager(conf); err != nil { @@ -315,6 +321,13 @@ func createStaticModeCommand() *cobra.Command { "A Lease object with this name will be created in the same Namespace as the controller.", ) + cmd.Flags().BoolVar( + &disableProductTelemetry, + productTelemetryDisableFlag, + false, + "Disable the collection of product telemetry.", + ) + cmd.Flags().BoolVar( &plus, plusFlag, diff --git a/deploy/helm-chart/README.md b/deploy/helm-chart/README.md index 9032a23ca..02bbf785c 100644 --- a/deploy/helm-chart/README.md +++ b/deploy/helm-chart/README.md @@ -293,6 +293,9 @@ The following tables lists the configurable parameters of the NGINX Gateway Fabr | `nginxGateway.replicaCount` | The number of replicas of the NGINX Gateway Fabric Deployment. | 1 | | `nginxGateway.leaderElection.enable` | Enable leader election. Leader election is used to avoid multiple replicas of the NGINX Gateway Fabric reporting the status of the Gateway API resources. | true | | `nginxGateway.leaderElection.lockName` | The name of the leader election lock. A Lease object with this name will be created in the same Namespace as the controller. | Autogenerated | +| `nginxGateway.securityContext.allowPrivilegeEscalation` | Some environments may need this set to true in order for the control plane to successfully reload NGINX. | false | +| `nginxGateway.productTelemetry.enable` | Enable the collection of product telemetry. | true | +| `nginxGateway.gwAPIExperimentalFeatures.enable` | Enable the experimental features of Gateway API which are supported by NGINX Gateway Fabric. Requires the Gateway APIs installed from the experimental channel. | false | | `nginx.image.repository` | The repository for the NGINX image. | ghcr.io/nginxinc/nginx-gateway-fabric/nginx | | `nginx.image.tag` | The tag for the NGINX image. | edge | | `nginx.image.pullPolicy` | The `imagePullPolicy` for the NGINX image. | Always | diff --git a/deploy/helm-chart/templates/deployment.yaml b/deploy/helm-chart/templates/deployment.yaml index 12479708f..dd272d91e 100644 --- a/deploy/helm-chart/templates/deployment.yaml +++ b/deploy/helm-chart/templates/deployment.yaml @@ -52,6 +52,9 @@ spec: {{- else }} - --leader-election-disable {{- end }} + {{- if not .Values.nginxGateway.productTelemetry.enable }} + - --product-telemetry-disable + {{- end }} {{- if .Values.nginxGateway.gwAPIExperimentalFeatures.enable }} - --gateway-api-experimental-features {{- end }} diff --git a/deploy/helm-chart/templates/rbac.yaml b/deploy/helm-chart/templates/rbac.yaml index 962f279ab..851ddb42b 100644 --- a/deploy/helm-chart/templates/rbac.yaml +++ b/deploy/helm-chart/templates/rbac.yaml @@ -39,20 +39,36 @@ rules: - get - list - watch -# FIXME(bjee19): make nodes, pods, replicasets permission dependent on telemetry being enabled. -# https://github.com/nginxinc/nginx-gateway-fabric/issues/1317. +{{- if .Values.nginxGateway.productTelemetry.enable }} - apiGroups: - "" resources: - pods verbs: - get +- apiGroups: + - apps + resources: + - replicasets + verbs: + - get +{{- end }} +{{- if .Values.nginx.plus }} +- apiGroups: + - apps + resources: + - replicasets + verbs: + - list +{{- end }} +{{- if or .Values.nginxGateway.productTelemetry.enable .Values.nginx.plus }} - apiGroups: - "" resources: - nodes verbs: - list +{{- end }} - apiGroups: - "" resources: @@ -60,13 +76,6 @@ rules: verbs: - create - patch -- apiGroups: - - apps - resources: - - replicasets - verbs: - - get - - list - apiGroups: - discovery.k8s.io resources: diff --git a/deploy/helm-chart/values.yaml b/deploy/helm-chart/values.yaml index a10d61a73..c0dbe5eb6 100644 --- a/deploy/helm-chart/values.yaml +++ b/deploy/helm-chart/values.yaml @@ -45,6 +45,10 @@ nginxGateway: ## Some environments may need this set to true in order for the control plane to successfully reload NGINX. allowPrivilegeEscalation: false + productTelemetry: + ## Enable the collection of product telemetry. + enable: true + ## The lifecycle of the nginx-gateway container. lifecycle: {} diff --git a/deploy/manifests/nginx-gateway-experimental.yaml b/deploy/manifests/nginx-gateway-experimental.yaml index a6480764d..7cf291288 100644 --- a/deploy/manifests/nginx-gateway-experimental.yaml +++ b/deploy/manifests/nginx-gateway-experimental.yaml @@ -37,14 +37,18 @@ rules: - get - list - watch -# FIXME(bjee19): make nodes, pods, replicasets permission dependent on telemetry being enabled. -# https://github.com/nginxinc/nginx-gateway-fabric/issues/1317. - apiGroups: - "" resources: - pods verbs: - get +- apiGroups: + - apps + resources: + - replicasets + verbs: + - get - apiGroups: - "" resources: @@ -58,13 +62,6 @@ rules: verbs: - create - patch -- apiGroups: - - apps - resources: - - replicasets - verbs: - - get - - list - apiGroups: - discovery.k8s.io resources: diff --git a/deploy/manifests/nginx-gateway.yaml b/deploy/manifests/nginx-gateway.yaml index 37fbd2dab..933e66d4e 100644 --- a/deploy/manifests/nginx-gateway.yaml +++ b/deploy/manifests/nginx-gateway.yaml @@ -36,14 +36,18 @@ rules: - get - list - watch -# FIXME(bjee19): make nodes, pods, replicasets permission dependent on telemetry being enabled. -# https://github.com/nginxinc/nginx-gateway-fabric/issues/1317. - apiGroups: - "" resources: - pods verbs: - get +- apiGroups: + - apps + resources: + - replicasets + verbs: + - get - apiGroups: - "" resources: @@ -57,13 +61,6 @@ rules: verbs: - create - patch -- apiGroups: - - apps - resources: - - replicasets - verbs: - - get - - list - apiGroups: - discovery.k8s.io resources: diff --git a/deploy/manifests/nginx-plus-gateway-experimental.yaml b/deploy/manifests/nginx-plus-gateway-experimental.yaml index e2e5027dd..88b5249c3 100644 --- a/deploy/manifests/nginx-plus-gateway-experimental.yaml +++ b/deploy/manifests/nginx-plus-gateway-experimental.yaml @@ -37,14 +37,24 @@ rules: - get - list - watch -# FIXME(bjee19): make nodes, pods, replicasets permission dependent on telemetry being enabled. -# https://github.com/nginxinc/nginx-gateway-fabric/issues/1317. - apiGroups: - "" resources: - pods verbs: - get +- apiGroups: + - apps + resources: + - replicasets + verbs: + - get +- apiGroups: + - apps + resources: + - replicasets + verbs: + - list - apiGroups: - "" resources: @@ -58,13 +68,6 @@ rules: verbs: - create - patch -- apiGroups: - - apps - resources: - - replicasets - verbs: - - get - - list - apiGroups: - discovery.k8s.io resources: diff --git a/deploy/manifests/nginx-plus-gateway.yaml b/deploy/manifests/nginx-plus-gateway.yaml index a34a38d08..e0de54f54 100644 --- a/deploy/manifests/nginx-plus-gateway.yaml +++ b/deploy/manifests/nginx-plus-gateway.yaml @@ -36,14 +36,24 @@ rules: - get - list - watch -# FIXME(bjee19): make nodes, pods, replicasets permission dependent on telemetry being enabled. -# https://github.com/nginxinc/nginx-gateway-fabric/issues/1317. - apiGroups: - "" resources: - pods verbs: - get +- apiGroups: + - apps + resources: + - replicasets + verbs: + - get +- apiGroups: + - apps + resources: + - replicasets + verbs: + - list - apiGroups: - "" resources: @@ -57,13 +67,6 @@ rules: verbs: - create - patch -- apiGroups: - - apps - resources: - - replicasets - verbs: - - get - - list - apiGroups: - discovery.k8s.io resources: diff --git a/internal/mode/static/config/config.go b/internal/mode/static/config/config.go index 3a21055fd..5f93a3e3d 100644 --- a/internal/mode/static/config/config.go +++ b/internal/mode/static/config/config.go @@ -36,8 +36,8 @@ type Config struct { MetricsConfig MetricsConfig // HealthConfig specifies the health probe config. HealthConfig HealthConfig - // TelemetryReportPeriod is the period at which telemetry reports are sent. - TelemetryReportPeriod time.Duration + // ProductTelemetryConfig contains the configuration for collecting product telemetry. + ProductTelemetryConfig ProductTelemetryConfig // UpdateGatewayClassStatus enables updating the status of the GatewayClass resource. UpdateGatewayClassStatus bool // Plus indicates whether NGINX Plus is being used. @@ -86,6 +86,14 @@ type LeaderElectionConfig struct { Enabled bool } +// ProductTelemetryConfig contains the configuration for collecting product telemetry. +type ProductTelemetryConfig struct { + // TelemetryReportPeriod is the period at which telemetry reports are sent. + TelemetryReportPeriod time.Duration + // Enabled is the flag for toggling the collection of product telemetry. + Enabled bool +} + // UsageReportConfig contains the configuration for NGINX Plus usage reporting. type UsageReportConfig struct { // SecretNsName is the namespaced name of the Secret containing the server credentials. diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 2652b72d7..f13d24cc5 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -242,19 +242,21 @@ func StartManager(cfg config.Config) error { return fmt.Errorf("cannot register status updater: %w", err) } - dataCollector := telemetry.NewDataCollectorImpl(telemetry.DataCollectorConfig{ - K8sClientReader: mgr.GetAPIReader(), - GraphGetter: processor, - ConfigurationGetter: eventHandler, - Version: cfg.Version, - PodNSName: types.NamespacedName{ - Namespace: cfg.GatewayPodConfig.Namespace, - Name: cfg.GatewayPodConfig.Name, - }, - ImageSource: cfg.ImageSource, - }) - if err = mgr.Add(createTelemetryJob(cfg, dataCollector, nginxChecker.getReadyCh())); err != nil { - return fmt.Errorf("cannot register telemetry job: %w", err) + if cfg.ProductTelemetryConfig.Enabled { + dataCollector := telemetry.NewDataCollectorImpl(telemetry.DataCollectorConfig{ + K8sClientReader: mgr.GetAPIReader(), + GraphGetter: processor, + ConfigurationGetter: eventHandler, + Version: cfg.Version, + PodNSName: types.NamespacedName{ + Namespace: cfg.GatewayPodConfig.Namespace, + Name: cfg.GatewayPodConfig.Name, + }, + ImageSource: cfg.ImageSource, + }) + if err = mgr.Add(createTelemetryJob(cfg, dataCollector, nginxChecker.getReadyCh())); err != nil { + return fmt.Errorf("cannot register telemetry job: %w", err) + } } cfg.Logger.Info("Starting manager") @@ -473,7 +475,7 @@ func createTelemetryJob( runnables.CronJobConfig{ Worker: telemetry.CreateTelemetryJobWorker(logger, exporter, dataCollector), Logger: logger, - Period: cfg.TelemetryReportPeriod, + Period: cfg.ProductTelemetryConfig.TelemetryReportPeriod, JitterFactor: telemetryJitterFactor, ReadyCh: readyCh, }, @@ -501,7 +503,7 @@ func createUsageReporterJob( Runnable: runnables.NewCronJob(runnables.CronJobConfig{ Worker: usage.CreateUsageJobWorker(logger, k8sClient, reporter, cfg), Logger: logger, - Period: cfg.TelemetryReportPeriod, + Period: cfg.ProductTelemetryConfig.TelemetryReportPeriod, JitterFactor: telemetryJitterFactor, ReadyCh: readyCh, }), diff --git a/site/content/reference/cli-help.md b/site/content/reference/cli-help.md index c306a29ce..f40c1067b 100644 --- a/site/content/reference/cli-help.md +++ b/site/content/reference/cli-help.md @@ -36,6 +36,7 @@ _Usage_: | _health-port_ | _int_ | Set the port where the health probe server is exposed. An integer between 1024 - 65535 (Default: `8081`). | | _leader-election-disable_ | _bool_ | Disable leader election, which is used to avoid multiple replicas of the NGINX Gateway Fabric reporting the status of the Gateway API resources. If disabled, all replicas of NGINX Gateway Fabric will update the statuses of the Gateway API resources (Default: `false`). | | _leader-election-lock-name_ | _string_ | The name of the leader election lock. A lease object with this name will be created in the same namespace as the controller (Default: `"nginx-gateway-leader-election-lock"`). | +| _product-telemetry-disable_ | _bool_ | Disable the collection of product telemetry (Default: `false`). | | _usage-report-secret_ | _string_ | The namespace/name of the Secret containing the credentials for NGINX Plus usage reporting. | | _usage-report-server-url_ | _string_ | The base server URL of the NGINX Plus usage reporting server. | | _usage-report-cluster-name_ | _string_ | The display name of the Kubernetes cluster in the NGINX Plus usage reporting server. |