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

PAYARA-4031 Master Password Synchronisation Inconsistencies Across Nodes #4133

Merged
merged 8 commits into from Aug 13, 2019

Conversation

@MattGill98
Copy link
Member

commented Aug 8, 2019

For anyone reviewing, a brief explanation of the behaviour change. The following functionality causes a problem:

  1. Create a node
  2. Create an instance on that node
  3. Change master password for domain
  4. Attempt to start instance (node still has the old master password saved, which doesn't unlock the keystore as the keystore has been synchronised from the DAS to the instance, so must be unlocked using the DAS master password)
  5. Attempt to change master password for node (can't as the change-master-password command for the node will check the password against the keystore of each instance, which has now been updated with the DAS keystore)

This PR attempts to remedy this by causing the change-master-password command, when run against a node, to only change the saved password for the node, and not touch existing keystores. This will mean that the keystores will only have their password changed when changing the master password for the DAS, and the master password should then be updated on each node to reflect this change.

@MattGill98 MattGill98 self-assigned this Aug 8, 2019

@MattGill98 MattGill98 marked this pull request as ready for review Aug 8, 2019

@MattGill98

This comment has been minimized.

Copy link
Member Author

commented Aug 8, 2019

jenkins test please

@MattGill98 MattGill98 force-pushed the MattGill98:PAYARA-4031 branch from bb38ce9 to 14b33d0 Aug 8, 2019

@MattGill98

This comment has been minimized.

Copy link
Member Author

commented Aug 8, 2019

jenkins test please

@MattGill98 MattGill98 requested review from Pandrex247 and fturizo Aug 8, 2019

@dmatej dmatej self-requested a review Aug 9, 2019

@dmatej
Copy link
Contributor

left a comment

Please add the issue id to the first line of the commit, I could not find it :)

Except that "Caught an Exception" message it works, I did this:

asadmin create-domain test-domain
asadmin start-domain test-domain
asadmin create-node-config --nodehost localhost test-local-node
asadmin create-instance --node test-local-node local-instance
asadmin start-instance local-instance
asadmin stop-instance local-instance
asadmin stop-domain test-domain
asadmin change-master-password --savemasterpassword true test-domain
Enter the current master password>
Enter the new master password> 
Enter the new master password again> 
**Caught an Exception: {0}**
Command change-master-password executed successfully.
asadmin start-domain test-domain
asadmin start-instance local-instance
remote failure: Could not start instance local-instance on node test-local-node (localhost).
asadmin change-master-password --savemasterpassword true --nodedir /app/appservers/payara5/glassfish/nodes test-local-node
asadmin start-instance local-instance
CLI801 Instance is already synchronized
Waiting for local-instance to start ............
Successfully started the instance: local-instance


@Pandrex247
Copy link
Member

left a comment

Looks OK apart from one issue

@MattGill98 MattGill98 force-pushed the MattGill98:PAYARA-4031 branch from d0004f8 to 5b82eae Aug 12, 2019

@MattGill98 MattGill98 requested review from dmatej and Pandrex247 Aug 12, 2019

@dmatej

This comment has been minimized.

Copy link
Contributor

commented on 5b82eae Aug 13, 2019

What about this?:

        } catch (IOException ex) {
                throw new CommandException(STRINGS.get("no.console"), ex);
        }
@MarkWareham

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2019

@dmatej changes requested are now resolved?

@MattGill98

This comment has been minimized.

Copy link
Member Author

commented Aug 13, 2019

jenkins test please

@dmatej
dmatej approved these changes Aug 13, 2019
@dmatej

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2019

@MarkWareham Now they are :-)

@dmatej

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2019

Jenkins test please

@MattGill98 MattGill98 merged commit 996e12d into payara:master Aug 13, 2019

15 of 19 checks passed

security/snyk - api/pom.xml (payara-ci) Test in progress
security/snyk - appserver/featuresets/pom.xml (payara-ci) Test in progress
security/snyk - appserver/osgi-platforms/pom.xml (payara-ci) Test in progress
security/snyk - appserver/security/pom.xml (payara-ci) Test in progress
Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - appserver/admingui/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/common/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/connectors/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ejb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/extras/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/installer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/load-balancer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/orb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/registration/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/transaction/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/test-utils/pom.xml (payara-ci) No new issues
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.