injector: enforce using configured images (#4131)

This change enforces that images configured by the user or install time defaults are always used at the time of sidecar injection. Previously, default images were encoded in the configurator which posed a security risk of not using configured images in case those values are unavailable in MeshConfig and the user overrides the defaults. It's common practice for users to use their own images from secure registries of their choice, so OSM must enforce that. This problem is made worse by the fact that OSM could silently use defaults that the user is unaware of without raising any warnings or approval from the user, which can compromise their security requirements. This change is also required to address #3715 where default image digests will be encoded in the CLI as a part of the release workflow without needing to rebuild the control plane binaries. Signed-off-by: Shashank Ram <[email protected]>
openservicemesh · Sep 17, 2021 · 60a9754 · 60a9754
1 parent 2bb06c0
commit 60a9754
Show file tree

Hide file tree

Showing 6 changed files with 131 additions and 36 deletions.
diff --git a/pkg/configurator/methods.go b/pkg/configurator/methods.go
@@ -129,29 +129,17 @@ func (c *client) GetEnvoyLogLevel() string {
 
 // GetEnvoyImage returns the envoy image
 func (c *client) GetEnvoyImage() string {
-	image := c.getMeshConfig().Spec.Sidecar.EnvoyImage
-	if image != "" {
-		return image
-	}
-	return constants.DefaultEnvoyImage
+	return c.getMeshConfig().Spec.Sidecar.EnvoyImage
 }
 
 // GetEnvoyWindowsImage returns the envoy windows image
 func (c *client) GetEnvoyWindowsImage() string {
-	image := c.getMeshConfig().Spec.Sidecar.EnvoyWindowsImage
-	if image != "" {
-		return image
-	}
-	return constants.DefaultEnvoyWindowsImage
+	return c.getMeshConfig().Spec.Sidecar.EnvoyWindowsImage
 }
 
 // GetInitContainerImage returns the init container image
 func (c *client) GetInitContainerImage() string {
-	initImage := c.getMeshConfig().Spec.Sidecar.InitContainerImage
-	if initImage != "" {
-		return initImage
-	}
-	return constants.DefaultInitContainerImage
+	return c.getMeshConfig().Spec.Sidecar.InitContainerImage
 }
 
 // GetServiceCertValidityPeriod returns the validity duration for service certificates, and a default in case of invalid duration

diff --git a/pkg/configurator/methods_test.go b/pkg/configurator/methods_test.go
@@ -233,7 +233,7 @@ func TestCreateUpdateConfig(t *testing.T) {
 			name:                  "GetEnvoyImage",
 			initialMeshConfigData: &v1alpha1.MeshConfigSpec{},
 			checkCreate: func(assert *tassert.Assertions, cfg Configurator) {
-				assert.Equal("envoyproxy/envoy-alpine@sha256:6502a637c6c5fba4d03d0672d878d12da4bcc7a0d0fb3f1d506982dde0039abd", cfg.GetEnvoyImage())
+				assert.Equal("", cfg.GetEnvoyImage())
 			},
 			updatedMeshConfigData: &v1alpha1.MeshConfigSpec{
 				Sidecar: v1alpha1.SidecarSpec{
@@ -244,11 +244,26 @@ func TestCreateUpdateConfig(t *testing.T) {
 				assert.Equal("envoyproxy/envoy-alpine:v1.17.1", cfg.GetEnvoyImage())
 			},
 		},
+		{
+			name:                  "GetEnvoyWindowsImage",
+			initialMeshConfigData: &v1alpha1.MeshConfigSpec{},
+			checkCreate: func(assert *tassert.Assertions, cfg Configurator) {
+				assert.Equal("", cfg.GetEnvoyWindowsImage())
+			},
+			updatedMeshConfigData: &v1alpha1.MeshConfigSpec{
+				Sidecar: v1alpha1.SidecarSpec{
+					EnvoyImage: "envoyproxy/envoy-windows:v1.17.1",
+				},
+			},
+			checkUpdate: func(assert *tassert.Assertions, cfg Configurator) {
+				assert.Equal("envoyproxy/envoy-windows:v1.17.1", cfg.GetEnvoyImage())
+			},
+		},
 		{
 			name:                  "GetInitContainerImage",
 			initialMeshConfigData: &v1alpha1.MeshConfigSpec{},
 			checkCreate: func(assert *tassert.Assertions, cfg Configurator) {
-				assert.Equal("openservicemesh/init:v0.9.2", cfg.GetInitContainerImage())
+				assert.Equal("", cfg.GetInitContainerImage())
 			},
 			updatedMeshConfigData: &v1alpha1.MeshConfigSpec{
 				Sidecar: v1alpha1.SidecarSpec{

diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go
@@ -58,17 +58,6 @@ const (
 	// DefaultOSMLogLevel is the default OSM log level if none is specified
 	DefaultOSMLogLevel = "info"
 
-	// DefaultEnvoyImage is the default envoy proxy sidecar image if not defined in the osm MeshConfig (v1.19.1)
-	DefaultEnvoyImage = "envoyproxy/envoy-alpine@sha256:6502a637c6c5fba4d03d0672d878d12da4bcc7a0d0fb3f1d506982dde0039abd"
-
-	// DefaultEnvoyWindowsImage is the default envoy proxy windows sidecar image if not defined in the osm MeshConfig (v1.19.1)
-	// TODO(#3864): This should be updated to the nanoserver based image when it becomes available
-	// See https://github.com/envoyproxy/envoy/issues/16759
-	DefaultEnvoyWindowsImage = "envoyproxy/envoy-windows@sha256:c904fda95891ebbccb9b1f24c1a9482c8d01cbca215dd081fc8c8db36db85f85"
-
-	// DefaultInitContainerImage is the default init container image if not defined in the osm MeshConfig
-	DefaultInitContainerImage = "openservicemesh/init:v0.9.2"
-
 	// EnvoyPrometheusInboundListenerPort is Envoy's inbound listener port number for prometheus
 	EnvoyPrometheusInboundListenerPort = 15010
 

diff --git a/pkg/injector/patch.go b/pkg/injector/patch.go
@@ -9,6 +9,7 @@ import (
 
 	mapset "github.com/deckarep/golang-set"
 	"github.com/google/uuid"
+	"github.com/pkg/errors"
 	"gomodules.xyz/jsonpatch/v2"
 	admissionv1 "k8s.io/api/admission/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -60,6 +61,9 @@ func (wh *mutatingWebhook) createPatch(pod *corev1.Pod, req *admissionv1.Admissi
 	// As a result we assume that the HNS redirection policies are already programmed via a CNI plugin.
 	// Skip adding the init container and only patch the pod spec with sidecar container.
 	podOS := pod.Spec.NodeSelector["kubernetes.io/os"]
+	if err := wh.verifyPrerequisites(podOS); err != nil {
+		return nil, err
+	}
 	if !strings.EqualFold(podOS, constants.OSWindows) {
 		// Build outbound port exclusion list
 		podOutboundPortExclusionList, _ := wh.getPortExclusionListForPod(pod, namespace, outboundPortExclusionListAnnotation)
@@ -105,6 +109,27 @@ func (wh *mutatingWebhook) createPatch(pod *corev1.Pod, req *admissionv1.Admissi
 	return json.Marshal(makePatches(req, pod))
 }
 
+// verifyPrerequisites verifies if the prerequisites to patch the request are met by returning an error if unmet
+func (wh *mutatingWebhook) verifyPrerequisites(podOS string) error {
+	isWindows := strings.EqualFold(podOS, constants.OSWindows)
+
+	// Verify that the required images are configured
+	if image := wh.configurator.GetEnvoyImage(); !isWindows && image == "" {
+		// Linux pods require Envoy Linux image
+		return errors.New("MeshConfig sidecar.envoyImage not set")
+	}
+	if image := wh.configurator.GetEnvoyWindowsImage(); isWindows && image == "" {
+		// Windows pods require Envoy Windows image
+		return errors.New("MeshConfig sidecar.envoyWindowsImage not set")
+	}
+	if image := wh.configurator.GetInitContainerImage(); !isWindows && image == "" {
+		// Linux pods require init container image
+		return errors.New("MeshConfig sidecar.initContainerImage not set")
+	}
+
+	return nil
+}
+
 func makePatches(req *admissionv1.AdmissionRequest, pod *corev1.Pod) []jsonpatch.JsonPatchOperation {
 	original := req.Object.Raw
 	current, err := json.Marshal(pod)

diff --git a/pkg/injector/patch_test.go b/pkg/injector/patch_test.go
@@ -140,11 +140,11 @@ func TestCreatePatch(t *testing.T) {
 				nonInjectNamespaces: mapset.NewSet(),
 			}
 
-			mockConfigurator.EXPECT().GetEnvoyWindowsImage().Return("").AnyTimes()
-			mockConfigurator.EXPECT().GetEnvoyImage().Return("").AnyTimes()
+			mockConfigurator.EXPECT().GetEnvoyWindowsImage().Return("envoy-linux-image").AnyTimes()
+			mockConfigurator.EXPECT().GetEnvoyImage().Return("envoy-windows-image").AnyTimes()
+			mockConfigurator.EXPECT().GetInitContainerImage().Return("init-container-image").AnyTimes()
 
 			mockConfigurator.EXPECT().GetEnvoyLogLevel().Return("").Times(1)
-			mockConfigurator.EXPECT().GetInitContainerImage().Return("").Times(1)
 			mockConfigurator.EXPECT().IsPrivilegedInitContainer().Return(false).Times(1)
 			mockConfigurator.EXPECT().GetOutboundIPRangeExclusionList().Return(nil).Times(1)
 			mockConfigurator.EXPECT().GetOutboundPortExclusionList().Return(nil).Times(1)
@@ -264,3 +264,70 @@ func TestMergePortExclusionLists(t *testing.T) {
 		})
 	}
 }
+
+func TestVerifyPrerequisites(t *testing.T) {
+	testCases := []struct {
+		name         string
+		podOS        string
+		linuxImage   string
+		windowsImage string
+		initImage    string
+		expectErr    bool
+	}{
+		{
+			name:       "prereqs met for linux pod",
+			linuxImage: "envoy",
+			initImage:  "init",
+			expectErr:  false,
+		},
+		{
+			name:       "prereqs not met for linux pod when init container image is missing",
+			linuxImage: "envoy",
+			expectErr:  true,
+		},
+		{
+			name:      "prereqs not met for linux pod when envoy container image is missing",
+			initImage: "init",
+			expectErr: true,
+		},
+		{
+			name:         "prereqs met for windows pod",
+			podOS:        "windows",
+			windowsImage: "windows",
+			initImage:    "init",
+			expectErr:    false,
+		},
+		{
+			name:         "prereqs met for windows pod when init container image is missing",
+			podOS:        "windows",
+			windowsImage: "envoy",
+			expectErr:    false,
+		},
+		{
+			name:      "prereqs not met for windows pod when envoy container image is missing",
+			initImage: "init",
+			expectErr: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			mockCtrl := gomock.NewController(t)
+			defer mockCtrl.Finish()
+
+			assert := tassert.New(t)
+			mockCfg := configurator.NewMockConfigurator(mockCtrl)
+
+			wh := &mutatingWebhook{
+				configurator: mockCfg,
+			}
+
+			mockCfg.EXPECT().GetEnvoyImage().Return(tc.linuxImage).AnyTimes()
+			mockCfg.EXPECT().GetEnvoyWindowsImage().Return(tc.windowsImage).AnyTimes()
+			mockCfg.EXPECT().GetInitContainerImage().Return(tc.initImage).AnyTimes()
+
+			err := wh.verifyPrerequisites(tc.podOS)
+			assert.Equal(tc.expectErr, err != nil)
+		})
+	}
+}
diff --git a/pkg/injector/webhook_test.go b/pkg/injector/webhook_test.go
@@ -956,8 +956,16 @@ func TestPodCreationHandler(t *testing.T) {
 }
 
 func TestWebhookMutate(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	mockConfigurator := configurator.NewMockConfigurator(mockCtrl)
+	mockConfigurator.EXPECT().GetEnvoyImage().Return("envoy-linux-image").AnyTimes()
+	mockConfigurator.EXPECT().GetEnvoyWindowsImage().Return("envoy-windows-image").AnyTimes()
+	mockConfigurator.EXPECT().GetInitContainerImage().Return("init-container-image").AnyTimes()
+
 	t.Run("invalid JSON", func(t *testing.T) {
-		var wh *mutatingWebhook
+		wh := &mutatingWebhook{
+			configurator: mockConfigurator,
+		}
 		req := &admissionv1.AdmissionRequest{
 			Object: runtime.RawExtension{Raw: []byte("{")},
 		}
@@ -978,6 +986,7 @@ func TestWebhookMutate(t *testing.T) {
 		wh := &mutatingWebhook{
 			nonInjectNamespaces: mapset.NewSet(),
 			kubeController:      kubeController,
+			configurator:        mockConfigurator,
 		}
 
 		req := &admissionv1.AdmissionRequest{
@@ -1010,8 +1019,9 @@ func TestWebhookMutate(t *testing.T) {
 		cfg.EXPECT().GetInboundPortExclusionList()
 		cfg.EXPECT().GetOutboundIPRangeExclusionList()
 		cfg.EXPECT().IsPrivilegedInitContainer()
-		cfg.EXPECT().GetInitContainerImage()
-		cfg.EXPECT().GetEnvoyImage()
+		cfg.EXPECT().GetInitContainerImage().Return("init-container-image").AnyTimes()
+		cfg.EXPECT().GetEnvoyImage().Return("envoy-linux-image").AnyTimes()
+		cfg.EXPECT().GetEnvoyWindowsImage().Return("envoy-windows-image").AnyTimes()
 		cfg.EXPECT().GetProxyResources()
 		cfg.EXPECT().GetEnvoyLogLevel()
 
@@ -1057,8 +1067,9 @@ func TestWebhookMutate(t *testing.T) {
 		cfg.EXPECT().GetInboundPortExclusionList()
 		cfg.EXPECT().GetOutboundIPRangeExclusionList()
 		cfg.EXPECT().IsPrivilegedInitContainer()
-		cfg.EXPECT().GetInitContainerImage()
-		cfg.EXPECT().GetEnvoyImage()
+		cfg.EXPECT().GetInitContainerImage().Return("init-container-image").AnyTimes()
+		cfg.EXPECT().GetEnvoyImage().Return("envoy-linux-image").AnyTimes()
+		cfg.EXPECT().GetEnvoyWindowsImage().Return("envoy-windows-image").AnyTimes()
 		cfg.EXPECT().GetProxyResources()
 		cfg.EXPECT().GetEnvoyLogLevel()