porch CLI: error instead of panic when the upstreamLock is not parsea…

…ble (#3938)
kptdev · May 1, 2023 · f6ce5c9 · f6ce5c9
1 parent 239a679
commit f6ce5c9
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 5 deletions.
diff --git a/commands/alpha/rpkg/update/discover.go b/commands/alpha/rpkg/update/discover.go
@@ -64,7 +64,10 @@ func (r *runner) discoverUpdates(cmd *cobra.Command, args []string) error {
 func (r *runner) findUpstreamUpdates(prs []porchapi.PackageRevision, repositories *configapi.RepositoryList, w io.Writer) error {
 	var upstreamUpdates [][]string
 	for _, pr := range prs {
-		availableUpdates, upstreamName, _ := r.availableUpdates(pr.Status.UpstreamLock, repositories)
+		availableUpdates, upstreamName, _, err := r.availableUpdates(pr.Status.UpstreamLock, repositories)
+		if err != nil {
+			return fmt.Errorf("could not parse upstreamLock in Kptfile of package %q: %s", pr.Name, err.Error())
+		}
 		if len(availableUpdates) == 0 {
 			upstreamUpdates = append(upstreamUpdates, []string{pr.Name, upstreamName, "No update available"})
 		} else {
@@ -84,7 +87,10 @@ func (r *runner) findDownstreamUpdates(prs []porchapi.PackageRevision, repositor
 	downstreamUpdatesMap := make(map[string][]porchapi.PackageRevision)
 
 	for _, pr := range prs {
-		availableUpdates, _, draftName := r.availableUpdates(pr.Status.UpstreamLock, repositories)
+		availableUpdates, _, draftName, err := r.availableUpdates(pr.Status.UpstreamLock, repositories)
+		if err != nil {
+			return fmt.Errorf("could not parse upstreamLock in Kptfile of package %q: %s", pr.Name, err.Error())
+		}
 		for _, update := range availableUpdates {
 			key := fmt.Sprintf("%s:%s:%s", update.Name, update.Spec.Revision, draftName)
 			downstreamUpdatesMap[key] = append(downstreamUpdatesMap[key], pr)
@@ -93,18 +99,22 @@ func (r *runner) findDownstreamUpdates(prs []porchapi.PackageRevision, repositor
 	return printDownstreamUpdates(downstreamUpdatesMap, args, w)
 }
 
-func (r *runner) availableUpdates(upstreamLock *porchapi.UpstreamLock, repositories *configapi.RepositoryList) ([]porchapi.PackageRevision, string, string) {
+func (r *runner) availableUpdates(upstreamLock *porchapi.UpstreamLock, repositories *configapi.RepositoryList) ([]porchapi.PackageRevision, string, string, error) {
 	var availableUpdates []porchapi.PackageRevision
 	var upstream string
 
 	if upstreamLock == nil || upstreamLock.Git == nil {
-		return nil, "", ""
+		return nil, "", "", nil
 	}
 	var currentUpstreamRevision string
 	var draftName string
 
 	// separate the revision number from the package name
 	lastIndex := strings.LastIndex(upstreamLock.Git.Ref, "/")
+	if lastIndex < 0 {
+		// "/" not found - upstreamLock.Git.Ref is not in the expected format
+		return nil, "", "", fmt.Errorf("malformed upstreamLock.Git.Ref %q", upstreamLock.Git.Ref)
+	}
 
 	if strings.HasPrefix(upstreamLock.Git.Ref, "drafts") {
 		// The upstream is not a published package, so doesn't have a revision number.
@@ -149,7 +159,7 @@ func (r *runner) availableUpdates(upstreamLock *porchapi.UpstreamLock, repositor
 		}
 	}
 
-	return availableUpdates, upstream, draftName
+	return availableUpdates, upstream, draftName, nil
 }
 
 // fetches all registered repositories

diff --git a/e2e/testdata/porch/rpkg-update/config.yaml b/e2e/testdata/porch/rpkg-update/config.yaml
@@ -111,3 +111,56 @@ commands:
       git-3f036055f7ba68706372cbe0c4b14d553794f7c4                         No update available
       git-7fcdd499f0790ac3bd8f805e3e5e00825641eb60                         No update available
       git-7ab0219ace10c1081a8b40a6b97d5da58bdb62e0   git                   No update available
+  - args:
+      - alpha
+      - rpkg
+      - push
+      - --namespace=rpkg-update
+      - git-7ab0219ace10c1081a8b40a6b97d5da58bdb62e0
+    stdin: |
+      apiVersion: config.kubernetes.io/v1
+      kind: ResourceList
+      items:
+      - apiVersion: kpt.dev/v1
+        kind: Kptfile
+        metadata:
+          name: basens-edit-clone
+          annotations:
+            config.kubernetes.io/index: '0'
+            config.kubernetes.io/local-config: "true"
+            config.kubernetes.io/path: Kptfile
+            internal.config.kubernetes.io/index: '0'
+            internal.config.kubernetes.io/path: 'Kptfile'
+        upstream:
+          type: git
+          git:
+            repo: http://git-server.test-git-namespace.svc.cluster.local:8080/rpkg-update
+            directory: base-ns
+            ref: basens/v1
+        upstreamLock:
+          type: git
+          git:
+            repo: http://git-server.test-git-namespace.svc.cluster.local:8080/rpkg-update
+            directory: base-ns
+            ref: invalid
+            commit: 8f3ee9e0d5724fbd0fbdcf81bef0d47d6d29cc64
+      - apiVersion: v1
+        data:
+          name: example
+        kind: ConfigMap
+        metadata:
+          annotations:
+            config.kubernetes.io/index: "0"
+            config.kubernetes.io/local-config: "true"
+            config.kubernetes.io/path: package-context.yaml
+            internal.config.kubernetes.io/index: "0"
+            internal.config.kubernetes.io/path: package-context.yaml
+          name: kptfile.kpt.dev
+  - args:
+      - alpha
+      - rpkg
+      - update
+      - --namespace=rpkg-update
+      - --discover=upstream
+    stderr: "Error: could not parse upstreamLock in Kptfile of package \"git-7ab0219ace10c1081a8b40a6b97d5da58bdb62e0\": malformed upstreamLock.Git.Ref \"invalid\" \n"
+    exitCode: 1