CP-47357: Add a Go JSON-RPC client file #5615

xueqingz · 2024-05-06T05:50:59Z

No description provided.

psafont · 2024-05-08T09:58:21Z

Is this the one needed for testing in the CI?

xueqingz · 2024-05-08T10:14:26Z

Is this the one needed for testing in the CI?

Yes, this is the JSON HTTP RPC client for Go SDK, we will add component test in following PRs in CI. Besides we also test the SDK functionalities in XenRT with https://github.com/xenserver/xenserver-samples/tree/master/go.

gangj · 2024-05-11T08:56:42Z

Would you please comment the test for this code here, either manual(snapshot) or XenRT(job) is ok, thanks (To indicate the current version of code passed the test).

xueqingz · 2024-05-11T10:36:08Z

For Golang lint checking.

root@3eea0d3e0e5d:/app/go/goSDK# golangci-lint run --config=/app/.golangci.yml
WARN The linter 'gomnd' is deprecated (since v1.58.0) due to: The linter has been renamed. Replaced by mnd. 
WARN The linter 'execinquery' is deprecated (since v1.58.0) due to: The repository of the linter has been archived by the owner.  
0 issues.

XenRT job base on 874f9a6 with this change:
3998847 rt-Next
3998852 Yangtze

minglumlu · 2024-05-13T03:24:33Z

ocaml/sdk-gen/go/autogen/src/jsonrpc_client.go

+}
+
+func convertUnhandledJSONData(jsonBytes []byte) []byte {
+	jsonString := string(jsonBytes)


Converting the bytes to string and then back to bytes may not be clear here.
What I'm concerning is the encoding in the converting.

When converting byte slices ([]byte) to strings in Go, encoding is not directly involved. Strings in Go are read-only slices of bytes, we can directly convert a []byte (byte slice) to a string. we can also use:

s := fmt.Sprintf("%s", b)

minglumlu · 2024-05-13T04:32:40Z

ocaml/sdk-gen/go/autogen/src/jsonrpc_client.go

+
+	request.Header.Set("Content-Type", "application/json; charset=utf-8")
+	request.Header.Set("Accept", "application/json")
+	request.Header.Set("User-Agent", "XenAPI/" + APIVersionLatest.String())


Something like XenAPI Go SDK/24.17.0?

Hi, ming. Here I just follow the code in XC https://code.citrite.net/projects/XSIF/repos/xencenter/browse/XenModel/XenAPI/Session.cs#47.

Hi, ming. Here I just follow the code in XC https://code.citrite.net/users/xueqingz/repos/xencenter/browse/XenModel/XenAPI/Session.cs#47.

Access is denied

sorry, update link.

minglumlu · 2024-05-13T04:42:19Z

ocaml/sdk-gen/go/autogen/src/jsonrpc_client.go

+		headers:    make(map[string]string),
+	}
+
+	if strings.HasPrefix(opts.URL, "https://") {


The "https://pkg.go.dev/net/url#Parse" could be used to parse a raw URL.

minglumlu · 2024-05-13T05:02:10Z

ocaml/sdk-gen/go/autogen/src/jsonrpc_client.go

+		caCertPool := x509.NewCertPool()
+		certs := []tls.Certificate{}
+		if opts.SecureOpts != nil {
+			skipVerify = false


I understand this should be false iif when the CA is presented. Otherwise, the CA is not required.
So it should be moved into if opts.SecureOpts.ServerCert != "" {

minglumlu · 2024-05-13T05:12:59Z

ocaml/sdk-gen/go/autogen/src/jsonrpc_client.go

+				certs = []tls.Certificate{cert}
+			}
+		}
+		tlsConfig := &tls.Config{


In the tls.Config, the CipherSuites needs to be set explicitly for the sake of security.
It would be better to set the TLS version explicitly as well.

minglumlu · 2024-05-13T05:25:00Z

ocaml/sdk-gen/go/autogen/src/jsonrpc_client.go

+				if err != nil {
+					log.Fatal(err)
+				}
+				caCertPool.AppendCertsFromPEM(caCert)


The return needs to be cared. An invalid cert can't be ignored.

minglumlu · 2024-05-13T05:41:28Z

ocaml/sdk-gen/go/autogen/src/jsonrpc_client.go

+func paramsParse(params ...interface{}) interface{} {
+	var finalParams interface{}
+	finalParams = params
+	if len(params) == 1 {


Could some comments be added to explain why this is required when there is only one parameter?
And why is a finalParams required here, as it gets a copy of a slice, which means it actually refers to the same memory location of params? This is because I understand the params is a slice rather than an array.

The paramParse is to handle different types of parameter support way in method. eg.
Examples:

Examples: 1.Nil: sendCall(ctx, "method_name") -> {"method": "method_name"} 2.Single value (simple type): sendCall(ctx, "method_name", 1) -> {"method": "method_name", "params": [1]} 3.Single value (slice/array): sendCall(ctx, "method_name", []int{1, 2, 3}) -> {"method": "method_name", "params": [1, 2, 3]} 4.Single value (struct): sendCall(ctx, "method_name", &Person{Name: "Alex", Age: 35}) -> {"method": "method_name", "params": {"name": "Alex", "age": 35}} 5.Multiple values: sendCall(ctx, "method_name", 1, 2, 3) -> {"method": "method_name", "params": [1, 2, 3]}

For case 3,4 ,we need to set finalParams = params[0].

I’ll add some comments on code.

After discussion about details , we think it's actually not needed, we only need to support 1, 2,5. remove this function.

Signed-off-by: xueqingz <xueqing.zhang@citrix.com>

xueqingz · 2024-05-14T10:33:15Z

checking golang lint on new commit:

root@28dd5689a02b:/app/go/goSDK# golangci-lint run --config=/app/.golangci.yml
WARN The linter 'execinquery' is deprecated (since v1.58.0) due to: The repository of the linter has been archived by the owner.  
WARN The linter 'gomnd' is deprecated (since v1.58.0) due to: The linter has been renamed. Replaced by mnd. 
0 issues.

Test for review changes:

bad http/https url

ubuntu@ubuntu-server:~/code/xenserver-samples/go$ go test base_test.go -ip="10.70.40.100" -username="root" -password="BOfpcNyZ5cMe" -ca_cert_path="/home/ubuntu/xapi-ssl.pem" -v
=== RUN   TestLogin
    base_test.go:35: call session.login_with_password() on htt://10.70.40.100/jsonrpc. Error making http request: Post "htt://10.70.40.100/jsonrpc": unsupported protocol scheme "htt"
--- FAIL: TestLogin (0.00s)
FAIL
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x6c05c2]

goroutine 1 [running]:
command-line-arguments.TestMain(0xc000110960)
        /home/ubuntu/code/xenserver-samples/go/base_test.go:49 +0xa2
main.main()
        _testmain.go:49 +0x195
FAIL    command-line-arguments  0.005s
FAIL

https with valid CA certificate/ https with opts.SecureOpts.ServerCert = "" :

ubuntu@ubuntu-server:~/code/xenserver-samples/go$ go test base_test.go -ip="10.70.40.100" -username="root" -password="BOfpcNyZ5cMe" -ca_cert_path="/home/ubuntu/xapi-ssl.pem" -v
=== RUN   TestLogin
/home/ubuntu/xapi-ssl.pem
    base_test.go:39: api version:  2.21
    base_test.go:40: xapi rpm version:  24.14
--- PASS: TestLogin (0.05s)
PASS
ok      command-line-arguments  0.052s

https with invalid CA certificate:

ubuntu@ubuntu-server:~/code/xenserver-samples/go$ go test base_test.go -ip="10.70.40.100" -username="root" -password="BOfpcNyZ5cMe" -ca_cert_path="/home/ubuntu/xapi-ssl-bad.pem" -v
=== RUN   TestLogin
2024/05/14 09:12:29 failed to parse CA certificate
FAIL    command-line-arguments  0.003s
FAIL

Try run xenserver-samples test manually, no special error found.

github-actions · 2024-05-14T10:45:19Z

pytype_reporter extracted 50 problem reports from pytype output

.

You can check the results of the job here

duobei approved these changes May 6, 2024

View reviewed changes

duobei requested a review from kc284 May 6, 2024 05:53

xueqingz force-pushed the private/xueqingz/CP-47357 branch from 21c0782 to 69fc086 Compare May 8, 2024 02:43

duobei requested a review from minglumlu May 8, 2024 03:01

minglumlu reviewed May 13, 2024

View reviewed changes

xueqingz added 2 commits May 14, 2024 10:19

CP-47357: Add a Go JSON-RPC client file

22e7ffa

Signed-off-by: xueqingz <xueqing.zhang@citrix.com>

CP-47357: fix review issues

890ae8d

Signed-off-by: xueqingz <xueqing.zhang@citrix.com>

xueqingz force-pushed the private/xueqingz/CP-47357 branch from 69fc086 to 890ae8d Compare May 14, 2024 10:20

minglumlu approved these changes May 14, 2024

View reviewed changes

xueqingz merged commit a20df67 into xapi-project:feature/go_sdk May 14, 2024
13 checks passed

CP-47357: Add a Go JSON-RPC client file #5615

CP-47357: Add a Go JSON-RPC client file #5615

Uh oh!

Conversation

xueqingz commented May 6, 2024

Uh oh!

psafont commented May 8, 2024

Uh oh!

xueqingz commented May 8, 2024

Uh oh!

gangj commented May 11, 2024

Uh oh!

xueqingz commented May 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

minglumlu May 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xueqingz May 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

minglumlu May 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xueqingz commented May 14, 2024

Uh oh!

Uh oh!

github-actions bot commented May 14, 2024

pytype_reporter extracted 50 problem reports from pytype output

Uh oh!

Uh oh!

xueqingz commented May 11, 2024 •

edited

Loading

minglumlu May 13, 2024 •

edited

Loading

xueqingz May 13, 2024 •

edited

Loading

minglumlu May 13, 2024 •

edited

Loading