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

Load versioned gssapi libs #9820

Merged
merged 1 commit into from
Jul 14, 2016
Merged

Load versioned gssapi libs #9820

merged 1 commit into from
Jul 14, 2016

Conversation

liggitt
Copy link
Contributor

@liggitt liggitt commented Jul 13, 2016

load versioned shared gssapi libs, rather than the unversioned development lib

@sgallagher
Copy link
Contributor

I don't particularly like this approach. The problem here is actually a bug in https://github.com/apcera/gssapi/

They have a function in https://github.com/apcera/gssapi/blob/master/lib.go#L305 that isn't properly figuring out what the correct suffix should be. I think it would be better for us to work with that upstream to fix the appendOSExt() routine so that we can rely on it and not have to hack this list every time MIT Kerberos adds a new function.

However, given that this is fairly time-sensitive, I guess we could hack around it for now. Inline comments forthcoming.

// MIT
"libgssapi_krb5.so.2.2",
"libgssapi_krb5.so.2",
"libgssapi_krb5.so",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we relying on any feature that's not available from libgssapi_krb5.so.2.0? If not, we should probably not explicitly try to load libgssapi_krb5.so.2.2.

Similarly, we should never try to load the unversioned shared object, because if it jumps to .so.3 at some point, that means that backwards-compatibility has been broken and we shouldn't assume that it works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tightened to the current major versions we work with

@liggitt
Copy link
Contributor Author

liggitt commented Jul 14, 2016

[test]

@liggitt
Copy link
Contributor Author

liggitt commented Jul 14, 2016

I agree the upstream component should probably take a list of acceptable lib files, I'll happily defer to it once a default lookup like this moves there

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 94786fc

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6239/)

@liggitt
Copy link
Contributor Author

liggitt commented Jul 14, 2016

[merge]

@liggitt
Copy link
Contributor Author

liggitt commented Jul 14, 2016

@enj fyi, this was probably why you thought fedora didn't install gssapi by default. on a clean fedora image, /usr/lib64/libgssapi_krb5.so.2 exists.

edit: maybe you were missing kinit, which doesn't seem to be installed by default

@openshift-bot
Copy link
Contributor

openshift-bot commented Jul 14, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/6239/) (Image: devenv-rhel7_4587)

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 94786fc

@enj
Copy link
Contributor

enj commented Jul 14, 2016

I believe it was the lack of kinit that threw me off.

@sgallagher
Copy link
Contributor

LGTM

@openshift-bot openshift-bot merged commit e1d5e16 into openshift:master Jul 14, 2016
@liggitt liggitt deleted the gssapi-versioned-libs branch July 14, 2016 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants