Add LegacyOptions and conversion to new Options

This will be temporary until we switch to structured config, then we can remove the LegacyOptions and conversions

pkg/apis/options/legacy_options.go

@@ -0,0 +1,98 @@
+package options
+
+import (
+	"fmt"
+	"net/url"
+	"strconv"
+	"time"
+
+	"github.com/oauth2-proxy/oauth2-proxy/pkg/logger"
+)
+
+type LegacyOptions struct {
+	// Legacy options related to upstream servers
+	LegacyFlushInterval                 time.Duration `flag:"flush-interval" cfg:"flush_interval"`
+	LegacyPassHostHeader                bool          `flag:"pass-host-header" cfg:"pass_host_header"`
+	LegacyProxyWebSockets               bool          `flag:"proxy-websockets" cfg:"proxy_websockets"`
+	LegacySSLUpstreamInsecureSkipVerify bool          `flag:"ssl-upstream-insecure-skip-verify" cfg:"ssl_upstream_insecure_skip_verify"`
+	LegacyUpstreams                     []string      `flag:"upstream" cfg:"upstreams"`
+
+	Options Options `cfg:",squash"`
+}
+
+func NewLegacyOptions() *LegacyOptions {
+	return &LegacyOptions{
+		LegacyPassHostHeader:  true,
+		LegacyProxyWebSockets: true,
+		LegacyFlushInterval:   time.Duration(1) * time.Second,
+
+		Options: *NewOptions(),
+	}
+}
+
+func (l *LegacyOptions) ToOptions() (*Options, error) {
+	upstreams, err := convertLegacyUpstreams(l.LegacyUpstreams, l.LegacySSLUpstreamInsecureSkipVerify, l.LegacyPassHostHeader, l.LegacyProxyWebSockets, l.LegacyFlushInterval)
+	if err != nil {
+		return nil, fmt.Errorf("error converting upstreams: %v", err)
+	}
+	l.Options.UpstreamServers = upstreams
+
+	return &l.Options, nil
+}
+
+func convertLegacyUpstreams(upstreamStrings []string, skipVerify, passHostHeader, proxyWebSockets bool, flushInterval time.Duration) (Upstreams, error) {
+	upstreams := Upstreams{}
+
+	for _, upstreamString := range upstreamStrings {
+		u, err := url.Parse(upstreamString)
+		if err != nil {
+			return nil, fmt.Errorf("could not parse upstream %q: %v", upstreamString, err)
+		}
+
+		if u.Path == "" {
+			u.Path = "/"
+		}
+
+		upstream := Upstream{
+			ID:                    u.Path,
+			Path:                  u.Path,
+			URI:                   upstreamString,
+			InsecureSkipTLSVerify: skipVerify,
+			PassHostHeader:        passHostHeader,
+			ProxyWebSockets:       proxyWebSockets,
+			FlushInterval:         &flushInterval,
+		}
+
+		switch u.Scheme {
+		case "file":
+			if u.Fragment != "" {
+				upstream.ID = u.Fragment
+				upstream.Path = u.Fragment
+			}
+		case "static":
+			responseCode, err := strconv.Atoi(u.Host)
+			if err != nil {
+				logger.Printf("unable to convert %q to int, use default \"200\"", u.Host)
+				responseCode = 200
+			}
+			upstream.Static = true
+			upstream.StaticCode = &responseCode
+
+			// These are not allowed to be empty and must be unique
+			upstream.ID = upstreamString
+			upstream.Path = upstreamString
+
+			// Force defaults compatible with static responses
+			upstream.URI = ""
+			upstream.InsecureSkipTLSVerify = false
+			upstream.PassHostHeader = true
+			upstream.ProxyWebSockets = false
+			flush := 1 * time.Second
+			upstream.FlushInterval = &flush
+		}
+
+		upstreams = append(upstreams, upstream)
+	}
+
+	return upstreams, nil
+}

