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

feat: use test host name for marker if localhost alias #370

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion runner/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package runner
import (
"errors"
"fmt"
"net"
"time"

schema "github.com/coreruleset/ftw-tests-schema/v2/types"
Expand Down Expand Up @@ -221,6 +222,8 @@ func RunStage(runContext *TestRunContext, ftwCheck *check.FTWCheck, testCase sch
}

func markAndFlush(runContext *TestRunContext, dest *ftwhttp.Destination, stageID string) ([]byte, error) {
host := "localhost"

rline := &ftwhttp.RequestLine{
Method: "GET",
// Use the `/status` endpoint of `httpbin` (http://httpbingo.org), if possible,
Expand All @@ -230,10 +233,19 @@ func markAndFlush(runContext *TestRunContext, dest *ftwhttp.Destination, stageID
Version: "HTTP/1.1",
}

// check if destination host is an alias for localhost
addrs, err := net.LookupHost(dest.DestAddr)
if err != nil {
return nil, fmt.Errorf("ftw/run: can't resolve destination %+v: %w", dest, err)
}
Comment on lines +238 to +240
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know if this even make sense. Maybe just ignoring the error and moving on?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it does. Running the test makes no sense because resolution will fail again.

// we only take the first IP address
if net.ParseIP(addrs[0]).IsLoopback() {
host = dest.DestAddr
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the comment, I think this should be set to the host entry of the current stage. The Host header will be the deciding factor for choosing the location / virtual host, so if we change the host here to mylocalhostalias but the test request has a host name of anotheralias, the two requests will end up in different locations / virtual hosts.

Suggested change
host = dest.DestAddr
// With multiple locations / virtual hosts, logs can be written to different files,
// in which case the internal requests for marking and flushing must end up in the
// same log file (host name match).
host = dest.DestAddr

}
headers := &ftwhttp.Header{
"Accept": "*/*",
"User-Agent": "go-ftw test agent",
"Host": "localhost",
"Host": host,
runContext.Config.LogMarkerHeaderName: stageID,
}

Expand Down