You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Lars Volker (Code Review)" <ge...@cloudera.org> on 2018/06/19 22:32:37 UTC

[Impala-ASF-CR] IMPALA-4669: [KSECURITY] Add security library to build

Hello Henry Robinson,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/10759

to review the following change.


Change subject: IMPALA-4669: [KSECURITY] Add security library to build
......................................................................

IMPALA-4669: [KSECURITY] Add security library to build

* Minor compilation fix
* Add krb5 as a non-toolchain dependency
* Handle legacy versions of libkrb5.so by providing implementation of
  krb5_is_config_principal().
* Link against openssl from the toolchain if 1.0.0 or higher not found
  on build machine.
* Update LICENSE.txt and NOTICE.txt re: OpenSSL code in x509_check_host.{h,c}.

Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Reviewed-on: http://gerrit.cloudera.org:8080/5717
Reviewed-by: Henry Robinson <he...@cloudera.com>
Tested-by: Henry Robinson <he...@cloudera.com>
---
M be/src/kudu/security/init.cc
M be/src/kudu/security/test/mini_kdc.cc
M be/src/kudu/security/token_verifier.cc
3 files changed, 53 insertions(+), 8 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/10759/1
-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>

[Impala-ASF-CR] IMPALA-4669: [KSECURITY] Add security library to build

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10759 )

Change subject: IMPALA-4669: [KSECURITY] Add security library to build
......................................................................


Patch Set 7:

This should be ready for review now.


-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Jul 2018 20:16:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4669: [KSECURITY] Add security library to build

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10759 )

Change subject: IMPALA-4669: [KSECURITY] Add security library to build
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10759/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10759/8//COMMIT_MSG@13
PS8, Line 13: The original commit message is below:
> Does the following need updating given this new patch no longer matches wha
I tried to keep the commit messages the same. The alternative seems to be rewriting them completely. Would you prefer the latter? I don't feel strongly about it.



-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 17:46:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4669: [KSECURITY] Add security library to build

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has removed Henry Robinson from this change.  ( http://gerrit.cloudera.org:8080/10759 )

Change subject: IMPALA-4669: [KSECURITY] Add security library to build
......................................................................


Removed reviewer Henry Robinson.
-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>

[Impala-ASF-CR] IMPALA-7006: [KSECURITY] Update security library integration

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10759 )

Change subject: IMPALA-7006: [KSECURITY] Update security library integration
......................................................................

IMPALA-7006: [KSECURITY] Update security library integration

This commit is part of a set of changes for IMPALA-7006. It started
based on an original change (f51c4435), which integrated Kudu's security
folder into our build.

This change removes several compile time checks that are now either done
in Kudu's own cmake files or that can be removed due to Impala
deprecating support for older OS versions in the 3.x line.

The removed checks are:

HAVE_KRB5_GET_INIT_CREDS_OPT_SET_OUT_CCACHE:
    We now check for this in Kudu's code.

HAVE_KRB5_IS_CONFIG_PRINCIPAL,
HAVE_KRB5_GET_INIT_CREDS_OPT_SET_FAST_CCACHE_NAME:
    These checks are not needed anymore. All OS versions supported by
    Impala now have sufficiently recent versions of Kerberos.

Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Reviewed-on: http://gerrit.cloudera.org:8080/10759
Reviewed-by: Lars Volker <lv...@cloudera.com>
Tested-by: Lars Volker <lv...@cloudera.com>
---
M be/CMakeLists.txt
M be/src/common/config.h.in
2 files changed, 0 insertions(+), 9 deletions(-)

Approvals:
  Lars Volker: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 13
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>

[Impala-ASF-CR] IMPALA-7006: [KSECURITY] Update security library integration

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10759 )

Change subject: IMPALA-7006: [KSECURITY] Update security library integration
......................................................................


Patch Set 12: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 12
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Jul 2018 21:35:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7006: [KSECURITY] Update security library integration

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Henry Robinson, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10759

