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]>

Author: shashankram

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

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{

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
 

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)

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)
+		})
+	}
+}

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()