-
Notifications
You must be signed in to change notification settings - Fork 252
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
krb5.conf parameter expansion #429
base: master
Are you sure you want to change the base?
krb5.conf parameter expansion #429
Conversation
Fixes jcmturner#428 Support parameter expansion for default_ccache_name, default_client_keytab_name and default_keytab_name. Additionally, parse krb5-config if it exists, for defaults that aren't set in krb5.conf. This should support MIT sites that use a self-maintained version or distros that change MIT defaults.
3a5ccd1
to
995c1e9
Compare
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.
Thanks for the contribution. There are some approach and design questions I think we need to work out. Thanks.
@@ -726,3 +731,100 @@ func (c *Config) JSON() (string, error) { | |||
} | |||
return string(b), nil | |||
} | |||
|
|||
var expandMap map[string]string |
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 have been trying to avoid global variables. Could this be a local variable to the LibDefaults.parseLines() method function?
|
||
var expandMap map[string]string | ||
|
||
func init() { |
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 like to avoid using Go's init function as if there are issues with it during runtime it can be confusing to debug as it was not explicitly called by another function. Could we initiate the expandMap within the LibDefaults.parseLines() method?
return | ||
} | ||
|
||
func ExpandParams(in string) (out string) { |
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 we should make this a private function.
|
||
if cfg == nil { | ||
// find krb5-config | ||
file, err := exec.LookPath("krb5-config") |
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.
Can you share some details on this file please? I've not been able to understand what this relates to. thanks
} | ||
|
||
var defaultPkgConfigVars = map[string]string{ | ||
"prefix": "/usr", |
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.
These things are unix specific and gokrb5 is intended to be platform independent. I'm not sure I completely understand the purpose of needing things lie exec_prefix in a krb5.conf. I need to understand this more so we can figure out what would be an appropriate platform independent approach.
Fixes #428
Support parameter expansion for default_ccache_name, default_client_keytab_name and default_keytab_name.
Additionally, parse krb5-config if it exists, for defaults that aren't set in krb5.conf. This should support MIT sites that use a self-maintained version or distros that change MIT defaults.