to look at the new patch set (#9).

Change subject: IMPALA-7006: [KSECURITY] Update security library integration
......................................................................

IMPALA-7006: [KSECURITY] Update security library integration

This commit is part of a set of changes for IMPALA-7006. It started
based on an original change (f51c4435), which integrated Kudu's security
folder into our build.

This change removes several compile time checks that are now either done
in Kudu's own cmake files or that can be removed due to Impala
deprecating support for older OS versions in the 3.x line.

The removed checks are:

HAVE_KRB5_GET_INIT_CREDS_OPT_SET_OUT_CCACHE:
    We now check for this in Kudu's code.

HAVE_KRB5_IS_CONFIG_PRINCIPAL,
HAVE_KRB5_GET_INIT_CREDS_OPT_SET_FAST_CCACHE_NAME:
    These checks are not needed anymore. All OS versions supported by
    Impala now have sufficiently recent versions of Kerberos.

Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
---
M be/CMakeLists.txt
M be/src/common/config.h.in
2 files changed, 0 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/10759/9
-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>

[Impala-ASF-CR] IMPALA-4669: [KSECURITY] Add security library to build

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10759 )

Change subject: IMPALA-4669: [KSECURITY] Add security library to build
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10759/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10759/8//COMMIT_MSG@13
PS8, Line 13: The original commit message is below:
Does the following need updating given this new patch no longer matches what's described below ?



-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 17:44:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7006: [KSECURITY] Update security library integration

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10759 )

Change subject: IMPALA-7006: [KSECURITY] Update security library integration
......................................................................


Patch Set 10: Code-Review+2

Rebased, carrying Michael's +2.


-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:04:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4669: [KSECURITY] Add security library to build

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10759 )

Change subject: IMPALA-4669: [KSECURITY] Add security library to build
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10759/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10759/8//COMMIT_MSG@13
PS8, Line 13: The original commit message is below:
> I tried to keep the commit messages the same. The alternative seems to be r
I see. This patch along with the other 2 patches for rpc and util directories  aim to get the kudu code to compile in Impala so they seem to be different from other cherry-picks. This patch has evolved since the original commit because we have pushed some of the changes into the Kudu code base. 

So, may make sense to me to actually document what we do in this patch. I find the the original commit message stale now as this patch doesn't actually do certain things claimed in the commit message.

What do others think ?



-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 19:52:36 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4669: [KSECURITY] Add security library to build

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Henry Robinson, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10759

to look at the new patch set (#8).

Change subject: IMPALA-4669: [KSECURITY] Add security library to build
......................................................................

IMPALA-4669: [KSECURITY] Add security library to build

NOTE: This commit is part of a set of changes for IMPALA-7006. It
contains pieces of a previous commit that need to be cherry picked
again after rebasing the code in be/src/kudu/{util,security,rpc}.

The original commit message is below:

* Minor compilation fix
* Add krb5 as a non-toolchain dependency
* Handle legacy versions of libkrb5.so by providing implementation of
  krb5_is_config_principal().
* Link against openssl from the toolchain if 1.0.0 or higher not found
  on build machine.
* Update LICENSE.txt and NOTICE.txt re: OpenSSL code in x509_check_host.{h,c}.

Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Reviewed-on: http://gerrit.cloudera.org:8080/5717
Reviewed-by: Henry Robinson <he...@cloudera.com>
Tested-by: Henry Robinson <he...@cloudera.com>
---
M be/CMakeLists.txt
M be/src/common/config.h.in
2 files changed, 0 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/10759/8
-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>

[Impala-ASF-CR] IMPALA-7006: [KSECURITY] Update security library integration

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10759 )

Change subject: IMPALA-7006: [KSECURITY] Update security library integration
......................................................................


Patch Set 12: Code-Review+2

Rebased, carrying Michael's +2.


-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 12
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Jul 2018 17:23:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4669: [KSECURITY] Add security library to build

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Henry Robinson, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10759

