From f3196daa2bc6714d3a4ebeed10eac105d9f271aa Mon Sep 17 00:00:00 2001 From: Martin Ashby Date: Mon, 26 Jan 2026 13:29:17 +0000 Subject: [PATCH 1/6] Added /deregister and /list capabilities These do the obvious thing. --- main.go | 47 +++++++++++++++++++++++++++++++++++++++++++++++ main_test.go | 51 +++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 94 insertions(+), 4 deletions(-) diff --git a/main.go b/main.go index d46d144..ae46dc5 100644 --- a/main.go +++ b/main.go @@ -40,6 +40,7 @@ func errResp(resp http.ResponseWriter, e error) { type RegStorage interface { Put(string, *url.URL) error + Remove(string) error All() (map[string]*url.URL, error) } @@ -51,6 +52,10 @@ func (m *RegStorageMemory) Put(name string, url *url.URL) error { m.upstreams[name] = url return nil } +func (m *RegStorageMemory) Remove(name string) error { + delete(m.upstreams, name) + return nil +} func (m *RegStorageMemory) All() (map[string]*url.URL, error) { return maps.Clone(m.upstreams), nil } @@ -90,6 +95,14 @@ func (m *RegStorageFile) Put(name string, url *url.URL) error { mm[name] = url return m.write(mm) } +func (m *RegStorageFile) Remove(name string) error { + mm, err := m.All() + if err != nil { + return err + } + delete(mm, name) + return m.write(mm) +} func (m *RegStorageFile) write(content map[string]*url.URL) error { sb := strings.Builder{} for name, u := range content { @@ -240,6 +253,38 @@ func (p *RegProxy) register(resp http.ResponseWriter, req *http.Request) { resp.WriteHeader(204) } +func (p *RegProxy) deregister(resp http.ResponseWriter, req *http.Request) { + var q upstream + err := json.NewDecoder(req.Body).Decode(&q) + if err != nil { + badRequest(resp, err.Error()) + return + } + log.Printf("Removing upstream %v", q) + if err := p.storage.Remove(q.Name); err != nil { + errResp(resp, err) + return + } + resp.WriteHeader(204) +} + +func (p *RegProxy) list(resp http.ResponseWriter, req *http.Request) { + all, err := p.storage.All() + if err != nil { + errResp(resp, err) + return + } + var res []upstream + for k, v := range all { + res = append(res, upstream{Name: k, Callback: v.String()}) + } + resp.WriteHeader(200) + err = json.NewEncoder(resp).Encode(res) + if err != nil { + log.Printf("error writing response %v", err) + } +} + func (p *RegProxy) health(resp http.ResponseWriter, _ *http.Request) { // https://inadarei.github.io/rfc-healthcheck/ resp.WriteHeader(200) @@ -288,6 +333,8 @@ func NewRegProxy( sm := http.NewServeMux() sm.HandleFunc("/health", rp.health) sm.HandleFunc("/register", rp.register) + sm.HandleFunc("/deregister", rp.deregister) + sm.HandleFunc("/list", rp.list) sm.HandleFunc("/", rp.proxy) rp.handler = sm return rp diff --git a/main_test.go b/main_test.go index cdfc5cb..0a915e0 100644 --- a/main_test.go +++ b/main_test.go @@ -5,7 +5,6 @@ import ( "context" "encoding/json" "fmt" - "golang.org/x/sync/errgroup" "io" "io/ioutil" "math/rand" @@ -17,6 +16,8 @@ import ( "strconv" "testing" "time" + + "golang.org/x/sync/errgroup" ) func withRegProxy(t *testing.T, f func(url string, t *testing.T)) { @@ -110,6 +111,30 @@ func register(url string, u upstream, t *testing.T) { } } +func deregister(url string, u upstream, t *testing.T) { + b, _ := json.Marshal(u) + r, err := http.Post(url+"/deregister", "application/json", bytes.NewReader(b)) + if err != nil { + t.Fatal(err) + } + if r.StatusCode != 204 { + t.Fatalf("Failed to register test callback") + } +} + +func list(u string, t *testing.T) []upstream { + r, err := http.Get(u + "/list") + if err != nil { + t.Fatal(err) + } + var res []upstream + err = json.NewDecoder(r.Body).Decode(&res) + if err != nil { + t.Fatal(err) + } + return res +} + func TestHappyPath(t *testing.T) { withRegProxy(t, func(url string, t *testing.T) { // GIVEN @@ -346,15 +371,33 @@ func TestFileStorage(t *testing.T) { r, err := http.Get(srv.URL) // THEN - if r.StatusCode != 200 { - t.Errorf("expected 200, got %v", r.StatusCode) - } if err != nil { t.Fatal(err) } + if r.StatusCode != 200 { + t.Errorf("expected 200, got %v", r.StatusCode) + } _, err = io.ReadAll(r.Body) if err != nil { t.Fatal(err) } + + // Deregister a host, verify + deregister(srv.URL, upstream{Name: "bar"}, t) + lst := list(srv.URL, t) + foundFoo, foundBar := false, false + for _, l := range lst { + if l.Name == "foo" { + foundFoo = true + } else if l.Name == "bar" { + foundBar = true + } + } + if !foundFoo { + t.Errorf("list of upstreams didn't contain 'foo'") + } + if foundBar { + t.Errorf("list of upstreams contained 'bar' after we deleted it") + } } } From 4d4f886fd26098e2e89ce6f1c2c4830bf9bf375a Mon Sep 17 00:00:00 2001 From: Martin Ashby Date: Thu, 5 Feb 2026 22:11:15 +0000 Subject: [PATCH 2/6] Don't require the callback URL unnecessarily for deregister requests Fixes: https://github.com/patientsknowbest/regproxy2/pull/5#discussion_r2764790339 PHR-16102 --- build-and-push-images.sh | 2 +- main.go | 17 +++++++++++++---- main_test.go | 24 ++++++++++++------------ 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/build-and-push-images.sh b/build-and-push-images.sh index 1a07e83..2aec760 100755 --- a/build-and-push-images.sh +++ b/build-and-push-images.sh @@ -3,6 +3,6 @@ ## Pushes to PKB private repository, change the registry if you want to push it elsewhere ## Increment this number when you push a new image -VERSION=7 +VERSION=8-MFATEST2 docker build . -t "europe-docker.pkg.dev/infra-240614/eu.gcr.io/regproxy2:$VERSION" docker push "europe-docker.pkg.dev/infra-240614/eu.gcr.io/regproxy2:$VERSION" diff --git a/main.go b/main.go index ae46dc5..b962466 100644 --- a/main.go +++ b/main.go @@ -78,7 +78,7 @@ func NewRegStorageFile(fileName string) (*RegStorageFile, error) { return nil, err } for name, u := range upstreams { - ups := upstream{ + ups := registerRequest{ Name: name, Callback: u.String(), } @@ -224,13 +224,13 @@ func (p *RegProxy) proxy(resp http.ResponseWriter, req *http.Request) { _ = latestSuccess.Write(resp) } -type upstream struct { +type registerRequest struct { Name string `json:"name"` Callback string `json:"callback"` } func (p *RegProxy) register(resp http.ResponseWriter, req *http.Request) { - var q upstream + var q registerRequest err := json.NewDecoder(req.Body).Decode(&q) if err != nil { badRequest(resp, err.Error()) @@ -253,8 +253,12 @@ func (p *RegProxy) register(resp http.ResponseWriter, req *http.Request) { resp.WriteHeader(204) } +type deregisterRequest struct { + Name string `json:"name"` +} + func (p *RegProxy) deregister(resp http.ResponseWriter, req *http.Request) { - var q upstream + var q deregisterRequest err := json.NewDecoder(req.Body).Decode(&q) if err != nil { badRequest(resp, err.Error()) @@ -268,6 +272,11 @@ func (p *RegProxy) deregister(resp http.ResponseWriter, req *http.Request) { resp.WriteHeader(204) } +type upstream struct { + Name string `json:"name"` + Callback string `json:"callback"` +} + func (p *RegProxy) list(resp http.ResponseWriter, req *http.Request) { all, err := p.storage.All() if err != nil { diff --git a/main_test.go b/main_test.go index 0a915e0..639e41b 100644 --- a/main_test.go +++ b/main_test.go @@ -100,7 +100,7 @@ func TestConcurrentRegister(t *testing.T) { }) } -func register(url string, u upstream, t *testing.T) { +func register(url string, u registerRequest, t *testing.T) { b, _ := json.Marshal(u) r, err := http.Post(url+"/register", "application/json", bytes.NewReader(b)) if err != nil { @@ -111,7 +111,7 @@ func register(url string, u upstream, t *testing.T) { } } -func deregister(url string, u upstream, t *testing.T) { +func deregister(url string, u deregisterRequest, t *testing.T) { b, _ := json.Marshal(u) r, err := http.Post(url+"/deregister", "application/json", bytes.NewReader(b)) if err != nil { @@ -146,11 +146,11 @@ func TestHappyPath(t *testing.T) { testServer2 := httptest.NewServer(handler) defer testServer1.Close() defer testServer2.Close() - us1 := upstream{ + us1 := registerRequest{ Name: "foo", Callback: testServer1.URL, } - us2 := upstream{ + us2 := registerRequest{ Name: "bar", Callback: testServer2.URL, } @@ -193,11 +193,11 @@ func TestOneFail(t *testing.T) { testServer2 := httptest.NewServer(handlerErr) defer testServer1.Close() defer testServer2.Close() - us1 := upstream{ + us1 := registerRequest{ Name: "foo", Callback: testServer1.URL, } - us2 := upstream{ + us2 := registerRequest{ Name: "bar", Callback: testServer2.URL, } @@ -224,7 +224,7 @@ func TestOneFail(t *testing.T) { func TestNoSuchHost(t *testing.T) { withRegProxy(t, func(url string, t *testing.T) { // GIVEN - register(url, upstream{ + register(url, registerRequest{ Name: "foo", Callback: "http://seriously.not.a.top.level.domain", }, t) @@ -256,11 +256,11 @@ func TestTimeout(t *testing.T) { testServer2 := httptest.NewServer(handler2) defer testServer1.Close() defer testServer2.Close() - us1 := upstream{ + us1 := registerRequest{ Name: "foo", Callback: testServer1.URL, } - us2 := upstream{ + us2 := registerRequest{ Name: "bar", Callback: testServer2.URL, } @@ -296,11 +296,11 @@ func TestFileStorage(t *testing.T) { testServer2 := httptest.NewServer(handler) defer testServer1.Close() defer testServer2.Close() - us1 := upstream{ + us1 := registerRequest{ Name: "foo", Callback: testServer1.URL, } - us2 := upstream{ + us2 := registerRequest{ Name: "bar", Callback: testServer2.URL, } @@ -383,7 +383,7 @@ func TestFileStorage(t *testing.T) { } // Deregister a host, verify - deregister(srv.URL, upstream{Name: "bar"}, t) + deregister(srv.URL, deregisterRequest{Name: "bar"}, t) lst := list(srv.URL, t) foundFoo, foundBar := false, false for _, l := range lst { From 488dcf611152b955f8298625b661ebc61b688d26 Mon Sep 17 00:00:00 2001 From: Martin Ashby Date: Thu, 5 Feb 2026 22:13:35 +0000 Subject: [PATCH 3/6] Apply simpler assertion for test of /list endpoint Fixes: https://github.com/patientsknowbest/regproxy2/pull/5#discussion_r2764777470 PHR-16102 --- go.mod | 4 ++++ go.sum | 3 +++ main_test.go | 16 ++-------------- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/go.mod b/go.mod index cba70f9..710612a 100644 --- a/go.mod +++ b/go.mod @@ -4,12 +4,16 @@ go 1.22.5 require ( github.com/google/uuid v1.6.0 + github.com/stretchr/testify v1.7.0 go.mercari.io/go-dnscache v0.0.0-20210517095825-88b046eb94f2 go.uber.org/zap v1.19.1 golang.org/x/sync v0.7.0 ) require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect go.uber.org/atomic v1.9.0 // indirect go.uber.org/multierr v1.7.0 // indirect + gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect ) diff --git a/go.sum b/go.sum index e991893..a1d5405 100644 --- a/go.sum +++ b/go.sum @@ -5,8 +5,10 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= +github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= @@ -57,6 +59,7 @@ golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 h1:qIbj1fsPNlZgppZ+VLlY7N33q108Sa+fhmuc+sWQYwY= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10= gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/main_test.go b/main_test.go index 639e41b..fc0b4ba 100644 --- a/main_test.go +++ b/main_test.go @@ -17,6 +17,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "golang.org/x/sync/errgroup" ) @@ -385,19 +386,6 @@ func TestFileStorage(t *testing.T) { // Deregister a host, verify deregister(srv.URL, deregisterRequest{Name: "bar"}, t) lst := list(srv.URL, t) - foundFoo, foundBar := false, false - for _, l := range lst { - if l.Name == "foo" { - foundFoo = true - } else if l.Name == "bar" { - foundBar = true - } - } - if !foundFoo { - t.Errorf("list of upstreams didn't contain 'foo'") - } - if foundBar { - t.Errorf("list of upstreams contained 'bar' after we deleted it") - } + assert.Equal(t, []upstream{{us1.Name, us1.Callback}}, lst) } } From 278afea643cac92442dc0992afc02aadec707365 Mon Sep 17 00:00:00 2001 From: Leon York Date: Fri, 6 Feb 2026 20:03:56 +0700 Subject: [PATCH 4/6] Add tests to ensure concurrent register/deregister works --- .github/workflows/go.yml | 4 +-- Dockerfile | 2 +- README.md | 2 +- go.mod | 4 +-- main_test.go | 62 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 68 insertions(+), 6 deletions(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 5c6fed6..d6514f0 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -16,10 +16,10 @@ jobs: - name: Set up Go uses: actions/setup-go@v5 with: - go-version: 1.22.5 + go-version: 1.24.0 - name: Build run: go build - name: Test - run: go test + run: go test -race \ No newline at end of file diff --git a/Dockerfile b/Dockerfile index 1979e13..7555976 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM golang:1.22.5 AS builder +FROM golang:1.24.0 AS builder WORKDIR /build # Separate dependency caching from compilation COPY go.mod go.mod diff --git a/README.md b/README.md index 3e5eb68..0634060 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ It can be used to implement a control plane for a dynamic set of services, where errors must be highlighted to the caller rather than ignored. ## Prerequisites -* go >= 1.22 +* go >= 1.24 * OR docker ## Instructions diff --git a/go.mod b/go.mod index 710612a..cc0e06c 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module regproxy2 -go 1.22.5 +go 1.24.0 require ( github.com/google/uuid v1.6.0 @@ -16,4 +16,4 @@ require ( go.uber.org/atomic v1.9.0 // indirect go.uber.org/multierr v1.7.0 // indirect gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect -) +) \ No newline at end of file diff --git a/main_test.go b/main_test.go index fc0b4ba..4e78ea8 100644 --- a/main_test.go +++ b/main_test.go @@ -101,6 +101,68 @@ func TestConcurrentRegister(t *testing.T) { }) } +func TestConcurrentRegisterAndDeregister(t *testing.T) { + errorGroup, _ := errgroup.WithContext(t.Context()) + errorGroupDeregister, _ := errgroup.WithContext(t.Context()) + + withRegProxy(t, func(url string, t *testing.T) { + created := make(chan string, 100) + for i := range 100 { + name := fmt.Sprintf("foo-%d", i) + request := fmt.Sprintf("{\"name\":\"%s\",\"callback\":\"baz\"}", name) + errorGroup.Go(func() error { + r, err := http.Post(url+"/register", "application/json", bytes.NewReader([]byte(request))) + if err != nil { + if i == 100 { + close(created) + } + return err + } + if r.StatusCode != 204 { + if i == 100 { + close(created) + } + return fmt.Errorf("Wrong status code from /register %d expected 204", r.StatusCode) + } + created <- name + + if i == 100 { + close(created) + } + return nil + }) + } + + go func() { + for name := range created { + request := fmt.Sprintf("{\"name\":\"%s\"}", name) + + errorGroupDeregister.Go(func() error { + r, err := http.Post(url+"/deregister", "application/json", bytes.NewReader([]byte(request))) + if err != nil { + return err + } + if r.StatusCode != 204 { + return fmt.Errorf("Wrong status code from /deregister %d expected 204", r.StatusCode) + } + + return nil + }) + } + }() + + resultingError := errorGroup.Wait() + if resultingError != nil { + t.Fatal(resultingError) + } + + resultingError = errorGroupDeregister.Wait() + if resultingError != nil { + t.Fatal(resultingError) + } + }) +} + func register(url string, u registerRequest, t *testing.T) { b, _ := json.Marshal(u) r, err := http.Post(url+"/register", "application/json", bytes.NewReader(b)) From 4b4873d6b538e7a9da0e1db7d8833e51e380b5ac Mon Sep 17 00:00:00 2001 From: Martin Ashby Date: Fri, 6 Feb 2026 16:56:35 +0000 Subject: [PATCH 5/6] Fix missing lock/unlock in deregister implementation Fixes: https://github.com/patientsknowbest/regproxy2/pull/5#discussion_r2774085175 --- main.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/main.go b/main.go index b962466..0d9d7d9 100644 --- a/main.go +++ b/main.go @@ -264,6 +264,8 @@ func (p *RegProxy) deregister(resp http.ResponseWriter, req *http.Request) { badRequest(resp, err.Error()) return } + p.writeLock.Lock() + defer p.writeLock.Unlock() log.Printf("Removing upstream %v", q) if err := p.storage.Remove(q.Name); err != nil { errResp(resp, err) @@ -278,6 +280,8 @@ type upstream struct { } func (p *RegProxy) list(resp http.ResponseWriter, req *http.Request) { + p.writeLock.Lock() + defer p.writeLock.Unlock() all, err := p.storage.All() if err != nil { errResp(resp, err) From 920896086aeb16995ccc54dff39b72d888c59804 Mon Sep 17 00:00:00 2001 From: Martin Ashby Date: Mon, 9 Feb 2026 14:53:05 +0000 Subject: [PATCH 6/6] update version to 8 --- build-and-push-images.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build-and-push-images.sh b/build-and-push-images.sh index 2aec760..5595a68 100755 --- a/build-and-push-images.sh +++ b/build-and-push-images.sh @@ -3,6 +3,6 @@ ## Pushes to PKB private repository, change the registry if you want to push it elsewhere ## Increment this number when you push a new image -VERSION=8-MFATEST2 +VERSION=8 docker build . -t "europe-docker.pkg.dev/infra-240614/eu.gcr.io/regproxy2:$VERSION" docker push "europe-docker.pkg.dev/infra-240614/eu.gcr.io/regproxy2:$VERSION"