-
Notifications
You must be signed in to change notification settings - Fork 229
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
Move configuration into a Config object #158
Conversation
This also resolves #141 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One quick question, and as I merged some of this other changes it needs rebasing with master. But looks good. Thanks.
return fmt.Sprintf("%s/%s-standalone%s/%s%s.json", baseURL, normalisedVersion, strictSuffix, strings.ToLower(kind), kindSuffix) | ||
} | ||
|
||
func determineBaseURL(config *Config) string { | ||
// Order of precendence: | ||
// 1. If --openshift is passed, return the openshift schema location |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this changes the existing behaviour? I think 1-3 should be reverse to match the current behaviour, which uses the ENV, then the flag, then checks the openshift option, then the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure this doesn't change the existing behavior. The old code relied on viper, which bound the environment variable to the flag, and viper gives the flag a higher precedence. This behavior is demonstrated in the series of acceptance tests added in #157.
Further, I think this order of precedence is closer to the norm; we can see here that kubectl follows this pattern with the kubeconfig
flag, and here that helm follows it with the home
flag.
The openshift
flag can go wherever we want it, but I think it makes the most sense as the highest precedent, since it is the most specific of the configurations.
This commit introduces the `Config` object, which contains all the configuration information that `kubeval.Validate` might need. The default settings can be obtained by using the `NewDefaultConfig` function. Functions `Validate` and `ValidateWithCache` have been modified to accept an optional Config object - if none is provided, the default is used. Unit tests have also been updated to reflect these changes.
960af92
to
fe7303a
Compare
This commit introduces the
Config
object, which contains all theconfiguration information that
kubeval.Validate
might need. The defaultsettings can be obtained by using the
NewDefaultConfig
function.Functions
Validate
andValidateWithCache
have been modified toaccept an optional Config object - if none is provided, the default is
used.
Unit tests have also been updated to reflect these changes.
Note that these changes also pass the newly added acceptance tests in #157