Skip to content

proxy mode: operate on http responses rather than requests#29

Open
aidansteele wants to merge 3 commits intoiann0036:mainfrom
aidansteele:main
Open

proxy mode: operate on http responses rather than requests#29
aidansteele wants to merge 3 commits intoiann0036:mainfrom
aidansteele:main

Conversation

@aidansteele
Copy link
Copy Markdown

@aidansteele aidansteele commented Apr 6, 2021

I moved the proxy mode logic into the response-handling function of goproxy, so that we have access to the response status code as per the TODO.

I also extracted the AWS hostname regexp to a global variable for a (probably irrelevant) speedup

uri := req.RequestURI

body, _ := ioutil.ReadAll(req.Body)
req.Body = ioutil.NopCloser(bytes.NewBuffer(body))
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I moved the request body draining into this handleAWSRequest function to more closely match e.g. what httputil.DumpRequest does.

@iann0036
Copy link
Copy Markdown
Owner

iann0036 commented Apr 8, 2021

Looks like this isn't working based on my initial tests. Can you confirm whether it worked for you?

@aidansteele
Copy link
Copy Markdown
Author

Nice catch. I missed that because my testing didn't involve POST bodies. I've since fixed that and added a basic E2E test to avoid regression. If you add some credentials to this repo, you could have a GitHub action to exercise these tests like:

on: [push, pull_request]
name: CI
jobs:
  build:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2

      - uses: actions/setup-go@v1
        with:
          go-version: 1.16

      - name: test
        run: go test -v ./...
        env:
          AWS_REGION: ap-southeast-2
          AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
          AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}

Comment on lines +18 to +22
parseConfig()
flag.Parse()
loadMaps()
readServiceFiles()
*modeFlag = "proxy"
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is pretty ugly, but I figured I didn't want to make bigger changes in a single PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants