Unable to use vendoring with golang-http

[alexellis]

Why do you need this?

I need to be able to use vendoring with the golang-http template because it is how I get private Go code into my functions.

Expected Behaviour

This should work without error, when using a vendor folder and any other stack.yml flags or build-args required:

faas-cli build

Current Behaviour

Step 29/42 : RUN ${GO} build --ldflags "-s -w" -a -installsuffix cgo -o handler .
 ---> Running in b9e5d187d314
Setting vendor mode env variables
main.go:18:2: cannot find package "github.com/openfaas/templates-sdk/go-http" in any of:
        /usr/local/go/src/github.com/openfaas/templates-sdk/go-http (from $GOROOT)
        /go/src/github.com/openfaas/templates-sdk/go-http (from $GOPATH)
The command '/bin/sh -c ${GO} build --ldflags "-s -w" -a -installsuffix cgo -o handler .' returned a non-zero code: 1
[0] < Building go1 done in 5.91s.
[0] Worker done.

Total build time: 5.91s
Errors received during build:
- [go1] received non-zero exit code from build, error: The command '/bin/sh -c ${GO} build --ldflags "-s -w" -a -installsuffix cgo -o handler .' returned a non-zero code: 1

List All Possible Solutions and Workarounds

1. Create a new separate template golang-http-vendoring that only works with vendoring
2. Stop maintaining a golang-http template at all, migrate everyone to golang-middleware
3. Use interfaces instead of structs, so that the main.go module doesn't have to reference the SDK, which seems to be the source of the problem - full example, and it's a breaking change: https://github.com/alexellis/golang-http-template
4. Consider a new direction that looks like AWS Lambda's Go handler - https://docs.aws.amazon.com/lambda/latest/dg/golang-handler.html https://docs.aws.amazon.com/lambda/latest/dg/golang-handler.html
5. Use Go's plugin capability, to build the function and entry point separately, and link them after the fact. Unfortunately this seems to require CGO and breaks portability - plugin: requires CGO_ENABLED=1 golang/go#19569

For 4), the function's handler owns the main() entrypoint instead of the hidden underlying code, and calls a utility method to set up the HTTP server etc.

package main

import (
        "fmt"
        "context"
        "github.com/openfaas/golang-http/entrypoint"
)

type MyEvent struct {
        Name string `json:"name"`
}

func HandleRequest(ctx context.Context, name MyEvent) (string, error) {
        return fmt.Sprintf("Hello %s!", name.Name ), nil
}

func main() {
        entrypoint.Start(HandleRequest)
}

For 3), the code would end up looking like this for a handler:

(this can be compared to the second code sample to see what's different)

package function

import (
    "fmt"
    "net/http"

    gohttp "github.com/alexellis/templates-sdk/go-http"
    logrus "github.com/sirupsen/logrus"
)

// Handle a function invocation
func Handle(req gohttp.Request) (gohttp.Response, error) {
    var err error

    logrus.Info("Value of body: ", string(req.GetBody()))

    message := fmt.Sprintf("Body: %s", string(req.GetBody()))

    return &gohttp.FunctionResponse{
        Body:       []byte(message),
        StatusCode: http.StatusOK,
    }, err
}

What is your preferred solution?

I like 3) and to lessen its impact to existing users, we could consider promoting golang-http into the primary community templates repository and deprecating the one we use here.

Alternatively, we could call it "go-http1.18orgolang-http1.18as a new template, withgolang-http1.19` and so on coming after that.

Steps to Reproduce (for bugs)

faas-cli template store pull golang-http-template
faas-cli new --lang golang-http  --prefix alexellis2 withlogrus

Edit withlogrus/handler.go:

package function

import (
	"fmt"
	"io"
	"net/http"

	"github.com/sirupsen/logrus"
)

func Handle(w http.ResponseWriter, r *http.Request) {
	var input []byte

	if r.Body != nil {
		defer r.Body.Close()

		body, _ := io.ReadAll(r.Body)

		input = body
	}

	logrus.Debug(fmt.Sprintf("Handle; %q", input))

	w.WriteHeader(http.StatusOK)
	w.Write([]byte(fmt.Sprintf("Body: %q", string(input))))
}

