diff --git a/.chloggen/insecure_endpoints.yaml b/.chloggen/insecure_endpoints.yaml new file mode 100644 index 0000000000..127e63dbd7 --- /dev/null +++ b/.chloggen/insecure_endpoints.yaml @@ -0,0 +1,16 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action) +component: opampbridge + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "Add TLS configuration support to the OpAMP Bridge, including options to disable TLS or skip certificate verification." + +# One or more tracking issues related to the change +issues: [4921] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: diff --git a/apis/v1alpha1/opampbridge_types.go b/apis/v1alpha1/opampbridge_types.go index 54d689242c..4655193d60 100644 --- a/apis/v1alpha1/opampbridge_types.go +++ b/apis/v1alpha1/opampbridge_types.go @@ -13,6 +13,9 @@ type OpAMPBridgeSpec struct { // OpAMP backend Server endpoint // +required Endpoint string `json:"endpoint"` + // TLS configuration for the connection to the OpAMP backend server. + // +optional + TLS *OpAMPBridgeTLSConfig `json:"tls,omitempty"` // Headers is an optional map of headers to use when connecting to the OpAMP Server, // typically used to set access tokens or other authorization headers. // +optional @@ -120,6 +123,16 @@ type AgentDescription struct { NonIdentifyingAttributes map[string]string `json:"non_identifying_attributes"` } +type OpAMPBridgeTLSConfig struct { + // Insecure indicates whether the endpoint should use TLS or not. + // When true, TLS is completely disabled. + // +optional + Insecure bool `json:"insecure,omitempty" yaml:"insecure,omitempty"` + // InsecureSkipVerify indicates to keep TLS but skip certificate validation. + // +optional + InsecureSkipVerify bool `json:"insecure_skip_verify,omitempty" yaml:"insecure_skip_verify,omitempty"` +} + // +kubebuilder:object:root=true // +kubebuilder:subresource:status // +kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp" diff --git a/apis/v1alpha1/zz_generated.deepcopy.go b/apis/v1alpha1/zz_generated.deepcopy.go index d4a66f98fa..8e62f8c3a6 100644 --- a/apis/v1alpha1/zz_generated.deepcopy.go +++ b/apis/v1alpha1/zz_generated.deepcopy.go @@ -811,6 +811,11 @@ func (in *OpAMPBridgeList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OpAMPBridgeSpec) DeepCopyInto(out *OpAMPBridgeSpec) { *out = *in + if in.TLS != nil { + in, out := &in.TLS, &out.TLS + *out = new(OpAMPBridgeTLSConfig) + **out = **in + } if in.Headers != nil { in, out := &in.Headers, &out.Headers *out = make(map[string]string, len(*in)) @@ -968,6 +973,21 @@ func (in *OpAMPBridgeStatus) DeepCopy() *OpAMPBridgeStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OpAMPBridgeTLSConfig) DeepCopyInto(out *OpAMPBridgeTLSConfig) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OpAMPBridgeTLSConfig. +func (in *OpAMPBridgeTLSConfig) DeepCopy() *OpAMPBridgeTLSConfig { + if in == nil { + return nil + } + out := new(OpAMPBridgeTLSConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OpenShiftRoute) DeepCopyInto(out *OpenShiftRoute) { *out = *in diff --git a/bundle/community/manifests/opentelemetry.io_opampbridges.yaml b/bundle/community/manifests/opentelemetry.io_opampbridges.yaml index 592307181e..906b67f66a 100644 --- a/bundle/community/manifests/opentelemetry.io_opampbridges.yaml +++ b/bundle/community/manifests/opentelemetry.io_opampbridges.yaml @@ -872,6 +872,13 @@ spec: type: object serviceAccount: type: string + tls: + properties: + insecure: + type: boolean + insecure_skip_verify: + type: boolean + type: object tolerations: items: properties: diff --git a/bundle/openshift/manifests/opentelemetry.io_opampbridges.yaml b/bundle/openshift/manifests/opentelemetry.io_opampbridges.yaml index 592307181e..906b67f66a 100644 --- a/bundle/openshift/manifests/opentelemetry.io_opampbridges.yaml +++ b/bundle/openshift/manifests/opentelemetry.io_opampbridges.yaml @@ -872,6 +872,13 @@ spec: type: object serviceAccount: type: string + tls: + properties: + insecure: + type: boolean + insecure_skip_verify: + type: boolean + type: object tolerations: items: properties: diff --git a/cmd/operator-opamp-bridge/internal/agent/agent.go b/cmd/operator-opamp-bridge/internal/agent/agent.go index 7eb93ca5c5..6008a4131c 100644 --- a/cmd/operator-opamp-bridge/internal/agent/agent.go +++ b/cmd/operator-opamp-bridge/internal/agent/agent.go @@ -6,8 +6,10 @@ package agent import ( "bytes" "context" + "crypto/tls" "errors" "fmt" + "net/url" "strings" "time" @@ -256,8 +258,34 @@ func (agent *Agent) Start() error { }, RemoteConfigStatus: agent.remoteConfigStatus, PackagesStateProvider: nil, - Capabilities: agent.config.GetCapabilities(), } + + // Configure TLS based on explicit tls settings. + if agent.config.TLS != nil { + if agent.config.TLS.Insecure { + parsedURL, parseErr := url.Parse(settings.OpAMPServerURL) + if parseErr != nil { + return fmt.Errorf("invalid OpAMP server endpoint %q: %w", settings.OpAMPServerURL, parseErr) + } + + if parsedURL.Scheme == "wss" || parsedURL.Scheme == "https" { + return fmt.Errorf( + "tls.insecure=true requires an insecure endpoint scheme (ws:// or http://), but got %q. "+ + "Please update the endpoint to use the insecure scheme when tls.insecure is enabled", + settings.OpAMPServerURL, + ) + } + + agent.logger.Info("TLS is disabled for the OpAMP client connection (tls.insecure=true). This connection is not encrypted.") + } else if agent.config.TLS.InsecureSkipVerify { + // TLS enabled but skip certificate verification (standard behavior) + settings.TLSConfig = &tls.Config{ + InsecureSkipVerify: true, //nolint:gosec // User explicitly opted in via tls.insecure_skip_verify. + } + agent.logger.Info("TLS is enabled for the OpAMP client connection but certificate verification is skipped (tls.insecure_skip_verify=true)") + } + } + err = agent.opampClient.SetAgentDescription(agent.agentDescription) if err != nil { return err @@ -266,6 +294,11 @@ func (agent *Agent) Start() error { if err != nil { return err } + capabilities := agent.config.GetCapabilities() + err = agent.opampClient.SetCapabilities(&capabilities) + if err != nil { + return err + } agent.logger.V(3).Info("Starting OpAMP client...") diff --git a/cmd/operator-opamp-bridge/internal/agent/agent_test.go b/cmd/operator-opamp-bridge/internal/agent/agent_test.go index 032767ddee..d98dc88e9d 100644 --- a/cmd/operator-opamp-bridge/internal/agent/agent_test.go +++ b/cmd/operator-opamp-bridge/internal/agent/agent_test.go @@ -1258,3 +1258,67 @@ func getMessageDataFromConfigFile(filemap map[string]string) (*types.MessageData } return toReturn, nil } + +func TestAgent_Start_TLSConfig(t *testing.T) { + tests := []struct { + name string + endpoint string + insecure bool + insecureSkipVerify bool + expectNil bool + expectSkip bool + expectURL string + }{ + { + name: "Insecure (no TLS)", + endpoint: "ws://127.0.0.1:4320/v1/opamp", + insecure: true, + expectNil: true, + expectURL: "ws://127.0.0.1:4320/v1/opamp", + }, + { + name: "Secure with Skip Verify", + endpoint: "wss://127.0.0.1:4320/v1/opamp", + insecure: false, + insecureSkipVerify: true, + expectNil: false, + expectSkip: true, + expectURL: "wss://127.0.0.1:4320/v1/opamp", + }, + { + name: "Secure (verify enabled)", + endpoint: "wss://127.0.0.1:4320/v1/opamp", + insecure: false, + insecureSkipVerify: false, + expectNil: true, + expectURL: "wss://127.0.0.1:4320/v1/opamp", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockClient := &mockOpampClient{} + conf := config.NewConfig(logr.Discard()) + conf.Endpoint = tt.endpoint + conf.TLS = &v1alpha1.OpAMPBridgeTLSConfig{ + Insecure: tt.insecure, + InsecureSkipVerify: tt.insecureSkipVerify, + } + applier := getFakeApplier(t, conf) + mp := newMockProxy(nil, nil, nil) + agent := NewAgent(l, applier, conf, mockClient, mp) + + err := agent.Start() + require.NoError(t, err) + + if tt.expectNil { + assert.Nil(t, mockClient.settings.TLSConfig) + } else { + require.NotNil(t, mockClient.settings.TLSConfig) + assert.Equal(t, tt.expectSkip, mockClient.settings.TLSConfig.InsecureSkipVerify) + } + assert.Equal(t, tt.expectURL, mockClient.settings.OpAMPServerURL) + agent.Shutdown() + }) + } +} diff --git a/cmd/operator-opamp-bridge/internal/config/config.go b/cmd/operator-opamp-bridge/internal/config/config.go index 7ddd65fd1b..f5f5b1a8b9 100644 --- a/cmd/operator-opamp-bridge/internal/config/config.go +++ b/cmd/operator-opamp-bridge/internal/config/config.go @@ -84,13 +84,14 @@ type Config struct { instanceId uuid.UUID `yaml:"-"` // ComponentsAllowed is a list of allowed OpenTelemetry components for each pipeline type (receiver, processor, etc.) - ComponentsAllowed map[string][]string `yaml:"componentsAllowed,omitempty"` - Endpoint string `yaml:"endpoint"` - Headers Headers `yaml:"headers,omitempty"` - Capabilities map[Capability]bool `yaml:"capabilities"` - HeartbeatInterval time.Duration `yaml:"heartbeatInterval,omitempty"` - Name string `yaml:"name,omitempty"` - AgentDescription AgentDescription `yaml:"description,omitempty"` + ComponentsAllowed map[string][]string `yaml:"componentsAllowed,omitempty"` + Endpoint string `yaml:"endpoint"` + TLS *v1alpha1.OpAMPBridgeTLSConfig `yaml:"tls,omitempty"` + Headers Headers `yaml:"headers,omitempty"` + Capabilities map[Capability]bool `yaml:"capabilities"` + HeartbeatInterval time.Duration `yaml:"heartbeatInterval,omitempty"` + Name string `yaml:"name,omitempty"` + AgentDescription AgentDescription `yaml:"description,omitempty"` } // AgentDescription is copied from the OpAMP Extension in the collector. diff --git a/config/crd/bases/opentelemetry.io_opampbridges.yaml b/config/crd/bases/opentelemetry.io_opampbridges.yaml index 4374586f4c..c1c2c8b2dc 100644 --- a/config/crd/bases/opentelemetry.io_opampbridges.yaml +++ b/config/crd/bases/opentelemetry.io_opampbridges.yaml @@ -870,6 +870,13 @@ spec: type: object serviceAccount: type: string + tls: + properties: + insecure: + type: boolean + insecure_skip_verify: + type: boolean + type: object tolerations: items: properties: diff --git a/docs/api/opampbridges.md b/docs/api/opampbridges.md index 1830233b75..8f44b7e69e 100644 --- a/docs/api/opampbridges.md +++ b/docs/api/opampbridges.md @@ -253,6 +253,13 @@ default.
the operator will not automatically create a ServiceAccount for the OpAMPBridge.
false + + tls + object + + TLS configuration for the connection to the OpAMP backend server.
+ + false tolerations []object @@ -3483,6 +3490,41 @@ PodSecurityContext, the value specified in SecurityContext takes precedence.
+### OpAMPBridge.spec.tls +[↩ Parent](#opampbridgespec) + + + +TLS configuration for the connection to the OpAMP backend server. + + + + + + + + + + + + + + + + + + + + + +
NameTypeDescriptionRequired
insecureboolean + Insecure indicates whether the endpoint should use TLS or not. +When true, TLS is completely disabled.
+ 		false
insecure_skip_verifyboolean + InsecureSkipVerify indicates to keep TLS but skip certificate validation.
+ 		false
+ + ### OpAMPBridge.spec.tolerations[index] [↩ Parent](#opampbridgespec) diff --git a/internal/manifests/opampbridge/configmap.go b/internal/manifests/opampbridge/configmap.go index 7a1ced9f8d..feb4eb7fd0 100644 --- a/internal/manifests/opampbridge/configmap.go +++ b/internal/manifests/opampbridge/configmap.go @@ -27,6 +27,10 @@ func ConfigMap(params manifests.Params) (*corev1.ConfigMap, error) { config["endpoint"] = params.OpAMPBridge.Spec.Endpoint } + if params.OpAMPBridge.Spec.TLS != nil { + config["tls"] = params.OpAMPBridge.Spec.TLS + } + if len(params.OpAMPBridge.Spec.Headers) > 0 { config["headers"] = params.OpAMPBridge.Spec.Headers } diff --git a/internal/webhook/opampbridge_webhook.go b/internal/webhook/opampbridge_webhook.go index 90abd69db5..37deba27ac 100644 --- a/internal/webhook/opampbridge_webhook.go +++ b/internal/webhook/opampbridge_webhook.go @@ -83,6 +83,10 @@ func (*OpAMPBridgeWebhook) validate(r *v1alpha1.OpAMPBridge) (admission.Warnings return warnings, errors.New("the OpAMP server endpoint is not specified") } + if r.Spec.TLS != nil && r.Spec.TLS.Insecure && r.Spec.TLS.InsecureSkipVerify { + return warnings, errors.New("tls.insecure and tls.insecure_skip_verify cannot both be true") + } + // validate OpAMPBridge capabilities if len(r.Spec.Capabilities) == 0 { return warnings, errors.New("the capabilities supported by OpAMP Bridge are not specified") diff --git a/internal/webhook/opampbridge_webhook_test.go b/internal/webhook/opampbridge_webhook_test.go index 5caadb95fc..e8a834cdcb 100644 --- a/internal/webhook/opampbridge_webhook_test.go +++ b/internal/webhook/opampbridge_webhook_test.go @@ -164,6 +164,22 @@ func TestOpAMPBridgeValidatingWebhook(t *testing.T) { }, expectedErr: "the capabilities supported by OpAMP Bridge are not specified", }, + { + name: "insecure and insecure skip verify both set", + opampBridge: v1alpha1.OpAMPBridge{ + Spec: v1alpha1.OpAMPBridgeSpec{ + Endpoint: "ws://opamp-server:4320/v1/opamp", + TLS: &v1alpha1.OpAMPBridgeTLSConfig{ + Insecure: true, + InsecureSkipVerify: true, + }, + Capabilities: map[v1alpha1.OpAMPBridgeCapability]bool{ + v1alpha1.OpAMPBridgeCapabilityReportsStatus: true, + }, + }, + }, + expectedErr: "tls.insecure and tls.insecure_skip_verify cannot both be true", + }, { name: "replica count greater than 1 should return error", opampBridge: v1alpha1.OpAMPBridge{ diff --git a/tests/e2e-opampbridge/opampbridge/01-assert.yaml b/tests/e2e-opampbridge/opampbridge/01-assert.yaml index 09fbea8dab..100db1798e 100644 --- a/tests/e2e-opampbridge/opampbridge/01-assert.yaml +++ b/tests/e2e-opampbridge/opampbridge/01-assert.yaml @@ -51,6 +51,8 @@ data: receivers: - otlp endpoint: ws://e2e-test-app-bridge-server:4320/v1/opamp + tls: + insecure: true kind: ConfigMap metadata: name: test-opamp-bridge diff --git a/tests/e2e-opampbridge/opampbridge/01-install.yaml b/tests/e2e-opampbridge/opampbridge/01-install.yaml index c51c16e98c..d422be43a8 100644 --- a/tests/e2e-opampbridge/opampbridge/01-install.yaml +++ b/tests/e2e-opampbridge/opampbridge/01-install.yaml @@ -61,4 +61,6 @@ spec: - memory_limiter receivers: - otlp - endpoint: ws://e2e-test-app-bridge-server:4320/v1/opamp \ No newline at end of file + endpoint: ws://e2e-test-app-bridge-server:4320/v1/opamp + tls: + insecure: true diff --git a/tests/e2e-opampbridge/opampbridge/chainsaw-test.yaml b/tests/e2e-opampbridge/opampbridge/chainsaw-test.yaml index 3738aca6d8..f60ca48bf7 100755 --- a/tests/e2e-opampbridge/opampbridge/chainsaw-test.yaml +++ b/tests/e2e-opampbridge/opampbridge/chainsaw-test.yaml @@ -137,3 +137,16 @@ spec: receivers: - jaeger - otlp + - name: Ensure no errors in bridge logs + try: + - script: + content: | + #!/bin/bash + logs=$(kubectl logs -l app.kubernetes.io/name=test-opamp-bridge -n $NAMESPACE) + echo "$logs" + # Check for any "level":"error" or "bad handshake" + if echo "$logs" | grep -qiE "\"level\":\"error\"|bad handshake"; then + echo "Found an error in the logs!" + exit 1 + fi + echo "No errors found."