From 88f32aeaa15dfe3f6596518e0d9970dbff4eff33 Mon Sep 17 00:00:00 2001 From: Fabian Stelzer Date: Fri, 17 Sep 2021 11:08:18 +0000 Subject: [PATCH] rename Upstreams to UpstreamConfig and its Configs member to Upstreams then --- docs/docs/configuration/alpha_config.md | 10 +++--- main_test.go | 8 ++--- oauthproxy_test.go | 34 +++++++++--------- pkg/apis/options/alpha_options.go | 9 +++-- pkg/apis/options/legacy_options.go | 8 ++--- pkg/apis/options/legacy_options_test.go | 6 ++-- pkg/apis/options/load_test.go | 8 ++--- pkg/apis/options/options.go | 2 +- pkg/apis/options/upstreams.go | 8 ++--- pkg/upstream/proxy.go | 4 +-- pkg/upstream/proxy_test.go | 8 ++--- pkg/validation/options_test.go | 2 +- pkg/validation/upstreams.go | 4 +-- pkg/validation/upstreams_test.go | 48 ++++++++++++------------- 14 files changed, 79 insertions(+), 80 deletions(-) diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md index 3cc54037..f13ea131 100644 --- a/docs/docs/configuration/alpha_config.md +++ b/docs/docs/configuration/alpha_config.md @@ -124,7 +124,7 @@ They may change between releases without notice. | Field | Type | Description | | ----- | ---- | ----------- | -| `upstreams` | _[Upstreams](#upstreams)_ | Upstreams is used to configure upstream servers.
Once a user is authenticated, requests to the server will be proxied to
these upstream servers based on the path mappings defined in this list. | +| `upstreamConfig` | _[UpstreamConfig](#upstreamconfig)_ | UpstreamConfig is used to configure upstream servers.
Once a user is authenticated, requests to the server will be proxied to
these upstream servers based on the path mappings defined in this list. | | `injectRequestHeaders` | _[[]Header](#header)_ | InjectRequestHeaders is used to configure headers that should be added
to requests to upstream servers.
Headers may source values from either the authenticated user's session
or from a static secret value. | | `injectResponseHeaders` | _[[]Header](#header)_ | InjectResponseHeaders is used to configure headers that should be added
to responses from the proxy.
This is typically used when using the proxy as an external authentication
provider in conjunction with another proxy such as NGINX and its
auth_request module.
Headers may source values from either the authenticated user's session
or from a static secret value. | | `server` | _[Server](#server)_ | Server is used to configure the HTTP(S) server for the proxy application.
You may choose to run both HTTP and HTTPS servers simultaneously.
This can be done by setting the BindAddress and the SecureBindAddress simultaneously.
To use the secure server you must configure a TLS certificate and key. | @@ -364,7 +364,7 @@ TLS contains the information for loading a TLS certifcate and key. ### Upstream -(**Appears on:** [Upstreams](#upstreams)) +(**Appears on:** [UpstreamConfig](#upstreamconfig)) Upstream represents the configuration for an upstream server. Requests will be proxied to this upstream if the path matches the request path. @@ -382,13 +382,13 @@ Requests will be proxied to this upstream if the path matches the request path. | `passHostHeader` | _bool_ | PassHostHeader determines whether the request host header should be proxied
to the upstream server.
Defaults to true. | | `proxyWebSockets` | _bool_ | ProxyWebSockets enables proxying of websockets to upstream servers
Defaults to true. | -### Upstreams +### UpstreamConfig (**Appears on:** [AlphaOptions](#alphaoptions)) -Upstreams is a collection of definitions for upstream servers. +UpstreamConfig is a collection of definitions for upstream servers. | Field | Type | Description | | ----- | ---- | ----------- | | `proxyRawPath` | _bool_ | ProxyRawPath will pass the raw url path to upstream allowing for url's
like: "/%2F/" which would otherwise be redirected to "/" | -| `configs` | _[[]Upstream](#upstream)_ | Upstream represents the configuration for an upstream server.
Requests will be proxied to this upstream if the path matches the request path. | +| `upstreams` | _[[]Upstream](#upstream)_ | Upstreams represents the configuration for the upstream servers.
Requests will be proxied to this upstream if the path matches the request path. | diff --git a/main_test.go b/main_test.go index dc7dcb82..4fa9efbf 100644 --- a/main_test.go +++ b/main_test.go @@ -26,9 +26,9 @@ client_secret="b2F1dGgyLXByb3h5LWNsaWVudC1zZWNyZXQK" ` const testAlphaConfig = ` -upstreams: +upstreamConfig: proxyrawpath: false - configs: + upstreams: - id: / path: / uri: http://httpbin @@ -104,8 +104,8 @@ redirect_url="http://localhost:4180/oauth2/callback" opts.Cookie.Secure = false opts.RawRedirectURL = "http://localhost:4180/oauth2/callback" - opts.UpstreamServers = options.Upstreams{ - Configs: []options.Upstream{ + opts.UpstreamServers = options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: "/", Path: "/", diff --git a/oauthproxy_test.go b/oauthproxy_test.go index d0eaaa4d..9add52ed 100644 --- a/oauthproxy_test.go +++ b/oauthproxy_test.go @@ -197,8 +197,8 @@ func TestBasicAuthPassword(t *testing.T) { basicAuthPassword := "This is a secure password" opts := baseTestOptions() - opts.UpstreamServers = options.Upstreams{ - Configs: []options.Upstream{ + opts.UpstreamServers = options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: providerServer.URL, Path: "/", @@ -348,8 +348,8 @@ func NewPassAccessTokenTest(opts PassAccessTokenTestOptions) (*PassAccessTokenTe })) patt.opts = baseTestOptions() - patt.opts.UpstreamServers = options.Upstreams{ - Configs: []options.Upstream{ + patt.opts.UpstreamServers = options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: patt.providerServer.URL, Path: "/", @@ -358,7 +358,7 @@ func NewPassAccessTokenTest(opts PassAccessTokenTestOptions) (*PassAccessTokenTe }, } if opts.ProxyUpstream.ID != "" { - patt.opts.UpstreamServers.Configs = append(patt.opts.UpstreamServers.Configs, opts.ProxyUpstream) + patt.opts.UpstreamServers.Upstreams = append(patt.opts.UpstreamServers.Upstreams, opts.ProxyUpstream) } patt.opts.Cookie.Secure = false @@ -1273,8 +1273,8 @@ func TestAuthSkippedForPreflightRequests(t *testing.T) { t.Cleanup(upstreamServer.Close) opts := baseTestOptions() - opts.UpstreamServers = options.Upstreams{ - Configs: []options.Upstream{ + opts.UpstreamServers = options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: upstreamServer.URL, Path: "/", @@ -1350,8 +1350,8 @@ func NewSignatureTest() (*SignatureTest, error) { if err != nil { return nil, err } - opts.UpstreamServers = options.Upstreams{ - Configs: []options.Upstream{ + opts.UpstreamServers = options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: upstreamServer.URL, Path: "/", @@ -1788,8 +1788,8 @@ func Test_noCacheHeaders(t *testing.T) { t.Cleanup(upstreamServer.Close) opts := baseTestOptions() - opts.UpstreamServers = options.Upstreams{ - Configs: []options.Upstream{ + opts.UpstreamServers = options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: upstreamServer.URL, Path: "/", @@ -2060,8 +2060,8 @@ func TestTrustedIPs(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { opts := baseTestOptions() - opts.UpstreamServers = options.Upstreams{ - Configs: []options.Upstream{ + opts.UpstreamServers = options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: "static", Path: "/", @@ -2255,8 +2255,8 @@ func TestAllowedRequest(t *testing.T) { t.Cleanup(upstreamServer.Close) opts := baseTestOptions() - opts.UpstreamServers = options.Upstreams{ - Configs: []options.Upstream{ + opts.UpstreamServers = options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: upstreamServer.URL, Path: "/", @@ -2372,8 +2372,8 @@ func TestProxyAllowedGroups(t *testing.T) { test, err := NewProcessCookieTestWithOptionsModifiers(func(opts *options.Options) { opts.Providers[0].AllowedGroups = tt.allowedGroups - opts.UpstreamServers = options.Upstreams{ - Configs: []options.Upstream{ + opts.UpstreamServers = options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: upstreamServer.URL, Path: "/", diff --git a/pkg/apis/options/alpha_options.go b/pkg/apis/options/alpha_options.go index ecfd81c8..04769d7f 100644 --- a/pkg/apis/options/alpha_options.go +++ b/pkg/apis/options/alpha_options.go @@ -9,10 +9,10 @@ package options // They may change between releases without notice. // ::: type AlphaOptions struct { - // Upstreams is used to configure upstream servers. + // UpstreamConfig is used to configure upstream servers. // Once a user is authenticated, requests to the server will be proxied to // these upstream servers based on the path mappings defined in this list. - Upstreams Upstreams `json:"upstreams,omitempty"` + UpstreamConfig UpstreamConfig `json:"upstreamConfig,omitempty"` // InjectRequestHeaders is used to configure headers that should be added // to requests to upstream servers. @@ -48,19 +48,18 @@ type AlphaOptions struct { // MergeInto replaces alpha options in the Options struct with the values // from the AlphaOptions func (a *AlphaOptions) MergeInto(opts *Options) { - opts.UpstreamServers = a.Upstreams + opts.UpstreamServers = a.UpstreamConfig opts.InjectRequestHeaders = a.InjectRequestHeaders opts.InjectResponseHeaders = a.InjectResponseHeaders opts.Server = a.Server opts.MetricsServer = a.MetricsServer opts.Providers = a.Providers - } // ExtractFrom populates the fields in the AlphaOptions with the values from // the Options func (a *AlphaOptions) ExtractFrom(opts *Options) { - a.Upstreams = opts.UpstreamServers + a.UpstreamConfig = opts.UpstreamServers a.InjectRequestHeaders = opts.InjectRequestHeaders a.InjectResponseHeaders = opts.InjectResponseHeaders a.Server = opts.Server diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go index fb51feed..3441fd41 100644 --- a/pkg/apis/options/legacy_options.go +++ b/pkg/apis/options/legacy_options.go @@ -114,13 +114,13 @@ func legacyUpstreamsFlagSet() *pflag.FlagSet { return flagSet } -func (l *LegacyUpstreams) convert() (Upstreams, error) { - upstreams := Upstreams{} +func (l *LegacyUpstreams) convert() (UpstreamConfig, error) { + upstreams := UpstreamConfig{} for _, upstreamString := range l.Upstreams { u, err := url.Parse(upstreamString) if err != nil { - return Upstreams{}, fmt.Errorf("could not parse upstream %q: %v", upstreamString, err) + return UpstreamConfig{}, fmt.Errorf("could not parse upstream %q: %v", upstreamString, err) } if u.Path == "" { @@ -169,7 +169,7 @@ func (l *LegacyUpstreams) convert() (Upstreams, error) { upstream.FlushInterval = nil } - upstreams.Configs = append(upstreams.Configs, upstream) + upstreams.Upstreams = append(upstreams.Upstreams, upstream) } return upstreams, nil diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go index 88c32817..ecf48494 100644 --- a/pkg/apis/options/legacy_options_test.go +++ b/pkg/apis/options/legacy_options_test.go @@ -26,8 +26,8 @@ var _ = Describe("Legacy Options", func() { truth := true staticCode := 204 - opts.UpstreamServers = Upstreams{ - Configs: []Upstream{ + opts.UpstreamServers = UpstreamConfig{ + Upstreams: []Upstream{ { ID: "/baz", Path: "/baz", @@ -221,7 +221,7 @@ var _ = Describe("Legacy Options", func() { Expect(err).ToNot(HaveOccurred()) } - Expect(upstreams.Configs).To(ConsistOf(in.expectedUpstreams)) + Expect(upstreams.Upstreams).To(ConsistOf(in.expectedUpstreams)) }, Entry("with no upstreams", &convertUpstreamsTableInput{ upstreamStrings: []string{}, diff --git a/pkg/apis/options/load_test.go b/pkg/apis/options/load_test.go index ca4918b2..335facf4 100644 --- a/pkg/apis/options/load_test.go +++ b/pkg/apis/options/load_test.go @@ -469,8 +469,8 @@ sub: It("should load a full example AlphaOptions", func() { config := []byte(` -upstreams: - configs: +upstreamConfig: + upstreams: - id: httpbin path: / uri: http://httpbin @@ -503,8 +503,8 @@ injectResponseHeaders: flushInterval := Duration(500 * time.Millisecond) Expect(into).To(Equal(&AlphaOptions{ - Upstreams: Upstreams{ - Configs: []Upstream{ + UpstreamConfig: UpstreamConfig{ + Upstreams: []Upstream{ { ID: "httpbin", Path: "/", diff --git a/pkg/apis/options/options.go b/pkg/apis/options/options.go index 0fd2ece1..d4bb4312 100644 --- a/pkg/apis/options/options.go +++ b/pkg/apis/options/options.go @@ -41,7 +41,7 @@ type Options struct { // Not used in the legacy config, name not allowed to match an external key (upstreams) // TODO(JoelSpeed): Rename when legacy config is removed - UpstreamServers Upstreams `cfg:",internal"` + UpstreamServers UpstreamConfig `cfg:",internal"` InjectRequestHeaders []Header `cfg:",internal"` InjectResponseHeaders []Header `cfg:",internal"` diff --git a/pkg/apis/options/upstreams.go b/pkg/apis/options/upstreams.go index 4e1a0547..9d735f13 100644 --- a/pkg/apis/options/upstreams.go +++ b/pkg/apis/options/upstreams.go @@ -7,15 +7,15 @@ const ( DefaultUpstreamFlushInterval = 1 * time.Second ) -// Upstreams is a collection of definitions for upstream servers. -type Upstreams struct { +// UpstreamConfig is a collection of definitions for upstream servers. +type UpstreamConfig struct { // ProxyRawPath will pass the raw url path to upstream allowing for url's // like: "/%2F/" which would otherwise be redirected to "/" ProxyRawPath bool `json:"proxyRawPath,omitempty"` - // Upstream represents the configuration for an upstream server. + // Upstreams represents the configuration for the upstream servers. // Requests will be proxied to this upstream if the path matches the request path. - Configs []Upstream `json:"configs,omitempty"` + Upstreams []Upstream `json:"upstreams,omitempty"` } // Upstream represents the configuration for an upstream server. diff --git a/pkg/upstream/proxy.go b/pkg/upstream/proxy.go index 27bd5229..eabb47ff 100644 --- a/pkg/upstream/proxy.go +++ b/pkg/upstream/proxy.go @@ -22,7 +22,7 @@ type ProxyErrorHandler func(http.ResponseWriter, *http.Request, error) // NewProxy creates a new multiUpstreamProxy that can serve requests directed to // multiple upstreams. -func NewProxy(upstreams options.Upstreams, sigData *options.SignatureData, writer pagewriter.Writer) (http.Handler, error) { +func NewProxy(upstreams options.UpstreamConfig, sigData *options.SignatureData, writer pagewriter.Writer) (http.Handler, error) { m := &multiUpstreamProxy{ serveMux: mux.NewRouter(), } @@ -31,7 +31,7 @@ func NewProxy(upstreams options.Upstreams, sigData *options.SignatureData, write m.serveMux.UseEncodedPath() } - for _, upstream := range sortByPathLongest(upstreams.Configs) { + for _, upstream := range sortByPathLongest(upstreams.Upstreams) { if upstream.Static { if err := m.registerStaticResponseHandler(upstream, writer); err != nil { return nil, fmt.Errorf("could not register static upstream %q: %v", upstream.ID, err) diff --git a/pkg/upstream/proxy_test.go b/pkg/upstream/proxy_test.go index 2745e680..87c27e4b 100644 --- a/pkg/upstream/proxy_test.go +++ b/pkg/upstream/proxy_test.go @@ -20,7 +20,7 @@ var _ = Describe("Proxy Suite", func() { target string response testHTTPResponse upstream string - upstreams options.Upstreams + upstreams options.UpstreamConfig } Context("multiUpstreamProxy", func() { @@ -40,8 +40,8 @@ var _ = Describe("Proxy Suite", func() { // Allows for specifying settings and even individual upstreams for specific tests and uses the default upstreams/configs otherwise upstreams := in.upstreams - if len(in.upstreams.Configs) == 0 { - upstreams.Configs = []options.Upstream{ + if len(in.upstreams.Upstreams) == 0 { + upstreams.Upstreams = []options.Upstream{ { ID: "http-backend", Path: "/http/", @@ -325,7 +325,7 @@ var _ = Describe("Proxy Suite", func() { upstream: "", }), Entry("containing an escaped '/' with ProxyRawPath", &proxyTableInput{ - upstreams: options.Upstreams{ProxyRawPath: true}, + upstreams: options.UpstreamConfig{ProxyRawPath: true}, target: "http://example.localhost/%2F/test1/%2F/test2", response: testHTTPResponse{ code: 404, diff --git a/pkg/validation/options_test.go b/pkg/validation/options_test.go index 70387a14..03afb22a 100644 --- a/pkg/validation/options_test.go +++ b/pkg/validation/options_test.go @@ -22,7 +22,7 @@ const ( func testOptions() *options.Options { o := options.NewOptions() - o.UpstreamServers.Configs = append(o.UpstreamServers.Configs, options.Upstream{ + o.UpstreamServers.Upstreams = append(o.UpstreamServers.Upstreams, options.Upstream{ ID: "upstream", Path: "/", URI: "http://127.0.0.1:8080/", diff --git a/pkg/validation/upstreams.go b/pkg/validation/upstreams.go index f45f3b94..ba1e216a 100644 --- a/pkg/validation/upstreams.go +++ b/pkg/validation/upstreams.go @@ -7,12 +7,12 @@ import ( "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" ) -func validateUpstreams(upstreams options.Upstreams) []string { +func validateUpstreams(upstreams options.UpstreamConfig) []string { msgs := []string{} ids := make(map[string]struct{}) paths := make(map[string]struct{}) - for _, upstream := range upstreams.Configs { + for _, upstream := range upstreams.Upstreams { msgs = append(msgs, validateUpstream(upstream, ids, paths)...) } diff --git a/pkg/validation/upstreams_test.go b/pkg/validation/upstreams_test.go index 1c974024..586c6d85 100644 --- a/pkg/validation/upstreams_test.go +++ b/pkg/validation/upstreams_test.go @@ -11,7 +11,7 @@ import ( var _ = Describe("Upstreams", func() { type validateUpstreamTableInput struct { - upstreams options.Upstreams + upstreams options.UpstreamConfig errStrings []string } @@ -54,12 +54,12 @@ var _ = Describe("Upstreams", func() { Expect(validateUpstreams(o.upstreams)).To(ConsistOf(o.errStrings)) }, Entry("with no upstreams", &validateUpstreamTableInput{ - upstreams: options.Upstreams{}, + upstreams: options.UpstreamConfig{}, errStrings: []string{}, }), Entry("with valid upstreams", &validateUpstreamTableInput{ - upstreams: options.Upstreams{ - Configs: []options.Upstream{ + upstreams: options.UpstreamConfig{ + Upstreams: []options.Upstream{ validHTTPUpstream, validStaticUpstream, validFileUpstream, @@ -68,8 +68,8 @@ var _ = Describe("Upstreams", func() { errStrings: []string{}, }), Entry("with an empty ID", &validateUpstreamTableInput{ - upstreams: options.Upstreams{ - Configs: []options.Upstream{ + upstreams: options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: "", Path: "/foo", @@ -80,8 +80,8 @@ var _ = Describe("Upstreams", func() { errStrings: []string{emptyIDMsg}, }), Entry("with an empty Path", &validateUpstreamTableInput{ - upstreams: options.Upstreams{ - Configs: []options.Upstream{ + upstreams: options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: "foo", Path: "", @@ -92,8 +92,8 @@ var _ = Describe("Upstreams", func() { errStrings: []string{emptyPathMsg}, }), Entry("with an empty Path", &validateUpstreamTableInput{ - upstreams: options.Upstreams{ - Configs: []options.Upstream{ + upstreams: options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: "foo", Path: "", @@ -104,8 +104,8 @@ var _ = Describe("Upstreams", func() { errStrings: []string{emptyPathMsg}, }), Entry("with an empty URI", &validateUpstreamTableInput{ - upstreams: options.Upstreams{ - Configs: []options.Upstream{ + upstreams: options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: "foo", Path: "/foo", @@ -116,8 +116,8 @@ var _ = Describe("Upstreams", func() { errStrings: []string{emptyURIMsg}, }), Entry("with an invalid URI", &validateUpstreamTableInput{ - upstreams: options.Upstreams{ - Configs: []options.Upstream{ + upstreams: options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: "foo", Path: "/foo", @@ -128,8 +128,8 @@ var _ = Describe("Upstreams", func() { errStrings: []string{invalidURIMsg}, }), Entry("with an invalid URI scheme", &validateUpstreamTableInput{ - upstreams: options.Upstreams{ - Configs: []options.Upstream{ + upstreams: options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: "foo", Path: "/foo", @@ -140,8 +140,8 @@ var _ = Describe("Upstreams", func() { errStrings: []string{invalidURISchemeMsg}, }), Entry("with a static upstream and invalid optons", &validateUpstreamTableInput{ - upstreams: options.Upstreams{ - Configs: []options.Upstream{ + upstreams: options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: "foo", Path: "/foo", @@ -163,8 +163,8 @@ var _ = Describe("Upstreams", func() { }, }), Entry("with duplicate IDs", &validateUpstreamTableInput{ - upstreams: options.Upstreams{ - Configs: []options.Upstream{ + upstreams: options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: "foo", Path: "/foo1", @@ -180,8 +180,8 @@ var _ = Describe("Upstreams", func() { errStrings: []string{multipleIDsMsg}, }), Entry("with duplicate Paths", &validateUpstreamTableInput{ - upstreams: options.Upstreams{ - Configs: []options.Upstream{ + upstreams: options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: "foo1", Path: "/foo", @@ -197,8 +197,8 @@ var _ = Describe("Upstreams", func() { errStrings: []string{multiplePathsMsg}, }), Entry("when a static code is supplied without static", &validateUpstreamTableInput{ - upstreams: options.Upstreams{ - Configs: []options.Upstream{ + upstreams: options.UpstreamConfig{ + Upstreams: []options.Upstream{ { ID: "foo", Path: "/foo",