Then edit withlogrus.yml, adding:

    build_args:
      GO111MODULE: off
      GOFLAGS: "-mod=vendor"

Then try a build:

cd golang-http-template

go get
go mod vendor

cd ..
faas-cli build -f withlogrus.yml

[alexellis]

@LucasRoesler do you see any other variations for potential solutions?

[willfore]

I think 3 would be the best solution. Seems I would need to change all of my functions :(. I personally don't use vendoring at all. I prefer to simply name modules in my functions or create and import my own modules to use them.

[alexellis]

Thanks for the input Billy.

If we went ahead with 3) and didn't give it a brand new name, then there are a couple of scenarios:

A) The change for all your functions could potentially be done with a search and replace across all files in VSCode.

B) You could also use the tag or SHA of before we make the change:

configuration:
  templates:
    - name: golang-http
      source: https://github.com/openfaas/golang-http-template#VERSION/TAG/SHA

One other option for 3) is for us to keep maintaining (for 6 mo?) a branch for people who don't use private Go modules / repos, that they can access via: https://github.com/openfaas/golang-http-template#classic

[willfore]

i will just make the changes whenever needed and roll the new ones out. Maintaining a classic may be a good idea but everyone has to change anyway since it's a core go problem. With enough notice and planned upgrade it shouldn't be an issue.

[kevin-lindsay-1]

Personally I have no preference between golang-http and golang-middleware. It would probably be nice to have a guide to move over the functionality that's abstracted away, such as WithContext.

Only thing I would currently change about golang-http's SDK is to instead allow the user to manually read the body, as reading all and converting it a []byte for the user has the disadvantage of making the entire body be in memory at once.

@alexellis I made a PoC repo for a rework which makes the major build change of moving main.go into function/. The module function is now the only module, and AFAIK you need no manual editing of go modules going forward.

This iteration provides a main.go, but doesn't prevent it from being overridden by the user.

https://github.com/kevin-lindsay-1/openfaas-golang-http-rework

[alexellis]

Hi Kevin,

The approach where the function has a main is 4). It would not duplicate the entry point code as per your PoC, but reference a separate repository, i.e.

import repo/with/sdk

func handler(sdk.Request) sdk.Response {

}
main() {
  sdk.Serve(handler)
}

If you don't want to buffer the request into memory, and you have a large request, then you should use golang-middleware instead.

You don't use WithContext when using golang-middleware, you can just use the context from the http.Request object.

For 3), you can try out my template that uses an interface, it works for your use-case of vendoring private code whilst continuing to use the Request / Response style we are used to seeing in functions.

Clone this repo, and try building the withlogrus example. You'll see that it looks very much like what you're using today.

https://github.com/alexellis/golang-http-template

golang-middleware is more like a micro service handler

Alex

[kevin-lindsay-1]

If you don't want to buffer the request into memory, and you have a large request, then you should use golang-middleware instead.
You don't use WithContext when using golang-middleware, you can just use the context from the http.Request object.

Good points, and yeah it sounds like our use cases have expanded a little, and it seems like our team is growing out of golang-http. It seems to me like the abstractions that it offers are something that a given user will probably inevitably have at least 1 function that needs a little more power, like streaming the HTTP body.

This pushes me in favor of "2", I'll probably be refactoring our stuff to use middleware, just so that we aren't using multiple templates when we don't need to, lowering our surface area for bugs internally.

For 3), you can try out my template that uses an interface, it works for your use-case of vendoring private code whilst continuing to use the Request / Response style we are used to seeing in functions.

Yes, I agree, I think it would have prevented the error I was seeing with GO111MODULE=off. I do think that an approach like injecting main.go into function/ (effectively going from 2* modules to 1) is worth considering, because it makes things easier to reason about, and kind of sidesteps the issue altogether. Any "munging" of go's modules is kind of "here be dragons" territory, so difficult to reason about unless you're already familiar with how it works.

*technically, it's munged, so it's not actually 2 when the build is done.

[mrwormhole]

