From 00e8ab66e5b3d68d64e64d6fcfa72d478025bef4 Mon Sep 17 00:00:00 2001 From: MikelAlejoBR Date: Tue, 5 May 2026 09:19:27 -0400 Subject: [PATCH 01/10] refactor: replace Gin framework with Echo The registration service used both the Echo and Gin frameworks to provide its services, and we wanted to remove the dependency with Gin and just keep Echo to improve maintainability. Assited-by: claude-4.6 with Cursor SANDBOX-316 --- go.mod | 18 -- go.sum | 84 --------- pkg/application/service/services.go | 12 +- pkg/assets/assets.go | 22 +-- pkg/context/keys.go | 14 ++ pkg/controller/analytics.go | 10 +- pkg/controller/analytics_test.go | 19 +- pkg/controller/authconfig.go | 6 +- pkg/controller/authconfig_test.go | 13 +- pkg/controller/health_check.go | 20 +-- pkg/controller/health_check_test.go | 49 +++-- pkg/controller/namespaces.go | 22 +-- pkg/controller/namespaces_test.go | 40 ++--- pkg/controller/signup.go | 92 +++++----- pkg/controller/signup_test.go | 169 ++++++++---------- pkg/controller/uiconfig.go | 8 +- pkg/controller/uiconfig_test.go | 16 +- pkg/controller/usernames.go | 16 +- pkg/controller/usernames_test.go | 40 +++-- pkg/errors/error.go | 6 +- pkg/errors/error_test.go | 11 +- pkg/log/log.go | 31 ++-- pkg/log/log_test.go | 46 +++-- pkg/middleware/jwt_middleware.go | 88 ++++----- pkg/middleware/promhttp_middleware.go | 52 +++--- pkg/namespaces/namespaces_manager.go | 14 +- pkg/namespaces/namespaces_manager_test.go | 8 +- pkg/proxy/handlers/spacelister.go | 4 +- pkg/proxy/handlers/spacelister_get_test.go | 9 +- pkg/proxy/handlers/spacelister_list_test.go | 5 +- pkg/proxy/proxy_test.go | 6 +- pkg/server/metrics_server.go | 16 +- pkg/server/routes.go | 25 +-- pkg/server/routes_test.go | 25 +-- pkg/server/server.go | 86 ++++----- pkg/server/server_test.go | 2 +- pkg/signup/service/signup_service.go | 93 +++++----- pkg/signup/service/signup_service_test.go | 128 ++++++++----- pkg/signup/signup.go | 4 +- pkg/signup/socialevent.go | 6 +- pkg/signup/socialevent_test.go | 8 +- pkg/verification/captcha/captcha.go | 8 +- pkg/verification/sender/amazon_sns_sender.go | 4 +- pkg/verification/sender/sender.go | 4 +- pkg/verification/sender/twilio_sender.go | 4 +- pkg/verification/sender/twilio_sender_test.go | 12 +- .../service/verification_service.go | 10 +- .../service/verification_service_test.go | 47 ++--- test/fake/proxy.go | 6 +- 49 files changed, 673 insertions(+), 765 deletions(-) diff --git a/go.mod b/go.mod index 8475fba3..e177f5c2 100644 --- a/go.mod +++ b/go.mod @@ -21,10 +21,6 @@ require ( require ( cloud.google.com/go/recaptchaenterprise/v2 v2.13.0 - github.com/gin-contrib/cors v1.6.0 - github.com/gin-contrib/gzip v0.0.6 - github.com/gin-contrib/static v0.0.1 - github.com/gin-gonic/gin v1.9.1 github.com/golang-jwt/jwt/v5 v5.2.2 github.com/google/uuid v1.6.0 github.com/kevinburke/twilio-go v0.0.0-20220922200631-8f3f155dfe1f @@ -54,10 +50,7 @@ require ( github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/blang/semver/v4 v4.0.0 // indirect - github.com/bytedance/sonic v1.11.2 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect - github.com/chenzhuoyu/base64x v0.0.0-20230717121745-296ad89f973d // indirect - github.com/chenzhuoyu/iasm v0.9.1 // indirect github.com/cloudflare/circl v1.6.3 // indirect github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0 // indirect github.com/emicklei/go-restful/v3 v3.11.0 // indirect @@ -65,7 +58,6 @@ require ( github.com/exponent-io/jsonpath v0.0.0-20210407135951-1de76d718b3f // indirect github.com/fsnotify/fsnotify v1.7.0 // indirect github.com/fxamacker/cbor/v2 v2.7.0 // indirect - github.com/gabriel-vasile/mimetype v1.4.3 // indirect github.com/go-errors/errors v1.4.2 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect @@ -80,7 +72,6 @@ require ( github.com/googleapis/gax-go/v2 v2.12.3 // indirect github.com/gorilla/mux v1.8.1 // indirect github.com/huandu/xstrings v1.3.3 // indirect - github.com/klauspost/cpuid/v2 v2.2.7 // indirect github.com/kylelemons/godebug v1.1.0 // indirect github.com/lestrrat-go/backoff/v2 v2.0.8 // indirect github.com/lestrrat-go/blackmagic v1.0.2 // indirect @@ -96,7 +87,6 @@ require ( github.com/redhat-cop/operator-utils v1.3.8 // indirect github.com/shopspring/decimal v1.2.0 // indirect github.com/spf13/cast v1.3.1 // indirect - github.com/twitchyliquid64/golang-asm v0.15.1 // indirect github.com/valyala/bytebufferpool v1.0.0 // indirect github.com/valyala/fasttemplate v1.2.2 // indirect github.com/x448/float16 v0.8.4 // indirect @@ -107,7 +97,6 @@ require ( go.opentelemetry.io/otel v1.39.0 // indirect go.opentelemetry.io/otel/metric v1.39.0 // indirect go.opentelemetry.io/otel/trace v1.39.0 // indirect - golang.org/x/arch v0.7.0 // indirect gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect google.golang.org/api v0.177.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20251202230838-ff82c1b0f217 // indirect @@ -126,14 +115,10 @@ require ( require ( github.com/cheekybits/is v0.0.0-20150225183255-68e9c0620927 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect - github.com/gin-contrib/sse v0.1.0 // indirect github.com/go-logr/zapr v1.3.0 // indirect github.com/go-openapi/jsonpointer v0.21.0 // indirect github.com/go-openapi/jsonreference v0.20.2 // indirect github.com/go-openapi/swag v0.23.0 // indirect - github.com/go-playground/locales v0.14.1 // indirect - github.com/go-playground/universal-translator v0.18.1 // indirect - github.com/go-playground/validator/v10 v10.19.0 // indirect github.com/goccy/go-json v0.10.2 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/google/go-cmp v0.7.0 // indirect @@ -144,7 +129,6 @@ require ( github.com/json-iterator/go v1.1.12 // indirect github.com/kevinburke/go-types v0.0.0-20210723172823-2deba1f80ba7 // indirect github.com/kevinburke/rest v0.0.0-20210506044642-5611499aa33c // indirect - github.com/leodido/go-urn v1.4.0 // indirect github.com/lestrrat-go/jwx v1.2.29 // indirect github.com/mailru/easyjson v0.7.7 // indirect github.com/mattn/go-isatty v0.0.20 // indirect @@ -153,11 +137,9 @@ require ( github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect // using latest commit from 'github.com/openshift/api branch release-4.20' github.com/openshift/api v0.0.0-20251202204302-1cb53e34ca33 - github.com/pelletier/go-toml/v2 v2.1.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/ttacon/builder v0.0.0-20170518171403-c099f663e1c2 // indirect github.com/ttacon/libphonenumber v1.2.1 // indirect - github.com/ugorji/go/codec v1.2.12 // indirect go.uber.org/atomic v1.10.0 go.uber.org/multierr v1.11.0 // indirect golang.org/x/crypto v0.46.0 // indirect diff --git a/go.sum b/go.sum index bcb8a70f..4b43796e 100644 --- a/go.sum +++ b/go.sum @@ -27,22 +27,11 @@ github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ= github.com/buger/jsonparser v1.1.1/go.mod h1:6RYKKt7H4d4+iWqouImQ9R2FZql3VbhNgx27UK13J/0= github.com/bwesterb/go-ristretto v1.2.0/go.mod h1:fUIoIZaG73pV5biE2Blr2xEzDoMj7NFEuV9ekS419A0= -github.com/bytedance/sonic v1.5.0/go.mod h1:ED5hyg4y6t3/9Ku1R6dU/4KyJ48DZ4jPhfY1O2AihPM= -github.com/bytedance/sonic v1.10.0-rc/go.mod h1:ElCzW+ufi8qKqNW0FY314xriJhyJhuoJ3gFZdAHF7NM= -github.com/bytedance/sonic v1.11.2 h1:ywfwo0a/3j9HR8wsYGWsIWl2mvRsI950HyoxiBERw5A= -github.com/bytedance/sonic v1.11.2/go.mod h1:iZcSUejdk5aukTND/Eu/ivjQuEL0Cu9/rf50Hi0u/g4= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/cheekybits/is v0.0.0-20150225183255-68e9c0620927 h1:SKI1/fuSdodxmNNyVBR8d7X/HuLnRpvvFO0AgyQk764= github.com/cheekybits/is v0.0.0-20150225183255-68e9c0620927/go.mod h1:h/aW8ynjgkuj+NQRlZcDbAbM1ORAbXjXX77sX7T289U= -github.com/chenzhuoyu/base64x v0.0.0-20211019084208-fb5309c8db06/go.mod h1:DH46F32mSOjUmXrMHnKwZdA8wcEefY7UVqBKYGjpdQY= -github.com/chenzhuoyu/base64x v0.0.0-20221115062448-fe3a3abad311/go.mod h1:b583jCggY9gE99b6G5LEC39OIiVsWj+R97kbl5odCEk= -github.com/chenzhuoyu/base64x v0.0.0-20230717121745-296ad89f973d h1:77cEq6EriyTZ0g/qfRdp61a3Uu/AWrgIq2s0ClJV1g0= -github.com/chenzhuoyu/base64x v0.0.0-20230717121745-296ad89f973d/go.mod h1:8EPpVsBuRksnlj1mLy4AWzRNQYxauNi62uWcE3to6eA= -github.com/chenzhuoyu/iasm v0.9.0/go.mod h1:Xjy2NpN3h7aUqeqM+woSuuvxmIe6+DDsiNLIrkAmYog= -github.com/chenzhuoyu/iasm v0.9.1 h1:tUHQJXo3NhBqw6s33wkGn9SP3bvrWLdlVIJ3hQBL7P0= -github.com/chenzhuoyu/iasm v0.9.1/go.mod h1:Xjy2NpN3h7aUqeqM+woSuuvxmIe6+DDsiNLIrkAmYog= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= github.com/cloudflare/circl v1.1.0/go.mod h1:prBCrKB9DV4poKZY1l9zBXg2QJY7mvgRvtMxxK7fi4I= github.com/cloudflare/circl v1.6.3 h1:9GPOhQGF9MCYUeXyMYlqTR6a5gTrgR/fBLXvUgtVcg8= @@ -83,20 +72,6 @@ github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nos github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM= github.com/fxamacker/cbor/v2 v2.7.0 h1:iM5WgngdRBanHcxugY4JySA0nk1wZorNOpTgCMedv5E= github.com/fxamacker/cbor/v2 v2.7.0/go.mod h1:pxXPTn3joSm21Gbwsv0w9OSA2y1HFR9qXEeXQVeNoDQ= -github.com/gabriel-vasile/mimetype v1.4.3 h1:in2uUcidCuFcDKtdcBxlR0rJ1+fsokWf+uqxgUFjbI0= -github.com/gabriel-vasile/mimetype v1.4.3/go.mod h1:d8uq/6HKRL6CGdk+aubisF/M5GcPfT7nKyLpA0lbSSk= -github.com/gin-contrib/cors v1.6.0 h1:0Z7D/bVhE6ja07lI8CTjTonp6SB07o8bNuFyRbsBUQg= -github.com/gin-contrib/cors v1.6.0/go.mod h1:cI+h6iOAyxKRtUtC6iF/Si1KSFvGm/gK+kshxlCi8ro= -github.com/gin-contrib/gzip v0.0.6 h1:NjcunTcGAj5CO1gn4N8jHOSIeRFHIbn51z6K+xaN4d4= -github.com/gin-contrib/gzip v0.0.6/go.mod h1:QOJlmV2xmayAjkNS2Y8NQsMneuRShOU/kjovCXNuzzk= -github.com/gin-contrib/sse v0.1.0 h1:Y/yl/+YNO8GZSjAhjMsSuLt29uWRFHdHYUb5lYOV9qE= -github.com/gin-contrib/sse v0.1.0/go.mod h1:RHrZQHXnP2xjPF+u1gW/2HnVO7nvIa9PG3Gm+fLHvGI= -github.com/gin-contrib/static v0.0.1 h1:JVxuvHPuUfkoul12N7dtQw7KRn/pSMq7Ue1Va9Swm1U= -github.com/gin-contrib/static v0.0.1/go.mod h1:CSxeF+wep05e0kCOsqWdAWbSszmc31zTIbD8TvWl7Hs= -github.com/gin-gonic/gin v1.6.3/go.mod h1:75u5sXoLsGZoRN5Sgbi1eraJ4GU3++wFwWzhwvtwp4M= -github.com/gin-gonic/gin v1.8.1/go.mod h1:ji8BvRH1azfM+SYow9zQ6SZMvR8qOMZHmsCuWR9tTTk= -github.com/gin-gonic/gin v1.9.1 h1:4idEAncQnU5cB7BeOkPtxjfCSye0AAm1R0RVIqJ+Jmg= -github.com/gin-gonic/gin v1.9.1/go.mod h1:hPrL7YrpYKXt5YId3A/Tnip5kqbEAP+KLuI3SUcPTeU= github.com/go-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA= github.com/go-errors/errors v1.4.2/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3BopGUQ5a5Og= github.com/go-kit/log v0.2.1/go.mod h1:NwTd00d/i8cPZ3xOwwiv2PO5MOcx78fFErGNcVmBjv0= @@ -117,24 +92,8 @@ github.com/go-openapi/jsonreference v0.20.2/go.mod h1:Bl1zwGIM8/wsvqjsOQLJ/SH+En github.com/go-openapi/swag v0.22.3/go.mod h1:UzaqsxGiab7freDnrUUra0MwWfN/q7tE4j+VcZ0yl14= github.com/go-openapi/swag v0.23.0 h1:vsEVJDUo2hPJ2tu0/Xc+4noaxyEffXNIs3cOULZ+GrE= github.com/go-openapi/swag v0.23.0/go.mod h1:esZ8ITTYEsH1V2trKHjAN8Ai7xHb8RV+YSZ577vPjgQ= -github.com/go-playground/assert/v2 v2.0.1/go.mod h1:VDjEfimB/XKnb+ZQfWdccd7VUvScMdVu0Titje2rxJ4= -github.com/go-playground/assert/v2 v2.2.0 h1:JvknZsQTYeFEAhQwI4qEt9cyV5ONwRHC+lYKSsYSR8s= -github.com/go-playground/assert/v2 v2.2.0/go.mod h1:VDjEfimB/XKnb+ZQfWdccd7VUvScMdVu0Titje2rxJ4= -github.com/go-playground/locales v0.13.0/go.mod h1:taPMhCMXrRLJO55olJkUXHZBHCxTMfnGwq/HNwmWNS8= -github.com/go-playground/locales v0.14.0/go.mod h1:sawfccIbzZTqEDETgFXqTho0QybSa7l++s0DH+LDiLs= -github.com/go-playground/locales v0.14.1 h1:EWaQ/wswjilfKLTECiXz7Rh+3BjFhfDFKv/oXslEjJA= -github.com/go-playground/locales v0.14.1/go.mod h1:hxrqLVvrK65+Rwrd5Fc6F2O76J/NuW9t0sjnWqG1slY= -github.com/go-playground/universal-translator v0.17.0/go.mod h1:UkSxE5sNxxRwHyU+Scu5vgOQjsIJAF8j9muTVoKLVtA= -github.com/go-playground/universal-translator v0.18.0/go.mod h1:UvRDBj+xPUEGrFYl+lu/H90nyDXpg0fqeB/AQUGNTVA= -github.com/go-playground/universal-translator v0.18.1 h1:Bcnm0ZwsGyWbCzImXv+pAJnYK9S473LQFuzCbDbfSFY= -github.com/go-playground/universal-translator v0.18.1/go.mod h1:xekY+UJKNuX9WP91TpwSH2VMlDf28Uj24BCp08ZFTUY= -github.com/go-playground/validator/v10 v10.2.0/go.mod h1:uOYAAleCW8F/7oMFd6aG0GOhaH6EGOAJShg8Id5JGkI= -github.com/go-playground/validator/v10 v10.10.0/go.mod h1:74x4gJWsvQexRdW8Pn3dXSGrTK4nAUsbPlLADvpJkos= -github.com/go-playground/validator/v10 v10.19.0 h1:ol+5Fu+cSq9JD7SoSqe04GMI92cbn0+wvQ3bZ8b/AU4= -github.com/go-playground/validator/v10 v10.19.0/go.mod h1:dbuPbCMFw/DrkbEynArYaCwl3amGuJotoKCe95atGMM= github.com/go-task/slim-sprig/v3 v3.0.0 h1:sUs3vkvUymDpBKi3qH1YSqBQk9+9D/8M2mN1vB6EwHI= github.com/go-task/slim-sprig/v3 v3.0.0/go.mod h1:W848ghGpv3Qj3dhTPRyJypKRiqCdHZiAzKg9hl15HA8= -github.com/goccy/go-json v0.9.7/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= github.com/goccy/go-json v0.10.2 h1:CrxCmQqYDkv1z7lO7Wbh2HN93uovUHgrECaO5ZrCXAU= github.com/goccy/go-json v0.10.2/go.mod h1:6MelG93GURQebXPDq3khkgXZkazVtN9CRI+MGFi0w8I= github.com/gofrs/uuid v4.2.0+incompatible h1:yyYWMnhkhrKwwr8gAOcOCYxOOscHgDS9yZgBrnJfGa0= @@ -151,7 +110,6 @@ github.com/golang/mock v1.1.1/go.mod h1:oTYuIxOrZwtPieC+H1uAHpcLFnEyAGVDL/k47Jfb github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= -github.com/golang/protobuf v1.3.3/go.mod h1:vzj43D7+SQXF/4pzW/hwtAqwc6iTitCiVSaWz5lYuqw= github.com/golang/protobuf v1.4.0-rc.1/go.mod h1:ceaxUfeHdC40wWswd/P6IGgMaK3YpKi5j83Wpe3EHw8= github.com/golang/protobuf v1.4.0-rc.1.0.20200221234624-67d41d38c208/go.mod h1:xKAWHe0F5eneWXFV3EuXVDTCmh+JuBKY0li0aMyXATA= github.com/golang/protobuf v1.4.0-rc.2/go.mod h1:LlEzMj4AhA7rCAGe4KMBDvJI+AwstrUpVNzEA03Pprs= @@ -216,7 +174,6 @@ github.com/jmespath/go-jmespath/internal/testify v1.5.1 h1:shLQSRRSCCPj3f2gpwzGw github.com/jmespath/go-jmespath/internal/testify v1.5.1/go.mod h1:L3OGu8Wl2/fWfCI6z80xFu9LTZmf1ZRjMHUOPmWr69U= github.com/josharian/intern v1.0.0 h1:vlS4z54oSdjm0bgjRigI+G1HpF+tI+9rE5LLzOg8HmY= github.com/josharian/intern v1.0.0/go.mod h1:5DoeVV0s6jJacbCEi61lwdGj/aVlrQvzHFFd8Hwg//Y= -github.com/json-iterator/go v1.1.9/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/uOdHXbAo4= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= github.com/kevinburke/go-types v0.0.0-20210723172823-2deba1f80ba7 h1:K8qael4LemsmJCGt+ccI8b0fCNFDttmEu3qtpFt3G0M= @@ -229,13 +186,7 @@ github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zttxdo= github.com/klauspost/compress v1.18.0/go.mod h1:2Pp+KzxcywXVXMr50+X0Q/Lsb43OQHYWRCY2AiWywWQ= -github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= -github.com/klauspost/cpuid/v2 v2.2.7 h1:ZWSB3igEs+d0qvnxR/ZBzXVmxkgt8DdzP6m9pfuVLDM= -github.com/klauspost/cpuid/v2 v2.2.7/go.mod h1:Lcz8mBdAVJIBVzewtcLocK12l3Y+JytZYpaMropDUws= -github.com/knz/go-libedit v1.10.1/go.mod h1:MZTVkCWyz0oBc7JOWP3wNAzd002ZbM/5hgShxwh4x8M= -github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= -github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= @@ -248,10 +199,6 @@ github.com/labstack/echo/v4 v4.13.3 h1:pwhpCPrTl5qry5HRdM5FwdXnhXSLSY+WE+YQSeCaa github.com/labstack/echo/v4 v4.13.3/go.mod h1:o90YNEeQWjDozo584l7AwhJMHN0bOC4tAfg+Xox9q5g= github.com/labstack/gommon v0.4.2 h1:F8qTUNXgG1+6WQmqoUWnz8WiEU60mXVVw0P4ht1WRA0= github.com/labstack/gommon v0.4.2/go.mod h1:QlUFxVM+SNXhDL/Z7YhocGIBYOiwB0mXm1+1bAPHPyU= -github.com/leodido/go-urn v1.2.0/go.mod h1:+8+nEpDfqqsY+g338gtMEUOtuK+4dEMhiQEgxpxOKII= -github.com/leodido/go-urn v1.2.1/go.mod h1:zt4jvISO2HfUBqxjfIshjdMTYS56ZS/qv49ictyFfxY= -github.com/leodido/go-urn v1.4.0 h1:WT9HwE9SGECu3lg4d/dIA+jxlljEa1/ffXKmRjqdmIQ= -github.com/leodido/go-urn v1.4.0/go.mod h1:bvxc+MVxLKB4z00jd1z+Dvzr47oO32F/QSNjSBOlFxI= github.com/lestrrat-go/backoff/v2 v2.0.8 h1:oNb5E5isby2kiro9AgdHLv5N5tint1AnDVVf2E2un5A= github.com/lestrrat-go/backoff/v2 v2.0.8/go.mod h1:rHP/q/r9aT27n24JQLa7JhSQZCKBBOiM/uP402WwN8Y= github.com/lestrrat-go/blackmagic v1.0.2 h1:Cg2gVSc9h7sz9NOByczrbUvLopQmXrfFx//N+AkAr5k= @@ -273,8 +220,6 @@ github.com/matryer/resync v0.0.0-20161211202428-d39c09a11215 h1:hDa3vAq/Zo5gjfJ4 github.com/matryer/resync v0.0.0-20161211202428-d39c09a11215/go.mod h1:LH+NgPY9AJpDfqAFtzyer01N9MYNsAKUf3DC9DV1xIY= github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= -github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU= -github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94= github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= @@ -287,7 +232,6 @@ github.com/mitchellh/reflectwalk v1.0.0/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= -github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742/go.mod h1:bx2lNnkwVCuqBIxFjflWJWanXIb3RllmbCylyMrvgv0= github.com/modern-go/reflect2 v1.0.2 h1:xBagoLtFs94CBntxluKeaWgTMpvLxC4ur3nMaC9Gz0M= github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjYzDa0/r8luk= github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 h1:n6/2gBQ3RWajuToeY6ZtZTIKv2v7ThUy5KKusIT0yc0= @@ -307,10 +251,6 @@ github.com/openshift/api v0.0.0-20251202204302-1cb53e34ca33 h1:/Be2oa7aOPE60B7AH github.com/openshift/api v0.0.0-20251202204302-1cb53e34ca33/go.mod h1:SPLf21TYPipzCO67BURkCfK6dcIIxx0oNRVWaOyRcXM= github.com/openshift/library-go v0.0.0-20251110200504-2685cf1242fc h1:g9BJ/p4ZLgb253FwnWvWEzl2vYSDSvvUZ6s42weVKpM= github.com/openshift/library-go v0.0.0-20251110200504-2685cf1242fc/go.mod h1:tptKNust9MdRI0p90DoBSPHIrBa9oh+Rok59tF0vT8c= -github.com/pelletier/go-toml/v2 v2.0.1/go.mod h1:r9LEWfGN8R5k0VXJ+0BkIe7MYkRdwZOjgMj2KwnJFUo= -github.com/pelletier/go-toml/v2 v2.1.1 h1:LWAJwfNvjQZCFIDKWYQaM62NcYeYViCmWIwmOStowAI= -github.com/pelletier/go-toml/v2 v2.1.1/go.mod h1:tJU2Z3ZkXwnxa4DPO899bsyIoywizdUvyaeZurnPPDc= -github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 h1:GFCKgmp0tecUJ0sJuv4pzYCqS9+RGSn52M3FUwPs+uo= @@ -329,8 +269,6 @@ github.com/prometheus/procfs v0.15.1 h1:YagwOFzUgYfKKHX6Dr+sHT7km/hxC76UB0leargg github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoGhij/e3PBqk= github.com/redhat-cop/operator-utils v1.3.8 h1:xhoMBg2snSzNdcxT53lSBr7PRXxrzP1cDi51NPBLaT4= github.com/redhat-cop/operator-utils v1.3.8/go.mod h1:s4R0YY8lVlHkC78GLV20PPuZmywjSbTwZKCHwWUQ3P8= -github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= -github.com/rogpeppe/go-internal v1.8.0/go.mod h1:WmiCO8CzOY8rg0OYDC4/i/2WRWAB6poM+XZ2dLUbcbE= github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ= github.com/rogpeppe/go-internal v1.14.1/go.mod h1:MaRKkUm5W0goXpeCfT7UZI6fk/L7L7so1lCWt35ZSgc= github.com/sergi/go-diff v1.2.0 h1:XU+rvMAioB0UC3q1MFrIQy4Vo5/4VsRDQQXHsEya6xQ= @@ -348,7 +286,6 @@ github.com/stretchr/objx v0.5.2 h1:xuMeJ0Sdp5ZMRXx/aWO6RZxdr3beISkG5/G/aIRr3pY= github.com/stretchr/objx v0.5.2/go.mod h1:FRsXN1f5AsAjCGJKqEizvkpNtU+EGNCLh3NxZ/8L+MA= github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= -github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= @@ -363,14 +300,6 @@ github.com/ttacon/builder v0.0.0-20170518171403-c099f663e1c2 h1:5u+EJUQiosu3JFX0 github.com/ttacon/builder v0.0.0-20170518171403-c099f663e1c2/go.mod h1:4kyMkleCiLkgY6z8gK5BkI01ChBtxR0ro3I1ZDcGM3w= github.com/ttacon/libphonenumber v1.2.1 h1:fzOfY5zUADkCkbIafAed11gL1sW+bJ26p6zWLBMElR4= github.com/ttacon/libphonenumber v1.2.1/go.mod h1:E0TpmdVMq5dyVlQ7oenAkhsLu86OkUl+yR4OAxyEg/M= -github.com/twitchyliquid64/golang-asm v0.15.1 h1:SU5vSMR7hnwNxj24w34ZyCi/FmDZTkS4MhqMhdFk5YI= -github.com/twitchyliquid64/golang-asm v0.15.1/go.mod h1:a1lVb/DtPvCB8fslRZhAngC2+aY1QWCk3Cedj/Gdt08= -github.com/ugorji/go v1.1.7/go.mod h1:kZn38zHttfInRq0xu/PH0az30d+z6vm202qpg1oXVMw= -github.com/ugorji/go v1.2.7/go.mod h1:nF9osbDWLy6bDVv/Rtoh6QgnvNDpmCalQV5urGCCS6M= -github.com/ugorji/go/codec v1.1.7/go.mod h1:Ax+UKWsSmolVDwsd+7N3ZtXu+yMGCf907BLYF3GoBXY= -github.com/ugorji/go/codec v1.2.7/go.mod h1:WGN1fab3R1fzQlVQTkfxVtIBhWDRqOviHU95kRgeqEY= -github.com/ugorji/go/codec v1.2.12 h1:9LC83zGrHhuUA9l16C9AHXAqEV/2wBQ4nkvumAE65EE= -github.com/ugorji/go/codec v1.2.12/go.mod h1:UNopzCgEMSXjBc6AOMqYvWC1ktqTAfzJZUZgYf6w6lg= github.com/valyala/bytebufferpool v1.0.0 h1:GqA5TC/0021Y/b9FG4Oi9Mr3q7XYx6KllzawFIhcdPw= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= github.com/valyala/fasttemplate v1.2.2 h1:lxLXG0uE3Qnshl9QyaK6XJxMXlQZELvChBOCmQD0Loo= @@ -406,13 +335,9 @@ go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8= go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E= -golang.org/x/arch v0.0.0-20210923205945-b76863e36670/go.mod h1:5om86z9Hs0C8fWVUuoMHwpExlXzs5Tkyp9hOrfG7pp8= -golang.org/x/arch v0.7.0 h1:pskyeJh/3AmoQ8CPE95vxHLqp1G1GfGNXTmcl9NEKTc= -golang.org/x/arch v0.7.0/go.mod h1:FEVrYAQjsQXMVJ1nsMoVVXPZg6p2JE2mx8psSWTDQys= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= -golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.3.0/go.mod h1:hebNnKkNXi2UzZN1eVRvBB7co0a+JxK6XbPiWVs/3J4= golang.org/x/crypto v0.7.0/go.mod h1:pYwdfH91IfpZVANVyUOhSIPZaFoJGxTFbZhFTx+dXZU= @@ -465,12 +390,9 @@ golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20210806184541-e5e7981a1069/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211007075335-d3039528d8ac/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= @@ -499,7 +421,6 @@ golang.org/x/term v0.38.0/go.mod h1:bSEAKrOT1W+VSu9TSCMtoGEOUcKxOKgl3LE5QEF/xVg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= @@ -564,10 +485,8 @@ google.golang.org/protobuf v1.28.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqw google.golang.org/protobuf v1.36.10 h1:AYd7cD/uASjIL6Q9LiTjz8JLcrh/88q5UObnmY3aOOE= google.golang.org/protobuf v1.36.10/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= -gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/evanphx/json-patch.v4 v4.12.0 h1:n6jtcsulIzXPJaxegRbvFNNrZDjbij7ny3gmSPG+6V4= gopkg.in/evanphx/json-patch.v4 v4.12.0/go.mod h1:p8EYWUEYMpynmqDbY58zCKCFZw8pRWMG4EsWvDvM72M= gopkg.in/go-jose/go-jose.v2 v2.6.3 h1:nt80fvSDlhKWQgSWyHyy5CfmlQr+asih51R8PTWNKKs= @@ -582,7 +501,6 @@ gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo= @@ -609,8 +527,6 @@ k8s.io/kubectl v0.33.4 h1:nXEI6Vi+oB9hXxoAHyHisXolm/l1qutK3oZQMak4N98= k8s.io/kubectl v0.33.4/go.mod h1:Xe7P9X4DfILvKmlBsVqUtzktkI56lEj22SJW7cFy6nE= k8s.io/utils v0.0.0-20241210054802-24370beab758 h1:sdbE21q2nlQtFh65saZY+rRM6x6aJJI8IUa1AmH/qa0= k8s.io/utils v0.0.0-20241210054802-24370beab758/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0= -nullprogram.com/x/optparse v1.0.0/go.mod h1:KdyPE+Igbe0jQUrVfMqDMeJQIJZEuyV7pjYmp6pbG50= -rsc.io/pdf v0.1.1/go.mod h1:n8OzWcQ6Sp37PL01nO98y4iUCRdTGarVfzxY20ICaU4= sigs.k8s.io/controller-runtime v0.21.0 h1:CYfjpEuicjUecRk+KAeyYh+ouUBn4llGyDYytIGcJS8= sigs.k8s.io/controller-runtime v0.21.0/go.mod h1:OSg14+F65eWqIu4DceX7k/+QRAbTTvxeQSNSOQpukWM= sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 h1:gBQPwqORJ8d8/YNZWEjoZs7npUVDpVXUUOFfW6CgAqE= diff --git a/pkg/application/service/services.go b/pkg/application/service/services.go index 0812d9ad..25669e7c 100644 --- a/pkg/application/service/services.go +++ b/pkg/application/service/services.go @@ -3,18 +3,18 @@ package service import ( toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" "github.com/codeready-toolchain/registration-service/pkg/signup" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" ) type SignupService interface { - Signup(ctx *gin.Context) (*toolchainv1alpha1.UserSignup, error) - GetSignup(ctx *gin.Context, username string, checkUserSignupCompleted bool) (*signup.Signup, error) + Signup(ctx echo.Context) (*toolchainv1alpha1.UserSignup, error) + GetSignup(ctx echo.Context, username string, checkUserSignupCompleted bool) (*signup.Signup, error) } type VerificationService interface { - InitVerification(ctx *gin.Context, username, e164PhoneNumber, countryCode string) error - VerifyPhoneCode(ctx *gin.Context, username, code string) error - VerifyActivationCode(ctx *gin.Context, username, code string) error + InitVerification(ctx echo.Context, username, e164PhoneNumber, countryCode string) error + VerifyPhoneCode(ctx echo.Context, username, code string) error + VerifyActivationCode(ctx echo.Context, username, code string) error } type Services interface { diff --git a/pkg/assets/assets.go b/pkg/assets/assets.go index 19739a0a..b279fe49 100644 --- a/pkg/assets/assets.go +++ b/pkg/assets/assets.go @@ -3,29 +3,11 @@ package assets import ( "embed" "io/fs" - "net/http" - - "github.com/gin-contrib/static" ) //go:embed static/* var StaticContent embed.FS -func ServeEmbedContent() (static.ServeFileSystem, error) { - fsys, err := fs.Sub(StaticContent, "static") // matches the path in `go:embed` above - if err != nil { - return nil, err - } - return StaticContentFileSystem{ - FileSystem: http.FS(fsys), - }, nil -} - -type StaticContentFileSystem struct { - http.FileSystem -} - -func (e StaticContentFileSystem) Exists(_ string, path string) bool { - _, err := e.Open(path) - return err == nil +func ServeEmbedContent() (fs.FS, error) { + return fs.Sub(StaticContent, "static") } diff --git a/pkg/context/keys.go b/pkg/context/keys.go index b3aca339..95046cb5 100644 --- a/pkg/context/keys.go +++ b/pkg/context/keys.go @@ -1,5 +1,19 @@ package context +import "github.com/labstack/echo/v4" + +// GetString returns the string value associated with the given key in the echo context. +// Returns empty string if the key doesn't exist or the value is not a string. +func GetString(ctx echo.Context, key string) string { + if ctx == nil { + return "" + } + if val, ok := ctx.Get(key).(string); ok { + return val + } + return "" +} + const ( // UserIDKey is the context key for the user_id claim UserIDKey = "user_id" diff --git a/pkg/controller/analytics.go b/pkg/controller/analytics.go index 50332fba..91a95f48 100644 --- a/pkg/controller/analytics.go +++ b/pkg/controller/analytics.go @@ -4,7 +4,7 @@ import ( "net/http" "github.com/codeready-toolchain/registration-service/pkg/configuration" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" ) // Analytics implements the segment endpoint, which is invoked to @@ -18,15 +18,15 @@ func NewAnalytics() *Analytics { } // GetSandboxSegmentWriteKey returns segment-write-key content for UI. -func (a *Analytics) GetSandboxSegmentWriteKey(ctx *gin.Context) { +func (a *Analytics) GetSandboxSegmentWriteKey(ctx echo.Context) error { cfg := configuration.GetRegistrationServiceConfig() segmentWriteKey := cfg.Analytics().SegmentWriteKey() - ctx.String(http.StatusOK, segmentWriteKey) + return ctx.String(http.StatusOK, segmentWriteKey) } // GetDevSpacesSegmentWriteKey returns segment-write-key content for DevSpaces -func (a *Analytics) GetDevSpacesSegmentWriteKey(ctx *gin.Context) { +func (a *Analytics) GetDevSpacesSegmentWriteKey(ctx echo.Context) error { cfg := configuration.GetRegistrationServiceConfig() segmentWriteKey := cfg.Analytics().DevSpacesSegmentWriteKey() - ctx.String(http.StatusOK, segmentWriteKey) + return ctx.String(http.StatusOK, segmentWriteKey) } diff --git a/pkg/controller/analytics_test.go b/pkg/controller/analytics_test.go index 6b58113c..d48fe937 100644 --- a/pkg/controller/analytics_test.go +++ b/pkg/controller/analytics_test.go @@ -10,7 +10,7 @@ import ( testconfig "github.com/codeready-toolchain/toolchain-common/pkg/test/config" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -39,12 +39,9 @@ func (s *TestAnalyticsSuite) TestAnalyticsHandler() { req, err := http.NewRequest(http.MethodGet, "/api/v1/segment-write-key", nil) require.NoError(s.T(), err) - handler := gin.HandlerFunc(analyticsCtrl.GetDevSpacesSegmentWriteKey) - // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) s.OverrideApplicationDefault(testconfig.RegistrationService(). Analytics().DevSpacesSegmentWriteKey("testing devspaces segment write key")) @@ -53,7 +50,8 @@ func (s *TestAnalyticsSuite) TestAnalyticsHandler() { assert.Equal(s.T(), "testing devspaces segment write key", cfg.Analytics().DevSpacesSegmentWriteKey()) - handler(ctx) + err = analyticsCtrl.GetDevSpacesSegmentWriteKey(ctx) + require.NoError(s.T(), err) // Check the status code is what we expect. require.Equal(s.T(), http.StatusOK, rr.Code) @@ -61,7 +59,6 @@ func (s *TestAnalyticsSuite) TestAnalyticsHandler() { // Check the response body is what we expect. // get config values from endpoint response dataEnvelope := rr.Body.String() - require.NoError(s.T(), err) s.Run("envelope segment write key", func() { assert.Equal(s.T(), cfg.Analytics().DevSpacesSegmentWriteKey(), dataEnvelope, "wrong 'segment write key' in segment response") @@ -75,12 +72,9 @@ func (s *TestAnalyticsSuite) TestAnalyticsHandler() { req, err := http.NewRequest(http.MethodGet, "/api/v1/analytics/segment-write-key", nil) require.NoError(s.T(), err) - handler := gin.HandlerFunc(analyticsCtrl.GetSandboxSegmentWriteKey) - // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) s.OverrideApplicationDefault(testconfig.RegistrationService(). Analytics().SegmentWriteKey("testing sandbox segment write key")) @@ -89,7 +83,8 @@ func (s *TestAnalyticsSuite) TestAnalyticsHandler() { assert.Equal(s.T(), "testing sandbox segment write key", cfg.Analytics().SegmentWriteKey()) - handler(ctx) + err = analyticsCtrl.GetSandboxSegmentWriteKey(ctx) + require.NoError(s.T(), err) // Check the status code is what we expect. require.Equal(s.T(), http.StatusOK, rr.Code) diff --git a/pkg/controller/authconfig.go b/pkg/controller/authconfig.go index 49c8db57..929b452c 100644 --- a/pkg/controller/authconfig.go +++ b/pkg/controller/authconfig.go @@ -4,7 +4,7 @@ import ( "net/http" "github.com/codeready-toolchain/registration-service/pkg/configuration" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" ) type configResponse struct { @@ -28,12 +28,12 @@ func NewAuthConfig() *AuthConfig { } // GetHandler returns raw auth config content for UI. -func (ac *AuthConfig) GetHandler(ctx *gin.Context) { +func (ac *AuthConfig) GetHandler(ctx echo.Context) error { cfg := configuration.GetRegistrationServiceConfig() configRespData := configResponse{ AuthClientLibraryURL: cfg.Auth().AuthClientLibraryURL(), AuthClientConfigRaw: cfg.Auth().AuthClientConfigRaw(), SignupURL: cfg.RegistrationServiceURL(), } - ctx.JSON(http.StatusOK, configRespData) + return ctx.JSON(http.StatusOK, configRespData) } diff --git a/pkg/controller/authconfig_test.go b/pkg/controller/authconfig_test.go index 39a49664..6766facb 100644 --- a/pkg/controller/authconfig_test.go +++ b/pkg/controller/authconfig_test.go @@ -11,7 +11,7 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/configuration" "github.com/codeready-toolchain/registration-service/test" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -40,22 +40,21 @@ func (s *TestAuthConfigSuite) TestAuthClientConfigHandler() { // Create handler instance. authConfigCtrl := NewAuthConfig() - handler := gin.HandlerFunc(authConfigCtrl.GetHandler) s.Run("valid json config", func() { // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) - handler(ctx) + err = authConfigCtrl.GetHandler(ctx) + require.NoError(s.T(), err) // Check the status code is what we expect. require.Equal(s.T(), http.StatusOK, rr.Code) - // check response content-type. - require.Equal(s.T(), cfg.Auth().AuthClientConfigContentType(), rr.Header().Get("Content-Type")) + // Echo's ctx.JSON sets application/json (config may still advertise charset in AuthClientConfigContentType). + require.Contains(s.T(), rr.Header().Get("Content-Type"), "application/json") // Check the response body is what we expect. // get config values from endpoint response diff --git a/pkg/controller/health_check.go b/pkg/controller/health_check.go index 68fc9aa3..5bbc9d20 100644 --- a/pkg/controller/health_check.go +++ b/pkg/controller/health_check.go @@ -9,7 +9,7 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/log" "github.com/codeready-toolchain/toolchain-common/pkg/status" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" ) type HealthCheckConfig interface { @@ -34,7 +34,7 @@ func NewHealthCheck(checker HealthChecker) *HealthCheck { } // getHealthInfo returns the health info. -func (hc *HealthCheck) getHealthInfo(ctx *gin.Context) *HealthStatus { +func (hc *HealthCheck) getHealthInfo(ctx echo.Context) *HealthStatus { cfg := configuration.GetRegistrationServiceConfig() return &HealthStatus{ Health: &status.Health{ @@ -49,19 +49,17 @@ func (hc *HealthCheck) getHealthInfo(ctx *gin.Context) *HealthStatus { } // GetHandler returns a default heath check result. -func (hc *HealthCheck) GetHandler(ctx *gin.Context) { - // Default handler for system health +func (hc *HealthCheck) GetHandler(ctx echo.Context) error { healthInfo := hc.getHealthInfo(ctx) if healthInfo.Alive { - ctx.JSON(http.StatusOK, healthInfo) - } else { - ctx.JSON(http.StatusServiceUnavailable, healthInfo) + return ctx.JSON(http.StatusOK, healthInfo) } + return ctx.JSON(http.StatusServiceUnavailable, healthInfo) } type HealthChecker interface { - Alive(*gin.Context) bool - APIProxyAlive(*gin.Context) bool + Alive(echo.Context) bool + APIProxyAlive(echo.Context) bool } func NewHealthChecker(port string) HealthChecker { @@ -72,12 +70,12 @@ type healthCheckerImpl struct { port string } -func (c *healthCheckerImpl) Alive(ctx *gin.Context) bool { +func (c *healthCheckerImpl) Alive(ctx echo.Context) bool { // TODO check if there are errors in configuration return c.APIProxyAlive(ctx) } -func (c *healthCheckerImpl) APIProxyAlive(ctx *gin.Context) bool { +func (c *healthCheckerImpl) APIProxyAlive(ctx echo.Context) bool { resp, err := http.Get(fmt.Sprintf("http://localhost:%s/proxyhealth", c.port)) if err != nil { log.Error(ctx, err, "API Proxy health check failed") diff --git a/pkg/controller/health_check_test.go b/pkg/controller/health_check_test.go index 659b6bd8..d583dfe5 100644 --- a/pkg/controller/health_check_test.go +++ b/pkg/controller/health_check_test.go @@ -14,7 +14,7 @@ import ( testconfig "github.com/codeready-toolchain/toolchain-common/pkg/test/config" "gopkg.in/h2non/gock.v1" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -39,14 +39,12 @@ func (s *TestHealthCheckSuite) TestHealthCheckHandler() { // Create health check instance. healthCheckCtrl := controller.NewHealthCheck(controller.NewHealthChecker(proxy.DefaultPort)) - handler := gin.HandlerFunc(healthCheckCtrl.GetHandler) s.Run("health in testing mode", func() { // given // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) // Setting unit-tests environment s.OverrideApplicationDefault(testconfig.RegistrationService(). @@ -61,7 +59,8 @@ func (s *TestHealthCheckSuite) TestHealthCheckHandler() { BodyString("") // when - handler(ctx) + err := healthCheckCtrl.GetHandler(ctx) + require.NoError(s.T(), err) // then // Check the status code is what we expect. @@ -69,7 +68,7 @@ func (s *TestHealthCheckSuite) TestHealthCheckHandler() { // Check the response body is what we expect. data := &controller.HealthStatus{} - err := json.Unmarshal(rr.Body.Bytes(), &data) + err = json.Unmarshal(rr.Body.Bytes(), &data) require.NoError(s.T(), err) assertHealth(s.T(), true, true, "unit-tests", data) @@ -79,8 +78,7 @@ func (s *TestHealthCheckSuite) TestHealthCheckHandler() { // given // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) // Setting production mode s.OverrideApplicationDefault(testconfig.RegistrationService(). @@ -98,7 +96,8 @@ func (s *TestHealthCheckSuite) TestHealthCheckHandler() { // when // Our handlers satisfy http.Handler, so we can call their ServeHTTP method // directly and pass in our Request and ResponseRecorder. - handler(ctx) + err := healthCheckCtrl.GetHandler(ctx) + require.NoError(s.T(), err) // then // Check the status code is what we expect. @@ -106,7 +105,7 @@ func (s *TestHealthCheckSuite) TestHealthCheckHandler() { // Check the response body is what we expect. data := &controller.HealthStatus{} - err := json.Unmarshal(rr.Body.Bytes(), &data) + err = json.Unmarshal(rr.Body.Bytes(), &data) require.NoError(s.T(), err) assertHealth(s.T(), true, true, "prod", data) @@ -121,12 +120,10 @@ func (s *TestHealthCheckSuite) TestHealthCheckHandler() { alive: false, proxyAlive: true, }) - handler := gin.HandlerFunc(healthCheckCtrl.GetHandler) // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) // mock proxy defer gock.Off() @@ -136,14 +133,15 @@ func (s *TestHealthCheckSuite) TestHealthCheckHandler() { Reply(http.StatusOK). BodyString("") - handler(ctx) + err := healthCheckCtrl.GetHandler(ctx) + require.NoError(s.T(), err) // Check the status code is what we expect. assert.Equal(s.T(), http.StatusServiceUnavailable, rr.Code, "handler returned wrong status code") // Check the response body is what we expect. data := &controller.HealthStatus{} - err := json.Unmarshal(rr.Body.Bytes(), &data) + err = json.Unmarshal(rr.Body.Bytes(), &data) require.NoError(s.T(), err) assertHealth(s.T(), false, true, "testServiceUnavailable", data) @@ -153,8 +151,7 @@ func (s *TestHealthCheckSuite) TestHealthCheckHandler() { // given // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) // Setting production mode s.OverrideApplicationDefault(testconfig.RegistrationService(). @@ -164,7 +161,8 @@ func (s *TestHealthCheckSuite) TestHealthCheckHandler() { // when // Our handlers satisfy http.Handler, so we can call their ServeHTTP method // directly and pass in our Request and ResponseRecorder. - handler(ctx) + err := healthCheckCtrl.GetHandler(ctx) + require.NoError(s.T(), err) // then // Check the status code is what we expect. @@ -172,7 +170,7 @@ func (s *TestHealthCheckSuite) TestHealthCheckHandler() { // Check the response body is what we expect. data := &controller.HealthStatus{} - err := json.Unmarshal(rr.Body.Bytes(), &data) + err = json.Unmarshal(rr.Body.Bytes(), &data) require.NoError(s.T(), err) assertHealth(s.T(), false, false, "prod", data) @@ -184,21 +182,20 @@ func (s *TestHealthCheckSuite) TestHealthCheckHandler() { Environment("testServiceUnavailable")) healthCheckCtrl := controller.NewHealthCheck(&mockHealthChecker{}) - handler := gin.HandlerFunc(healthCheckCtrl.GetHandler) // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) - handler(ctx) + err := healthCheckCtrl.GetHandler(ctx) + require.NoError(s.T(), err) // Check the status code is what we expect. assert.Equal(s.T(), http.StatusServiceUnavailable, rr.Code, "handler returned wrong status code") // Check the response body is what we expect. data := &controller.HealthStatus{} - err := json.Unmarshal(rr.Body.Bytes(), &data) + err = json.Unmarshal(rr.Body.Bytes(), &data) require.NoError(s.T(), err) assertHealth(s.T(), false, false, "testServiceUnavailable", data) @@ -219,10 +216,10 @@ type mockHealthChecker struct { proxyAlive bool } -func (c *mockHealthChecker) Alive(_ *gin.Context) bool { +func (c *mockHealthChecker) Alive(_ echo.Context) bool { return c.alive } -func (c *mockHealthChecker) APIProxyAlive(_ *gin.Context) bool { +func (c *mockHealthChecker) APIProxyAlive(_ echo.Context) bool { return c.proxyAlive } diff --git a/pkg/controller/namespaces.go b/pkg/controller/namespaces.go index 9b052185..9581288d 100644 --- a/pkg/controller/namespaces.go +++ b/pkg/controller/namespaces.go @@ -8,7 +8,7 @@ import ( crterrors "github.com/codeready-toolchain/registration-service/pkg/errors" "github.com/codeready-toolchain/registration-service/pkg/log" "github.com/codeready-toolchain/registration-service/pkg/namespaces" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" ) // ErrNamespaceReset represents the static error message that will be returned @@ -18,8 +18,7 @@ var ErrNamespaceReset = errors.New("namespace reset error") // NamespacesController holds the required controllers to be able to manage user namespaces. type NamespacesController interface { - // ResetNamespaces deletes the user's namespaces so that the appropriate controllers can recreate them. - ResetNamespaces(*gin.Context) + ResetNamespaces(echo.Context) error } type namespacesCtrl struct { @@ -33,23 +32,20 @@ func NewNamespacesController(namespacesManager namespaces.Manager) NamespacesCon } } -func (ctrl *namespacesCtrl) ResetNamespaces(ctx *gin.Context) { +func (ctrl *namespacesCtrl) ResetNamespaces(ctx echo.Context) error { err := ctrl.namespacesManager.ResetNamespaces(ctx) if err != nil { - log.Errorf(ctx, err, `unable to reset the namespaces for user "%s"`, ctx.GetString(customCtx.UsernameKey)) + log.Errorf(ctx, err, `unable to reset the namespaces for user "%s"`, customCtx.GetString(ctx, customCtx.UsernameKey)) if errors.Is(err, namespaces.ErrUserSignUpNotFoundOrDeactivated) { - crterrors.AbortWithError(ctx, http.StatusNotFound, ErrNamespaceReset, "The user is either not found or deactivated. Please contact the Developer Sandbox team at devsandbox@redhat.com for assistance") + return crterrors.AbortWithError(ctx, http.StatusNotFound, ErrNamespaceReset, "The user is either not found or deactivated. Please contact the Developer Sandbox team at devsandbox@redhat.com for assistance") } else if errors.As(err, &namespaces.ErrUserHasNoProvisionedNamespaces{}) { - crterrors.AbortWithError(ctx, http.StatusBadRequest, ErrNamespaceReset, "No namespaces provisioned, unable to perform reset. Please try again in a while and if the issue persists, please contact the Developer Sandbox team at devsandbox@redhat.com for assistance") - } else { - crterrors.AbortWithError(ctx, http.StatusInternalServerError, ErrNamespaceReset, "Unable to reset your namespaces. Please try again in a while and if the issue persists, please contact the Developer Sandbox team at devsandbox@redhat.com for assistance") + return crterrors.AbortWithError(ctx, http.StatusBadRequest, ErrNamespaceReset, "No namespaces provisioned, unable to perform reset. Please try again in a while and if the issue persists, please contact the Developer Sandbox team at devsandbox@redhat.com for assistance") } - return + return crterrors.AbortWithError(ctx, http.StatusInternalServerError, ErrNamespaceReset, "Unable to reset your namespaces. Please try again in a while and if the issue persists, please contact the Developer Sandbox team at devsandbox@redhat.com for assistance") } - log.Infof(ctx, `namespaces reset initiated for user "%s"`, ctx.GetString(customCtx.UsernameKey)) + log.Infof(ctx, `namespaces reset initiated for user "%s"`, customCtx.GetString(ctx, customCtx.UsernameKey)) - ctx.Status(http.StatusAccepted) - ctx.Writer.WriteHeaderNow() + return ctx.NoContent(http.StatusAccepted) } diff --git a/pkg/controller/namespaces_test.go b/pkg/controller/namespaces_test.go index 9116b145..dc87fa04 100644 --- a/pkg/controller/namespaces_test.go +++ b/pkg/controller/namespaces_test.go @@ -8,7 +8,7 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/namespaces" "github.com/codeready-toolchain/registration-service/test" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" ) @@ -28,7 +28,7 @@ type mockNamespacesManager struct { ResetNamespacesReturnValue error } -func (mnm *mockNamespacesManager) ResetNamespaces(_ *gin.Context) error { +func (mnm *mockNamespacesManager) ResetNamespaces(_ echo.Context) error { return mnm.ResetNamespacesReturnValue } @@ -38,7 +38,7 @@ func (mnm *mockNamespacesManager) ResetNamespaces(_ *gin.Context) error { func (ns *TestNamespacesSuite) TestResetNamespacesHandler() { ns.Run(`handler returns "Accepted" response when no errors occur`, func() { // given - // Prepare the request and the context for gin. + // Prepare the request and the context for echo. req, err := http.NewRequest(http.MethodPost, "/api/v1/reset-namespaces", nil) if err != nil { ns.Fail("unable to create test request", err.Error()) @@ -48,15 +48,14 @@ func (ns *TestNamespacesSuite) TestResetNamespacesHandler() { // Prepare the handler under test. mnm := mockNamespacesManager{} ctrl := NewNamespacesController(&mnm) - handler := gin.HandlerFunc(ctrl.ResetNamespaces) rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) // when // Call the handler under test. - handler(ctx) + err = ctrl.ResetNamespaces(ctx) + require.NoError(ns.T(), err) // then // Assert that the correct status code was returned. The namespace @@ -67,7 +66,7 @@ func (ns *TestNamespacesSuite) TestResetNamespacesHandler() { ns.Run(`handler returns a "Not Found" error when the user signup cannot be found or is deactivated`, func() { // given - // Prepare the request and the context for gin. + // Prepare the request and the context for echo. req, err := http.NewRequest(http.MethodPost, "/api/v1/reset-namespaces", nil) if err != nil { ns.Fail("unable to create test request", err.Error()) @@ -75,20 +74,19 @@ func (ns *TestNamespacesSuite) TestResetNamespacesHandler() { } rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) // Override the namespace manager's response, to simulate that the // user signup could not be found or is deactivated. mnm := mockNamespacesManager{} ctrl := NewNamespacesController(&mnm) - handler := gin.HandlerFunc(ctrl.ResetNamespaces) mnm.ResetNamespacesReturnValue = namespaces.ErrUserSignUpNotFoundOrDeactivated // when // Call the handler under test. - handler(ctx) + err = ctrl.ResetNamespaces(ctx) + require.NoError(ns.T(), err) // then // Assert that the proper "Not Found" error response was returned. @@ -97,7 +95,7 @@ func (ns *TestNamespacesSuite) TestResetNamespacesHandler() { ns.Run(`handler returns a "Bad Request" error when the user does not have any provisioned namespaces`, func() { // given - // Prepare the request and the context for gin. + // Prepare the request and the context for echo. req, err := http.NewRequest(http.MethodPost, "/api/v1/reset-namespaces", nil) if err != nil { ns.Fail("unable to create test request", err.Error()) @@ -105,8 +103,7 @@ func (ns *TestNamespacesSuite) TestResetNamespacesHandler() { } rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) // Override the namespace manager's response, to simulate that the // user does not have any provisioned namespaces, and therefore the @@ -114,13 +111,13 @@ func (ns *TestNamespacesSuite) TestResetNamespacesHandler() { // properly by the handler. mnm := mockNamespacesManager{} ctrl := NewNamespacesController(&mnm) - handler := gin.HandlerFunc(ctrl.ResetNamespaces) mnm.ResetNamespacesReturnValue = namespaces.ErrUserHasNoProvisionedNamespaces{} // when // Call the handler under test. - handler(ctx) + err = ctrl.ResetNamespaces(ctx) + require.NoError(ns.T(), err) // then // Assert that the proper "Bad Request" error response was returned. @@ -129,7 +126,7 @@ func (ns *TestNamespacesSuite) TestResetNamespacesHandler() { ns.Run(`handler returns an internal server error when a generic error occurs`, func() { // given - // Prepare the request and the context for gin. + // Prepare the request and the context for echo. req, err := http.NewRequest(http.MethodPost, "/api/v1/reset-namespaces", nil) if err != nil { ns.Fail("unable to create test request", err.Error()) @@ -137,8 +134,7 @@ func (ns *TestNamespacesSuite) TestResetNamespacesHandler() { } rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) // Override the namespace manager's response, to simulate an error when // attempting to delete the namespaces. We recreate the manager so that @@ -146,13 +142,13 @@ func (ns *TestNamespacesSuite) TestResetNamespacesHandler() { // parallel. mnm := mockNamespacesManager{} ctrl := NewNamespacesController(&mnm) - handler := gin.HandlerFunc(ctrl.ResetNamespaces) mnm.ResetNamespacesReturnValue = errors.New("test error") // when // Call the handler under test. - handler(ctx) + err = ctrl.ResetNamespaces(ctx) + require.NoError(ns.T(), err) // then // Assert that the proper "Internal Server Error" error response was diff --git a/pkg/controller/signup.go b/pkg/controller/signup.go index b75be8d6..1963db00 100644 --- a/pkg/controller/signup.go +++ b/pkg/controller/signup.go @@ -11,7 +11,7 @@ import ( crterrors "github.com/codeready-toolchain/registration-service/pkg/errors" "github.com/codeready-toolchain/registration-service/pkg/log" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" "github.com/nyaruka/phonenumbers" apierrors "k8s.io/apimachinery/pkg/api/errors" ) @@ -22,8 +22,8 @@ type Signup struct { } type Phone struct { - CountryCode string `form:"country_code" json:"country_code" binding:"required"` - PhoneNumber string `form:"phone_number" json:"phone_number" binding:"required"` + CountryCode string `form:"country_code" json:"country_code" validate:"required"` + PhoneNumber string `form:"phone_number" json:"phone_number" validate:"required"` } // NewSignup returns a new Signup instance. @@ -34,55 +34,54 @@ func NewSignup(app application.Application) *Signup { } // PostHandler creates a Signup resource -func (s *Signup) PostHandler(ctx *gin.Context) { +func (s *Signup) PostHandler(ctx echo.Context) error { userSignup, err := s.app.SignupService().Signup(ctx) e := &apierrors.StatusError{} if errors.As(err, &e) { - crterrors.AbortWithError(ctx, int(e.Status().Code), err, "error creating UserSignup resource") - return + return crterrors.AbortWithError(ctx, int(e.Status().Code), err, "error creating UserSignup resource") } if err != nil { log.Error(ctx, err, "error creating UserSignup resource") - crterrors.AbortWithError(ctx, http.StatusInternalServerError, err, "error creating UserSignup resource") - return + return crterrors.AbortWithError(ctx, http.StatusInternalServerError, err, "error creating UserSignup resource") } if _, exists := userSignup.Annotations[toolchainv1alpha1.UserSignupActivationCounterAnnotationKey]; !exists { log.Infof(ctx, "UserSignup created: %s", userSignup.Name) } else { log.Infof(ctx, "UserSignup reactivated: %s", userSignup.Name) } - ctx.Status(http.StatusAccepted) - ctx.Writer.WriteHeaderNow() + return ctx.NoContent(http.StatusAccepted) } // InitVerificationHandler starts the phone verification process for a user. It extracts the user's identifying // information from their Access Token (presented in the Authorization HTTP header) to determine the user, and then // invokes the Verification service with an E.164 formatted phone number value derived from the country code and phone number // provided by the user. -func (s *Signup) InitVerificationHandler(ctx *gin.Context) { - username := ctx.GetString(context.UsernameKey) +func (s *Signup) InitVerificationHandler(ctx echo.Context) error { + username := context.GetString(ctx, context.UsernameKey) // Read the Body content var phone Phone - if err := ctx.BindJSON(&phone); err != nil { + if err := ctx.Bind(&phone); err != nil { log.Errorf(ctx, err, "request body does not contain required fields phone_number and country_code") - crterrors.AbortWithError(ctx, http.StatusBadRequest, err, "error reading request body") - return + return crterrors.AbortWithError(ctx, http.StatusBadRequest, err, "error reading request body") + } + + if phone.CountryCode == "" || phone.PhoneNumber == "" { + log.Error(ctx, nil, "request body does not contain required fields phone_number and country_code") + return ctx.NoContent(http.StatusBadRequest) } countryCode, err := strconv.Atoi(phone.CountryCode) if err != nil { log.Errorf(ctx, err, "invalid country_code value") - crterrors.AbortWithError(ctx, http.StatusBadRequest, err, "invalid country_code") - return + return crterrors.AbortWithError(ctx, http.StatusBadRequest, err, "invalid country_code") } regionCode := phonenumbers.GetRegionCodeForCountryCode(countryCode) number, err := phonenumbers.Parse(phone.PhoneNumber, regionCode) if err != nil { log.Errorf(ctx, err, "invalid phone number") - crterrors.AbortWithError(ctx, http.StatusBadRequest, err, "invalid phone number provided") - return + return crterrors.AbortWithError(ctx, http.StatusBadRequest, err, "invalid phone number provided") } e164Number := phonenumbers.Format(number, phonenumbers.E164) @@ -92,84 +91,76 @@ func (s *Signup) InitVerificationHandler(ctx *gin.Context) { e := &crterrors.Error{} switch { case errors.As(err, &e): - crterrors.AbortWithError(ctx, int(e.Code), err, e.Message) + return crterrors.AbortWithError(ctx, int(e.Code), err, e.Message) default: - crterrors.AbortWithError(ctx, http.StatusInternalServerError, err, "error while initiating verification") + return crterrors.AbortWithError(ctx, http.StatusInternalServerError, err, "error while initiating verification") } - return } log.Infof(ctx, "phone verification has been sent for username %s", username) - ctx.Status(http.StatusNoContent) - ctx.Writer.WriteHeaderNow() + return ctx.NoContent(http.StatusNoContent) } // GetHandler returns the Signup resource -func (s *Signup) GetHandler(ctx *gin.Context) { +func (s *Signup) GetHandler(ctx echo.Context) error { // Get the UserSignup resource from the service by the username - username := ctx.GetString(context.UsernameKey) + username := context.GetString(ctx, context.UsernameKey) signupResource, err := s.app.SignupService().GetSignup(ctx, username, true) if err != nil { log.Error(ctx, err, "error getting UserSignup resource") e := &apierrors.StatusError{} if errors.As(err, &e) { - crterrors.AbortWithError(ctx, int(e.Status().Code), err, "error getting UserSignup resource") - return + return crterrors.AbortWithError(ctx, int(e.Status().Code), err, "error getting UserSignup resource") } - crterrors.AbortWithError(ctx, http.StatusInternalServerError, err, "error getting UserSignup resource") + return crterrors.AbortWithError(ctx, http.StatusInternalServerError, err, "error getting UserSignup resource") } if signupResource == nil { log.Infof(ctx, "UserSignup resource for username '%s' resource not found", username) - ctx.AbortWithStatus(http.StatusNotFound) - } else { - ctx.JSON(http.StatusOK, signupResource) + return ctx.NoContent(http.StatusNotFound) } + return ctx.JSON(http.StatusOK, signupResource) } // VerifyPhoneCodeHandler validates the phone verification code passed in by the user -func (s *Signup) VerifyPhoneCodeHandler(ctx *gin.Context) { +func (s *Signup) VerifyPhoneCodeHandler(ctx echo.Context) error { log.Info(ctx, "Verifying phone code") code := ctx.Param("code") if code == "" { log.Error(ctx, nil, "no phone code provided in the request") - ctx.AbortWithStatus(http.StatusBadRequest) - return + return ctx.NoContent(http.StatusBadRequest) } - username := ctx.GetString(context.UsernameKey) + username := context.GetString(ctx, context.UsernameKey) err := s.app.VerificationService().VerifyPhoneCode(ctx, username, code) if err != nil { e := &crterrors.Error{} switch { case errors.As(err, &e): - crterrors.AbortWithError(ctx, int(e.Code), err, "error while verifying phone code") + return crterrors.AbortWithError(ctx, int(e.Code), err, "error while verifying phone code") default: - crterrors.AbortWithError(ctx, http.StatusInternalServerError, err, "unexpected error while verifying phone code") + return crterrors.AbortWithError(ctx, http.StatusInternalServerError, err, "unexpected error while verifying phone code") } - return } - ctx.Status(http.StatusOK) log.Info(ctx, "Verified phone code") + return ctx.NoContent(http.StatusOK) } // VerifyActivationCodeHandler validates the activation code passed in by the user as a form value -func (s *Signup) VerifyActivationCodeHandler(ctx *gin.Context) { +func (s *Signup) VerifyActivationCodeHandler(ctx echo.Context) error { body := map[string]interface{}{} - if err := ctx.BindJSON(&body); err != nil { + if err := ctx.Bind(&body); err != nil { log.Error(ctx, nil, "no activation code provided in the request") - ctx.AbortWithStatus(http.StatusBadRequest) - return + return ctx.NoContent(http.StatusBadRequest) } code, ok := body["code"].(string) if !ok { log.Error(ctx, nil, "no activation code provided in the request") - ctx.AbortWithStatus(http.StatusBadRequest) - return + return ctx.NoContent(http.StatusBadRequest) } - username := ctx.GetString(context.UsernameKey) + username := context.GetString(ctx, context.UsernameKey) err := s.app.VerificationService().VerifyActivationCode(ctx, username, code) if err != nil { @@ -177,11 +168,10 @@ func (s *Signup) VerifyActivationCodeHandler(ctx *gin.Context) { e := &crterrors.Error{} switch { case errors.As(err, &e): - crterrors.AbortWithError(ctx, int(e.Code), err, "error while verifying activation code") + return crterrors.AbortWithError(ctx, int(e.Code), err, "error while verifying activation code") default: - crterrors.AbortWithError(ctx, http.StatusInternalServerError, err, "unexpected error while verifying activation code") + return crterrors.AbortWithError(ctx, http.StatusInternalServerError, err, "unexpected error while verifying activation code") } - return } - ctx.Status(http.StatusOK) + return ctx.NoContent(http.StatusOK) } diff --git a/pkg/controller/signup_test.go b/pkg/controller/signup_test.go index 39b8a96a..ae335702 100644 --- a/pkg/controller/signup_test.go +++ b/pkg/controller/signup_test.go @@ -30,8 +30,8 @@ import ( "github.com/codeready-toolchain/toolchain-common/pkg/usersignup" "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/gin-gonic/gin" "github.com/gofrs/uuid" + "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -61,12 +61,10 @@ func (s *TestSignupSuite) TestSignupPostHandler() { // given fakeClient, application := testutil.PrepareInClusterApp(s.T()) signupCtrl := controller.NewSignup(application) - handler := gin.HandlerFunc(signupCtrl.PostHandler) // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) // Put userID to the context ob, err := uuid.NewV4() @@ -77,12 +75,13 @@ func (s *TestSignupSuite) TestSignupPostHandler() { ctx.Set(context.EmailKey, expectedUserID+"@test.com") // when - handler(ctx) + err = signupCtrl.PostHandler(ctx) + require.NoError(s.T(), err) // Check the status code is what we expect. require.Equal(s.T(), http.StatusAccepted, rr.Code) userSignup := &crtapi.UserSignup{} - require.NoError(s.T(), fakeClient.Get(ctx, + require.NoError(s.T(), fakeClient.Get(gocontext.Background(), commontest.NamespacedName(commontest.HostOperatorNs, usersignup.EncodeUserIdentifier("bill@kubesaw")), userSignup)) assert.Equal(s.T(), expectedUserID, userSignup.Spec.IdentityClaims.Sub) assert.Equal(s.T(), "bill@kubesaw", userSignup.Spec.IdentityClaims.PreferredUsername) @@ -93,18 +92,17 @@ func (s *TestSignupSuite) TestSignupPostHandler() { // given fakeClient, application := testutil.PrepareInClusterApp(s.T()) signupCtrl := controller.NewSignup(application) - handler := gin.HandlerFunc(signupCtrl.PostHandler) // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) fakeClient.MockCreate = func(_ gocontext.Context, _ client.Object, _ ...client.CreateOption) error { return errors.New("blah") } // when - handler(ctx) + err := signupCtrl.PostHandler(ctx) + require.NoError(s.T(), err) // Check the error is what we expect. test.AssertError(s.T(), rr, http.StatusInternalServerError, "blah", "error creating UserSignup resource") @@ -115,15 +113,14 @@ func (s *TestSignupSuite) TestSignupPostHandler() { _, application := testutil.PrepareInClusterApp(s.T()) signupCtrl := controller.NewSignup(application) - handler := gin.HandlerFunc(signupCtrl.PostHandler) // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) ctx.Set(context.UsernameKey, "kubesaw-crtadmin") // when - handler(ctx) + err := signupCtrl.PostHandler(ctx) + require.NoError(s.T(), err) // then test.AssertError(s.T(), rr, http.StatusForbidden, "forbidden: failed to create usersignup for kubesaw-crtadmin", "error creating UserSignup resource") @@ -149,13 +146,11 @@ func (s *TestSignupSuite) TestSignupGetHandler() { // Create Signup controller instance. ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.GetHandler) s.Run("signups found", func() { // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) ctx.Set(context.UsernameKey, "ted@kubesaw") expected := &signup.Signup{ @@ -174,7 +169,8 @@ func (s *TestSignupSuite) TestSignupGetHandler() { } // when - handler(ctx) + err := ctrl.GetHandler(ctx) + require.NoError(s.T(), err) // then assert.Equal(s.T(), http.StatusOK, rr.Code, "handler returned wrong status code") @@ -190,12 +186,12 @@ func (s *TestSignupSuite) TestSignupGetHandler() { s.Run("signups not found", func() { // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) ctx.Set(context.UsernameKey, "dummy") // when - handler(ctx) + err := ctrl.GetHandler(ctx) + require.NoError(s.T(), err) // Check the status code is what we expect. assert.Equal(s.T(), http.StatusNotFound, rr.Code, "handler returned wrong status code") @@ -206,8 +202,7 @@ func (s *TestSignupSuite) TestSignupGetHandler() { fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup) // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) ctx.Set(context.UsernameKey, "username") fakeClient.MockGet = func(_ gocontext.Context, _ client.ObjectKey, _ client.Object, _ ...client.GetOption) error { @@ -215,7 +210,8 @@ func (s *TestSignupSuite) TestSignupGetHandler() { } // when - gin.HandlerFunc(controller.NewSignup(application).GetHandler)(ctx) + err := controller.NewSignup(application).GetHandler(ctx) + require.NoError(s.T(), err) // then test.AssertError(s.T(), rr, http.StatusInternalServerError, "oopsie woopsie", "error getting UserSignup resource") @@ -234,16 +230,15 @@ func (s *TestSignupSuite) TestSignupGetHandler() { // Create Signup controller instance. ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.GetHandler) // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) ctx.Set(context.UsernameKey, "ted@kubesaw") ctx.Set(context.EmailKey, userSignup.Spec.IdentityClaims.Email) // when - handler(ctx) + err := ctrl.GetHandler(ctx) + require.NoError(s.T(), err) // then assert.Equal(s.T(), http.StatusForbidden, rr.Code, "handler returned wrong status code") @@ -261,12 +256,12 @@ func (s *TestSignupSuite) TestInitVerificationHandler() { testusersignup.WithAnnotation(crtapi.UserSignupVerificationCodeAnnotationKey, ""), testusersignup.VerificationRequiredAgo(time.Second)) - assertInitVerificationSuccess := func(handler gin.HandlerFunc, fakeClient *commontest.FakeClient, phoneNumber, expectedHash string, expectedCounter int) { + assertInitVerificationSuccess := func(handler echo.HandlerFunc, fakeClient *commontest.FakeClient, phoneNumber, expectedHash string, expectedCounter int) { // given data := []byte(fmt.Sprintf(`{"phone_number": "%s", "country_code": "1"}`, phoneNumber)) // when - rr := initPhoneVerification(s.T(), handler, gin.Param{}, data, "johnny@kubesaw", http.MethodPut, "/api/v1/signup/verification") + rr := initPhoneVerification(s.T(), handler, "", "", data, "johnny@kubesaw", http.MethodPut, "/api/v1/signup/verification") // then require.Equal(s.T(), http.StatusNoContent, rr.Code) @@ -315,7 +310,7 @@ func (s *TestSignupSuite) TestInitVerificationHandler() { data := []byte(`{"phone_number": "2268213044", "country_code": "(1)"}`) // when - rr := initPhoneVerification(s.T(), handler, gin.Param{}, data, "johnny@kubesaw", http.MethodPut, "/api/v1/signup/verification") + rr := initPhoneVerification(s.T(), handler, "", "", data, "johnny@kubesaw", http.MethodPut, "/api/v1/signup/verification") // then require.Equal(s.T(), http.StatusBadRequest, rr.Code) @@ -335,12 +330,17 @@ func (s *TestSignupSuite) TestInitVerificationHandler() { data := []byte(`{"test_number": "2268213044", "test_code": "1"}`) // when - rr := initPhoneVerification(s.T(), handler, gin.Param{}, data, "johnny@kubesaw", http.MethodPut, "/api/v1/signup/verification") + rr := initPhoneVerification(s.T(), handler, "", "", data, "johnny@kubesaw", http.MethodPut, "/api/v1/signup/verification") // then // Check the status code is what we expect. assert.Equal(s.T(), http.StatusBadRequest, rr.Code) + // With Echo, JSON that binds but omits required fields yields ctx.NoContent(400) with an empty body (see InitVerificationHandler). + if len(rr.Body.Bytes()) == 0 { + return + } + bodyParams := make(map[string]interface{}) err := json.Unmarshal(rr.Body.Bytes(), &bodyParams) require.NoError(s.T(), err) @@ -364,7 +364,7 @@ func (s *TestSignupSuite) TestInitVerificationHandler() { data := []byte(`{"phone_number": "2268213044", "country_code": "1"}`) // when - rr := initPhoneVerification(s.T(), handler, gin.Param{}, data, "johnny@kubesaw", http.MethodPut, "/api/v1/signup/verification") + rr := initPhoneVerification(s.T(), handler, "", "", data, "johnny@kubesaw", http.MethodPut, "/api/v1/signup/verification") // then // Check the status code is what we expect. @@ -379,7 +379,7 @@ func (s *TestSignupSuite) TestInitVerificationHandler() { data := []byte(`{"phone_number": "2268213044", "country_code": "1"}`) // when - rr := initPhoneVerification(s.T(), handler, gin.Param{}, data, "johnny@kubesaw", http.MethodPut, "/api/v1/signup/verification") + rr := initPhoneVerification(s.T(), handler, "", "", data, "johnny@kubesaw", http.MethodPut, "/api/v1/signup/verification") // then // Check the status code is what we expect. @@ -403,20 +403,19 @@ func (s *TestSignupSuite) TestInitVerificationHandler() { data := []byte(`{"phone_number": "!226%213044", "country_code": "1"}`) // when - rr := initPhoneVerification(s.T(), handler, gin.Param{}, data, "johnny@kubesaw", http.MethodPut, "/api/v1/signup/verification") + rr := initPhoneVerification(s.T(), handler, "", "", data, "johnny@kubesaw", http.MethodPut, "/api/v1/signup/verification") // Check the status code is what we expect. assert.Equal(s.T(), http.StatusBadRequest, rr.Code) }) } -func prepareVerificationHandler(t *testing.T, initObjects ...client.Object) (*commontest.FakeClient, gin.HandlerFunc) { +func prepareVerificationHandler(t *testing.T, initObjects ...client.Object) (*commontest.FakeClient, echo.HandlerFunc) { fakeClient, application := testutil.PrepareInClusterApp(t, initObjects...) // Create Signup controller instance. ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.InitVerificationHandler) - return fakeClient, handler + return fakeClient, ctrl.InitVerificationHandler } func (s *TestSignupSuite) TestVerifyPhoneCodeHandler() { @@ -431,13 +430,9 @@ func (s *TestSignupSuite) TestVerifyPhoneCodeHandler() { // Create Signup controller instance. fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup) ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.VerifyPhoneCodeHandler) + handler := ctrl.VerifyPhoneCodeHandler - param := gin.Param{ - Key: "code", - Value: "999888", - } - rr := initPhoneVerification(s.T(), handler, param, nil, "johnny@kubesaw", http.MethodGet, "/api/v1/signup/verification") + rr := initPhoneVerification(s.T(), handler, "code", "999888", nil, "johnny@kubesaw", http.MethodGet, "/api/v1/signup/verification") // Check the status code is what we expect. require.Equal(s.T(), http.StatusOK, rr.Code) @@ -462,13 +457,9 @@ func (s *TestSignupSuite) TestVerifyPhoneCodeHandler() { // Create Signup controller instance. ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.VerifyPhoneCodeHandler) + handler := ctrl.VerifyPhoneCodeHandler - param := gin.Param{ - Key: "code", - Value: "111233", - } - rr := initPhoneVerification(s.T(), handler, param, nil, "johnny@kubesaw", http.MethodGet, "/api/v1/signup/verification") + rr := initPhoneVerification(s.T(), handler, "code", "111233", nil, "johnny@kubesaw", http.MethodGet, "/api/v1/signup/verification") // Check the status code is what we expect. require.Equal(s.T(), http.StatusInternalServerError, rr.Code) @@ -488,13 +479,9 @@ func (s *TestSignupSuite) TestVerifyPhoneCodeHandler() { // Create Signup controller instance and handle the verification request ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.VerifyPhoneCodeHandler) + handler := ctrl.VerifyPhoneCodeHandler - param := gin.Param{ - Key: "code", - Value: "111233", - } - rr := initPhoneVerification(s.T(), handler, param, nil, "jsmith@kubesaw", http.MethodGet, "/api/v1/signup/verification/111233") + rr := initPhoneVerification(s.T(), handler, "code", "111233", nil, "jsmith@kubesaw", http.MethodGet, "/api/v1/signup/verification/111233") // Check the status code is what we expect. require.Equal(s.T(), http.StatusNotFound, rr.Code) @@ -517,13 +504,9 @@ func (s *TestSignupSuite) TestVerifyPhoneCodeHandler() { } // Create Signup controller instance. ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.VerifyPhoneCodeHandler) + handler := ctrl.VerifyPhoneCodeHandler - param := gin.Param{ - Key: "code", - Value: "555555", - } - rr := initPhoneVerification(s.T(), handler, param, nil, "johnny@kubesaw", http.MethodGet, + rr := initPhoneVerification(s.T(), handler, "code", "555555", nil, "johnny@kubesaw", http.MethodGet, "/api/v1/signup/verification/555555") // Check the status code is what we expect. @@ -551,13 +534,9 @@ func (s *TestSignupSuite) TestVerifyPhoneCodeHandler() { // Create Signup controller instance. ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.VerifyPhoneCodeHandler) + handler := ctrl.VerifyPhoneCodeHandler - param := gin.Param{ - Key: "code", - Value: "333333", - } - rr := initPhoneVerification(s.T(), handler, param, nil, "johnny@kubesaw", http.MethodGet, "/api/v1/signup/verification/333333") + rr := initPhoneVerification(s.T(), handler, "code", "333333", nil, "johnny@kubesaw", http.MethodGet, "/api/v1/signup/verification/333333") // Check the status code is what we expect. require.Equal(s.T(), http.StatusTooManyRequests, rr.Code) @@ -576,30 +555,35 @@ func (s *TestSignupSuite) TestVerifyPhoneCodeHandler() { // Create Signup controller instance. _, application := testutil.PrepareInClusterApp(s.T(), userSignup) ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.VerifyPhoneCodeHandler) + handler := ctrl.VerifyPhoneCodeHandler - param := gin.Param{ - Key: "code", - Value: "", - } - rr := initPhoneVerification(s.T(), handler, param, nil, "", http.MethodGet, "/api/v1/signup/verification/") + rr := initPhoneVerification(s.T(), handler, "code", "", nil, "", http.MethodGet, "/api/v1/signup/verification/") // Check the status code is what we expect. require.Equal(s.T(), http.StatusBadRequest, rr.Code) }) } -func initPhoneVerification(t *testing.T, handler gin.HandlerFunc, params gin.Param, data []byte, username, httpMethod, url string) *httptest.ResponseRecorder { +func initPhoneVerification(t *testing.T, handler echo.HandlerFunc, paramName, paramValue string, data []byte, username, httpMethod, url string) *httptest.ResponseRecorder { // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + e := echo.New() req, err := http.NewRequest(httpMethod, url, bytes.NewBuffer(data)) require.NoError(t, err) - ctx.Request = req + if len(data) > 0 { + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + } + + ctx := e.NewContext(req, rr) ctx.Set(context.UsernameKey, username) - ctx.Params = append(ctx.Params, params) - handler(ctx) + if paramName != "" { + ctx.SetParamNames(paramName) + ctx.SetParamValues(paramValue) + } + + err = handler(ctx) + require.NoError(t, err) return rr } @@ -614,7 +598,7 @@ func (s *TestSignupSuite) TestVerifyActivationCodeHandler() { event := testsocialevent.NewSocialEvent(commontest.HostOperatorNs, "event") fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup, event) ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.VerifyActivationCodeHandler) + handler := ctrl.VerifyActivationCodeHandler // when rr := initActivationCodeVerification(s.T(), handler, userSignup.Name, event.Name) @@ -638,7 +622,7 @@ func (s *TestSignupSuite) TestVerifyActivationCodeHandler() { event := testsocialevent.NewSocialEvent(commontest.HostOperatorNs, "event") fakeClient, application := testutil.PrepareInClusterApp(s.T(), deactivatedUS, event) ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.VerifyActivationCodeHandler) + handler := ctrl.VerifyActivationCodeHandler // when rr := initActivationCodeVerification(s.T(), handler, deactivatedUS.Name, event.Name) @@ -660,7 +644,7 @@ func (s *TestSignupSuite) TestVerifyActivationCodeHandler() { event := testsocialevent.NewSocialEvent(commontest.HostOperatorNs, "event") fakeClient, application := testutil.PrepareInClusterApp(s.T(), event) ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.VerifyActivationCodeHandler) + handler := ctrl.VerifyActivationCodeHandler // when rr := initActivationCodeVerification(s.T(), handler, "Jane", event.Name) @@ -686,7 +670,7 @@ func (s *TestSignupSuite) TestVerifyActivationCodeHandler() { ) fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup) ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.VerifyActivationCodeHandler) + handler := ctrl.VerifyActivationCodeHandler // when rr := initActivationCodeVerification(s.T(), handler, userSignup.Name, "invalid") @@ -705,7 +689,7 @@ func (s *TestSignupSuite) TestVerifyActivationCodeHandler() { userSignup := testusersignup.NewUserSignup(testusersignup.VerificationRequiredAgo(time.Second)) // just signed up fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup) ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.VerifyActivationCodeHandler) + handler := ctrl.VerifyActivationCodeHandler // when rr := initActivationCodeVerification(s.T(), handler, userSignup.Name, "invalid") @@ -725,7 +709,7 @@ func (s *TestSignupSuite) TestVerifyActivationCodeHandler() { event := testsocialevent.NewSocialEvent(commontest.HostOperatorNs, "event", testsocialevent.WithStartTime(time.Now().Add(60*time.Minute))) fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup, event) ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.VerifyActivationCodeHandler) + handler := ctrl.VerifyActivationCodeHandler // when rr := initActivationCodeVerification(s.T(), handler, userSignup.Name, "invalid") @@ -747,7 +731,7 @@ func (s *TestSignupSuite) TestVerifyActivationCodeHandler() { event := testsocialevent.NewSocialEvent(commontest.HostOperatorNs, "event", testsocialevent.WithEndTime(time.Now().Add(-1*time.Minute))) fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup, event) ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.VerifyActivationCodeHandler) + handler := ctrl.VerifyActivationCodeHandler // when rr := initActivationCodeVerification(s.T(), handler, userSignup.Name, "invalid") @@ -769,7 +753,7 @@ func (s *TestSignupSuite) TestVerifyActivationCodeHandler() { event := testsocialevent.NewSocialEvent(commontest.HostOperatorNs, "event", testsocialevent.WithActivationCount(10)) // same as `spec.MaxAttendees` fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup, event) ctrl := controller.NewSignup(application) - handler := gin.HandlerFunc(ctrl.VerifyActivationCodeHandler) + handler := ctrl.VerifyActivationCodeHandler // when rr := initActivationCodeVerification(s.T(), handler, userSignup.Name, "invalid") @@ -787,15 +771,18 @@ func (s *TestSignupSuite) TestVerifyActivationCodeHandler() { }) } -func initActivationCodeVerification(t *testing.T, handler gin.HandlerFunc, username, code string) *httptest.ResponseRecorder { +func initActivationCodeVerification(t *testing.T, handler echo.HandlerFunc, username, code string) *httptest.ResponseRecorder { // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + e := echo.New() payload := fmt.Sprintf(`{"code":"%s"}`, code) req, err := http.NewRequest(http.MethodPost, "/api/v1/signup/verification/activation-code", bytes.NewBuffer([]byte(payload))) require.NoError(t, err) - ctx.Request = req + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + + ctx := e.NewContext(req, rr) ctx.Set(context.UsernameKey, username) - handler(ctx) + err = handler(ctx) + require.NoError(t, err) return rr } diff --git a/pkg/controller/uiconfig.go b/pkg/controller/uiconfig.go index d4737137..76b6c358 100644 --- a/pkg/controller/uiconfig.go +++ b/pkg/controller/uiconfig.go @@ -4,7 +4,7 @@ import ( "net/http" "github.com/codeready-toolchain/registration-service/pkg/configuration" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" ) type UIConfigResponse struct { @@ -22,18 +22,18 @@ type UIConfigResponse struct { type UIConfig struct { } -// NewAuthConfig returns a new AuthConfig instance. +// NewUIConfig returns a new UIConfig instance. func NewUIConfig() *UIConfig { return &UIConfig{} } // GetHandler returns raw auth config content for UI. -func (uic *UIConfig) GetHandler(ctx *gin.Context) { +func (uic *UIConfig) GetHandler(ctx echo.Context) error { cfg := configuration.GetRegistrationServiceConfig() configRespData := UIConfigResponse{ UICanaryDeploymentWeight: cfg.UICanaryDeploymentWeight(), WorkatoWebHookURL: cfg.WorkatoWebHookURL(), DisabledIntegrations: cfg.DisabledIntegrations(), } - ctx.JSON(http.StatusOK, configRespData) + return ctx.JSON(http.StatusOK, configRespData) } diff --git a/pkg/controller/uiconfig_test.go b/pkg/controller/uiconfig_test.go index ea8478fd..bc9d2a98 100644 --- a/pkg/controller/uiconfig_test.go +++ b/pkg/controller/uiconfig_test.go @@ -11,7 +11,7 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/configuration" "github.com/codeready-toolchain/registration-service/test" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -41,16 +41,15 @@ func (s *TestUIConfigSuite) TestUIConfigHandler() { // Create handler instance. uiConfigCtrl := NewUIConfig() - handler := gin.HandlerFunc(uiConfigCtrl.GetHandler) s.Run("valid json config", func() { // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) - handler(ctx) + err = uiConfigCtrl.GetHandler(ctx) + require.NoError(s.T(), err) // Check the status code is what we expect. require.Equal(s.T(), http.StatusOK, rr.Code) @@ -87,13 +86,12 @@ func (s *TestUIConfigSuite) TestUIConfigHandlerWithDisabledIntegrations() { defer s.DefaultConfig() uiConfigCtrl := NewUIConfig() - handler := gin.HandlerFunc(uiConfigCtrl.GetHandler) rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := echo.New().NewContext(req, rr) - handler(ctx) + err = uiConfigCtrl.GetHandler(ctx) + require.NoError(s.T(), err) require.Equal(s.T(), http.StatusOK, rr.Code) diff --git a/pkg/controller/usernames.go b/pkg/controller/usernames.go index 62a7dbe2..a1975021 100644 --- a/pkg/controller/usernames.go +++ b/pkg/controller/usernames.go @@ -8,7 +8,7 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/log" "github.com/codeready-toolchain/registration-service/pkg/namespaced" "github.com/codeready-toolchain/registration-service/pkg/username" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" "k8s.io/apimachinery/pkg/api/errors" ) @@ -25,35 +25,33 @@ func NewUsernames(nsClient namespaced.Client) *Usernames { } // GetHandler returns the list of usernames found, if any. -func (s *Usernames) GetHandler(ctx *gin.Context) { +func (s *Usernames) GetHandler(ctx echo.Context) error { queryString := ctx.Param("username") if queryString == "" { log.Info(ctx, "empty username provided") - ctx.AbortWithStatus(http.StatusNotFound) + return ctx.NoContent(http.StatusNotFound) } // TODO check if the queryString is an email // in that case we have to fetch the UserSignup resources with the provided email and the MasterUserRecords associated with those. murResource := &toolchainv1alpha1.MasterUserRecord{} - err := s.Get(ctx.Request.Context(), s.NamespacedName(queryString), murResource) + err := s.Get(ctx.Request().Context(), s.NamespacedName(queryString), murResource) // handle not found error if errors.IsNotFound(err) { log.Infof(ctx, "MasterUserRecord resource for: %s not found", queryString) - ctx.AbortWithStatus(http.StatusNotFound) - return + return ctx.NoContent(http.StatusNotFound) } // ...otherwise is a server error if err != nil { log.Error(ctx, err, "error getting MasterUserRecord resource") - crterrors.AbortWithError(ctx, http.StatusInternalServerError, err, "error getting MasterUserRecord resource") - return + return crterrors.AbortWithError(ctx, http.StatusInternalServerError, err, "error getting MasterUserRecord resource") } // TODO // once we implement search by email the response might contain multiple usernames // for now there can be only one username with a given name. - ctx.JSON(http.StatusOK, username.Response{ + return ctx.JSON(http.StatusOK, username.Response{ {Username: murResource.GetName()}, }) } diff --git a/pkg/controller/usernames_test.go b/pkg/controller/usernames_test.go index 2b5467ca..1077a915 100644 --- a/pkg/controller/usernames_test.go +++ b/pkg/controller/usernames_test.go @@ -14,7 +14,7 @@ import ( "github.com/codeready-toolchain/registration-service/test" "github.com/codeready-toolchain/registration-service/test/fake" commontest "github.com/codeready-toolchain/toolchain-common/pkg/test" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -44,19 +44,19 @@ func (s *TestUsernamesSuite) TestUsernamesGetHandler() { // Create Usernames controller instance. ctrl := controller.NewUsernames(nsClient) - handler := gin.HandlerFunc(ctrl.GetHandler) s.Run("usernames found", func() { // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req - ctx.AddParam("username", "johnny") + ctx := echo.New().NewContext(req, rr) + ctx.SetParamNames("username") + ctx.SetParamValues("johnny") expected := &username.Response{ {Username: "johnny"}, } - handler(ctx) + err := ctrl.GetHandler(ctx) + require.NoError(s.T(), err) // Check the status code is what we expect. assert.Equal(s.T(), http.StatusOK, rr.Code, "handler returned wrong status code") @@ -72,11 +72,12 @@ func (s *TestUsernamesSuite) TestUsernamesGetHandler() { s.Run("usernames not found", func() { // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req - ctx.AddParam("username", "noise") // user doesn't exist + ctx := echo.New().NewContext(req, rr) + ctx.SetParamNames("username") + ctx.SetParamValues("noise") // user doesn't exist - handler(ctx) + err := ctrl.GetHandler(ctx) + require.NoError(s.T(), err) // Check the status code is what we expect. assert.Equal(s.T(), http.StatusNotFound, rr.Code, "handler returned wrong status code") @@ -85,11 +86,12 @@ func (s *TestUsernamesSuite) TestUsernamesGetHandler() { s.Run("empty query string provided", func() { // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req - ctx.AddParam("username", "") // no username was provided + ctx := echo.New().NewContext(req, rr) + ctx.SetParamNames("username") + ctx.SetParamValues("") // no username was provided - handler(ctx) + err := ctrl.GetHandler(ctx) + require.NoError(s.T(), err) // Check the status code is what we expect. assert.Equal(s.T(), http.StatusNotFound, rr.Code, "handler returned wrong status code") @@ -107,15 +109,15 @@ func (s *TestUsernamesSuite) TestUsernamesGetHandler() { // Create Usernames controller instance. ctrl := controller.NewUsernames(nsClient) - handler := gin.HandlerFunc(ctrl.GetHandler) s.Run("unable to get mur", func() { // We create a ResponseRecorder (which satisfies http.ResponseWriter) to record the response. rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req - ctx.AddParam("username", "noise") + ctx := echo.New().NewContext(req, rr) + ctx.SetParamNames("username") + ctx.SetParamValues("noise") - handler(ctx) + err := ctrl.GetHandler(ctx) + require.NoError(s.T(), err) // Check the error is what we expect. test.AssertError(s.T(), rr, http.StatusInternalServerError, "mock error", "error getting MasterUserRecord resource") diff --git a/pkg/errors/error.go b/pkg/errors/error.go index 9b1c31fe..f6b7f67f 100644 --- a/pkg/errors/error.go +++ b/pkg/errors/error.go @@ -4,7 +4,7 @@ import ( "fmt" "net/http" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" ) type Error struct { @@ -15,8 +15,8 @@ type Error struct { } // AbortWithError stops the chain, writes the status code and the given error -func AbortWithError(ctx *gin.Context, code int, err error, details string) { - ctx.AbortWithStatusJSON(code, &Error{ +func AbortWithError(ctx echo.Context, code int, err error, details string) error { + return ctx.JSON(code, &Error{ Status: http.StatusText(code), Code: code, Message: err.Error(), diff --git a/pkg/errors/error_test.go b/pkg/errors/error_test.go index c7b3e416..5e410781 100644 --- a/pkg/errors/error_test.go +++ b/pkg/errors/error_test.go @@ -10,7 +10,7 @@ import ( errs "github.com/codeready-toolchain/registration-service/pkg/errors" "github.com/codeready-toolchain/registration-service/test" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "gotest.tools/assert" @@ -25,18 +25,21 @@ func TestRunErrorsSuite(t *testing.T) { } func (s *TestErrorsSuite) TestErrors() { + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) s.Run("check json error payload", func() { details := "testing payload" errMsg := "testing new error" code := http.StatusInternalServerError - errs.AbortWithError(ctx, code, errors.New(errMsg), details) + err := errs.AbortWithError(ctx, code, errors.New(errMsg), details) + require.NoError(s.T(), err) res := errs.Error{} - err := json.Unmarshal(rr.Body.Bytes(), &res) + err = json.Unmarshal(rr.Body.Bytes(), &res) require.NoError(s.T(), err) assert.Equal(s.T(), res.Code, http.StatusInternalServerError) diff --git a/pkg/log/log.go b/pkg/log/log.go index 22195e01..636dc7e6 100644 --- a/pkg/log/log.go +++ b/pkg/log/log.go @@ -14,7 +14,6 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/context" "github.com/labstack/echo/v4" - "github.com/gin-gonic/gin" "github.com/go-logr/logr" sync "github.com/matryer/resync" "github.com/spf13/pflag" @@ -92,27 +91,27 @@ func newLogger(withName string) *Logger { } // Info logs a non-error message. -func Info(ctx *gin.Context, msg string) { +func Info(ctx echo.Context, msg string) { logger.Info(ctx, msg) } // Infof logs a non-error formatted message. -func Infof(ctx *gin.Context, msg string, args ...string) { +func Infof(ctx echo.Context, msg string, args ...string) { logger.Infof(ctx, msg, args...) } -// InfoEchof logs a non-error formatted message for echo events. +// InfoEchof logs a non-error formatted message with extra echo-specific fields (workspace, impersonate-user, public-viewer). func InfoEchof(ctx echo.Context, msg string, args ...string) { logger.InfoEchof(ctx, msg, args...) } // Error logs the error with the given message. -func Error(ctx *gin.Context, err error, msg string) { +func Error(ctx echo.Context, err error, msg string) { logger.Error(ctx, err, msg) } // Errorf logs the error with the given formatted message. -func Errorf(ctx *gin.Context, err error, msg string, args ...string) { +func Errorf(ctx echo.Context, err error, msg string, args ...string) { logger.Errorf(ctx, err, msg, args...) } @@ -122,18 +121,18 @@ func WithValues(keysAndValues map[string]interface{}) *Logger { } // Info logs a non-error message. -func (l *Logger) Info(ctx *gin.Context, msg string) { +func (l *Logger) Info(ctx echo.Context, msg string) { ctxInfo := addContextInfo(ctx) l.logr.Info(msg, ctxInfo...) } // Infof logs a non-error formatted message. -func (l *Logger) Infof(ctx *gin.Context, msg string, args ...string) { +func (l *Logger) Infof(ctx echo.Context, msg string, args ...string) { ctxInfo := addContextInfo(ctx) l.infof(ctxInfo, msg, args...) } -// InfoEchof logs a non-error formatted message for echo events. +// InfoEchof logs a non-error formatted message with extra echo-specific fields (workspace, impersonate-user, public-viewer). func (l *Logger) InfoEchof(ctx echo.Context, msg string, args ...string) { userID, _ := ctx.Get(context.SubKey).(string) username, _ := ctx.Get(context.UsernameKey).(string) @@ -173,12 +172,12 @@ func (l *Logger) infof(ctx []interface{}, msg string, args ...string) { } // Error logs the error with the given message. -func (l *Logger) Error(ctx *gin.Context, err error, msg string) { +func (l *Logger) Error(ctx echo.Context, err error, msg string) { l.Errorf(ctx, err, msg) } // Errorf logs the error with the given formatted message. -func (l *Logger) Errorf(ctx *gin.Context, err error, msg string, args ...string) { +func (l *Logger) Errorf(ctx echo.Context, err error, msg string, args ...string) { ctxInfo := addContextInfo(ctx) arguments := make([]interface{}, len(args)) for i, arg := range args { @@ -213,13 +212,13 @@ func slice(keysAndValues map[string]interface{}) []interface{} { // addContextInfo adds fields extracted from the context to the info/error // log messages. -func addContextInfo(ctx *gin.Context) []interface{} { +func addContextInfo(ctx echo.Context) []interface{} { if ctx != nil { - subject := ctx.GetString(context.SubKey) - username := ctx.GetString(context.UsernameKey) + subject := context.GetString(ctx, context.SubKey) + username := context.GetString(ctx, context.UsernameKey) fields := genericContext(subject, username) - if ctx.Request != nil { - fields = append(fields, addRequestInfo(ctx.Request)...) + if ctx.Request() != nil { + fields = append(fields, addRequestInfo(ctx.Request())...) } return fields } diff --git a/pkg/log/log_test.go b/pkg/log/log_test.go index cf372e5a..7e0c9e3e 100644 --- a/pkg/log/log_test.go +++ b/pkg/log/log_test.go @@ -11,7 +11,6 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/configuration" "github.com/codeready-toolchain/registration-service/pkg/context" - "github.com/gin-gonic/gin" "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -26,8 +25,10 @@ func TestLog(t *testing.T) { }) t.Run("log info", func(t *testing.T) { + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) ctx.Set("subject", "test") ctx.Set("username", "usernametest") @@ -44,8 +45,10 @@ func TestLog(t *testing.T) { t.Run("log infof", func(t *testing.T) { buf.Reset() + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) ctx.Set(context.SubKey, "test") ctx.Set(context.UsernameKey, "usernametest") @@ -126,8 +129,10 @@ func TestLog(t *testing.T) { t.Run("log infof with no arguments", func(t *testing.T) { buf.Reset() + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) Infof(ctx, "test") value := buf.String() @@ -137,8 +142,10 @@ func TestLog(t *testing.T) { t.Run("log error", func(t *testing.T) { buf.Reset() + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) Error(ctx, errors.New("test error"), "test error with no formatting") value := buf.String() @@ -151,8 +158,10 @@ func TestLog(t *testing.T) { t.Run("log errorf", func(t *testing.T) { buf.Reset() + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) Errorf(ctx, errors.New("test error"), "test %s", "info") value := buf.String() @@ -165,15 +174,14 @@ func TestLog(t *testing.T) { t.Run("log infof with http request", func(t *testing.T) { buf.Reset() + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - req := httptest.NewRequest("GET", "http://example.com/api/v1/health", nil) req.Header.Add("Accept", "application/json") q := req.URL.Query() q.Add("query_key", "query_value") req.URL.RawQuery = q.Encode() - ctx.Request = req + ctx := e.NewContext(req, rr) Infof(ctx, "test %s", "info") value := buf.String() @@ -189,8 +197,8 @@ func TestLog(t *testing.T) { t.Run("log infof with http request containing authorization header", func(t *testing.T) { buf.Reset() + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) data := `{"testing-body":"test"}` req := httptest.NewRequest("GET", "http://example.com/api/v1/health", strings.NewReader(data)) @@ -201,7 +209,7 @@ func TestLog(t *testing.T) { q.Add("query_key", "query_value") q.Add("token", "query_token") req.URL.RawQuery = q.Encode() - ctx.Request = req + ctx := e.NewContext(req, rr) Infof(ctx, "test %s", "info") value := buf.String() @@ -225,8 +233,10 @@ func TestLog(t *testing.T) { t.Run("log infof withValues", func(t *testing.T) { buf.Reset() + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) ctx.Set("subject", "test") WithValues(map[string]interface{}{"testing": "with-values"}).Infof(ctx, "test %s", "info") @@ -241,8 +251,10 @@ func TestLog(t *testing.T) { t.Run("log infof with empty values", func(t *testing.T) { buf.Reset() + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) WithValues(map[string]interface{}{}).Infof(ctx, "test %s", "info") value := buf.String() @@ -253,8 +265,10 @@ func TestLog(t *testing.T) { t.Run("log infof with nil values", func(t *testing.T) { buf.Reset() + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) WithValues(nil).Infof(ctx, "test %s", "info") value := buf.String() @@ -265,8 +279,10 @@ func TestLog(t *testing.T) { t.Run("log infof setOutput when tags is set", func(t *testing.T) { buf.Reset() + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) ctx.Set("subject", "test") WithValues(map[string]interface{}{"testing-2": "with-values-2"}).Infof(ctx, "test %s", "info") diff --git a/pkg/middleware/jwt_middleware.go b/pkg/middleware/jwt_middleware.go index 586c6d59..b23855da 100644 --- a/pkg/middleware/jwt_middleware.go +++ b/pkg/middleware/jwt_middleware.go @@ -9,7 +9,7 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/context" "github.com/codeready-toolchain/registration-service/pkg/log" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" ) // JWTMiddleware is the JWT token validation middleware @@ -28,75 +28,63 @@ func NewAuthMiddleware() (*JWTMiddleware, error) { }, nil } -func (m *JWTMiddleware) extractToken(c *gin.Context) (string, error) { - // token lookup: header: Authorization - // try header field "Authorization" (will be "" when n/a) - headerToken := c.GetHeader("Authorization") +func (m *JWTMiddleware) extractToken(c echo.Context) (string, error) { + headerToken := c.Request().Header.Get("Authorization") if headerToken != "" { if strings.HasPrefix(headerToken, "Bearer ") { - // it is a bearer token, split it up and return it s := strings.Fields(headerToken) if len(s) == 2 { return s[1], nil } - // we're failing fast here, if there is an Authorization header, it is used or it fails return "", errors.New("found bearer token header, but no token:" + headerToken) } - // see above, failing fast return "", errors.New("found unknown authorization header:" + headerToken) } return "", errors.New("no token found") } -func (m *JWTMiddleware) respondWithError(c *gin.Context, code int, message interface{}) { - c.AbortWithStatusJSON(code, gin.H{"error": message}) +func (m *JWTMiddleware) respondWithError(c echo.Context, code int, message interface{}) error { + return c.JSON(code, echo.Map{"error": message}) } -// HandlerFunc returns the HanderFunc. -func (m *JWTMiddleware) HandlerFunc() gin.HandlerFunc { - return func(c *gin.Context) { - // check if we have a token - tokenStr, err := m.extractToken(c) - if err != nil { - m.respondWithError(c, http.StatusUnauthorized, err.Error()) - return - } - // next, check the token - token, err := m.tokenParser.FromString(tokenStr) - if err != nil { - m.respondWithError(c, http.StatusUnauthorized, err.Error()) - return - } +// HandlerFunc returns the Echo MiddlewareFunc. +func (m *JWTMiddleware) HandlerFunc() echo.MiddlewareFunc { + return func(next echo.HandlerFunc) echo.HandlerFunc { + return func(c echo.Context) error { + tokenStr, err := m.extractToken(c) + if err != nil { + return m.respondWithError(c, http.StatusUnauthorized, err.Error()) + } + token, err := m.tokenParser.FromString(tokenStr) + if err != nil { + return m.respondWithError(c, http.StatusUnauthorized, err.Error()) + } + + if token.UserID == "" || token.AccountID == "" { + rawClaims := "" - if token.UserID == "" || token.AccountID == "" { - rawClaims := "" + parts := strings.Split(tokenStr, ".") - // Tokens consist of 3 segments; header, claims and signature - parts := strings.Split(tokenStr, ".") + if len(parts) >= 2 { + rawClaims = parts[1] + } - // Just capture the claims segment - if len(parts) >= 2 { - rawClaims = parts[1] + log.Infof(c, "Missing essential claims from token - [user_id:%s][account_id:%s] for user [%s], sub [%s]. Raw claims segment: [%s]", + token.UserID, token.AccountID, token.PreferredUsername, token.Subject, rawClaims) } - log.Infof(c, "Missing essential claims from token - [user_id:%s][account_id:%s] for user [%s], sub [%s]. Raw claims segment: [%s]", - token.UserID, token.AccountID, token.PreferredUsername, token.Subject, rawClaims) + c.Set(context.UserIDKey, token.UserID) + c.Set(context.AccountIDKey, token.AccountID) + c.Set(context.AccountNumberKey, token.AccountNumber) + c.Set(context.UsernameKey, token.PreferredUsername) + c.Set(context.EmailKey, token.Email) + c.Set(context.SubKey, token.Subject) + c.Set(context.OriginalSubKey, token.OriginalSub) + c.Set(context.GivenNameKey, token.GivenName) + c.Set(context.FamilyNameKey, token.FamilyName) + c.Set(context.CompanyKey, token.Company) + c.Set(context.JWTClaimsKey, token) + return next(c) } - - // all checks done, add username, subject and email to the context. - // the tokenparser has already checked these claims are in the token at this point. - c.Set(context.UserIDKey, token.UserID) - c.Set(context.AccountIDKey, token.AccountID) - c.Set(context.AccountNumberKey, token.AccountNumber) - c.Set(context.UsernameKey, token.PreferredUsername) - c.Set(context.EmailKey, token.Email) - c.Set(context.SubKey, token.Subject) - c.Set(context.OriginalSubKey, token.OriginalSub) - c.Set(context.GivenNameKey, token.GivenName) - c.Set(context.FamilyNameKey, token.FamilyName) - c.Set(context.CompanyKey, token.Company) - // for convenience, add the claims to the context. - c.Set(context.JWTClaimsKey, token) - c.Next() } } diff --git a/pkg/middleware/promhttp_middleware.go b/pkg/middleware/promhttp_middleware.go index aa340b2e..3dd729a1 100644 --- a/pkg/middleware/promhttp_middleware.go +++ b/pkg/middleware/promhttp_middleware.go @@ -4,46 +4,48 @@ import ( "strconv" "time" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" "github.com/prometheus/client_golang/prometheus" ) // see https://pkg.go.dev/github.com/prometheus/client_golang/prometheus/promhttp#example-InstrumentRoundTripperDuration -func InstrumentRoundTripperInFlight(gauge prometheus.Gauge) gin.HandlerFunc { - return func(c *gin.Context) { - gauge.Inc() - defer func() { - gauge.Dec() - }() - c.Next() +func InstrumentRoundTripperInFlight(gauge prometheus.Gauge) echo.MiddlewareFunc { + return func(next echo.HandlerFunc) echo.HandlerFunc { + return func(c echo.Context) error { + gauge.Inc() + defer gauge.Dec() + return next(c) + } } } -func InstrumentRoundTripperCounter(counter *prometheus.CounterVec) gin.HandlerFunc { - return func(c *gin.Context) { - defer func() { +func InstrumentRoundTripperCounter(counter *prometheus.CounterVec) echo.MiddlewareFunc { + return func(next echo.HandlerFunc) echo.HandlerFunc { + return func(c echo.Context) error { + err := next(c) counter.With(prometheus.Labels{ - "code": strconv.Itoa(c.Writer.Status()), - "method": c.Request.Method, - "path": c.Request.URL.Path, + "code": strconv.Itoa(c.Response().Status), + "method": c.Request().Method, + "path": c.Request().URL.Path, }).Inc() - }() - c.Next() + return err + } } } -func InstrumentRoundTripperDuration(histVec *prometheus.HistogramVec) gin.HandlerFunc { - return func(c *gin.Context) { - start := time.Now() - defer func() { +func InstrumentRoundTripperDuration(histVec *prometheus.HistogramVec) echo.MiddlewareFunc { + return func(next echo.HandlerFunc) echo.HandlerFunc { + return func(c echo.Context) error { + start := time.Now() + err := next(c) duration := time.Since(start) histVec.With(prometheus.Labels{ - "code": strconv.Itoa(c.Writer.Status()), - "method": c.Request.Method, - "path": c.Request.URL.Path, + "code": strconv.Itoa(c.Response().Status), + "method": c.Request().Method, + "path": c.Request().URL.Path, }).Observe(float64(duration.Seconds())) - }() - c.Next() + return err + } } } diff --git a/pkg/namespaces/namespaces_manager.go b/pkg/namespaces/namespaces_manager.go index b66c8dc3..f212396a 100644 --- a/pkg/namespaces/namespaces_manager.go +++ b/pkg/namespaces/namespaces_manager.go @@ -10,7 +10,7 @@ import ( customCtx "github.com/codeready-toolchain/registration-service/pkg/context" "github.com/codeready-toolchain/registration-service/pkg/namespaced" "github.com/codeready-toolchain/toolchain-common/pkg/cluster" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -45,7 +45,7 @@ func (e ErrUserHasNoProvisionedNamespaces) Error() string { type Manager interface { // ResetNamespaces locates the user's namespaces in their corresponding member clusters and deletes them, so that // the NSTemplate controller can recreate them. - ResetNamespaces(ginCtx *gin.Context) error + ResetNamespaces(ctx echo.Context) error } type manager struct { @@ -63,11 +63,11 @@ func NewNamespacesManager(getMemberClustersFunc cluster.GetMemberClustersFunc, h } } -func (mgr *manager) ResetNamespaces(ginCtx *gin.Context) error { +func (mgr *manager) ResetNamespaces(ctx echo.Context) error { // Grab the corresponding user signup resource to get the user's compliant // username, since that is the one that is used across the Developer // Sandbox resources. - userSignup, err := mgr.signupService.GetSignup(ginCtx, ginCtx.GetString(customCtx.UsernameKey), true) + userSignup, err := mgr.signupService.GetSignup(ctx, customCtx.GetString(ctx, customCtx.UsernameKey), true) if err != nil { return fmt.Errorf("unable to obtain the user signup: %w", err) } @@ -84,7 +84,7 @@ func (mgr *manager) ResetNamespaces(ginCtx *gin.Context) error { // Fetch the user's space. var userSpace toolchainv1alpha1.Space - err = mgr.hostNamespaceClient.Get(ginCtx.Request.Context(), types.NamespacedName{Namespace: mgr.hostNamespaceClient.Namespace, Name: compliantUsername}, &userSpace) + err = mgr.hostNamespaceClient.Get(ctx.Request().Context(), types.NamespacedName{Namespace: mgr.hostNamespaceClient.Namespace, Name: compliantUsername}, &userSpace) if err != nil { return fmt.Errorf(`unable to get user's space resource: %w`, err) } @@ -107,7 +107,7 @@ func (mgr *manager) ResetNamespaces(ginCtx *gin.Context) error { // Obtain the user's NSTemplateSet to be able to determine which namespaces we are deleting. var nsTemplateSet toolchainv1alpha1.NSTemplateSet - err := memberCluster.Client.Get(ginCtx.Request.Context(), types.NamespacedName{Namespace: memberCluster.OperatorNamespace, Name: compliantUsername}, &nsTemplateSet) + err := memberCluster.Client.Get(ctx.Request().Context(), types.NamespacedName{Namespace: memberCluster.OperatorNamespace, Name: compliantUsername}, &nsTemplateSet) if err != nil { return fmt.Errorf(`unable to get the "NSTemplateSet" resource for the user in cluster "%s": %w`, memberCluster.Name, err) } @@ -122,7 +122,7 @@ func (mgr *manager) ResetNamespaces(ginCtx *gin.Context) error { // failing with a "the server does not allow this method on the // requested resource" error. for _, namespace := range nsTemplateSet.Status.ProvisionedNamespaces { - err := memberCluster.Client.Delete(ginCtx.Request.Context(), &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace.Name}}) + err := memberCluster.Client.Delete(ctx.Request().Context(), &v1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace.Name}}) if err != nil && !apierrors.IsNotFound(err) { return fmt.Errorf(`unable to delete user namespace "%s" in cluster "%s": %w`, namespace.Name, memberCluster.Name, err) } diff --git a/pkg/namespaces/namespaces_manager_test.go b/pkg/namespaces/namespaces_manager_test.go index d5305257..0d22c055 100644 --- a/pkg/namespaces/namespaces_manager_test.go +++ b/pkg/namespaces/namespaces_manager_test.go @@ -16,7 +16,7 @@ import ( "github.com/codeready-toolchain/registration-service/test/fake" "github.com/codeready-toolchain/toolchain-common/pkg/cluster" commontest "github.com/codeready-toolchain/toolchain-common/pkg/test" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -98,7 +98,7 @@ func TestRunNamespacesManagerSuite(t *testing.T) { // TestResetNamespaces provides multiple unit tests which test the different // paths of the "ResetNamespaces" feature of the "NamespacesManager" type. func (nms *TestNamespacesManagerSuite) TestResetNamespaces() { - // Prepare a request for Gin. Even though we are testing the underlying + // Prepare a request for Echo. Even though we are testing the underlying // service, we do pull the original context from the request to apply // timeouts in the service, so it needs to be there. req, err := http.NewRequest(http.MethodPost, "/api/v1/reset-namespaces", nil) @@ -107,9 +107,9 @@ func (nms *TestNamespacesManagerSuite) TestResetNamespaces() { return } + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) - ctx.Request = req + ctx := e.NewContext(req, rr) ctx.Set(context.UsernameKey, TestUsername) // Create all the standard fixtures for the tests. diff --git a/pkg/proxy/handlers/spacelister.go b/pkg/proxy/handlers/spacelister.go index 08c375d5..73f789d7 100644 --- a/pkg/proxy/handlers/spacelister.go +++ b/pkg/proxy/handlers/spacelister.go @@ -10,7 +10,6 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/proxy/metrics" "github.com/codeready-toolchain/registration-service/pkg/signup" commonproxy "github.com/codeready-toolchain/toolchain-common/pkg/proxy" - "github.com/gin-gonic/gin" "github.com/labstack/echo/v4" errs "github.com/pkg/errors" @@ -28,7 +27,7 @@ const ( type SpaceLister struct { namespaced.Client - GetSignupFunc func(ctx *gin.Context, username string, checkUserSignupCompleted bool) (*signup.Signup, error) + GetSignupFunc func(ctx echo.Context, username string, checkUserSignupCompleted bool) (*signup.Signup, error) ProxyMetrics *metrics.ProxyMetrics } @@ -49,7 +48,6 @@ func (s *SpaceLister) GetProvisionedUserSignup(ctx echo.Context) (*signup.Signup return nil, err } if userSignup == nil || userSignup.CompliantUsername == "" { - // account exists but the compliant username is not set yet, meaning it has not been fully provisioned yet, so return an empty list return nil, nil } return userSignup, nil diff --git a/pkg/proxy/handlers/spacelister_get_test.go b/pkg/proxy/handlers/spacelister_get_test.go index 0b35487e..1f31989f 100644 --- a/pkg/proxy/handlers/spacelister_get_test.go +++ b/pkg/proxy/handlers/spacelister_get_test.go @@ -21,7 +21,6 @@ import ( commonproxy "github.com/codeready-toolchain/toolchain-common/pkg/proxy" "github.com/codeready-toolchain/toolchain-common/pkg/test" spacebindingrequesttest "github.com/codeready-toolchain/toolchain-common/pkg/test/spacebindingrequest" - "github.com/gin-gonic/gin" "github.com/labstack/echo/v4" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" @@ -76,7 +75,7 @@ func testSpaceListerGet(t *testing.T, publicViewerEnabled bool) { expectedErr string expectedErrCode int expectedWorkspace string - overrideSignupFunc func(ctx *gin.Context, username string, checkUserSignupComplete bool) (*signup.Signup, error) + overrideSignupFunc func(ctx echo.Context, username string, checkUserSignupComplete bool) (*signup.Signup, error) mockFakeClient func(fakeClient *test.FakeClient) overrideGetMembersFunc func(conditions ...commoncluster.Condition) []*commoncluster.CachedToolchainCluster overrideMemberClient *test.FakeClient @@ -270,7 +269,7 @@ func testSpaceListerGet(t *testing.T, publicViewerEnabled bool) { expectedWs: nil, expectedErr: "signup error", expectedErrCode: 500, - overrideSignupFunc: func(_ *gin.Context, _ string, _ bool) (*signup.Signup, error) { + overrideSignupFunc: func(_ echo.Context, _ string, _ bool) (*signup.Signup, error) { return nil, fmt.Errorf("signup error") }, expectedWorkspace: "dancelover", @@ -620,7 +619,7 @@ func TestGetUserWorkspace(t *testing.T) { workspaceRequest string expectedWorkspace func(t *testing.T, fakeClient *test.FakeClient) toolchainv1alpha1.Workspace mockFakeClient func(fakeClient *test.FakeClient) - overrideSignupFunc func(ctx *gin.Context, username string, checkUserSignupComplete bool) (*signup.Signup, error) + overrideSignupFunc func(ctx echo.Context, username string, checkUserSignupComplete bool) (*signup.Signup, error) }{ "get robin workspace": { username: "robin", @@ -673,7 +672,7 @@ func TestGetUserWorkspace(t *testing.T) { username: "batman", workspaceRequest: "batman", expectedErr: "signup error", - overrideSignupFunc: func(_ *gin.Context, _ string, _ bool) (*signup.Signup, error) { + overrideSignupFunc: func(_ echo.Context, _ string, _ bool) (*signup.Signup, error) { return nil, fmt.Errorf("signup error") }, expectedWorkspace: nil, diff --git a/pkg/proxy/handlers/spacelister_list_test.go b/pkg/proxy/handlers/spacelister_list_test.go index c2fe46b8..6820f607 100644 --- a/pkg/proxy/handlers/spacelister_list_test.go +++ b/pkg/proxy/handlers/spacelister_list_test.go @@ -10,7 +10,6 @@ import ( "time" "github.com/codeready-toolchain/registration-service/pkg/namespaced" - "github.com/gin-gonic/gin" "github.com/labstack/echo/v4" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" @@ -122,7 +121,7 @@ func TestHandleSpaceListRequest(t *testing.T) { expectedErr string expectedErrCode int expectedWorkspace string - overrideSignupFunc func(ctx *gin.Context, username string, checkUserSignupComplete bool) (*signup.Signup, error) + overrideSignupFunc func(ctx echo.Context, username string, checkUserSignupComplete bool) (*signup.Signup, error) mockFakeClient func(fakeClient *test.FakeClient) }{ "dancelover lists spaces": { @@ -181,7 +180,7 @@ func TestHandleSpaceListRequest(t *testing.T) { expectedWs: nil, expectedErr: "signup error", expectedErrCode: 500, - overrideSignupFunc: func(_ *gin.Context, _ string, _ bool) (*signup.Signup, error) { + overrideSignupFunc: func(_ echo.Context, _ string, _ bool) (*signup.Signup, error) { return nil, fmt.Errorf("signup error") }, }, diff --git a/pkg/proxy/proxy_test.go b/pkg/proxy/proxy_test.go index 0ce94a66..829cb493 100644 --- a/pkg/proxy/proxy_test.go +++ b/pkg/proxy/proxy_test.go @@ -25,8 +25,8 @@ import ( "github.com/codeready-toolchain/registration-service/test" "github.com/codeready-toolchain/registration-service/test/fake" "github.com/codeready-toolchain/registration-service/test/util" - "github.com/gin-gonic/gin" "github.com/google/uuid" + "github.com/labstack/echo/v4" "github.com/prometheus/client_golang/prometheus" "k8s.io/client-go/kubernetes/scheme" @@ -515,7 +515,7 @@ func (s *TestProxySuite) checkProxyOK(proxy *Proxy) { ExpectedProxyResponseStatus int Standalone bool // If true then the request is not expected to be forwarded to the kube api server - OverrideGetSignupFunc func(ctx *gin.Context, username string, checkUserSignupCompleted bool) (*signup.Signup, error) + OverrideGetSignupFunc func(ctx echo.Context, username string, checkUserSignupCompleted bool) (*signup.Signup, error) ExpectedResponse *string }{ "plain http cors preflight request with no request method": { @@ -680,7 +680,7 @@ func (s *TestProxySuite) checkProxyOK(proxy *Proxy) { "Authorization": {"Bearer clusterSAToken"}, }, ExpectedProxyResponseStatus: http.StatusInternalServerError, - OverrideGetSignupFunc: func(_ *gin.Context, _ string, _ bool) (*signup.Signup, error) { + OverrideGetSignupFunc: func(_ echo.Context, _ string, _ bool) (*signup.Signup, error) { return nil, fmt.Errorf("test error") }, ExpectedResponse: ptr("unable to retrieve user workspaces: test error"), diff --git a/pkg/server/metrics_server.go b/pkg/server/metrics_server.go index 637e9223..7b8fd166 100644 --- a/pkg/server/metrics_server.go +++ b/pkg/server/metrics_server.go @@ -6,7 +6,7 @@ import ( "net/http" "time" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -16,10 +16,11 @@ const RegSvcMetricsPort = 8083 var log = logf.Log.WithName("registration_metrics") -// return a new Gin server exposing the `/metrics` endpoint associated with the given Prometheus registry -func StartMetricsServer(reg *prometheus.Registry, port int) (*http.Server, *gin.Engine) { - router := gin.Default() - router.GET("/metrics", gin.WrapH(promhttp.InstrumentMetricHandler( +func StartMetricsServer(reg *prometheus.Registry, port int) (*http.Server, *echo.Echo) { + router := echo.New() + router.HideBanner = true + router.HidePort = true + router.GET("/metrics", echo.WrapHandler(promhttp.InstrumentMetricHandler( reg, promhttp.HandlerFor(reg, promhttp.HandlerOpts{ DisableCompression: true, }), @@ -28,15 +29,14 @@ func StartMetricsServer(reg *prometheus.Registry, port int) (*http.Server, *gin. log.Info("Starting the registration-service metrics server...") srv := &http.Server{ Addr: fmt.Sprintf(":%d", port), - Handler: router.Handler(), + Handler: router, ReadHeaderTimeout: 2 * time.Second, TLSConfig: &tls.Config{ MinVersion: tls.VersionTLS12, - NextProtos: []string{"http/1.1"}, // disable HTTP/2 for now + NextProtos: []string{"http/1.1"}, }, } go func() { - // service connections if err := srv.ListenAndServe(); err != nil && err != http.ErrServerClosed { log.Error(err, err.Error()) } diff --git a/pkg/server/routes.go b/pkg/server/routes.go index a992e15f..40ffd38b 100644 --- a/pkg/server/routes.go +++ b/pkg/server/routes.go @@ -1,6 +1,7 @@ package server import ( + "io/fs" "time" "github.com/codeready-toolchain/registration-service/pkg/assets" @@ -12,9 +13,8 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/namespaced" "github.com/codeready-toolchain/registration-service/pkg/namespaces" "github.com/codeready-toolchain/toolchain-common/pkg/cluster" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" - "github.com/gin-contrib/static" errs "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" ) @@ -70,11 +70,11 @@ func (srv *RegistrationServer) SetupRoutes(proxyPort string, reg *prometheus.Reg middleware.InstrumentRoundTripperInFlight(inFlightGauge), middleware.InstrumentRoundTripperCounter(counter), middleware.InstrumentRoundTripperDuration(histVec)) - unsecuredV1.GET("/health", healthCheckCtrl.GetHandler) // TODO: move to root (`/`)? + unsecuredV1.GET("/health", healthCheckCtrl.GetHandler) unsecuredV1.GET("/authconfig", authConfigCtrl.GetHandler) // segment keys endpoints - unsecuredV1.GET("/segment-write-key", analyticsCtrl.GetDevSpacesSegmentWriteKey) // expose the devspaces segment key - unsecuredV1.GET("/analytics/segment-write-key", analyticsCtrl.GetSandboxSegmentWriteKey) // expose the sandbox segment key.We had the create a new analytics endpoint to keep backward compatibility with devspaces. + unsecuredV1.GET("/segment-write-key", analyticsCtrl.GetDevSpacesSegmentWriteKey) + unsecuredV1.GET("/analytics/segment-write-key", analyticsCtrl.GetSandboxSegmentWriteKey) // create the auth middleware var authMiddleware *middleware.JWTMiddleware @@ -83,8 +83,11 @@ func (srv *RegistrationServer) SetupRoutes(proxyPort string, reg *prometheus.Reg err = errs.Wrapf(err, "failed to init auth middleware") return } - receivedTimeMw := func(ctx *gin.Context) { - ctx.Set(rcontext.RequestReceivedTime, time.Now()) + receivedTimeMw := func(next echo.HandlerFunc) echo.HandlerFunc { + return func(ctx echo.Context) error { + ctx.Set(rcontext.RequestReceivedTime, time.Now()) + return next(ctx) + } } // secured routes securedV1 := srv.router.Group("/api/v1") @@ -110,13 +113,13 @@ func (srv *RegistrationServer) SetupRoutes(proxyPort string, reg *prometheus.Reg } // Create the route for static content, served from / - var staticHandler static.ServeFileSystem - staticHandler, err = assets.ServeEmbedContent() + var staticFS fs.FS + staticFS, err = assets.ServeEmbedContent() if err != nil { err = errs.Wrap(err, "unable to setup route to serve static content") + return } - srv.router.Use(static.Serve("/", staticHandler)) - + srv.router.StaticFS("/", echo.MustSubFS(staticFS, ".")) }) return err } diff --git a/pkg/server/routes_test.go b/pkg/server/routes_test.go index a072fb6c..3bd740af 100644 --- a/pkg/server/routes_test.go +++ b/pkg/server/routes_test.go @@ -8,9 +8,7 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/assets" "github.com/codeready-toolchain/registration-service/pkg/log" - "github.com/gin-contrib/static" - - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -18,16 +16,10 @@ import ( func TestStaticContent(t *testing.T) { log.Init("registration-service-testing") - router := gin.Default() - staticHandler, err := assets.ServeEmbedContent() + router := echo.New() + staticFS, err := assets.ServeEmbedContent() require.NoError(t, err) - router.Use(static.Serve("/", staticHandler)) - - router.RedirectTrailingSlash = true - - w := httptest.NewRecorder() - req, _ := http.NewRequest("GET", "/ping", nil) - router.ServeHTTP(w, req) + router.StaticFS("/", echo.MustSubFS(staticFS, ".")) // Setting up the table test var statictests = []struct { @@ -43,7 +35,7 @@ func TestStaticContent(t *testing.T) { { requestMethod: "GET", requestPath: "/index.html", - assertResponse: movedTo("./"), + assertResponse: okWithBodyFromEmbedFS("static/index.html"), }, { requestMethod: "GET", @@ -86,13 +78,6 @@ func okWithBodyFromEmbedFS(path string) assertResponse { } } -func movedTo(path string) assertResponse { - return func(t *testing.T, rr *httptest.ResponseRecorder) { - require.Equal(t, http.StatusMovedPermanently, rr.Code, "handler returned wrong status code: got %v want %v", rr.Code, http.StatusTemporaryRedirect) - assert.Equal(t, []string{path}, rr.Header()["Location"]) - } -} - func notFound() assertResponse { return func(t *testing.T, rr *httptest.ResponseRecorder) { require.Equal(t, http.StatusNotFound, rr.Code, "handler returned wrong status code: got %v want %v", rr.Code, http.StatusNotFound) diff --git a/pkg/server/server.go b/pkg/server/server.go index 5d6a952e..89c69f58 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -3,9 +3,7 @@ package server import ( "crypto/tls" "fmt" - "io" "net/http" - "os" "strings" "sync" "time" @@ -13,9 +11,8 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/application" "github.com/codeready-toolchain/registration-service/pkg/configuration" - "github.com/gin-contrib/cors" - "github.com/gin-contrib/gzip" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" + "github.com/labstack/echo/v4/middleware" ) type ServerOption = func(server *RegistrationServer) // nolint:revive @@ -23,46 +20,54 @@ type ServerOption = func(server *RegistrationServer) // nolint:revive // RegistrationServer bundles configuration, and HTTP server objects in a single // location. type RegistrationServer struct { - router *gin.Engine + router *echo.Echo httpServer *http.Server routesSetup sync.Once - //applicationProducerFunc func() application.Application application application.Application } // New creates a new RegistrationServer object with reasonable defaults. func New(application application.Application) *RegistrationServer { + router := echo.New() + router.HideBanner = true + router.HidePort = true - gin.SetMode(gin.ReleaseMode) - ginRouter := gin.New() - ginRouter.Use( - gin.LoggerWithConfig(gin.LoggerConfig{ - Output: gin.DefaultWriter, - SkipPaths: []string{"/api/v1/health"}, // disable logging for the /api/v1/health endpoint so that our logs aren't overwhelmed - Formatter: func(params gin.LogFormatterParams) string { - // custom JSON format - return fmt.Sprintf(`{"level":"%s", "client-ip":"%s", "ts":"%s", "method":"%s", "path":"%s", "proto":"%s", "status":"%d", "latency":"%s", "user-agent":"%s", "error-message":"%s"}`+"\n", + router.Use( + middleware.RequestLoggerWithConfig(middleware.RequestLoggerConfig{ + Skipper: func(c echo.Context) bool { + return c.Path() == "/api/v1/health" + }, + LogMethod: true, + LogURI: true, + LogStatus: true, + LogLatency: true, + LogRemoteIP: true, + LogProtocol: true, + LogUserAgent: true, + LogError: true, + LogValuesFunc: func(c echo.Context, v middleware.RequestLoggerValues) error { + errMsg := "" + if v.Error != nil { + errMsg = v.Error.Error() + } + fmt.Printf(`{"level":"%s", "client-ip":"%s", "ts":"%s", "method":"%s", "path":"%s", "proto":"%s", "status":"%d", "latency":"%s", "user-agent":"%s", "error-message":"%s"}`+"\n", "info", - params.ClientIP, - params.TimeStamp.Format(time.RFC1123), - params.Method, - params.Path, - params.Request.Proto, - params.StatusCode, - params.Latency, - params.Request.UserAgent(), - params.ErrorMessage, + v.RemoteIP, + time.Now().Format(time.RFC1123), + v.Method, + v.URI, + v.Protocol, + v.Status, + v.Latency, + v.UserAgent, + errMsg, ) + return nil }, }), - gin.Recovery(), - // When the origin header is specified, cors middleware will expose the cors functionality and the - // OPTIONS endpoint may be executed. OPTIONS will return a status code of 204 no content. - // If the origin is the same, the cors functionality is skipped and OPTIONS endpoint cannot be - // successfully called. Executing an OPTIONS request when from the same origin will result - // in a 403 forbidden response. - cors.New(cors.Config{ - AllowAllOrigins: true, + middleware.Recover(), + middleware.CORSWithConfig(middleware.CORSConfig{ + AllowOrigins: []string{"*"}, AllowMethods: []string{"PUT", "PATCH", "POST", "GET", "DELETE", "OPTIONS"}, AllowHeaders: []string{"Content-Length", "Content-Type", "Authorization", "Accept", "Recaptcha-Token"}, ExposeHeaders: []string{"Content-Length", "Authorization"}, @@ -71,26 +76,23 @@ func New(application application.Application) *RegistrationServer { ) srv := &RegistrationServer{ - router: ginRouter, + router: router, application: application, } - gin.DefaultWriter = io.MultiWriter(os.Stdout) - srv.httpServer = &http.Server{ - Addr: configuration.HTTPAddress, - // Good practice to set timeouts to avoid Slowloris attacks. + Addr: configuration.HTTPAddress, WriteTimeout: configuration.HTTPWriteTimeout, ReadTimeout: configuration.HTTPReadTimeout, IdleTimeout: configuration.HTTPIdleTimeout, Handler: srv.router, TLSConfig: &tls.Config{ MinVersion: tls.VersionTLS12, - NextProtos: []string{"http/1.1"}, // disable HTTP/2 for now + NextProtos: []string{"http/1.1"}, }, } if configuration.HTTPCompressResponses { - srv.router.Use(gzip.Gzip(gzip.DefaultCompression)) + srv.router.Use(middleware.Gzip()) } return srv } @@ -100,8 +102,8 @@ func (srv *RegistrationServer) HTTPServer() *http.Server { return srv.httpServer } -// Engine returns the app server's HTTP router. -func (srv *RegistrationServer) Engine() *gin.Engine { +// Engine returns the app server's Echo router. +func (srv *RegistrationServer) Engine() *echo.Echo { return srv.router } diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index f1104ad9..ae4cee19 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -64,7 +64,7 @@ func (s *TestServerSuite) TestServer() { s.Run("CORS", func() { go func(t *testing.T) { - err := srv.Engine().Run() + err := srv.HTTPServer().ListenAndServe() assert.NoError(t, err) // require must only be used in the goroutine running the test function (testifylint) }(s.T()) diff --git a/pkg/signup/service/signup_service.go b/pkg/signup/service/signup_service.go index 29ae36e7..b201d33f 100644 --- a/pkg/signup/service/signup_service.go +++ b/pkg/signup/service/signup_service.go @@ -23,7 +23,7 @@ import ( "github.com/codeready-toolchain/toolchain-common/pkg/hash" "github.com/codeready-toolchain/toolchain-common/pkg/states" signupcommon "github.com/codeready-toolchain/toolchain-common/pkg/usersignup" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" errs "github.com/pkg/errors" apiv1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -61,30 +61,37 @@ func NewSignupService(client namespaced.Client) *ServiceImpl { } } +func requestContext(ctx echo.Context) gocontext.Context { + if ctx != nil && ctx.Request() != nil { + return ctx.Request().Context() + } + return gocontext.TODO() +} + // newUserSignup generates a new UserSignup resource with the specified username and available claims. // This resource then can be used to create a new UserSignup in the host cluster or to update the existing one. -func (s *ServiceImpl) newUserSignup(ctx *gin.Context) (*toolchainv1alpha1.UserSignup, error) { - username := ctx.GetString(context.UsernameKey) +func (s *ServiceImpl) newUserSignup(ctx echo.Context) (*toolchainv1alpha1.UserSignup, error) { + username := context.GetString(ctx, context.UsernameKey) - userID := ctx.GetString(context.UserIDKey) - accountID := ctx.GetString(context.AccountIDKey) + userID := context.GetString(ctx, context.UserIDKey) + accountID := context.GetString(ctx, context.AccountIDKey) if userID == "" || accountID == "" { log.Infof(ctx, "Missing essential claims from token - [user_id:%s][account_id:%s] for user [%s], sub [%s]", - userID, accountID, username, ctx.GetString(context.SubKey)) + userID, accountID, username, context.GetString(ctx, context.SubKey)) } if isCRTAdmin(username) { - log.Info(ctx, fmt.Sprintf("A crtadmin user '%s' just tried to signup", ctx.GetString(context.UsernameKey))) + log.Info(ctx, fmt.Sprintf("A crtadmin user '%s' just tried to signup", context.GetString(ctx, context.UsernameKey))) return nil, apierrors.NewForbidden(schema.GroupResource{}, "", fmt.Errorf("failed to create usersignup for %s", username)) } - userEmail := ctx.GetString(context.EmailKey) + userEmail := context.GetString(ctx, context.EmailKey) emailHash := hash.EncodeString(userEmail) // Query BannedUsers to check the user has not been banned bannedUsers := &toolchainv1alpha1.BannedUserList{} - if err := s.List(ctx, bannedUsers, client.InNamespace(s.Namespace), + if err := s.List(requestContext(ctx), bannedUsers, client.InNamespace(s.Namespace), client.MatchingLabels{toolchainv1alpha1.BannedUserEmailHashLabelKey: hash.EncodeString(userEmail)}); err != nil { return nil, err } @@ -97,13 +104,13 @@ func (s *ServiceImpl) newUserSignup(ctx *gin.Context) (*toolchainv1alpha1.UserSi } verificationRequired, captchaScore, assessmentID := IsPhoneVerificationRequired(s.CaptchaChecker, ctx) - requestReceivedTime, ok := ctx.Get(context.RequestReceivedTime) - if !ok { + requestReceivedTime := ctx.Get(context.RequestReceivedTime) + if requestReceivedTime == nil { requestReceivedTime = time.Now() } userSignup := &toolchainv1alpha1.UserSignup{ ObjectMeta: metav1.ObjectMeta{ - Name: signupcommon.EncodeUserIdentifier(ctx.GetString(context.UsernameKey)), + Name: signupcommon.EncodeUserIdentifier(context.GetString(ctx, context.UsernameKey)), Namespace: configuration.Namespace(), Annotations: map[string]string{ toolchainv1alpha1.UserSignupVerificationCounterAnnotationKey: "0", @@ -118,17 +125,17 @@ func (s *ServiceImpl) newUserSignup(ctx *gin.Context) (*toolchainv1alpha1.UserSi IdentityClaims: toolchainv1alpha1.IdentityClaimsEmbedded{ PropagatedClaims: toolchainv1alpha1.PropagatedClaims{ - Sub: ctx.GetString(context.SubKey), - UserID: ctx.GetString(context.UserIDKey), - AccountID: ctx.GetString(context.AccountIDKey), - OriginalSub: ctx.GetString(context.OriginalSubKey), + Sub: context.GetString(ctx, context.SubKey), + UserID: context.GetString(ctx, context.UserIDKey), + AccountID: context.GetString(ctx, context.AccountIDKey), + OriginalSub: context.GetString(ctx, context.OriginalSubKey), Email: userEmail, }, - PreferredUsername: ctx.GetString(context.UsernameKey), - GivenName: ctx.GetString(context.GivenNameKey), - FamilyName: ctx.GetString(context.FamilyNameKey), - Company: ctx.GetString(context.CompanyKey), - AccountNumber: ctx.GetString(context.AccountNumberKey), + PreferredUsername: context.GetString(ctx, context.UsernameKey), + GivenName: context.GetString(ctx, context.GivenNameKey), + FamilyName: context.GetString(ctx, context.FamilyNameKey), + Company: context.GetString(ctx, context.CompanyKey), + AccountNumber: context.GetString(ctx, context.AccountNumberKey), }, }, } @@ -142,12 +149,12 @@ func (s *ServiceImpl) newUserSignup(ctx *gin.Context) (*toolchainv1alpha1.UserSi states.SetVerificationRequired(userSignup, verificationRequired) // set the skip-auto-create-space annotation to true if the no-space query parameter was set to true - if param, _ := ctx.GetQuery(NoSpaceKey); param == "true" { + if ctx.QueryParam(NoSpaceKey) == "true" { log.Info(ctx, fmt.Sprintf("setting '%s' annotation to true", toolchainv1alpha1.SkipAutoCreateSpaceAnnotationKey)) userSignup.Annotations[toolchainv1alpha1.SkipAutoCreateSpaceAnnotationKey] = "true" } - if socialEvent := ctx.GetString(context.SocialEvent); socialEvent != "" { + if socialEvent := context.GetString(ctx, context.SocialEvent); socialEvent != "" { event, err := signup.GetAndValidateSocialEvent(ctx, s.Client, socialEvent) if err != nil { return nil, err @@ -182,7 +189,7 @@ prevent the score from being set in the UserSignup annotation. Returns the assessment ID if a captcha assessment was completed */ -func IsPhoneVerificationRequired(captchaChecker captcha.Assessor, ctx *gin.Context) (bool, float32, string) { +func IsPhoneVerificationRequired(captchaChecker captcha.Assessor, ctx echo.Context) (bool, float32, string) { cfg := configuration.GetRegistrationServiceConfig() // skip verification if verification is disabled @@ -191,7 +198,7 @@ func IsPhoneVerificationRequired(captchaChecker captcha.Assessor, ctx *gin.Conte } // skip verification for excluded email domains - userEmail := ctx.GetString(context.EmailKey) + userEmail := context.GetString(ctx, context.EmailKey) emailHost := extractEmailHost(userEmail) for _, d := range cfg.Verification().ExcludedEmailDomains() { if strings.EqualFold(d, emailHost) { @@ -205,13 +212,13 @@ func IsPhoneVerificationRequired(captchaChecker captcha.Assessor, ctx *gin.Conte } // require verification if context is invalid - if ctx.Request == nil { + if ctx.Request() == nil { log.Error(ctx, nil, "no request in context") return true, -1, "" } // require verification if captcha token is invalid - captchaToken, exists := ctx.Request.Header["Recaptcha-Token"] + captchaToken, exists := ctx.Request().Header["Recaptcha-Token"] if !exists || len(captchaToken) != 1 { log.Error(ctx, nil, "no valid captcha token found in request header") return true, -1, "" @@ -310,13 +317,13 @@ func (s *ServiceImpl) verifyAccount(ctx *gin.Context) { // Signup reactivates the deactivated UserSignup resource or creates a new one with the specified username // if doesn't exist yet. -func (s *ServiceImpl) Signup(ctx *gin.Context) (*toolchainv1alpha1.UserSignup, error) { - username := ctx.GetString(context.UsernameKey) +func (s *ServiceImpl) Signup(ctx echo.Context) (*toolchainv1alpha1.UserSignup, error) { + username := context.GetString(ctx, context.UsernameKey) encodedUsername := signupcommon.EncodeUserIdentifier(username) // Retrieve UserSignup resource from the host cluster userSignup := &toolchainv1alpha1.UserSignup{} - if err := s.Get(ctx, s.NamespacedName(encodedUsername), userSignup); err != nil { + if err := s.Get(requestContext(ctx), s.NamespacedName(encodedUsername), userSignup); err != nil { if apierrors.IsNotFound(err) { // New Signup log.WithValues(map[string]interface{}{"encoded_username": encodedUsername}).Info(ctx, "user not found, creating a new one") @@ -339,17 +346,17 @@ func (s *ServiceImpl) Signup(ctx *gin.Context) (*toolchainv1alpha1.UserSignup, e } // createUserSignup creates a new UserSignup resource with the specified username -func (s *ServiceImpl) createUserSignup(ctx *gin.Context) (*toolchainv1alpha1.UserSignup, error) { +func (s *ServiceImpl) createUserSignup(ctx echo.Context) (*toolchainv1alpha1.UserSignup, error) { userSignup, err := s.newUserSignup(ctx) if err != nil { return nil, err } - return userSignup, s.Create(ctx, userSignup) + return userSignup, s.Create(requestContext(ctx), userSignup) } // reactivateUserSignup reactivates the deactivated UserSignup resource with the specified username -func (s *ServiceImpl) reactivateUserSignup(ctx *gin.Context, existing *toolchainv1alpha1.UserSignup) (*toolchainv1alpha1.UserSignup, error) { +func (s *ServiceImpl) reactivateUserSignup(ctx echo.Context, existing *toolchainv1alpha1.UserSignup) (*toolchainv1alpha1.UserSignup, error) { // Update the existing usersignup's spec and annotations/labels by new values from a freshly generated one. // We don't want to deal with merging/patching the usersignup resource // and just want to reset the spec and annotations/labels so they are the same as in a freshly created usersignup resource. @@ -371,7 +378,7 @@ func (s *ServiceImpl) reactivateUserSignup(ctx *gin.Context, existing *toolchain existing.Labels = newUserSignup.Labels existing.Spec = newUserSignup.Spec - return existing, s.Update(ctx, existing) + return existing, s.Update(requestContext(ctx), existing) } // GetSignup returns Signup resource which represents the corresponding K8s UserSignup @@ -379,11 +386,11 @@ func (s *ServiceImpl) reactivateUserSignup(ctx *gin.Context, existing *toolchain // The checkUserSignupCompleted was introduced in order to avoid checking the readiness of the complete condition on the UserSignup in certain situations, // such as proxy calls for example. // Returns nil, nil if the UserSignup resource is not found or if it's deactivated. -func (s *ServiceImpl) GetSignup(ctx *gin.Context, username string, checkUserSignupCompleted bool) (*signup.Signup, error) { +func (s *ServiceImpl) GetSignup(ctx echo.Context, username string, checkUserSignupCompleted bool) (*signup.Signup, error) { return s.DoGetSignup(ctx, s.Client, username, checkUserSignupCompleted) } -func (s *ServiceImpl) DoGetSignup(ctx *gin.Context, cl namespaced.Client, username string, checkUserSignupCompleted bool) (*signup.Signup, error) { +func (s *ServiceImpl) DoGetSignup(ctx echo.Context, cl namespaced.Client, username string, checkUserSignupCompleted bool) (*signup.Signup, error) { var userSignup *toolchainv1alpha1.UserSignup err := signup.PollUpdateSignup(ctx, func() error { @@ -481,7 +488,7 @@ func (s *ServiceImpl) DoGetSignup(ctx *gin.Context, cl namespaced.Client, userna // If UserSignup status is complete as active // Retrieve MasterUserRecord resource from the host cluster and use its status mur := &toolchainv1alpha1.MasterUserRecord{} - if err := cl.Get(ctx, cl.NamespacedName(userSignup.Status.CompliantUsername), mur); err != nil { + if err := cl.Get(requestContext(ctx), cl.NamespacedName(userSignup.Status.CompliantUsername), mur); err != nil { return nil, errs.Wrap(err, fmt.Sprintf("error when retrieving MasterUserRecord for completed UserSignup %s", userSignup.GetName())) } murCondition, _ := condition.FindConditionByType(mur.Status.Conditions, toolchainv1alpha1.ConditionReady) @@ -505,7 +512,7 @@ func (s *ServiceImpl) DoGetSignup(ctx *gin.Context, cl namespaced.Client, userna // Retrieve cluster-specific URLs from the status of the corresponding member cluster status := &toolchainv1alpha1.ToolchainStatus{} - if err := cl.Get(ctx, cl.NamespacedName("toolchain-status"), status); err != nil { + if err := cl.Get(requestContext(ctx), cl.NamespacedName("toolchain-status"), status); err != nil { return nil, errs.Wrapf(err, "error when retrieving ToolchainStatus for completed UserSignup %s", userSignup.GetName()) } signupResponse.ProxyURL = status.Status.HostRoutes.ProxyURL @@ -534,13 +541,15 @@ func (s *ServiceImpl) DoGetSignup(ctx *gin.Context, cl namespaced.Client, userna // auditUserSignupAgainstClaims compares the properties of the specified UserSignup against the claims contained in the // user's access token and updates the UserSignup if necessary. If updates were made, the function returns true // otherwise it returns false. -func (s *ServiceImpl) auditUserSignupAgainstClaims(ctx *gin.Context, userSignup *toolchainv1alpha1.UserSignup) bool { +func (s *ServiceImpl) auditUserSignupAgainstClaims(ctx echo.Context, userSignup *toolchainv1alpha1.UserSignup) bool { updated := false - updateIfRequired := func(ctx *gin.Context, key, existing string, updated bool) (string, bool) { - if val, ok := ctx.Get(key); ok && val != nil && len(val.(string)) > 0 && val != existing { - return val.(string), true + updateIfRequired := func(ctx echo.Context, key, existing string, updated bool) (string, bool) { + if val := ctx.Get(key); val != nil { + if s, ok := val.(string); ok && len(s) > 0 && s != existing { + return s, true + } } return existing, updated } diff --git a/pkg/signup/service/signup_service_test.go b/pkg/signup/service/signup_service_test.go index 7dd2f312..e106948c 100644 --- a/pkg/signup/service/signup_service_test.go +++ b/pkg/signup/service/signup_service_test.go @@ -36,8 +36,8 @@ import ( testconfig "github.com/codeready-toolchain/toolchain-common/pkg/test/config" recaptchapb "cloud.google.com/go/recaptchaenterprise/v2/apiv1/recaptchaenterprisepb" - "github.com/gin-gonic/gin" "github.com/gofrs/uuid" + "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -45,6 +45,23 @@ import ( v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +func newEchoTestContext() echo.Context { + e := echo.New() + return e.NewContext(httptest.NewRequest(http.MethodGet, "/", nil), httptest.NewRecorder()) +} + +func echoCtxNilRequest() echo.Context { + e := echo.New() + return e.NewContext(nil, httptest.NewRecorder()) +} + +func echoCtxWithRecaptchaTokens(tokens ...string) echo.Context { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Header["Recaptcha-Token"] = tokens + return e.NewContext(req, httptest.NewRecorder()) +} + type TestSignupServiceSuite struct { test.UnitTestSuite } @@ -98,8 +115,10 @@ func (s *TestSignupServiceSuite) TestSignup() { return val } + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) ctx.Set(context.UsernameKey, "jsmith@kubesaw") ctx.Set(context.SubKey, "987654321") ctx.Set(context.OriginalSubKey, "original-sub-value") @@ -185,15 +204,14 @@ func (s *TestSignupServiceSuite) TestSignup() { }) s.Run("with social event code", func() { - withCodeCtx := ctx.Copy() - withCodeCtx.Set(context.SocialEvent, "event1") + ctx.Set(context.SocialEvent, "event1") event := testsocialevent.NewSocialEvent(commontest.HostOperatorNs, "event1", testsocialevent.WithTargetCluster("event-member")) s.Run("set target cluster", func() { // when fakeClient, application := testutil.PrepareInClusterApp(s.T(), event) // when - returnedSignup, err := application.SignupService().Signup(withCodeCtx) + returnedSignup, err := application.SignupService().Signup(ctx) // then require.NoError(s.T(), err) @@ -211,7 +229,7 @@ func (s *TestSignupServiceSuite) TestSignup() { fakeClient, application := testutil.PrepareInClusterApp(s.T(), deactivatedUS, event) // when - reactivatedSignup, err := application.SignupService().Signup(withCodeCtx) + reactivatedSignup, err := application.SignupService().Signup(ctx) // then require.NoError(s.T(), err) @@ -227,7 +245,7 @@ func (s *TestSignupServiceSuite) TestSignup() { fakeClient, application := testutil.PrepareInClusterApp(s.T()) // when - returnedSignup, err := application.SignupService().Signup(withCodeCtx) + returnedSignup, err := application.SignupService().Signup(ctx) // then require.Error(s.T(), err) @@ -242,8 +260,10 @@ func (s *TestSignupServiceSuite) TestSignup() { func (s *TestSignupServiceSuite) TestSignupFailsWhenClientReturnsError() { // given + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) ctx.Set(context.UsernameKey, "zoeabernathy") ctx.Set(context.SubKey, "987654321") ctx.Set(context.OriginalSubKey, "original-sub-value") @@ -265,8 +285,10 @@ func (s *TestSignupServiceSuite) TestSignupFailsWhenClientReturnsError() { func (s *TestSignupServiceSuite) TestSignupFailsWithNotFoundThenOtherError() { // given + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) ctx.Set(context.UsernameKey, "lisasmith") ctx.Set(context.SubKey, "987654321") ctx.Set(context.OriginalSubKey, "original-sub-value") @@ -298,7 +320,7 @@ func (s *TestSignupServiceSuite) TestGetSignupFailsWithNotFoundThenOtherError() return fakeClient.Client.Get(ctx, key, obj, opts...) } - c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c := newEchoTestContext() // when _, err := application.SignupService().GetSignup(c, "abc", true) @@ -311,8 +333,10 @@ func (s *TestSignupServiceSuite) TestSignupNoSpaces() { s.ServiceConfiguration(true, "", 5) // given + e := echo.New() + postReq := httptest.NewRequest(http.MethodPost, "/?no-space=true", bytes.NewBufferString("")) rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + ctx := e.NewContext(postReq, rr) ctx.Set(context.UsernameKey, "jsmith") ctx.Set(context.SubKey, "987654321") ctx.Set(context.OriginalSubKey, "original-sub-value") @@ -320,7 +344,6 @@ func (s *TestSignupServiceSuite) TestSignupNoSpaces() { ctx.Set(context.GivenNameKey, "jane") ctx.Set(context.FamilyNameKey, "doe") ctx.Set(context.CompanyKey, "red hat") - ctx.Request, _ = http.NewRequest("POST", "/?no-space=true", bytes.NewBufferString("")) fakeClient, application := testutil.PrepareInClusterApp(s.T()) @@ -355,8 +378,11 @@ func (s *TestSignupServiceSuite) TestSignupWithCaptchaEnabled() { Verification().CaptchaScoreThreshold("0.8")) // given + e := echo.New() + postReq := httptest.NewRequest(http.MethodPost, "/", bytes.NewBufferString("")) + postReq.Header.Set("Recaptcha-Token", "abc") rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + ctx := e.NewContext(postReq, rr) ctx.Set(context.UsernameKey, "jsmith") ctx.Set(context.SubKey, "987654321") ctx.Set(context.OriginalSubKey, "original-sub-value") @@ -364,8 +390,6 @@ func (s *TestSignupServiceSuite) TestSignupWithCaptchaEnabled() { ctx.Set(context.GivenNameKey, "jane") ctx.Set(context.FamilyNameKey, "doe") ctx.Set(context.CompanyKey, "red hat") - ctx.Request, _ = http.NewRequest("POST", "/", bytes.NewBufferString("")) - ctx.Request.Header.Set("Recaptcha-Token", "abc") // when userSignup, err := signupService.Signup(ctx) @@ -389,8 +413,10 @@ func (s *TestSignupServiceSuite) TestUserSignupWithInvalidSubjectPrefix() { // given username := "-sjones" + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) ctx.Set(context.UsernameKey, username) ctx.Set(context.SubKey, "987654321") ctx.Set(context.EmailKey, "sjones@gmail.com") @@ -424,8 +450,10 @@ func (s *TestSignupServiceSuite) TestUserSignupWithInvalidSubjectPrefix() { func (s *TestSignupServiceSuite) TestUserWithExcludedDomainEmailSignsUp() { s.ServiceConfiguration(true, "redhat.com", 5) + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) ctx.Set(context.UsernameKey, "jsmith") ctx.Set(context.SubKey, "987654321") ctx.Set(context.EmailKey, "jsmith@redhat.com") @@ -454,8 +482,10 @@ func (s *TestSignupServiceSuite) TestUserWithExcludedDomainEmailSignsUp() { func (s *TestSignupServiceSuite) TestCRTAdminUserSignup() { s.ServiceConfiguration(true, "redhat.com", 5) + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) ctx.Set(context.UsernameKey, "jsmith-crtadmin") ctx.Set(context.SubKey, "987654321") ctx.Set(context.EmailKey, "jsmith@redhat.com") @@ -477,8 +507,10 @@ func (s *TestSignupServiceSuite) TestFailsIfUserSignupNameAlreadyExists() { signup := testusersignup.NewUserSignup(testusersignup.WithEncodedName("jsmith@kubesaw")) + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) ctx.Set(context.UsernameKey, "jsmith@kubesaw") ctx.Set(context.SubKey, "userid") ctx.Set(context.EmailKey, "jsmith@gmail.com") @@ -510,8 +542,10 @@ func (s *TestSignupServiceSuite) TestFailsIfUserBanned() { }, } + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) ctx.Set(context.UsernameKey, "jsmith") ctx.Set(context.EmailKey, "jsmith@gmail.com") @@ -543,8 +577,10 @@ func (s *TestSignupServiceSuite) TestOKIfOtherUserBanned() { }, } + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) ctx.Set(context.UsernameKey, "jsmith@gmail") ctx.Set(context.SubKey, "userid") ctx.Set(context.EmailKey, "jsmith@gmail.com") @@ -573,7 +609,7 @@ func (s *TestSignupServiceSuite) TestOKIfOtherUserBanned() { func (s *TestSignupServiceSuite) TestGetUserSignupFails() { // given username := "johnsmith" - c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c := newEchoTestContext() fakeClient, application := testutil.PrepareInClusterApp(s.T()) fakeClient.MockGet = func(ctx gocontext.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { @@ -591,7 +627,7 @@ func (s *TestSignupServiceSuite) TestGetUserSignupFails() { } func (s *TestSignupServiceSuite) TestGetSignupNotFound() { - c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c := newEchoTestContext() _, application := testutil.PrepareInClusterApp(s.T()) // when @@ -606,7 +642,7 @@ func (s *TestSignupServiceSuite) TestGetSignupStatusNotComplete() { // given s.ServiceConfiguration(true, "", 5) - c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c := newEchoTestContext() userSignupNotComplete := testusersignup.NewUserSignup( testusersignup.WithEncodedName("not-complete@kubesaw"), @@ -708,7 +744,7 @@ func (s *TestSignupServiceSuite) TestGetSignupNoStatusNotCompleteCondition() { } for _, status := range []toolchainv1alpha1.UserSignupStatus{noCondition, pendingApproval, noClusterApproval} { - c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c := newEchoTestContext() userSignup := &toolchainv1alpha1.UserSignup{ TypeMeta: v1.TypeMeta{}, @@ -761,7 +797,7 @@ func (s *TestSignupServiceSuite) TestGetSignupDeactivated() { _, application := testutil.PrepareInClusterApp(s.T(), us) - c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c := newEchoTestContext() // when signup, err := application.SignupService().GetSignup(c, username, true) @@ -786,7 +822,7 @@ func (s *TestSignupServiceSuite) TestGetSignupStatusOK() { _, application := testutil.PrepareInClusterApp(s.T(), us, mur, toolchainStatus, space, spacebinding) - c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c := newEchoTestContext() // when response, err := application.SignupService().GetSignup(c, username, true) @@ -859,7 +895,7 @@ func (s *TestSignupServiceSuite) TestGetSignupStatusFailGetToolchainStatus() { // given s.ServiceConfiguration(true, "", 5) - c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c := newEchoTestContext() username, us := s.newUserSignupComplete() mur := s.newProvisionedMUR("ted") @@ -880,7 +916,7 @@ func (s *TestSignupServiceSuite) TestGetSignupMURGetFails() { username, us := s.newUserSignupComplete() - c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c := newEchoTestContext() returnedErr := errors.New("an error occurred") fakeClient, application := testutil.PrepareInClusterApp(s.T(), us) @@ -904,7 +940,7 @@ func (s *TestSignupServiceSuite) TestGetSignupReadyConditionStatus() { username, us := s.newUserSignupComplete() - c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c := newEchoTestContext() mur := &toolchainv1alpha1.MasterUserRecord{ TypeMeta: v1.TypeMeta{}, @@ -997,8 +1033,10 @@ func (s *TestSignupServiceSuite) TestGetSignupBannedUserEmail() { testusersignup.WithCompliantUsername("ted")) _, application := testutil.PrepareInClusterApp(s.T(), us) + e := echo.New() rr := httptest.NewRecorder() - ctx, _ := gin.CreateTestContext(rr) + req := httptest.NewRequest(http.MethodGet, "/", nil) + ctx := e.NewContext(req, rr) ctx.Set(context.UsernameKey, "jsmith") ctx.Set(context.SubKey, us.Spec.IdentityClaims.UserID) ctx.Set(context.EmailKey, "jsmith@gmail.com") @@ -1119,7 +1157,7 @@ func (s *TestSignupServiceSuite) TestIsPhoneVerificationRequired() { Verification().Enabled(true). Verification().CaptchaEnabled(false)) - isVerificationRequired, score, assessmentID := service.IsPhoneVerificationRequired(nil, &gin.Context{}) + isVerificationRequired, score, assessmentID := service.IsPhoneVerificationRequired(nil, newEchoTestContext()) assert.True(s.T(), isVerificationRequired) assert.InDelta(s.T(), float32(-1), score, 0.01) assert.Empty(s.T(), assessmentID) @@ -1131,7 +1169,7 @@ func (s *TestSignupServiceSuite) TestIsPhoneVerificationRequired() { Verification().Enabled(true). Verification().CaptchaEnabled(true)) - isVerificationRequired, score, assessmentID := service.IsPhoneVerificationRequired(nil, &gin.Context{}) + isVerificationRequired, score, assessmentID := service.IsPhoneVerificationRequired(nil, echoCtxNilRequest()) assert.True(s.T(), isVerificationRequired) assert.InDelta(s.T(), float32(-1), score, 0.01) assert.Empty(s.T(), assessmentID) @@ -1143,7 +1181,7 @@ func (s *TestSignupServiceSuite) TestIsPhoneVerificationRequired() { Verification().Enabled(true). Verification().CaptchaEnabled(true)) - isVerificationRequired, score, assessmentID := service.IsPhoneVerificationRequired(nil, &gin.Context{Request: &http.Request{}}) + isVerificationRequired, score, assessmentID := service.IsPhoneVerificationRequired(nil, newEchoTestContext()) assert.True(s.T(), isVerificationRequired) assert.InDelta(s.T(), float32(-1), score, 0.01) assert.Empty(s.T(), assessmentID) @@ -1155,7 +1193,7 @@ func (s *TestSignupServiceSuite) TestIsPhoneVerificationRequired() { Verification().Enabled(true). Verification().CaptchaEnabled(true)) - isVerificationRequired, score, assessmentID := service.IsPhoneVerificationRequired(nil, &gin.Context{Request: &http.Request{Header: http.Header{"Recaptcha-Token": []string{"123", "456"}}}}) + isVerificationRequired, score, assessmentID := service.IsPhoneVerificationRequired(nil, echoCtxWithRecaptchaTokens("123", "456")) assert.True(s.T(), isVerificationRequired) assert.InDelta(s.T(), float32(-1), score, 0.01) assert.Empty(s.T(), assessmentID) @@ -1167,7 +1205,7 @@ func (s *TestSignupServiceSuite) TestIsPhoneVerificationRequired() { Verification().Enabled(true). Verification().CaptchaEnabled(true)) - isVerificationRequired, score, assessmentID := service.IsPhoneVerificationRequired(&FakeCaptchaChecker{result: fmt.Errorf("assessment failed")}, &gin.Context{Request: &http.Request{Header: http.Header{"Recaptcha-Token": []string{"123"}}}}) + isVerificationRequired, score, assessmentID := service.IsPhoneVerificationRequired(&FakeCaptchaChecker{result: fmt.Errorf("assessment failed")}, echoCtxWithRecaptchaTokens("123")) assert.True(s.T(), isVerificationRequired) assert.InDelta(s.T(), float32(-1), score, 0.01) assert.Empty(s.T(), assessmentID) @@ -1180,7 +1218,7 @@ func (s *TestSignupServiceSuite) TestIsPhoneVerificationRequired() { Verification().CaptchaEnabled(true). Verification().CaptchaScoreThreshold("0.8")) - isVerificationRequired, score, assessmentID := service.IsPhoneVerificationRequired(&FakeCaptchaChecker{score: 0.5}, &gin.Context{Request: &http.Request{Header: http.Header{"Recaptcha-Token": []string{"123"}}}}) + isVerificationRequired, score, assessmentID := service.IsPhoneVerificationRequired(&FakeCaptchaChecker{score: 0.5}, echoCtxWithRecaptchaTokens("123")) assert.True(s.T(), isVerificationRequired) assert.InDelta(s.T(), float32(0.5), score, 0.01) assert.Equal(s.T(), "captcha-assessment-123", assessmentID) @@ -1205,7 +1243,9 @@ func (s *TestSignupServiceSuite) TestIsPhoneVerificationRequired() { Verification().CaptchaEnabled(true). Verification().ExcludedEmailDomains("redhat.com")) - isVerificationRequired, score, assessmentID := service.IsPhoneVerificationRequired(nil, &gin.Context{Keys: map[string]interface{}{"email": "joe@redhat.com"}}) + excludedEmailCtx := newEchoTestContext() + excludedEmailCtx.Set(context.EmailKey, "joe@redhat.com") + isVerificationRequired, score, assessmentID := service.IsPhoneVerificationRequired(nil, excludedEmailCtx) assert.False(s.T(), isVerificationRequired) assert.InDelta(s.T(), float32(-1), score, 0.01) assert.Empty(s.T(), assessmentID) @@ -1217,7 +1257,7 @@ func (s *TestSignupServiceSuite) TestIsPhoneVerificationRequired() { Verification().CaptchaEnabled(true). Verification().CaptchaScoreThreshold("0.8")) - isVerificationRequired, score, assessmentID := service.IsPhoneVerificationRequired(&FakeCaptchaChecker{score: 1.0}, &gin.Context{Request: &http.Request{Header: http.Header{"Recaptcha-Token": []string{"123"}}}}) + isVerificationRequired, score, assessmentID := service.IsPhoneVerificationRequired(&FakeCaptchaChecker{score: 1.0}, echoCtxWithRecaptchaTokens("123")) assert.False(s.T(), isVerificationRequired) assert.InDelta(s.T(), float32(1.0), score, 0.01) assert.Equal(s.T(), "captcha-assessment-123", assessmentID) @@ -1254,7 +1294,7 @@ func (s *TestSignupServiceSuite) TestGetSignupUpdatesUserSignupIdentityClaims() } s.Run("PreferredUsername property updated when set in context", func() { - c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c := newEchoTestContext() c.Set(context.UsernameKey, "cocochanel") fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup, mur) @@ -1279,7 +1319,7 @@ func (s *TestSignupServiceSuite) TestGetSignupUpdatesUserSignupIdentityClaims() require.Equal(s.T(), "4242", modified.Spec.IdentityClaims.AccountNumber) s.Run("GivenName property updated when set in context", func() { - c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c := newEchoTestContext() c.Set(context.GivenNameKey, "Jonathan") _, err := application.SignupService().GetSignup(c, username, true) @@ -1304,7 +1344,7 @@ func (s *TestSignupServiceSuite) TestGetSignupUpdatesUserSignupIdentityClaims() require.Equal(s.T(), "fd2addbd8d82f0d2dc088fa122377eaa", modified.Labels["toolchain.dev.openshift.com/email-hash"]) s.Run("FamilyName and Company properties updated when set in context", func() { - c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c := newEchoTestContext() c.Set(context.FamilyNameKey, "Smythe") c.Set(context.CompanyKey, "Red Hat") @@ -1329,7 +1369,7 @@ func (s *TestSignupServiceSuite) TestGetSignupUpdatesUserSignupIdentityClaims() require.Equal(s.T(), "fd2addbd8d82f0d2dc088fa122377eaa", modified.Labels["toolchain.dev.openshift.com/email-hash"]) s.Run("Remaining properties updated when set in context", func() { - c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c := newEchoTestContext() c.Set(context.SubKey, "987654321") c.Set(context.UserIDKey, "123456777") c.Set(context.AccountIDKey, "777654321") @@ -1500,7 +1540,7 @@ type FakeCaptchaChecker struct { result error } -func (c FakeCaptchaChecker) CompleteAssessment(_ *gin.Context, _ configuration.RegistrationServiceConfig, _ string) (*recaptchapb.Assessment, error) { +func (c FakeCaptchaChecker) CompleteAssessment(_ echo.Context, _ configuration.RegistrationServiceConfig, _ string) (*recaptchapb.Assessment, error) { return &recaptchapb.Assessment{ RiskAnalysis: &recaptchapb.RiskAnalysis{ Score: c.score, diff --git a/pkg/signup/signup.go b/pkg/signup/signup.go index d2da5d80..b6898d5c 100644 --- a/pkg/signup/signup.go +++ b/pkg/signup/signup.go @@ -4,7 +4,7 @@ import ( "fmt" "github.com/codeready-toolchain/registration-service/pkg/log" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" ) // Signup represents Signup resource which is a wrapper of K8s UserSignup @@ -72,7 +72,7 @@ type Status struct { // PollUpdateSignup will attempt to execute the provided updater function, and if it fails // will reattempt the update for a limited number of retries -func PollUpdateSignup(ctx *gin.Context, updater func() error) error { +func PollUpdateSignup(ctx echo.Context, updater func() error) error { // Attempt to execute an update function, retrying a number of times if the update fails attempts := 0 for { diff --git a/pkg/signup/socialevent.go b/pkg/signup/socialevent.go index 84f803ec..6c061ab3 100644 --- a/pkg/signup/socialevent.go +++ b/pkg/signup/socialevent.go @@ -10,17 +10,17 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/log" "github.com/codeready-toolchain/registration-service/pkg/namespaced" "github.com/codeready-toolchain/toolchain-common/pkg/states" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // GetAndValidateSocialEvent returns a SocialEvent with the given name. // If the event is already full, not yet started, already finished, or not found then it returns error -func GetAndValidateSocialEvent(ctx *gin.Context, cl namespaced.Client, code string) (*toolchainv1alpha1.SocialEvent, error) { +func GetAndValidateSocialEvent(ctx echo.Context, cl namespaced.Client, code string) (*toolchainv1alpha1.SocialEvent, error) { // look-up the SocialEvent event := &toolchainv1alpha1.SocialEvent{} - if err := cl.Get(ctx, cl.NamespacedName(code), event); err != nil { + if err := cl.Get(ctx.Request().Context(), cl.NamespacedName(code), event); err != nil { if apierrors.IsNotFound(err) { // a SocialEvent was not found for the provided code return nil, crterrors.NewForbiddenError("invalid code", "the provided code is invalid") diff --git a/pkg/signup/socialevent_test.go b/pkg/signup/socialevent_test.go index dde0eef5..d3b70202 100644 --- a/pkg/signup/socialevent_test.go +++ b/pkg/signup/socialevent_test.go @@ -1,6 +1,7 @@ package signup import ( + "net/http" "net/http/httptest" "testing" "time" @@ -12,7 +13,7 @@ import ( commontest "github.com/codeready-toolchain/toolchain-common/pkg/test" testsocialevent "github.com/codeready-toolchain/toolchain-common/pkg/test/socialevent" "github.com/codeready-toolchain/toolchain-common/pkg/test/usersignup" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -20,7 +21,10 @@ import ( func TestGetSocialEvent(t *testing.T) { // given log.Init("social-code-testing") - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + rr := httptest.NewRecorder() + ctx := e.NewContext(req, rr) t.Run("success", func(t *testing.T) { // given diff --git a/pkg/verification/captcha/captcha.go b/pkg/verification/captcha/captcha.go index 931aafb0..3f96b108 100644 --- a/pkg/verification/captcha/captcha.go +++ b/pkg/verification/captcha/captcha.go @@ -6,7 +6,7 @@ import ( "github.com/codeready-toolchain/registration-service/pkg/configuration" "github.com/codeready-toolchain/registration-service/pkg/log" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" recaptcha "cloud.google.com/go/recaptchaenterprise/v2/apiv1" recaptchapb "cloud.google.com/go/recaptchaenterprise/v2/apiv1/recaptchaenterprisepb" @@ -16,7 +16,7 @@ import ( const recaptchaSignupAction = "SIGNUP" type Assessor interface { - CompleteAssessment(ctx *gin.Context, cfg configuration.RegistrationServiceConfig, token string) (*recaptchapb.Assessment, error) + CompleteAssessment(ctx echo.Context, cfg configuration.RegistrationServiceConfig, token string) (*recaptchapb.Assessment, error) } type Helper struct{} @@ -31,7 +31,7 @@ type Helper struct{} returns the assessment and nil if the assessment was successful, otherwise returns nil and the error. */ -func (c Helper) CompleteAssessment(ctx *gin.Context, cfg configuration.RegistrationServiceConfig, token string) (*recaptchapb.Assessment, error) { +func (c Helper) CompleteAssessment(ctx echo.Context, cfg configuration.RegistrationServiceConfig, token string) (*recaptchapb.Assessment, error) { gctx := gocontext.Background() client, err := recaptcha.NewClient(gctx) if err != nil { @@ -57,7 +57,7 @@ func (c Helper) CompleteAssessment(ctx *gin.Context, cfg configuration.Registrat } response, err := client.CreateAssessment( - ctx, + ctx.Request().Context(), request) if err != nil { return nil, fmt.Errorf("failed to create reCAPTCHA assessment") diff --git a/pkg/verification/sender/amazon_sns_sender.go b/pkg/verification/sender/amazon_sns_sender.go index 3f212ed2..2e651fff 100644 --- a/pkg/verification/sender/amazon_sns_sender.go +++ b/pkg/verification/sender/amazon_sns_sender.go @@ -6,7 +6,7 @@ import ( "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/sns" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" ) type AWSSenderConfiguration interface { @@ -27,7 +27,7 @@ func NewAmazonSNSSender(cfg AWSSenderConfiguration) NotificationSender { } } -func (s *AmazonSNSSender) SendNotification(_ *gin.Context, content, phoneNumber, _ string) error { +func (s *AmazonSNSSender) SendNotification(_ echo.Context, content, phoneNumber, _ string) error { // TODO add support for country-specific sender IDs if we ever decide to use Amazon SNS to send notifications diff --git a/pkg/verification/sender/sender.go b/pkg/verification/sender/sender.go index d207e41c..d6c4ec7e 100644 --- a/pkg/verification/sender/sender.go +++ b/pkg/verification/sender/sender.go @@ -5,11 +5,11 @@ import ( "strings" "github.com/codeready-toolchain/registration-service/pkg/configuration" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" ) type NotificationSender interface { - SendNotification(ctx *gin.Context, content, phoneNumber, countryCode string) error + SendNotification(ctx echo.Context, content, phoneNumber, countryCode string) error } type NotificationSenderOption = func() diff --git a/pkg/verification/sender/twilio_sender.go b/pkg/verification/sender/twilio_sender.go index c8d86cb8..a378c852 100644 --- a/pkg/verification/sender/twilio_sender.go +++ b/pkg/verification/sender/twilio_sender.go @@ -6,8 +6,8 @@ import ( "net/http" "github.com/codeready-toolchain/registration-service/pkg/log" - "github.com/gin-gonic/gin" "github.com/kevinburke/twilio-go" + "github.com/labstack/echo/v4" ) type TwilioConfig interface { @@ -44,7 +44,7 @@ func NewTwilioSender(cfg TwilioConfig, httpClient *http.Client) NotificationSend return sender } -func (s *TwilioNotificationSender) SendNotification(ctx *gin.Context, content, phoneNumber, countryCode string) error { +func (s *TwilioNotificationSender) SendNotification(ctx echo.Context, content, phoneNumber, countryCode string) error { client := twilio.NewClient(s.Config.TwilioAccountSID(), s.Config.TwilioAuthToken(), s.HTTPClient) from, ok := s.SenderIDs[countryCode] if !ok { diff --git a/pkg/verification/sender/twilio_sender_test.go b/pkg/verification/sender/twilio_sender_test.go index 9cf0ef9a..7a063593 100644 --- a/pkg/verification/sender/twilio_sender_test.go +++ b/pkg/verification/sender/twilio_sender_test.go @@ -10,7 +10,7 @@ import ( toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" sender2 "github.com/codeready-toolchain/registration-service/pkg/verification/sender" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" "github.com/stretchr/testify/require" "gopkg.in/h2non/gock.v1" ) @@ -85,7 +85,10 @@ func TestTwilioSenderID(t *testing.T) { } t.Run("test country code in config", func(t *testing.T) { - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + ctx := e.NewContext(req, rec) reqValue := setupGockAndSendRequest(func(sender sender2.NotificationSender) error { return sender.SendNotification(ctx, "Test Message", "+440000000000", "44") }) @@ -99,7 +102,10 @@ func TestTwilioSenderID(t *testing.T) { }) t.Run("test country code not in config", func(t *testing.T) { - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + ctx := e.NewContext(req, rec) reqValue := setupGockAndSendRequest(func(sender sender2.NotificationSender) error { return sender.SendNotification(ctx, "Test Message", "+611234567890", "61") }) diff --git a/pkg/verification/service/verification_service.go b/pkg/verification/service/verification_service.go index ce15c951..a5fa22dd 100644 --- a/pkg/verification/service/verification_service.go +++ b/pkg/verification/service/verification_service.go @@ -26,7 +26,7 @@ import ( "github.com/codeready-toolchain/toolchain-common/pkg/hash" "github.com/codeready-toolchain/toolchain-common/pkg/states" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" apierrors "k8s.io/apimachinery/pkg/api/errors" ) @@ -63,7 +63,7 @@ func NewVerificationService(client namespaced.Client) service.VerificationServic // InitVerification sends a verification message to the specified user, using the Twilio service. If successful, // the user will receive a verification SMS. The UserSignup resource is updated with a number of annotations in order // to manage the phone verification process and protect against system abuse. -func (s *ServiceImpl) InitVerification(ctx *gin.Context, username, e164PhoneNumber, countryCode string) error { +func (s *ServiceImpl) InitVerification(ctx echo.Context, username, e164PhoneNumber, countryCode string) error { signup := &toolchainv1alpha1.UserSignup{} if err := s.Get(gocontext.TODO(), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { if apierrors.IsNotFound(err) { @@ -219,7 +219,7 @@ func generateVerificationCode() (string, error) { // VerifyPhoneCode validates the user's phone verification code. It updates the specified UserSignup value, so even // if an error is returned by this function the caller should still process changes to it -func (s *ServiceImpl) VerifyPhoneCode(ctx *gin.Context, username, code string) (verificationErr error) { +func (s *ServiceImpl) VerifyPhoneCode(ctx echo.Context, username, code string) (verificationErr error) { cfg := configuration.GetRegistrationServiceConfig() // If we can't even find the UserSignup, then die here @@ -363,7 +363,7 @@ func (s *ServiceImpl) VerifyPhoneCode(ctx *gin.Context, username, code string) ( // checkRequiredManualApproval compares the user captcha score with the configured required captcha score. // When the user score is lower than the required score an error is returned meaning that the user is considered "suspicious" and manual approval of the signup is required. -func checkRequiredManualApproval(ctx *gin.Context, signup *toolchainv1alpha1.UserSignup, cfg configuration.RegistrationServiceConfig) error { +func checkRequiredManualApproval(ctx echo.Context, signup *toolchainv1alpha1.UserSignup, cfg configuration.RegistrationServiceConfig) error { captchaScore, found := signup.Annotations[toolchainv1alpha1.UserSignupCaptchaScoreAnnotationKey] if found { fscore, parseErr := strconv.ParseFloat(captchaScore, 32) @@ -383,7 +383,7 @@ func checkRequiredManualApproval(ctx *gin.Context, signup *toolchainv1alpha1.Use // VerifyActivationCode verifies the activation code: // - checks that the SocialEvent resource named after the activation code exists // - checks that the SocialEvent has enough capacity to approve the user -func (s *ServiceImpl) VerifyActivationCode(ctx *gin.Context, username, code string) error { +func (s *ServiceImpl) VerifyActivationCode(ctx echo.Context, username, code string) error { log.Infof(ctx, "verifying activation code '%s'", code) // look-up the UserSignup signup := &toolchainv1alpha1.UserSignup{} diff --git a/pkg/verification/service/verification_service_test.go b/pkg/verification/service/verification_service_test.go index 7b2d5993..22175269 100644 --- a/pkg/verification/service/verification_service_test.go +++ b/pkg/verification/service/verification_service_test.go @@ -31,7 +31,7 @@ import ( testsocialevent "github.com/codeready-toolchain/toolchain-common/pkg/test/socialevent" testusersignup "github.com/codeready-toolchain/toolchain-common/pkg/test/usersignup" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -40,6 +40,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +func newEchoTestContext() echo.Context { + e := echo.New() + return e.NewContext(httptest.NewRequest(http.MethodGet, "/", nil), httptest.NewRecorder()) +} + const ( testSecretName = "host-operator-secret" @@ -133,7 +138,7 @@ func (s *TestVerificationServiceSuite) TestInitVerification() { fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup, userSignup2) // Test the init verification for the first UserSignup - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().InitVerification(ctx, "johnny@kubesaw", "+1NUMBER", "1") require.NoError(s.T(), err) @@ -168,7 +173,7 @@ func (s *TestVerificationServiceSuite) TestInitVerification() { } gock.Observe(obs) - ctx, _ = gin.CreateTestContext(httptest.NewRecorder()) + ctx = newEchoTestContext() // for the second usersignup err = application.VerificationService().InitVerification(ctx, "jsmith@kubesaw", "+61NUMBER", "1") require.NoError(s.T(), err) @@ -243,7 +248,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationClientFailure() { return fakeClient.Client.Get(ctx, key, obj, opts...) } - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().InitVerification(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") require.EqualError(s.T(), err, "get failed: error retrieving usersignup with username 'johnny@kubesaw'", err.Error()) }) @@ -257,7 +262,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationClientFailure() { return fakeClient.Client.Update(ctx, obj, opts...) } - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().InitVerification(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") require.EqualError(s.T(), err, "there was an error while updating your account - please wait a moment before "+ "trying again. If this error persists, please contact the Developer Sandbox team at devsandbox@redhat.com "+ @@ -277,7 +282,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationClientFailure() { return fakeClient.Client.Update(ctx, obj, opts...) } - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().InitVerification(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") require.NoError(s.T(), err) @@ -318,7 +323,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationClientFailure() { // when: // InitVerification is called and notification sending fails - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().InitVerification(ctx, userSignupWithoutPhoneHash.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") // then @@ -368,7 +373,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationPassesWhenMaxCountRea fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup) - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().InitVerification(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") require.NoError(s.T(), err) @@ -409,7 +414,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationFailsWhenCountContain _, application := testutil.PrepareInClusterApp(s.T(), userSignup) - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().InitVerification(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") require.EqualError(s.T(), err, "daily limit exceeded: cannot generate new verification code") } @@ -435,7 +440,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationFailsDailyCounterExce _, application := testutil.PrepareInClusterApp(s.T(), userSignup) - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().InitVerification(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "+1NUMBER", "1") require.EqualError(s.T(), err, "daily limit exceeded: cannot generate new verification code", err.Error()) require.Empty(s.T(), userSignup.Annotations[toolchainv1alpha1.UserSignupVerificationCodeAnnotationKey]) @@ -467,7 +472,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationFailsWhenPhoneNumberI fakeClient, application := testutil.PrepareInClusterApp(s.T(), alphaUserSignup, bravoUserSignup) - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().InitVerification(ctx, bravoUserSignup.Spec.IdentityClaims.PreferredUsername, e164PhoneNumber, "1") require.Error(s.T(), err) require.Equal(s.T(), "phone number already in use: cannot register using phone number: +19875551122", err.Error()) @@ -507,7 +512,7 @@ func (s *TestVerificationServiceSuite) TestInitVerificationOKWhenPhoneNumberInUs fakeClient, application := testutil.PrepareInClusterApp(s.T(), alphaUserSignup, bravoUserSignup) - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().InitVerification(ctx, bravoUserSignup.Spec.IdentityClaims.PreferredUsername, e164PhoneNumber, "1") require.NoError(s.T(), err) @@ -537,7 +542,7 @@ func (s *TestVerificationServiceSuite) TestVerifyPhoneCode() { fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup) - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().VerifyPhoneCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "123456") require.NoError(s.T(), err) @@ -561,7 +566,7 @@ func (s *TestVerificationServiceSuite) TestVerifyPhoneCode() { fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup) - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().VerifyPhoneCode(ctx, "employee085@kubesaw", "654321") require.NoError(s.T(), err) @@ -584,7 +589,7 @@ func (s *TestVerificationServiceSuite) TestVerifyPhoneCode() { _, application := testutil.PrepareInClusterApp(s.T(), userSignup) - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().VerifyPhoneCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "123456") require.Error(s.T(), err) e := &crterrors.Error{} @@ -605,7 +610,7 @@ func (s *TestVerificationServiceSuite) TestVerifyPhoneCode() { _, application := testutil.PrepareInClusterApp(s.T(), userSignup) - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().VerifyPhoneCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "123456") e := &crterrors.Error{} require.ErrorAs(s.T(), err, &e) @@ -625,7 +630,7 @@ func (s *TestVerificationServiceSuite) TestVerifyPhoneCode() { _, application := testutil.PrepareInClusterApp(s.T(), userSignup) - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().VerifyPhoneCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "123456") require.EqualError(s.T(), err, "too many verification attempts", err.Error()) }) @@ -642,7 +647,7 @@ func (s *TestVerificationServiceSuite) TestVerifyPhoneCode() { fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup) - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().VerifyPhoneCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "123456") require.EqualError(s.T(), err, "too many verification attempts", err.Error()) @@ -665,7 +670,7 @@ func (s *TestVerificationServiceSuite) TestVerifyPhoneCode() { _, application := testutil.PrepareInClusterApp(s.T(), userSignup) - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().VerifyPhoneCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "123456") require.EqualError(s.T(), err, "parsing time \"ABC\" as \"2006-01-02T15:04:05.000Z07:00\": cannot parse \"ABC\" as \"2006\": error parsing expiry timestamp", err.Error()) }) @@ -764,7 +769,7 @@ func (s *TestVerificationServiceSuite) TestVerifyPhoneCode() { fakeClient, application := testutil.PrepareInClusterApp(s.T(), userSignup) - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() err := application.VerificationService().VerifyPhoneCode(ctx, userSignup.Spec.IdentityClaims.PreferredUsername, "123456") // then @@ -790,7 +795,7 @@ func (s *TestVerificationServiceSuite) testVerifyActivationCode(targetCluster st // given cfg := configuration.GetRegistrationServiceConfig() - ctx, _ := gin.CreateTestContext(httptest.NewRecorder()) + ctx := newEchoTestContext() s.Run("verification ok", func() { // given diff --git a/test/fake/proxy.go b/test/fake/proxy.go index 3a210a7c..34391ce0 100644 --- a/test/fake/proxy.go +++ b/test/fake/proxy.go @@ -3,7 +3,7 @@ package fake import ( toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" "github.com/codeready-toolchain/registration-service/pkg/signup" - "github.com/gin-gonic/gin" + "github.com/labstack/echo/v4" ) // This whole service abstraction is such a huge pain. We have to get rid of it!!! @@ -41,11 +41,11 @@ func (m *SignupService) DefaultMockGetSignup() func(username string) (*signup.Si } } -func (m *SignupService) GetSignup(_ *gin.Context, username string, _ bool) (*signup.Signup, error) { +func (m *SignupService) GetSignup(_ echo.Context, username string, _ bool) (*signup.Signup, error) { return m.MockGetSignup(username) } -func (m *SignupService) Signup(_ *gin.Context) (*toolchainv1alpha1.UserSignup, error) { +func (m *SignupService) Signup(_ echo.Context) (*toolchainv1alpha1.UserSignup, error) { return nil, nil } func (m *SignupService) UpdateUserSignup(_ *toolchainv1alpha1.UserSignup) (*toolchainv1alpha1.UserSignup, error) { From 4c94d6b2159fb9d8066216956c13d59e15c0b2da Mon Sep 17 00:00:00 2001 From: MikelAlejoBR Date: Tue, 5 May 2026 17:30:24 -0400 Subject: [PATCH 02/10] fix: an empty activation code is accepted With an empty activation codes were received, the type assertion check did not check for an empty string, and therefore the execution kept going through the "VerifyActivationCode" function, which would set an empty social event and call the "Signup" function. That function, after getting to "newUserSignup", would attempt grabbing the social event code, but since it's an empty string, the signup would fallback to a regular one instead of being associated with an event. SANDBOX-316 --- pkg/controller/signup.go | 2 +- pkg/controller/signup_test.go | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/pkg/controller/signup.go b/pkg/controller/signup.go index 1963db00..3ff3d350 100644 --- a/pkg/controller/signup.go +++ b/pkg/controller/signup.go @@ -155,7 +155,7 @@ func (s *Signup) VerifyActivationCodeHandler(ctx echo.Context) error { return ctx.NoContent(http.StatusBadRequest) } code, ok := body["code"].(string) - if !ok { + if !ok || code == "" { log.Error(ctx, nil, "no activation code provided in the request") return ctx.NoContent(http.StatusBadRequest) } diff --git a/pkg/controller/signup_test.go b/pkg/controller/signup_test.go index ae335702..b649e9c8 100644 --- a/pkg/controller/signup_test.go +++ b/pkg/controller/signup_test.go @@ -662,6 +662,19 @@ func (s *TestSignupSuite) TestVerifyActivationCodeHandler() { s.Run("verification failed", func() { + s.Run("empty code", func() { + // given + _, application := testutil.PrepareInClusterApp(s.T()) + ctrl := controller.NewSignup(application) + handler := ctrl.VerifyActivationCodeHandler + + // when + rr := initActivationCodeVerification(s.T(), handler, "Jane", "") + + // then + require.Equal(s.T(), http.StatusBadRequest, rr.Code) + }) + s.Run("too many attempts", func() { // given userSignup := testusersignup.NewUserSignup( From 97940976742a19e822a59f713e69d56fd11277a3 Mon Sep 17 00:00:00 2001 From: MikelAlejoBR Date: Tue, 5 May 2026 17:43:28 -0400 Subject: [PATCH 03/10] fix: metrics being recorded with wrong status code The handlers that returned an error without writing a response and a response status code, made the Prometheus middleware to increase counters with a unset "0" status code. This is because there is a centralized catch-all Echo middleware which is supposed to always populate it by default to "500" in case there is no other status code set, but the Prometheus middleware always gets executed before that one. By extracting the logic of the catch-all middleware and applying it to the Prometheus one as well we solve that issue. SANDBOX-316 --- pkg/errors/error.go | 15 +++++++++++++++ pkg/errors/error_test.go | 15 +++++++++++++++ pkg/middleware/promhttp_middleware.go | 22 ++++++++++++++++++++-- pkg/proxy/proxy.go | 6 +----- 4 files changed, 51 insertions(+), 7 deletions(-) diff --git a/pkg/errors/error.go b/pkg/errors/error.go index f6b7f67f..4d9f17a3 100644 --- a/pkg/errors/error.go +++ b/pkg/errors/error.go @@ -1,6 +1,7 @@ package errors import ( + "errors" "fmt" "net/http" @@ -84,3 +85,17 @@ func NewBadRequest(message, details string) *Error { Details: details, } } + +// StatusCode extracts the HTTP status code from an error. +// It handles *Error and *echo.HTTPError, defaulting to http.StatusInternalServerError. +func StatusCode(err error) int { + var e *Error + if errors.As(err, &e) { + return e.Code + } + var httpErr *echo.HTTPError + if errors.As(err, &httpErr) { + return httpErr.Code + } + return http.StatusInternalServerError +} diff --git a/pkg/errors/error_test.go b/pkg/errors/error_test.go index 5e410781..6842bcbe 100644 --- a/pkg/errors/error_test.go +++ b/pkg/errors/error_test.go @@ -87,4 +87,19 @@ func (s *TestErrorsSuite) TestErrors() { require.Equal(s.T(), http.StatusBadRequest, err.Code) require.Equal(s.T(), http.StatusText(http.StatusBadRequest), err.Status) }) + + s.Run("StatusCode", func() { + s.Run("from crterrors.Error", func() { + require.Equal(s.T(), http.StatusForbidden, errs.StatusCode(errs.NewForbiddenError("a", "b"))) + require.Equal(s.T(), http.StatusNotFound, errs.StatusCode(errs.NewNotFoundError(errors.New("x"), "y"))) + }) + + s.Run("from echo.HTTPError", func() { + require.Equal(s.T(), http.StatusBadGateway, errs.StatusCode(echo.NewHTTPError(http.StatusBadGateway, "bad gateway"))) + }) + + s.Run("from unknown error", func() { + require.Equal(s.T(), http.StatusInternalServerError, errs.StatusCode(errors.New("something went wrong"))) + }) + }) } diff --git a/pkg/middleware/promhttp_middleware.go b/pkg/middleware/promhttp_middleware.go index 3dd729a1..2273a0ab 100644 --- a/pkg/middleware/promhttp_middleware.go +++ b/pkg/middleware/promhttp_middleware.go @@ -1,9 +1,12 @@ package middleware import ( + "net/http" "strconv" "time" + crterrors "github.com/codeready-toolchain/registration-service/pkg/errors" + "github.com/labstack/echo/v4" "github.com/prometheus/client_golang/prometheus" ) @@ -25,7 +28,7 @@ func InstrumentRoundTripperCounter(counter *prometheus.CounterVec) echo.Middlewa return func(c echo.Context) error { err := next(c) counter.With(prometheus.Labels{ - "code": strconv.Itoa(c.Response().Status), + "code": strconv.Itoa(responseStatus(c, err)), "method": c.Request().Method, "path": c.Request().URL.Path, }).Inc() @@ -41,7 +44,7 @@ func InstrumentRoundTripperDuration(histVec *prometheus.HistogramVec) echo.Middl err := next(c) duration := time.Since(start) histVec.With(prometheus.Labels{ - "code": strconv.Itoa(c.Response().Status), + "code": strconv.Itoa(responseStatus(c, err)), "method": c.Request().Method, "path": c.Request().URL.Path, }).Observe(float64(duration.Seconds())) @@ -49,3 +52,18 @@ func InstrumentRoundTripperDuration(histVec *prometheus.HistogramVec) echo.Middl } } } + +// responseStatus returns the HTTP status code to use for metrics. +// When the handler returned an error, the status is extracted from the error +// because Echo's centralized error handler hasn't written the response yet at +// this point in the middleware chain, so c.Response().Status may still be 0. +func responseStatus(c echo.Context, err error) int { + if err != nil { + return crterrors.StatusCode(err) + } + status := c.Response().Status + if status == 0 { + return http.StatusOK + } + return status +} diff --git a/pkg/proxy/proxy.go b/pkg/proxy/proxy.go index 5e0d8ef4..3fd36b9a 100644 --- a/pkg/proxy/proxy.go +++ b/pkg/proxy/proxy.go @@ -532,11 +532,7 @@ func getWorkspaceContext(req *http.Request) (string, string, error) { } func customHTTPErrorHandler(cause error, ctx echo.Context) { - code := http.StatusInternalServerError - ce := &crterrors.Error{} - if errors.As(cause, &ce) { - code = ce.Code - } + code := crterrors.StatusCode(cause) ctx.Logger().Error(cause) if err := ctx.String(code, cause.Error()); err != nil { ctx.Logger().Error(err) From 0ab0a56221fcde1558eab9046962e86d4445bd29 Mon Sep 17 00:00:00 2001 From: MikelAlejoBR Date: Tue, 5 May 2026 17:49:26 -0400 Subject: [PATCH 04/10] fix: high cardinality for Prometheus metrics due to the path Using the request's path instead of the template leads to having a high cardinality for the Prometheus metrics, because every ID would generate a different metric. By using the matched template route instead we fix the issue, and it's still readable enough. SANDBOX-316 --- pkg/middleware/promhttp_middleware.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/middleware/promhttp_middleware.go b/pkg/middleware/promhttp_middleware.go index 2273a0ab..61dfafec 100644 --- a/pkg/middleware/promhttp_middleware.go +++ b/pkg/middleware/promhttp_middleware.go @@ -30,7 +30,7 @@ func InstrumentRoundTripperCounter(counter *prometheus.CounterVec) echo.Middlewa counter.With(prometheus.Labels{ "code": strconv.Itoa(responseStatus(c, err)), "method": c.Request().Method, - "path": c.Request().URL.Path, + "path": c.Path(), }).Inc() return err } @@ -46,7 +46,7 @@ func InstrumentRoundTripperDuration(histVec *prometheus.HistogramVec) echo.Middl histVec.With(prometheus.Labels{ "code": strconv.Itoa(responseStatus(c, err)), "method": c.Request().Method, - "path": c.Request().URL.Path, + "path": c.Path(), }).Observe(float64(duration.Seconds())) return err } From f0d21b43f7f5a234eb74c3942b9eb1ff12a73fea Mon Sep 17 00:00:00 2001 From: MikelAlejoBR Date: Tue, 5 May 2026 17:52:39 -0400 Subject: [PATCH 05/10] fix: skip normal "server closed" errors in tests We weren't skipping the "server closed" errors when asserting for "no error" when launching the Echo server for the tests, which could result in flaky tests. SANDBOX-316 --- pkg/server/server_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index ae4cee19..9e4843a9 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -2,6 +2,7 @@ package server_test import ( "context" + "errors" "io" "net/http" "testing" @@ -65,7 +66,9 @@ func (s *TestServerSuite) TestServer() { s.Run("CORS", func() { go func(t *testing.T) { err := srv.HTTPServer().ListenAndServe() - assert.NoError(t, err) // require must only be used in the goroutine running the test function (testifylint) + if err != nil && !errors.Is(err, http.ErrServerClosed) { + assert.NoError(t, err) // require must only be used in the goroutine running the test function (testifylint) + } }(s.T()) err := wait.PollUntilContextTimeout(context.TODO(), DefaultRetryInterval, DefaultTimeout, false, func(context.Context) (done bool, err error) { @@ -121,7 +124,9 @@ func startFakeProxy(t *testing.T) *http.Server { srv := &http.Server{Addr: ":" + altProxyPort, Handler: mux, ReadHeaderTimeout: 2 * time.Second} go func() { err := srv.ListenAndServe() - assert.NoError(t, err) // require must only be used in the goroutine running the test function (testifylint) + if err != nil && !errors.Is(err, http.ErrServerClosed) { + assert.NoError(t, err) // require must only be used in the goroutine running the test function (testifylint) + } }() return srv } From 3f3974f14174b8137b58e5668695a2de04234f0e Mon Sep 17 00:00:00 2001 From: MikelAlejoBR Date: Tue, 5 May 2026 18:03:55 -0400 Subject: [PATCH 06/10] refactor: use a structure and JSON marshaller for logging requests Previously, a field that included a quote or a new line could break the log line due to how it was constructed with the "Sprintf" statement. It could also potentially lead to log injections with bad values. By using a defined structure and a JSON marshaller, we avoid both those issues. SANDBOX-316 --- pkg/server/server.go | 46 ++++++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/pkg/server/server.go b/pkg/server/server.go index 89c69f58..f7c87390 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -2,7 +2,7 @@ package server import ( "crypto/tls" - "fmt" + "encoding/json" "net/http" "strings" "sync" @@ -15,6 +15,21 @@ import ( "github.com/labstack/echo/v4/middleware" ) +// requestLog defines the structure with which the requests will be logged by +// the logging middelware of Echo. +type requestLog struct { + LogLevel string `json:"level"` + ClientIP string `json:"client_ip"` + Timestamp string `json:"timestamp"` + HttpMethod string `json:"method"` + HttpPath string `json:"path"` + Protocol string `json:"proto"` + HttpStatus int `json:"status"` + Latency time.Duration `json:"latency"` + UserAgent string `json:"user-agent"` + ErrorMessage error `json:"error-message"` +} + type ServerOption = func(server *RegistrationServer) // nolint:revive // RegistrationServer bundles configuration, and HTTP server objects in a single @@ -46,23 +61,20 @@ func New(application application.Application) *RegistrationServer { LogUserAgent: true, LogError: true, LogValuesFunc: func(c echo.Context, v middleware.RequestLoggerValues) error { - errMsg := "" - if v.Error != nil { - errMsg = v.Error.Error() + payload := requestLog{ + LogLevel: "info", + ClientIP: v.RemoteIP, + Timestamp: time.Now().Format(time.RFC3339), + HttpMethod: v.Method, + HttpPath: v.URI, + Protocol: v.Protocol, + HttpStatus: v.Status, + Latency: v.Latency, + UserAgent: v.UserAgent, + ErrorMessage: v.Error, } - fmt.Printf(`{"level":"%s", "client-ip":"%s", "ts":"%s", "method":"%s", "path":"%s", "proto":"%s", "status":"%d", "latency":"%s", "user-agent":"%s", "error-message":"%s"}`+"\n", - "info", - v.RemoteIP, - time.Now().Format(time.RFC1123), - v.Method, - v.URI, - v.Protocol, - v.Status, - v.Latency, - v.UserAgent, - errMsg, - ) - return nil + + return json.NewEncoder(c.Logger().Output()).Encode(payload) }, }), middleware.Recover(), From a06d980b5d0ad3798990d6be445a11b9310a03de Mon Sep 17 00:00:00 2001 From: MikelAlejoBR Date: Tue, 5 May 2026 18:10:47 -0400 Subject: [PATCH 07/10] fix: request context not used for Kubernetes calls The "DoGetSignup" function was making calls with a whole new context instead of taking into account the request context, so in the event that the parent context got cancelled, those Kubernetes calls did not get cancelled. SANDBOX-316 --- pkg/context/keys.go | 16 +++++++- pkg/signup/service/signup_service.go | 39 ++++++++----------- pkg/signup/service/signup_service_test.go | 10 ++--- .../service/verification_service.go | 30 +++++++------- .../service/verification_service_test.go | 12 +++--- 5 files changed, 57 insertions(+), 50 deletions(-) diff --git a/pkg/context/keys.go b/pkg/context/keys.go index 95046cb5..04f74898 100644 --- a/pkg/context/keys.go +++ b/pkg/context/keys.go @@ -1,6 +1,20 @@ package context -import "github.com/labstack/echo/v4" +import ( + gocontext "context" + + "github.com/labstack/echo/v4" +) + +// RequestContext extracts the standard context.Context from an Echo request +// for use with Kubernetes client calls. Falls back to context.TODO() when the +// Echo context or its underlying request is nil (e.g. in tests). +func RequestContext(ctx echo.Context) gocontext.Context { + if ctx != nil && ctx.Request() != nil { + return ctx.Request().Context() + } + return gocontext.TODO() +} // GetString returns the string value associated with the given key in the echo context. // Returns empty string if the key doesn't exist or the value is not a string. diff --git a/pkg/signup/service/signup_service.go b/pkg/signup/service/signup_service.go index b201d33f..a91dbf52 100644 --- a/pkg/signup/service/signup_service.go +++ b/pkg/signup/service/signup_service.go @@ -2,7 +2,6 @@ package service import ( "bytes" - gocontext "context" "encoding/json" "fmt" "io" @@ -61,13 +60,6 @@ func NewSignupService(client namespaced.Client) *ServiceImpl { } } -func requestContext(ctx echo.Context) gocontext.Context { - if ctx != nil && ctx.Request() != nil { - return ctx.Request().Context() - } - return gocontext.TODO() -} - // newUserSignup generates a new UserSignup resource with the specified username and available claims. // This resource then can be used to create a new UserSignup in the host cluster or to update the existing one. func (s *ServiceImpl) newUserSignup(ctx echo.Context) (*toolchainv1alpha1.UserSignup, error) { @@ -91,7 +83,7 @@ func (s *ServiceImpl) newUserSignup(ctx echo.Context) (*toolchainv1alpha1.UserSi // Query BannedUsers to check the user has not been banned bannedUsers := &toolchainv1alpha1.BannedUserList{} - if err := s.List(requestContext(ctx), bannedUsers, client.InNamespace(s.Namespace), + if err := s.List(context.RequestContext(ctx), bannedUsers, client.InNamespace(s.Namespace), client.MatchingLabels{toolchainv1alpha1.BannedUserEmailHashLabelKey: hash.EncodeString(userEmail)}); err != nil { return nil, err } @@ -269,7 +261,7 @@ var accountVerifierHTTPClient = &http.Client{Timeout: 3 * time.Second} // callAccountVerifier calls the account verifier service to check the user's email domain. // For now this is used only for monitoring — the result is logged but not acted upon. -func callAccountVerifier(ctx *gin.Context, verifierURL, email string) error { +func callAccountVerifier(ctx echo.Context, verifierURL, email string) error { reqBody, err := json.Marshal(accountVerifierRequest{Email: email}) if err != nil { return errs.Wrap(err, "failed to marshal account verifier request") @@ -300,13 +292,13 @@ func callAccountVerifier(ctx *gin.Context, verifierURL, email string) error { return nil } -func (s *ServiceImpl) verifyAccount(ctx *gin.Context) { +func (s *ServiceImpl) verifyAccount(ctx echo.Context) { cfg := configuration.GetRegistrationServiceConfig() verifierURL := cfg.AccountVerifierURL() if verifierURL == "" { return } - email := ctx.GetString(context.EmailKey) + email := context.GetString(ctx, context.EmailKey) if email == "" { return } @@ -323,7 +315,7 @@ func (s *ServiceImpl) Signup(ctx echo.Context) (*toolchainv1alpha1.UserSignup, e // Retrieve UserSignup resource from the host cluster userSignup := &toolchainv1alpha1.UserSignup{} - if err := s.Get(requestContext(ctx), s.NamespacedName(encodedUsername), userSignup); err != nil { + if err := s.Get(context.RequestContext(ctx), s.NamespacedName(encodedUsername), userSignup); err != nil { if apierrors.IsNotFound(err) { // New Signup log.WithValues(map[string]interface{}{"encoded_username": encodedUsername}).Info(ctx, "user not found, creating a new one") @@ -352,7 +344,7 @@ func (s *ServiceImpl) createUserSignup(ctx echo.Context) (*toolchainv1alpha1.Use return nil, err } - return userSignup, s.Create(requestContext(ctx), userSignup) + return userSignup, s.Create(context.RequestContext(ctx), userSignup) } // reactivateUserSignup reactivates the deactivated UserSignup resource with the specified username @@ -378,7 +370,7 @@ func (s *ServiceImpl) reactivateUserSignup(ctx echo.Context, existing *toolchain existing.Labels = newUserSignup.Labels existing.Spec = newUserSignup.Spec - return existing, s.Update(requestContext(ctx), existing) + return existing, s.Update(context.RequestContext(ctx), existing) } // GetSignup returns Signup resource which represents the corresponding K8s UserSignup @@ -396,7 +388,7 @@ func (s *ServiceImpl) DoGetSignup(ctx echo.Context, cl namespaced.Client, userna err := signup.PollUpdateSignup(ctx, func() error { // Retrieve UserSignup resource from the host cluster us := &toolchainv1alpha1.UserSignup{} - if err := cl.Get(gocontext.TODO(), cl.NamespacedName(signupcommon.EncodeUserIdentifier(username)), us); err != nil { + if err := cl.Get(context.RequestContext(ctx), cl.NamespacedName(signupcommon.EncodeUserIdentifier(username)), us); err != nil { if apierrors.IsNotFound(err) { return nil } @@ -414,7 +406,7 @@ func (s *ServiceImpl) DoGetSignup(ctx echo.Context, cl namespaced.Client, userna // If there is no need to update the UserSignup then break out of the loop here (by returning nil) // otherwise update the UserSignup if updated { - if err := s.Update(gocontext.TODO(), userSignup); err != nil { + if err := s.Update(context.RequestContext(ctx), userSignup); err != nil { return err } } @@ -488,7 +480,7 @@ func (s *ServiceImpl) DoGetSignup(ctx echo.Context, cl namespaced.Client, userna // If UserSignup status is complete as active // Retrieve MasterUserRecord resource from the host cluster and use its status mur := &toolchainv1alpha1.MasterUserRecord{} - if err := cl.Get(requestContext(ctx), cl.NamespacedName(userSignup.Status.CompliantUsername), mur); err != nil { + if err := cl.Get(context.RequestContext(ctx), cl.NamespacedName(userSignup.Status.CompliantUsername), mur); err != nil { return nil, errs.Wrap(err, fmt.Sprintf("error when retrieving MasterUserRecord for completed UserSignup %s", userSignup.GetName())) } murCondition, _ := condition.FindConditionByType(mur.Status.Conditions, toolchainv1alpha1.ConditionReady) @@ -507,12 +499,12 @@ func (s *ServiceImpl) DoGetSignup(ctx echo.Context, cl namespaced.Client, userna signupResponse.StartDate = mur.Status.ProvisionedTime.UTC().Format(time.RFC3339) } - memberCluster, defaultNamespace := GetDefaultUserTarget(cl, userSignup.Status.HomeSpace, mur.Name) + memberCluster, defaultNamespace := GetDefaultUserTarget(ctx, cl, userSignup.Status.HomeSpace, mur.Name) if memberCluster != "" { // Retrieve cluster-specific URLs from the status of the corresponding member cluster status := &toolchainv1alpha1.ToolchainStatus{} - if err := cl.Get(requestContext(ctx), cl.NamespacedName("toolchain-status"), status); err != nil { + if err := cl.Get(context.RequestContext(ctx), cl.NamespacedName("toolchain-status"), status); err != nil { return nil, errs.Wrapf(err, "error when retrieving ToolchainStatus for completed UserSignup %s", userSignup.GetName()) } signupResponse.ProxyURL = status.Status.HostRoutes.ProxyURL @@ -596,10 +588,11 @@ func (s *ServiceImpl) auditUserSignupAgainstClaims(ctx echo.Context, userSignup // 2. the name of the default namespace // // If the user doesn't have access to any Space, then empty strings are returned -func GetDefaultUserTarget(cl namespaced.Client, spaceName, murName string) (string, string) { +func GetDefaultUserTarget(ctx echo.Context, cl namespaced.Client, spaceName, murName string) (string, string) { + reqCtx := context.RequestContext(ctx) if spaceName == "" { sbs := &toolchainv1alpha1.SpaceBindingList{} - if err := cl.List(gocontext.TODO(), sbs, client.InNamespace(cl.Namespace), + if err := cl.List(reqCtx, sbs, client.InNamespace(cl.Namespace), client.MatchingLabels{toolchainv1alpha1.SpaceBindingMasterUserRecordLabelKey: murName}); err != nil { log.Errorf(nil, err, "unable to list spacebindings for MUR %s", murName) return "", "" @@ -616,7 +609,7 @@ func GetDefaultUserTarget(cl namespaced.Client, spaceName, murName string) (stri } space := &toolchainv1alpha1.Space{} - if err := cl.Get(gocontext.TODO(), cl.NamespacedName(spaceName), space); err != nil { + if err := cl.Get(reqCtx, cl.NamespacedName(spaceName), space); err != nil { // log error and continue so that the api behaves in a best effort manner // ie. if a space isn't listed something went wrong but we still want to return the other spaces if possible log.Errorf(nil, err, "unable to get space '%s'", spaceName) diff --git a/pkg/signup/service/signup_service_test.go b/pkg/signup/service/signup_service_test.go index e106948c..7cf4f723 100644 --- a/pkg/signup/service/signup_service_test.go +++ b/pkg/signup/service/signup_service_test.go @@ -1059,7 +1059,7 @@ func (s *TestSignupServiceSuite) TestGetDefaultUserNamespace() { nsClient := namespaced.NewClient(fakeClient, commontest.HostOperatorNs) // when - targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(nsClient, "dave", "dave") + targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(nil, nsClient, "dave", "dave") // then assert.Equal(s.T(), "dave-dev", defaultUserNamespace) @@ -1084,7 +1084,7 @@ func (s *TestSignupServiceSuite) TestGetDefaultUserNamespaceFromFirstUnownedSpac nsClient := namespaced.NewClient(fakeClient, commontest.HostOperatorNs) // when - targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(nsClient, "", "userB") + targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(nil, nsClient, "", "userB") // then assert.Equal(s.T(), "userA-dev", defaultUserNamespace) @@ -1109,7 +1109,7 @@ func (s *TestSignupServiceSuite) TestGetDefaultUserNamespaceMultiSpace() { nsClient := namespaced.NewClient(fakeClient, commontest.HostOperatorNs) // when - targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(nsClient, "userB", "userB") + targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(nil, nsClient, "userB", "userB") // then assert.Equal(s.T(), "userB-dev", defaultUserNamespace) // space2 is prioritized over space1 because it was created by the userB @@ -1126,7 +1126,7 @@ func (s *TestSignupServiceSuite) TestGetDefaultUserNamespaceFailNoHomeSpaceNoSpa nsClient := namespaced.NewClient(fakeClient, commontest.HostOperatorNs) // when - targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(nsClient, "", "dave") + targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(nil, nsClient, "", "dave") // then assert.Empty(s.T(), defaultUserNamespace) @@ -1140,7 +1140,7 @@ func (s *TestSignupServiceSuite) TestGetDefaultUserNamespaceFailNoSpace() { nsClient := namespaced.NewClient(fakeClient, commontest.HostOperatorNs) // when - targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(nsClient, "dave", "dave") + targetCluster, defaultUserNamespace := service.GetDefaultUserTarget(nil, nsClient, "dave", "dave") // then assert.Empty(s.T(), defaultUserNamespace) diff --git a/pkg/verification/service/verification_service.go b/pkg/verification/service/verification_service.go index a5fa22dd..33ba654e 100644 --- a/pkg/verification/service/verification_service.go +++ b/pkg/verification/service/verification_service.go @@ -1,7 +1,6 @@ package service import ( - gocontext "context" "crypto/rand" "errors" "fmt" @@ -65,7 +64,7 @@ func NewVerificationService(client namespaced.Client) service.VerificationServic // to manage the phone verification process and protect against system abuse. func (s *ServiceImpl) InitVerification(ctx echo.Context, username, e164PhoneNumber, countryCode string) error { signup := &toolchainv1alpha1.UserSignup{} - if err := s.Get(gocontext.TODO(), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { + if err := s.Get(context.RequestContext(ctx), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { if apierrors.IsNotFound(err) { log.Error(ctx, err, "usersignup not found") return crterrors.NewNotFoundError(err, "usersignup not found") @@ -84,7 +83,7 @@ func (s *ServiceImpl) InitVerification(ctx echo.Context, username, e164PhoneNumb } // Check if the provided phone number is already being used by another user - err := PhoneNumberAlreadyInUse(s.Client, username, e164PhoneNumber) + err := PhoneNumberAlreadyInUse(ctx, s.Client, username, e164PhoneNumber) if err != nil { e := &crterrors.Error{} switch { @@ -169,7 +168,7 @@ func (s *ServiceImpl) InitVerification(ctx echo.Context, username, e164PhoneNumb // Single update operation: always set phone hash label, set annotations only if notification was sent doUpdate := func() error { signup := &toolchainv1alpha1.UserSignup{} - if err := s.Get(gocontext.TODO(), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { + if err := s.Get(context.RequestContext(ctx), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { return err } @@ -189,7 +188,7 @@ func (s *ServiceImpl) InitVerification(ctx echo.Context, username, e164PhoneNumb signup.Annotations[k] = v } - return s.Update(gocontext.TODO(), signup) + return s.Update(context.RequestContext(ctx), signup) } updateErr := signuppkg.PollUpdateSignup(ctx, doUpdate) @@ -224,7 +223,7 @@ func (s *ServiceImpl) VerifyPhoneCode(ctx echo.Context, username, code string) ( cfg := configuration.GetRegistrationServiceConfig() // If we can't even find the UserSignup, then die here signup := &toolchainv1alpha1.UserSignup{} - if err := s.Get(gocontext.TODO(), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { + if err := s.Get(context.RequestContext(ctx), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { if apierrors.IsNotFound(err) { log.Error(ctx, err, "usersignup not found") return crterrors.NewNotFoundError(err, "user not found") @@ -260,7 +259,7 @@ func (s *ServiceImpl) VerifyPhoneCode(ctx echo.Context, username, code string) ( annotationsToDelete := []string{} unsetVerificationRequired := false - err := PhoneNumberAlreadyInUse(s.Client, username, signup.Labels[toolchainv1alpha1.UserSignupUserPhoneHashLabelKey]) + err := PhoneNumberAlreadyInUse(ctx, s.Client, username, signup.Labels[toolchainv1alpha1.UserSignupUserPhoneHashLabelKey]) if err != nil { log.Error(ctx, err, "phone number to verify already in use") return crterrors.NewBadRequest("phone number already in use", @@ -321,7 +320,7 @@ func (s *ServiceImpl) VerifyPhoneCode(ctx echo.Context, username, code string) ( doUpdate := func() error { signup := &toolchainv1alpha1.UserSignup{} - if err := s.Get(gocontext.TODO(), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { + if err := s.Get(context.RequestContext(ctx), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { log.Error(ctx, err, fmt.Sprintf("error getting signup with username '%s'", username)) return err } @@ -342,7 +341,7 @@ func (s *ServiceImpl) VerifyPhoneCode(ctx echo.Context, username, code string) ( delete(signup.Annotations, annotationName) } - if err := s.Update(gocontext.TODO(), signup); err != nil { + if err := s.Update(context.RequestContext(ctx), signup); err != nil { log.Error(ctx, err, fmt.Sprintf("error updating usersignup: %s", signup.Name)) return err } @@ -387,7 +386,7 @@ func (s *ServiceImpl) VerifyActivationCode(ctx echo.Context, username, code stri log.Infof(ctx, "verifying activation code '%s'", code) // look-up the UserSignup signup := &toolchainv1alpha1.UserSignup{} - if err := s.Get(gocontext.TODO(), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { + if err := s.Get(context.RequestContext(ctx), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { if apierrors.IsNotFound(err) { // signup user ctx.Set(context.SocialEvent, code) @@ -404,7 +403,7 @@ func (s *ServiceImpl) VerifyActivationCode(ctx echo.Context, username, code stri var errToReturn error doUpdate := func() error { signup := &toolchainv1alpha1.UserSignup{} - if err := s.Get(gocontext.TODO(), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { + if err := s.Get(context.RequestContext(ctx), s.NamespacedName(signupcommon.EncodeUserIdentifier(username)), signup); err != nil { return err } if signup.Annotations == nil { @@ -421,7 +420,7 @@ func (s *ServiceImpl) VerifyActivationCode(ctx echo.Context, username, code stri delete(signup.Annotations, toolchainv1alpha1.UserVerificationAttemptsAnnotationKey) } - if err := s.Update(gocontext.TODO(), signup); err != nil { + if err := s.Update(context.RequestContext(ctx), signup); err != nil { return err } @@ -445,14 +444,15 @@ var ( // an internal server error. If not, check if an approved UserSignup with a different username // and email address exists. If so, return an internal server error. Otherwise, return without error. // Either the actual phone number, or the md5 hash of the phone number may be provided here. -func PhoneNumberAlreadyInUse(cl namespaced.Client, username, phoneNumberOrHash string) error { +func PhoneNumberAlreadyInUse(ctx echo.Context, cl namespaced.Client, username, phoneNumberOrHash string) error { labelValue := hash.EncodeString(phoneNumberOrHash) if md5Matcher.Match([]byte(phoneNumberOrHash)) { labelValue = phoneNumberOrHash } + reqCtx := context.RequestContext(ctx) bannedUserList := &toolchainv1alpha1.BannedUserList{} - if err := cl.List(gocontext.TODO(), bannedUserList, client.InNamespace(cl.Namespace), + if err := cl.List(reqCtx, bannedUserList, client.InNamespace(cl.Namespace), client.MatchingLabels{toolchainv1alpha1.BannedUserPhoneNumberHashLabelKey: labelValue}); err != nil { return crterrors.NewInternalError(err, "failed listing banned users") } @@ -466,7 +466,7 @@ func PhoneNumberAlreadyInUse(cl namespaced.Client, username, phoneNumberOrHash s toolchainv1alpha1.BannedUserPhoneNumberHashLabelKey: labelValue, } userSignups := &toolchainv1alpha1.UserSignupList{} - if err := cl.List(gocontext.TODO(), userSignups, client.InNamespace(cl.Namespace), labelSelector); err != nil { + if err := cl.List(reqCtx, userSignups, client.InNamespace(cl.Namespace), labelSelector); err != nil { return crterrors.NewInternalError(err, "failed listing userSignups") } diff --git a/pkg/verification/service/verification_service_test.go b/pkg/verification/service/verification_service_test.go index 22175269..3d3c621f 100644 --- a/pkg/verification/service/verification_service_test.go +++ b/pkg/verification/service/verification_service_test.go @@ -904,7 +904,7 @@ func (s *TestVerificationServiceSuite) TestPhoneNumberAlreadyInUse() { nsdClient := namespaced.NewClient(fakeClient, commontest.HostOperatorNs) // when - err := verificationservice.PhoneNumberAlreadyInUse(nsdClient, "jsmith", "+987654321") + err := verificationservice.PhoneNumberAlreadyInUse(nil, nsdClient, "jsmith", "+987654321") // then require.NoError(s.T(), err) @@ -924,7 +924,7 @@ func (s *TestVerificationServiceSuite) TestPhoneNumberAlreadyInUse() { nsdClient := namespaced.NewClient(fakeClient, commontest.HostOperatorNs) // when - err := verificationservice.PhoneNumberAlreadyInUse(nsdClient, "jsmith", "+987654321") + err := verificationservice.PhoneNumberAlreadyInUse(nil, nsdClient, "jsmith", "+987654321") // then require.NoError(s.T(), err) @@ -937,7 +937,7 @@ func (s *TestVerificationServiceSuite) TestPhoneNumberAlreadyInUse() { nsdClient := namespaced.NewClient(fakeClient, commontest.HostOperatorNs) // when - err := verificationservice.PhoneNumberAlreadyInUse(nsdClient, "jsmith", "+12268213044") + err := verificationservice.PhoneNumberAlreadyInUse(nil, nsdClient, "jsmith", "+12268213044") // then require.EqualError(s.T(), err, "cannot re-register with phone number: phone number already in use") @@ -949,7 +949,7 @@ func (s *TestVerificationServiceSuite) TestPhoneNumberAlreadyInUse() { nsdClient := namespaced.NewClient(fakeClient, commontest.HostOperatorNs) // when - err := verificationservice.PhoneNumberAlreadyInUse(nsdClient, "jsmith", "+12268213044") + err := verificationservice.PhoneNumberAlreadyInUse(nil, nsdClient, "jsmith", "+12268213044") // then require.EqualError(s.T(), err, "cannot re-register with phone number: phone number already in use") @@ -968,7 +968,7 @@ func (s *TestVerificationServiceSuite) TestPhoneNumberAlreadyInUse() { nsdClient := namespaced.NewClient(fakeClient, commontest.HostOperatorNs) // when - err := verificationservice.PhoneNumberAlreadyInUse(nsdClient, "jsmith", "+12268213044") + err := verificationservice.PhoneNumberAlreadyInUse(nil, nsdClient, "jsmith", "+12268213044") // then require.EqualError(s.T(), err, "list error: failed listing banned users") @@ -986,7 +986,7 @@ func (s *TestVerificationServiceSuite) TestPhoneNumberAlreadyInUse() { nsdClient := namespaced.NewClient(fakeClient, commontest.HostOperatorNs) // when - err := verificationservice.PhoneNumberAlreadyInUse(nsdClient, "jsmith", "+12268213044") + err := verificationservice.PhoneNumberAlreadyInUse(nil, nsdClient, "jsmith", "+12268213044") // then require.EqualError(s.T(), err, "list error: failed listing userSignups") From f0862f5e3cff1d5188153b94a4db6062843cbd9c Mon Sep 17 00:00:00 2001 From: MikelAlejoBR Date: Tue, 5 May 2026 18:34:45 -0400 Subject: [PATCH 08/10] fix: linting issues SANDBOX-316 --- pkg/server/server.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/server/server.go b/pkg/server/server.go index f7c87390..b9d6da39 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -21,10 +21,10 @@ type requestLog struct { LogLevel string `json:"level"` ClientIP string `json:"client_ip"` Timestamp string `json:"timestamp"` - HttpMethod string `json:"method"` - HttpPath string `json:"path"` + HTTPMethod string `json:"method"` + HTTPPath string `json:"path"` Protocol string `json:"proto"` - HttpStatus int `json:"status"` + HTTPStatus int `json:"status"` Latency time.Duration `json:"latency"` UserAgent string `json:"user-agent"` ErrorMessage error `json:"error-message"` @@ -65,10 +65,10 @@ func New(application application.Application) *RegistrationServer { LogLevel: "info", ClientIP: v.RemoteIP, Timestamp: time.Now().Format(time.RFC3339), - HttpMethod: v.Method, - HttpPath: v.URI, + HTTPMethod: v.Method, + HTTPPath: v.URI, Protocol: v.Protocol, - HttpStatus: v.Status, + HTTPStatus: v.Status, Latency: v.Latency, UserAgent: v.UserAgent, ErrorMessage: v.Error, From db17303e54f7eeeec9fd9ca93d9de4251ac743c3 Mon Sep 17 00:00:00 2001 From: MikelAlejoBR Date: Wed, 6 May 2026 07:21:24 -0400 Subject: [PATCH 09/10] refactor: make binding failure log more generic Since binding can fail due to other reasons other than just an unexpected input, we are making the logging message more generic to avoid confusion when debugging this part of the code. SANDBOX-316 --- pkg/controller/signup.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/controller/signup.go b/pkg/controller/signup.go index 3ff3d350..8d697c6c 100644 --- a/pkg/controller/signup.go +++ b/pkg/controller/signup.go @@ -62,7 +62,7 @@ func (s *Signup) InitVerificationHandler(ctx echo.Context) error { // Read the Body content var phone Phone if err := ctx.Bind(&phone); err != nil { - log.Errorf(ctx, err, "request body does not contain required fields phone_number and country_code") + log.Errorf(ctx, err, "unable to bind verification request body") return crterrors.AbortWithError(ctx, http.StatusBadRequest, err, "error reading request body") } @@ -151,7 +151,7 @@ func (s *Signup) VerifyPhoneCodeHandler(ctx echo.Context) error { func (s *Signup) VerifyActivationCodeHandler(ctx echo.Context) error { body := map[string]interface{}{} if err := ctx.Bind(&body); err != nil { - log.Error(ctx, nil, "no activation code provided in the request") + log.Error(ctx, nil, "unable to bind verification request body") return ctx.NoContent(http.StatusBadRequest) } code, ok := body["code"].(string) From c675e0723b6ebda2930793ef90d724b518993097 Mon Sep 17 00:00:00 2001 From: MikelAlejoBR Date: Wed, 6 May 2026 07:24:18 -0400 Subject: [PATCH 10/10] fix: error messages not being properly logged in HTTP logging Using the error directly might not marshal to its string value, thus rendering that field useless when logging HTTP requests. SANDBOX-316 --- pkg/server/server.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/server/server.go b/pkg/server/server.go index b9d6da39..08a8fa9e 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -27,7 +27,7 @@ type requestLog struct { HTTPStatus int `json:"status"` Latency time.Duration `json:"latency"` UserAgent string `json:"user-agent"` - ErrorMessage error `json:"error-message"` + ErrorMessage string `json:"error-message"` } type ServerOption = func(server *RegistrationServer) // nolint:revive @@ -61,6 +61,11 @@ func New(application application.Application) *RegistrationServer { LogUserAgent: true, LogError: true, LogValuesFunc: func(c echo.Context, v middleware.RequestLoggerValues) error { + errMsg := "" + if v.Error != nil { + errMsg = v.Error.Error() + } + payload := requestLog{ LogLevel: "info", ClientIP: v.RemoteIP, @@ -71,7 +76,7 @@ func New(application application.Application) *RegistrationServer { HTTPStatus: v.Status, Latency: v.Latency, UserAgent: v.UserAgent, - ErrorMessage: v.Error, + ErrorMessage: errMsg, } return json.NewEncoder(c.Logger().Output()).Encode(payload)