to look at the new patch set (#2).

Change subject: IMPALA-4669: [KSECURITY] Add security library to build
......................................................................

IMPALA-4669: [KSECURITY] Add security library to build

* Minor compilation fix
* Add krb5 as a non-toolchain dependency
* Handle legacy versions of libkrb5.so by providing implementation of
  krb5_is_config_principal().
* Link against openssl from the toolchain if 1.0.0 or higher not found
  on build machine.
* Update LICENSE.txt and NOTICE.txt re: OpenSSL code in x509_check_host.{h,c}.

Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Reviewed-on: http://gerrit.cloudera.org:8080/5717
Reviewed-by: Henry Robinson <he...@cloudera.com>
Tested-by: Henry Robinson <he...@cloudera.com>
---
M be/CMakeLists.txt
M be/src/common/config.h.in
M be/src/kudu/security/init.cc
3 files changed, 12 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/10759/2
-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>

[Impala-ASF-CR] IMPALA-4669: [KSECURITY] Add security library to build

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Henry Robinson, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10759

to look at the new patch set (#6).

Change subject: IMPALA-4669: [KSECURITY] Add security library to build
......................................................................

IMPALA-4669: [KSECURITY] Add security library to build

* Minor compilation fix
* Add krb5 as a non-toolchain dependency
* Handle legacy versions of libkrb5.so by providing implementation of
  krb5_is_config_principal().
* Link against openssl from the toolchain if 1.0.0 or higher not found
  on build machine.
* Update LICENSE.txt and NOTICE.txt re: OpenSSL code in x509_check_host.{h,c}.

Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Reviewed-on: http://gerrit.cloudera.org:8080/5717
Reviewed-by: Henry Robinson <he...@cloudera.com>
Tested-by: Henry Robinson <he...@cloudera.com>
---
M be/CMakeLists.txt
M be/src/common/config.h.in
2 files changed, 0 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/10759/6
-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 6
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>

[Impala-ASF-CR] IMPALA-7006: [KSECURITY] Update security library integration

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10759 )

Change subject: IMPALA-7006: [KSECURITY] Update security library integration
......................................................................


Patch Set 8:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/10759/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/10759/8//COMMIT_MSG@13
PS8, Line 13: The original commit message is below:
> I see. This patch along with the other 2 patches for rpc and util directori
I updated the commit message.



-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 20:24:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4669: [KSECURITY] Add security library to build

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has removed Henry Robinson from this change.  ( http://gerrit.cloudera.org:8080/10759 )

Change subject: IMPALA-4669: [KSECURITY] Add security library to build
......................................................................


Removed reviewer Henry Robinson.
-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>

[Impala-ASF-CR] IMPALA-7006: [KSECURITY] Update security library integration

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10759 )

Change subject: IMPALA-7006: [KSECURITY] Update security library integration
......................................................................


Patch Set 9: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 01:58:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4669: [KSECURITY] Add security library to build

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10759 )

Change subject: IMPALA-4669: [KSECURITY] Add security library to build
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10759/1/be/src/kudu/security/init.cc
File be/src/kudu/security/init.cc:

http://gerrit.cloudera.org:8080/#/c/10759/1/be/src/kudu/security/init.cc@244
PS1, Line 244: HAVE_KRB5_IS_CONFIG_PRINCIPAL
Applicable to Kudu too ?


http://gerrit.cloudera.org:8080/#/c/10759/1/be/src/kudu/security/init.cc@316
PS1, Line 316: HAVE_KRB5_GET_INIT_CREDS_OPT_SET_OUT_CCACHE
This may be ported to Kudu code too as it seems to make it clearer what we are missing from the Kerberos library.



-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Jun 2018 23:36:05 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4669: [KSECURITY] Add security library to build

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Henry Robinson, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10759

to look at the new patch set (#3).

Change subject: IMPALA-4669: [KSECURITY] Add security library to build
......................................................................

IMPALA-4669: [KSECURITY] Add security library to build

* Minor compilation fix
* Add krb5 as a non-toolchain dependency
* Handle legacy versions of libkrb5.so by providing implementation of
  krb5_is_config_principal().
* Link against openssl from the toolchain if 1.0.0 or higher not found
  on build machine.
* Update LICENSE.txt and NOTICE.txt re: OpenSSL code in x509_check_host.{h,c}.

Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Reviewed-on: http://gerrit.cloudera.org:8080/5717
Reviewed-by: Henry Robinson <he...@cloudera.com>
Tested-by: Henry Robinson <he...@cloudera.com>
---
M be/CMakeLists.txt
M be/src/common/config.h.in
M be/src/kudu/security/init.cc
3 files changed, 12 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/59/10759/3
-- 
To view, visit http://gerrit.cloudera.org:8080/10759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifab51d887f5e771ad62eeddc14b9c47f42c3130d
Gerrit-Change-Number: 10759
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>