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

Add Ingress Annotation List to Telemetry #5516

Merged
merged 10 commits into from
May 14, 2024
1 change: 1 addition & 0 deletions docs/content/overview/product-telemetry.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ These are the data points collected and reported by NGINX Ingress Controller:
- **Services** Number of Services referenced by VirtualServers, VirtualServerRoutes, TransportServers and Ingresses.
- **Ingresses** The number of Ingress resources managed by the NGINX Ingress Controller.
- **IngressClasses** Number of Ingress Classes in the cluster.
- **IngressAnnotations** List of Ingress annotations managed by NGINX Ingress Controller
- **AccessControlPolicies** Number of AccessControl policies.
- **RateLimitPolicies** Number of RateLimit policies.
- **JWTAuthPolicies** Number of JWTAuth policies.
Expand Down
8 changes: 8 additions & 0 deletions internal/configs/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,14 @@ var validPathRegex = map[string]bool{
"exact": true,
}

// List of Ingress Annotation Keys used by the Ingress Controller
var allowedAnnotationKeys = []string{
AlexFenlon marked this conversation as resolved.
Show resolved Hide resolved
"nginx.org",
"nginx.com",
"f5.com",
"ingress.kubernetes.io/ssl-redirect",
}

func parseAnnotations(ingEx *IngressEx, baseCfgParams *ConfigParams, isPlus bool, hasAppProtect bool, hasAppProtectDos bool, enableInternalRoutes bool) ConfigParams {
cfgParams := *baseCfgParams

Expand Down
54 changes: 53 additions & 1 deletion internal/configs/configurator.go
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,7 @@ func (cnf *Configurator) addOrUpdateTransportServer(transportServerEx *Transport
if err != nil {
return false, nil, err
}
return (changed || ptChanged), warnings, nil
return changed || ptChanged, warnings, nil
}
return changed, warnings, nil
}
Expand Down Expand Up @@ -1548,6 +1548,58 @@ func (cnf *Configurator) GetIngressCounts() map[string]int {
return counters
}

// GetIngressAnnotations returns a list of annotation keys set across all Ingress resources
func (cnf *Configurator) GetIngressAnnotations() []string {
if cnf == nil || cnf.ingresses == nil {
return nil
}

annotationSet := make(map[string]bool)
AlexFenlon marked this conversation as resolved.
Show resolved Hide resolved
annotationSet = cnf.getMinionIngressAnnotations(annotationSet)
annotationSet = cnf.getStandardIngressAnnotations(annotationSet)

var annotations []string
for key := range annotationSet {
annotations = append(annotations, key)
}

return annotations
}

// getStandardIngressAnnotations returns a map of allowedAnnotations detected in Standard or Master Ingresses
func (cnf *Configurator) getStandardIngressAnnotations(annotationSet map[string]bool) map[string]bool {
for _, ing := range cnf.ingresses {
if ing != nil && ing.Ingress != nil && ing.Ingress.Annotations != nil {
for key := range ing.Ingress.Annotations {
for _, allowedAnnotationKey := range allowedAnnotationKeys {
AlexFenlon marked this conversation as resolved.
Show resolved Hide resolved
if strings.Contains(key, allowedAnnotationKey) {
annotationSet[key] = true
}
}
}
}
}
return annotationSet
}

// getMinionIngressAnnotations returns a map of allowedAnnotations detected in Minion Ingresses
func (cnf *Configurator) getMinionIngressAnnotations(annotationSet map[string]bool) map[string]bool {
for _, ing := range cnf.mergeableIngresses {
for _, minionIng := range ing.Minions {
if minionIng != nil && minionIng.Ingress.Annotations != nil {
for key := range minionIng.Ingress.Annotations {
for _, allowedAnnotationKey := range allowedAnnotationKeys {
if strings.Contains(key, allowedAnnotationKey) {
annotationSet[key] = true
}
}
}
}
}
}
return annotationSet
}

// GetServiceCount returns the total number of unique services referenced by Ingresses, VS's, VSR's, and TS's
func (cnf *Configurator) GetServiceCount() int {
setOfUniqueServices := make(map[string]bool)
Expand Down
141 changes: 141 additions & 0 deletions internal/configs/configurator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1426,6 +1426,147 @@ func TestStreamUpstreamsForName_ReturnsStreamUpstreamsNamesOnValidServiceName(t
}
}

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

tcnf := createTestConfigurator(t)

ingress := &IngressEx{
Ingress: &networking.Ingress{
ObjectMeta: meta_v1.ObjectMeta{
Name: "test-ingress",
Namespace: "default",
Annotations: map[string]string{
"appprotect.f5.com/app-protect-enable": "False",
"nginx.org/proxy-set-header": "X-Forwarded-ABC",
"ingress.kubernetes.io/ssl-redirect": "True",
},
},
},
}

_, err := tcnf.AddOrUpdateIngress(ingress)
if err != nil {
t.Fatalf("AddOrUpdateIngress returned error: %v", err)
}

annotationList := tcnf.GetIngressAnnotations()

expectedAnnotations := []string{
"appprotect.f5.com/app-protect-enable",
"nginx.org/proxy-set-header",
"ingress.kubernetes.io/ssl-redirect",
}

