EBS-1070 Fix #427 VSDK fix required to handle empty system password during Lua to VSDK upgrade

[Balamuruhan]

Problem:

For EBS plugin implemented in vsdk, we use passwordCredentialsSupplier for System Password field which is retrieved from UI and used in our workflows. But this field is optional field, where user may not provide it in most of the cases. So when we upgrade the plugin from Lua to VSDK and empty password is passed in for System password field. After upgrade, when VSDK code retrieves it using retrieve_credentials() it fails with AttributeError as retrieve_credentials() returns the KeyPairCredentials() instead of PasswordCredentials().

Call the retrieve_credentials() with dict {'password': '', 'type': 'NamedPasswordCredential'},

from dlpx.virtualization import libs
vault = {'password': '', 'type': 'NamedPasswordCredential'}
password = libs.retrieve_credentials(vault).password # We hit AttributeError as it returns KeyPairCredentials object which doesn't have attribute password.

Solution:

This is because there is a check credentials_result.password != "" which is incorrect as password in this failure is observed when an empty string used from plugin, instead we tried to check whether there is a attribute "password" in the credentials_result object using the condition,

if hasattr(credentials_result, "password")

But irrespective of whether it is PasswordCredentials or KeyPairCredentials the password attribute will be present even when it's a key pair result,

This is the protobuf definition of that result object:

message CredentialsResult {
    message KeyPair {
        string private_key = 1;
        string public_key = 2;
    }
    string username = 1;
    oneof result {
        string password = 2;
        KeyPair key_pair = 3;
    }
}

So we currently check whether key_pair.private_key and key_pair.public_key are empty to consider credentials_result as PasswordCredentials otherwise it can be considered as KeyPairCredentials.

Thanks to @rasantel for the help and suggestion.

Testing

EBS upgrade:
Tested EBS plugin with Datasets by performing upgrade from Lua to VSDK with empty system password with this change and Datasets are recovered successfully after upgrade.

Blackbox runs:
appdata_python_samples
QARunner -d dlpxdc.co group -g dlpx-master nightly -r appdata_python_samples remote --run-params='-p virt-sdk-repo=https://github.com/Balamuruhan/virtualization-sdk.git -p virt-sdk-branch=develop -p qa-plugin-repo=https://gitlab.delphix.com/balamuruhan.suriyakumar/qa-appdata-toolkits.git'

http://selfservice.jenkins.delphix.com/job/blackbox-self-service/15404/consoleFull

APPDATA_PYTHON_DIRECT_CENTOS73
QARunner -d dlpxdc.co group -g dlpx-master nightly -r appdata_basic --run-config APPDATA_PYTHON_DIRECT_CENTOS73 remote --run-params='-p virt-sdk-repo=https://github.com/Balamuruhan/virtualization-sdk.git -p virt-sdk-branch=develop -p qa-plugin-repo=https://gitlab.delphix.com/balamuruhan.suriyakumar/qa-appdata-toolkits.git'

http://selfservice.jenkins.delphix.com/job/blackbox-self-service/15401/consoleFull

APPDATA_PYTHON_STAGED_CENTOS73
QARunner -d dlpxdc.co group -g dlpx-master nightly -r appdata_basic --run-config APPDATA_PYTHON_STAGED_CENTOS73 remote --run-params='-p virt-sdk-repo=https://github.com/Balamuruhan/virtualization-sdk.git -p virt-sdk-branch=develop -p qa-plugin-repo=https://gitlab.delphix.com/balamuruhan.suriyakumar/qa-appdata-toolkits.git'

http://selfservice.jenkins.delphix.com/job/blackbox-self-service/15405/consoleFull

For unit tests:
Failure is due to the windows-latest image that doesn't support Python 2.7, I have submitted TOOLS ticket for the new image to be used.
https://delphix.atlassian.net/browse/TOOL-13254

[mrburke]

I think this looks good but can take another pass when the unit tests are updated as well

[vimleshmishra]

Able to reproduce this issue in-house, the suggested solution will definitely work.

But one alternate decent solution could be simply checking below before returning "PasswordCredentials"

if hasattr(credentials_result, "password")

The actual bug is the engine was returning empty password [but not key_pair] while the plugin is returning "PasswordCredentials" only if non-empty password OR else returns "KeyPairCredentials". While in this case since the engine returns empty password plugin will return the "KeyPairCredentials" which won't have the "password" attribute.

[ravi-cm]

Able to reproduce this issue in-house, the suggested solution will definitely work.

But one alternate decent solution could be simply checking below before returning "PasswordCredentials"

if hasattr(credentials_result, "password")

The actual bug is the engine was returning empty password [but not key_pair] while the plugin is returning "PasswordCredentials" only if non-empty password OR else returns "KeyPairCredentials". While in this case since the engine returns empty password plugin will return the "KeyPairCredentials" which won't have the "password" attribute.

Would this work if, for some reason a KeyPairCredentials with a password field is returned since protobuf definition allows that?

[ravi-cm]

lgtm.

[vimleshmishra]

Would this work if, for some reason, a KeyPairCredentials with a password field is returned since protobuf definition allows that?

But protobuf also allow password along with KeyPairCredentials private and public keys.
While the engine is used to make sure that only one should be set either password or private/public key