Skip to content

Commit 0d04f79

Browse files
valaparthvivalaparthvi
authored
Fix GetDeployment function when Devfile SchemaVersion is less than 2.1.0 (#161)
* Fix GetDeployment function when Devfile SchemaVersion is less than 2.1.0 Signed-off-by: Parthvi Vala <[email protected]> * Add test coverage Signed-off-by: Parthvi Vala <[email protected]> * Remove error check for GetAttributes Signed-off-by: Parthvi Vala <[email protected]> Signed-off-by: Parthvi Vala <[email protected]>
1 parent 4d107dd commit 0d04f79

File tree

4 files changed

+116
-22
lines changed

4 files changed

+116
-22
lines changed

‎pkg/devfile/generator/generators.go‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ package generator
1818
import (
1919
"fmt"
2020

21+
"github.com/devfile/api/v2/pkg/attributes"
22+
"github.com/devfile/library/v2/pkg/devfile/parser/data"
23+
2124
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
2225
"github.com/devfile/library/v2/pkg/devfile/parser"
2326
"github.com/devfile/library/v2/pkg/devfile/parser/data/v2/common"
@@ -185,10 +188,12 @@ func GetDeployment(devfileObj parser.DevfileObj, deployParams DeploymentParams)
185188
Containers: deployParams.Containers,
186189
Volumes: deployParams.Volumes,
187190
}
188-
189-
globalAttributes, err := devfileObj.Data.GetAttributes()
190-
if err != nil {
191-
return nil, err
191+
var globalAttributes attributes.Attributes
192+
// attributes is not supported in versions less than 2.1.0, so we skip it
193+
if devfileObj.Data.GetSchemaVersion() > string(data.APISchemaVersion200) {
194+
// the only time GetAttributes will return an error is if DevfileSchemaVersion is 2.0.0, a case we've already covered;
195+
// so we'll skip checking for error here
196+
globalAttributes, _ = devfileObj.Data.GetAttributes()
192197
}
193198
components, err := devfileObj.Data.GetDevfileContainerComponents(common.DevfileOptions{})
194199
if err != nil {

‎pkg/devfile/generator/generators_test.go‎

Lines changed: 87 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,12 @@ package generator
1717

1818
import (
1919
"fmt"
20-
apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2120
"reflect"
2221
"strings"
2322
"testing"
2423

24+
apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
25+
2526
"github.com/stretchr/testify/assert"
2627
appsv1 "k8s.io/api/apps/v1"
2728
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -1081,6 +1082,7 @@ func TestGetDeployment(t *testing.T) {
10811082
expected *appsv1.Deployment
10821083
attributes attributes.Attributes
10831084
wantErr bool
1085+
devObj func(ctrl *gomock.Controller, containerComponents []v1.Component) parser.DevfileObj
10841086
}{
10851087
{
10861088
// Currently dedicatedPod can only filter out annotations
@@ -1248,26 +1250,97 @@ func TestGetDeployment(t *testing.T) {
12481250
expected: nil,
12491251
wantErr: trueBool,
12501252
},
1253+
{
1254+
name: "skip getting global attributes for SchemaVersion less than 2.1.0",
1255+
containerComponents: []v1.Component{
1256+
testingutil.GenerateDummyContainerComponent("container1", nil, []v1.Endpoint{
1257+
{
1258+
Name: "http-8080",
1259+
TargetPort: 8080,
1260+
},
1261+
}, nil, v1.Annotation{
1262+
Deployment: map[string]string{
1263+
"key1": "value1",
1264+
},
1265+
}, nil),
1266+
testingutil.GenerateDummyContainerComponent("container2", nil, nil, nil, v1.Annotation{
1267+
Deployment: map[string]string{
1268+
"key2": "value2",
1269+
},
1270+
}, nil),
1271+
},
1272+
deploymentParams: DeploymentParams{
1273+
ObjectMeta: metav1.ObjectMeta{
1274+
Annotations: map[string]string{
1275+
"preserved-key": "preserved-value",
1276+
},
1277+
},
1278+
Containers: containers,
1279+
},
1280+
expected: &appsv1.Deployment{
1281+
ObjectMeta: objectMeta,
1282+
Spec: appsv1.DeploymentSpec{
1283+
Strategy: appsv1.DeploymentStrategy{
1284+
Type: appsv1.RecreateDeploymentStrategyType,
1285+
},
1286+
Selector: &metav1.LabelSelector{
1287+
MatchLabels: nil,
1288+
},
1289+
Template: corev1.PodTemplateSpec{
1290+
ObjectMeta: objectMeta,
1291+
Spec: corev1.PodSpec{
1292+
Containers: containers,
1293+
},
1294+
},
1295+
},
1296+
},
1297+
attributes: nil,
1298+
wantErr: false,
1299+
devObj: func(ctrl *gomock.Controller, containerComponents []v1.Component) parser.DevfileObj {
1300+
mockDevfileData := data.NewMockDevfileData(ctrl)
1301+
1302+
options := common.DevfileOptions{
1303+
ComponentOptions: common.ComponentOptions{
1304+
ComponentType: v1.ContainerComponentType,
1305+
},
1306+
}
1307+
// set up the mock data
1308+
mockDevfileData.EXPECT().GetSchemaVersion().Return("2.0.0")
1309+
mockDevfileData.EXPECT().GetDevfileContainerComponents(common.DevfileOptions{}).Return(containerComponents, nil).AnyTimes()
1310+
mockDevfileData.EXPECT().GetComponents(options).Return(containerComponents, nil).AnyTimes()
1311+
1312+
devObj := parser.DevfileObj{
1313+
Data: mockDevfileData,
1314+
}
1315+
return devObj
1316+
},
1317+
},
12511318
}
12521319

12531320
for _, tt := range tests {
12541321
t.Run(tt.name, func(t *testing.T) {
12551322
ctrl := gomock.NewController(t)
12561323
defer ctrl.Finish()
1257-
mockDevfileData := data.NewMockDevfileData(ctrl)
1258-
1259-
options := common.DevfileOptions{
1260-
ComponentOptions: common.ComponentOptions{
1261-
ComponentType: v1.ContainerComponentType,
1262-
},
1263-
}
1264-
// set up the mock data
1265-
mockDevfileData.EXPECT().GetAttributes().Return(tt.attributes, nil).AnyTimes()
1266-
mockDevfileData.EXPECT().GetDevfileContainerComponents(common.DevfileOptions{}).Return(tt.containerComponents, nil).AnyTimes()
1267-
mockDevfileData.EXPECT().GetComponents(options).Return(tt.containerComponents, nil).AnyTimes()
1324+
var devObj parser.DevfileObj
1325+
if tt.devObj != nil {
1326+
devObj = tt.devObj(ctrl, tt.containerComponents)
1327+
} else {
1328+
mockDevfileData := data.NewMockDevfileData(ctrl)
12681329

1269-
devObj := parser.DevfileObj{
1270-
Data: mockDevfileData,
1330+
options := common.DevfileOptions{
1331+
ComponentOptions: common.ComponentOptions{
1332+
ComponentType: v1.ContainerComponentType,
1333+
},
1334+
}
1335+
// set up the mock data
1336+
mockDevfileData.EXPECT().GetSchemaVersion().Return("2.1.0")
1337+
mockDevfileData.EXPECT().GetAttributes().Return(tt.attributes, nil).AnyTimes()
1338+
mockDevfileData.EXPECT().GetDevfileContainerComponents(common.DevfileOptions{}).Return(tt.containerComponents, nil).AnyTimes()
1339+
mockDevfileData.EXPECT().GetComponents(options).Return(tt.containerComponents, nil).AnyTimes()
1340+
1341+
devObj = parser.DevfileObj{
1342+
Data: mockDevfileData,
1343+
}
12711344
}
12721345
deploy, err := GetDeployment(devObj, tt.deploymentParams)
12731346
// Checks for unexpected error cases

‎pkg/devfile/generator/utils.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,7 @@ func getPodTemplateSpec(globalAttributes attributes.Attributes, components []v1.
250250
Volumes: podTemplateSpecParams.Volumes,
251251
},
252252
}
253-
254-
if needsPodOverrides(globalAttributes, components) {
253+
if len(globalAttributes) != 0 && needsPodOverrides(globalAttributes, components) {
255254
patchedPodTemplateSpec, err := applyPodOverrides(globalAttributes, components, podTemplateSpec)
256255
if err != nil {
257256
return nil, err

‎pkg/devfile/generator/utils_test.go‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package generator
1717

1818
import (
1919
"fmt"
20+
2021
"github.com/hashicorp/go-multierror"
2122
"github.com/stretchr/testify/assert"
2223
"k8s.io/utils/pointer"
@@ -37,6 +38,7 @@ import (
3738
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
3839

3940
corev1 "k8s.io/api/core/v1"
41+
apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
4042
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
4143
"k8s.io/apimachinery/pkg/api/resource"
4244
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -640,15 +642,27 @@ func TestGetPodTemplateSpec(t *testing.T) {
640642
namespace string
641643
serviceAccount string
642644
labels map[string]string
645+
attributes attributes.Attributes
643646
}{
647+
{
648+
podName: "podSpecTest",
649+
namespace: "default",
650+
labels: map[string]string{
651+
"app": "app",
652+
"component": "frontend",
653+
},
654+
},
644655
{
645656
podName: "podSpecTest",
646657
namespace: "default",
647-
serviceAccount: "default",
658+
serviceAccount: "new-service-account",
648659
labels: map[string]string{
649660
"app": "app",
650661
"component": "frontend",
651662
},
663+
attributes: attributes.Attributes{
664+
PodOverridesAttribute: apiext.JSON{Raw: []byte("{\"spec\": {\"serviceAccountName\": \"new-service-account\"}}")},
665+
},
652666
},
653667
}
654668

@@ -663,7 +677,7 @@ func TestGetPodTemplateSpec(t *testing.T) {
663677
InitContainers: container,
664678
}
665679

666-
podTemplateSpec, err := getPodTemplateSpec(nil, nil, podTemplateSpecParams)
680+
podTemplateSpec, err := getPodTemplateSpec(tt.attributes, nil, podTemplateSpecParams)
667681
if err != nil {
668682
t.Errorf("TestGetPodTemplateSpec() error: %s", err.Error())
669683
}
@@ -674,6 +688,9 @@ func TestGetPodTemplateSpec(t *testing.T) {
674688
if podTemplateSpec.Namespace != tt.namespace {
675689
t.Errorf("TestGetPodTemplateSpec() error: expected namespace %s, actual %s", tt.namespace, podTemplateSpec.Namespace)
676690
}
691+
if tt.serviceAccount != "" && podTemplateSpec.Spec.ServiceAccountName != tt.serviceAccount {
692+
t.Errorf("TestGetPodTemplateSpec() error: expected serviceAccountName %s, actual %s", tt.serviceAccount, podTemplateSpec.Spec.ServiceAccountName)
693+
}
677694
if !hasVolumeWithName("vol1", podTemplateSpec.Spec.Volumes) {
678695
t.Errorf("TestGetPodTemplateSpec() error: volume with name: %s not found", "vol1")
679696
}

0 commit comments

Comments
 (0)