Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AMBARI-24672] Make sure we regenerate all service specific keytabs on all hosts where they are needed #2359

Merged
merged 1 commit into from
Sep 27, 2018

Conversation

smolnar82
Copy link
Contributor

What changes were proposed in this pull request?

In case there is a component that requires another service's headless keytab and there is no any component installed of that service the service level keytab regeneration did not work properly: the headless keytab was not regenerated.

How was this patch tested?

Latest unit test results in ambari-server:

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 31:08 min
[INFO] Finished at: 2018-09-21T15:17:50+02:00
[INFO] Final Memory: 170M/1245M
[INFO] ------------------------------------------------------------------------

Additionally the following E2E test steps were executed:

  1. installed a secure cluster with HDFS and TEZ (+other dependencies) using Ambari 2.7.1.0-163
  2. regenerated all keytabs (Kerberos / Regenerate Keytabs): no errors; all keytab files were updated on the disk
  3. removed HDFS from host 3 client where Tez Client was installed; please note that Tez Client is mapped to the headless keytab of HDFS at this point
  4. regenerate keytabs for HDFS only: on host 3 only Datanode's keytab was regenerated
  5. built the new binary, replaced ambari-server-x.jar and restarted the server then repeated the last step: all relevant keytabs on all hosts were updates properly (including the HDFS headless keytab on host 3)

@smolnar82 smolnar82 requested review from rlevas, a user and zeroflag September 21, 2018 13:31
@smolnar82 smolnar82 self-assigned this Sep 21, 2018