I found a hacky way to build but it is essentially localizing the req/res models(to be honest, I wanted to remove this intermediary a while ago by including that file into golang-http templates), I think golang-http template can exist but we need to localize github.com/alexellis/templates-sdk/go-http into that repository if that makes sense.

Example can be found here but also you may or may not be able to build successfully because "handler/handler" is not a package for the hello function

[alexellis]

Hi @mrwormhole that's similar to how the "go" classic template works. For some reason I can't remember, we moved away from that to a separate dependency. It could have been about intellisense, IDEs or unit-testing.

So this may (with additional validation) be an alternative to the interfaces that I've suggested above.

Alex

[mrwormhole]

I can understand totally why it was moved to a separate place but for golang-http template to work with go.work, I feel like we need to localize this into golang-http repository, the impact is changing all the docs for faasd examples but it will probably not impact anywhere else if go-http is used somewhere else outside of these templates.

since golang-http has its own mod file with "handler" I know intellisense won't work but we can create a go.mod file at the top perhaps and remove go.work from the highest level? need to confirm with @LucasRoesler

if there are no solutions, I guess we wait but I am pretty sure they won't support go.work with -mod=vendor flag while modules are on. The problem is workspaces don't support partial dependency fetching, if we have a single go.mod and we have dependencies there and we have a single vendor folder and some dependencies don't exist in that vendor folder, Go is smart enough to download missing dependencies and use vendor for existing dependencies, this doesn't happen when you have a go.work based project with multiple submodules. Meaning that it either uses vendors of every subfolder or goes the full download of every subfolder mode. (this ruins the private repo users experience for this specific template)

Technically, we can remove golang-http from templates and go home or provide golang-http without workspaces(shell scripts were dreaded for the end users but it worked for this golang-http template) but I do feel like there is a value there for beginners and old existing projects and I think go.work is awesome use-case even though it conflicts with build phase of templates.

Needs a discussion with @LucasRoesler

[LucasRoesler]

@alexellis and @mrwormhole

I think after a lot of time thinking on this, that just reverting to the previous behavior before workspaces makes the most sense. I am not a purist on this, if a little bash makes the template work smoothly for 99% of use-cases, then let's use some bash. Especially because the bash script is just calling some go mod edit commands vs directly editing the files, it is relatively stable.

Originally, I thought the workspaces would allow us to really remove the bash scripts, but they don't seem to work as expected.

I have prepared a branch for re-adding the bash scripts and tested it against my golang template sandbox https://github.com/LucasRoesler/golang-http-template-examples it seems to behave well. But I could be missing an important test case, so please take a look and let me know.

Here is the branch I created https://github.com/openfaas/golang-http-template/compare/master...LucasRoesler:feat-revert-to-bash-scripts?expand=1

As an aside, but something I saw in the conversation above. In general, I would expect most projects to use only one or the other template, not a mix of both. Just for code consistency in the project to ease the developer experience, I would avoid mixing and matching templates within a project. Within an organization, sure use whichever fits best for your project, but stick with just one for a project.

[mrwormhole]

@LucasRoesler @alexellis
makes sense to me, I will try it out at the weekend but could we use shell scripts part for only golang-http and keep the go.work in golang-http-middlewares?

nothing is wrong with working scripts, it is just maintenance overhead sometimes(better if it was possible to abstract away), I think go.work use case in middlewares is a top notch use case of workspaces in this project

It worked perfectly for that template and it was great thinking on that one, so I guess it is better if we only tweak the non-working one with old working scripts?

[alexellis]

It looks like we have three main options for going forward.

1. Archive the golang-http template
2. Retain workspaces, without bash scripts and use the interface change as I proposed, which will require all users to make some changes to continue using the template, along with fixes from us to documentation, blog posts, example repos and eBooks.
3. Revert to how things were prior to the introduction of workspaces and do some additional checking

I'd be interested in speed differences between 2 and 3 for an uncached container build, or one with --no-cache passed in.

Alex

[mrwormhole]

I did follow up on number 3 after giving tons of thoughts; I have followed up Lucas's feat-revert-to-bash-scripts branch and opened up this #81