diff --git a/internal/gnmic/coverage_gaps_test.go b/internal/gnmic/coverage_gaps_test.go index 7726feb..92bde40 100644 --- a/internal/gnmic/coverage_gaps_test.go +++ b/internal/gnmic/coverage_gaps_test.go @@ -129,24 +129,27 @@ func TestBuildSubscriptionConfig_Full(t *testing.T) { Start: now, End: now, }, - StreamSubscriptions: []string{"default/child"}, + // bare names, as they appear in the CR + StreamSubscriptions: []string{"child"}, } child := gnmicv1alpha1.SubscriptionSpec{Paths: []string{"/child"}, Mode: "ONCE"} - cfg := buildSubscriptionConfig("default/parent", spec, []string{"out"}, map[string]gnmicv1alpha1.SubscriptionSpec{ - "default/child": child, + // allSubs is keyed the way the plan builder keys it: namespace/pipeline/name. + cfg := buildSubscriptionConfig("default/p1/parent", spec, []string{"out"}, map[string]gnmicv1alpha1.SubscriptionSpec{ + "default/p1/child": child, }) if cfg.Encoding == nil || cfg.Qos == nil || cfg.History == nil { t.Fatal("expected optional fields") } if len(cfg.StreamSubscriptions) != 1 || cfg.StreamSubscriptions[0] == nil { - t.Fatal("expected child subscription") + t.Fatalf("expected child subscription, got %+v", cfg.StreamSubscriptions) } - // missing child in map is skipped - spec.StreamSubscriptions = []string{"default/missing"} - cfg = buildSubscriptionConfig("default/p", spec, nil, map[string]gnmicv1alpha1.SubscriptionSpec{}) - if cfg.StreamSubscriptions[0] != nil { - t.Fatal("expected nil slot for missing child") + // a name that is not in the pipeline is left out entirely rather than leaving a + // nil in the slice, which marshalled as a literal null on the wire + spec.StreamSubscriptions = []string{"missing"} + cfg = buildSubscriptionConfig("default/p1/parent", spec, nil, map[string]gnmicv1alpha1.SubscriptionSpec{}) + if len(cfg.StreamSubscriptions) != 0 { + t.Fatalf("expected no entries, got %+v", cfg.StreamSubscriptions) } } diff --git a/internal/gnmic/profile_mapping_test.go b/internal/gnmic/profile_mapping_test.go new file mode 100644 index 0000000..e3f16fe --- /dev/null +++ b/internal/gnmic/profile_mapping_test.go @@ -0,0 +1,281 @@ +package gnmic + +import ( + "encoding/json" + "strings" + "testing" + "time" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + gnmicv1alpha1 "github.com/gnmic/operator/api/v1alpha1" +) + +// fullProfile sets every TargetProfile field that describes how to reach a target, +// so a test can assert none of them is dropped. +func fullProfile() *gnmicv1alpha1.TargetProfileSpec { + return &gnmicv1alpha1.TargetProfileSpec{ + Encoding: "JSON_IETF", + Timeout: metav1.Duration{Duration: 7 * time.Second}, + RetryTimer: metav1.Duration{Duration: 3 * time.Second}, + Proxy: "socks5://jump.example.com:1080", + GzipCompression: true, + Labels: map[string]string{"site": "ams1", "tier": "leaf"}, + TCPKeepAlive: &metav1.Duration{Duration: 30 * time.Second}, + GRCPKeepAlive: &gnmicv1alpha1.GRPCKeepAliveConfig{ + Time: metav1.Duration{Duration: 10 * time.Second}, + Timeout: metav1.Duration{Duration: 3 * time.Second}, + PermitWithoutStream: true, + }, + } +} + +func testTarget() *gnmicv1alpha1.Target { + return &gnmicv1alpha1.Target{ + ObjectMeta: metav1.ObjectMeta{Namespace: "ns", Name: "t1"}, + Spec: gnmicv1alpha1.TargetSpec{Address: "10.0.0.1:57400", Profile: "prof"}, + } +} + +// The profile fields below used to be applied only after the TLS decision, every +// branch of which returned early — so keepalives reached the collector only when +// cluster clientTLS and profile TLS were both set, which excludes the plaintext +// default. proxy, gzipCompression and labels were never mapped at all. +func TestProfileConnectionFieldsSurviveEveryTLSCombination(t *testing.T) { + tests := []struct { + name string + profileTLS *gnmicv1alpha1.TargetTLSConfig + clientTLS *ClientTLSPaths + }{ + {name: "plaintext"}, + { + name: "profile TLS only", + profileTLS: &gnmicv1alpha1.TargetTLSConfig{MinVersion: "1.2"}, + }, + { + name: "cluster clientTLS only", + clientTLS: &ClientTLSPaths{CertFile: "/c", KeyFile: "/k", CAFile: "/ca"}, + }, + { + name: "both", + profileTLS: &gnmicv1alpha1.TargetTLSConfig{MinVersion: "1.2"}, + clientTLS: &ClientTLSPaths{CertFile: "/c", KeyFile: "/k", CAFile: "/ca"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + profile := fullProfile() + profile.TLS = tt.profileTLS + + cfg := buildTargetConfig(testTarget(), profile, nil, tt.clientTLS) + + if cfg.Proxy != profile.Proxy { + t.Errorf("proxy = %q, want %q", cfg.Proxy, profile.Proxy) + } + if cfg.Gzip == nil || !*cfg.Gzip { + t.Errorf("gzip = %v, want true", cfg.Gzip) + } + if got := cfg.EventTags["site"]; got != "ams1" { + t.Errorf("event-tags = %v, want the profile labels", cfg.EventTags) + } + if cfg.TCPKeepalive != 30*time.Second { + t.Errorf("tcp keepalive = %v, want 30s", cfg.TCPKeepalive) + } + if cfg.GRPCKeepalive == nil { + t.Fatal("grpc keepalive dropped") + } + if cfg.GRPCKeepalive.Time != 10*time.Second || + cfg.GRPCKeepalive.Timeout != 3*time.Second || + !cfg.GRPCKeepalive.PermitWithoutStream { + t.Errorf("grpc keepalive = %+v", cfg.GRPCKeepalive) + } + // The non-TLS basics still come through. + if cfg.Timeout != 7*time.Second || cfg.RetryTimer != 3*time.Second { + t.Errorf("timeout=%v retry=%v", cfg.Timeout, cfg.RetryTimer) + } + }) + } +} + +// The labels map must not be shared with the CR the informer cache holds. +func TestProfileLabelsAreCloned(t *testing.T) { + profile := fullProfile() + cfg := buildTargetConfig(testTarget(), profile, nil, nil) + + cfg.EventTags["site"] = "mutated" + if profile.Labels["site"] != "ams1" { + t.Fatal("mutating the target config changed the profile's labels map") + } +} + +// serverName selects the SNI the collector sends, which is meaningful with or without +// verification. It used to be applied only when cluster clientTLS was configured. +func TestProfileServerNameAppliesWithoutClusterClientTLS(t *testing.T) { + profile := fullProfile() + profile.TLS = &gnmicv1alpha1.TargetTLSConfig{ + ServerName: "router.example.com", + MinVersion: "1.2", + MaxVersion: "1.3", + CipherSuites: []string{"TLS_AES_128_GCM_SHA256"}, + } + + cfg := buildTargetConfig(testTarget(), profile, nil, nil) + if cfg.TLSServerName != "router.example.com" { + t.Errorf("tls-server-name = %q, want router.example.com", cfg.TLSServerName) + } + if cfg.TLSMinVersion != "1.2" || cfg.TLSMaxVersion != "1.3" || len(cfg.CipherSuites) != 1 { + t.Errorf("tls options dropped: %+v", cfg) + } + if cfg.SkipVerify == nil || !*cfg.SkipVerify { + t.Error("expected skip-verify with no CA to verify against") + } +} + +// The TLS decision itself must be unchanged by the restructuring. +func TestTargetTLSDecisionUnchanged(t *testing.T) { + target := testTarget() + + plain := buildTargetConfig(target, &gnmicv1alpha1.TargetProfileSpec{}, nil, nil) + if plain.Insecure == nil || !*plain.Insecure { + t.Error("plaintext: want insecure") + } + if plain.SkipVerify != nil { + t.Error("plaintext: skip-verify should be unset") + } + + withCA := buildTargetConfig(target, &gnmicv1alpha1.TargetProfileSpec{}, nil, + &ClientTLSPaths{CertFile: "/c", KeyFile: "/k", CAFile: "/ca"}) + if withCA.TLSCA == nil || withCA.SkipVerify == nil || *withCA.SkipVerify { + t.Errorf("with CA: want verification on, got %+v", withCA) + } + if withCA.Insecure != nil { + t.Error("with CA: insecure should be unset") + } + + noCA := buildTargetConfig(target, &gnmicv1alpha1.TargetProfileSpec{}, nil, + &ClientTLSPaths{CertFile: "/c", KeyFile: "/k"}) + if noCA.SkipVerify == nil || !*noCA.SkipVerify { + t.Errorf("without CA: want skip-verify, got %+v", noCA) + } +} + +// A tunnel target built from the same profile must get the same connection settings +// as a declared one. The inlined copy this replaces never mapped the keepalives. +func TestTunnelTargetMatchGetsTheSameProfileFields(t *testing.T) { + profile := fullProfile() + + // TunnelTargetPolicySpec.Match points at an unexported type, so it cannot be + // constructed from outside api/v1alpha1 -- decode the spec the way the API server + // does instead. + var policy gnmicv1alpha1.TunnelTargetPolicySpec + if err := json.Unmarshal([]byte( + `{"profile":"prof","match":{"type":"nokia_srlinux","id":"leaf.*"}}`), &policy); err != nil { + t.Fatal(err) + } + if policy.Match == nil { + t.Fatal("match did not decode") + } + + match := buildTunnelTargetMatch(&policy, profile, &Credentials{Username: "u", Password: "p"}, nil) + if match.Type != "nokia_srlinux" || match.ID != "leaf.*" { + t.Errorf("match criteria = %+v", match) + } + cfg := match.Config + if cfg == nil { + t.Fatal("no target config built") + } + if cfg.TCPKeepalive != 30*time.Second { + t.Errorf("tcp keepalive = %v, want 30s", cfg.TCPKeepalive) + } + if cfg.GRPCKeepalive == nil || cfg.GRPCKeepalive.Time != 10*time.Second { + t.Errorf("grpc keepalive = %+v", cfg.GRPCKeepalive) + } + if cfg.Proxy != profile.Proxy { + t.Errorf("proxy = %q", cfg.Proxy) + } + if cfg.Gzip == nil || !*cfg.Gzip { + t.Error("gzip dropped") + } + if cfg.EventTags["tier"] != "leaf" { + t.Errorf("event-tags = %v", cfg.EventTags) + } + if cfg.Username == nil || *cfg.Username != "u" { + t.Error("credentials dropped") + } + if cfg.Insecure == nil || !*cfg.Insecure { + t.Error("expected plaintext with no TLS configured") + } +} + +// ---------------------------------------------------------------- #12 + +// streamSubscriptions names siblings in the same pipeline. The lookup used the bare +// name against a map keyed namespace/pipeline/name, so it never matched, and the +// pre-sized slice left a nil per entry that marshalled as a literal null. +func TestStreamSubscriptionsResolveByPipelineScopedKey(t *testing.T) { + parent := &gnmicv1alpha1.SubscriptionSpec{ + Mode: "STREAM/SAMPLE", + Paths: []string{"/parent"}, + StreamSubscriptions: []string{"childA", "childB"}, + } + allSubs := map[string]gnmicv1alpha1.SubscriptionSpec{ + "ns/pipe/parent": *parent, + "ns/pipe/childA": {Mode: "STREAM/ON_CHANGE", Paths: []string{"/a"}}, + "ns/pipe/childB": {Mode: "STREAM/SAMPLE", Paths: []string{"/b"}}, + } + + cfg := buildSubscriptionConfig("ns/pipe/parent", parent, []string{"ns/pipe/out"}, allSubs) + + if len(cfg.StreamSubscriptions) != 2 { + t.Fatalf("resolved %d stream subscriptions, want 2: %+v", len(cfg.StreamSubscriptions), cfg.StreamSubscriptions) + } + for i, sub := range cfg.StreamSubscriptions { + if sub == nil { + t.Fatalf("stream subscription %d is nil; it would serialize as null", i) + } + } + if cfg.StreamSubscriptions[0].Name != "ns/pipe/childA" { + t.Errorf("child name = %q, want the pipeline-scoped key", cfg.StreamSubscriptions[0].Name) + } + if cfg.StreamSubscriptions[0].StreamMode != "ON_CHANGE" { + t.Errorf("child stream mode = %q", cfg.StreamSubscriptions[0].StreamMode) + } +} + +// A name that is not in the pipeline is left out rather than leaving a hole. +func TestUnknownStreamSubscriptionLeavesNoNull(t *testing.T) { + parent := &gnmicv1alpha1.SubscriptionSpec{ + Mode: "STREAM/SAMPLE", + StreamSubscriptions: []string{"present", "absent"}, + } + allSubs := map[string]gnmicv1alpha1.SubscriptionSpec{ + "ns/pipe/present": {Mode: "ONCE", Paths: []string{"/p"}}, + } + + cfg := buildSubscriptionConfig("ns/pipe/parent", parent, nil, allSubs) + if len(cfg.StreamSubscriptions) != 1 || cfg.StreamSubscriptions[0] == nil { + t.Fatalf("stream subscriptions = %+v, want exactly one non-nil", cfg.StreamSubscriptions) + } + + // The payload the collectors receive must not contain a null entry. + body, err := json.Marshal(cfg) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(body), "null") { + t.Errorf("serialized config contains null: %s", body) + } +} + +func TestSiblingSubscriptionKey(t *testing.T) { + for _, tc := range []struct{ subNN, name, want string }{ + {"ns/pipe/parent", "child", "ns/pipe/child"}, + {"ns/parent", "child", "ns/child"}, + {"parent", "child", "child"}, + } { + if got := siblingSubscriptionKey(tc.subNN, tc.name); got != tc.want { + t.Errorf("siblingSubscriptionKey(%q, %q) = %q, want %q", tc.subNN, tc.name, got, tc.want) + } + } +} diff --git a/internal/gnmic/subscription.go b/internal/gnmic/subscription.go index 26bc8b7..4a4ebc3 100644 --- a/internal/gnmic/subscription.go +++ b/internal/gnmic/subscription.go @@ -25,10 +25,6 @@ func buildSubscriptionConfig(subNN string, subscription *gnmicv1alpha1.Subscript Outputs: outputs, } - if len(outputs) > 0 { - config.Outputs = outputs - } - if subscription.Encoding != "" { config.Encoding = &subscription.Encoding } @@ -49,19 +45,37 @@ func buildSubscriptionConfig(subNN string, subscription *gnmicv1alpha1.Subscript } } // handle streamSubscriptions - if len(subscription.StreamSubscriptions) > 0 { - config.StreamSubscriptions = make([]*gapi.SubscriptionConfig, len(subscription.StreamSubscriptions)) - for i, streamSubscription := range subscription.StreamSubscriptions { - streamSubSpec, ok := allSubs[streamSubscription] - if !ok { - continue - } - config.StreamSubscriptions[i] = buildSubscriptionConfig(streamSubscription, &streamSubSpec, nil, nil) + // + // spec.streamSubscriptions holds bare Subscription names, while allSubs is keyed + // "//" so two pipelines sharing one Subscription CR + // keep separate output bindings. Looking up the bare name therefore never matched, + // and because the slice was pre-sized to the number of names, every entry stayed + // nil and marshalled as a literal null in the payload sent to the collectors. + for _, name := range subscription.StreamSubscriptions { + key := siblingSubscriptionKey(subNN, name) + streamSubSpec, ok := allSubs[key] + if !ok { + // A referenced stream subscription has to be selected into the same + // pipeline, or there is nothing to point at. + logger.Warn("stream subscription not found in the pipeline, skipping", + "subscription", subNN, "streamSubscription", name, "lookup", key) + continue } + config.StreamSubscriptions = append(config.StreamSubscriptions, + buildSubscriptionConfig(key, &streamSubSpec, nil, nil)) } return config } +// siblingSubscriptionKey resolves a bare Subscription name against the +// pipeline-scoped key of the Subscription that referenced it. +func siblingSubscriptionKey(subNN, name string) string { + if i := strings.LastIndex(subNN, Delimiter); i >= 0 { + return subNN[:i+1] + name + } + return name +} + // specModeToConfig splits a mode string like "STREAM/SAMPLE" into mode and stream mode func specModeToConfig(mode string) (string, string) { parts := strings.SplitN(mode, "/", 2) diff --git a/internal/gnmic/target.go b/internal/gnmic/target.go index d0890c7..c150047 100644 --- a/internal/gnmic/target.go +++ b/internal/gnmic/target.go @@ -1,6 +1,7 @@ package gnmic import ( + "maps" "time" gnmicv1alpha1 "github.com/gnmic/operator/api/v1alpha1" @@ -9,8 +10,9 @@ import ( "k8s.io/utils/ptr" ) -// buildTargetConfig creates a gNMIc TargetConfig from a Target and TargetProfile -// clientTLS contains paths to client certificates for mTLS with targets (from cluster.Spec.ClientTLS) +// buildTargetConfig creates a gNMIc TargetConfig from a Target and TargetProfile. +// clientTLS contains paths to client certificates for mTLS with targets (from +// cluster.Spec.ClientTLS). func buildTargetConfig(target *gnmicv1alpha1.Target, profile *gnmicv1alpha1.TargetProfileSpec, creds *Credentials, clientTLS *ClientTLSPaths) *gapi.TargetConfig { config := &gapi.TargetConfig{ Name: target.Namespace + Delimiter + target.Name, @@ -20,40 +22,80 @@ func buildTargetConfig(target *gnmicv1alpha1.Target, profile *gnmicv1alpha1.Targ Encoding: ptr.To(profile.Encoding), } - // set credentials if provided - if creds != nil { - if creds.Username != "" { - config.Username = ptr.To(creds.Username) - } - if creds.Password != "" { - config.Password = ptr.To(creds.Password) - } - if creds.Token != "" { - config.Token = ptr.To(creds.Token) + applyProfileConnection(config, profile) + applyCredentials(config, creds) + applyTargetTLS(config, profile.TLS, clientTLS) + + return config +} + +// applyProfileConnection maps the TargetProfile fields that describe how to reach a +// target, independently of TLS. +// +// These used to sit after the TLS decision, every branch of which returned early, so +// the keepalives only reached a collector when cluster clientTLS and profile TLS +// happened to both be set — never in the plaintext default, which is the common case. +// proxy, gzipCompression and labels were never mapped at all, despite having a home in +// TargetConfig. +func applyProfileConnection(config *gapi.TargetConfig, profile *gnmicv1alpha1.TargetProfileSpec) { + if profile.Proxy != "" { + config.Proxy = profile.Proxy + } + if profile.GzipCompression { + config.Gzip = ptr.To(true) + } + if len(profile.Labels) > 0 { + // gNMIc's event-tags are attached to every event produced from the target, + // which is what the CRD field documents: "labels to add to the target's + // updates". Cloned so the plan never aliases the informer cache's map. + config.EventTags = maps.Clone(profile.Labels) + } + if profile.TCPKeepAlive != nil { + config.TCPKeepalive = profile.TCPKeepAlive.Duration + } + if profile.GRCPKeepAlive != nil { + config.GRPCKeepalive = &gapi.ClientKeepalive{ + Time: profile.GRCPKeepAlive.Time.Duration, + Timeout: profile.GRCPKeepAlive.Timeout.Duration, + PermitWithoutStream: profile.GRCPKeepAlive.PermitWithoutStream, } } +} - // no client TLS configuration at the cluster level or target profile level - if clientTLS == nil && profile.TLS == nil { - config.Insecure = ptr.To(true) - return config +// applyCredentials copies whichever credentials the profile's secret supplied. +func applyCredentials(config *gapi.TargetConfig, creds *Credentials) { + if creds == nil { + return } - // TLS not enabled at the cluster level but enabled at the target profile level - if clientTLS == nil && profile.TLS != nil { - config.SkipVerify = ptr.To(true) - if profile.TLS.MaxVersion != "" { - config.TLSMaxVersion = profile.TLS.MaxVersion - } - if profile.TLS.MinVersion != "" { - config.TLSMinVersion = profile.TLS.MinVersion - } - if len(profile.TLS.CipherSuites) > 0 { - config.CipherSuites = profile.TLS.CipherSuites + if creds.Username != "" { + config.Username = ptr.To(creds.Username) + } + if creds.Password != "" { + config.Password = ptr.To(creds.Password) + } + if creds.Token != "" { + config.Token = ptr.To(creds.Token) + } +} + +// applyTargetTLS decides how the collector talks TLS to a target. +// +// - neither side configured: plaintext. +// - profile TLS only: TLS with no client certificate and no CA to verify against, +// so verification is skipped. +// - cluster clientTLS: present the client certificate, and verify only when a CA +// bundle came with it. +func applyTargetTLS(config *gapi.TargetConfig, profileTLS *gnmicv1alpha1.TargetTLSConfig, clientTLS *ClientTLSPaths) { + if clientTLS == nil { + if profileTLS == nil { + config.Insecure = ptr.To(true) + return } - return config + config.SkipVerify = ptr.To(true) + applyProfileTLSOptions(config, profileTLS) + return } - // use client TLS configuration from cluster (for mTLS with targets) if clientTLS.CertFile != "" { config.TLSCert = ptr.To(clientTLS.CertFile) } @@ -67,33 +109,29 @@ func buildTargetConfig(target *gnmicv1alpha1.Target, profile *gnmicv1alpha1.Targ // TLS is enabled but without CA verification (TrustBundleRef not supported yet) config.SkipVerify = ptr.To(true) } - if profile.TLS == nil { - return config - } - if profile.TLS.ServerName != "" { - config.TLSServerName = profile.TLS.ServerName - } - if profile.TLS.MaxVersion != "" { - config.TLSMaxVersion = profile.TLS.MaxVersion + if profileTLS != nil { + applyProfileTLSOptions(config, profileTLS) } - if profile.TLS.MinVersion != "" { - config.TLSMinVersion = profile.TLS.MinVersion +} + +// applyProfileTLSOptions copies the TLS knobs a TargetProfile can set. +// +// serverName is included here rather than only on the clientTLS path: it selects the +// SNI the collector sends, which is meaningful with or without verification, and +// leaving it out silently ignored a field the profile had set. +func applyProfileTLSOptions(config *gapi.TargetConfig, profileTLS *gnmicv1alpha1.TargetTLSConfig) { + if profileTLS.ServerName != "" { + config.TLSServerName = profileTLS.ServerName } - if len(profile.TLS.CipherSuites) > 0 { - config.CipherSuites = profile.TLS.CipherSuites + if profileTLS.MaxVersion != "" { + config.TLSMaxVersion = profileTLS.MaxVersion } - - if profile.TCPKeepAlive != nil { - config.TCPKeepalive = profile.TCPKeepAlive.Duration + if profileTLS.MinVersion != "" { + config.TLSMinVersion = profileTLS.MinVersion } - if profile.GRCPKeepAlive != nil { - config.GRPCKeepalive = &gapi.ClientKeepalive{ - Time: profile.GRCPKeepAlive.Time.Duration, - Timeout: profile.GRCPKeepAlive.Timeout.Duration, - PermitWithoutStream: profile.GRCPKeepAlive.PermitWithoutStream, - } + if len(profileTLS.CipherSuites) > 0 { + config.CipherSuites = profileTLS.CipherSuites } - return config } func durationOrDefault(duration *metav1.Duration, defaultDuration time.Duration) time.Duration { diff --git a/internal/gnmic/tunneltarget.go b/internal/gnmic/tunneltarget.go index e77509f..87cb1b2 100644 --- a/internal/gnmic/tunneltarget.go +++ b/internal/gnmic/tunneltarget.go @@ -8,9 +8,14 @@ import ( "k8s.io/utils/ptr" ) -// buildTunnelTargetMatch creates a TunnelTargetMatch from a TunnelTargetPolicy and TargetProfile -// clientTLS contains paths to client certificates for mTLS with targets (from cluster.Spec.ClientTLS) -// TODO: finish mapping fields from profile to config, reuse the same function as for Targets +// buildTunnelTargetMatch creates a TunnelTargetMatch from a TunnelTargetPolicy and +// TargetProfile. clientTLS contains paths to client certificates for mTLS with +// targets (from cluster.Spec.ClientTLS). +// +// The profile is mapped through the same helpers as a static Target, so a tunnel +// target and a declared one built from the same TargetProfile get the same +// connection settings. The hand-inlined copy this replaces had drifted: it never +// mapped the keepalives at all. func buildTunnelTargetMatch( policySpec *gnmicv1alpha1.TunnelTargetPolicySpec, profile *gnmicv1alpha1.TargetProfileSpec, @@ -25,81 +30,22 @@ func buildTunnelTargetMatch( match.ID = policySpec.Match.ID } - // build target config from profile - if profile != nil { - config := &gapi.TargetConfig{ - Timeout: durationOrDefault(&profile.Timeout, 10*time.Second), - RetryTimer: durationOrDefault(&profile.RetryTimer, 2*time.Second), - Encoding: ptr.To(profile.Encoding), - } - - // set credentials if provided - if creds != nil { - if creds.Username != "" { - config.Username = ptr.To(creds.Username) - } - if creds.Password != "" { - config.Password = ptr.To(creds.Password) - } - if creds.Token != "" { - config.Token = ptr.To(creds.Token) - } - } - - // no client TLS configuration at the cluster level or target profile level - if clientTLS == nil && profile.TLS == nil { - config.Insecure = ptr.To(true) - match.Config = config - return match - } - // TLS not enabled at the cluster level but enabled at the target profile level - if clientTLS == nil && profile.TLS != nil { - config.SkipVerify = ptr.To(true) - if profile.TLS.MaxVersion != "" { - config.TLSMaxVersion = profile.TLS.MaxVersion - } - if profile.TLS.MinVersion != "" { - config.TLSMinVersion = profile.TLS.MinVersion - } - if len(profile.TLS.CipherSuites) > 0 { - config.CipherSuites = profile.TLS.CipherSuites - } - match.Config = config - return match - } + if profile == nil { + return match + } - // use client TLS configuration from cluster (for mTLS with targets) - if clientTLS.CertFile != "" { - config.TLSCert = ptr.To(clientTLS.CertFile) - } - if clientTLS.KeyFile != "" { - config.TLSKey = ptr.To(clientTLS.KeyFile) - } - if clientTLS.CAFile != "" { - config.TLSCA = ptr.To(clientTLS.CAFile) - config.SkipVerify = ptr.To(false) - } else { - // TLS is enabled but without CA verification (TrustBundleRef not supported yet) - config.SkipVerify = ptr.To(true) - } - if profile.TLS == nil { - match.Config = config - return match - } - if profile.TLS.ServerName != "" { - config.TLSServerName = profile.TLS.ServerName - } - if profile.TLS.MaxVersion != "" { - config.TLSMaxVersion = profile.TLS.MaxVersion - } - if profile.TLS.MinVersion != "" { - config.TLSMinVersion = profile.TLS.MinVersion - } - if len(profile.TLS.CipherSuites) > 0 { - config.CipherSuites = profile.TLS.CipherSuites - } - match.Config = config + // A tunnel target's name and address are supplied by the device when it dials + // in, so only the profile-derived settings are configured here. + config := &gapi.TargetConfig{ + Timeout: durationOrDefault(&profile.Timeout, 10*time.Second), + RetryTimer: durationOrDefault(&profile.RetryTimer, 2*time.Second), + Encoding: ptr.To(profile.Encoding), } + applyProfileConnection(config, profile) + applyCredentials(config, creds) + applyTargetTLS(config, profile.TLS, clientTLS) + + match.Config = config return match }