I don't know why - I just ported this from master to sentry HA branch. I
guess we do not really support anything other then ALL on URI privileges -
there is no concept of "insert" or "select" on URI, so ALL is the only
thing that makes sense, but I don't think it is shell's business to deal
with it.
On Thu, Oct 5, 2017 at 2:06 AM, Colm O hEigeartaigh <cohei...@apache.org>
wrote:
> OK I can add a switch to call the validation code for the shell only. Do
> you know why the CommandUtil includes:
>
> tSentryPrivilege.setAction(AccessConstants.ALL);
>
> for the URI case? This looks a bit dodgy to me as it would override the
> value parsed via PRIVILEGE_ACTION_NAME.
>
> Colm.
>
> On Wed, Oct 4, 2017 at 11:37 PM, Alexander Kolbasov <ak...@cloudera.com>
> wrote:
>
> > Interesting - both were introduced by Colin Ma. I definitely agree that
> we
> > do not need two duplicates of the same similar functionality. The
> exception
> > part is interesting - the validator throws unchecked exceptions.
> >
> > The version in SentryServiceUtil is used only in import policy code path
> > where validation should be always ok.
> >
> > The second one is used by shell only.
> >
> > There is another internal version in SentryStore:
> >
> > private void convertToTSentryPrivilege(MSentryPrivilege
> mSentryPrivilege,
> > TSentryPrivilege privilege) {
> > privilege.setCreateTime(mSentryPrivilege.getCreateTime());
> > privilege.setAction(fromNULLCol(mSentryPrivilege.getAction()));
> > privilege.setPrivilegeScope(mSentryPrivilege.getPrivilegeScope());
> > privilege.setServerName(fromNULLCol(mSentryPrivilege.
> getServerName()));
> > privilege.setDbName(fromNULLCol(mSentryPrivilege.getDbName()));
> > privilege.setTableName(fromNULLCol(mSentryPrivilege.getTableName()));
> > privilege.setColumnName(fromNULLCol(mSentryPrivilege.
> getColumnName()));
> > privilege.setURI(fromNULLCol(mSentryPrivilege.getURI()));
> > if (mSentryPrivilege.getGrantOption() != null) {
> > privilege.setGrantOption(TSentryGrantOption.valueOf(
> mSentryPrivilege.
> > getGrantOption().toString().toUpperCase()));
> > } else {
> > privilege.setGrantOption(TSentryGrantOption.UNSET);
> > }
> > }
> >
> >
> >
> > On Mon, Oct 2, 2017 at 10:07 AM, Colm O hEigeartaigh <
> cohei...@apache.org>
> > wrote:
> >
> > > Hi all,
> > >
> > > I'm preparing a patch to consolidate some duplicate code in
> > > sentry-provider-db, and have a question:
> > >
> > > CommandUtil.convertToTSentryPrivilege:
> > >
> > > https://github.com/apache/sentry/blob/186e75543a23f286e24c56f7385909
> > > 9405b7b73a/sentry-provider/sentry-provider-db/src/main/
> > > java/org/apache/sentry/provider/db/tools/command/
> > hive/CommandUtil.java#L38
> > >
> > > is identical to SentryServiceUtil.convertToTSentryPrivilege:
> > >
> > > https://github.com/apache/sentry/blob/186e75543a23f286e24c56f7385909
> > > 9405b7b73a/sentry-provider/sentry-provider-db/src/main/
> > > java/org/apache/sentry/service/thrift/SentryServiceUtil.java#L56
> > >
> > > apart from two differences:
> > >
> > > a) The CommandUtil version sets:
> > > tSentryPrivilege.setAction(AccessConstants.ALL); for the
> > > PRIVILEGE_URI_NAME case.
> > > b) The CommandUtil version "validates" the privilege hierarchy,
> throwing
> > an
> > > exception if one of the names is not specified.
> > >
> > > Ideally I'd like to remove CommandUtil altogether and just reference
> the
> > > methods in SentryServiceUtil. Should these extra pieces also be in
> > > SentryServiceUtil? Or is there a reason that they are different?
> > >
> > > Colm.
> > >
> > >
> > > --
> > > Colm O hEigeartaigh
> > >
> > > Talend Community Coder
> > > http://coders.talend.com
> > >
> >
>
>
>
> --
> Colm O hEigeartaigh
>
> Talend Community Coder
> http://coders.talend.com
>
