Fix: nil pointer panic in getMatchingDefinitionRevision on malformed revision version (#7207)

* Fix: nil pointer panic in getMatchingDefinitionRevision on malformed revision version

getMatchingDefinitionRevision dereferenced the semver result (v.String()) before
checking semver.NewVersion's parse error. A DefinitionRevision whose name carries
the expected prefix but an unparseable version segment (e.g. configmap-component-v1.bad)
made semver.NewVersion return (nil, err), so v.String() panicked instead of returning
the error. Move the error check ahead of the map write so a bad version returns the
error as intended. Valid input is unaffected.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>

* test: cover empty version segment in getMatchingDefinitionRevision

Add a regression case for a DefinitionRevision whose version segment is
empty (name ending in the definition name plus a trailing hyphen, e.g.
configmap-component-). Such a name is filtered out by the revisionPrefix
HasPrefix check before it reaches semver.NewVersion, and semver.NewVersion
would return an error for an empty string anyway, so the same err != nil
guard that fixes the malformed-version panic already rejects it cleanly.
This locks in that behaviour and addresses the empty-segment edge case
raised in review.

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>

---------

Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
(cherry picked from commit 74c5724554)

pkg/oam/util/helper.go

@@ -351,10 +351,10 @@ func getMatchingDefinitionRevision(exactRevisionName, definitionName string, rev
 		if strings.HasPrefix(revision.Name, revisionPrefix) {
 			version := strings.Split(revision.Name, definitionName+"-")[1]
 			v, err := semver.NewVersion(version)
-			orignalVersions[v.String()] = version
 			if err != nil {
 				return "", err
 			}
+			orignalVersions[v.String()] = version
 			definitionVersions = append(definitionVersions, v)
 		}
 	}

pkg/oam/util/helper_test.go

@@ -992,6 +992,16 @@ func TestGetLatestDefinitionRevisionName(t *testing.T) {
 		defRevisionList.DeepCopyInto(list.(*v1beta1.DefinitionRevisionList))
 		return nil
 	}}
+	malformedListCli := test.MockClient{MockList: func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
+		defRevisionList := getMalformedComponentDefRevisionList()
+		defRevisionList.DeepCopyInto(list.(*v1beta1.DefinitionRevisionList))
+		return nil
+	}}
+	emptyVersionListCli := test.MockClient{MockList: func(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
+		defRevisionList := getEmptyVersionComponentDefRevisionList()
+		defRevisionList.DeepCopyInto(list.(*v1beta1.DefinitionRevisionList))
+		return nil
+	}}
 
 	testcases := []struct {
 		name                    string
@@ -1053,6 +1063,24 @@ func TestGetLatestDefinitionRevisionName(t *testing.T) {
 			client:                  &traitListCli,
 			err:                     fmt.Errorf("error finding definition revision for Name: scaler-trait, Type: Trait"),
 		},
+		{
+			name:                    "Malformed Component version segment returns error not panic",
+			inputRevisionName:       "configmap-component-v1",
+			definitionName:          "configmap-component",
+			definitionType:          "Component",
+			expectedDefRevisionName: "",
+			client:                  &malformedListCli,
+			err:                     fmt.Errorf("error finding definition revision for Name: configmap-component, Type: Component"),
+		},
+		{
+			name:                    "Empty Component version segment returns error not panic",
+			inputRevisionName:       "configmap-component-v1",
+			definitionName:          "configmap-component",
+			definitionType:          "Component",
+			expectedDefRevisionName: "",
+			client:                  &emptyVersionListCli,
+			err:                     fmt.Errorf("error finding definition revision for Name: configmap-component, Type: Component"),
+		},
 	}
 	ctx := context.Background()
 	for _, tc := range testcases {
@@ -1221,6 +1249,36 @@ func getComponentDefRevisionList() v1beta1.DefinitionRevisionList {
 	return compRevisionList
 }
 
+func getMalformedComponentDefRevisionList() v1beta1.DefinitionRevisionList {
+	compDefRevision := componentDefinitionRevision.DeepCopy()
+	compDefRevision.Name = "configmap-component-v1.bad"
+
+	return v1beta1.DefinitionRevisionList{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "DefinitionRevision",
+			APIVersion: "core.oam.dev/v1beta1",
+		},
+		Items: []v1beta1.DefinitionRevision{
+			*compDefRevision,
+		},
+	}
+}
+
+func getEmptyVersionComponentDefRevisionList() v1beta1.DefinitionRevisionList {
+	compDefRevision := componentDefinitionRevision.DeepCopy()
+	compDefRevision.Name = "configmap-component-"
+
+	return v1beta1.DefinitionRevisionList{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "DefinitionRevision",
+			APIVersion: "core.oam.dev/v1beta1",
+		},
+		Items: []v1beta1.DefinitionRevision{
+			*compDefRevision,
+		},
+	}
+}
+
 func getTraitDefRevisionList() v1beta1.DefinitionRevisionList {
 	traitDefRevision1 := traitDefinitionRevision.DeepCopy()
 	traitDefRevision1.Spec.TraitDefinition.Spec.Version = "1.2.0"