Use shared helper to build http client for log and run #39

Doridian · 2022-09-07T22:49:14Z

Also this fixes gok run building anything at all.

Looking at packer.Build in its current version, it just isn't suited to "build current directory" it seems. Or at least I could not find any way to get it to build local packages (such as . or ./package) no matter what I tried to pass to it from the gok run code. (I fully expect this to not just get merged as I put it, but rather invite a discussion on the correct way to fix gok run)

stapelberg · 2022-09-09T13:16:01Z

Thanks for your PR! We should probably split it up into separate PRs, but we can discuss first what needs to be done.

Also this fixes gok run building anything at all.

Looking at packer.Build in its current version, it just isn't suited to "build current directory" it seems. Or at least I could not find any way to get it to build local packages (such as . or ./package) no matter what I tried to pass to it from the gok run code. (I fully expect this to not just get merged as I put it, but rather invite a discussion on the correct way to fix gok run)

This bit I don’t understand. I use gok run all the time, and it works just fine to build the current directory? Most recently, I ran GOARCH=amd64 gok run -i router7 to deploy the Go program in the working directory to my router.

Can you describe in detail what you see that doesn’t work please?

Doridian · 2022-09-09T13:24:49Z

This bit I don’t understand. I use gok run all the time, and it works just fine to build the current directory? Most recently, I ran GOARCH=amd64 gok run -i router7 to deploy the Go program in the working directory to my router.

Can you describe in detail what you see that doesn’t work please?

