Feature: Reject unknown webhook sources (#28)

Co-authored-by: Danny Petitclerc <[email protected]> Co-authored-by: David Katz <[email protected]>
kanopy-platform · Nov 1, 2024 · 276460d · 276460d
1 parent 34edfba
commit 276460d
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 13 deletions.
diff --git a/internal/admission/eventsource/handler.go b/internal/admission/eventsource/handler.go
@@ -24,12 +24,14 @@ type Handler struct {
 	annotationKey string
 	meshChecker   MeshChecker
 	decoder       *admission.Decoder
+	knownSources  map[string]bool
 }
 
-func NewHandler(mc MeshChecker) *Handler {
+func NewHandler(mc MeshChecker, knownSources map[string]bool) *Handler {
 	return &Handler{
 		annotationKey: DefaultAnnotationKey,
 		meshChecker:   mc,
+		knownSources:  knownSources,
 	}
 }
 
@@ -62,12 +64,16 @@ func (h *Handler) Handle(ctx context.Context, req admission.Request) admission.R
 		return admission.Errored(http.StatusBadRequest, err)
 	}
 
-	_, ok := out.Annotations[h.annotationKey]
+	sourceValue, ok := out.Annotations[h.annotationKey]
 	if !ok {
 		log.V(1).Info("Annotation not found, ignoring eventsource")
 		return admission.Allowed("No modifications needed")
 	}
 
+	if _, ok := h.knownSources[sourceValue]; !ok {
+		return admission.Denied(fmt.Sprintf("Unknown webhook source '%s'. Only known webhook sources are allowed.", sourceValue))
+	}
+
 	onMesh, err := h.meshChecker.OnMesh(out.Namespace)
 	if err != nil {
 		return admission.Errored(http.StatusInternalServerError, err)

diff --git a/internal/admission/eventsource/handler_test.go b/internal/admission/eventsource/handler_test.go
@@ -29,7 +29,11 @@ func TestEventSourceHandler(t *testing.T) {
 		Mesh: true,
 	}
 
-	handler := eventsource.NewHandler(fmc)
+	knownSources := map[string]bool{
+		"github": true,
+	}
+
+	handler := eventsource.NewHandler(fmc, knownSources)
 	scheme := runtime.NewScheme()
 	utilruntime.Must(esv1alpha1.AddToScheme(scheme))
 	decoder, err := admission.NewDecoder(scheme)
@@ -55,7 +59,7 @@ func TestEventSourceHandler(t *testing.T) {
 			es: esv1alpha1.EventSource{
 				ObjectMeta: v1.ObjectMeta{
 					Annotations: map[string]string{
-						eventsource.DefaultAnnotationKey: "true",
+						eventsource.DefaultAnnotationKey: "github",
 					},
 				},
 				Spec: esv1alpha1.EventSourceSpec{
@@ -79,7 +83,7 @@ func TestEventSourceHandler(t *testing.T) {
 			es: esv1alpha1.EventSource{
 				ObjectMeta: v1.ObjectMeta{
 					Annotations: map[string]string{
-						eventsource.DefaultAnnotationKey: "false",
+						eventsource.DefaultAnnotationKey: "github",
 					},
 				},
 				Spec: esv1alpha1.EventSourceSpec{
@@ -96,7 +100,7 @@ func TestEventSourceHandler(t *testing.T) {
 			es: esv1alpha1.EventSource{
 				ObjectMeta: v1.ObjectMeta{
 					Annotations: map[string]string{
-						eventsource.DefaultAnnotationKey: "true",
+						eventsource.DefaultAnnotationKey: "github",
 					},
 				},
 			},
@@ -107,13 +111,25 @@ func TestEventSourceHandler(t *testing.T) {
 			es: esv1alpha1.EventSource{
 				ObjectMeta: v1.ObjectMeta{
 					Annotations: map[string]string{
-						eventsource.DefaultAnnotationKey: "true",
+						eventsource.DefaultAnnotationKey: "github",
 					},
 				},
 			},
 			nsErr: errors.New("test error"),
 		},
+		{
+			name: "Unknown Source",
+			es: esv1alpha1.EventSource{
+				ObjectMeta: v1.ObjectMeta{
+					Annotations: map[string]string{
+						eventsource.DefaultAnnotationKey: "unknown-source",
+					},
+				},
+			},
+			err: true,
+		},
 	}
+
 	for _, test := range tests {
 		if test.key == "" {
 			test.key = eventsource.DefaultAnnotationKey
@@ -140,7 +156,7 @@ func TestEventSourceHandler(t *testing.T) {
 
 		if test.nsErr != nil {
 			assert.False(t, resp.AdmissionResponse.Allowed, test.name)
-			assert.True(t, resp.AdmissionResponse.Result.Message == test.nsErr.Error(), test.name)
+			assert.Equal(t, test.nsErr.Error(), resp.AdmissionResponse.Result.Message, test.name)
 			continue
 		}
 
@@ -151,14 +167,19 @@ func TestEventSourceHandler(t *testing.T) {
 			continue
 		}
 
+		if test.err {
+			assert.False(t, resp.AdmissionResponse.Allowed, test.name)
+			assert.Contains(t, resp.AdmissionResponse.Result.Reason, "Unknown webhook source", test.name)
+			continue
+		}
+
 		if test.nsOnMesh != nil {
 			assert.False(t, resp.AdmissionResponse.Allowed, test.name)
 			continue
 		}
 
 		assert.True(t, resp.AdmissionResponse.Allowed, test.name)
 		assert.Equal(t, 1, len(resp.Patches), test.name)
-
 	}
 }
 

diff --git a/internal/admission/handler_test.go b/internal/admission/handler_test.go
@@ -66,6 +66,10 @@ func TestRoutingHandler(t *testing.T) {
 		Mesh: true,
 	}
 
+	knownSources := map[string]bool{
+		"github": true,
+	}
+
 	tests := map[string]struct {
 		sensor *sensor.Handler
 		es     *eventsource.Handler
@@ -74,8 +78,8 @@ func TestRoutingHandler(t *testing.T) {
 	}{
 		"empty":      {sdeny: true, esdeny: true},
 		"sensoronly": {sensor: sensor.NewHandler(&frlg, rc), esdeny: true},
-		"esonly":     {es: eventsource.NewHandler(fmc), sdeny: true},
-		"both":       {sensor: sensor.NewHandler(&frlg, rc), es: eventsource.NewHandler(fmc)},
+		"esonly":     {es: eventsource.NewHandler(fmc, knownSources), sdeny: true},
+		"both":       {sensor: sensor.NewHandler(&frlg, rc), es: eventsource.NewHandler(fmc, knownSources)},
 	}
 
 	for name, test := range tests {

diff --git a/internal/cli/cli.go b/internal/cli/cli.go
@@ -245,8 +245,6 @@ func (c *RootCommand) runE(cmd *cobra.Command, args []string) error {
 		filteredIstioInformerFactory.Start(wait.NeverStop)
 		filteredIstioInformerFactory.WaitForCacheSync(wait.NeverStop)
 
-		eventSourceHandler = esadd.NewHandler(nsInformer)
-
 		gws := stringutils.StringToMap(viper.GetString("gateway-selector"), ",", "=")
 		if len(gws) == 0 {
 			return fmt.Errorf("Invalid gateway-selector: %s", viper.GetString("gateway-selector"))
@@ -270,6 +268,8 @@ func (c *RootCommand) runE(cmd *cobra.Command, args []string) error {
 			return err
 		}
 
+		eventSourceHandler = esadd.NewHandler(nsInformer, escc.GetKnownSources())
+
 		esi.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
 			AddFunc: func(new interface{}) {}})
 

diff --git a/internal/controllers/eventsource/controller.go b/internal/controllers/eventsource/controller.go
@@ -52,6 +52,14 @@ func (c *EventSourceIngressControllerConfig) SetIPGetter(name string, getter v1.
 	c.ipGetters[name] = getter
 }
 
+func (c *EventSourceIngressControllerConfig) GetKnownSources() map[string]bool {
+	knownSources := make(map[string]bool)
+	for source := range c.ipGetters {
+		knownSources[source] = true
+	}
+	return knownSources
+}
+
 func NewEventSourceIngressController(esl eslister.EventSourceLister, svcl corev1lister.ServiceLister, config EventSourceIngressControllerConfig, igc v1.IngressConfigurator) *EventSourceIngressController {
 	return &EventSourceIngressController{
 		esLister:      esl,

diff --git a/internal/controllers/eventsource/controller_test.go b/internal/controllers/eventsource/controller_test.go
@@ -341,3 +341,27 @@ func TestServiceToPortMapping(t *testing.T) {
 	}
 
 }
+
+// Faking IPGetter interface
+type FakeIPGetter struct{}
+
+func (f *FakeIPGetter) GetIPs() ([]string, error) {
+	return []string{"0.0.0.0/32"}, nil
+}
+
+func TestGetKnownSources(t *testing.T) {
+	escConfig := NewEventSourceIngressControllerConfig()
+
+	// Mimic the cli configuration
+	escConfig.SetIPGetter("github", &FakeIPGetter{})
+	escConfig.SetIPGetter("officeips", &FakeIPGetter{})
+
+	knownSources := escConfig.GetKnownSources()
+
+	expectedSources := map[string]bool{
+		"github":    true,
+		"officeips": true,
+	}
+
+	assert.Equal(t, expectedSources, knownSources)
+}