Skip to content

Commit 8da708b

Browse files
dc-tecdc-tec
authored
fix(openbao): guard metrics-only listener configuration (#481)
1 parent 5ae44b3 commit 8da708b

6 files changed

Lines changed: 187 additions & 24 deletions

File tree

‎internal/adapter/config/builder.go‎

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,11 @@ import (
1313
)
1414

1515
const (
16-
configUnsealKeyPath = "file:///etc/bao/unseal/key"
17-
configUnsealKeyID = "operator-generated-v1"
18-
configMaxRequestDuration = "90s"
19-
configNodeIDTemplate = "${HOSTNAME}"
16+
configUnsealKeyPath = "file:///etc/bao/unseal/key"
17+
configUnsealKeyID = "operator-generated-v1"
18+
configMaxRequestDuration = "90s"
19+
configNodeIDTemplate = "${HOSTNAME}"
20+
configScrapeProfileAllNodes = "AllNodes"
2021

2122
jwtPolicyHealthStepDownAutopilot = `path "sys/health" { capabilities = ["read"] }
2223
path "sys/step-down" { capabilities = ["sudo", "update"] }
@@ -223,6 +224,11 @@ func validateConfigVersionCompatibility(cluster *openbaov1alpha1.OpenBaoCluster)
223224
if cluster == nil {
224225
return fmt.Errorf("cluster is required")
225226
}
227+
228+
if err := validateMetricsOnlyListenerCompatibility(cluster); err != nil {
229+
return err
230+
}
231+
226232
if cluster.Spec.Configuration == nil || cluster.Spec.Configuration.Plugin == nil {
227233
return nil
228234
}
@@ -243,6 +249,34 @@ func validateConfigVersionCompatibility(cluster *openbaov1alpha1.OpenBaoCluster)
243249
return fmt.Errorf("spec.configuration.plugin.{autoDownload,autoRegister,downloadBehavior} requires OpenBao >= 2.5.0 (spec.version=%q)", cluster.Spec.Version)
244250
}
245251

252+
func validateMetricsOnlyListenerCompatibility(cluster *openbaov1alpha1.OpenBaoCluster) error {
253+
if !workloadMetricsEnabled(cluster) {
254+
return nil
255+
}
256+
257+
metrics := cluster.Spec.Observability.Metrics
258+
allNodes := strings.TrimSpace(metrics.ScrapeProfile) == configScrapeProfileAllNodes
259+
listener := metrics.MetricsOnlyListener
260+
if allNodes && listener != nil && listener.Enabled != nil && !*listener.Enabled {
261+
return fmt.Errorf("spec.observability.metrics.metricsOnlyListener.enabled cannot be false when scrapeProfile is AllNodes")
262+
}
263+
264+
listenerExplicitlyEnabled := listener != nil && listener.Enabled != nil && *listener.Enabled
265+
if !allNodes && !listenerExplicitlyEnabled {
266+
return nil
267+
}
268+
269+
ok, err := openBaoVersionAtLeast(cluster.Spec.Version, 2, 5, 0)
270+
if err != nil {
271+
return fmt.Errorf("failed to validate metrics-only listener version compatibility: %w", err)
272+
}
273+
if ok {
274+
return nil
275+
}
276+
277+
return fmt.Errorf("spec.observability.metrics.metricsOnlyListener requires OpenBao >= 2.5.0 (spec.version=%q)", cluster.Spec.Version)
278+
}
279+
246280
func openBaoVersionAtLeast(version string, wantMajor, wantMinor, wantPatch int) (bool, error) {
247281
return platformsemver.AtLeast(version, wantMajor, wantMinor, wantPatch)
248282
}

‎internal/adapter/config/builder_fuzz_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func FuzzRenderSelfInitHCL(f *testing.F) {
8484
bootstrapOn: false,
8585
clusterName: "demo",
8686
clusterNS: "default",
87-
clusterVer: "2.5.0",
87+
clusterVer: testOpenBaoVersion250,
8888
},
8989
{
9090
name: "policy",
@@ -96,7 +96,7 @@ func FuzzRenderSelfInitHCL(f *testing.F) {
9696
bootstrapOn: true,
9797
clusterName: "bootstrap",
9898
clusterNS: "operators",
99-
clusterVer: "2.5.0",
99+
clusterVer: testOpenBaoVersion250,
100100
},
101101
{
102102
name: "",
@@ -206,7 +206,7 @@ func sanitizeVersion(v string) string {
206206
if _, err := platformsemver.Parse(v); err == nil {
207207
return v
208208
}
209-
return "2.5.0"
209+
return testOpenBaoVersion250
210210
}
211211

212212
func FuzzRenderHCL(f *testing.F) {
@@ -228,7 +228,7 @@ func FuzzRenderHCL(f *testing.F) {
228228
telemetryAddr string
229229
enableObs bool
230230
}{
231-
{"demo", "default", "2.5.0", "demo", "default", 8200, 8201, "info", "3600h", "7200h", "continue", "file", "stdout", `{"file_path":"stdout"}`, "127.0.0.1:8125", true},
231+
{"demo", "default", testOpenBaoVersion250, "demo", "default", 8200, 8201, "info", "3600h", "7200h", "continue", "file", "stdout", `{"file_path":"stdout"}`, "127.0.0.1:8125", true},
232232
{"acme", "operators", "2.4.4", "acme", "operators", 8200, 8201, "debug", "", "", "", "socket", "custom-socket", `{"address":"127.0.0.1:9000"}`, "", false},
233233
{"broken", "default", "not-a-version", "", "", 0, -1, "", "", "", "", "", "", `{}`, "", false},
234234
}

‎internal/adapter/config/builder_test.go‎

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ import (
1313
openbaov1alpha1 "github.com/dc-tec/openbao-operator/api/v1alpha1"
1414
)
1515

16+
const (
17+
testOpenBaoVersion250 = "2.5.0"
18+
testOpenBaoImage250 = "openbao/openbao:2.5.0"
19+
testMetricsListenerVersionHint = "requires OpenBao >= 2.5.0"
20+
)
21+
1622
func newMinimalCluster(name, namespace string) *openbaov1alpha1.OpenBaoCluster {
1723
return &openbaov1alpha1.OpenBaoCluster{
1824
ObjectMeta: metav1.ObjectMeta{
@@ -153,8 +159,8 @@ func TestRenderHCLIncludesCoreStanzas(t *testing.T) {
153159

154160
func TestRenderHCLWithStructuredConfiguration(t *testing.T) {
155161
cluster := newMinimalCluster("structured-config", "default")
156-
cluster.Spec.Version = "2.5.0"
157-
cluster.Spec.Image = "openbao/openbao:2.5.0"
162+
cluster.Spec.Version = testOpenBaoVersion250
163+
cluster.Spec.Image = testOpenBaoImage250
158164
uiEnabled := true
159165
autoDownload := true
160166
autoRegister := false
@@ -230,7 +236,7 @@ func TestRenderHCLRejectsPluginAutoConfigForUnsupportedVersions(t *testing.T) {
230236
if err == nil {
231237
t.Fatalf("RenderHCL() expected error, got nil")
232238
}
233-
if !strings.Contains(err.Error(), "requires OpenBao >= 2.5.0") {
239+
if !strings.Contains(err.Error(), testMetricsListenerVersionHint) {
234240
t.Fatalf("RenderHCL() error = %v, want version gate error", err)
235241
}
236242
})
@@ -557,10 +563,12 @@ func TestRenderHCLWithObservabilityMetricsTelemetry(t *testing.T) {
557563

558564
func TestRenderHCLWithAllNodeMetricsListener(t *testing.T) {
559565
cluster := newMinimalCluster("metrics-listener", "default")
566+
cluster.Spec.Version = testOpenBaoVersion250
567+
cluster.Spec.Image = testOpenBaoImage250
560568
cluster.Spec.Observability = &openbaov1alpha1.ObservabilityConfig{
561569
Metrics: &openbaov1alpha1.MetricsConfig{
562570
Enabled: true,
563-
ScrapeProfile: "AllNodes",
571+
ScrapeProfile: configScrapeProfileAllNodes,
564572
},
565573
}
566574

@@ -591,8 +599,69 @@ func TestRenderHCLWithAllNodeMetricsListener(t *testing.T) {
591599
}
592600
}
593601

602+
func TestRenderHCLWithMetricsOnlyListenerRejectsUnsupportedVersions(t *testing.T) {
603+
tests := []struct {
604+
name string
605+
configure func(*openbaov1alpha1.MetricsConfig)
606+
wantErrSubstr string
607+
}{
608+
{
609+
name: "all nodes",
610+
configure: func(metrics *openbaov1alpha1.MetricsConfig) {
611+
metrics.ScrapeProfile = configScrapeProfileAllNodes
612+
},
613+
wantErrSubstr: testMetricsListenerVersionHint,
614+
},
615+
{
616+
name: "explicit listener",
617+
configure: func(metrics *openbaov1alpha1.MetricsConfig) {
618+
enabled := true
619+
metrics.MetricsOnlyListener = &openbaov1alpha1.MetricsOnlyListenerConfig{
620+
Enabled: &enabled,
621+
}
622+
},
623+
wantErrSubstr: testMetricsListenerVersionHint,
624+
},
625+
{
626+
name: "all nodes disabled listener",
627+
configure: func(metrics *openbaov1alpha1.MetricsConfig) {
628+
enabled := false
629+
metrics.ScrapeProfile = configScrapeProfileAllNodes
630+
metrics.MetricsOnlyListener = &openbaov1alpha1.MetricsOnlyListenerConfig{
631+
Enabled: &enabled,
632+
}
633+
},
634+
wantErrSubstr: "cannot be false when scrapeProfile is AllNodes",
635+
},
636+
}
637+
638+
for _, tt := range tests {
639+
t.Run(tt.name, func(t *testing.T) {
640+
cluster := newMinimalCluster("metrics-version", "default")
641+
metrics := &openbaov1alpha1.MetricsConfig{Enabled: true}
642+
tt.configure(metrics)
643+
cluster.Spec.Observability = &openbaov1alpha1.ObservabilityConfig{Metrics: metrics}
644+
645+
_, err := RenderHCL(cluster, InfrastructureDetails{
646+
HeadlessServiceName: cluster.Name,
647+
Namespace: cluster.Namespace,
648+
APIPort: 8200,
649+
ClusterPort: 8201,
650+
})
651+
if err == nil {
652+
t.Fatal("expected error, got nil")
653+
}
654+
if !strings.Contains(err.Error(), tt.wantErrSubstr) {
655+
t.Fatalf("RenderHCL() error = %v, want containing %q", err, tt.wantErrSubstr)
656+
}
657+
})
658+
}
659+
}
660+
594661
func TestRenderHCLWithMetricsOnlyListenerRejectsACME(t *testing.T) {
595662
cluster := newMinimalCluster("metrics-acme", "default")
663+
cluster.Spec.Version = testOpenBaoVersion250
664+
cluster.Spec.Image = testOpenBaoImage250
596665
cluster.Spec.TLS.Mode = openbaov1alpha1.TLSModeACME
597666
cluster.Spec.TLS.ACME = &openbaov1alpha1.ACMEConfig{
598667
Email: "platform@example.com",
@@ -602,7 +671,7 @@ func TestRenderHCLWithMetricsOnlyListenerRejectsACME(t *testing.T) {
602671
cluster.Spec.Observability = &openbaov1alpha1.ObservabilityConfig{
603672
Metrics: &openbaov1alpha1.MetricsConfig{
604673
Enabled: true,
605-
ScrapeProfile: "AllNodes",
674+
ScrapeProfile: configScrapeProfileAllNodes,
606675
},
607676
}
608677

‎internal/adapter/config/render_listener.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ func metricsOnlyListenerEnabled(cluster *openbaov1alpha1.OpenBaoCluster) bool {
126126
if listener != nil && listener.Enabled != nil {
127127
return *listener.Enabled
128128
}
129-
return cluster.Spec.Observability.Metrics.ScrapeProfile == "AllNodes"
129+
return cluster.Spec.Observability.Metrics.ScrapeProfile == configScrapeProfileAllNodes
130130
}
131131

132132
func metricsOnlyListenerPort(cluster *openbaov1alpha1.OpenBaoCluster) int32 {
@@ -146,5 +146,5 @@ func metricsOnlyListenerUnauthenticatedAccess(cluster *openbaov1alpha1.OpenBaoCl
146146
return *enabled
147147
}
148148
}
149-
return cluster.Spec.Observability.Metrics.ScrapeProfile == "AllNodes"
149+
return cluster.Spec.Observability.Metrics.ScrapeProfile == configScrapeProfileAllNodes
150150
}

‎internal/service/networking/policy_rules.go‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,12 @@ func buildNetworkPolicyIngressRules(
5858
}
5959

6060
if cluster.Spec.Network != nil {
61+
trustedIngressPorts := []intstr.IntOrString{apiPort}
62+
if metricsOnlyListenerEnabled(cluster) {
63+
trustedIngressPorts = append(trustedIngressPorts, intstr.FromInt(int(metricsOnlyListenerPort(cluster))))
64+
}
6165
for _, peer := range cluster.Spec.Network.TrustedIngressPeers {
62-
rules = appendIngressPeerRule(rules, peer, apiPort)
66+
rules = appendIngressPeerRule(rules, peer, trustedIngressPorts...)
6367
}
6468
}
6569

@@ -103,16 +107,18 @@ func buildNetworkPolicyIngressRules(
103107
func appendIngressPeerRule(
104108
rules []networkingv1.NetworkPolicyIngressRule,
105109
peer networkingv1.NetworkPolicyPeer,
106-
apiPort intstr.IntOrString,
110+
ports ...intstr.IntOrString,
107111
) []networkingv1.NetworkPolicyIngressRule {
112+
networkPolicyPorts := make([]networkingv1.NetworkPolicyPort, 0, len(ports))
113+
for _, port := range ports {
114+
networkPolicyPorts = append(networkPolicyPorts, networkingv1.NetworkPolicyPort{
115+
Protocol: &[]corev1.Protocol{corev1.ProtocolTCP}[0],
116+
Port: &port,
117+
})
118+
}
108119
return append(rules, networkingv1.NetworkPolicyIngressRule{
109-
From: []networkingv1.NetworkPolicyPeer{peer},
110-
Ports: []networkingv1.NetworkPolicyPort{
111-
{
112-
Protocol: &[]corev1.Protocol{corev1.ProtocolTCP}[0],
113-
Port: &apiPort,
114-
},
115-
},
120+
From: []networkingv1.NetworkPolicyPeer{peer},
121+
Ports: networkPolicyPorts,
116122
})
117123
}
118124

‎internal/service/networking/policy_test.go‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,60 @@ func TestBuildNetworkPolicy_TrustedIngressPeers(t *testing.T) {
609609
}
610610
}
611611

612+
func TestBuildNetworkPolicy_TrustedIngressPeersAllowMetricsListenerPort(t *testing.T) {
613+
cluster := &openbaov1alpha1.OpenBaoCluster{
614+
ObjectMeta: metav1.ObjectMeta{
615+
Name: "trusted-metrics",
616+
Namespace: "openbao",
617+
},
618+
Spec: openbaov1alpha1.OpenBaoClusterSpec{
619+
Observability: &openbaov1alpha1.ObservabilityConfig{
620+
Metrics: &openbaov1alpha1.MetricsConfig{
621+
Enabled: true,
622+
ScrapeProfile: metricsScrapeProfileAllNode,
623+
MetricsOnlyListener: &openbaov1alpha1.MetricsOnlyListenerConfig{
624+
Port: 9202,
625+
},
626+
},
627+
},
628+
Network: &openbaov1alpha1.NetworkConfig{
629+
TrustedIngressPeers: []networkingv1.NetworkPolicyPeer{
630+
{
631+
NamespaceSelector: &metav1.LabelSelector{
632+
MatchLabels: map[string]string{
633+
"kubernetes.io/metadata.name": "monitoring",
634+
},
635+
},
636+
},
637+
},
638+
},
639+
},
640+
}
641+
642+
policy, err := buildNetworkPolicy(cluster, &apiServerInfo{ServiceNetworkCIDR: "10.96.0.0/12"}, "openbao-operator-system")
643+
if err != nil {
644+
t.Fatalf("buildNetworkPolicy() error: %v", err)
645+
}
646+
647+
var foundMonitoringRule bool
648+
for _, rule := range policy.Spec.Ingress {
649+
if !networkPolicyRuleAllowsPort(rule.Ports, constants.PortAPI) ||
650+
!networkPolicyRuleAllowsPort(rule.Ports, 9202) {
651+
continue
652+
}
653+
for _, peer := range rule.From {
654+
if peer.NamespaceSelector != nil &&
655+
peer.NamespaceSelector.MatchLabels["kubernetes.io/metadata.name"] == "monitoring" {
656+
foundMonitoringRule = true
657+
}
658+
}
659+
}
660+
661+
if !foundMonitoringRule {
662+
t.Fatal("expected trusted monitoring peer to allow API and metrics listener ports")
663+
}
664+
}
665+
612666
func TestBuildNetworkPolicy_IngressEnabledDoesNotAllowUnrestrictedAPIIngress(t *testing.T) {
613667
cluster := &openbaov1alpha1.OpenBaoCluster{
614668
ObjectMeta: metav1.ObjectMeta{

0 commit comments

Comments
 (0)