Gabriella Lotz has posted comments on this change. ( http://gerrit.cloudera.org:8080/23052 )
Change subject: SPNEGO authentication in ExternalMiniCluster ...................................................................... Patch Set 2: (10 comments) http://gerrit.cloudera.org:8080/#/c/23052/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23052/2//COMMIT_MSG@7 PS2, Line 7: [WIP] > nit: I think the patch is out of the WIP state. :) Done http://gerrit.cloudera.org:8080/#/c/23052/2//COMMIT_MSG@9 PS2, Line 9: REST API calls > nit: calls to the webserver Done http://gerrit.cloudera.org:8080/#/c/23052/2//COMMIT_MSG@18 PS2, Line 18: --spnego_keytab_file > As per the latest patchset I dont think we set --spnego_keytab_file, right? Done http://gerrit.cloudera.org:8080/#/c/23052/2/src/kudu/mini-cluster/external_mini_cluster-test.cc File src/kudu/mini-cluster/external_mini_cluster-test.cc: http://gerrit.cloudera.org:8080/#/c/23052/2/src/kudu/mini-cluster/external_mini_cluster-test.cc@348 PS2, Line 348: TEST_P(ExternalMiniClusterTest, TestRestApiSpnegoConnectionThroughCurl) { > Please add a positive and negative API call inside here: Done http://gerrit.cloudera.org:8080/#/c/23052/2/src/kudu/mini-cluster/external_mini_cluster-test.cc@351 PS2, Line 351: opts.extra_master_flags.emplace_back("--enable_rest_api"); > I think it would make sense to add a bool option to ExternalMiniClusterOpti Done http://gerrit.cloudera.org:8080/#/c/23052/2/src/kudu/mini-cluster/external_mini_cluster.cc File src/kudu/mini-cluster/external_mini_cluster.cc: http://gerrit.cloudera.org:8080/#/c/23052/2/src/kudu/mini-cluster/external_mini_cluster.cc@1476 PS2, Line 1476: // --------------------------------------------------------------------------- > nit: here and below: I think we dont need the ascii frame around the commen Done http://gerrit.cloudera.org:8080/#/c/23052/2/src/kudu/mini-cluster/external_mini_cluster.cc@1490 PS2, Line 1490: Web UI > nit: webserver Done http://gerrit.cloudera.org:8080/#/c/23052/2/src/kudu/mini-cluster/external_mini_cluster.cc@1498 PS2, Line 1498: const auto kdc_env = kdc->GetEnvVars(); : env_vars->insert(kdc_env.begin(), kdc_env.end()); > nit: in the end we did not end up modifying the kdc_env vars. therefore we Done http://gerrit.cloudera.org:8080/#/c/23052/2/src/kudu/mini-cluster/external_mini_cluster.cc@1505 PS2, Line 1505: flags->emplace_back(Substitute("--keytab_file=$0", kt_path)); : flags->emplace_back(Substitute("--principal=$0", spn)); : flags->emplace_back("--rpc_authentication=required"); : flags->emplace_back("--superuser_acl=test-admin"); : flags->emplace_back("--user_acl=test-user"); > nit: here + the webserver flag below: Done http://gerrit.cloudera.org:8080/#/c/23052/2/src/kudu/mini-cluster/external_mini_cluster.cc@1511 PS2, Line 1511: Web UI > nit: webserver Done -- To view, visit http://gerrit.cloudera.org:8080/23052 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6035bdf7f2f0f08f53485a7a1aa82ed26148cc4 Gerrit-Change-Number: 23052 Gerrit-PatchSet: 2 Gerrit-Owner: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Gabriella Lotz <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Comment-Date: Tue, 24 Jun 2025 13:50:31 +0000 Gerrit-HasComments: Yes
