[GitHub] flink issue #5172: [FLINK-8275] [Security] fix keytab local path in YarnTask...

[suez1224]

Github user suez1224 commented on the issue:

https://github.com/apache/flink/pull/5172
  
It's already merged to master, and should be available in Flink 1.4.1.

On Wed, Feb 7, 2018 at 11:11 PM, toggm  wrote:

> What happended with this PR? Why was it closed?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> , or 
mute
> the thread
> 

> .
>



-- 
"So you have to trust that the dots will somehow connect in your future."



---

[GitHub] flink issue #5172: [FLINK-8275] [Security] fix keytab local path in YarnTask...

[toggm]

Github user toggm commented on the issue:

https://github.com/apache/flink/pull/5172
  
What happended with this PR? Why was it closed?


---

[GitHub] flink issue #5172: [FLINK-8275] [Security] fix keytab local path in YarnTask...

[suez1224]

Github user suez1224 commented on the issue:

https://github.com/apache/flink/pull/5172
  
@tzulitai thanks a lot for the refactoring, the commit looks good to me.


---

[GitHub] flink issue #5172: [FLINK-8275] [Security] fix keytab local path in YarnTask...

[tzulitai]

Github user tzulitai commented on the issue:

https://github.com/apache/flink/pull/5172
  
@suez1224 Thanks a lot for the contribution!
I've had a look and the changes LGTM. I did have a comment regarding 
injecting a dependency for the runner, which I've added a commit for.

Can you take a look at 2ffa659 before I actually merge this, and let me 
know what you think? Thanks!
Will make sure this gets in for Flink 1.4.1 ..


---

[GitHub] flink issue #5172: [FLINK-8275] [Security] fix keytab local path in YarnTask...

[toggm]

Github user toggm commented on the issue:

https://github.com/apache/flink/pull/5172
  
We try to use apache flink in Kerberos secured environment and had to 
backport to flink 1.3.2 because of that issue. Would be good to have that PR to 
be able to migrate to the latest flink version.


---

[GitHub] flink issue #5172: [FLINK-8275] [Security] fix keytab local path in YarnTask...

[suez1224]

Github user suez1224 commented on the issue:

https://github.com/apache/flink/pull/5172
  
Thanks a lot for the review, @EronWright. Could you please take another 
look?


---

[GitHub] flink issue #5172: [FLINK-8275] [Security] fix keytab local path in YarnTask...

[EronWright]

Github user EronWright commented on the issue:

https://github.com/apache/flink/pull/5172
  
This PR probably fixes the problem, but it would be good to address the 
deeper problem that the code is confusing.   At least we could add some 
commentary to the code.  The specific problems, in my view, are:
1. A filename is transmitted from client -> AM -> TM in the env variable 
`_KEYTAB_PATH` but the value doesn't appear to be used.   In effect it is a 
flag asserting that a keytab named `krb5.keytab` is available.  Alternatives:
  a. Use `krb5.keytab` as the value.
  b. Eliminate the env check and simply look for the file; if present, use 
it.
2. The existence of the "integration test code" has an unclear purpose.   
It mutates the Hadoop configuration, why?   Is the code active in any 
production scenario?

Note that `YarnTaskExecutorRunner` implements this in a slightly different 
way, and should be re-tested for 1.5.0 (since I don't think it is in use yet).



---

[GitHub] flink issue #5172: [FLINK-8275] [Security] fix keytab local path in YarnTask...

[tzulitai]

Github user tzulitai commented on the issue:

https://github.com/apache/flink/pull/5172
  
@suez1224 keep in mind, that contribution PRs should initially have one 
commit with the commit message appropriately set (the title of the PR would be 
a good commit message for your case).


---

[GitHub] flink issue #5172: [FLINK-8275] [Security] fix keytab local path in YarnTask...

[tzulitai]

Github user tzulitai commented on the issue:

https://github.com/apache/flink/pull/5172
  
cc @EronWright, gentle ping since you mentioned to include you in the 
review :)


---

[GitHub] flink issue #5172: [FLINK-8275] [Security] fix keytab local path in YarnTask...

[tzulitai]

Github user tzulitai commented on the issue:

https://github.com/apache/flink/pull/5172
  
Thanks for the PR @suez1224.
There is a duplicate JIRA for this issue: 
https://issues.apache.org/jira/browse/FLINK-8270.
Can you take a look at the suggestions explained in that JIRA, and include 
that in your solution too?


---