[Bug] Dupllicated AnalyzesTablesCommand in kyuubi authz

[AngersZhuuuu]

Code of Conduct

• I agree to follow this project's Code of Conduct

Search before asking

• I have searched in the issues and found no similar issues.

Describe the bug

kyuubi/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilder.scala

Lines 159 to 187 in 1a69772

	plan.getClass.getName match {
	case classname if DB_COMMAND_SPECS.contains(classname) =>
	val desc = DB_COMMAND_SPECS(classname)
	desc.databaseDescs.foreach { databaseDesc =>
	try {
	val database = databaseDesc.extract(plan)
	if (databaseDesc.isInput) {
	inputObjs += PrivilegeObject(database)
	} else {
	outputObjs += PrivilegeObject(database)
	}
	} catch {
	case e: Exception =>
	LOG.debug(databaseDesc.error(plan, e))
	}
	}
	desc.operationType
	case classname if TABLE_COMMAND_SPECS.contains(classname) =>
	val spec = TABLE_COMMAND_SPECS(classname)
	spec.tableDescs.foreach { td =>
	if (td.isInput) {
	inputObjs ++= getTablePriv(td)
	} else {
	outputObjs ++= getTablePriv(td)
	}
	}
	spec.queries(plan).foreach(buildQuery(_, inputObjs, spark = spark))
	spec.operationType

As the code listed above, duplicated AnalyzeTablesCommand in authz seems to be an issue, it won't be processed more than one time.

additionally, the extractor is incorrect.

kyuubi/extensions/spark/kyuubi-spark-authz/src/main/resources/table_command_spec.json

Lines 912 to 921 in 1a69772

	"classname" : "org.apache.spark.sql.execution.command.AnalyzeTablesCommand",
	"tableDescs" : [ {
	"fieldName" : "tableIdent",
	"fieldExtractor" : "TableIdentifierTableExtractor",
	"columnDesc" : null,
	"actionTypeDesc" : null,
	"tableTypeDesc" : null,
	"catalogDesc" : null,
	"isInput" : true,
	"setCurrentDatabaseIfMissing" : false

case class AnalyzeTablesCommand(
    databaseName: Option[String],
    noScan: Boolean) extends LeafRunnableCommand {
  ...
}

Affects Version(s)

master

Kyuubi Server Log Output

No response

Kyuubi Engine Log Output

No response

Kyuubi Server Configurations

No response

Kyuubi Engine Configurations

No response

Additional context

No response

Are you willing to submit PR?

• Yes. I would be willing to submit a PR with guidance from the Kyuubi community to fix.
• No. I cannot submit a PR at this time.

[github-actions]

Hello @AngersZhuuuu,
Thanks for finding the time to report the issue!
We really appreciate the community's efforts to improve Apache Kyuubi.

[pan3793]

quoted from #5270 (comment)

It seems that they are applied to the same plan of analyzing tables in db, extracting privilege objects for both Databases and Tables.

it looks like it's by design the AnalyzeTablesCommand should be present in both database_command_list.json and table_command_list.json, we should not simply remove it from table_command_list.json.

[pan3793]

When taking a look deeply into the code, the execution of AnalyzeTablesCommand actually triggers the read privilege checking of each table, thus, even AnalyzeTablesCommand listed in table_command_spec.json does not take effects, table privilege still to be checked, this is somewhat of a coincidence.

AnalyzeTablesCommand#run => CommandUtils.analyzeTable => sparkSession.table(tableIdentWithDB).count()

[pan3793]

Currently, Analyze*Command defined isIntput=true, which means only read privilege is required, but actually, those commands will update the table properties, I suppose they should be treated as isInput=false.

With the above assumption, AnalyzeTablesCommand should also manually check the user has both the write and read privileges of each table under the databases, instead of just removing it from table_command_spec.json

[pan3793]

@yaooqinn @bowenliang123 @zhouyifan279 WDYT?

[bowenliang123]

Currently, Analyze*Command defined isIntput=true, which means only read privilege is required, but actually, those commands will update the table properties, I suppose they should be treated as isInput=false.

With the above assumption, AnalyzeTablesCommand should also manually check the user has both the write and read privileges of each table under the databases, instead of just removing it from table_command_spec.json

SGTM. Agree with your suggestions of 1 and 2 based on the discussions.