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

Collect count of secrets #5404

Merged
merged 8 commits into from
Apr 16, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
8 changes: 4 additions & 4 deletions charts/nginx-ingress/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
16 changes: 9 additions & 7 deletions cmd/nginx-ingress/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func main() {
nginxManager.CreateTLSPassthroughHostsConfig(emptyFile)
}

cpcfg := startChildProcesses(nginxManager)
process := startChildProcesses(nginxManager)

plusClient := createPlusClient(*nginxPlus, useFakeNginxManager, nginxManager)

Expand Down Expand Up @@ -227,7 +227,7 @@ func main() {
}()
}

go handleTermination(lbc, nginxManager, syslogListener, cpcfg)
go handleTermination(lbc, nginxManager, syslogListener, process)

lbc.Run()

Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -473,7 +475,7 @@ func startChildProcesses(nginxManager nginx.Manager) childProcessConfig {
nginxManager.AgentStart(agentDone, *agentInstanceGroup)
}

return childProcessConfig{
return childProcesses{
nginxDone: nginxDone,
aPPluginEnable: *appProtect,
aPPluginDone: aPPluginDone,
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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)

Expand Down
1 change: 1 addition & 0 deletions docs/content/overview/product-telemetry.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions internal/k8s/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
6 changes: 6 additions & 0 deletions internal/k8s/secrets/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -101,6 +102,11 @@ func (s *LocalSecretStore) GetSecret(key string) *SecretReference {
return secretRef
}

// GetSecretReferenceMap returns a map that maps a secret key <namespace/name> 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)
}
Expand Down
58 changes: 56 additions & 2 deletions internal/k8s/secrets/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package secrets

import (
"errors"
"fmt"
"testing"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -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()
Expand All @@ -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{}

Expand Down Expand Up @@ -303,3 +304,56 @@ 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)
}
}
}

func TestGetSecretReferenceMapAddInvalidSecret(t *testing.T) {
t.Parallel()

manager := &fakeSecretFileManager{}
store := NewLocalSecretStore(manager)
store.AddOrUpdateSecret(invalidSecret)

secretRefMap := store.GetSecretReferenceMap()

validSecretKey := getResourceKey(&invalidSecret.ObjectMeta)

storedSecretKey := getResourceKey(&invalidSecret.ObjectMeta)

if _, ok := secretRefMap[storedSecretKey]; ok {
t.Errorf("expected secret %v not to be in store", store.GetSecret(validSecretKey).Secret)
}
}
8 changes: 8 additions & 0 deletions internal/telemetry/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
21 changes: 21 additions & 0 deletions internal/telemetry/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -892,3 +892,24 @@ var (
},
}
)

// Secrets for testing.
var (
secret1 = &apiCoreV1.Secret{
j1m-ryan marked this conversation as resolved.
Show resolved Hide resolved
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{},
}
)
13 changes: 13 additions & 0 deletions internal/telemetry/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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),
},
}

Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -192,5 +204,6 @@ func (c *Collector) BuildReport(ctx context.Context) (Report, error) {
VirtualServers: vsCount,
VirtualServerRoutes: vsrCount,
TransportServers: tsCount,
Secrets: secrets,
}, err
}
Loading
Loading