Repository: kptdev/kpt

Issue:
rpkg update panics out 
I am trying to update my downstream packages, but the `kpt alpha rpkg update --discover=upstream` panics out with below stack 

```
$ kpt version
1.0.0-beta.31
```

followed the command instructions as per output of below command

```
kpt alpha rpkg update --help
```


```
$ kpt alpha rpkg update --discover=upstream
panic: runtime error: slice bounds out of range [:-1]

goroutine 1 [running]:
github.com/GoogleContainerTools/kpt/commands/alpha/rpkg/update.(*runner).availableUpdates(0xc0006b2380, 0xc0014dad68, 0xc0022ca000)
/go/src/github.com/GoogleContainerTools/kpt/commands/alpha/rpkg/update/discover.go:120 +0xa69
github.com/GoogleContainerTools/kpt/commands/alpha/rpkg/update.(*runner).findUpstreamUpdates(0xc0006b2380?, {0xc0021f9980?, 0xa, 0x0?}, 0x0?, {0x21949c0, 0xc000012018})
/go/src/github.com/GoogleContainerTools/kpt/commands/alpha/rpkg/update/discover.go:67 +0x109
github.com/GoogleContainerTools/kpt/commands/alpha/rpkg/update.(*runner).discoverUpdates(0xc0006b2380, 0x0?, {0xc0023d6750?, 0x0, 0x1})
/go/src/github.com/GoogleContainerTools/kpt/commands/alpha/rpkg/update/discover.go:56 +0x172
github.com/GoogleContainerTools/kpt/commands/alpha/rpkg/update.(*runner).runE(0x0?, 0x0?, {0xc0023d6750?, 0x0?, 0x0?})
/go/src/github.com/GoogleContainerTools/kpt/commands/alpha/rpkg/update/command.go:123 +0x77
github.com/spf13/cobra.(*Command).execute(0xc0023a3200, {0xc0023d6730, 0x1, 0x1})
/root/go/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:916 +0x862
github.com/spf13/cobra.(*Command).ExecuteC(0xc0006bc000)
/root/go/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:1044 +0x3bd
github.com/spf13/cobra.(*Command).Execute(...)
/root/go/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:968
k8s.io/component-base/cli.run(0xc0006bc000)
/root/go/pkg/mod/k8s.io/component-base@v0.25.3/cli/run.go:146 +0x317
k8s.io/component-base/cli.RunNoErrOutput(...)
/root/go/pkg/mod/k8s.io/component-base@v0.25.3/cli/run.go:84
main.runMain()
/go/src/github.com/GoogleContainerTools/kpt/main.go:62 +0x45
main.main()
/go/src/github.com/GoogleContainerTools/kpt/main.go:46 +0x19
```


```
$ kpt alpha rpkg update --discover=downstream
panic: runtime error: slice bounds out of range [:-1]

goroutine 1 [running]:
github.com/GoogleContainerTools/kpt/commands/alpha/rpkg/update.(*runner).availableUpdates(0xc00059f110, 0xc00132f560, 0xc002068930)
/go/src/github.com/GoogleContainerTools/kpt/commands/alpha/rpkg/update/discover.go:120 +0xa69
github.com/GoogleContainerTools/kpt/commands/alpha/rpkg/update.(*runner).findDownstreamUpdates(0xc000130008?, {0xc002379980, 0xa, 0x0?}, 0x0?, {0xc0012582d0, 0x0, 0x1}, {0x21949c0, 0xc000130008})
/go/src/github.com/GoogleContainerTools/kpt/commands/alpha/rpkg/update/discover.go:87 +0x130
github.com/GoogleContainerTools/kpt/commands/alpha/rpkg/update.(*runner).discoverUpdates(0xc00059f110, 0x0?, {0xc0012582d0?, 0x0, 0x1})
/go/src/github.com/GoogleContainerTools/kpt/commands/alpha/rpkg/update/discover.go:58 +0x206
github.com/GoogleContainerTools/kpt/commands/alpha/rpkg/update.(*runner).runE(0x0?, 0x0?, {0xc0012582d0?, 0x0?, 0x0?})
/go/src/github.com/GoogleContainerTools/kpt/commands/alpha/rpkg/update/command.go:123 +0x77
github.com/spf13/cobra.(*Command).execute(0xc000ae8c00, {0xc0012581b0, 0x1, 0x1})
/root/go/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:916 +0x862
github.com/spf13/cobra.(*Command).ExecuteC(0xc000521500)
/root/go/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:1044 +0x3bd
github.com/spf13/cobra.(*Command).Execute(...)
/root/go/pkg/mod/github.com/spf13/cobra@v1.6.1/command.go:968
k8s.io/component-base/cli.run(0xc000521500)
/root/go/pkg/mod/k8s.io/component-base@v0.25.3/cli/run.go:146 +0x317
k8s.io/component-base/cli.RunNoErrOutput(...)
/root/go/pkg/mod/k8s.io/component-base@v0.25.3/cli/run.go:84
main.runMain()
/go/src/github.com/GoogleContainerTools/kpt/main.go:62 +0x45
main.main()
/go/src/github.com/GoogleContainerTools/kpt/main.go:46 +0x19
```

Hints from issue comments:
@natasha41575: Do you mind sharing what repos/packagerevisions you had registered when you ran into this issue, i.e. the output of `kpt alpha rpkg get` and `kpt alpha repo get`? If any of your packagerevisions are clones, could you also share how you created them?

While we should never panic, this issue is odd because it implies a malformed `UpstreamLock` in the Kptfile (or one of our assumptions about ref formats was incorrect) and I'd like to understand how that happened.@Cbkhare: Hi, 

below are the requested details 

```
$ kpt alpha repo get -A
NAME             TYPE   CONTENT   DEPLOYMENT   READY   ADDRESS
test-catalogue   git    Package   false        True    https://github.com/********/*********************.git

$ kpt alpha rpkg get -A
NAMESPACE   NAME                                                      PACKAGE                       WORKSPACENAME   REVISION   LATEST   LIFECYCLE   REPOSITORY
test        test-catalogue-233a4ca4388204556f5bceff99a9799d1a86ed1a   test-pk2                      v1              v1         true     Published   test-catalogue
test        test-catalogue-7a25f33ed16f31e2df4cc29da9c34565848d81c4   test-pkg                      v1              main       false    Published   test-catalogue
test        test-catalogue-93b612f846d238c0ee3cae8441f2e924d07eeca1   test-pkg                      v1              v1         true     Published   test-catalogue
test        test-catalogue-76dd1a835ac600377616d401ecb05323bf17f2a9   test-pkg-docker               v1              v1         true     Published   test-catalogue
test        test-catalogue-912d60e5385d7979399530e0c14ccde526622213   test-pkg-out/test-pkg-test    v1                         false    Proposed    test-catalogue
test        test-catalogue-bbb48d4284bd16b74cd089314906ed3b2ea7ba45   test-pkg-out/test-pkg-test1   v1                         false    Draft       test-catalogue
test        test-catalogue-88eafbcb6d7b73c5d934ba48ef4031334983e1e0   test-pkg-test1                v1                         false    Draft       test-catalogue
test        test-catalogue-2495313e2ff74c68b606b9ec2c16e04a0a7efc6a   test-pkg2                     v1              main       false    Published   test-catalogue
test        test-catalogue-170e5f5a6ed89619f1a0aee50623e6f98b27ee02   test-pkg2-u                   v1              main       false    Published   test-catalogue
test        test-catalogue-9e4212ac5bce692e4f2744c5ff32136cb078fa27   test-pkg2-u1                  v1                         false    Proposed    test-catalogue

```

Some of the above packages are clone. Below is the command I used to clone one of the package, with the output 


```
$ kpt alpha rpkg clone test-catalogue-7a25f33ed16f31e2df4cc29da9c34565848d81c4 test-pk2 --repository=test-catalogue --ref=v2 --namespace=test --directory=test-pkg
```

result was 

```kpt alpha rpkg get -A | grep test-pk2
NAMESPACE   NAME                                                      PACKAGE                       WORKSPACENAME   REVISION   LATEST   LIFECYCLE   REPOSITORY
test        test-catalogue-233a4ca4388204556f5bceff99a9799d1a86ed1a   test-pk2          v1                         false    Draft       test-catalogue
  
```

@natasha41575: Hmmm... I'm not able to reproduce the issue. Here are the steps I followed:

```
$ kpt alpha repo reg https://github.com/natasha41575/blueprints.git -ndefault --repo-basic-username=natasha41575 --repo-basic-password=*** -ntest

$ kpt alpha repo get -ntest
NAME         TYPE   CONTENT   DEPLOYMENT   READY   ADDRESS
blueprints   git    Package                True    https://github.com/natasha41575/blueprints.git

$  kpt alpha rpkg init test-pkg --workspace=v1 --repository=blueprints -ntest
blueprints-f09e7e39a212cd16f5abcbf8fec35099cbee266d created

$ kpt alpha rpkg clone blueprints-f09e7e39a212cd16f5abcbf8fec35099cbee266d -ntest --workspace=clone --repository=blueprints test-pkg-clone
blueprints-8c50f85abaa5d22110b7c826bcb77f8f33d39dbb created

$ kpt alpha rpkg clone blueprints-f09e7e39a212cd16f5abcbf8fec35099cbee266d test-pk2 --repository=blueprints --ref=v2 --namespace=test --directory=test-pkg
blueprints-07772b7fb702c1068c74e9345d820f758e446bb1 created

$ kpt alpha rpkg get -ntest
NAME                                                  PACKAGE          WORKSPACENAME   REVISION   LATEST   LIFECYCLE   REPOSITORY
blueprints-07772b7fb702c1068c74e9345d820f758e446bb1   test-pk2         v1                         false    Draft       blueprints
blueprints-f09e7e39a212cd16f5abcbf8fec35099cbee266d   test-pkg         v1                         false    Draft       blueprints
blueprints-8c50f85abaa5d22110b7c826bcb77f8f33d39dbb   test-pkg-clone   clone                      false    Draft       blueprints

$ kpt alpha rpkg update --discover=downstream
All downstream packages are up to date.

$ kpt alpha rpkg update --discover=upstream
PACKAGE REVISION                                      UPSTREAM REPOSITORY   UPSTREAM UPDATES
blueprints-07772b7fb702c1068c74e9345d820f758e446bb1   blueprints            No update available
blueprints-f09e7e39a212cd16f5abcbf8fec35099cbee266d                         No update available
blueprints-8c50f85abaa5d22110b7c826bcb77f8f33d39dbb   blueprints            No update available
```

Can you double check my above steps and let me know if there is anything differently that you did? Would you mind also sharing the Kptfiles of your packages? The `upstreamLock.Git.Ref` in the Kptfile for cloned packages should be of the format `drafts/pkg-name/workspace` or `pkg-name/version`. If they are not, then porch won't know how to parse them, so I'm wondering if one of the commands you ran incorrectly wrote out an invalid `upstreamLock.Git.Ref`. Please also let us know if you made any changes to the package outside of running `kpt alpha rpkg/repo` commands. Also please let us know what version of kpt CLI you are using and what version of porch you have running?

I also created a PR here https://github.com/GoogleContainerTools/kpt/pull/3938 to error out instead of panic. 

As a side note, I don't think the `--ref=v2` does anything for your `rpkg clone` command. I think this is only relevant if you are cloning directly with a git URL or from OCI. 

@Cbkhare: Hi Natasha, 

Thanks for checking this. Below are some pointers wrt to above comment of yours. 

1) 2 of my Directory in the repository have the kptfiles with details as follows. I can see the ref is `main`. This was a cloned package. 
```
upstreamLock:
  type: git
  git:
    repo: https://github.com/***********/*****************.git
    directory: test-pkg
    ref: main
    commit: 4331f0d83c6aa9f98b5a51e6 
```
2) I added some files in the Directory from the Github UI. 

3) kpt version 
```
~$ kpt version
1.0.0-beta.31
```@natasha41575: Ah I see, that would explain it. `rpkg update --discover` doesn't handle clones from `main`. That is definitely a bug and we should see if we can fix it. @liamfallon: Duplicated to Porch in the Nephio project: https://github.com/nephio-project/nephio/issues/612

Solution patch:
diff --git a/commands/alpha/rpkg/update/discover.go b/commands/alpha/rpkg/update/discover.go
index b7e5bb29..3f25f87f 100644
--- 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



Test patch:
diff --git a/e2e/testdata/porch/rpkg-update/config.yaml b/e2e/testdata/porch/rpkg-update/config.yaml
index 2477322c..1a7139a0 100644
--- 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
\ No newline at end of file