Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor runtime property configuration #2663

Open
aeijdenberg opened this issue Jul 11, 2018 · 2 comments
Open

Refactor runtime property configuration #2663

aeijdenberg opened this issue Jul 11, 2018 · 2 comments
Labels
community Community Raised Issue triage Requires review of inportance and prioritisation

Comments

@aeijdenberg
Copy link
Contributor

aeijdenberg commented Jul 11, 2018

Detailed Description

We'd like to add support for specifying runtime configuration properties via CloudFoundry user-provided services.

For example, an operator would specify prior to deployment:

cf create-user-provided-service stratos-properties -p '{"CF_CLIENT": "stratos","CF_CLIENT_SECRET": "xxxx"}'

And then in the manifest.yml:

applications:
  - name: console
    ...
    services:
    - stratos-properties

We've found this pattern works better than other obvious alternatives with CloudFoundry because:

  1. Putting secrets (such as the client secret) in the manifest.yml as an environment variable is challenging with respect to public source repositories.
  2. Setting environment variables on an already running app doesn't work well as many zero-downtime deploy plugins create a new app on each deployment.

Context

Currently runtime properties are spread between:

  1. ./config.properties file
  2. Environment variables (sometimes set via manifest.yml)
  3. /etc/secrets directory

Some settings appear to only work if specified in the ./config.properties file, others appear to allow overriding by environment variables.

Possible Implementation

Last year we made a small Go library to handle looking up environment variables from a list of sources, e.g. first look for environment variables, falling back to data provided by a user provided services, falling back to defaults.

We (cloud.gov.au team) worked with 18F to integrate this with their console.

I've made an experimental change to integrate the same library with the Stratos codebase which is linked as a PR below.

The change is actually quite small - the main part (in main.go) is this method, which initialises an env.VarSet:

// getEnvironmentLookup return a search path for configuration settings
func getEnvironmentLookup() *env.VarSet {
	// Make environment lookup
	envLookup := env.NewVarSet()

	// Environment variables directly set trump all others
	envLookup.AppendSource(os.LookupEnv)

	// If running in CloudFoundry, fallback to a user provided service (if set)
	cfApp, err := cfenv.Current()
	if err == nil {
		envLookup.AppendSource(env.NewLookupFromUPS(cfApp, "stratos-properties"))
	}

	// Fallback to a "config.properties" files in our directory
	envLookup.AppendSource(config.NewConfigFileLookup("./config.properties"))

	// Fallback to individual files in the "/etc/secrets" directory
	envLookup.AppendSource(config.NewSecretsDirLookup("/etc/secrets"))

	return envLookup
}

which is passed around to each component that needs to lookup anything from any of the config sources (that is, it is a replacement for the the config.GetValue() calls).

The branch above compiles and pushes, I haven't tested any deeper than that yet, but we can look at if this is a direction worth pursuing.

@aeijdenberg
Copy link
Contributor Author

@nwmac - is this set of proposed changes of interest to the Stratos team? We found that having the option to specify config this way worked well for deployment within a CloudFoundry app. I've just updated the linked branch above to rebase on current code.

@aeijdenberg
Copy link
Contributor Author

@nwmac - now the big Go refactor has been merged, I've rebased our branch and submitted PR #2866 which implements as described above.

Let me know your thoughts.

@richard-cox richard-cox added the community Community Raised Issue label Oct 24, 2018
@kreinecke kreinecke added the triage Requires review of inportance and prioritisation label Jun 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community Raised Issue triage Requires review of inportance and prioritisation
Projects
None yet
Development

No branches or pull requests

3 participants