Fix API Server panic when volume component has no ephemeral field set (#7080)

* Add unit test highlighting the issue

* Safely dereference volume component 'ephemeral' field
This commit is contained in:
Armel Soro
2023-09-09 00:18:05 +02:00
committed by GitHub
parent bfffaa8879
commit 0deb3de931
2 changed files with 69 additions and 2 deletions

View File

@@ -8,9 +8,10 @@ import (
"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/api/v2/pkg/devfile"
"github.com/devfile/library/v2/pkg/devfile/parser/data/v2/common"
"k8s.io/utils/pointer"
. "github.com/redhat-developer/odo/pkg/apiserver-gen/go"
"github.com/redhat-developer/odo/pkg/libdevfile"
"k8s.io/utils/pointer"
)
const (
@@ -319,7 +320,7 @@ func (o *DevfileState) getVolumes() ([]Volume, error) {
for _, volume := range volumes {
result = append(result, Volume{
Name: volume.Name,
Ephemeral: *volume.Volume.Ephemeral,
Ephemeral: pointer.BoolDeref(volume.Volume.Ephemeral, false),
Size: volume.Volume.Size,
})
}

View File

@@ -3,8 +3,13 @@ package devstate
import (
"testing"
"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/library/v2/pkg/devfile/parser/data"
"github.com/google/go-cmp/cmp"
"k8s.io/utils/pointer"
. "github.com/redhat-developer/odo/pkg/apiserver-gen/go"
"github.com/redhat-developer/odo/pkg/testingutil"
)
func TestDevfileState_GetContent(t *testing.T) {
@@ -44,3 +49,64 @@ func TestDevfileState_GetContent(t *testing.T) {
})
}
}
func TestDevfileState_getVolumes(t *testing.T) {
var (
volWithNoEphemeral = testingutil.GetFakeVolumeComponent("vol-ephemeral-not-set", "1Gi")
volEphemeral = testingutil.GetFakeVolumeComponent("vol-ephemeral-true", "2Gi")
volEphemeralFalse = testingutil.GetFakeVolumeComponent("vol-ephemeral-false", "3Gi")
)
volWithNoEphemeral.Volume.Ephemeral = nil
volEphemeral.Volume.Ephemeral = pointer.Bool(true)
volEphemeralFalse.Volume.Ephemeral = pointer.Bool(false)
tests := []struct {
name string
state func() (DevfileState, error)
want []Volume
wantErr bool
}{
{
name: "should not panic if 'ephemeral' is not set on the Devfile volume component",
state: func() (DevfileState, error) {
devfileData, err := data.NewDevfileData(string(data.APISchemaVersion220))
if err != nil {
return DevfileState{}, err
}
err = devfileData.AddComponents([]v1alpha2.Component{
volEphemeral,
volWithNoEphemeral,
volEphemeralFalse,
})
if err != nil {
return DevfileState{}, err
}
s := NewDevfileState()
s.Devfile.Data = devfileData
return s, nil
},
want: []Volume{
{Name: "vol-ephemeral-true", Ephemeral: true, Size: "2Gi"},
{Name: "vol-ephemeral-not-set", Ephemeral: false, Size: "1Gi"},
{Name: "vol-ephemeral-false", Ephemeral: false, Size: "3Gi"},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
o, err := tt.state()
if err != nil {
t.Fatalf("DevfileState.getVolumes() error preparing Devfile state: %v", err)
return
}
got, err := o.getVolumes()
if (err != nil) != tt.wantErr {
t.Errorf("DevfileState.getVolumes() error = %v, wantErr %v", err, tt.wantErr)
return
}
if diff := cmp.Diff(tt.want, got); diff != "" {
t.Errorf("DevfileState.getVolumes() mismatch (-want +got):\n%s", diff)
}
})
}
}