if len(annotationList) != len(expectedAnnotations) {
t.Errorf("got %d annotations, want %d", len(annotationList), len(expectedAnnotations))
}

foundAnnotations := make(map[string]bool)
for _, annotation := range annotationList {
foundAnnotations[annotation] = true
}

for _, expected := range expectedAnnotations {
if !foundAnnotations[expected] {
t.Errorf("expected annotation %q not found", expected)
}
}
}

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

tcnf := createTestConfigurator(t)

ingress := &IngressEx{
Ingress: &networking.Ingress{
ObjectMeta: meta_v1.ObjectMeta{
Name: "test-ingress",
Namespace: "default",
Annotations: map[string]string{
"kubectl.kubernetes.io/last-applied-configuration": "s",
"alb.ingress.kubernetes.io/group.order": "0",
"alb.ingress.kubernetes.io/ip-address-type": "ipv4",
"alb.ingress.kubernetes.io/scheme": "internal",
},
},
},
}

_, err := tcnf.AddOrUpdateIngress(ingress)
if err != nil {
t.Fatalf("AddOrUpdateIngress returned error: %v", err)
}

expectedAnnotations := []string{
"alb.ingress.kubernetes.io/scheme",
"alb.ingress.kubernetes.io/group.order",
"alb.ingress.kubernetes.io/ip-address-type",
}

annotationList := tcnf.GetIngressAnnotations()

foundAnnotations := make(map[string]bool)
for _, annotation := range annotationList {
foundAnnotations[annotation] = true
}

for _, expected := range expectedAnnotations {
if foundAnnotations[expected] {
t.Errorf("expected annotation %q not found", expected)
}
}
}

AlexFenlon marked this conversation as resolved.
Show resolved Hide resolved
func TestGetMixedIngressAnnotations(t *testing.T) {
t.Parallel()

tcnf := createTestConfigurator(t)

ingress := &IngressEx{
Ingress: &networking.Ingress{
ObjectMeta: meta_v1.ObjectMeta{
Name: "test-ingress",
Namespace: "default",
Annotations: map[string]string{
"kubectl.kubernetes.io/last-applied-configuration": "s",
"alb.ingress.kubernetes.io/group.order": "0",
"alb.ingress.kubernetes.io/ip-address-type": "ipv4",
"alb.ingress.kubernetes.io/scheme": "internal",
"appprotect.f5.com/app-protect-enable": "False",
"nginx.org/proxy-set-header": "X-Forwarded-ABC",
"ingress.kubernetes.io/ssl-redirect": "True",
},
},
},
}

_, err := tcnf.AddOrUpdateIngress(ingress)
if err != nil {
t.Fatalf("AddOrUpdateIngress returned error: %v", err)
}

expectedAnnotations := []string{
"ingress.kubernetes.io/ssl-redirect",
"nginx.org/proxy-set-header",
"appprotect.f5.com/app-protect-enable",
}

annotationList := tcnf.GetIngressAnnotations()

foundAnnotations := make(map[string]bool)
for _, annotation := range annotationList {
foundAnnotations[annotation] = true
}

for _, expected := range expectedAnnotations {
if !foundAnnotations[expected] {
t.Errorf("expected annotation %q not found", expected)
}
}
}

var (
invalidVirtualServerEx = &VirtualServerEx{
VirtualServer: &conf_v1.VirtualServer{},
Expand Down
9 changes: 9 additions & 0 deletions internal/telemetry/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,15 @@ func (c *Collector) IngressCount() int {
return total
}

// IngressAnnotations returns a list of all the unique annotations found in Ingresses.
func (c *Collector) IngressAnnotations() []string {
if c.Config.Configurator == nil {
return nil
}
annotations := c.Config.Configurator.GetIngressAnnotations()
return annotations
}

// IngressClassCount returns number of Ingress Classes.
func (c *Collector) IngressClassCount(ctx context.Context) (int, error) {
ic, err := c.Config.K8sClientReader.NetworkingV1().IngressClasses().List(ctx, metaV1.ListOptions{})
Expand Down
5 changes: 5 additions & 0 deletions internal/telemetry/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ func (c *Collector) Collect(ctx context.Context) {
OIDCPolicies: int64(report.OIDCCount),
WAFPolicies: int64(report.WAFCount),
GlobalConfiguration: report.GlobalConfiguration,
IngressAnnotations: report.IngressAnnotations,
},
}

Expand Down Expand Up @@ -171,6 +172,7 @@ type Report struct {
OIDCCount int
WAFCount int
GlobalConfiguration bool
IngressAnnotations []string
}

// BuildReport takes context, collects telemetry data and builds the report.
Expand Down Expand Up @@ -237,6 +239,8 @@ func (c *Collector) BuildReport(ctx context.Context) (Report, error) {
oidcCount := policies["OIDC"]
wafCount := policies["WAF"]

ingressAnnotations := c.IngressAnnotations()

return Report{
Name: "NIC",
Version: c.Config.Version,
Expand All @@ -263,5 +267,6 @@ func (c *Collector) BuildReport(ctx context.Context) (Report, error) {
OIDCCount: oidcCount,
WAFCount: wafCount,
GlobalConfiguration: c.Config.GlobalConfiguration,
IngressAnnotations: ingressAnnotations,
}, err
}
Loading
Loading