You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2017/02/24 18:31:38 UTC

[kudu-CR] [system catalog] an option to reset CA entries in system table

Alexey Serbin has uploaded a new change for review.

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

Change subject: [system_catalog] an option to reset CA entries in system table
......................................................................

[system_catalog] an option to reset CA entries in system table

Introduced a command-line flag to reset CA entries in the system catalog
table. The flag is marked as 'hidden' and 'unsafe'.

To reset IPKI CA entries in the system table, run the master server with
the flag set (--ipki_reset_ca=true) once.

Change-Id: I9c9cdab5f6a2887304f60705d2945d1462c369bc
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master_cert_authority.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
5 files changed, 50 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/6135/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I9c9cdab5f6a2887304f60705d2945d1462c369bc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] [system catalog] an option to reset CA entries in system table

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: [system_catalog] an option to reset CA entries in system table
......................................................................


Patch Set 1:

> > This is a little funny as a flag, since if you're running
 > > multi-master, and you start with this, you'll end up with a new
 > CA,
 > > and then on first fail-over, the second master will also create a
 > > new CA.
 > 
 > I'm also not a fan of one-shot flags. In my opinion we should
 > eliminate existing ones and avoid introducing new ones, so I'd also
 > be in favor of fashioning this into a CLI tool instead.

I agree -- it's better to have this functionality somewhere else.  Otherwise it's just a piece of code which is going to be used 1e-100 of all runtime, and that does not make much sense.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9c9cdab5f6a2887304f60705d2945d1462c369bc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [system catalog] an option to reset CA entries in system table

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: [system_catalog] an option to reset CA entries in system table
......................................................................


Patch Set 1:

This is a little funny as a flag, since if you're running multi-master, and you start with this, you'll end up with a new CA, and then on first fail-over, the second master will also create a new CA.

A couple ideas:

1) an offline tool would be "safer" as a one-shot? though if it's a lot of work to build, probably not worth it
2) what if we added a log line on "LoadCertificateAuthority" which output some kind of unique ID for the loaded cert? And then the flag could be --remove_cert_id=<cert>. That would make it safe to leave the flag set, since on failover the new master would see that the cert had already been deleted?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9c9cdab5f6a2887304f60705d2945d1462c369bc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [system catalog] an option to reset CA entries in system table

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change.

Change subject: [system_catalog] an option to reset CA entries in system table
......................................................................


Patch Set 1:

> This is a little funny as a flag, since if you're running
 > multi-master, and you start with this, you'll end up with a new CA,
 > and then on first fail-over, the second master will also create a
 > new CA.
 > 

Right.  This looks like a hack and really it is :)  This is the patch that I put up yesterday to quickly fix an issue JD had on his cluster.

 > A couple ideas:
 > 
 > 1) an offline tool would be "safer" as a one-shot? though if it's a
 > lot of work to build, probably not worth it

I think an additional off-line tool is much better, of course.

 > 2) what if we added a log line on "LoadCertificateAuthority" which
 > output some kind of unique ID for the loaded cert? And then the
 > flag could be --remove_cert_id=<cert>. That would make it safe to
 > leave the flag set, since on failover the new master would see that
 > the cert had already been deleted?

That might be a better approach as well.  However, it requires to know what certificates you have there.  And stand-alone tool could be much better anyway.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9c9cdab5f6a2887304f60705d2945d1462c369bc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] NOT FOR COMMIT: [system catalog] an option to reset CA entries in system table

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,

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

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

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

Change subject: NOT FOR COMMIT: [system_catalog] an option to reset CA entries in system table
......................................................................

NOT FOR COMMIT: [system_catalog] an option to reset CA entries in system table

Introduced a command-line flag to reset CA entries in the system catalog
table. The flag is marked as 'hidden' and 'unsafe'.

To reset IPKI CA entries in the system table, run the master server with
the flag set (--ipki_reset_ca=true) once.

Change-Id: I9c9cdab5f6a2887304f60705d2945d1462c369bc
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master_cert_authority.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
5 files changed, 51 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/35/6135/2
-- 
To view, visit http://gerrit.cloudera.org:8080/6135
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9c9cdab5f6a2887304f60705d2945d1462c369bc
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [system catalog] an option to reset CA entries in system table

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has abandoned this change.

Change subject: [system_catalog] an option to reset CA entries in system table
......................................................................


Abandoned

If needed, should be done as a separate utility.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: I9c9cdab5f6a2887304f60705d2945d1462c369bc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] [system catalog] an option to reset CA entries in system table

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.

Change subject: [system_catalog] an option to reset CA entries in system table
......................................................................


Patch Set 1:

> This is a little funny as a flag, since if you're running
 > multi-master, and you start with this, you'll end up with a new CA,
 > and then on first fail-over, the second master will also create a
 > new CA.

I'm also not a fan of one-shot flags. In my opinion we should eliminate existing ones and avoid introducing new ones, so I'd also be in favor of fashioning this into a CLI tool instead.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9c9cdab5f6a2887304f60705d2945d1462c369bc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [system catalog] an option to reset CA entries in system table

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has restored this change.

Change subject: [system_catalog] an option to reset CA entries in system table
......................................................................


Restored

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

Gerrit-MessageType: restore
Gerrit-Change-Id: I9c9cdab5f6a2887304f60705d2945d1462c369bc
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins