Skip to content

Commit 8aaac10

Browse files
committed
fixup! fixup! fixup! fixup! fixup! fixup! fixes
Signed-off-by: Oleksii Kurinnyi <[email protected]>
1 parent 1110865 commit 8aaac10

File tree

3 files changed

+49
-167
lines changed

3 files changed

+49
-167
lines changed

controllers/workspace/devworkspace_controller.go

Lines changed: 21 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -69,22 +69,6 @@ import (
6969
"sigs.k8s.io/controller-runtime/pkg/reconcile"
7070
)
7171

72-
// applyHomeInitDefaults applies default values for image and command fields
73-
// of the init-persistent-home container.
74-
func applyHomeInitDefaults(c corev1.Container, workspace *common.DevWorkspaceWithConfig) (corev1.Container, error) {
75-
if c.Image == "" {
76-
inferred := home.InferWorkspaceImage(&workspace.Spec.Template)
77-
if inferred == "" {
78-
return c, fmt.Errorf("unable to infer workspace image for %s; specify image explicitly", constants.HomeInitComponentName)
79-
}
80-
c.Image = inferred
81-
}
82-
if len(c.Command) == 0 {
83-
c.Command = []string{"/bin/sh", "-c"}
84-
}
85-
return c, nil
86-
}
87-
8872
// validateNoAdvancedFields validates that the init-persistent-home container
8973
// does not use advanced Kubernetes container fields that could make behavior unpredictable.
9074
func validateNoAdvancedFields(c corev1.Container) error {
@@ -127,23 +111,36 @@ func validateNoAdvancedFields(c corev1.Container) error {
127111
}
128112

129113
// validateHomeInitContainer validates all aspects of the init-persistent-home container.
114+
// It only validates fields that are present, as missing fields will be filled by
115+
// the strategic merge with the default init container.
130116
func validateHomeInitContainer(c corev1.Container) error {
131-
if err := validateImageReference(c.Image); err != nil {
132-
return fmt.Errorf("invalid image reference for %s: %w", constants.HomeInitComponentName, err)
117+
// Only validate if present
118+
if c.Image != "" {
119+
if err := validateImageReference(c.Image); err != nil {
120+
return fmt.Errorf("invalid image reference for %s: %w", constants.HomeInitComponentName, err)
121+
}
133122
}
134123

135-
if len(c.Command) != 2 || c.Command[0] != "/bin/sh" || c.Command[1] != "-c" {
136-
return fmt.Errorf("command must be exactly [/bin/sh, -c] for %s", constants.HomeInitComponentName)
124+
// Only validate if present
125+
if c.Command != nil {
126+
if len(c.Command) != 2 || c.Command[0] != "/bin/sh" || c.Command[1] != "-c" {
127+
return fmt.Errorf("command must be exactly [/bin/sh, -c] for %s", constants.HomeInitComponentName)
128+
}
137129
}
138130

139-
if len(c.Args) != 1 {
140-
return fmt.Errorf("args must contain exactly one script string for %s", constants.HomeInitComponentName)
131+
// Only validate if present
132+
if c.Args != nil {
133+
if len(c.Args) != 1 {
134+
return fmt.Errorf("args must contain exactly one script string for %s", constants.HomeInitComponentName)
135+
}
141136
}
142137

138+
// Always validate - should not be provided
143139
if len(c.VolumeMounts) > 0 {
144140
return fmt.Errorf("volumeMounts are not allowed for %s; persistent-home is auto-mounted at /home/user/", constants.HomeInitComponentName)
145141
}
146142

143+
// Always validate - should not be provided
147144
if err := validateNoAdvancedFields(c); err != nil {
148145
return err
149146
}
@@ -209,28 +206,6 @@ func ensureHomeInitContainerFields(c *corev1.Container) error {
209206
return nil
210207
}
211208

212-
// defaultAndValidateHomeInitContainer applies defaults and validation for a custom
213-
// DWOC-provided init container named init-persistent-home. It ensures a shell execution
214-
// model, a single script arg, injects the persistent-home mount at /home/user/, and
215-
// defaults image to the inferred workspace image if not provided.
216-
func defaultAndValidateHomeInitContainer(c corev1.Container, workspace *common.DevWorkspaceWithConfig) (corev1.Container, error) {
217-
var err error
218-
219-
if c, err = applyHomeInitDefaults(c, workspace); err != nil {
220-
return c, err
221-
}
222-
223-
if err = validateHomeInitContainer(c); err != nil {
224-
return c, err
225-
}
226-
227-
if err = ensureHomeInitContainerFields(&c); err != nil {
228-
return c, err
229-
}
230-
231-
return c, nil
232-
}
233-
234209
const (
235210
startingWorkspaceRequeueInterval = 5 * time.Second
236211
)
@@ -552,9 +527,8 @@ func (r *DevWorkspaceReconciler) Reconcile(ctx context.Context, req ctrl.Request
552527
reqLogger.Info("Skipping init-persistent-home container: DisableInitContainer is true")
553528
continue
554529
}
555-
// Apply defaults and validation for init-persistent-home
556-
container, err = defaultAndValidateHomeInitContainer(container, workspace)
557-
if err != nil {
530+
// Validate custom home init container
531+
if err := validateHomeInitContainer(container); err != nil {
558532
return r.failWorkspace(workspace, fmt.Sprintf("Invalid %s container: %s", constants.HomeInitComponentName, err), metrics.ReasonBadRequest, reqLogger, &reconcileStatus), nil
559533
}
560534
}

controllers/workspace/init_container_validation_test.go

Lines changed: 27 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -19,71 +19,34 @@ import (
1919
"strings"
2020
"testing"
2121

22-
dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
23-
"github.com/devfile/devworkspace-operator/apis/controller/v1alpha1"
24-
"github.com/devfile/devworkspace-operator/pkg/common"
2522
"github.com/devfile/devworkspace-operator/pkg/constants"
2623
"github.com/stretchr/testify/assert"
2724
corev1 "k8s.io/api/core/v1"
2825
"k8s.io/apimachinery/pkg/api/resource"
2926
)
3027

31-
func TestDefaultAndValidateHomeInitContainer(t *testing.T) {
32-
workspace := &common.DevWorkspaceWithConfig{
33-
DevWorkspace: &dw.DevWorkspace{
34-
Spec: dw.DevWorkspaceSpec{
35-
Template: dw.DevWorkspaceTemplateSpec{
36-
DevWorkspaceTemplateSpecContent: dw.DevWorkspaceTemplateSpecContent{
37-
Components: []dw.Component{
38-
{
39-
Name: "main-container",
40-
ComponentUnion: dw.ComponentUnion{
41-
Container: &dw.ContainerComponent{
42-
Container: dw.Container{
43-
Image: "test-image:latest",
44-
},
45-
},
46-
},
47-
},
48-
},
49-
},
50-
},
51-
},
52-
},
53-
Config: &v1alpha1.OperatorConfiguration{
54-
Workspace: &v1alpha1.WorkspaceConfig{},
55-
},
56-
}
57-
28+
func TestValidateInitContainer(t *testing.T) {
5829
tests := []struct {
5930
name string
6031
container corev1.Container
6132
expectError bool
6233
errorMsg string
63-
validate func(t *testing.T, result corev1.Container)
6434
}{
6535
{
66-
name: "Defaults image when empty",
36+
name: "Accepts container with only args (image and command will be filled by merge)",
6737
container: corev1.Container{
6838
Name: constants.HomeInitComponentName,
6939
Args: []string{"echo 'test'"},
7040
},
7141
expectError: false,
72-
validate: func(t *testing.T, result corev1.Container) {
73-
assert.Equal(t, "test-image:latest", result.Image)
74-
},
7542
},
7643
{
77-
name: "Defaults command when empty",
44+
name: "Accepts container with only image (command and args will be filled by merge)",
7845
container: corev1.Container{
7946
Name: constants.HomeInitComponentName,
8047
Image: "custom-image:latest",
81-
Args: []string{"echo 'test'"},
8248
},
8349
expectError: false,
84-
validate: func(t *testing.T, result corev1.Container) {
85-
assert.Equal(t, []string{"/bin/sh", "-c"}, result.Command)
86-
},
8750
},
8851
{
8952
name: "Accepts valid command",
@@ -100,7 +63,18 @@ func TestDefaultAndValidateHomeInitContainer(t *testing.T) {
10063
container: corev1.Container{
10164
Name: constants.HomeInitComponentName,
10265
Image: "custom-image:latest",
103-
Command: []string{"/bin/bash"},
66+
Command: []string{"/bin/sh"},
67+
Args: []string{"echo 'test'"},
68+
},
69+
expectError: true,
70+
errorMsg: "command must be exactly [/bin/sh, -c]",
71+
},
72+
{
73+
name: "Rejects empty command",
74+
container: corev1.Container{
75+
Name: constants.HomeInitComponentName,
76+
Image: "custom-image:latest",
77+
Command: []string{},
10478
Args: []string{"echo 'test'"},
10579
},
10680
expectError: true,
@@ -131,7 +105,6 @@ func TestDefaultAndValidateHomeInitContainer(t *testing.T) {
131105
container: corev1.Container{
132106
Name: constants.HomeInitComponentName,
133107
Image: "custom-image:latest",
134-
Args: []string{"echo 'test'"},
135108
VolumeMounts: []corev1.VolumeMount{
136109
{
137110
Name: "custom-volume",
@@ -142,44 +115,29 @@ func TestDefaultAndValidateHomeInitContainer(t *testing.T) {
142115
expectError: true,
143116
errorMsg: "volumeMounts are not allowed for init-persistent-home",
144117
},
145-
{
146-
name: "Injects persistent-home volumeMount",
147-
container: corev1.Container{
148-
Name: constants.HomeInitComponentName,
149-
Image: "custom-image:latest",
150-
Args: []string{"echo 'test'"},
151-
},
152-
expectError: false,
153-
validate: func(t *testing.T, result corev1.Container) {
154-
assert.Len(t, result.VolumeMounts, 1)
155-
assert.Equal(t, constants.HomeVolumeName, result.VolumeMounts[0].Name)
156-
assert.Equal(t, constants.HomeUserDirectory, result.VolumeMounts[0].MountPath)
157-
},
158-
},
159118
{
160119
name: "Allows env variables",
161120
container: corev1.Container{
162121
Name: constants.HomeInitComponentName,
163122
Image: "custom-image:latest",
164-
Args: []string{"echo 'test'"},
165123
Env: []corev1.EnvVar{
166-
{Name: "TEST_VAR", Value: "test-value"},
124+
{
125+
Name: "TEST_VAR",
126+
Value: "test-var",
127+
},
167128
},
168129
},
169130
expectError: false,
170-
validate: func(t *testing.T, result corev1.Container) {
171-
assert.Len(t, result.Env, 1)
172-
assert.Equal(t, "TEST_VAR", result.Env[0].Name)
173-
},
174131
},
175132
{
176133
name: "Rejects ports",
177134
container: corev1.Container{
178135
Name: constants.HomeInitComponentName,
179136
Image: "custom-image:latest",
180-
Args: []string{"echo 'test'"},
181137
Ports: []corev1.ContainerPort{
182-
{ContainerPort: 8080},
138+
{
139+
ContainerPort: 8080,
140+
},
183141
},
184142
},
185143
expectError: true,
@@ -190,10 +148,11 @@ func TestDefaultAndValidateHomeInitContainer(t *testing.T) {
190148
container: corev1.Container{
191149
Name: constants.HomeInitComponentName,
192150
Image: "custom-image:latest",
193-
Args: []string{"echo 'test'"},
194151
LivenessProbe: &corev1.Probe{
195152
ProbeHandler: corev1.ProbeHandler{
196-
HTTPGet: &corev1.HTTPGetAction{Path: "/health"},
153+
HTTPGet: &corev1.HTTPGetAction{
154+
Path: "/health",
155+
},
197156
},
198157
},
199158
},
@@ -205,7 +164,6 @@ func TestDefaultAndValidateHomeInitContainer(t *testing.T) {
205164
container: corev1.Container{
206165
Name: constants.HomeInitComponentName,
207166
Image: "custom-image:latest",
208-
Args: []string{"echo 'test'"},
209167
SecurityContext: &corev1.SecurityContext{
210168
RunAsUser: new(int64),
211169
},
@@ -218,7 +176,6 @@ func TestDefaultAndValidateHomeInitContainer(t *testing.T) {
218176
container: corev1.Container{
219177
Name: constants.HomeInitComponentName,
220178
Image: "custom-image:latest",
221-
Args: []string{"echo 'test'"},
222179
Resources: corev1.ResourceRequirements{
223180
Limits: corev1.ResourceList{
224181
corev1.ResourceMemory: resource.MustParse("128Mi"),
@@ -233,7 +190,6 @@ func TestDefaultAndValidateHomeInitContainer(t *testing.T) {
233190
container: corev1.Container{
234191
Name: constants.HomeInitComponentName,
235192
Image: "custom-image:latest",
236-
Args: []string{"echo 'test'"},
237193
WorkingDir: "/tmp",
238194
},
239195
expectError: true,
@@ -243,8 +199,7 @@ func TestDefaultAndValidateHomeInitContainer(t *testing.T) {
243199
name: "Rejects image with whitespace",
244200
container: corev1.Container{
245201
Name: constants.HomeInitComponentName,
246-
Image: "nginx\nmalicious",
247-
Args: []string{"echo 'test'"},
202+
Image: "nginx\tmalicious",
248203
},
249204
expectError: true,
250205
errorMsg: "invalid image reference",
@@ -253,7 +208,7 @@ func TestDefaultAndValidateHomeInitContainer(t *testing.T) {
253208

254209
for _, tt := range tests {
255210
t.Run(tt.name, func(t *testing.T) {
256-
result, err := defaultAndValidateHomeInitContainer(tt.container, workspace)
211+
err := validateHomeInitContainer(tt.container)
257212

258213
if tt.expectError {
259214
assert.Error(t, err)
@@ -262,47 +217,11 @@ func TestDefaultAndValidateHomeInitContainer(t *testing.T) {
262217
}
263218
} else {
264219
assert.NoError(t, err)
265-
if tt.validate != nil {
266-
tt.validate(t, result)
267-
}
268220
}
269221
})
270222
}
271223
}
272224

273-
func TestDefaultAndValidateHomeInitContainer_NoWorkspaceImage(t *testing.T) {
274-
workspaceNoImage := &common.DevWorkspaceWithConfig{
275-
DevWorkspace: &dw.DevWorkspace{
276-
Spec: dw.DevWorkspaceSpec{
277-
Template: dw.DevWorkspaceTemplateSpec{
278-
DevWorkspaceTemplateSpecContent: dw.DevWorkspaceTemplateSpecContent{
279-
Components: []dw.Component{
280-
{
281-
Name: "volume-component",
282-
ComponentUnion: dw.ComponentUnion{
283-
Volume: &dw.VolumeComponent{},
284-
},
285-
},
286-
},
287-
},
288-
},
289-
},
290-
},
291-
Config: &v1alpha1.OperatorConfiguration{
292-
Workspace: &v1alpha1.WorkspaceConfig{},
293-
},
294-
}
295-
296-
container := corev1.Container{
297-
Name: constants.HomeInitComponentName,
298-
Args: []string{"echo 'test'"},
299-
}
300-
301-
_, err := defaultAndValidateHomeInitContainer(container, workspaceNoImage)
302-
assert.Error(t, err)
303-
assert.Contains(t, err.Error(), "unable to infer workspace image")
304-
}
305-
306225
func TestValidateImageReference(t *testing.T) {
307226
tests := []struct {
308227
name string

pkg/library/home/persistentHome.go

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -62,19 +62,8 @@ func AddPersistentHomeVolume(workspace *common.DevWorkspaceWithConfig) (*v1alpha
6262
Path: constants.HomeUserDirectory,
6363
}
6464

65-
// Determine if a custom home init container is configured via DWOC
66-
hasCustomHomeInit := false
67-
if workspace.Config != nil && workspace.Config.Workspace != nil {
68-
for _, c := range workspace.Config.Workspace.InitContainers {
69-
if c.Name == constants.HomeInitComponentName {
70-
hasCustomHomeInit = true
71-
break
72-
}
73-
}
74-
}
75-
7665
// Add default init container only if not disabled and no custom init is configured
77-
if (workspace.Config.Workspace.PersistUserHome.DisableInitContainer == nil || !*workspace.Config.Workspace.PersistUserHome.DisableInitContainer) && !hasCustomHomeInit {
66+
if workspace.Config.Workspace.PersistUserHome.DisableInitContainer == nil || !*workspace.Config.Workspace.PersistUserHome.DisableInitContainer {
7867
err := addInitContainer(dwTemplateSpecCopy)
7968
if err != nil {
8069
return nil, fmt.Errorf("failed to add init container for home persistence setup: %w", err)

0 commit comments

Comments
 (0)