Set<ResolvedKerberosKeytab> keytabsToInject = kerberosKeytabController.getFilteredKeytabs(serviceComponentFilter, kerberosCommandParameters.getHostFilter(), kerberosCommandParameters.getIdentityFilter());
if (serviceComponentFilter != null) {
keytabsToInject.addAll(kerberosKeytabController.findKeytabsForServiceIdentities(executionCommand.getClusterName(), serviceComponentFilter));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this will negate any host filter. Have you tested regenerating keytab files for a specific host? Also, does this break any operations related to cleaning up identities?

When I was testing this for a different patch, I believe that the kerberosKeytabController.getFilteredKeytabs call above found both service and user (headless) principals related to a given service.

Copy link
Contributor Author

@smolnar82 smolnar82 Sep 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is true - kerberosKeytabController.getFilteredKeytabs finds both service and user principals - only if the given service has any components requiring the user principal on that host.
I can reproduce this issue with 2.7.1.0-163 (see description above)

This was not the case in the cluster where QE found the issue: they created the cluster with BP - topology validation was switched off - with a host group where Druid Historical was present but no HDFS Client

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeorg.apache.ambari.server.orm.dao.KerberosKeytabPrincipalDAO.findByFilter(KerberosKeytabPrincipalFilter filter): since the given filter is populated with service name = HDFS the underlying DB query will find only the principals for the Datanode (which does not need the HDFS user principal)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which means that we may convert everything in the service name filter to the principal filter and do not check the service name at all.

final Collection<KerberosIdentityDescriptor> serviceIdentities = serviceComponentFilter == null ? null : calculateServiceIdentities(getClusterName(), serviceComponentFilter);
for (ResolvedKerberosKeytab rkk : kerberosKeytabController.getFilteredKeytabs(serviceComponentFilter, getHostFilter(), getIdentityFilter())) {
final Collection<KerberosIdentityDescriptor> serviceIdentities = serviceComponentFilter == null ? null : kerberosKeytabController.calculateServiceIdentities(getClusterName(), serviceComponentFilter);
final Set<ResolvedKerberosKeytab> filteredKeytabs = kerberosKeytabController.getFilteredKeytabs(serviceComponentFilter, getHostFilter(), getIdentityFilter());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not clear why this is not pulling back all of the relevant ResolvedKerberosKeytab items. If the Kerberos descriptor for TEZ_CLIENT indicates that it needs the HDFS headless keytab file, there should be a set of records in the Ambari database mapping the HDFS principal name and keytab file to TEZ_CLIENT. If so, then this filter should return the correct items. If not, maybe the problem is there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SELECT * FROM kerberos_keytab_principal inner join kkp_mapping_service using (kkp_id) where principal_name='hdfs-c1@EXAMPLE.COM';
59	/etc/security/keytabs/hdfs.headless.keytab	hdfs-c1@EXAMPLE.COM	1	1	HDFS	NAMENODE
59	/etc/security/keytabs/hdfs.headless.keytab	hdfs-c1@EXAMPLE.COM	1	1	HDFS	HDFS_CLIENT
59	/etc/security/keytabs/hdfs.headless.keytab	hdfs-c1@EXAMPLE.COM	1	1	MAPREDUCE2	HISTORYSERVER
59	/etc/security/keytabs/hdfs.headless.keytab	hdfs-c1@EXAMPLE.COM	1	1	TEZ	TEZ_CLIENT
59	/etc/security/keytabs/hdfs.headless.keytab	hdfs-c1@EXAMPLE.COM	1	1	YARN	APP_TIMELINE_SERVER
59	/etc/security/keytabs/hdfs.headless.keytab	hdfs-c1@EXAMPLE.COM	1	1	YARN	TIMELINE_READER

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it simply queries the DB (where we have no information about the Kerberos descriptor unless the user submitted one) based on the service filter we passed (i.e HDFS):

SELECT kkp.*, h.host_name, kkpm.service_name, kkpm.component_name
FROM kerberos_keytab_principal kkp, hosts h, kkp_mapping_service kkpm
WHERE kkp.host_id = h.host_id 
AND kkp.kkp_id = kkpm.kkp_id
AND kkpm.service_name = 'HDFS' 
AND h.host_name = 'c7403.ambari.apache.org'
ORDER BY h.host_name, kkpm.service_name, kkpm.component_name;

Returns:

kkp_id keytab_path principal_name host_id is_distributed host_name service_name component_name
6 /etc/security/keytabs/spnego.service.keytab HTTP/c7403.ambari.apache.org@AMBARI.APACHE.ORG 3 1 c7403.ambari.apache.org HDFS DATANODE
30 /etc/security/keytabs/smokeuser.headless.keytab ambari-qa-cluster1@AMBARI.APACHE.ORG 3 1 c7403.ambari.apache.org HDFS DATANODE
37 /etc/security/keytabs/dn.service.keytab dn/c7403.ambari.apache.org@AMBARI.APACHE.ORG 3 1 c7403.ambari.apache.org HDFS DATANODE

This is why it would only regenerate DN's keytab.

However we know that other service components needs the headless keytab:

SELECT kkp.*, h.host_name, kkpm.service_name, kkpm.component_name
FROM kerberos_keytab_principal kkp, hosts h, kkp_mapping_service kkpm
WHERE kkp.host_id = h.host_id 
AND kkp.kkp_id = kkpm.kkp_id
AND kkp.principal_name = 'hdfs-cluster1@AMBARI.APACHE.ORG' 
AND h.host_name = 'c7403.ambari.apache.org'
ORDER BY h.host_name, kkpm.service_name, kkpm.component_name

Returns:

kkp_id keytab_path principal_name host_id is_distributed host_name service_name component_name
15 /etc/security/keytabs/hdfs.headless.keytab hdfs-cluster1@AMBARI.APACHE.ORG 3 1 c7403.ambari.apache.org TEZ TEZ_CLIENT
15 /etc/security/keytabs/hdfs.headless.keytab hdfs-cluster1@AMBARI.APACHE.ORG 3 1 c7403.ambari.apache.org AMBARI_METRICS METRICS_COLLECTOR

This is why we need to add other services; let me think it over again; there may be a more elegant and easy way to solve this issue.

@@ -54,6 +58,13 @@
@Inject
private KerberosKeytabPrincipalDAO kerberosKeytabPrincipalDAO;

//TODO: due to circular dependencies in Guice this field cannot be injected with Guice's @Inject annotation; for now we should statically inject in AmbariServer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems pretty bad..

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about injecting a Provider of KerberosHelper? That would allow lazy injection

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would also work (as described here); but - IMO - Guice providers are just hiding cyclic dependencies which is the real issue here.
In this case it's not better or worse than the solution I applied (we have plenty of these in the code anyway); one advantage would be is to not add another static injection.

Long story short: I'll submit a new patch in 5-10 mins.

@asfgit
Copy link

asfgit commented Sep 21, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/4030/
Test PASSed.

…ntities instead of service/component filtering
@asfgit
Copy link

asfgit commented Sep 24, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/4039/
Test PASSed.

@smolnar82
Copy link
Contributor Author

@rlevas @zeroflag @dlysnichenko
Please review when you have time.
Thanks!

@@ -54,6 +58,13 @@
@Inject
private KerberosKeytabPrincipalDAO kerberosKeytabPrincipalDAO;

//TODO: due to circular dependencies in Guice this field cannot be injected with Guice's @Inject annotation; for now we should statically inject in AmbariServer
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about injecting a Provider of KerberosHelper? That would allow lazy injection

@smolnar82
Copy link
Contributor Author

smolnar82 commented Sep 25, 2018

@dlysnichenko

I just arrived to change the code to have a provider here (in KerberosKeytabController). Unfortunately we have hundreds of references to KerberosHelper in our code with simple @Inject which should be changed to a provider since we can not use both (you either bind the interface to a class or you bind it to a provider; you cannot have both). In this case this would mean hundreds of changes; with so many modifications we would lose the main point of this PR.
So that I leave my patch as is...

@rlevas
Any objection?

@smolnar82
Copy link
Contributor Author

FYI: to address the circular dependency issue with KerberosHelper I created a new JIRA.

@smolnar82 smolnar82 merged commit 91f1902 into apache:trunk Sep 27, 2018
vishalsuvagia pushed a commit to vishalsuvagia/ambari that referenced this pull request Oct 1, 2018
vishalsuvagia pushed a commit to vishalsuvagia/ambari that referenced this pull request Oct 1, 2018
* upstream/trunk: (99 commits)
  AMBARI-24708. Automation script for upgrade old style isilon cluster to the new mpack based structure (amagyar) (apache#2397)
  AMBARI-24700 - Slider widget: insert a space char between label value and unit-name (apache#2399)
  AMBARI-24245. Fix FindBugs warnings (apache#2398)
  AMBARI-24707. Build fails due to missing ambari-utility (apache#2395)
  [AMBARI-24706] Fix issues in ambari common python package publishing utility. (apache#2394)
  AMBARI-24669. Component status can stuck in starting/stopping status on heartbeat lost. (mpapirkovskyy) (apache#2385)
  [AMBARI-24696] UI: Options Page to choose Rolling or Express Restart. (apache#2389)
  AMBARI-24466. Add README.md for Ambari project (apache#2390)
  AMBARI-24672. Regenrating keytabs for services using all of their identities instead of service/component filtering (apache#2359)
  [AMBARI-24695] Remove ambari-metrics from ambari repository. (apache#2388)
  [AMBARI-24632] APT/DPKG existence check doesn't work for system packages (dgrinenko) (apache#2366)
  AMBARI-24693. Remove ambari-infra and ambari-logsearch from ambari repository. (apache#2387)
  [AMBARI-24691] Refactor Upgrade Classes to Proper Packages (apache#2386)
  AMBARI-24690. Ambari-server setup-ldap throws an error when the OU has spaces (amagyar) (apache#2382)
  [AMBARI-24685] - Create a Maven Consumable Ambari SPI Client Library
  [AMBARI-24687] Upgrade Pack Should Allow Source Information (apache#2375)
  [AMBARI-24682] Rolling Restarts: Option to specify number of failures per batch (dsen) (apache#2371)
  AMBARI-24670. Directory creation sometimes fails with parallel_execution=1 (aonishuk)
  AMBARI-24670. Directory creation sometimes fails with parallel_execution=1 (aonishuk)
  AMBARI-24689. All component statuses should be re-send on registration (aonishuk)
  ...

# Conflicts:
#	README.md
#	ambari-common/src/main/python/resource_management/libraries/functions/module_version.py
#	ambari-common/src/main/python/resource_management/libraries/functions/mpack_version.py
#	ambari-server-spi/pom.xml
#	ambari-server/src/test/python/TestVersion.py
#	ambari-server/src/test/resources/stacks/HDP/2.2.0/upgrades/upgrade_with_source.xml
#	ambari-web/test/controllers/global/cluster_controller_test.js
vishalsuvagia pushed a commit to vishalsuvagia/ambari that referenced this pull request Nov 14, 2018
* origin/trunk: (109 commits)
  AMBARI-24888 fix SingleHostTopologyUpdater (benyoka) (apache#2603)
  [AMBARI-24892] Warn During Upgrade When Plugin ClassLoader is not Found (apache#2604)
  AMBARI-24890 Restart option should not be shown if service components are not created in a cluster
  AMBARI-24877 Heatmap tab, metrics tab and QuickLinks section in summary tab should be hidden if service has only client service component
  AMBARI-24889. Pre-Upgrade Checklist: Components are Installed
  AMBARI-24881 Add Service Request JSON (benyoka) (apache#2594)
  AMBARI-24572 Fixing the cluster name alignment (sjanardhanhw via ababiichuk)
  AMBARI-24603 Better scrolling for long list of components in dropdown in host details page (sjanardhanhw via ababiichuk)
  Revert space changes.
  AMBARI-24886 : Add stack feature constant to support Ranger admin users password change.
  resolving merge issue
  AMBARI-24700 - Slider widget: insert a space char between label value and unit-name (apache#2399)
  AMBARI-24245. Fix FindBugs warnings (apache#2398)
  AMBARI-24707. Build fails due to missing ambari-utility (apache#2395)
  [AMBARI-24706] Fix issues in ambari common python package publishing utility. (apache#2394)
  AMBARI-24669. Component status can stuck in starting/stopping status on heartbeat lost. (mpapirkovskyy) (apache#2385)
  [AMBARI-24696] UI: Options Page to choose Rolling or Express Restart. (apache#2389)
  AMBARI-24466. Add README.md for Ambari project (apache#2390)
  AMBARI-24672. Regenrating keytabs for services using all of their identities instead of service/component filtering (apache#2359)
  [AMBARI-24695] Remove ambari-metrics from ambari repository. (apache#2388)
  ...
@smolnar82 smolnar82 deleted the AMBARI-24672 branch January 31, 2019 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants