Skip to content

Commit 60a9754

Browse files
shashankramshashankram
authored
injector: enforce using configured images (openservicemesh#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 openservicemesh#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]>
1 parent 2bb06c0 commit 60a9754

File tree

6 files changed

+131
-36
lines changed

6 files changed

+131
-36
lines changed

‎pkg/configurator/methods.go‎

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -129,29 +129,17 @@ func (c *client) GetEnvoyLogLevel() string {
129129

130130
// GetEnvoyImage returns the envoy image
131131
func (c *client) GetEnvoyImage() string {
132-
image := c.getMeshConfig().Spec.Sidecar.EnvoyImage
133-
if image != "" {
134-
return image
135-
}
136-
return constants.DefaultEnvoyImage
132+
return c.getMeshConfig().Spec.Sidecar.EnvoyImage
137133
}
138134

139135
// GetEnvoyWindowsImage returns the envoy windows image
140136
func (c *client) GetEnvoyWindowsImage() string {
141-
image := c.getMeshConfig().Spec.Sidecar.EnvoyWindowsImage
142-
if image != "" {
143-
return image
144-
}
145-
return constants.DefaultEnvoyWindowsImage
137+
return c.getMeshConfig().Spec.Sidecar.EnvoyWindowsImage
146138
}
147139

148140
// GetInitContainerImage returns the init container image
149141
func (c *client) GetInitContainerImage() string {
150-
initImage := c.getMeshConfig().Spec.Sidecar.InitContainerImage
151-
if initImage != "" {
152-
return initImage
153-
}
154-
return constants.DefaultInitContainerImage
142+
return c.getMeshConfig().Spec.Sidecar.InitContainerImage
155143
}
156144

157145
// GetServiceCertValidityPeriod returns the validity duration for service certificates, and a default in case of invalid duration

‎pkg/configurator/methods_test.go‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ func TestCreateUpdateConfig(t *testing.T) {
233233
name: "GetEnvoyImage",
234234
initialMeshConfigData: &v1alpha1.MeshConfigSpec{},
235235
checkCreate: func(assert *tassert.Assertions, cfg Configurator) {
236-
assert.Equal("envoyproxy/envoy-alpine@sha256:6502a637c6c5fba4d03d0672d878d12da4bcc7a0d0fb3f1d506982dde0039abd", cfg.GetEnvoyImage())
236+
assert.Equal("", cfg.GetEnvoyImage())
237237
},
238238
updatedMeshConfigData: &v1alpha1.MeshConfigSpec{
239239
Sidecar: v1alpha1.SidecarSpec{
@@ -244,11 +244,26 @@ func TestCreateUpdateConfig(t *testing.T) {
244244
assert.Equal("envoyproxy/envoy-alpine:v1.17.1", cfg.GetEnvoyImage())
245245
},
246246
},
247+
{
248+
name: "GetEnvoyWindowsImage",
249+
initialMeshConfigData: &v1alpha1.MeshConfigSpec{},
250+
checkCreate: func(assert *tassert.Assertions, cfg Configurator) {
251+
assert.Equal("", cfg.GetEnvoyWindowsImage())
252+
},
253+
updatedMeshConfigData: &v1alpha1.MeshConfigSpec{
254+
Sidecar: v1alpha1.SidecarSpec{
255+
EnvoyImage: "envoyproxy/envoy-windows:v1.17.1",
256+
},
257+
},
258+
checkUpdate: func(assert *tassert.Assertions, cfg Configurator) {
259+
assert.Equal("envoyproxy/envoy-windows:v1.17.1", cfg.GetEnvoyImage())
260+
},
261+
},
247262
{
248263
name: "GetInitContainerImage",
249264
initialMeshConfigData: &v1alpha1.MeshConfigSpec{},
250265
checkCreate: func(assert *tassert.Assertions, cfg Configurator) {
251-
assert.Equal("openservicemesh/init:v0.9.2", cfg.GetInitContainerImage())
266+
assert.Equal("", cfg.GetInitContainerImage())
252267
},
253268
updatedMeshConfigData: &v1alpha1.MeshConfigSpec{
254269
Sidecar: v1alpha1.SidecarSpec{

‎pkg/constants/constants.go‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -58,17 +58,6 @@ const (
5858
// DefaultOSMLogLevel is the default OSM log level if none is specified
5959
DefaultOSMLogLevel = "info"
6060

61-
// DefaultEnvoyImage is the default envoy proxy sidecar image if not defined in the osm MeshConfig (v1.19.1)
62-
DefaultEnvoyImage = "envoyproxy/envoy-alpine@sha256:6502a637c6c5fba4d03d0672d878d12da4bcc7a0d0fb3f1d506982dde0039abd"
63-
64-
// DefaultEnvoyWindowsImage is the default envoy proxy windows sidecar image if not defined in the osm MeshConfig (v1.19.1)
65-
// TODO(#3864): This should be updated to the nanoserver based image when it becomes available
66-
// See https://github.com/envoyproxy/envoy/issues/16759
67-
DefaultEnvoyWindowsImage = "envoyproxy/envoy-windows@sha256:c904fda95891ebbccb9b1f24c1a9482c8d01cbca215dd081fc8c8db36db85f85"
68-
69-
// DefaultInitContainerImage is the default init container image if not defined in the osm MeshConfig
70-
DefaultInitContainerImage = "openservicemesh/init:v0.9.2"
71-
7261
// EnvoyPrometheusInboundListenerPort is Envoy's inbound listener port number for prometheus
7362
EnvoyPrometheusInboundListenerPort = 15010
7463

‎pkg/injector/patch.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
mapset "github.com/deckarep/golang-set"
1111
"github.com/google/uuid"
12+
"github.com/pkg/errors"
1213
"gomodules.xyz/jsonpatch/v2"
1314
admissionv1 "k8s.io/api/admission/v1"
1415
corev1 "k8s.io/api/core/v1"
@@ -60,6 +61,9 @@ func (wh *mutatingWebhook) createPatch(pod *corev1.Pod, req *admissionv1.Admissi
6061
// As a result we assume that the HNS redirection policies are already programmed via a CNI plugin.
6162
// Skip adding the init container and only patch the pod spec with sidecar container.
6263
podOS := pod.Spec.NodeSelector["kubernetes.io/os"]
64+
if err := wh.verifyPrerequisites(podOS); err != nil {
65+
return nil, err
66+
}
6367
if !strings.EqualFold(podOS, constants.OSWindows) {
6468
// Build outbound port exclusion list
6569
podOutboundPortExclusionList, _ := wh.getPortExclusionListForPod(pod, namespace, outboundPortExclusionListAnnotation)
@@ -105,6 +109,27 @@ func (wh *mutatingWebhook) createPatch(pod *corev1.Pod, req *admissionv1.Admissi
105109
return json.Marshal(makePatches(req, pod))
106110
}
107111

112+
// verifyPrerequisites verifies if the prerequisites to patch the request are met by returning an error if unmet
113+
func (wh *mutatingWebhook) verifyPrerequisites(podOS string) error {
114+
isWindows := strings.EqualFold(podOS, constants.OSWindows)
115+
116+
// Verify that the required images are configured
117+
if image := wh.configurator.GetEnvoyImage(); !isWindows && image == "" {
118+
// Linux pods require Envoy Linux image
119+
return errors.New("MeshConfig sidecar.envoyImage not set")
120+
}
121+
if image := wh.configurator.GetEnvoyWindowsImage(); isWindows && image == "" {
122+
// Windows pods require Envoy Windows image
123+
return errors.New("MeshConfig sidecar.envoyWindowsImage not set")
124+
}
125+
if image := wh.configurator.GetInitContainerImage(); !isWindows && image == "" {
126+
// Linux pods require init container image
127+
return errors.New("MeshConfig sidecar.initContainerImage not set")
128+
}
129+
130+
return nil
131+
}
132+
108133
func makePatches(req *admissionv1.AdmissionRequest, pod *corev1.Pod) []jsonpatch.JsonPatchOperation {
109134
original := req.Object.Raw
110135
current, err := json.Marshal(pod)

‎pkg/injector/patch_test.go‎

Lines changed: 70 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,11 @@ func TestCreatePatch(t *testing.T) {
140140
nonInjectNamespaces: mapset.NewSet(),
141141
}
142142

143-
mockConfigurator.EXPECT().GetEnvoyWindowsImage().Return("").AnyTimes()
144-
mockConfigurator.EXPECT().GetEnvoyImage().Return("").AnyTimes()
143+
mockConfigurator.EXPECT().GetEnvoyWindowsImage().Return("envoy-linux-image").AnyTimes()
144+
mockConfigurator.EXPECT().GetEnvoyImage().Return("envoy-windows-image").AnyTimes()
145+
mockConfigurator.EXPECT().GetInitContainerImage().Return("init-container-image").AnyTimes()
145146

146147
mockConfigurator.EXPECT().GetEnvoyLogLevel().Return("").Times(1)
147-
mockConfigurator.EXPECT().GetInitContainerImage().Return("").Times(1)
148148
mockConfigurator.EXPECT().IsPrivilegedInitContainer().Return(false).Times(1)
149149
mockConfigurator.EXPECT().GetOutboundIPRangeExclusionList().Return(nil).Times(1)
150150
mockConfigurator.EXPECT().GetOutboundPortExclusionList().Return(nil).Times(1)
@@ -264,3 +264,70 @@ func TestMergePortExclusionLists(t *testing.T) {
264264
})
265265
}
266266
}
267+
268+
func TestVerifyPrerequisites(t *testing.T) {
269+
testCases := []struct {
270+
name string
271+
podOS string
272+
linuxImage string
273+
windowsImage string
274+
initImage string
275+
expectErr bool
276+
}{
277+
{
278+
name: "prereqs met for linux pod",
279+
linuxImage: "envoy",
280+
initImage: "init",
281+
expectErr: false,
282+
},
283+
{
284+
name: "prereqs not met for linux pod when init container image is missing",
285+
linuxImage: "envoy",
286+
expectErr: true,
287+
},
288+
{
289+
name: "prereqs not met for linux pod when envoy container image is missing",
290+
initImage: "init",
291+
expectErr: true,
292+
},
293+
{
294+
name: "prereqs met for windows pod",
295+
podOS: "windows",
296+
windowsImage: "windows",
297+
initImage: "init",
298+
expectErr: false,
299+
},
300+
{
301+
name: "prereqs met for windows pod when init container image is missing",
302+
podOS: "windows",
303+
windowsImage: "envoy",
304+
expectErr: false,
305+
},
306+
{
307+
name: "prereqs not met for windows pod when envoy container image is missing",
308+
initImage: "init",
309+
expectErr: true,
310+
},
311+
}
312+
313+
for _, tc := range testCases {
314+
t.Run(tc.name, func(t *testing.T) {
315+
mockCtrl := gomock.NewController(t)
316+
defer mockCtrl.Finish()
317+
318+
assert := tassert.New(t)
319+
mockCfg := configurator.NewMockConfigurator(mockCtrl)
320+
321+
wh := &mutatingWebhook{
322+
configurator: mockCfg,
323+
}
324+
325+
mockCfg.EXPECT().GetEnvoyImage().Return(tc.linuxImage).AnyTimes()
326+
mockCfg.EXPECT().GetEnvoyWindowsImage().Return(tc.windowsImage).AnyTimes()
327+
mockCfg.EXPECT().GetInitContainerImage().Return(tc.initImage).AnyTimes()
328+
329+
err := wh.verifyPrerequisites(tc.podOS)
330+
assert.Equal(tc.expectErr, err != nil)
331+
})
332+
}
333+
}

‎pkg/injector/webhook_test.go‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -956,8 +956,16 @@ func TestPodCreationHandler(t *testing.T) {
956956
}
957957

958958
func TestWebhookMutate(t *testing.T) {
959+
mockCtrl := gomock.NewController(t)
960+
mockConfigurator := configurator.NewMockConfigurator(mockCtrl)
961+
mockConfigurator.EXPECT().GetEnvoyImage().Return("envoy-linux-image").AnyTimes()
962+
mockConfigurator.EXPECT().GetEnvoyWindowsImage().Return("envoy-windows-image").AnyTimes()
963+
mockConfigurator.EXPECT().GetInitContainerImage().Return("init-container-image").AnyTimes()
964+
959965
t.Run("invalid JSON", func(t *testing.T) {
960-
var wh *mutatingWebhook
966+
wh := &mutatingWebhook{
967+
configurator: mockConfigurator,
968+
}
961969
req := &admissionv1.AdmissionRequest{
962970
Object: runtime.RawExtension{Raw: []byte("{")},
963971
}
@@ -978,6 +986,7 @@ func TestWebhookMutate(t *testing.T) {
978986
wh := &mutatingWebhook{
979987
nonInjectNamespaces: mapset.NewSet(),
980988
kubeController: kubeController,
989+
configurator: mockConfigurator,
981990
}
982991

983992
req := &admissionv1.AdmissionRequest{
@@ -1010,8 +1019,9 @@ func TestWebhookMutate(t *testing.T) {
10101019
cfg.EXPECT().GetInboundPortExclusionList()
10111020
cfg.EXPECT().GetOutboundIPRangeExclusionList()
10121021
cfg.EXPECT().IsPrivilegedInitContainer()
1013-
cfg.EXPECT().GetInitContainerImage()
1014-
cfg.EXPECT().GetEnvoyImage()
1022+
cfg.EXPECT().GetInitContainerImage().Return("init-container-image").AnyTimes()
1023+
cfg.EXPECT().GetEnvoyImage().Return("envoy-linux-image").AnyTimes()
1024+
cfg.EXPECT().GetEnvoyWindowsImage().Return("envoy-windows-image").AnyTimes()
10151025
cfg.EXPECT().GetProxyResources()
10161026
cfg.EXPECT().GetEnvoyLogLevel()
10171027

@@ -1057,8 +1067,9 @@ func TestWebhookMutate(t *testing.T) {
10571067
cfg.EXPECT().GetInboundPortExclusionList()
10581068
cfg.EXPECT().GetOutboundIPRangeExclusionList()
10591069
cfg.EXPECT().IsPrivilegedInitContainer()
1060-
cfg.EXPECT().GetInitContainerImage()
1061-
cfg.EXPECT().GetEnvoyImage()
1070+
cfg.EXPECT().GetInitContainerImage().Return("init-container-image").AnyTimes()
1071+
cfg.EXPECT().GetEnvoyImage().Return("envoy-linux-image").AnyTimes()
1072+
cfg.EXPECT().GetEnvoyWindowsImage().Return("envoy-windows-image").AnyTimes()
10621073
cfg.EXPECT().GetProxyResources()
10631074
cfg.EXPECT().GetEnvoyLogLevel()
10641075

0 commit comments

Comments
 (0)