Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: cyphernetes shell -A results inpanic #104

Closed
jameskim0987 opened this issue Oct 9, 2024 · 6 comments · Fixed by #107
Closed

bug: cyphernetes shell -A results inpanic #104

jameskim0987 opened this issue Oct 9, 2024 · 6 comments · Fixed by #107
Labels
bug Something isn't working shell
Milestone

Comments

@jameskim0987
Copy link
Contributor

jameskim0987 commented Oct 9, 2024

Issues observed:

  1. When running cyphernetes shell -A or cyphernetes shell --all-namespaces and a query, it results in a panic.
  2. When running cyphernetes shell -A or cyphernetes shell --all-namespaces, the namespace defaults to default
jameskim:~/workspace/cyphernetes/dist $ ./cyphernetes shell -A

                __                    __
 ______ _____  / /  ___ _______  ___ / /____ ___
/ __/ // / _ \/ _ \/ -_) __/ _ \/ -_) __/ -_|_-<
\__/\_, / .__/_//_/\__/_/ /_//_/\__/\__/\__/___/
   /___/_/ Interactive Shell

🔎 fetching resource specs from openapi... done!

Type 'exit' or press Ctrl-D to exit
Type 'help' for information on how to use the shell

(docker-desktop) default » MATCH (p:Pods) RETURN p.metadata.name;

2024/10/08 21:10:08 Lexing...  77  ( M )
2024/10/08 21:10:08 Scanned token: -2
2024/10/08 21:10:08 Token text: �
2024/10/08 21:10:08 Returning MATCH token
2024/10/08 21:10:08 Lexing...  32  (   )
2024/10/08 21:10:08 Scanned token: 40
2024/10/08 21:10:08 Token text: (
2024/10/08 21:10:08 Returning LPAREN token
2024/10/08 21:10:08 Lexing...  112  ( p )
2024/10/08 21:10:08 Scanned token: -2
2024/10/08 21:10:08 Token text: �
2024/10/08 21:10:08 Returning IDENT token with value: p
2024/10/08 21:10:08 Lexing...  58  ( : )
2024/10/08 21:10:08 Scanned token: 58
2024/10/08 21:10:08 Token text: :
2024/10/08 21:10:08 Returning COLON token
2024/10/08 21:10:08 Lexing...  80  ( P )
2024/10/08 21:10:08 Scanned token: -2
2024/10/08 21:10:08 Token text: �
2024/10/08 21:10:08 Returning IDENT token with value: Pods
2024/10/08 21:10:08 Lexing...  41  ( ) )
2024/10/08 21:10:08 Scanned token: 41
2024/10/08 21:10:08 Token text: )
2024/10/08 21:10:08 Returning RPAREN token
2024/10/08 21:10:08 Lexing...  32  (   )
2024/10/08 21:10:08 Scanned token: -2
2024/10/08 21:10:08 Token text: �
2024/10/08 21:10:08 Returning RETURN token
2024/10/08 21:10:08 Lexing...  32  (   )
2024/10/08 21:10:08 Returning JSONPATH token with value: p.metadata.name
2024/10/08 21:10:08 Lexing...  -1  ( � )
2024/10/08 21:10:08 Scanned token: -1
2024/10/08 21:10:08 Token text: �
2024/10/08 21:10:08 Returning EOF token
2024/10/08 21:10:08 Lexing...  -1  ( � )
2024/10/08 21:10:08 Zero (buffered EOF)
2024/10/08 21:10:08 Node pattern found. Name: p Kind: Pods
2024/10/08 21:10:08 Listing resources of kind: Pods with fieldSelector:  and labelSelector:
panic: interface conversion: interface {} is nil, not []map[string]interface {}

goroutine 1 [running]:
github.com/avitaltamir/cyphernetes/pkg/parser.(*QueryExecutor).processNodes(0xc000450630, 0xc00078e190, 0xc000a512d8)
        /Users/jameskim/workspace/cyphernetes/pkg/parser/k8s_query.go:710 +0x707
github.com/avitaltamir/cyphernetes/pkg/parser.(*QueryExecutor).Execute(0xc000450630, 0xc00023e150, {0x0?, 0xaa7e37a?})
        /Users/jameskim/workspace/cyphernetes/pkg/parser/k8s_query.go:84 +0xe05
main.executeStatement({0xc001a16090?, 0x10?})
        /Users/jameskim/workspace/cyphernetes/cmd/cyphernetes/shell.go:553 +0x95
main.processQuery({0xc001a16090, 0x25})
        /Users/jameskim/workspace/cyphernetes/cmd/cyphernetes/shell.go:490 +0x231
