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
PHOENIX-4688 Kerberize python phoenixdb #307
Conversation
…e links for views and not for indexes
…ies in CQSI Signed-off-by: aertoria <castives@gmail.com>
…ableRef, Map<ImmutableBytesPtr,RowMutationState>> mutations
…ableRef, Map<ImmutableBytesPtr,RowMutationState>> mutations (addendum)
…mTablesToSystemNamespaceIT
…ng Map<TableRef, Map<ImmutableBytesPtr,RowMutationState>> mutations (addendum)" This reverts commit 4e0c0a3.
…rk in SkipScanFilter
…ableRef, Map<ImmutableBytesPtr,RowMutationState>> mutations (addendum)
…a we are using in hadoop ecosystem
…statement Signed-off-by: aertoria <castives@gmail.com>
…rcase Schema Names)
…y setting attempt on read-only configuration(Rajeshbabu)
…y setting attempt on read-only configuration-addendum(Rajeshbabu)
…lause using 4.10 phoenix client on 4.13 phoenix server
…ifferent client fails
…ifferent client fails (addendum)
…getExplainPlan() and pull optimize() out of getExplainPlan()
…oshihiro Suzuki) When using the thin-client in Spark, we encounter problems in that Spark is placing its own version of avatica on the classpath as well. We can relocate most of Avatica (all but the protobuf generated messages as their classnames are required to be 'org.apache.calcite.avatica.proto' presently) and hadoop-common to avoid future problems.
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.
Pulling this down and trying it out today. Some general thoughts:
- Need instructions on building (now that it's more complicated)
- How do we make sure requests-kerberos is built in a way that phoenixdb will grab it? (e.g.
0.13.0.dev0-phoenixdb
)
export KRB5_TRACE=/dev/stdout | ||
|
||
#echo "RUNNING KINIT" | ||
kinit -kt $KEYTAB_LOC $PRINC |
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 we kinit to a custom location? e.g. the -c
option. Then, later, we just set the variable KRB5CCNAME
in the shell ENV.
This would help prevent us from bashing the user's ticket (if they already have one).
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 tried something similar
File KRB5CCNAME = File.createTempFile("krb5ccname", null);
kinitEnv.put("KRB5CCNAME", KRB5CCNAME.getAbsolutePath());
This stalled, although looking at the code now it probably should have been a directory, which is why kinit stalled
I just tried on the command line and MAC OS (Heimdal) kinit does not require a directory, I edited out my real user name and realm
$ export KRB5CCNAME=cc
$ kinit -c cc me
me@REALM.COM's password:
$ klist
Credentials cache: FILE:cc
Principal: me@REALM.COM
Issued Expires Principal
Jul 10 18:08:52 2018 Jul 11 18:08:47 2018 krbtgt/REALM.COM@REALM.COM
So it looks like my code is correct and I still do not know why kinit stalled
I can try this again...
However activate will modify the environment to ensure that python is used from the virtual environment where activate was run. At this point we can
- try to figure out what the environment changes are and capture them
- pass the when executing python
or just continue running in the same shell, which is why I stopped attempts to make ny further reductions to the shell script
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 just tried on the command line and MAC OS (Heimdal) kinit does not require a directory
Yeah, convention is to use ${tmpdir}/krb5cc_$(current-user uid)
.
pass the when executing python or just continue running in the same shell, which is why I stopped attempts to make ny further reductions to the shell script
Oh right, I forgot they would bash the environment. Let's just let this be for now. Will be easier to come back to it later.
function cleanup { | ||
set +e | ||
set +u | ||
kdestroy |
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.
If we use a custom directory for the kinit
, then this just becomes removing that custom directory.
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.
Not to be overly pedantic, but you would want to still pass krb5ccname and just call kdestroy to make sure proper cleanup is done. Especially in cases where memory caches are used vs file based caches. If i get java called kinit to work, I am fairly sure I can get kdestroy to work as well
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.
Ok, cool. I didn't think kdestroy was doing more than just cleaning up those token :)
cmdList.add(Integer.toString(PQS_PORT)); | ||
cmdList.add(Paths.get(currentDirectory, "src", "it", "bin", "test_phoenixdb.py").toString()); | ||
|
||
// kinit in some random credcache |
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.
Delete this stuff?
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'll give this one more shot in a bit
deleted all references to kinit
LOG.info("MINIKDC PORT " + kdcPort); | ||
// Render a Heimdal compatible krb5.conf | ||
// Currently kinit will only try tcp if the KDC is defined as | ||
// kdc = tcp/hostname:port |
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.
Nice! I learned something here :)
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.
Yes mini KDC will render a krb5.conf file, however it is useless on MAC OS as Heimdal seemingly decided to want to specify protocols as opposed to trying them all. This has been fixed but Apple has not packaged it yet I guess
return dest; | ||
} | ||
|
||
String kinit(String principal, File keytab, File krb5ConfFile) throws IOException{ |
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.
This is unused now, right?
# The ASF licenses this file to You under the Apache License, Version 2.0 | ||
# (the "License"); you may not use this file except in compliance with | ||
# the License. You may obtain a copy of the License at | ||
# Copyright 2015 Lukas Lalinsky |
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.
Any reason for the re-add of this? We don't need this after the IP Clearance process, I think. NOTICE file should be sufficient.
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 am not sure how that went back in, it is possible that I may have copied init.py from the time I was doing this work on my own before I found out that this has been moved to phoenix. I will change the header
python/requests-kerberos/LICENSE
Outdated
@@ -0,0 +1,15 @@ | |||
ISC License |
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.
Just calling out that this is allowed: ISC is a Category-A license per https://www.apache.org/legal/resolved.html
@@ -47,6 +47,11 @@ | |||
<plugin> | |||
<groupId>org.apache.maven.plugins</groupId> | |||
<artifactId>maven-failsafe-plugin</artifactId> | |||
<configuration> | |||
<excludes> | |||
<exclude>**/SecureQueryServerPhoenixDBIT.java</exclude> |
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.
You not intending for this test to be executed during the normal build process?
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.
There are a few prerequisites
- Either anaconda or virtual env must to be installed
- System must provide either MIT or Heimdal kerberos utilities and libraries
Ah, I see this is updated in python/requests-kerberos/requests_kerberos/init.py. We should update python/phoenixdb/requirements.txt to make sure we pull that 0.13.0.dev0-phoenixdb, right? |
Tried to run the test, but it failed with:
Might have been me hacking on things... digging more. |
@joshelser If you are still unable to get .deactivate to work, remove it from the shell script for now and we can revisit it. Again I am being overly pedantic here, but a the shell exits, we really do not need to clean up any environment. |
Ok. I'm looking at this again today. Thanks for the feedback so far. The test/code looks good to me. I think we just need to update documentation to explain what we're doing (maybe a README.md in |
Ugh, I'm getting frustrated: I have MIT kerberos on my Mac, so I unblocked myself first by just forcing the minikdc config file to be made instead of the if-branch you added, Lev. The next thing, I get a failure trying to launch python from the virtualenv:
This was reproducible doing it by hand, so I thought maybe it was related to me using python-2.7.14 (old). So, I switched over to Python-3.6.4, reinstalled everything, and I got this.
I ran into another GH issue saying that pykerberos==1.1.14 fixed it for them, but I'm not seeing a difference locally. How do you feel about requiring Docker, @pu239ppy? ;) |
@joshelser Not sure how you got MIT on Mac OS X, is it in some ports package? I can try this later on ubuntu perhaps, if you want a test with MIT. I suppose integrating docker into the mix would make things interesting. I'll try it over the weekend if I get the time. On the other front I think the only outstanding issues we have now is
|
Yeah, homebrew has a
I'm playing with different versions of python now, but am just worried about the feasibility of this actually working on the general person's machine given how much I'm struggling :\ |
Ah, I think my issue might have been cruft sitting in Getting a straightforward HTTP/401 error now. That I know how to deal with :) |
Ok, where I'm at now:
This gets the phoenixdb client to actually submit the initial POST and get the
I can't seem to unwrap what's wrong with the request to the KDC which is preventing that from happening. Need to find more debug... |
Turning back on
So, definitely the KDC throwing a fit and telling us to go away: |
If there's a bright-side, it's something in the python code, not the test harness/setup itself. Spinning up the JDBC thin client against this setup works:
|
On the note about docker, trying to think about this, I feel like a docker environment that can spin up all of the necessary stuff to run both of the python tests as well as this new one will be best. Essentially, in the docker container, we have PQS up with all of the necessary environment stuff which will make all of our current tests (and any future test) that much easier to automate. I'm also happy to try to help with that. I know you've spent a bunch of time on this already @pu239ppy |
Thanks for the update, @pu239ppy. I think you got bit by 5.x moving over to master. Any chance you could throw a rebase on here? Holler if it's ready for me to look at, too. I see your new jar for testing. |
Hi pu239ppy, I have some questions, can you help me. Am I wrong, please guide me, thank you very much. |
@zhouwei0914 This PR is no longer active, please see #344
It turned out that 2 Produced the desired effect that I abandoned the path of attempting to path requests-kerberos. However in the current PR #344 on top of Phoenix 5 it appears that this strategy no longer works. See additional details in later comments in PHOENIX-4688 |
Lets rip out httplib and replace with requests and use requests kerberos
Notes
mvn -Dit.test=org.apache.phoenix.end2end.SecureQueryServerPhoenixDBIT verify
conda create -y -p $PY_ENV_PATH || virtualenv $PY_ENV_PATH
Standalone install
This is currently a manual procedure with a few steps due to the current version not being in a public pipy and also the fact that we had to fork requests-kerberos
Example script