(Note: I have GOARCH/GOOS/... set via export so I don't have to retype them constantly)

When I use gok run with the current version of gok, I first hit one issue (note: I used tls=self-signed to set up my instance, gokr-packer correctly uses the right TLS cert and functions. I assume if my appliance had a public hostname and default trusted cert, this would just work):

$ gok run --instance streamdeckpi
2022/09/09 06:21:24 basename: "agent"
[done] in 0.00s
2022/09/09 06:21:47 checking target partuuid support: Get "https://streamdeckpi/update/features": x509: “gokrazy” certificate is not trusted

If I fix that issue (which my PR was first meant to address with the httpClient things), I get hit by the next problem:

$ gok run --instance streamdeckpi
2022/09/09 06:26:21 basename: "agent"
[done] in 0.00s
2022/09/09 06:26:21 Using certificate /Users/doridian/Library/Application Support/gokrazy/hosts/streamdeckpi/cert.pem
2022/09/09 06:26:25 binary agent not installed; are you not in a directory where .go files declare “package main”?

(Quite simply, the packer.Build just builds nothing at all, and I quite definitely am inside a folder with package main), see:

$ go build . && file agent
agent: ELF 32-bit LSB executable, ARM, EABI5 version 1 (SYSV), statically linked, Go BuildID=t-Ef-HUSrp9knXF8y_iO/jHutp5hou9sRr3gjE88i/U7xChbd3elYxoFohVaPI/idyzGdZyBk1v9icvhilD, with debug_info, not stripped

stapelberg · 2022-09-09T14:02:07Z

How can I reproduce this? Which repository are you working with? In which directory are you building?

Doridian · 2022-09-09T14:04:53Z

How can I reproduce this? Which repository are you working with? In which directory are you building?

For me, this reproduces in any (freshly cloned or not) repo (the below log is with the TLS error fixed, but the same goes for the TLS error).
For example (note: I tested this with a folder named just hello as well, that does not fix it):

$ cd ~/Programming
$ git clone https://github.com/gokrazy/hello.git gokrazy-hello
Cloning into 'gokrazy-hello'...
remote: Enumerating objects: 8, done.
remote: Total 8 (delta 0), reused 0 (delta 0), pack-reused 8
Receiving objects: 100% (8/8), done.
Resolving deltas: 100% (1/1), done.
$ cd gokrazy-hello
$ gok run --instance streamdeckpi
2022/09/09 07:04:03 basename: "gokrazy-hello"
[done] in 0.00s
2022/09/09 07:04:03 Using certificate /Users/doridian/Library/Application Support/gokrazy/hosts/streamdeckpi/cert.pem
2022/09/09 07:04:07 binary gokrazy-hello not installed; are you not in a directory where .go files declare “package main”?

//EDIT: I should apologize, completely forgot my bug report 101 there to actually provide a repro, whoops.

As for versions: Everything is using a git main branch version, compiled locally.

$ uname -a
Darwin capefox 21.6.0 Darwin Kernel Version 21.6.0: Wed Aug 10 14:28:23 PDT 2022; root:xnu-8020.141.5~2/RELEASE_ARM64_T6000 arm64
$ go version
go version go1.19 darwin/arm64

I use macOS, but I am pretty sure the error would also happen on Linux. I'll test this in Docker in a bit.

Yes, it happens in a very stock Linux as well (latest golang Docker image):

root@21ec47653bbb:/tmp# git clone https://github.com/gokrazy/tools
Cloning into 'tools'...
[...]
root@21ec47653bbb:/tmp# git clone https://github.com/gokrazy/hello
Cloning into 'hello'...
[...]

root@21ec47653bbb:/tmp# ls
hello  tools
root@21ec47653bbb:/tmp# cd tools/
root@21ec47653bbb:/tmp/tools# go build ./cmd/gok
root@21ec47653bbb:/tmp/tools# cd ../hello/

root@21ec47653bbb:/tmp/hello# GOARCH=arm GOARM=6 ../tools/gok run -i streamdeckpi
2022/09/09 14:16:54 basename: "hello"
[done] in 0.00s
2022/09/09 14:16:57 checking target partuuid support: Get "https://streamdeckpi/update/features": x509: certificate signed by unknown authority

[ repeat build steps for gok-tlsfix, except with my fixes for just TLS, and not packer.Build ]
[ See: https://github.com/Doridian/gokrazy-tools/tree/only-fixtls ]

root@21ec47653bbb:/tmp/hello# GOARCH=arm GOARM=6 ../tools/gok-tlsfix run -i streamdeckpi
2022/09/09 14:17:58 basename: "hello"
[done] in 0.00s
2022/09/09 14:17:58 Using certificate /root/.config/gokrazy/hosts/streamdeckpi/cert.pem
2022/09/09 14:18:02 binary hello not installed; are you not in a directory where .go files declare “package main”?

related to #39

stapelberg · 2022-09-13T18:47:47Z

Thanks for the steps. Turns out the answer to “how can I reproduce this?” is “by building the gok program from the latest commit” — when I worked on commit c3979e1, apparently I never re-compiled gok, which is why I didn’t notice the issue.

I have added an integration test to prevent such problems in the future, and pushed a fix.

Could you rebase your pull request please?

Doridian · 2022-09-13T19:19:54Z

Rebase complete. Tested locally on my Pi and run/logs still works.

stapelberg

Can you also update the commit message please? This commit no longer fixes building :)

stapelberg · 2022-09-13T19:21:47Z

go.mod

@@ -3,19 +3,19 @@ module github.com/gokrazy/tools
 go 1.18

 require (
-	github.com/breml/rootcerts v0.2.0
+	github.com/breml/rootcerts v0.2.6


Are any changes to go.mod and go.sum actually necessary, or could you revert these?

Some of them might be, unless you updated it to use the latest gokrazy/internal (for the hostname() change I PR'd).
Usually I never bother with manually fudging with go.mod and just run go mod tidy, which updates and fixes checksums all on its own whenever I have to touch dependencies (especially as it erases old checksums that are no longer necessary preventing the go.sum inflating to a giant size).

stapelberg · 2022-09-13T19:24:28Z

internal/helpers/httpclient.go

+	"github.com/gokrazy/internal/updateflag"
+)
+
+func GetHTTPClient(instance string) (*http.Client, *url.URL, error) {


Could this function instead live in the github.com/gokrazy/internal/httpclient package, with the rest of the shared httpclient functionality? :)

It can, I was just being lazy and didn't wanna file two PRs :p

stapelberg · 2022-09-13T19:24:49Z

internal/helpers/httpclient.go

+func GetHTTPClient(instance string) (*http.Client, *url.URL, error) {
+	_, updateHostname := updateflag.GetUpdateTarget(instance)
+	const configBaseName = "http-password.txt"
+	pw, err := config.HostnameSpecific(updateHostname).ReadFile(configBaseName)


nit: just use "http-password.txt" directly (without the constant) for consistency

Doridian · 2022-09-13T19:33:07Z

New PR for the HTTPClient functionality only: gokrazy/internal#14
Will have to update this PR once that one is merged for the go.mod hash.

stapelberg · 2022-09-13T20:16:20Z

Now merged! Also did a go mod tidy.

Doridian · 2022-09-13T21:40:32Z

PR rebased ontop of main with updated internal repo hash.

stapelberg added a commit that referenced this pull request Sep 13, 2022

make packer.BuildDir function configurable, fix gok run

2e1812d

related to #39

stapelberg requested changes Sep 13, 2022

View reviewed changes

Use shared helper to build http client for log and run.

326daa2

stapelberg approved these changes Sep 14, 2022

View reviewed changes

stapelberg merged commit 01edd60 into gokrazy:main Sep 14, 2022

Doridian deleted the pr-fixtls branch September 14, 2022 06:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use shared helper to build http client for log and run #39

Use shared helper to build http client for log and run #39

Doridian commented Sep 7, 2022 •

edited

Loading

stapelberg commented Sep 9, 2022

Doridian commented Sep 9, 2022 •

edited

Loading

stapelberg commented Sep 9, 2022

Doridian commented Sep 9, 2022 •

edited

Loading

stapelberg commented Sep 13, 2022

Doridian commented Sep 13, 2022

stapelberg left a comment

stapelberg Sep 13, 2022

Doridian Sep 13, 2022 •

edited

Loading

stapelberg Sep 13, 2022

Doridian Sep 13, 2022

stapelberg Sep 13, 2022

Doridian commented Sep 13, 2022 •

edited

Loading

stapelberg commented Sep 13, 2022

Doridian commented Sep 13, 2022

Use shared helper to build http client for log and run #39

Use shared helper to build http client for log and run #39

Conversation

Doridian commented Sep 7, 2022 • edited Loading

stapelberg commented Sep 9, 2022

Doridian commented Sep 9, 2022 • edited Loading

stapelberg commented Sep 9, 2022

Doridian commented Sep 9, 2022 • edited Loading

stapelberg commented Sep 13, 2022

Doridian commented Sep 13, 2022

stapelberg left a comment

Choose a reason for hiding this comment

stapelberg Sep 13, 2022

Choose a reason for hiding this comment

Doridian Sep 13, 2022 • edited Loading

Choose a reason for hiding this comment

stapelberg Sep 13, 2022

Choose a reason for hiding this comment

Doridian Sep 13, 2022

Choose a reason for hiding this comment

stapelberg Sep 13, 2022

Choose a reason for hiding this comment

Doridian commented Sep 13, 2022 • edited Loading

stapelberg commented Sep 13, 2022

Doridian commented Sep 13, 2022

Doridian commented Sep 7, 2022 •

edited

Loading

Doridian commented Sep 9, 2022 •

edited

Loading

Doridian commented Sep 9, 2022 •

edited

Loading

Doridian Sep 13, 2022 •

edited

Loading

Doridian commented Sep 13, 2022 •

edited

Loading