taubyte / dream Goto Github PK

Hey guys, I'm not experienced in Go, but have a great experience programming in general.

I was investigating api.go file and have a suggestion related to the imports that can be handled better with a simple try catch.

It would be easy to print which package is dealing failing to import locally(cause: versioning, updates, path, broken)

Apart from that, I really enjoyed playing with the app. Great invention!

Make is easy to install and use even if you don't have golang

npm i dreamland
$ dream

We can reduce the number of responsibilities that this golang file has by placing the types EchartNode, EchartLinks, EchartCat, and Echart into another go file (EchartTypes.go) and importing the Echart types as a package.

[/Users/margaretcamilletti/go/src/dreamland/service/new.go:30-32] - G402 (CWE-295): TLS MinVersion too low. (Confidence: HIGH, Severity: HIGH)
29: c.client.Transport = &http.Transport{

30: TLSClientConfig: &tls.Config{
31: RootCAs: rootCAs,
32: },
33: }

[/Users/margaretcamilletti/go/src/dreamland/service/new.go:37] - G402 (CWE-295): TLS InsecureSkipVerify set true. (Confidence: HIGH, Severity: HIGH)
36: TLSClientConfig: &tls.Config{

37: InsecureSkipVerify: true,
38: },

[/Users/margaretcamilletti/go/src/dreamland/cli/inject/service.go:40] - G602 (CWE-118): Potentially accessing slice out of bounds (Confidence: MEDIUM, Severity: MEDIUM)
39: if http != 0 {

40: others["http"] = http
41: }

[/Users/margaretcamilletti/go/src/dreamland/service/cors/helpers.go:26] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
25: func OutError(w http.ResponseWriter, code int, msg string) {

26: w.Write([]byte(msg))
27: w.WriteHeader(code)

[/Users/margaretcamilletti/go/src/dreamland/cli/command/universe0.go:28] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
27: }

28: ctx.Set("universe", universe)
29: return action(ctx)

[/Users/margaretcamilletti/go/src/dreamland/cli/command/universe.go:30] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
29: }

30: ctx.Set("universe", universe)
31: return action(ctx)

[/Users/margaretcamilletti/go/src/dreamland/cli/command/names.go:28] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
27: }

28: ctx.Set("names", names)
29: return action(ctx)

[/Users/margaretcamilletti/go/src/dreamland/cli/command/name.go:39] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
38: }

39: ctx.Set("name", name)
40: return action(ctx)

I would like to suggest of adding comments on each part of a method or a function that what exactly it is doing or what each function parameter represents. This would help a new reader to under stand the code easily. And Also If I were you I would have use better variables names or meaning full variable names like what it is doing.

Proposing to rebuild this cli from the ground up.

This module can be simplified greatly, some more helpers should be added to tau/libdream before rebuilding though.

service/universe_test.go

code contains some code that could be more compartmentalized into a function for easier readability. The err variable is reassigned often and can be challenging to track
eg. lines 96-108

  func TestKillService(t *testing.T) {
	  err := universe.KillService("seer")
  
	  if err != nil {      // should not fail
		  t.Errorf("Failed kill call with error: %v", err)
	  } else { 		  // should fail
		  err = universe.KillService("seer")
		  if err == nil {
			  t.Error("Expected killing seer again to fail")
		  }
	  }
  }

lines 73-94 could also be compartmentalized.

Using consistent naming conventions in the imported packages to improve the code readability.
Consider using the library 'logrus' for better error handling.

Hi there! I had a chance to analyse a code of this repo and I have a few suggections for you:

1.Consider adding comments and documentation to your code to explain the purpose of the functions, expected input and output, and any relevant details about how the HTTP requests are being handled. This will make it easier for others (and your future self) to understand and use the package.
2. The code unmarshals the HTTP response body twice in some cases (once to check for errors and then to parse the response into the ret interface). You can optimize this by unmarshaling once and reusing the result if there are no errors.
3. Be cautious when sending and handling sensitive data, such as authorization tokens. Ensure that sensitive data is handled securely and doesn't leak in error messages or logs.

Let me know if there is something I'm missing or if you have any questions
Thanks for your time

Description:
The current implementation of the bindConfigServices function could create outputs and/or errors that would be confusing to debug.

Issue:
Looking at the https://github.com/taubyte/dreamland/blob/main/cli/new/helpers.go file, it seems that if the conditional check on line 78 fails, then we will end up with a port of 0 and a sub of "" for any given service. Apparently it is possible to use port 0 to bind to a random available port, however this could cause problems with the use of TCP as 0 is a reserved port not intended to be used. In any case, a sub of "" is not in the list of ValidSubBinds found in the file https://github.com/taubyte/dreamland/blob/main/cli/common/vars.go. We see sub being validated on line 86, but not if the conditional check on line 78 fails. So this seems like a contradiction of logic: likely it should be validated both times.

Also, if it is possible for any service to bind to port 0, I would expect the conditional on line 112 to fail from time to time, in which case we would get an error for two services having duplicate port bindings. And the reason they are duplicate is just that they ended up taking on a default value after the conditional on line 78 failed. This gives the appearance that the code is written assuming the conditional on 78 will not fail.

Possible Solution:
It would likely be better to save the confusion by just creating a requirement that the result of strings.Split(bind, "@") on line 63 have a length == 2, instead of letting the inputs fall all the way through to the validation of duplicate ports to fail. This could save some debugging efforts insofar as we would not have to read all the times that port gets initialized, reassigned, or accessed on the binds map.