Feature - Add env variable support for alpha struct (#2375)

* added envsubstring package and added simple test cases.imple tests.

* added documentation

* added changelog entry

* added documentation to wrong file


.

* changed tests to ginkgo format

* update project to use better maintained library

* use defer to clear test variable after tests finished

* updated docs for the new package documentation and fixed bad english

* refactored function to "reduce" complexity.

* updated changelog for new version

updated readme

* minor formatting

---------

Co-authored-by: Haydn Evans <h.evans@douglas.de>
Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

CHANGELOG.md

@@ -28,6 +28,7 @@
 - [#2295](https://github.com/oauth2-proxy/oauth2-proxy/pull/2295) Change base-image to [GoogleContainerTools/distroless](https://github.com/GoogleContainerTools/distroless) (@kvanzuijlen)
 - [#2356](https://github.com/oauth2-proxy/oauth2-proxy/pull/2356) Update go-jose dependency (@dasvh)
 - [#2357](https://github.com/oauth2-proxy/oauth2-proxy/pull/2357) Update ojg to latest release (@bitfehler)
+- [#1922](https://github.com/oauth2-proxy/oauth2-proxy/pull/1922) Added support for env variables in the alpha struct (@hevans-dglcom)
 
 # V7.5.1
 

docs/docs/configuration/alpha_config.md

@@ -67,6 +67,20 @@ the new config.
 oauth2-proxy --alpha-config ./path/to/new/config.yaml --config ./path/to/existing/config.cfg
 ```
 
+## Using ENV variables in the alpha configuration
+
+The alpha package supports the use of environment variables in place of yaml keys, allowing sensitive values to be pulled from somewhere other than the yaml file.
+When using environment variables, your yaml will look like this:
+
+```yaml
+    providers:
+    - provider: azure
+      clientSecret: ${CLIENT_SECRET}
+      ...
+```
+Where CLIENT_SECRET is an environment variable.
+More information and available patterns can be found [here](https://github.com/a8m/envsubst#docs)
+
 ## Removed options
 
 The following flags/options and their respective environment variables are no

docs/docs/configuration/alpha_config.md.tmpl

@@ -67,6 +67,20 @@ the new config.
 oauth2-proxy --alpha-config ./path/to/new/config.yaml --config ./path/to/existing/config.cfg
 ```
 
+## Using ENV variables in the alpha configuration
+
+The alpha package supports the use of environment variables in place of yaml keys, allowing sensitive values to be pulled from somewhere other than the yaml file.
+When using environment variables, your yaml will look like this:
+
+```yaml
+    providers:
+    - provider: azure
+      clientSecret: ${CLIENT_SECRET}
+      ...
+```
+Where CLIENT_SECRET is an environment variable.
+More information and available patterns can be found [here](https://github.com/a8m/envsubst#docs)
+
 ## Removed options
 
 The following flags/options and their respective environment variables are no

go.mod

@@ -5,6 +5,7 @@ go 1.19
 require (
 	cloud.google.com/go/compute/metadata v0.2.3
 	github.com/Bose/minisentinel v0.0.0-20200130220412-917c5a9223bb
+	github.com/a8m/envsubst v1.4.2
 	github.com/alicebob/miniredis/v2 v2.23.0
 	github.com/benbjohnson/clock v1.3.0
 	github.com/bitly/go-simplejson v0.5.1

go.sum

@@ -47,6 +47,8 @@ github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym
 github.com/FZambia/sentinel v1.0.0 h1:KJ0ryjKTZk5WMp0dXvSdNqp3lFaW1fNFuEYfrkLOYIc=
 github.com/FZambia/sentinel v1.0.0/go.mod h1:ytL1Am/RLlAoAXG6Kj5LNuw/TRRQrv2rt2FT26vP5gI=
 github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
+github.com/a8m/envsubst v1.4.2 h1:4yWIHXOLEJHQEFd4UjrWDrYeYlV7ncFWJOCBRLOZHQg=
+github.com/a8m/envsubst v1.4.2/go.mod h1:MVUTQNGQ3tsjOOtKCNd+fl8RzhsXcDvvAEzkhGtlsbY=
 github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/alicebob/gopher-json v0.0.0-20180125190556-5a6b3ba71ee6/go.mod h1:SGnFV6hVsYE877CKEZ6tDNTjaSXYUk6QqoIK6PrAtcc=

pkg/apis/options/load.go

@@ -7,6 +7,7 @@ import (
 	"reflect"
 	"strings"
 
+	"github.com/a8m/envsubst"
 	"github.com/ghodss/yaml"
 	"github.com/mitchellh/mapstructure"
 	"github.com/spf13/pflag"
@@ -140,25 +141,37 @@ func isUnexported(name string) bool {
 
 // LoadYAML will load a YAML based configuration file into the options interface provided.
 func LoadYAML(configFileName string, into interface{}) error {
-	v := viper.New()
-	v.SetConfigFile(configFileName)
-	v.SetConfigType("yaml")
-	v.SetTypeByDefaultValue(true)
-
-	if configFileName == "" {
-		return errors.New("no configuration file provided")
-	}
-
-	data, err := os.ReadFile(configFileName)
+	buffer, err := loadAndParseYaml(configFileName)
 	if err != nil {
-		return fmt.Errorf("unable to load config file: %w", err)
+		return err
 	}
 
 	// UnmarshalStrict will return an error if the config includes options that are
-	// not mapped to felds of the into struct
-	if err := yaml.UnmarshalStrict(data, into, yaml.DisallowUnknownFields); err != nil {
+	// not mapped to fields of the into struct
+	if err := yaml.UnmarshalStrict(buffer, into, yaml.DisallowUnknownFields); err != nil {
 		return fmt.Errorf("error unmarshalling config: %w", err)
 	}
 
 	return nil
 }
+
+// Performs the heavy lifting of the LoadYaml function
+func loadAndParseYaml(configFileName string) ([]byte, error) {
+	if configFileName == "" {
+		return nil, errors.New("no configuration file provided")
+	}
+
+	unparsedBuffer, err := os.ReadFile(configFileName)
+	if err != nil {
+		return nil, fmt.Errorf("unable to load config file: %w", err)
+	}
+
+	// We now parse over the yaml with env substring, and fill in the ENV's
+	buffer, err := envsubst.Bytes(unparsedBuffer)
+	if err != nil {
+		return nil, fmt.Errorf("error in substituting env variables : %w", err)
+	}
+
+	return buffer, nil
+
+}

pkg/apis/options/load_test.go

@@ -386,8 +386,13 @@ sub:
 
 		DescribeTable("LoadYAML",
 			func(in loadYAMLTableInput) {
-				var configFileName string
+				// Set the required environment variables before running the test
+				os.Setenv("TESTUSER", "Alice")
 
+				// Unset the environment variables after running the test
+				defer os.Unsetenv("TESTUSER")
+
+				var configFileName string
 				if in.configFile != nil {
 					By("Creating a config file")
 					configFile, err := os.CreateTemp("", "oauth2-proxy-test-config-file")
@@ -407,12 +412,14 @@ sub:
 				} else {
 					input = &TestOptions{}
 				}
+
 				err := LoadYAML(configFileName, input)
 				if in.expectedErr != nil {
 					Expect(err).To(MatchError(in.expectedErr.Error()))
 				} else {
 					Expect(err).ToNot(HaveOccurred())
 				}
+
 				Expect(input).To(Equal(in.expectedOutput))
 			},
 			Entry("with a valid input", loadYAMLTableInput{
@@ -466,6 +473,20 @@ sub:
 				expectedOutput: &TestOptions{},
 				expectedErr:    errors.New("error unmarshalling config: error unmarshaling JSON: while decoding JSON: json: cannot unmarshal string into Go struct field TestOptions.StringSliceOption of type []string"),
 			}),
+			Entry("with a config file containing environment variable references", loadYAMLTableInput{
+				configFile: []byte("stringOption: ${TESTUSER}"),
+				input:      &TestOptions{},
+				expectedOutput: &TestOptions{
+					StringOption: "Alice",
+				},
+			}),
+			Entry("with a config file containing env variable references, with a fallback value", loadYAMLTableInput{
+				configFile: []byte("stringOption: ${TESTUSER2=Bob}"),
+				input:      &TestOptions{},
+				expectedOutput: &TestOptions{
+					StringOption: "Bob",
+				},
+			}),
 		)
 	})