pkg/apis/options/legacy_options_test.go

@@ -0,0 +1,194 @@
+package options
+
+import (
+	"time"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Legacy Options", func() {
+	Context("ToOptions", func() {
+		It("converts the options as expected", func() {
+			opts := NewOptions()
+
+			legacyOpts := NewLegacyOptions()
+
+			// Set upstreams and related options to test their conversion
+			flushInterval := 5 * time.Second
+			legacyOpts.LegacyFlushInterval = flushInterval
+			legacyOpts.LegacyPassHostHeader = true
+			legacyOpts.LegacyProxyWebSockets = true
+			legacyOpts.LegacySSLUpstreamInsecureSkipVerify = true
+			legacyOpts.LegacyUpstreams = []string{"http://foo.bar/baz", "file://var/lib/website#/bar"}
+
+			opts.UpstreamServers = Upstreams{
+				{
+					ID:                    "/baz",
+					Path:                  "/baz",
+					URI:                   "http://foo.bar/baz",
+					FlushInterval:         &flushInterval,
+					InsecureSkipTLSVerify: true,
+					PassHostHeader:        true,
+					ProxyWebSockets:       true,
+				},
+				{
+					ID:                    "/bar",
+					Path:                  "/bar",
+					URI:                   "file://var/lib/website#/bar",
+					FlushInterval:         &flushInterval,
+					InsecureSkipTLSVerify: true,
+					PassHostHeader:        true,
+					ProxyWebSockets:       true,
+				},
+			}
+
+			converted, err := legacyOpts.ToOptions()
+			Expect(err).ToNot(HaveOccurred())
+			Expect(converted).To(Equal(opts))
+		})
+	})
+
+	Context("Legacy Upstreams", func() {
+		type convertUpstreamsTableInput struct {
+			upstreamStrings   []string
+			expectedUpstreams Upstreams
+			errMsg            string
+		}
+
+		defaultFlushInterval := 1 * time.Second
+
+		// Non defaults for these options
+		skipVerify := true
+		passHostHeader := false
+		proxyWebSockets := true
+		flushInterval := 5 * time.Second
+
+		// Test cases and expected outcomes
+		validHTTP := "http://foo.bar/baz"
+		validHTTPUpstream := Upstream{
+			ID:                    "/baz",
+			Path:                  "/baz",
+			URI:                   validHTTP,
+			InsecureSkipTLSVerify: skipVerify,
+			PassHostHeader:        passHostHeader,
+			ProxyWebSockets:       proxyWebSockets,
+			FlushInterval:         &flushInterval,
+		}
+
+		// Test cases and expected outcomes
+		emptyPathHTTP := "http://foo.bar"
+		emptyPathHTTPUpstream := Upstream{
+			ID:                    "/",
+			Path:                  "/",
+			URI:                   emptyPathHTTP,
+			InsecureSkipTLSVerify: skipVerify,
+			PassHostHeader:        passHostHeader,
+			ProxyWebSockets:       proxyWebSockets,
+			FlushInterval:         &flushInterval,
+		}
+
+		validFileWithFragment := "file://var/lib/website#/bar"
+		validFileWithFragmentUpstream := Upstream{
+			ID:                    "/bar",
+			Path:                  "/bar",
+			URI:                   validFileWithFragment,
+			InsecureSkipTLSVerify: skipVerify,
+			PassHostHeader:        passHostHeader,
+			ProxyWebSockets:       proxyWebSockets,
+			FlushInterval:         &flushInterval,
+		}
+
+		validStatic := "static://204"
+		validStaticCode := 204
+		validStaticUpstream := Upstream{
+			ID:                    validStatic,
+			Path:                  validStatic,
+			URI:                   "",
+			Static:                true,
+			StaticCode:            &validStaticCode,
+			InsecureSkipTLSVerify: false,
+			PassHostHeader:        true,
+			ProxyWebSockets:       false,
+			FlushInterval:         &defaultFlushInterval,
+		}
+
+		invalidStatic := "static://abc"
+		invalidStaticCode := 200
+		invalidStaticUpstream := Upstream{
+			ID:                    invalidStatic,
+			Path:                  invalidStatic,
+			URI:                   "",
+			Static:                true,
+			StaticCode:            &invalidStaticCode,
+			InsecureSkipTLSVerify: false,
+			PassHostHeader:        true,
+			ProxyWebSockets:       false,
+			FlushInterval:         &defaultFlushInterval,
+		}
+
+		invalidHTTP := ":foo"
+		invalidHTTPErrMsg := "could not parse upstream \":foo\": parse \":foo\": missing protocol scheme"
+
+		DescribeTable("convertLegacyUpstreams",
+			func(o *convertUpstreamsTableInput) {
+				upstreams, err := convertLegacyUpstreams(o.upstreamStrings, skipVerify, passHostHeader, proxyWebSockets, flushInterval)
+
+				if o.errMsg != "" {
+					Expect(err).To(HaveOccurred())
+					Expect(err.Error()).To(Equal(o.errMsg))
+				} else {
+					Expect(err).ToNot(HaveOccurred())
+				}
+
+				Expect(upstreams).To(ConsistOf(o.expectedUpstreams))
+			},
+			Entry("with no upstreams", &convertUpstreamsTableInput{
+				upstreamStrings:   []string{},
+				expectedUpstreams: Upstreams{},
+				errMsg:            "",
+			}),
+			Entry("with a valid HTTP upstream", &convertUpstreamsTableInput{
+				upstreamStrings:   []string{validHTTP},
+				expectedUpstreams: Upstreams{validHTTPUpstream},
+				errMsg:            "",
+			}),
+			Entry("with a HTTP upstream with an empty path", &convertUpstreamsTableInput{
+				upstreamStrings:   []string{emptyPathHTTP},
+				expectedUpstreams: Upstreams{emptyPathHTTPUpstream},
+				errMsg:            "",
+			}),
+			Entry("with a valid File upstream with a fragment", &convertUpstreamsTableInput{
+				upstreamStrings:   []string{validFileWithFragment},
+				expectedUpstreams: Upstreams{validFileWithFragmentUpstream},
+				errMsg:            "",
+			}),
+			Entry("with a valid static upstream", &convertUpstreamsTableInput{
+				upstreamStrings:   []string{validStatic},
+				expectedUpstreams: Upstreams{validStaticUpstream},
+				errMsg:            "",
+			}),
+			Entry("with an invalid static upstream, code is 200", &convertUpstreamsTableInput{
+				upstreamStrings:   []string{invalidStatic},
+				expectedUpstreams: Upstreams{invalidStaticUpstream},
+				errMsg:            "",
+			}),
+			Entry("with an invalid HTTP upstream", &convertUpstreamsTableInput{
+				upstreamStrings:   []string{invalidHTTP},
+				expectedUpstreams: Upstreams{},
+				errMsg:            invalidHTTPErrMsg,
+			}),
+			Entry("with an invalid HTTP upstream and other upstreams", &convertUpstreamsTableInput{
+				upstreamStrings:   []string{validHTTP, invalidHTTP},
+				expectedUpstreams: Upstreams{},
+				errMsg:            invalidHTTPErrMsg,
+			}),
+			Entry("with multiple valid upstreams", &convertUpstreamsTableInput{
+				upstreamStrings:   []string{validHTTP, validFileWithFragment, validStatic},
+				expectedUpstreams: Upstreams{validHTTPUpstream, validFileWithFragmentUpstream, validStaticUpstream},
+				errMsg:            "",
+			}),
+		)
+	})
+})

pkg/apis/options/load_test.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
-	"testing"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/ginkgo/extensions/table"
@@ -12,11 +11,6 @@ import (
 	"github.com/spf13/pflag"
 )
 
-func TestOptionsSuite(t *testing.T) {
-	RegisterFailHandler(Fail)
-	RunSpecs(t, "Options Suite")
-}
-
 var _ = Describe("Load", func() {
 	Context("with a testOptions structure", func() {
 		type TestOptionSubStruct struct {
@@ -300,6 +294,11 @@ var _ = Describe("Load", func() {
 				input:          &Options{},
 				expectedOutput: NewOptions(),
 			}),
+			Entry("with an empty LegacyOptions struct, should return default values", &testOptionsTableInput{
+				flagSet:        NewFlagSet,
+				input:          &LegacyOptions{},
+				expectedOutput: NewLegacyOptions(),
+			}),
 		)
 	})
 })

pkg/apis/options/options.go

@@ -24,7 +24,6 @@ type Options struct {
 	ProxyPrefix        string   `flag:"proxy-prefix" cfg:"proxy_prefix"`
 	PingPath           string   `flag:"ping-path" cfg:"ping_path"`
 	PingUserAgent      string   `flag:"ping-user-agent" cfg:"ping_user_agent"`
-	ProxyWebSockets    bool     `flag:"proxy-websockets" cfg:"proxy_websockets"`
 	HTTPAddress        string   `flag:"http-address" cfg:"http_address"`
 	HTTPSAddress       string   `flag:"https-address" cfg:"https_address"`
 	ReverseProxy       bool     `flag:"reverse-proxy" cfg:"reverse_proxy"`
@@ -64,26 +63,26 @@ type Options struct {
 	Session SessionOptions `cfg:",squash"`
 	Logging Logging        `cfg:",squash"`
 
-	Upstreams                     []string      `flag:"upstream" cfg:"upstreams"`
+	// Not used in the legacy config, name not allowed to match an external key (upstreams)
-	SkipAuthRegex                 []string      `flag:"skip-auth-regex" cfg:"skip_auth_regex"`
+	// TODO(JoelSpeed): Rename when legacy config is removed
-	SkipAuthStripHeaders          bool          `flag:"skip-auth-strip-headers" cfg:"skip_auth_strip_headers"`
+	UpstreamServers Upstreams `cfg:",internal"`
-	SkipJwtBearerTokens           bool          `flag:"skip-jwt-bearer-tokens" cfg:"skip_jwt_bearer_tokens"`
+
-	ExtraJwtIssuers               []string      `flag:"extra-jwt-issuers" cfg:"extra_jwt_issuers"`
+	SkipAuthRegex         []string `flag:"skip-auth-regex" cfg:"skip_auth_regex"`
-	PassBasicAuth                 bool          `flag:"pass-basic-auth" cfg:"pass_basic_auth"`
+	SkipAuthStripHeaders  bool     `flag:"skip-auth-strip-headers" cfg:"skip_auth_strip_headers"`
-	SetBasicAuth                  bool          `flag:"set-basic-auth" cfg:"set_basic_auth"`
+	SkipJwtBearerTokens   bool     `flag:"skip-jwt-bearer-tokens" cfg:"skip_jwt_bearer_tokens"`
-	PreferEmailToUser             bool          `flag:"prefer-email-to-user" cfg:"prefer_email_to_user"`
+	ExtraJwtIssuers       []string `flag:"extra-jwt-issuers" cfg:"extra_jwt_issuers"`
-	BasicAuthPassword             string        `flag:"basic-auth-password" cfg:"basic_auth_password"`
+	PassBasicAuth         bool     `flag:"pass-basic-auth" cfg:"pass_basic_auth"`
-	PassAccessToken               bool          `flag:"pass-access-token" cfg:"pass_access_token"`
+	SetBasicAuth          bool     `flag:"set-basic-auth" cfg:"set_basic_auth"`
-	PassHostHeader                bool          `flag:"pass-host-header" cfg:"pass_host_header"`
+	PreferEmailToUser     bool     `flag:"prefer-email-to-user" cfg:"prefer_email_to_user"`
-	SkipProviderButton            bool          `flag:"skip-provider-button" cfg:"skip_provider_button"`
+	BasicAuthPassword     string   `flag:"basic-auth-password" cfg:"basic_auth_password"`
-	PassUserHeaders               bool          `flag:"pass-user-headers" cfg:"pass_user_headers"`
+	PassAccessToken       bool     `flag:"pass-access-token" cfg:"pass_access_token"`
-	SSLInsecureSkipVerify         bool          `flag:"ssl-insecure-skip-verify" cfg:"ssl_insecure_skip_verify"`
+	SkipProviderButton    bool     `flag:"skip-provider-button" cfg:"skip_provider_button"`
-	SSLUpstreamInsecureSkipVerify bool          `flag:"ssl-upstream-insecure-skip-verify" cfg:"ssl_upstream_insecure_skip_verify"`
+	PassUserHeaders       bool     `flag:"pass-user-headers" cfg:"pass_user_headers"`
-	SetXAuthRequest               bool          `flag:"set-xauthrequest" cfg:"set_xauthrequest"`
+	SSLInsecureSkipVerify bool     `flag:"ssl-insecure-skip-verify" cfg:"ssl_insecure_skip_verify"`
-	SetAuthorization              bool          `flag:"set-authorization-header" cfg:"set_authorization_header"`
+	SetXAuthRequest       bool     `flag:"set-xauthrequest" cfg:"set_xauthrequest"`
-	PassAuthorization             bool          `flag:"pass-authorization-header" cfg:"pass_authorization_header"`
+	SetAuthorization      bool     `flag:"set-authorization-header" cfg:"set_authorization_header"`
-	SkipAuthPreflight             bool          `flag:"skip-auth-preflight" cfg:"skip_auth_preflight"`
+	PassAuthorization     bool     `flag:"pass-authorization-header" cfg:"pass_authorization_header"`
-	FlushInterval                 time.Duration `flag:"flush-interval" cfg:"flush_interval"`
+	SkipAuthPreflight     bool     `flag:"skip-auth-preflight" cfg:"skip_auth_preflight"`
 
 	// These options allow for other providers besides Google, with
 	// potential overrides.
@@ -114,7 +113,6 @@ type Options struct {
 
 	// internal values that are set after config validation
 	redirectURL        *url.URL
-	proxyURLs          []*url.URL
 	compiledRegex      []*regexp.Regexp
 	provider           providers.Provider
 	signatureData      *SignatureData
@@ -125,7 +123,6 @@ type Options struct {
 
 // Options for Getting internal values
 func (o *Options) GetRedirectURL() *url.URL                        { return o.redirectURL }
-func (o *Options) GetProxyURLs() []*url.URL                        { return o.proxyURLs }
 func (o *Options) GetCompiledRegex() []*regexp.Regexp              { return o.compiledRegex }
 func (o *Options) GetProvider() providers.Provider                 { return o.provider }
 func (o *Options) GetSignatureData() *SignatureData                { return o.signatureData }
@@ -135,7 +132,6 @@ func (o *Options) GetRealClientIPParser() ipapi.RealClientIPParser { return o.re
 
 // Options for Setting internal values
 func (o *Options) SetRedirectURL(s *url.URL)                        { o.redirectURL = s }
-func (o *Options) SetProxyURLs(s []*url.URL)                        { o.proxyURLs = s }
 func (o *Options) SetCompiledRegex(s []*regexp.Regexp)              { o.compiledRegex = s }
 func (o *Options) SetProvider(s providers.Provider)                 { o.provider = s }
 func (o *Options) SetSignatureData(s *SignatureData)                { o.signatureData = s }
@@ -149,7 +145,6 @@ func NewOptions() *Options {
 		ProxyPrefix:                      "/oauth2",
 		ProviderType:                     "google",
 		PingPath:                         "/ping",
-		ProxyWebSockets:                  true,
 		HTTPAddress:                      "127.0.0.1:4180",
 		HTTPSAddress:                     ":443",
 		RealClientIPHeader:               "X-Real-IP",
@@ -160,13 +155,10 @@ func NewOptions() *Options {
 		AzureTenant:                      "common",
 		SetXAuthRequest:                  false,
 		SkipAuthPreflight:                false,
-		SkipAuthStripHeaders:             false,
-		FlushInterval:                    time.Duration(1) * time.Second,
 		PassBasicAuth:                    true,
 		SetBasicAuth:                     false,
 		PassUserHeaders:                  true,
 		PassAccessToken:                  false,
-		PassHostHeader:                   true,
 		SetAuthorization:                 false,
 		PassAuthorization:                false,
 		PreferEmailToUser:                false,

pkg/apis/options/options_suite_test.go

@@ -0,0 +1,16 @@
+package options
+
+import (
+	"testing"
+
+	"github.com/oauth2-proxy/oauth2-proxy/pkg/logger"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+func TestOptionsSuite(t *testing.T) {
+	logger.SetOutput(GinkgoWriter)
+
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Options Suite")
+}

pkg/validation/options.go

@@ -176,17 +176,7 @@ func Validate(o *options.Options) error {
 	redirectURL, msgs = parseURL(o.RawRedirectURL, "redirect", msgs)
 	o.SetRedirectURL(redirectURL)
 
-	for _, u := range o.Upstreams {
+	msgs = append(msgs, validateUpstreams(o.UpstreamServers)...)
-		upstreamURL, err := url.Parse(u)
-		if err != nil {
-			msgs = append(msgs, fmt.Sprintf("error parsing upstream: %s", err))
-		} else {
-			if upstreamURL.Path == "" {
-				upstreamURL.Path = "/"
-			}
-			o.SetProxyURLs(append(o.GetProxyURLs(), upstreamURL))
-		}
-	}
 
 	for _, u := range o.SkipAuthRegex {
 		compiledRegex, err := regexp.Compile(u)

pkg/validation/options_test.go

@@ -22,7 +22,11 @@ const (
 
 func testOptions() *options.Options {
 	o := options.NewOptions()
-	o.Upstreams = append(o.Upstreams, "http://127.0.0.1:8080/")
+	o.UpstreamServers = append(o.UpstreamServers, options.Upstream{
+		ID:   "upstream",
+		Path: "/",
+		URI:  "http://127.0.0.1:8080/",
+	})
 	o.Cookie.Secret = cookieSecret
 	o.ClientID = clientID
 	o.ClientSecret = clientSecret
@@ -140,26 +144,6 @@ func TestRedirectURL(t *testing.T) {
 	assert.Equal(t, expected, o.GetRedirectURL())
 }
 
-func TestProxyURLs(t *testing.T) {
-	o := testOptions()
-	o.Upstreams = append(o.Upstreams, "http://127.0.0.1:8081")
-	assert.Equal(t, nil, Validate(o))
-	expected := []*url.URL{
-		{Scheme: "http", Host: "127.0.0.1:8080", Path: "/"},
-		// note the '/' was added
-		{Scheme: "http", Host: "127.0.0.1:8081", Path: "/"},
-	}
-	assert.Equal(t, expected, o.GetProxyURLs())
-}
-
-func TestProxyURLsError(t *testing.T) {
-	o := testOptions()
-	o.Upstreams = append(o.Upstreams, "127.0.0.1:8081")
-	err := Validate(o)
-	assert.NotEqual(t, nil, err)
-	assert.Contains(t, err.Error(), "error parsing upstream")
-}
-
 func TestCompiledRegex(t *testing.T) {
 	o := testOptions()
 	regexps := []string{"/foo/.*", "/ba[rz]/quux"}