Repository: metal3-io/baremetal-operator
Pull request #2322 (2025-03-17 14:51:39)

:warning: Only accept HTTPHeadersRef in same namespace
The BMCEventSubscription spec uses a Secret reference for the HTTPHeadersRef field. This allows the user to specify any namespace. To prevent leaking Secrets, this commit restricts the namespace to the same namespace that the BMCEventSubscription is in.



Patch:
diff --git a/apis/metal3.io/v1alpha1/bmceventsubscription_validation.go b/apis/metal3.io/v1alpha1/bmceventsubscription_validation.go
index 1aefae1b..e4e8c209 100644
--- a/apis/metal3.io/v1alpha1/bmceventsubscription_validation.go
+++ b/apis/metal3.io/v1alpha1/bmceventsubscription_validation.go
@@ -29,6 +29,12 @@ func (s *BMCEventSubscription) validateSubscription() []error {
 		errs = append(errs, errors.New("hostName cannot be empty"))
 	}
 
+	if s.Spec.HTTPHeadersRef != nil {
+		if s.Spec.HTTPHeadersRef.Namespace != s.Namespace {
+			errs = append(errs, errors.New("httpHeadersRef secret must be in the same namespace as the BMCEventSubscription"))
+		}
+	}
+
 	if s.Spec.Destination == "" {
 		errs = append(errs, errors.New("destination cannot be empty"))
 	} else {
diff --git a/controllers/metal3.io/bmceventsubscription_controller.go b/controllers/metal3.io/bmceventsubscription_controller.go
index 934eb810..b272427b 100644
--- a/controllers/metal3.io/bmceventsubscription_controller.go
+++ b/controllers/metal3.io/bmceventsubscription_controller.go
@@ -243,6 +243,10 @@ func (r *BMCEventSubscriptionReconciler) getHTTPHeaders(ctx context.Context, sub
 		return headers, nil
 	}
 
+	if subscription.Spec.HTTPHeadersRef.Namespace != subscription.Namespace {
+		return headers, errors.New("httpHeadersRef secret must be in the same namespace as the BMCEventSubscription")
+	}
+
 	secret := &corev1.Secret{}
 	secretKey := types.NamespacedName{
 		Name:      subscription.Spec.HTTPHeadersRef.Name,


Test patch:
diff --git a/apis/metal3.io/v1alpha1/bmceventsubscription_validation_test.go b/apis/metal3.io/v1alpha1/bmceventsubscription_validation_test.go
index c10514b7..6274118d 100644
--- a/apis/metal3.io/v1alpha1/bmceventsubscription_validation_test.go
+++ b/apis/metal3.io/v1alpha1/bmceventsubscription_validation_test.go
@@ -3,6 +3,7 @@ package v1alpha1
 import (
 	"testing"
 
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -69,6 +70,28 @@ func TestBMCEventSubscriptionValidateCreate(t *testing.T) {
 			oldS:      nil,
 			wantedErr: "hostname-only destination must have a trailing slash",
 		},
+		{
+			name: "httpHeadersRef valid",
+			newS: &BMCEventSubscription{
+				TypeMeta:   tm,
+				ObjectMeta: om,
+				Spec: BMCEventSubscriptionSpec{HostName: "worker-01", Destination: "http://localhost/abc/abc.php",
+					HTTPHeadersRef: &corev1.SecretReference{Namespace: om.Namespace, Name: "headers"}},
+			},
+			oldS:      nil,
+			wantedErr: "",
+		},
+		{
+			name: "httpHeadersRef in different namespace",
+			newS: &BMCEventSubscription{
+				TypeMeta:   tm,
+				ObjectMeta: om,
+				Spec: BMCEventSubscriptionSpec{HostName: "worker-01", Destination: "http://localhost/abc/abc.php",
+					HTTPHeadersRef: &corev1.SecretReference{Namespace: "different", Name: "headers"}},
+			},
+			oldS:      nil,
+			wantedErr: "httpHeadersRef secret must be in the same namespace as the BMCEventSubscription",
+		},
 	}
 
 	for _, tt := range tests {
diff --git a/controllers/metal3.io/bmceventsubscription_controller_test.go b/controllers/metal3.io/bmceventsubscription_controller_test.go
index 537c6ce5..052b99d0 100644
--- a/controllers/metal3.io/bmceventsubscription_controller_test.go
+++ b/controllers/metal3.io/bmceventsubscription_controller_test.go
@@ -139,3 +139,84 @@ func TestBMCGetProvisioner(t *testing.T) {
 		})
 	}
 }
+
+func TestGetHTTPHeaders(t *testing.T) {
+	// NOTE: This subscription references the defaultSecretName for http headers.
+	// The secret is automatically created by newBMCTestReconciler.
+	host := newDefaultHost(t)
+	subscription := newDefaultSubscription(t)
+	r := newBMCTestReconciler(subscription, host)
+
+	for _, tc := range []struct {
+		Scenario      string
+		Subscription  *metal3api.BMCEventSubscription
+		Secret        *corev1.Secret
+		ExpectedError bool
+	}{
+		{
+			Scenario:     "Secret exists and has some content",
+			Subscription: subscription,
+			// Already created by newBMCTestReconciler.
+			Secret:        nil,
+			ExpectedError: false,
+		},
+		{
+			Scenario: "Secret does not exist",
+			Subscription: &metal3api.BMCEventSubscription{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-subscription",
+					Namespace: namespace,
+				},
+				Spec: metal3api.BMCEventSubscriptionSpec{
+					HostName: host.Name,
+					HTTPHeadersRef: &corev1.SecretReference{
+						Name:      "non-existent-secret",
+						Namespace: namespace,
+					},
+				},
+			},
+			Secret:        nil,
+			ExpectedError: true,
+		},
+		{
+			Scenario: "Secret in wrong namespace",
+			Subscription: &metal3api.BMCEventSubscription{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-subscription",
+					Namespace: namespace,
+				},
+				Spec: metal3api.BMCEventSubscriptionSpec{
+					HostName: host.Name,
+					HTTPHeadersRef: &corev1.SecretReference{
+						Name:      "test",
+						Namespace: "separate-namespace",
+					},
+				},
+			},
+			Secret:        nil,
+			ExpectedError: true,
+		},
+	} {
+		t.Run(tc.Scenario, func(t *testing.T) {
+			if tc.Secret != nil {
+				err := r.Create(context.Background(), tc.Secret)
+				if err != nil {
+					t.Fatal(err)
+				}
+			}
+
+			headers, err := r.getHTTPHeaders(context.Background(), *tc.Subscription)
+			if tc.ExpectedError && err == nil {
+				t.Error("Expected error but got none")
+			}
+			if !tc.ExpectedError {
+				if err != nil {
+					t.Errorf("Expected no error but got: %v", err)
+				}
+				if len(headers) == 0 {
+					t.Error("Expected headers but got none")
+				}
+			}
+		})
+	}
+}