Harden Istio injection check (#7)

- Update istio injection check to validate against mutating webhooks instead of solely on labels themselves. This is similar to the `istioctl x check-inject` method.
- Add `--max-processors` flag to limit the number of parallel processes during namespace/pod and node processing.

Author: inFocus7

.github/workflows/pull-request.yaml

@@ -37,7 +37,7 @@ jobs:
   e2e-tests:
     name: Run E2E Tests
     runs-on: ubuntu-24.04
-    timeout-minutes: 10 # As of 04-11-2025, tests usually complete around 2 minute, so 10 minutes is a safe timeout.
+    timeout-minutes: 10 # As of 04-30-2025, tests usually complete in around 3 minutes, so 10 minutes is a safe timeout.
     steps:
       - uses: actions/checkout@v4
         with:

Makefile

@@ -83,6 +83,9 @@ run-unit-tests:
 run-e2e-tests:
 	@gotestsum --junitfile junit-e2e-test.xml -- -tags=e2e ./...
 
+run-performance-tests:
+	@gotestsum --junitfile junit-performance-test.xml -- -tags=performance ./...
+
 add-test-dependencies:
 	@helm repo add metrics-server "https://kubernetes-sigs.github.io/metrics-server/"
 	@helm repo add istio "https://istio-release.storage.googleapis.com/charts"

README.md

@@ -14,6 +14,8 @@ The Istio Usage Collector is a Go implementation of the [`gather-cluster-info.sh
 
 This data is collected into a JSON or YAML file that can be used for further analysis through our detailed migration estimator tool.
 
+Istio data gathered and reported is based on automatic sidecar injection defined within Istio's MutatingAdmissionWebhooks.
+
 ## Installation
 
 ### Downloading release

changelogs/v0.1.5.yaml

@@ -0,0 +1,8 @@
+new-feature:
+- Add a `max-processors` flag to the command to limit the number of processors used.
+
+enhancement:
+- Use Istio's MutatingAdmissionWebhooks to check for automatic istio injection instead of manual label checks. This is to simulate Istio's `istioctl x check-inject` command.
+
+other:
+- Add performance tests for the namespace processor.

cmd/root.go

@@ -24,6 +24,7 @@ type CommandFlags struct {
 	OutputFilePrefix   string
 	EnableDebug        bool
 	NoProgress         bool
+	MaxProcessors      int
 }
 
 // DefaultFlags returns a CommandFlags struct initialized with default values
@@ -37,6 +38,7 @@ func DefaultFlags() *CommandFlags {
 		OutputFilePrefix:   "",
 		EnableDebug:        false,
 		NoProgress:         false,
+		MaxProcessors:      0,
 	}
 }
 
@@ -135,6 +137,7 @@ func GetCommand(customFlags ...*CommandFlags) *cobra.Command {
 				OutputFormat:       flags.OutputFormat,
 				OutputFilePrefix:   prefix,
 				NoProgress:         flags.NoProgress,
+				MaxProcessors:      flags.MaxProcessors,
 			}
 
 			// Gather cluster information
@@ -157,6 +160,7 @@ func GetCommand(customFlags ...*CommandFlags) *cobra.Command {
 	cmd.PersistentFlags().StringVarP(&flags.OutputFilePrefix, "output-prefix", "p", "", "Custom prefix for the output file. If not set, uses the cluster name.")
 	cmd.PersistentFlags().BoolVar(&flags.EnableDebug, "debug", false, "Enable debug mode.")
 	cmd.PersistentFlags().BoolVar(&flags.NoProgress, "no-progress", false, "Disable the progress bar while processing resources.")
+	cmd.PersistentFlags().IntVar(&flags.MaxProcessors, "max-processors", 0, "Maximum number of processors to use. If not set, or <= 0, it will use all available processors.")
 
 	return cmd
 }

cmd/root_test.go

@@ -19,6 +19,7 @@ func TestRootCommandFlags(t *testing.T) {
 	assert.NotNil(t, cmd.Flag("output-prefix"))
 	assert.NotNil(t, cmd.Flag("debug"))
 	assert.NotNil(t, cmd.Flag("no-progress"))
+	assert.NotNil(t, cmd.Flag("max-processors"))
 
 	assert.Nil(t, cmd.Flag("version")) // This is only set for builds in standalone mode, not part of the command in general
 }
@@ -45,6 +46,7 @@ func TestRootCommandFlagParsing(t *testing.T) {
 				OutputFilePrefix:   "", // Default is empty string, where during execution, the context name is used
 				EnableDebug:        false,
 				NoProgress:         false,
+				MaxProcessors:      0,
 			},
 		},
 		{
@@ -58,6 +60,7 @@ func TestRootCommandFlagParsing(t *testing.T) {
 				"--output-prefix", "my-prefix",
 				"--debug",
 				"--no-progress",
+				"--max-processors", "1",
 			},
 			expectedFlags: CommandFlags{
 				HideNames:          true,
@@ -68,6 +71,7 @@ func TestRootCommandFlagParsing(t *testing.T) {
 				OutputFilePrefix:   "my-prefix",
 				EnableDebug:        true,
 				NoProgress:         true,
+				MaxProcessors:      1,
 			},
 		},
 		{
@@ -89,6 +93,7 @@ func TestRootCommandFlagParsing(t *testing.T) {
 				OutputFilePrefix:   "short-p",
 				EnableDebug:        false, // default
 				NoProgress:         false, // default
+				MaxProcessors:      0,     // default
 			},
 		},
 	}
@@ -134,6 +139,10 @@ func TestRootCommandFlagParsing(t *testing.T) {
 			noProgress, err := cmdFlags.GetBool("no-progress")
 			assert.NoError(t, err)
 			assert.Equal(t, tt.expectedFlags.NoProgress, noProgress, "Flag NoProgress mismatch")
+
+			maxProcessors, err := cmdFlags.GetInt("max-processors")
+			assert.NoError(t, err)
+			assert.Equal(t, tt.expectedFlags.MaxProcessors, maxProcessors, "Flag MaxProcessors mismatch")
 		})
 	}
 }

go.mod

@@ -3,6 +3,7 @@ module github.com/solo-io/istio-usage-collector
 go 1.23.5
 
 require (
+	github.com/google/go-cmp v0.6.0
 	github.com/pterm/pterm v0.12.80
 	github.com/spf13/cobra v1.9.1
 	github.com/stretchr/testify v1.10.0
@@ -11,6 +12,7 @@ require (
 	k8s.io/apimachinery v0.32.3
 	k8s.io/client-go v0.32.3
 	k8s.io/metrics v0.32.3
+	sigs.k8s.io/yaml v1.4.0
 )
 
 require (
@@ -28,7 +30,6 @@ require (
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/protobuf v1.5.4 // indirect
 	github.com/google/gnostic-models v0.6.8 // indirect
-	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/google/gofuzz v1.2.0 // indirect
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/gookit/color v1.5.4 // indirect
@@ -61,5 +62,4 @@ require (
 	k8s.io/utils v0.0.0-20241104100929-3ea5e8cea738 // indirect
 	sigs.k8s.io/json v0.0.0-20241010143419-9aa6b5e7a4b3 // indirect
 	sigs.k8s.io/structured-merge-diff/v4 v4.4.2 // indirect
-	sigs.k8s.io/yaml v1.4.0 // indirect
 )

internal/gatherer/gatherer.go

@@ -11,6 +11,8 @@ import (
 	"sync"
 	"time"
 
+	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
+
 	"github.com/solo-io/istio-usage-collector/internal/logging"
 	"github.com/solo-io/istio-usage-collector/internal/utils"
 	"github.com/solo-io/istio-usage-collector/pkg/models"
@@ -197,7 +199,12 @@ func processNodes(ctx context.Context, clientset kubernetes.Interface, metricsCl
 	errorCh := make(chan error, totalNodes)
 
 	// Use a semaphore to control concurrency
-	concurrentLimit := runtime.NumCPU() * 2
+	var concurrentLimit int
+	if cfg.MaxProcessors > 0 {
+		concurrentLimit = cfg.MaxProcessors
+	} else {
+		concurrentLimit = runtime.NumCPU() * 2
+	}
 	semaphore := make(chan struct{}, concurrentLimit)
 
 	logging.Debug("Processing nodes with up to %d concurrent requests", concurrentLimit)
@@ -326,10 +333,30 @@ func processNamespaces(ctx context.Context, clientset kubernetes.Interface, metr
 	errorCh := make(chan error, totalNamespaces)
 
 	// Use a semaphore to control concurrency based on system resources
-	concurrentLimit := runtime.NumCPU() * 4
+	var concurrentLimit int
+	if cfg.MaxProcessors > 0 {
+		concurrentLimit = cfg.MaxProcessors
+	} else {
+		concurrentLimit = runtime.NumCPU() * 4
+	}
 	semaphore := make(chan struct{}, concurrentLimit)
 
 	logging.Debug("Processing namespaces with up to %d concurrent requests", concurrentLimit)
+
+	// Get all mutating webhook configurations - istio uses mwhs to define its automatic sidecar injection policy
+	webhooks, err := clientset.AdmissionregistrationV1().MutatingWebhookConfigurations().List(ctx, metav1.ListOptions{})
+	if err != nil {
+		logging.Warn("Failed to list mutating webhook configurations: %v", err)
+	}
+	if webhooks == nil || len(webhooks.Items) == 0 {
+		logging.Warn("No mutating webhook configurations found in cluster %s", cfg.KubeContext)
+	}
+	// filter out non-istio webhooks
+	istioWebhooks := utils.FilterIstioWebhooks(webhooks.Items)
+	if len(istioWebhooks) == 0 {
+		logging.Warn("No Istio-related mutating webhook configurations found in cluster %s", cfg.KubeContext)
+	}
+
 	for _, ns := range namespaces.Items {
 		// Check parent context for cancellation before spawning more goroutines
 		if ctx.Err() != nil {
@@ -367,8 +394,7 @@ func processNamespaces(ctx context.Context, clientset kubernetes.Interface, metr
 				return
 			}
 
-			nsInfo, err := processNamespace(workerCtx, clientset, metricsClient, namespace.Name, hasMetrics)
-
+			nsInfo, err := processNamespace(workerCtx, clientset, metricsClient, namespace.Name, hasMetrics, istioWebhooks)
 			if progress != nil {
 				progress.Increment()
 			}
@@ -410,8 +436,9 @@ func processNamespaces(ctx context.Context, clientset kubernetes.Interface, metr
 	return nil
 }
 
-// processNamespace processes an individual namespace
-func processNamespace(ctx context.Context, clientset kubernetes.Interface, metricsClient metricsv.Interface, namespace string, hasMetrics bool) (*models.NamespaceInfo, error) {
+// processNamespace processes an individual namespace and its pods
+// TODO: We currently don't check for Sidecar CRs -- https://istio.io/latest/docs/reference/config/networking/sidecar/
+func processNamespace(ctx context.Context, clientset kubernetes.Interface, metricsClient metricsv.Interface, namespace string, hasMetrics bool, istioWebhooks []admissionregistrationv1.MutatingWebhookConfiguration) (*models.NamespaceInfo, error) {
 	if ctx.Err() != nil {
 		return nil, ctx.Err()
 	}
@@ -422,19 +449,6 @@ func processNamespace(ctx context.Context, clientset kubernetes.Interface, metri
 		return nil, fmt.Errorf("failed to get namespace details: %w", err)
 	}
 
-	isIstioInjected := false
-	if value, ok := ns.Labels["istio-injection"]; ok && value == "enabled" {
-		isIstioInjected = true
-	} else if _, ok := ns.Labels["istio.io/rev"]; ok {
-		isIstioInjected = true
-	}
-
-	if isIstioInjected {
-		logging.Debug("Namespace %s has Istio injection enabled", namespace)
-	} else {
-		logging.Debug("Namespace %s has no Istio injection", namespace)
-	}
-
 	// Get pods in the namespace
 	pods, err := clientset.CoreV1().Pods(namespace).List(ctx, metav1.ListOptions{})
 	if err != nil {
@@ -478,11 +492,26 @@ func processNamespace(ctx context.Context, clientset kubernetes.Interface, metri
 	istioCpuActual := 0.0
 	istioMemActual := 0.0
 
+	// Whether the namespace has at least one pod with istio injection
+	isIstioInjected := false
+
 	// Process all pods
 	for _, pod := range pods.Items {
+		// Check if istio injection is enabled on the pod-level
+		isPodIstioInjected := utils.CheckInject(istioWebhooks, pod.Labels, ns.Labels)
+
+		// If any pod within the namespace has istio injection occurring, we should count the namespace as having istio injected
+		isIstioInjected = isIstioInjected || isPodIstioInjected
+
 		// Check each container
 		for _, container := range pod.Spec.Containers {
-			isIstioProxy := container.Name == "istio-proxy"
+			// we only count istio-proxy container as an istio sidecar if the pod has istio injection enabled
+			isIstioProxyContainer := container.Name == "istio-proxy"
+			isIstioProxy := isIstioProxyContainer && isPodIstioInjected
+			if isIstioProxyContainer && !isPodIstioInjected {
+				// add a debug log if the pod has an istio-proxy container but istio injection is disabled, meaning we won't treat it as an istio sidecar
+				logging.Debug("%s.%s does not have istio injection enabled, treating its 'istio-proxy' container as a regular container", namespace, pod.Name)
+			}
 
 			// Count container types
 			if isIstioProxy {
@@ -539,9 +568,10 @@ func processNamespace(ctx context.Context, clientset kubernetes.Interface, metri
 		}
 	}
 
-	// Create namespace info
+	// Create namespace info (before appending actual resource usage and istio resources)
 	nsInfo := &models.NamespaceInfo{
-		Pods:            len(pods.Items),
+		Pods: len(pods.Items),
+		// the namespace had istio injected if it was either enabled on the namespace-level OR within any of its pods
 		IsIstioInjected: isIstioInjected,
 		Resources: models.ResourceInfo{
 			Regular: models.ContainerResources{
@@ -561,8 +591,8 @@ func processNamespace(ctx context.Context, clientset kubernetes.Interface, metri
 		}
 	}
 
-	// Add Istio resources if the namespace has Istio injection
-	if isIstioInjected {
+	// Only add the Istio resources field if the namespace contained at least one pod with istio injection
+	if nsInfo.IsIstioInjected {
 		nsInfo.Resources.Istio = &models.ContainerResources{
 			Containers: istioContainers,
 			Request: models.Resources{
@@ -578,7 +608,6 @@ func processNamespace(ctx context.Context, clientset kubernetes.Interface, metri
 			}
 		}
 	}
-
 	return nsInfo, nil
 }