setup opentelemetry trust incoming spans

kubernetes · Jul 22, 2022 · b29de25 · b29de25
1 parent ff6066a
commit b29de25
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 31 deletions.
diff --git a/internal/ingress/annotations/opentelemetry/main.go b/internal/ingress/annotations/opentelemetry/main.go
@@ -29,8 +29,10 @@ type opentelemetry struct {
 
 // Config contains the configuration to be used in the Ingress
 type Config struct {
-	Enabled bool `json:"enabled"`
-	Set     bool `json:"set"`
+	Enabled      bool `json:"enabled"`
+	Set          bool `json:"set"`
+	TrustEnabled bool `json:"trust-enabled"`
+	TrustSet     bool `json:"trust-set"`
 }
 
 // Equal tests for equality between two Config types
@@ -43,6 +45,10 @@ func (bd1 *Config) Equal(bd2 *Config) bool {
 		return false
 	}
 
+	if bd1.TrustEnabled != bd2.TrustEnabled {
+		return false
+	}
+
 	return true
 }
 
@@ -57,5 +63,10 @@ func (s opentelemetry) Parse(ing *networking.Ingress) (interface{}, error) {
 		return &Config{Set: false, Enabled: false}, nil
 	}
 
-	return &Config{Set: true, Enabled: enabled}, nil
+	trustSpan, err := parser.GetBoolAnnotation("opentelemetry-trust-incoming-span", ing)
+	if err != nil {
+		return &Config{Set: true, Enabled: enabled}, nil
+	}
+
+	return &Config{Set: true, Enabled: enabled, TrustSet: true, TrustEnabled: trustSpan}, nil
 }
diff --git a/internal/ingress/controller/config/config.go b/internal/ingress/controller/config/config.go
@@ -567,6 +567,11 @@ type Configuration struct {
 	// OpenTelemetryOperationName specifies a custom name for the server span
 	OpenTelemetryOperationName string `json:"opentelemetry-operation-name"`
 
+	// OpenTelemetryTrustIncomingSpan sets whether or not to trust incoming trace spans
+	// If false, incoming span headers will be rejected
+	// Default: true
+	OpenTelemetryTrustIncomingSpan bool `json:"opentelemetry-trust-incoming-span"`
+
 	// OtlpExporterHost defines the host of the OpenTelemetry collector instance
 	// where the data will be transmitted
 	OtlpCollectorHost string `json:"otlp-collector-host"`
@@ -925,6 +930,7 @@ func NewDefault() Configuration {
 		BindAddressIpv4:                        defBindAddress,
 		BindAddressIpv6:                        defBindAddress,
 		OpentracingTrustIncomingSpan:           true,
+		OpenTelemetryTrustIncomingSpan:         true,
 		ZipkinCollectorPort:                    9411,
 		ZipkinServiceName:                      "nginx",
 		ZipkinSampleRate:                       1.0,

diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go
@@ -1612,33 +1612,29 @@ func buildOpentracingForLocation(isOTEnabled bool, isOTTrustSet bool, location *
 	return opc
 }
 
-func buildOpenTelemetryForLocation(isOTEnabled bool, location *ingress.Location) string {
+func buildOpenTelemetryForLocation(isOTEnabled bool, isOTTrustSet bool, location *ingress.Location) string {
 	isOTEnabledInLoc := location.OpenTelemetry.Enabled
 	isOTSetInLoc := location.OpenTelemetry.Set
 
 	if isOTEnabled {
 		if isOTSetInLoc && !isOTEnabledInLoc {
 			return "opentelemetry off;"
 		}
-
-		opc := openTelemetryPropagateContext(location)
-		if opc != "" {
-			opc = fmt.Sprintf("opentelemetry on;\n%v", opc)
-		}
-
-		return opc
+	} else if !isOTSetInLoc || !isOTEnabledInLoc {
+		return ""
 	}
 
-	if isOTSetInLoc && isOTEnabledInLoc {
-		opc := openTelemetryPropagateContext(location)
-		if opc != "" {
-			opc = fmt.Sprintf("opentelemetry on;\n%v", opc)
-		}
+	opc := openTelemetryPropagateContext(location)
+	if opc != "" {
+		opc = fmt.Sprintf("opentelemetry on;\n%v", opc)
+	}
 
-		return opc
+	if (!isOTTrustSet && !location.OpenTelemetry.TrustSet) ||
+		(location.OpenTelemetry.TrustSet && !location.OpenTelemetry.TrustEnabled) {
+		opc = opc + "\nopentelemetry_trust_incoming_span off;"
 	}
 
-	return ""
+	return opc
 }
 
 // shouldLoadOpenTelemetryModule determines whether or not the OpenTelemetry module needs to be loaded.

diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go
@@ -1828,32 +1828,44 @@ func TestShouldLoadModSecurityModule(t *testing.T) {
 
 func TestOpentelemetryForLocation(t *testing.T) {
 	trueVal := true
+	falseVal := false
 
 	loadOT := `opentelemetry on;
 opentelemetry_propagate;`
+	loadOTUntrustedSpan := `opentelemetry on;
+opentelemetry_propagate;
+opentelemetry_trust_incoming_span off;`
 	testCases := []struct {
-		description string
-		globalOT    bool
-		isSetInLoc  bool
-		isOTInLoc   *bool
-		expected    string
+		description     string
+		globalOT        bool
+		isSetInLoc      bool
+		isOTInLoc       *bool
+		globalTrust     bool
+		isTrustSetInLoc bool
+		isTrustInLoc    *bool
+		expected        string
 	}{
-		{"globally enabled, without annotation", true, false, nil, loadOT},
-		{"globally enabled and enabled in location", true, true, &trueVal, loadOT},
-		{"globally disabled and not enabled in location", false, false, nil, ""},
-		{"globally disabled but enabled in location", false, true, &trueVal, loadOT},
-		{"globally disabled, enabled in location but false", false, true, &trueVal, loadOT},
+		{"globally enabled, without annotation", true, false, nil, true, false, nil, loadOT},
+		{"globally enabled and enabled in location", true, true, &trueVal, true, false, nil, loadOT},
+		{"globally disabled and not enabled in location", false, false, nil, true, false, nil, ""},
+		{"globally disabled but enabled in location", false, true, &trueVal, true, false, nil, loadOT},
+		{"globally trusted, not trusted in location", true, false, nil, true, true, &falseVal, loadOTUntrustedSpan},
+		{"not globally trusted, trust set in location", true, false, nil, false, true, &trueVal, loadOT},
+		{"not globally trusted, trust not set in location", true, false, nil, false, false, nil, loadOTUntrustedSpan},
 	}
 
 	for _, testCase := range testCases {
 		il := &ingress.Location{
-			OpenTelemetry: opentelemetry.Config{Set: testCase.isSetInLoc},
+			OpenTelemetry: opentelemetry.Config{Set: testCase.isSetInLoc, TrustSet: testCase.isTrustSetInLoc},
 		}
 		if il.OpenTelemetry.Set {
 			il.OpenTelemetry.Enabled = *testCase.isOTInLoc
 		}
+		if il.OpenTelemetry.TrustSet {
+			il.OpenTelemetry.TrustEnabled = *testCase.isTrustInLoc
+		}
 
-		actual := buildOpenTelemetryForLocation(testCase.globalOT, il)
+		actual := buildOpenTelemetryForLocation(testCase.globalOT, testCase.globalTrust, il)
 
 		if testCase.expected != actual {
 			t.Errorf("%v: expected '%v' but returned '%v'", testCase.description, testCase.expected, actual)

diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl
@@ -1207,7 +1207,7 @@ stream {
 
             {{ buildOpentracingForLocation $all.Cfg.EnableOpentracing $all.Cfg.OpentracingTrustIncomingSpan $location }}
 
-            {{ buildOpenTelemetryForLocation $all.Cfg.EnableOpenTelemetry $location }}
+            {{ buildOpenTelemetryForLocation $all.Cfg.EnableOpenTelemetry $all.Cfg.OpenTelemetryTrustIncomingSpan $location }}
 
             {{ if $location.Mirror.Source }}
             mirror {{ $location.Mirror.Source }};