From 06624ee50c7f231e2f1bce6ab96d347ee859be20 Mon Sep 17 00:00:00 2001 From: mustard Date: Tue, 27 Aug 2024 09:07:54 +0000 Subject: [PATCH 1/8] Remove allowCredentials --- components/ws-proxy/pkg/proxy/routes.go | 55 ------------------------- 1 file changed, 55 deletions(-) diff --git a/components/ws-proxy/pkg/proxy/routes.go b/components/ws-proxy/pkg/proxy/routes.go index 6af4d7bc5efc31..dde6c217fca61d 100644 --- a/components/ws-proxy/pkg/proxy/routes.go +++ b/components/ws-proxy/pkg/proxy/routes.go @@ -45,7 +45,6 @@ import ( type RouteHandlerConfig struct { Config *Config DefaultTransport http.RoundTripper - CorsHandler mux.MiddlewareFunc WorkspaceAuthHandler mux.MiddlewareFunc } @@ -61,15 +60,9 @@ func WithDefaultAuth(infoprov common.WorkspaceInfoProvider) RouteHandlerConfigOp // NewRouteHandlerConfig creates a new instance. func NewRouteHandlerConfig(config *Config, opts ...RouteHandlerConfigOpt) (*RouteHandlerConfig, error) { - corsHandler, err := corsHandler(config.GitpodInstallation.Scheme, config.GitpodInstallation.HostName) - if err != nil { - return nil, err - } - cfg := &RouteHandlerConfig{ Config: config, DefaultTransport: createDefaultTransport(config.TransportConfig), - CorsHandler: corsHandler, WorkspaceAuthHandler: func(h http.Handler) http.Handler { return h }, } for _, o := range opts { @@ -179,7 +172,6 @@ func (ir *ideRoutes) HandleSSHHostKeyRoute(route *mux.Route, hostKeyList []ssh.S } r := route.Subrouter() r.Use(logRouteHandlerHandler("HandleSSHHostKeyRoute")) - r.Use(ir.Config.CorsHandler) r.NewRoute().HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { rw.Header().Add("Content-Type", "application/json") rw.Write(byt) @@ -189,7 +181,6 @@ func (ir *ideRoutes) HandleSSHHostKeyRoute(route *mux.Route, hostKeyList []ssh.S func (ir *ideRoutes) HandleCreateKeyRoute(route *mux.Route, hostKeyList []ssh.Signer) { r := route.Subrouter() r.Use(logRouteHandlerHandler("HandleCreateKeyRoute")) - r.Use(ir.Config.CorsHandler) r.Use(ir.workspaceMustExistHandler) r.Use(ir.Config.WorkspaceAuthHandler) @@ -253,7 +244,6 @@ func extractCloseErrorCode(errStr string) string { func (ir *ideRoutes) HandleSSHOverWebsocketTunnel(route *mux.Route, sshGatewayServer *sshproxy.Server) { r := route.Subrouter() r.Use(logRouteHandlerHandler("HandleSSHOverWebsocketTunnel")) - r.Use(ir.Config.CorsHandler) r.Use(ir.workspaceMustExistHandler) r.Use(ir.Config.WorkspaceAuthHandler) @@ -297,7 +287,6 @@ func (ir *ideRoutes) HandleSSHOverWebsocketTunnel(route *mux.Route, sshGatewaySe func (ir *ideRoutes) HandleDirectSupervisorRoute(route *mux.Route, authenticated bool) { r := route.Subrouter() r.Use(logRouteHandlerHandler(fmt.Sprintf("HandleDirectSupervisorRoute (authenticated: %v)", authenticated))) - r.Use(ir.Config.CorsHandler) r.Use(ir.workspaceMustExistHandler) if authenticated { r.Use(ir.Config.WorkspaceAuthHandler) @@ -382,7 +371,6 @@ type BlobserveInlineVars struct { func (ir *ideRoutes) HandleRoot(route *mux.Route) { r := route.Subrouter() r.Use(logRouteHandlerHandler("handleRoot")) - r.Use(ir.Config.CorsHandler) r.Use(ir.workspaceMustExistHandler) proxyPassWoSensitiveCookies := sensitiveCookieHandler(ir.Config.Config.GitpodInstallation.HostName)(proxyPass(ir.Config, ir.InfoProvider, workspacePodResolver)) @@ -516,7 +504,6 @@ func installDebugWorkspaceRoutes(r *mux.Router, config *RouteHandlerConfig, info } r.Use(logHandler) - r.Use(config.CorsHandler) r.Use(config.WorkspaceAuthHandler) // filter all session cookies r.Use(sensitiveCookieHandler(config.Config.GitpodInstallation.HostName)) @@ -654,48 +641,6 @@ func buildWorkspacePodURL(protocol api.PortProtocol, ipAddress string, port stri return url.Parse(fmt.Sprintf("%v://%v:%v", portProtocol, ipAddress, port)) } -// corsHandler produces the CORS handler for workspaces. -func corsHandler(scheme, hostname string) (mux.MiddlewareFunc, error) { - origin := fmt.Sprintf("%s://%s", scheme, hostname) - - domainRegex := strings.ReplaceAll(hostname, ".", "\\.") - originRegex, err := regexp.Compile(".*" + domainRegex) - if err != nil { - return nil, err - } - - return handlers.CORS( - handlers.AllowedOriginValidator(func(origin string) bool { - // Is the origin a subdomain of the installations hostname? - matches := originRegex.Match([]byte(origin)) - return matches - }), - // TODO(gpl) For domain-based workspace access with authentication (for accessing the IDE) we need to respond with the precise Origin header that was sent - handlers.AllowedOrigins([]string{origin}), - handlers.AllowedMethods([]string{ - "GET", - "POST", - "OPTIONS", - }), - handlers.AllowedHeaders([]string{ - // "Accept", "Accept-Language", "Content-Language" are allowed per default - "Cache-Control", - "Content-Type", - "DNT", - "If-Modified-Since", - "Keep-Alive", - "Origin", - "User-Agent", - "X-Requested-With", - }), - handlers.AllowCredentials(), - // required to be able to read Authorization header in frontend - handlers.ExposedHeaders([]string{"Authorization"}), - handlers.MaxAge(60), - handlers.OptionStatusCode(200), - ), nil -} - type wsproxyContextKey struct{} var ( From 87aaf052ba1805124596c2133ca35f322176395e Mon Sep 17 00:00:00 2001 From: mustard Date: Tue, 27 Aug 2024 09:15:38 +0000 Subject: [PATCH 2/8] fix tests --- components/ws-proxy/pkg/proxy/routes_test.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/components/ws-proxy/pkg/proxy/routes_test.go b/components/ws-proxy/pkg/proxy/routes_test.go index 187eac00ff894e..0077897822b1a3 100644 --- a/components/ws-proxy/pkg/proxy/routes_test.go +++ b/components/ws-proxy/pkg/proxy/routes_test.go @@ -450,7 +450,7 @@ func TestRoutes(t *testing.T) { }, }, { - Desc: "CORS preflight", + Desc: "no CORS allow in workspace urls", Config: &config, Request: modifyRequest(httptest.NewRequest("GET", workspaces[0].URL+"somewhere/in/the/ide", nil), addHostHeader, @@ -462,12 +462,9 @@ func TestRoutes(t *testing.T) { Expectation: Expectation{ Status: http.StatusOK, Header: http.Header{ - "Access-Control-Allow-Credentials": {"true"}, - "Access-Control-Allow-Origin": {"test-domain.com"}, - "Access-Control-Expose-Headers": {"Authorization"}, - "Content-Length": {"37"}, - "Content-Type": {"text/plain; charset=utf-8"}, - "Vary": {"Accept-Encoding"}, + "Content-Length": {"37"}, + "Content-Type": {"text/plain; charset=utf-8"}, + "Vary": {"Accept-Encoding"}, }, Body: "workspace hit: /somewhere/in/the/ide\n", }, From 0aa58c7630a26045e9005e4f5bbd07e020e5f0ef Mon Sep 17 00:00:00 2001 From: mustard Date: Wed, 28 Aug 2024 09:40:56 +0000 Subject: [PATCH 3/8] Revert "Remove allowCredentials" This reverts commit 06624ee50c7f231e2f1bce6ab96d347ee859be20. --- components/ws-proxy/pkg/proxy/routes.go | 55 +++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/components/ws-proxy/pkg/proxy/routes.go b/components/ws-proxy/pkg/proxy/routes.go index dde6c217fca61d..6af4d7bc5efc31 100644 --- a/components/ws-proxy/pkg/proxy/routes.go +++ b/components/ws-proxy/pkg/proxy/routes.go @@ -45,6 +45,7 @@ import ( type RouteHandlerConfig struct { Config *Config DefaultTransport http.RoundTripper + CorsHandler mux.MiddlewareFunc WorkspaceAuthHandler mux.MiddlewareFunc } @@ -60,9 +61,15 @@ func WithDefaultAuth(infoprov common.WorkspaceInfoProvider) RouteHandlerConfigOp // NewRouteHandlerConfig creates a new instance. func NewRouteHandlerConfig(config *Config, opts ...RouteHandlerConfigOpt) (*RouteHandlerConfig, error) { + corsHandler, err := corsHandler(config.GitpodInstallation.Scheme, config.GitpodInstallation.HostName) + if err != nil { + return nil, err + } + cfg := &RouteHandlerConfig{ Config: config, DefaultTransport: createDefaultTransport(config.TransportConfig), + CorsHandler: corsHandler, WorkspaceAuthHandler: func(h http.Handler) http.Handler { return h }, } for _, o := range opts { @@ -172,6 +179,7 @@ func (ir *ideRoutes) HandleSSHHostKeyRoute(route *mux.Route, hostKeyList []ssh.S } r := route.Subrouter() r.Use(logRouteHandlerHandler("HandleSSHHostKeyRoute")) + r.Use(ir.Config.CorsHandler) r.NewRoute().HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { rw.Header().Add("Content-Type", "application/json") rw.Write(byt) @@ -181,6 +189,7 @@ func (ir *ideRoutes) HandleSSHHostKeyRoute(route *mux.Route, hostKeyList []ssh.S func (ir *ideRoutes) HandleCreateKeyRoute(route *mux.Route, hostKeyList []ssh.Signer) { r := route.Subrouter() r.Use(logRouteHandlerHandler("HandleCreateKeyRoute")) + r.Use(ir.Config.CorsHandler) r.Use(ir.workspaceMustExistHandler) r.Use(ir.Config.WorkspaceAuthHandler) @@ -244,6 +253,7 @@ func extractCloseErrorCode(errStr string) string { func (ir *ideRoutes) HandleSSHOverWebsocketTunnel(route *mux.Route, sshGatewayServer *sshproxy.Server) { r := route.Subrouter() r.Use(logRouteHandlerHandler("HandleSSHOverWebsocketTunnel")) + r.Use(ir.Config.CorsHandler) r.Use(ir.workspaceMustExistHandler) r.Use(ir.Config.WorkspaceAuthHandler) @@ -287,6 +297,7 @@ func (ir *ideRoutes) HandleSSHOverWebsocketTunnel(route *mux.Route, sshGatewaySe func (ir *ideRoutes) HandleDirectSupervisorRoute(route *mux.Route, authenticated bool) { r := route.Subrouter() r.Use(logRouteHandlerHandler(fmt.Sprintf("HandleDirectSupervisorRoute (authenticated: %v)", authenticated))) + r.Use(ir.Config.CorsHandler) r.Use(ir.workspaceMustExistHandler) if authenticated { r.Use(ir.Config.WorkspaceAuthHandler) @@ -371,6 +382,7 @@ type BlobserveInlineVars struct { func (ir *ideRoutes) HandleRoot(route *mux.Route) { r := route.Subrouter() r.Use(logRouteHandlerHandler("handleRoot")) + r.Use(ir.Config.CorsHandler) r.Use(ir.workspaceMustExistHandler) proxyPassWoSensitiveCookies := sensitiveCookieHandler(ir.Config.Config.GitpodInstallation.HostName)(proxyPass(ir.Config, ir.InfoProvider, workspacePodResolver)) @@ -504,6 +516,7 @@ func installDebugWorkspaceRoutes(r *mux.Router, config *RouteHandlerConfig, info } r.Use(logHandler) + r.Use(config.CorsHandler) r.Use(config.WorkspaceAuthHandler) // filter all session cookies r.Use(sensitiveCookieHandler(config.Config.GitpodInstallation.HostName)) @@ -641,6 +654,48 @@ func buildWorkspacePodURL(protocol api.PortProtocol, ipAddress string, port stri return url.Parse(fmt.Sprintf("%v://%v:%v", portProtocol, ipAddress, port)) } +// corsHandler produces the CORS handler for workspaces. +func corsHandler(scheme, hostname string) (mux.MiddlewareFunc, error) { + origin := fmt.Sprintf("%s://%s", scheme, hostname) + + domainRegex := strings.ReplaceAll(hostname, ".", "\\.") + originRegex, err := regexp.Compile(".*" + domainRegex) + if err != nil { + return nil, err + } + + return handlers.CORS( + handlers.AllowedOriginValidator(func(origin string) bool { + // Is the origin a subdomain of the installations hostname? + matches := originRegex.Match([]byte(origin)) + return matches + }), + // TODO(gpl) For domain-based workspace access with authentication (for accessing the IDE) we need to respond with the precise Origin header that was sent + handlers.AllowedOrigins([]string{origin}), + handlers.AllowedMethods([]string{ + "GET", + "POST", + "OPTIONS", + }), + handlers.AllowedHeaders([]string{ + // "Accept", "Accept-Language", "Content-Language" are allowed per default + "Cache-Control", + "Content-Type", + "DNT", + "If-Modified-Since", + "Keep-Alive", + "Origin", + "User-Agent", + "X-Requested-With", + }), + handlers.AllowCredentials(), + // required to be able to read Authorization header in frontend + handlers.ExposedHeaders([]string{"Authorization"}), + handlers.MaxAge(60), + handlers.OptionStatusCode(200), + ), nil +} + type wsproxyContextKey struct{} var ( From 13bb4cad17a9f2482ff5f7faf4a88a64e3615c8e Mon Sep 17 00:00:00 2001 From: Huiwen Date: Wed, 28 Aug 2024 12:06:27 +0000 Subject: [PATCH 4/8] Use FeatureFlag `ws_proxy_cors_enabled` --- components/ws-proxy/go.mod | 2 + components/ws-proxy/go.sum | 7 +++ components/ws-proxy/pkg/proxy/routes.go | 52 ++++++++++++++++--- .../pkg/components/ws-proxy/deployment.go | 1 + 4 files changed, 56 insertions(+), 6 deletions(-) diff --git a/components/ws-proxy/go.mod b/components/ws-proxy/go.mod index 50021149b40729..e1d2817fd4508e 100644 --- a/components/ws-proxy/go.mod +++ b/components/ws-proxy/go.mod @@ -30,8 +30,10 @@ require ( require ( github.com/beorn7/perks v1.0.1 // indirect + github.com/blang/semver v3.5.1+incompatible // indirect github.com/cenkalti/backoff/v4 v4.2.1 // indirect github.com/cespare/xxhash/v2 v2.2.0 // indirect + github.com/configcat/go-sdk/v7 v7.6.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/emicklei/go-restful/v3 v3.11.0 // indirect github.com/evanphx/json-patch/v5 v5.8.0 // indirect diff --git a/components/ws-proxy/go.sum b/components/ws-proxy/go.sum index 04a8ecf5e176e7..d5fdc8595668f8 100644 --- a/components/ws-proxy/go.sum +++ b/components/ws-proxy/go.sum @@ -6,6 +6,8 @@ github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a h1:idn718Q4 github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= +github.com/blang/semver v3.5.1+incompatible h1:cQNTCjp13qL8KC3Nbxr/y2Bqb63oX6wdnnjpJbkM4JQ= +github.com/blang/semver v3.5.1+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869 h1:DDGfHa7BWjL4YnC6+E63dPcxHo2sUxDIu8g3QgEJdRY= github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869/go.mod h1:Ekp36dRnpXw/yCqJaO+ZrUyxD+3VXMFFr56k5XYrpB4= github.com/bombsimon/logrusr/v2 v2.0.1 h1:1VgxVNQMCvjirZIYaT9JYn6sAVGVEcNtRE0y4mvaOAM= @@ -17,6 +19,8 @@ github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= +github.com/configcat/go-sdk/v7 v7.6.0 h1:CthQJ7DMz4bvUrpc8aek6VouJjisCvZCfuTG2gyNzL4= +github.com/configcat/go-sdk/v7 v7.6.0/go.mod h1:2245V6Igy1Xz6GXvcYuK5z996Ct0VyzyuI470XS6aTw= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -35,6 +39,8 @@ github.com/evanphx/json-patch/v5 v5.8.0/go.mod h1:VNkHZ/282BpEyt/tObQO8s5CMPmYYq github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/felixge/httpsnoop v1.0.3 h1:s/nj+GCswXYzN5v2DpNMuMQYe+0DDwt5WVCU6CWBdXk= github.com/felixge/httpsnoop v1.0.3/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= +github.com/frankban/quicktest v1.11.2 h1:mjwHjStlXWibxOohM7HYieIViKyh56mmt3+6viyhDDI= +github.com/frankban/quicktest v1.11.2/go.mod h1:K+q6oSqb0W0Ininfk863uOk1lMy69l/P6txr3mVT54s= github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA= github.com/fsnotify/fsnotify v1.7.0/go.mod h1:40Bi/Hjc2AVfZrqy+aj+yEI+/bRxZnMJyTJwOpGvigM= github.com/gitpod-io/golang-crypto v0.0.0-20231122075959-de838e9cb174 h1:OoCsCV3wxfN5FvbuhwuUgz6rZvsyfPxX3jZ2fKA+9H8= @@ -77,6 +83,7 @@ github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6 github.com/google/gnostic-models v0.6.8 h1:yo/ABAfM5IMRsS1VnXjTBvUb61tFIHozhlYvRgGre9I= github.com/google/gnostic-models v0.6.8/go.mod h1:5n7qKqH0f5wFt+aWF8CW6pZLLNOfYuF5OpfBSENuI8U= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= +github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= diff --git a/components/ws-proxy/pkg/proxy/routes.go b/components/ws-proxy/pkg/proxy/routes.go index 6af4d7bc5efc31..32b812612af989 100644 --- a/components/ws-proxy/pkg/proxy/routes.go +++ b/components/ws-proxy/pkg/proxy/routes.go @@ -34,6 +34,7 @@ import ( "github.com/sirupsen/logrus" "golang.org/x/xerrors" + "github.com/gitpod-io/gitpod/common-go/experiments" "github.com/gitpod-io/gitpod/common-go/log" gitpod "github.com/gitpod-io/gitpod/gitpod-protocol" "github.com/gitpod-io/gitpod/ws-manager/api" @@ -47,8 +48,12 @@ type RouteHandlerConfig struct { DefaultTransport http.RoundTripper CorsHandler mux.MiddlewareFunc WorkspaceAuthHandler mux.MiddlewareFunc + + CorsEnabled bool } +const experimentsCorsEnabled = "ws_proxy_cors_enabled" + // RouteHandlerConfigOpt modifies the router handler config. type RouteHandlerConfigOpt func(*Config, *RouteHandlerConfig) @@ -65,12 +70,19 @@ func NewRouteHandlerConfig(config *Config, opts ...RouteHandlerConfigOpt) (*Rout if err != nil { return nil, err } + experimentsClient := experiments.NewClient(experiments.WithPollInterval(time.Second * 3)) + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(time.Minute*5)) + defer cancel() + ffValue := waitStringValue(ctx, experimentsClient, experimentsCorsEnabled, "nope", experiments.Attributes{}) + corsEnabled := ffValue == "true" + log.WithField("ffValue", ffValue).WithField("corsEnabled", corsEnabled).Info("feature flag final value") cfg := &RouteHandlerConfig{ Config: config, DefaultTransport: createDefaultTransport(config.TransportConfig), CorsHandler: corsHandler, WorkspaceAuthHandler: func(h http.Handler) http.Handler { return h }, + CorsEnabled: corsEnabled, } for _, o := range opts { o(config, cfg) @@ -163,6 +175,22 @@ type ideRoutes struct { workspaceMustExistHandler mux.MiddlewareFunc } +func waitStringValue(ctx context.Context, client experiments.Client, experimentName, nopeValue string, attributes experiments.Attributes) string { + ticker := time.NewTicker(1 * time.Second) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return nopeValue + case <-ticker.C: + value := client.GetStringValue(ctx, experimentName, nopeValue, attributes) + if value != nopeValue { + return value + } + } + } +} + func (ir *ideRoutes) HandleSSHHostKeyRoute(route *mux.Route, hostKeyList []ssh.Signer) { shk := make([]struct { Type string `json:"type"` @@ -179,7 +207,9 @@ func (ir *ideRoutes) HandleSSHHostKeyRoute(route *mux.Route, hostKeyList []ssh.S } r := route.Subrouter() r.Use(logRouteHandlerHandler("HandleSSHHostKeyRoute")) - r.Use(ir.Config.CorsHandler) + if ir.Config.CorsEnabled { + r.Use(ir.Config.CorsHandler) + } r.NewRoute().HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { rw.Header().Add("Content-Type", "application/json") rw.Write(byt) @@ -189,7 +219,9 @@ func (ir *ideRoutes) HandleSSHHostKeyRoute(route *mux.Route, hostKeyList []ssh.S func (ir *ideRoutes) HandleCreateKeyRoute(route *mux.Route, hostKeyList []ssh.Signer) { r := route.Subrouter() r.Use(logRouteHandlerHandler("HandleCreateKeyRoute")) - r.Use(ir.Config.CorsHandler) + if ir.Config.CorsEnabled { + r.Use(ir.Config.CorsHandler) + } r.Use(ir.workspaceMustExistHandler) r.Use(ir.Config.WorkspaceAuthHandler) @@ -253,7 +285,9 @@ func extractCloseErrorCode(errStr string) string { func (ir *ideRoutes) HandleSSHOverWebsocketTunnel(route *mux.Route, sshGatewayServer *sshproxy.Server) { r := route.Subrouter() r.Use(logRouteHandlerHandler("HandleSSHOverWebsocketTunnel")) - r.Use(ir.Config.CorsHandler) + if ir.Config.CorsEnabled { + r.Use(ir.Config.CorsHandler) + } r.Use(ir.workspaceMustExistHandler) r.Use(ir.Config.WorkspaceAuthHandler) @@ -297,7 +331,9 @@ func (ir *ideRoutes) HandleSSHOverWebsocketTunnel(route *mux.Route, sshGatewaySe func (ir *ideRoutes) HandleDirectSupervisorRoute(route *mux.Route, authenticated bool) { r := route.Subrouter() r.Use(logRouteHandlerHandler(fmt.Sprintf("HandleDirectSupervisorRoute (authenticated: %v)", authenticated))) - r.Use(ir.Config.CorsHandler) + if ir.Config.CorsEnabled { + r.Use(ir.Config.CorsHandler) + } r.Use(ir.workspaceMustExistHandler) if authenticated { r.Use(ir.Config.WorkspaceAuthHandler) @@ -382,7 +418,9 @@ type BlobserveInlineVars struct { func (ir *ideRoutes) HandleRoot(route *mux.Route) { r := route.Subrouter() r.Use(logRouteHandlerHandler("handleRoot")) - r.Use(ir.Config.CorsHandler) + if ir.Config.CorsEnabled { + r.Use(ir.Config.CorsHandler) + } r.Use(ir.workspaceMustExistHandler) proxyPassWoSensitiveCookies := sensitiveCookieHandler(ir.Config.Config.GitpodInstallation.HostName)(proxyPass(ir.Config, ir.InfoProvider, workspacePodResolver)) @@ -516,7 +554,9 @@ func installDebugWorkspaceRoutes(r *mux.Router, config *RouteHandlerConfig, info } r.Use(logHandler) - r.Use(config.CorsHandler) + if config.CorsEnabled { + r.Use(config.CorsHandler) + } r.Use(config.WorkspaceAuthHandler) // filter all session cookies r.Use(sensitiveCookieHandler(config.Config.GitpodInstallation.HostName)) diff --git a/install/installer/pkg/components/ws-proxy/deployment.go b/install/installer/pkg/components/ws-proxy/deployment.go index 783050c1c0d805..d1280b4029d602 100644 --- a/install/installer/pkg/components/ws-proxy/deployment.go +++ b/install/installer/pkg/components/ws-proxy/deployment.go @@ -142,6 +142,7 @@ func deployment(ctx *common.RenderContext) ([]runtime.Object, error) { common.DefaultEnv(&ctx.Config), common.WorkspaceTracingEnv(ctx, Component), common.AnalyticsEnv(&ctx.Config), + common.ConfigcatEnv(ctx), )), ReadinessProbe: &corev1.Probe{ InitialDelaySeconds: int32(2), From c0ecc26cd894d6024e49703c49d0274a05032404 Mon Sep 17 00:00:00 2001 From: Huiwen Date: Wed, 28 Aug 2024 16:18:07 +0000 Subject: [PATCH 5/8] Fix test failed --- components/ws-proxy/pkg/config/config.go | 31 ++++++++++ components/ws-proxy/pkg/proxy/config.go | 1 + components/ws-proxy/pkg/proxy/routes.go | 64 +++++--------------- components/ws-proxy/pkg/proxy/routes_test.go | 1 + 4 files changed, 48 insertions(+), 49 deletions(-) diff --git a/components/ws-proxy/pkg/config/config.go b/components/ws-proxy/pkg/config/config.go index 97e812141f5022..0ac273789e3b67 100644 --- a/components/ws-proxy/pkg/config/config.go +++ b/components/ws-proxy/pkg/config/config.go @@ -5,14 +5,20 @@ package config import ( + "context" "encoding/json" "os" + "time" "golang.org/x/xerrors" + "github.com/gitpod-io/gitpod/common-go/experiments" + "github.com/gitpod-io/gitpod/common-go/log" "github.com/gitpod-io/gitpod/ws-proxy/pkg/proxy" ) +const experimentsCorsEnabled = "ws_proxy_cors_enabled" + // Config configures this service. type Config struct { Ingress proxy.HostBasedIngressConfig `json:"ingress"` @@ -64,5 +70,30 @@ func GetConfig(fn string) (*Config, error) { return nil, xerrors.Errorf("config validation error: %w", err) } + timeout := time.Minute * 5 + log.WithField("timeout", timeout).Info("nil CorsEnabled, wait for Feature Flag") + experimentsClient := experiments.NewClient(experiments.WithPollInterval(time.Second * 3)) + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + ffValue := waitStringValue(ctx, experimentsClient, experimentsCorsEnabled, "nope", experiments.Attributes{}) + corsEnabled := ffValue == "true" + log.WithField("ffValue", ffValue).WithField("corsEnabled", corsEnabled).Info("feature flag final value") + return &cfg, nil } + +func waitStringValue(ctx context.Context, client experiments.Client, experimentName, nopeValue string, attributes experiments.Attributes) string { + ticker := time.NewTicker(1 * time.Second) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return nopeValue + case <-ticker.C: + value := client.GetStringValue(ctx, experimentName, nopeValue, attributes) + if value != nopeValue { + return value + } + } + } +} diff --git a/components/ws-proxy/pkg/proxy/config.go b/components/ws-proxy/pkg/proxy/config.go index 7d452b10e89a42..6c468720478a1e 100644 --- a/components/ws-proxy/pkg/proxy/config.go +++ b/components/ws-proxy/pkg/proxy/config.go @@ -29,6 +29,7 @@ type Config struct { BuiltinPages BuiltinPagesConfig `json:"builtinPages"` SSHGatewayCAKeyFile string `json:"sshCAKeyFile"` + CorsEnabled bool } // Validate validates the configuration to catch issues during startup and not at runtime. diff --git a/components/ws-proxy/pkg/proxy/routes.go b/components/ws-proxy/pkg/proxy/routes.go index 32b812612af989..24510f60f44b29 100644 --- a/components/ws-proxy/pkg/proxy/routes.go +++ b/components/ws-proxy/pkg/proxy/routes.go @@ -34,7 +34,6 @@ import ( "github.com/sirupsen/logrus" "golang.org/x/xerrors" - "github.com/gitpod-io/gitpod/common-go/experiments" "github.com/gitpod-io/gitpod/common-go/log" gitpod "github.com/gitpod-io/gitpod/gitpod-protocol" "github.com/gitpod-io/gitpod/ws-manager/api" @@ -48,12 +47,8 @@ type RouteHandlerConfig struct { DefaultTransport http.RoundTripper CorsHandler mux.MiddlewareFunc WorkspaceAuthHandler mux.MiddlewareFunc - - CorsEnabled bool } -const experimentsCorsEnabled = "ws_proxy_cors_enabled" - // RouteHandlerConfigOpt modifies the router handler config. type RouteHandlerConfigOpt func(*Config, *RouteHandlerConfig) @@ -66,23 +61,15 @@ func WithDefaultAuth(infoprov common.WorkspaceInfoProvider) RouteHandlerConfigOp // NewRouteHandlerConfig creates a new instance. func NewRouteHandlerConfig(config *Config, opts ...RouteHandlerConfigOpt) (*RouteHandlerConfig, error) { - corsHandler, err := corsHandler(config.GitpodInstallation.Scheme, config.GitpodInstallation.HostName) + corsHandler, err := corsHandler(config.CorsEnabled, config.GitpodInstallation.Scheme, config.GitpodInstallation.HostName) if err != nil { return nil, err } - experimentsClient := experiments.NewClient(experiments.WithPollInterval(time.Second * 3)) - ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(time.Minute*5)) - defer cancel() - ffValue := waitStringValue(ctx, experimentsClient, experimentsCorsEnabled, "nope", experiments.Attributes{}) - corsEnabled := ffValue == "true" - log.WithField("ffValue", ffValue).WithField("corsEnabled", corsEnabled).Info("feature flag final value") - cfg := &RouteHandlerConfig{ Config: config, DefaultTransport: createDefaultTransport(config.TransportConfig), CorsHandler: corsHandler, WorkspaceAuthHandler: func(h http.Handler) http.Handler { return h }, - CorsEnabled: corsEnabled, } for _, o := range opts { o(config, cfg) @@ -175,22 +162,6 @@ type ideRoutes struct { workspaceMustExistHandler mux.MiddlewareFunc } -func waitStringValue(ctx context.Context, client experiments.Client, experimentName, nopeValue string, attributes experiments.Attributes) string { - ticker := time.NewTicker(1 * time.Second) - defer ticker.Stop() - for { - select { - case <-ctx.Done(): - return nopeValue - case <-ticker.C: - value := client.GetStringValue(ctx, experimentName, nopeValue, attributes) - if value != nopeValue { - return value - } - } - } -} - func (ir *ideRoutes) HandleSSHHostKeyRoute(route *mux.Route, hostKeyList []ssh.Signer) { shk := make([]struct { Type string `json:"type"` @@ -207,9 +178,7 @@ func (ir *ideRoutes) HandleSSHHostKeyRoute(route *mux.Route, hostKeyList []ssh.S } r := route.Subrouter() r.Use(logRouteHandlerHandler("HandleSSHHostKeyRoute")) - if ir.Config.CorsEnabled { - r.Use(ir.Config.CorsHandler) - } + r.Use(ir.Config.CorsHandler) r.NewRoute().HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { rw.Header().Add("Content-Type", "application/json") rw.Write(byt) @@ -219,9 +188,8 @@ func (ir *ideRoutes) HandleSSHHostKeyRoute(route *mux.Route, hostKeyList []ssh.S func (ir *ideRoutes) HandleCreateKeyRoute(route *mux.Route, hostKeyList []ssh.Signer) { r := route.Subrouter() r.Use(logRouteHandlerHandler("HandleCreateKeyRoute")) - if ir.Config.CorsEnabled { - r.Use(ir.Config.CorsHandler) - } + r.Use(ir.Config.CorsHandler) + r.Use(ir.workspaceMustExistHandler) r.Use(ir.Config.WorkspaceAuthHandler) @@ -285,9 +253,7 @@ func extractCloseErrorCode(errStr string) string { func (ir *ideRoutes) HandleSSHOverWebsocketTunnel(route *mux.Route, sshGatewayServer *sshproxy.Server) { r := route.Subrouter() r.Use(logRouteHandlerHandler("HandleSSHOverWebsocketTunnel")) - if ir.Config.CorsEnabled { - r.Use(ir.Config.CorsHandler) - } + r.Use(ir.Config.CorsHandler) r.Use(ir.workspaceMustExistHandler) r.Use(ir.Config.WorkspaceAuthHandler) @@ -331,9 +297,7 @@ func (ir *ideRoutes) HandleSSHOverWebsocketTunnel(route *mux.Route, sshGatewaySe func (ir *ideRoutes) HandleDirectSupervisorRoute(route *mux.Route, authenticated bool) { r := route.Subrouter() r.Use(logRouteHandlerHandler(fmt.Sprintf("HandleDirectSupervisorRoute (authenticated: %v)", authenticated))) - if ir.Config.CorsEnabled { - r.Use(ir.Config.CorsHandler) - } + r.Use(ir.Config.CorsHandler) r.Use(ir.workspaceMustExistHandler) if authenticated { r.Use(ir.Config.WorkspaceAuthHandler) @@ -418,9 +382,7 @@ type BlobserveInlineVars struct { func (ir *ideRoutes) HandleRoot(route *mux.Route) { r := route.Subrouter() r.Use(logRouteHandlerHandler("handleRoot")) - if ir.Config.CorsEnabled { - r.Use(ir.Config.CorsHandler) - } + r.Use(ir.Config.CorsHandler) r.Use(ir.workspaceMustExistHandler) proxyPassWoSensitiveCookies := sensitiveCookieHandler(ir.Config.Config.GitpodInstallation.HostName)(proxyPass(ir.Config, ir.InfoProvider, workspacePodResolver)) @@ -554,9 +516,7 @@ func installDebugWorkspaceRoutes(r *mux.Router, config *RouteHandlerConfig, info } r.Use(logHandler) - if config.CorsEnabled { - r.Use(config.CorsHandler) - } + r.Use(config.CorsHandler) r.Use(config.WorkspaceAuthHandler) // filter all session cookies r.Use(sensitiveCookieHandler(config.Config.GitpodInstallation.HostName)) @@ -695,7 +655,13 @@ func buildWorkspacePodURL(protocol api.PortProtocol, ipAddress string, port stri } // corsHandler produces the CORS handler for workspaces. -func corsHandler(scheme, hostname string) (mux.MiddlewareFunc, error) { +func corsHandler(enabled bool, scheme, hostname string) (mux.MiddlewareFunc, error) { + if !enabled { + // empty handler + return func(h http.Handler) http.Handler { + return h + }, nil + } origin := fmt.Sprintf("%s://%s", scheme, hostname) domainRegex := strings.ReplaceAll(hostname, ".", "\\.") diff --git a/components/ws-proxy/pkg/proxy/routes_test.go b/components/ws-proxy/pkg/proxy/routes_test.go index 0077897822b1a3..1f808c4ff57ec4 100644 --- a/components/ws-proxy/pkg/proxy/routes_test.go +++ b/components/ws-proxy/pkg/proxy/routes_test.go @@ -95,6 +95,7 @@ var ( BuiltinPages: BuiltinPagesConfig{ Location: "../../public", }, + CorsEnabled: false, } ) From 026bdb9c4e168204614a5e76803b30abf91d8bea Mon Sep 17 00:00:00 2001 From: Huiwen Date: Wed, 28 Aug 2024 16:43:55 +0000 Subject: [PATCH 6/8] fixup --- components/ws-proxy/pkg/config/config.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/components/ws-proxy/pkg/config/config.go b/components/ws-proxy/pkg/config/config.go index 0ac273789e3b67..f5a127d9cb9962 100644 --- a/components/ws-proxy/pkg/config/config.go +++ b/components/ws-proxy/pkg/config/config.go @@ -71,18 +71,19 @@ func GetConfig(fn string) (*Config, error) { } timeout := time.Minute * 5 - log.WithField("timeout", timeout).Info("nil CorsEnabled, wait for Feature Flag") + log.WithField("timeout", timeout).Info("waiting for Feature Flag") experimentsClient := experiments.NewClient(experiments.WithPollInterval(time.Second * 3)) ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() - ffValue := waitStringValue(ctx, experimentsClient, experimentsCorsEnabled, "nope", experiments.Attributes{}) + ffValue := waitExperimentsStringValue(ctx, experimentsClient, experimentsCorsEnabled, "nope", experiments.Attributes{}) corsEnabled := ffValue == "true" - log.WithField("ffValue", ffValue).WithField("corsEnabled", corsEnabled).Info("feature flag final value") + cfg.Proxy.CorsEnabled = corsEnabled + log.WithField("ffValue", ffValue).WithField("corsEnabled", cfg.Proxy.CorsEnabled).Info("feature flag final value") return &cfg, nil } -func waitStringValue(ctx context.Context, client experiments.Client, experimentName, nopeValue string, attributes experiments.Attributes) string { +func waitExperimentsStringValue(ctx context.Context, client experiments.Client, experimentName, nopeValue string, attributes experiments.Attributes) string { ticker := time.NewTicker(1 * time.Second) defer ticker.Stop() for { From 9304761e46a5c6e760763a33f4a928e6222a3b83 Mon Sep 17 00:00:00 2001 From: Huiwen Date: Thu, 29 Aug 2024 06:32:56 +0000 Subject: [PATCH 7/8] fix network --- install/installer/pkg/components/proxy/networkpolicy.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/install/installer/pkg/components/proxy/networkpolicy.go b/install/installer/pkg/components/proxy/networkpolicy.go index 71e8225901e7ef..f9f9d1c61fe3f8 100644 --- a/install/installer/pkg/components/proxy/networkpolicy.go +++ b/install/installer/pkg/components/proxy/networkpolicy.go @@ -104,6 +104,10 @@ func networkpolicy(ctx *common.RenderContext) ([]runtime.Object, error) { PodSelector: &metav1.LabelSelector{MatchLabels: map[string]string{ "component": common.DashboardComponent, }}, + }, { + PodSelector: &metav1.LabelSelector{MatchLabels: map[string]string{ + "component": common.WSProxyComponent, + }}, }}, }}, }, From 32eb9516dbdd4ee280a70c61f6f54dd0d006996b Mon Sep 17 00:00:00 2001 From: Huiwen Date: Thu, 29 Aug 2024 06:46:58 +0000 Subject: [PATCH 8/8] fixup --- install/installer/pkg/common/common.go | 25 +++++++++++++++++++ install/installer/pkg/common/common_test.go | 18 +++++++++++++ .../pkg/components/proxy/networkpolicy.go | 4 --- .../pkg/components/ws-proxy/deployment.go | 3 ++- 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/install/installer/pkg/common/common.go b/install/installer/pkg/common/common.go index b45c128bb8dd6a..b34f362014c745 100644 --- a/install/installer/pkg/common/common.go +++ b/install/installer/pkg/common/common.go @@ -409,6 +409,31 @@ func ConfigcatEnv(ctx *RenderContext) []corev1.EnvVar { } } +func ConfigcatEnvOutOfCluster(ctx *RenderContext) []corev1.EnvVar { + var sdkKey string + _ = ctx.WithExperimental(func(cfg *experimental.Config) error { + if cfg.WebApp != nil && cfg.WebApp.ConfigcatKey != "" { + sdkKey = cfg.WebApp.ConfigcatKey + } + return nil + }) + + if sdkKey == "" { + return nil + } + + return []corev1.EnvVar{ + { + Name: "CONFIGCAT_SDK_KEY", + Value: "gitpod", + }, + { + Name: "CONFIGCAT_BASE_URL", + Value: fmt.Sprintf("https://%s/configcat", ctx.Config.Domain), + }, + } +} + func ConfigcatProxyEnv(ctx *RenderContext) []corev1.EnvVar { var ( sdkKey string diff --git a/install/installer/pkg/common/common_test.go b/install/installer/pkg/common/common_test.go index 8309a193590a62..245258da1618f5 100644 --- a/install/installer/pkg/common/common_test.go +++ b/install/installer/pkg/common/common_test.go @@ -11,9 +11,11 @@ import ( "github.com/gitpod-io/gitpod/common-go/baseserver" "github.com/gitpod-io/gitpod/installer/pkg/common" config "github.com/gitpod-io/gitpod/installer/pkg/config/v1" + "github.com/gitpod-io/gitpod/installer/pkg/config/v1/experimental" "github.com/gitpod-io/gitpod/installer/pkg/config/versions" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" ) func TestKubeRBACProxyContainer_DefaultPorts(t *testing.T) { @@ -73,3 +75,19 @@ func TestServerComponentWaiterContainer(t *testing.T) { require.Equal(t, labels, "app=gitpod,component=server") require.Equal(t, []string{"-v", "component", "--namespace", "test_namespace", "--component", common.ServerComponent, "--labels", labels, "--image", ctx.Config.Repository + "/server:" + "happy_path_server_image"}, container.Args) } + +func TestConfigcatEnvOutOfCluster(t *testing.T) { + ctx, err := common.NewRenderContext(config.Config{ + Domain: "gitpod.io", + Experimental: &experimental.Config{ + WebApp: &experimental.WebAppConfig{ + ConfigcatKey: "foo", + }, + }, + }, versions.Manifest{}, "test_namespace") + require.NoError(t, err) + + envVars := common.ConfigcatEnvOutOfCluster(ctx) + require.Equal(t, len(envVars), 2) + require.Equal(t, envVars, []v1.EnvVar([]v1.EnvVar{{Name: "CONFIGCAT_SDK_KEY", Value: "gitpod"}, {Name: "CONFIGCAT_BASE_URL", Value: "https://gitpod.io/configcat"}})) +} diff --git a/install/installer/pkg/components/proxy/networkpolicy.go b/install/installer/pkg/components/proxy/networkpolicy.go index f9f9d1c61fe3f8..71e8225901e7ef 100644 --- a/install/installer/pkg/components/proxy/networkpolicy.go +++ b/install/installer/pkg/components/proxy/networkpolicy.go @@ -104,10 +104,6 @@ func networkpolicy(ctx *common.RenderContext) ([]runtime.Object, error) { PodSelector: &metav1.LabelSelector{MatchLabels: map[string]string{ "component": common.DashboardComponent, }}, - }, { - PodSelector: &metav1.LabelSelector{MatchLabels: map[string]string{ - "component": common.WSProxyComponent, - }}, }}, }}, }, diff --git a/install/installer/pkg/components/ws-proxy/deployment.go b/install/installer/pkg/components/ws-proxy/deployment.go index d1280b4029d602..46244ee02efe0c 100644 --- a/install/installer/pkg/components/ws-proxy/deployment.go +++ b/install/installer/pkg/components/ws-proxy/deployment.go @@ -142,7 +142,8 @@ func deployment(ctx *common.RenderContext) ([]runtime.Object, error) { common.DefaultEnv(&ctx.Config), common.WorkspaceTracingEnv(ctx, Component), common.AnalyticsEnv(&ctx.Config), - common.ConfigcatEnv(ctx), + // ws-proxy and proxy may not in the same cluster + common.ConfigcatEnvOutOfCluster(ctx), )), ReadinessProbe: &corev1.Probe{ InitialDelaySeconds: int32(2),