-
Notifications
You must be signed in to change notification settings - Fork 124
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
Faster amps install #1280
Faster amps install #1280
Conversation
"Security", | ||
new DatabaseClientFactory.DigestAuthContext(securityUsername, securityPassword) | ||
); | ||
//new AmpsInstaller(securityStagingClient).installAmps(stagingModulesDatabaseName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@grechaw ,
I think here it is assumed that "App-Services" is set to "digest" auth. In case if all servers were set to cert-auth, digest with ssl or other authentication type, this may not work. I see the same in undo() method as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I know -- I was hoping I could ask you how to generalize.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can use a method like newInstallerClient() in DeployHubAmpsCommand but that would mean that 'sslContext' for stagingAppConfig has to be set to the one obtained using 'admin' certificate (cert whose common name is 'admin' or the security user , user will be logged as the security user). Is that an acceptable change ?
private DatabaseClient newInstallerClient() {
ManageConfig manageConfig = ((HubConfigImpl)hubConfig).getManageConfig();
AppConfig stagingAppConfig = hubConfig.getStagingAppConfig();
DatabaseClientConfig config = new DatabaseClientConfig(hubConfig.getHost(), 8000, manageConfig.getSecurityUsername(), manageConfig.getSecurityPassword());
config.setCertFile(stagingAppConfig.getAppServicesCertFile());
config.setCertPassword(stagingAppConfig.getAppServicesCertPassword());
config.setDatabase("Security");
config.setExternalName(stagingAppConfig.getAppServicesExternalName());
config.setSecurityContextType(stagingAppConfig.getAppServicesSecurityContextType());
config.setSslContext(stagingAppConfig.getAppServicesSslContext());
config.setSslHostnameVerifier(stagingAppConfig.getAppServicesSslHostnameVerifier());
config.setTrustManager(stagingAppConfig.getAppServicesTrustManager());
return configuredDatabaseClientFactory.newDatabaseClient(config);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @srinathgit that's exactly what i needed. I'll take it for a spin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok ... One thing I noticed with using this newInstallerClient() method is that, if all the servers (App-Services, Manage and Admin) are set to cert-auth, setting stagingAppConfig 's 'sslContext' to the one obtained from certificate whose common name matches 'securityUser' results in LoadModulesCommand, DeleteModulesCommand, GenerateModelArtifactsCommand and LoadSchemasCommand (and maybe some other command ?)running as 'securityUser' and not as 'hub-admin-user' as intended.
|
It sounds like you've identified a general problem though -- whenever ml-app-deployer chooses to use the security user, then the certs will no longer be valid right? |
|
But that's why I wanted a special client for just these amps -- it has to be done by the security user and should never be attempted by the manage user. That's why I reached in and got the security username and password from manage client. I'm willing (if you are) to take this as a design bug. This amps installation method will no longer be needed once CMA supports amps (see blocked bug |
meaning, once CMA and ml-app-deployer have a fix, then we can again support certs for security user installation. I don't think it's an important must-have right now (my opinion only, not informed) |
yeah, I agree using newInstallerClient() will be a design bug; hopefully CMA supports amps sooner so that it is no longer needed. Until it happens, we may have to document that if users have all the servers set to ssl/cert auth they will have to override stagingAppConfig's certificate related properties to use security user's credentials |
This method speeds up amp installation a lot.
I forgot to remove the amps from the project and deployment -- if we want to go ahead with this technique ill do the extra bit.