diff --git a/charts/nginx-ingress/values.yaml b/charts/nginx-ingress/values.yaml index bbe95d2cd6..1595529946 100644 --- a/charts/nginx-ingress/values.yaml +++ b/charts/nginx-ingress/values.yaml @@ -265,19 +265,19 @@ controller: ## The Ingress Controller processes all the resources that do not have the "ingressClassName" field for all versions of kubernetes. name: nginx - ## Creates a new IngressClass object with the name "controller.ingressClass.name". Set to false to use an existing IngressClass with the same name. If you use helm upgrade, do not change the values from the previous release as helm will delete IngressClass objects managed by helm. If you are upgrading from a release earlier than 3.3.0, do not set the value to false. + ## Creates a new IngressClass object with the name "controller.ingressClass.name". To use an existing IngressClass with the same name, set this value to false. If you use helm upgrade, do not change the values from the previous release as helm will delete IngressClass objects managed by helm. If you are upgrading from a release earlier than 3.3.0, do not set the value to false. create: true ## New Ingresses without an ingressClassName field specified will be assigned the class specified in `controller.ingressClass`. Requires "controller.ingressClass.create". setAsDefaultIngress: false - ## Comma separated list of namespaces to watch for Ingress resources. By default the Ingress Controller watches all namespaces. Mutually exclusive with "controller.watchNamespaceLabel". + ## Comma separated list of namespaces to watch for Ingress resources. By default, the Ingress Controller watches all namespaces. Mutually exclusive with "controller.watchNamespaceLabel". watchNamespace: "" - ## Configures the Ingress Controller to watch only those namespaces with label foo=bar. By default the Ingress Controller watches all namespaces. Mutually exclusive with "controller.watchNamespace". + ## Configures the Ingress Controller to watch only those namespaces with label foo=bar. By default, the Ingress Controller watches all namespaces. Mutually exclusive with "controller.watchNamespace". watchNamespaceLabel: "" - ## Comma separated list of namespaces to watch for Secret resources. By default the Ingress Controller watches all namespaces. + ## Comma separated list of namespaces to watch for Secret resources. By default, the Ingress Controller watches all namespaces. watchSecretNamespace: "" ## Enable the custom resources. diff --git a/cmd/nginx-ingress/main.go b/cmd/nginx-ingress/main.go index 95326892b6..cbd1701dad 100644 --- a/cmd/nginx-ingress/main.go +++ b/cmd/nginx-ingress/main.go @@ -135,7 +135,7 @@ func main() { nginxManager.CreateTLSPassthroughHostsConfig(emptyFile) } - cpcfg := startChildProcesses(nginxManager) + process := startChildProcesses(nginxManager) plusClient := createPlusClient(*nginxPlus, useFakeNginxManager, nginxManager) @@ -227,7 +227,7 @@ func main() { }() } - go handleTermination(lbc, nginxManager, syslogListener, cpcfg) + go handleTermination(lbc, nginxManager, syslogListener, process) lbc.Run() @@ -439,7 +439,7 @@ func getAgentVersionInfo(nginxManager nginx.Manager) string { return nginxManager.AgentVersion() } -type childProcessConfig struct { +type childProcesses struct { nginxDone chan error aPPluginEnable bool aPPluginDone chan error @@ -449,7 +449,9 @@ type childProcessConfig struct { agentDone chan error } -func startChildProcesses(nginxManager nginx.Manager) childProcessConfig { +// newChildProcesses starts the several child processes based on flags set. +// AppProtect. AppProtectDos, Agent. +func startChildProcesses(nginxManager nginx.Manager) childProcesses { var aPPluginDone chan error if *appProtect { @@ -473,7 +475,7 @@ func startChildProcesses(nginxManager nginx.Manager) childProcessConfig { nginxManager.AgentStart(agentDone, *agentInstanceGroup) } - return childProcessConfig{ + return childProcesses{ nginxDone: nginxDone, aPPluginEnable: *appProtect, aPPluginDone: aPPluginDone, @@ -566,7 +568,7 @@ func processNginxConfig(staticCfgParams *configs.StaticConfigParams, cfgParams * } } -// getSocketClient gets an http.Client with the a unix socket transport. +// getSocketClient gets a http.Client with a unix socket transport. func getSocketClient(sockPath string) *http.Client { return &http.Client{ Transport: &http.Transport{ @@ -594,7 +596,7 @@ func getAndValidateSecret(kubeClient *kubernetes.Clientset, secretNsName string) return secret, nil } -func handleTermination(lbc *k8s.LoadBalancerController, nginxManager nginx.Manager, listener metrics.SyslogListener, cpcfg childProcessConfig) { +func handleTermination(lbc *k8s.LoadBalancerController, nginxManager nginx.Manager, listener metrics.SyslogListener, cpcfg childProcesses) { signalChan := make(chan os.Signal, 1) signal.Notify(signalChan, syscall.SIGTERM) diff --git a/docs/content/overview/product-telemetry.md b/docs/content/overview/product-telemetry.md index 641c5624eb..b8660bca92 100644 --- a/docs/content/overview/product-telemetry.md +++ b/docs/content/overview/product-telemetry.md @@ -32,6 +32,7 @@ These are the data points collected and reported by NGINX Ingress Controller: - **VirtualServerRoutes** The number of VirtualServerRoute resources managed by NGINX Ingress Controller. - **TransportServers** The number of TransportServer resources managed by NGINX Ingress Controller. - **Replicas** Number of Deployment replicas, or Daemonset instances. +- **Secrets** Number of Secret resources managed by NGINX Ingress Controller. ## Opt out diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index eec50e05fb..3728ce1854 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -364,6 +364,7 @@ func NewLoadBalancerController(input NewLoadBalancerControllerInput) *LoadBalanc CustomK8sClientReader: input.ConfClient, Period: 24 * time.Hour, Configurator: lbc.configurator, + SecretStore: lbc.secretStore, Version: input.NICVersion, PodNSName: types.NamespacedName{ Namespace: os.Getenv("POD_NAMESPACE"), diff --git a/internal/k8s/secrets/store.go b/internal/k8s/secrets/store.go index 3352fe910c..b810e6ec24 100644 --- a/internal/k8s/secrets/store.go +++ b/internal/k8s/secrets/store.go @@ -25,6 +25,7 @@ type SecretStore interface { AddOrUpdateSecret(secret *api_v1.Secret) DeleteSecret(key string) GetSecret(key string) *SecretReference + GetSecretReferenceMap() map[string]*SecretReference } // LocalSecretStore implements SecretStore interface. @@ -101,6 +102,11 @@ func (s *LocalSecretStore) GetSecret(key string) *SecretReference { return secretRef } +// GetSecretReferenceMap returns a map that maps a secret key to a SecretReference +func (s *LocalSecretStore) GetSecretReferenceMap() map[string]*SecretReference { + return s.secrets +} + func getResourceKey(meta *metav1.ObjectMeta) string { return fmt.Sprintf("%s/%s", meta.Namespace, meta.Name) } @@ -150,3 +156,8 @@ func (s *FakeSecretStore) GetSecret(key string) *SecretReference { return secretRef } + +// GetSecretReferenceMap returns a map that maps a secret key to a SecretReference +func (s *FakeSecretStore) GetSecretReferenceMap() map[string]*SecretReference { + return s.secrets +} diff --git a/internal/k8s/secrets/store_test.go b/internal/k8s/secrets/store_test.go index 58dd5e63b0..f27f83fa94 100644 --- a/internal/k8s/secrets/store_test.go +++ b/internal/k8s/secrets/store_test.go @@ -2,6 +2,7 @@ package secrets import ( "errors" + "fmt" "testing" "github.com/google/go-cmp/cmp" @@ -101,7 +102,7 @@ func TestAddOrUpdateSecret(t *testing.T) { // Make the secret invalid expectedManager = &fakeSecretFileManager{ - DeletedSecret: "default/tls-secret", + DeletedSecret: fmt.Sprintf("%s/%s", validSecret.Namespace, validSecret.Name), } manager.Reset() @@ -116,7 +117,7 @@ func TestAddOrUpdateSecret(t *testing.T) { expectedSecretRef = &SecretReference{ Secret: invalidSecret, Path: "", - Error: errors.New("Failed to validate TLS cert and key: x509: malformed certificate"), + Error: errors.New("failed to validate TLS cert and key: x509: malformed certificate"), } expectedManager = &fakeSecretFileManager{} @@ -303,3 +304,38 @@ func TestDeleteSecretInvalidSecret(t *testing.T) { t.Errorf("DeleteSecret() returned unexpected result (-want +got):\n%s", diff) } } + +func TestGetSecretReferenceMapAddSecret(t *testing.T) { + t.Parallel() + + testCases := []struct { + testName string + manager *fakeSecretFileManager + secret *api_v1.Secret + expectedSecretKey string + }{ + { + testName: "Add valid secret to store", + manager: &fakeSecretFileManager{}, + secret: validSecret, + expectedSecretKey: getResourceKey(&validSecret.ObjectMeta), + }, + { + testName: "Add invalid secret to store", + manager: &fakeSecretFileManager{}, + secret: invalidSecret, + expectedSecretKey: getResourceKey(&invalidSecret.ObjectMeta), + }, + } + + for _, test := range testCases { + store := NewLocalSecretStore(test.manager) + store.AddOrUpdateSecret(test.secret) + + secretRefMap := store.GetSecretReferenceMap() + + if _, ok := secretRefMap[test.expectedSecretKey]; !ok { + t.Errorf("test case %s expected secret %v to be in store", test.testName, store.GetSecret(test.expectedSecretKey).Secret) + } + } +} diff --git a/internal/k8s/secrets/validation.go b/internal/k8s/secrets/validation.go index 80ffdfe22b..0f2908a5f0 100644 --- a/internal/k8s/secrets/validation.go +++ b/internal/k8s/secrets/validation.go @@ -44,7 +44,7 @@ func ValidateTLSSecret(secret *api_v1.Secret) error { _, err := tls.X509KeyPair(secret.Data[api_v1.TLSCertKey], secret.Data[api_v1.TLSPrivateKeyKey]) if err != nil { - return fmt.Errorf("Failed to validate TLS cert and key: %w", err) + return fmt.Errorf("failed to validate TLS cert and key: %w", err) } return nil @@ -86,7 +86,7 @@ func ValidateCASecret(secret *api_v1.Secret) error { _, err := x509.ParseCertificate(block.Bytes) if err != nil { - return fmt.Errorf("Failed to validate certificate: %w", err) + return fmt.Errorf("failed to validate certificate: %w", err) } return nil diff --git a/internal/telemetry/cluster.go b/internal/telemetry/cluster.go index 167fefcd6a..220ef40a0f 100644 --- a/internal/telemetry/cluster.go +++ b/internal/telemetry/cluster.go @@ -115,6 +115,14 @@ func (c *Collector) InstallationID(ctx context.Context) (_ string, err error) { } } +// Secrets returns the number of secrets watched by NIC. +func (c *Collector) Secrets() (int, error) { + if c.Config.SecretStore == nil { + return 0, errors.New("nil secret store") + } + return len(c.Config.SecretStore.GetSecretReferenceMap()), nil +} + // 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/cluster_test.go b/internal/telemetry/cluster_test.go index 7bfaa6257d..bf78a010a4 100644 --- a/internal/telemetry/cluster_test.go +++ b/internal/telemetry/cluster_test.go @@ -892,3 +892,24 @@ var ( }, } ) + +// Secrets for testing. +var ( + secret1 = &apiCoreV1.Secret{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "tls-secret-1", + Namespace: "default", + }, + Type: apiCoreV1.SecretTypeTLS, + Data: map[string][]byte{}, + } + + secret2 = &apiCoreV1.Secret{ + ObjectMeta: metaV1.ObjectMeta{ + Name: "tls-secret-2", + Namespace: "default", + }, + Type: apiCoreV1.SecretTypeTLS, + Data: map[string][]byte{}, + } +) diff --git a/internal/telemetry/collector.go b/internal/telemetry/collector.go index e3af28c595..94b68ff0f9 100644 --- a/internal/telemetry/collector.go +++ b/internal/telemetry/collector.go @@ -7,6 +7,8 @@ import ( "runtime" "time" + "github.com/nginxinc/kubernetes-ingress/internal/k8s/secrets" + tel "github.com/nginxinc/telemetry-exporter/pkg/telemetry" "github.com/nginxinc/kubernetes-ingress/internal/configs" @@ -57,6 +59,9 @@ type CollectorConfig struct { Configurator *configs.Configurator + // SecretStore for access to secrets managed by NIC. + SecretStore secrets.SecretStore + // Version represents NIC version. Version string @@ -110,6 +115,7 @@ func (c *Collector) Collect(ctx context.Context) { VirtualServerRoutes: int64(report.VirtualServerRoutes), TransportServers: int64(report.TransportServers), Replicas: int64(report.NICReplicaCount), + Secrets: int64(report.Secrets), }, } @@ -136,6 +142,7 @@ type Report struct { VirtualServers int VirtualServerRoutes int TransportServers int + Secrets int } // BuildReport takes context, collects telemetry data and builds the report. @@ -179,6 +186,11 @@ func (c *Collector) BuildReport(ctx context.Context) (Report, error) { glog.Errorf("Error collecting telemetry data: InstallationID: %v", err) } + secrets, err := c.Secrets() + if err != nil { + glog.Errorf("Error collecting telemetry data: Secrets: %v", err) + } + return Report{ Name: "NIC", Version: c.Config.Version, @@ -192,5 +204,6 @@ func (c *Collector) BuildReport(ctx context.Context) (Report, error) { VirtualServers: vsCount, VirtualServerRoutes: vsrCount, TransportServers: tsCount, + Secrets: secrets, }, err } diff --git a/internal/telemetry/collector_test.go b/internal/telemetry/collector_test.go index 5e177cbe4b..aa9343618d 100644 --- a/internal/telemetry/collector_test.go +++ b/internal/telemetry/collector_test.go @@ -8,6 +8,8 @@ import ( "testing" "time" + "github.com/nginxinc/kubernetes-ingress/internal/k8s/secrets" + "github.com/nginxinc/kubernetes-ingress/internal/configs" "github.com/nginxinc/kubernetes-ingress/internal/configs/version1" "github.com/nginxinc/kubernetes-ingress/internal/configs/version2" @@ -236,8 +238,8 @@ func TestCollectClusterVersion(t *testing.T) { } td := telemetry.Data{ - telData, - nicResourceCounts, + Data: telData, + NICResourceCounts: nicResourceCounts, } want := fmt.Sprintf("%+v", &td) @@ -345,6 +347,7 @@ func TestCountVirtualServers(t *testing.T) { c, err := telemetry.NewCollector(telemetry.CollectorConfig{ K8sClientReader: newTestClientset(kubeNS, node1, pod1, replica), + SecretStore: newSecretStore(t), Configurator: configurator, Version: telemetryNICData.ProjectVersion, }) @@ -510,6 +513,7 @@ func TestCountTransportServers(t *testing.T) { c, err := telemetry.NewCollector(telemetry.CollectorConfig{ K8sClientReader: newTestClientset(kubeNS, node1, pod1, replica), + SecretStore: newSecretStore(t), Configurator: configurator, Version: telemetryNICData.ProjectVersion, }) @@ -522,7 +526,7 @@ func TestCountTransportServers(t *testing.T) { } for _, ts := range test.transportServers { - _, err := configurator.AddOrUpdateTransportServer(ts) + _, err = configurator.AddOrUpdateTransportServer(ts) if err != nil { t.Fatal(err) } @@ -540,7 +544,7 @@ func TestCountTransportServers(t *testing.T) { for i := 0; i < test.deleteCount; i++ { ts := test.transportServers[i] key := getResourceKey(ts.TransportServer.Namespace, ts.TransportServer.Name) - err := configurator.DeleteTransportServer(key) + err = configurator.DeleteTransportServer(key) if err != nil { t.Fatal(err) } @@ -557,6 +561,111 @@ func TestCountTransportServers(t *testing.T) { } } +func TestCountSecretsWithTwoSecrets(t *testing.T) { + t.Parallel() + + buf := &bytes.Buffer{} + exp := &telemetry.StdoutExporter{Endpoint: buf} + cfg := telemetry.CollectorConfig{ + Configurator: newConfigurator(t), + K8sClientReader: newTestClientset(node1, kubeNS), + SecretStore: newSecretStore(t), + Version: telemetryNICData.ProjectVersion, + } + + // Add multiple secrets. + cfg.SecretStore.AddOrUpdateSecret(secret1) + cfg.SecretStore.AddOrUpdateSecret(secret2) + + c, err := telemetry.NewCollector(cfg, telemetry.WithExporter(exp)) + if err != nil { + t.Fatal(err) + } + c.Collect(context.Background()) + + telData := tel.Data{ + ProjectName: telemetryNICData.ProjectName, + ProjectVersion: telemetryNICData.ProjectVersion, + ProjectArchitecture: telemetryNICData.ProjectArchitecture, + ClusterNodeCount: 1, + ClusterID: telemetryNICData.ClusterID, + ClusterVersion: telemetryNICData.ClusterVersion, + ClusterPlatform: "other", + } + + nicResourceCounts := telemetry.NICResourceCounts{ + VirtualServers: 0, + VirtualServerRoutes: 0, + TransportServers: 0, + Secrets: 2, + } + + td := telemetry.Data{ + Data: telData, + NICResourceCounts: nicResourceCounts, + } + + want := fmt.Sprintf("%+v", &td) + got := buf.String() + if !cmp.Equal(want, got) { + t.Error(cmp.Diff(want, got)) + } +} + +func TestCountSecretsAddTwoSecretsAndDeleteOne(t *testing.T) { + t.Parallel() + + buf := &bytes.Buffer{} + exp := &telemetry.StdoutExporter{Endpoint: buf} + cfg := telemetry.CollectorConfig{ + Configurator: newConfigurator(t), + K8sClientReader: newTestClientset(node1, kubeNS), + SecretStore: newSecretStore(t), + Version: telemetryNICData.ProjectVersion, + } + + // Add multiple secrets. + cfg.SecretStore.AddOrUpdateSecret(secret1) + cfg.SecretStore.AddOrUpdateSecret(secret2) + + // Delete one secret. + cfg.SecretStore.DeleteSecret(fmt.Sprintf("%s/%s", secret2.Namespace, secret2.Name)) + + c, err := telemetry.NewCollector(cfg, telemetry.WithExporter(exp)) + if err != nil { + t.Fatal(err) + } + c.Collect(context.Background()) + + telData := tel.Data{ + ProjectName: telemetryNICData.ProjectName, + ProjectVersion: telemetryNICData.ProjectVersion, + ProjectArchitecture: telemetryNICData.ProjectArchitecture, + ClusterNodeCount: 1, + ClusterID: telemetryNICData.ClusterID, + ClusterVersion: telemetryNICData.ClusterVersion, + ClusterPlatform: "other", + } + + nicResourceCounts := telemetry.NICResourceCounts{ + VirtualServers: 0, + VirtualServerRoutes: 0, + TransportServers: 0, + Secrets: 1, + } + + td := telemetry.Data{ + Data: telData, + NICResourceCounts: nicResourceCounts, + } + + want := fmt.Sprintf("%+v", &td) + got := buf.String() + if !cmp.Equal(want, got) { + t.Error(cmp.Diff(want, got)) + } +} + func getResourceKey(namespace, name string) string { return fmt.Sprintf("%s_%s", namespace, name) } @@ -599,6 +708,12 @@ func newConfigurator(t *testing.T) *configs.Configurator { return cnf } +func newSecretStore(t *testing.T) *secrets.LocalSecretStore { + t.Helper() + configurator := newConfigurator(t) + return secrets.NewLocalSecretStore(configurator) +} + // newTestClientset takes k8s runtime objects and returns a k8s fake clientset. // The clientset is configured to return kubernetes version v1.29.2. // (call to Discovery().ServerVersion()) diff --git a/internal/telemetry/data.avdl b/internal/telemetry/data.avdl index ea774378fa..96c59e738c 100644 --- a/internal/telemetry/data.avdl +++ b/internal/telemetry/data.avdl @@ -39,11 +39,14 @@ It is the UID of the `kube-system` Namespace. */ /** VirtualServerRoutes is the number of VirtualServerRoute resources managed by the Ingress Controller. */ long? VirtualServerRoutes = null; - /** TransportServers is the number of TransportServer resources by the Ingress Controller. */ + /** TransportServers is the number of TransportServer resources managed by the Ingress Controller. */ long? TransportServers = null; /** Replicas is the number of NIC replicas. */ long? Replicas = null; + /** Secrets is the number of Secret resources managed by the Ingress Controller. */ + long? Secrets = null; + } } diff --git a/internal/telemetry/exporter.go b/internal/telemetry/exporter.go index 01cd2a9800..211474ede0 100644 --- a/internal/telemetry/exporter.go +++ b/internal/telemetry/exporter.go @@ -66,9 +66,10 @@ type NICResourceCounts struct { VirtualServers int64 // VirtualServerRoutes is the number of VirtualServerRoute resources managed by the Ingress Controller. VirtualServerRoutes int64 - // TransportServers is the number of TransportServer resources by the Ingress Controller. + // TransportServers is the number of TransportServer resources managed by the Ingress Controller. TransportServers int64 - // Replicas is the number of NIC replicas. Replicas int64 + // Secrets is the number of Secret resources managed by the Ingress Controller. + Secrets int64 } diff --git a/internal/telemetry/nicresourcecounts_attributes_generated.go b/internal/telemetry/nicresourcecounts_attributes_generated.go index 8e0d8599a9..61fae501ae 100644 --- a/internal/telemetry/nicresourcecounts_attributes_generated.go +++ b/internal/telemetry/nicresourcecounts_attributes_generated.go @@ -17,6 +17,7 @@ func (d *NICResourceCounts) Attributes() []attribute.KeyValue { attrs = append(attrs, attribute.Int64("VirtualServerRoutes", d.VirtualServerRoutes)) attrs = append(attrs, attribute.Int64("TransportServers", d.TransportServers)) attrs = append(attrs, attribute.Int64("Replicas", d.Replicas)) + attrs = append(attrs, attribute.Int64("Secrets", d.Secrets)) return attrs }