Skip to content

Ambari-25333 branch 2.7#3046

Closed
apappu wants to merge 3 commits intoapache:branch-2.7from
apappu:AMBARI-25333-branch-2.7
Closed

Ambari-25333 branch 2.7#3046
apappu wants to merge 3 commits intoapache:branch-2.7from
apappu:AMBARI-25333-branch-2.7

Conversation

@apappu
Copy link
Contributor

@apappu apappu commented Jul 10, 2019

What changes were proposed in this pull request?

  1. Checking if the file present/exist in the file system - if not re-creating the keytab file.

(Please fill in changes proposed in this fix)

  1. Checking if the file present/exist in the file system - if not re-creating the keytab file.

How was this patch tested?

  1. Deployed the changes in Kerberos cluster, deleted the files under ambari-server/data/cache folder and tried to re-generate keytabs for missing. it has re-created the files.

(Please explain how this patch was tested. Ex: unit tests, manual tests)
(If this patch involves UI changes, please attach a screen-shot; otherwise, remove this)

Please review Ambari Contributing Guide before opening a pull request.

@apappu apappu changed the title Ambari 25333 branch 2.7 Ambari-25333 branch 2.7 Jul 10, 2019
@asfgit
Copy link

asfgit commented Jul 10, 2019

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

} else {
try {
if(regenerateKeytabs) {
Keytab keytab = createKeytab(resolvedPrincipal.getPrincipal(), password, keyNumber, operationHandler, visitedPrincipalKeys != null, true, actionLog);
Copy link
Contributor

Choose a reason for hiding this comment

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

we can't create here a keytab as password would be always null and keytab would be invalid at the end

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the proper place to do the change is CreatePrincipalsServerAction, there is a place like:

      } else if (!StringUtils.isEmpty(kerberosPrincipalEntity.getCachedKeytabPath())) {
        // This principal has been processed and a keytab file has been cached for it... do not process it.
        processPrincipal = false;
      } else {

if we change it to something like:

      } else if (!StringUtils.isEmpty(kerberosPrincipalEntity.getCachedKeytabPath())) {
        File file = new File(kerberosPrincipalEntity.getCachedKeytabPath());

        // Process the principal if the cache is missing, otherwise skip it
        processPrincipal = !file.exists();
      } else {

It would process with the the proper keytab re-generation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hapylestat I thought changing CreateKeytab operation code is more relevant - in the above recommendation we are touching the CreatePrincipleAction right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can't create here a keytab as password would be always null and keytab would be invalid at the end

Look like we don't need password here - see https://github.com/apache/ambari/blob/branch-2.7/ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KDCKerberosOperationHandler.java#L177

Copy link
Contributor

Choose a reason for hiding this comment

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

for Kerberos yes, but check please code for non-Kerberos handlers

@apappu apappu closed this Jul 15, 2019
@apappu apappu deleted the AMBARI-25333-branch-2.7 branch July 15, 2019 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants