nephio-project / nephio

Nephio is a Kubernetes-based automation platform for deploying and managing highly distributed, interconnected workloads such as 5G Network Functions, and the underlying infrastructure on which those workloads depend.
Apache License 2.0
93 stars 52 forks source link

Porchctl repo command issues #755

Open dgeorgievski opened 1 week ago

dgeorgievski commented 1 week ago

While reviewing the fix for Porch support of private Git repos https://github.com/nephio-project/nephio/issues/488 https://github.com/nephio-project/porch/pull/58

I came across the following potential bugs in porchctl repo command

The result of porchctl repo get -A/--all-namespaces misses the NAMESPACE column

 $ porchctl repo get -A
NAME               TYPE   CONTENT   DEPLOYMENT   READY   ADDRESS
my-app             git    Package   false        True    https://gitlab.myprivaterepo.com/dimitar.georgievski/my-app.git
testrepo           git    Package   false        True    http://gitea-http.gitea.svc.cluster.local:3000/nephio/testrepo.git

# the desired output
$ kubectl get repositories -A
NAMESPACE    NAME               TYPE   CONTENT   DEPLOYMENT   READY   ADDRESS
gitlab       my-app             git    Package   false        True    https://gitlab.myprivaterepo.com/dimitar.georgievski/my-app.git
porch-test   testrepo           git    Package   false        True    http://gitea-http.gitea.svc.cluster.local:3000/nephio/testrepo.git

A simple fix for adding the missing column could be like https://github.com/dgeorgievski/porch/blob/fix-porchctl-repo/pkg/cli/commands/repo/get/command.go#L84-L91

Invoking of porchctl repo reg without -n/--namespace fails

The intent is to register a new Git repo in the current namespace

$ porchctl repo reg https://gitlab.myprivaterepo.com/dimitar.georgievski/my-app.git --branch master
Error: an empty namespace may not be set when a resource name is provided

The same error is displayed in the case of porchctl repo unreg invocation

$ porchctl repo unreg my-app
Error: an empty namespace may not be set when a resource name is provided 

Again, these simple fixes would help address the issues repo reg : https://github.com/dgeorgievski/porch/blob/fix-porchctl-repo/pkg/cli/commands/repo/reg/command.go#L93-L100 repo unreg: https://github.com/dgeorgievski/porch/blob/fix-porchctl-repo/pkg/cli/commands/repo/unreg/command.go#L75-L82

dgeorgievski commented 1 week ago

I can create a PR if you are OK with it.