main.runShell(0xc0002eaa00?, {0xbc31de6?, 0x4?, 0xbc31dea?})
        /Users/jameskim/workspace/cyphernetes/cmd/cyphernetes/shell.go:415 +0x1678
github.com/spf13/cobra.(*Command).execute(0xcf01e20, {0xc00044da70, 0x1, 0x1})
        /Users/jameskim/go/pkg/mod/github.com/spf13/[email protected]/command.go:989 +0xa91
github.com/spf13/cobra.(*Command).ExecuteC(0xcf01860)
        /Users/jameskim/go/pkg/mod/github.com/spf13/[email protected]/command.go:1117 +0x3ff
github.com/spf13/cobra.(*Command).Execute(...)
        /Users/jameskim/go/pkg/mod/github.com/spf13/[email protected]/command.go:1041
main.Execute()
        /Users/jameskim/workspace/cyphernetes/cmd/cyphernetes/root.go:23 +0x1a
main.main()
        /Users/jameskim/workspace/cyphernetes/cmd/cyphernetes/main.go:7 +0xf
@jameskim0987 jameskim0987 changed the title cyphernetes shell -A results in a panic bug: cyphernetes shell -A results in a panic Oct 9, 2024
@jameskim0987 jameskim0987 changed the title bug: cyphernetes shell -A results in a panic bug: cyphernetes shell -A results inpanic Oct 9, 2024
@jameskim0987
Copy link
Contributor Author

I can investigate this one

@AvitalTamir
Copy link
Owner

AvitalTamir commented Oct 9, 2024

Hey @jameskim0987 thanks for reporting!
Curious to what you find, I can't reproduce - however this flag doesn't actually work for me and I still launch into the default namespace. This is a regression and I assume related to a recent change in how we manage the Namespace variable in the parser package which wasn't covered by tests 😬

Curious, if you're in the shell and do \n all does it also crash? I'm able to switch to all namespaces mode using \n all.

@AvitalTamir AvitalTamir added bug Something isn't working shell labels Oct 9, 2024
@jameskim0987
Copy link
Contributor Author

jameskim0987 commented Oct 9, 2024

@AvitalTamir Hi, thanks for getting back. It is interesting to hear that this issue is not reproducible. I find it odd that it works fails on my machine 😅 ( I'm running on darwin btw )

When I run the shell with just ./cyphernetes shell ( without the -A ), the queries run as expected. Even if I do a '\n all within that shell and run a query, I get the expected result so I think it is fair to say the regular ./cyphernetes shell works.
Also, ./cyphernetes shell -n <NAMESPACE> works fine as well. So all good here.

I guess on my machine, that -A is having some issues. I have a question about that flag though. From the help command output it says:

Flags:
  -A, --all-namespaces     Query all namespaces
  -h, --help               help for cyphernetes
  -l, --loglevel string    The log level to use (debug, info, warn, error, fatal, panic) (default "info")
  -n, --namespace string   The namespace to query against (default "default")

but traversing through the codebase, I did not see a handle for the case when parser.AllNamespaces == true. Would this mean we should remove support for -A and expect the user to use \n all? or should we create a handler for that case? ( imo, I like the former because \n all is already a valid command within the cyphernetes shell so having -A as part of the shell execution argument seems redundant ) but curious to hear your thoughts.

Another question, suppose we keep the -A flag, would this indicate that throughout the entire shell -A session, would the user be locked into all namespaces and not able to change to a different namespace?

( On the side, when flagged with -A, I've been trying to make the shell start with the red ALL NAMESPACES but having some issues... I also tried debugging through why it results in a nil value which I half understand but I think I need more of a deep dive on the logic like resultCache vs resultMap and namespace conventions within the code )

EDIT: I have the changes needed: see: #107. Let me know how it looks ( I will have to add some tests )

@AvitalTamir
Copy link
Owner

Looks good @jameskim0987!
Tested it here, works great for me now.
Cheers, approved and let me know when you're ready 👍

@AvitalTamir AvitalTamir added this to the v0.13 milestone Oct 10, 2024
@jameskim0987
Copy link
Contributor Author

@AvitalTamir I've been working on getting the tests added that covers the shell flags and its expected output. However, I'm having some unexpected issues.
When the tests are ran separately one by one, they all pass but when it is ran as an entire suite like make test, some tests fail.

Usually when this happens, I suspect something shared is being used across the tests so I've been trying to deep dive with a debugger but I wasn't able to get it to the bottom of it yet. I'll have to spend a bit more time.

In case you might have insight on this, this is the test content: 3d01e9d

Let me know if you have encountered this before, it looks like some logic in the Cobra package is causing some unexpected behaviors but I'll need to look more in depth!

@AvitalTamir
Copy link
Owner

Oh wow, never came across anything like it, I'll take a look!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working shell
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants