You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/01/22 12:07:34 UTC
[GitHub] [ignite] map7000 opened a new pull request #8686: Ignite 14023
map7000 opened a new pull request #8686:
URL: https://github.com/apache/ignite/pull/8686
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
### The Contribution Checklist
- [ ] There is a single JIRA ticket related to the pull request.
- [ ] The web-link to the pull request is attached to the JIRA ticket.
- [ ] The JIRA ticket has the _Patch Available_ state.
- [ ] The pull request body describes changes that have been made.
The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
- [ ] The pull request title is treated as the final commit message.
The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
- [ ] A reviewer has been mentioned through the JIRA comments
(see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers))
- [ ] The pull request has been checked by the Teamcity Bot and
the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
### Notes
- [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
- [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
- [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
- [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586482224
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_configuration/__init__.py
##########
@@ -43,8 +43,10 @@ class IgniteConfiguration(NamedTuple):
data_storage: DataStorageConfiguration = None
caches: list = []
local_host: str = None
- ssl_context_factory: SslContextFactory = None
+ ssl_params: SslParams = None
connector_configuration: ConnectorConfiguration = None
+ auth: bool = False
Review comment:
renamed to auth_enabled
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586320997
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_auth": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"]
+ }
+ }
+ """
+ username, password = None, None
+ if _globals.get('use_auth'):
+ if user in _globals and 'credentials' in _globals[user]:
+ username, password = _globals[user]['credentials']
Review comment:
it's not a variable or parameter. it's dictionary key
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r594289577
##########
File path: modules/ducktests/tests/ignitetest/tests/auth_test.py
##########
@@ -0,0 +1,147 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains password based authentication tests
+"""
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.auth import DEFAULT_AUTH_PASSWORD, DEFAULT_AUTH_USERNAME
+from ignitetest.services.utils.control_utility import ControlUtility, ControlUtilityError
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.utils import ignite_versions, cluster
+from ignitetest.services.utils.ignite_configuration.discovery import from_ignite_cluster
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+WRONG_PASSWORD = "wrong_password"
+TEST_USERNAME = "admin"
+TEST_PASSWORD = "qwe123"
+TEST_PASSWORD2 = "123qwe"
+
+ADD_USER = 'adduser'
+UPDATE_USER = 'updateuser'
+REMOVE_USER = 'removeuser'
+
+
+# pylint: disable=W0223
+class AuthenticationTests(IgniteTest):
+ """
+ Tests Ignite Control Utility Activation command with enabled Authentication
+ """
+ NUM_NODES = 2
+
+ @cluster(num_nodes=NUM_NODES - 1)
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_with_authentication(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth_enabled=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(persistent=True)
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES - 1)
+
+ servers.start()
+
+ ControlUtility(cluster=servers, username=DEFAULT_AUTH_USERNAME, password=DEFAULT_AUTH_PASSWORD).activate()
+
+ # Run command with right password
+ check_authenticate(servers, DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD)
+
+ # Run command with wrong password
+ check_authenticate(servers, DEFAULT_AUTH_USERNAME, WRONG_PASSWORD, True)
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignite_versions(str(DEV_BRANCH))
+ def test_change_users(self, ignite_version):
Review comment:
We can just merge those two tests into one.
Why do we need to start servers two time?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r582058570
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_context.py
##########
@@ -19,20 +19,24 @@
import os
DEFAULT_PASSWORD = "123456"
-DEFAULT_SERVER_KEYSTORE = "server.jks"
-DEFAULT_CLIENT_KEYSTORE = "client.jks"
-DEFAULT_ADMIN_KEYSTORE = "admin.jks"
DEFAULT_TRUSTSTORE = "truststore.jks"
+DEFAULT_ROOT = "/opt/"
+default_keystore = {
+ 'server': 'server.jks',
+ 'client': 'client.jks',
Review comment:
Used in IgniteService and IgniteApplicationService
def update_config_with_globals
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r577505127
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -313,6 +305,33 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _parse_ssl_params(self, user, globals_dict, **kwargs):
+ ssl_dict = None
+ if globals_dict.get('use_ssl', False):
+ ssl_dict = globals_dict.get(user, {}).get('ssl', {})
+ elif kwargs.get('key_store_jks') is not None or kwargs.get('key_store_path') is not None:
+ ssl_dict = kwargs
+ return None if ssl_dict is None else \
+ SslContextFactory(key_store_path=ssl_dict.get("key_store_path",
Review comment:
I suppose,that some of these lines of code can be extracted to method.
You can define it in __parse_ssl_params.
```
def get_store_path(type, ssl_dict):
path_key = f'{type}_path'
store_name = f'{type}_jks'
default_name = DEFAULT_TRUSTSTORE if type == 'trust_store' else DEFAULT_ADMIN_KEYSTORE
return ssl_dict.get(path_key, self.jks_path(ssl_disct.get(store_name, default_name))
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8686: Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r572009018
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -313,6 +303,39 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _get_creds_from_globals(self, user, globals_dict, **kwargs):
+
+ creds = dict()
+
+ if globals_dict.get("use_ssl", False):
+ admin_dict = globals_dict.get(user, {})
+ creds = self._get_ssl_from_dict(admin_dict.get('ssl', {}))
+ elif kwargs.get('key_store_jks') is not None or kwargs.get('key_store_path') is not None:
+ creds = self._get_ssl_from_dict(kwargs)
+
+ if globals_dict.get("use_auth", False):
+ admin_dict = globals_dict.get(user, {})
Review comment:
admin_dict -> user_dict?
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -313,6 +303,39 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _get_creds_from_globals(self, user, globals_dict, **kwargs):
+
+ creds = dict()
Review comment:
this line, `creds = dict()`, on 328 line: `creds = {}`. Let's make it the same
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -313,6 +303,39 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _get_creds_from_globals(self, user, globals_dict, **kwargs):
+
+ creds = dict()
+
+ if globals_dict.get("use_ssl", False):
+ admin_dict = globals_dict.get(user, {})
Review comment:
admin_dict -> user_dict?
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -313,6 +303,39 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _get_creds_from_globals(self, user, globals_dict, **kwargs):
+
+ creds = dict()
+
+ if globals_dict.get("use_ssl", False):
+ admin_dict = globals_dict.get(user, {})
+ creds = self._get_ssl_from_dict(admin_dict.get('ssl', {}))
+ elif kwargs.get('key_store_jks') is not None or kwargs.get('key_store_path') is not None:
+ creds = self._get_ssl_from_dict(kwargs)
+
+ if globals_dict.get("use_auth", False):
+ admin_dict = globals_dict.get(user, {})
+ creds['login'] = admin_dict.get("login", DEFAULT_AUTH_LOGIN)
+ creds['password'] = admin_dict.get("password", DEFAULT_AUTH_PASSWORD)
+ elif kwargs.get('login') is not None:
+ creds['login'] = kwargs.get("login", DEFAULT_AUTH_LOGIN)
+ creds['password'] = kwargs.get("password", DEFAULT_AUTH_PASSWORD)
+
+ return creds
+
+ def _get_ssl_from_dict(self, ssl_dict):
+
+ creds = {}
+
+ creds['key_store_path'] = ssl_dict.get("key_store_path",
Review comment:
WDYT if creds will be not a dict, but a class with well defined structure?
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,148 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params pasrsing
+"""
+
+import pytest
+from ignitetest.services.utils.control_utility import ControlUtility
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.certificate_dir = ''
+
+
+class Context:
+ """
+ Need this to initialise ControlUtility
+ """
+ def __init__(self, globals_dict):
+ self.logger = ''
+ self.globals = globals_dict
+
+
+class CheckControlUtility:
+ """
+ Check that control_utulity.py correctly parse globals
+ """
+
+ test_admin_data_full = {"login": "admin1", "password": "qwe123",
+ "ssl": {"key_store_jks": "admin1.jks", "key_store_password": "qwe123",
+ "trust_store_jks": "truststore.jks", "trust_store_password": "qwe123"}}
+
+ test_admin_data_ssl = {
+ "ssl": {"key_store_jks": "admin1.jks", "key_store_password": "qwe123", "trust_store_jks": "truststore.jks",
+ "trust_store_password": "qwe123"}}
+ test_admin_data_auth = {"login": "admin1", "password": "qwe123"}
+
+ globals_full = {
+ 'use_ssl': True,
+ 'use_auth': True,
+ 'admin': test_admin_data_full
+ }
+
+ globals_auth = {
+ 'use_ssl': True,
+ 'use_auth': True,
+ 'admin': test_admin_data_auth
+ }
+
+ globals_ssl = {
+ 'use_ssl': True,
+ 'use_auth': True,
+ 'admin': test_admin_data_ssl
+ }
+
+ globals_no_ssl = {
+ 'use_auth': True,
+ 'admin': test_admin_data_full
+ }
+
+ globals_no_auth = {
+ 'use_ssl': True,
+ 'admin': test_admin_data_full
+ }
+
+ globals_no_ssl_no_auth = {
+ 'admin': test_admin_data_full
+ }
+
+ globals_ssl_auth = {
+ 'use_ssl': True,
+ 'use_auth': True
+ }
+
+ globals_nil = {}
+
+ @staticmethod
+ @pytest.mark.parametrize("test_globals, test_kwargs, expected",
+ [(globals_full, {}, {'key_store_path': 'admin1.jks', 'key_store_password': 'qwe123',
Review comment:
I'd suggest you create a variable that contains expected structure the same way as you create variable for every globals case.
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -374,6 +397,7 @@ class ControlUtilityError(RemoteCommandError):
"""
Error is raised when control utility failed.
"""
+
Review comment:
Why do you make this change? Do we have a check for that?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586360424
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -416,20 +416,17 @@ def __update_node_log_file(self, node):
self.logger.debug(f"rotating {node.log_file} to {rotated_log} on {node.name}")
node.account.ssh(f"mv {node.log_file} {rotated_log}")
- def _update_ssl_config_with_globals(self, dict_name: str, default_jks: str):
+ def _update_ssl_config_with_globals(self, dict_name: str):
"""
Update ssl configuration.
"""
- _dict = self.globals.get(dict_name)
-
- if _dict is not None:
- ssl_context_factory = SslContextFactory(self.install_root, **_dict)
- else:
- ssl_context_factory = SslContextFactory(self.install_root, default_jks)
-
- self.config = self.config._replace(ssl_context_factory=ssl_context_factory)
- self.config = self.config._replace(connector_configuration=ConnectorConfiguration(
- ssl_enabled=True, ssl_context_factory=ssl_context_factory))
+ ssl_params = None
+ if self.config.ssl_params is None:
+ ssl_params = get_ssl_params_from_globals(self.globals, dict_name)
+ if ssl_params:
+ self.config = self.config._replace(ssl_params=ssl_params)
+ self.config = self.config._replace(connector_configuration=ConnectorConfiguration(
+ ssl_enabled=True, ssl_params=ssl_params))
Review comment:
We have SSL params in IgniteConfiguration and in ConnectorConfiguration, so we should change them both
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586479000
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_auth": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"]
+ }
+ }
+ """
+ username, password = None, None
+ if _globals.get('use_auth'):
Review comment:
I didn't get the meaning, It's already constant and used in Unit test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r572901128
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -313,6 +303,39 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _get_creds_from_globals(self, user, globals_dict, **kwargs):
+
+ creds = dict()
+
+ if globals_dict.get("use_ssl", False):
+ admin_dict = globals_dict.get(user, {})
+ creds = self._get_ssl_from_dict(admin_dict.get('ssl', {}))
+ elif kwargs.get('key_store_jks') is not None or kwargs.get('key_store_path') is not None:
+ creds = self._get_ssl_from_dict(kwargs)
+
+ if globals_dict.get("use_auth", False):
+ admin_dict = globals_dict.get(user, {})
+ creds['login'] = admin_dict.get("login", DEFAULT_AUTH_LOGIN)
+ creds['password'] = admin_dict.get("password", DEFAULT_AUTH_PASSWORD)
+ elif kwargs.get('login') is not None:
+ creds['login'] = kwargs.get("login", DEFAULT_AUTH_LOGIN)
+ creds['password'] = kwargs.get("password", DEFAULT_AUTH_PASSWORD)
+
+ return creds
+
+ def _get_ssl_from_dict(self, ssl_dict):
+
+ creds = {}
+
+ creds['key_store_path'] = ssl_dict.get("key_store_path",
Review comment:
Good idea. Dict changed to class
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586497377
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_params.py
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License
+
+"""
+This module contains classes and utilities for Ignite SslContextFactory.
+
+This file contains three user presets:
+1. server for Ignite(clientMode=False)
+2. client for Ignte(clientMode=True) node and ThinClient
+3. admin for GridClient and console utils (control.sh)
+
+Keystores for this presets are generated automaticaly on creating envoriment
+If you like to specify different certificate for preset you can pass them throw globals
+If you specyfy ssl_params in test, you override globals
+"""
+import os
+
+
+DEFAULT_KEYSTORE = 'server.jks'
+DEFAULT_CLIENT_KEYSTORE = 'client.jks'
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
+DEFAULT_PASSWORD = "123456"
+DEFAULT_TRUSTSTORE = "truststore.jks"
+DEFAULT_ROOT = "/opt/"
+
+default_keystore = {
+ 'server': DEFAULT_KEYSTORE,
+ 'client': DEFAULT_CLIENT_KEYSTORE,
+ 'admin': DEFAULT_ADMIN_KEYSTORE
+}
+
+
+class SslParams:
+ """
+ Params for Ignite SslContextFactory.
+ """
+
+ # pylint: disable=R0913
+ def __init__(self, root_dir: str = DEFAULT_ROOT,
+ key_store_jks: str = DEFAULT_KEYSTORE, key_store_password: str = DEFAULT_PASSWORD,
+ trust_store_jks: str = DEFAULT_TRUSTSTORE, trust_store_password: str = DEFAULT_PASSWORD,
+ key_store_path: str = None, trust_store_path: str = None):
+ certificate_dir = os.path.join(root_dir, "ignite-dev", "modules", "ducktests", "tests", "certs")
+
+ self.key_store_path = key_store_path if key_store_path is not None \
+ else os.path.join(certificate_dir, key_store_jks)
+ self.key_store_password = key_store_password
+ self.trust_store_path = trust_store_path if trust_store_path is not None \
+ else os.path.join(certificate_dir, trust_store_jks)
+ self.trust_store_password = trust_store_password
+
+
+def get_ssl_params_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_ssl": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"],
+ "ssl": {
+ "key_store_jks": ....
+ }
+ }
+
+ }
+ """
+ root_dir = _globals.get("install_root", DEFAULT_ROOT)
+ ssl_param = None
+ if _globals.get('use_ssl'):
+ if user in _globals and 'ssl' in _globals[user]:
+ ssl_param = _globals[user]['ssl']
+ else:
+ ssl_param = {'key_store_jks': default_keystore[user]}
Review comment:
Check added
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r577506126
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -313,6 +305,33 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _parse_ssl_params(self, user, globals_dict, **kwargs):
+ ssl_dict = None
+ if globals_dict.get('use_ssl', False):
+ ssl_dict = globals_dict.get(user, {}).get('ssl', {})
+ elif kwargs.get('key_store_jks') is not None or kwargs.get('key_store_path') is not None:
+ ssl_dict = kwargs
+ return None if ssl_dict is None else \
+ SslContextFactory(key_store_path=ssl_dict.get("key_store_path",
+ self.jks_path(
+ ssl_dict.get('key_store_jks', DEFAULT_ADMIN_KEYSTORE))),
+ key_store_password=ssl_dict.get('key_store_password', DEFAULT_PASSWORD),
+ trust_store_path=ssl_dict.get("trust_store_path",
+ self.jks_path(
+ ssl_dict.get('trust_store_jks', DEFAULT_TRUSTSTORE))),
+ trust_store_password=ssl_dict.get('trust_store_password', DEFAULT_PASSWORD))
+
+ @staticmethod
+ def _parse_creds(user, globals_dict, **kwargs):
+ creds_dict = None
+ if globals_dict.get('use_auth', False):
Review comment:
You don't need here default key, just
```
if globals_dict.get('use_auth'):
.....
elif kwargs.get('login')
....
```
None is Falsy value
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r572907721
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -374,6 +397,7 @@ class ControlUtilityError(RemoteCommandError):
"""
Error is raised when control utility failed.
"""
+
Review comment:
I think, linter force me to do this change. If you talk about '\n' symbol
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586450655
##########
File path: modules/ducktests/tests/ignitetest/services/ignite.py
##########
@@ -63,8 +62,7 @@ def pids(self, node):
return []
def update_config_with_globals(self):
- if self.globals.get("use_ssl", False):
- self._update_ssl_config_with_globals("server", DEFAULT_SERVER_KEYSTORE)
+ self._update_ssl_config_with_globals("server")
Review comment:
I just modify function parsing this parameter from globals.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586466224
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -37,33 +37,19 @@ class ControlUtility:
BASE_COMMAND = "control.sh"
# pylint: disable=R0913
- def __init__(self, cluster,
- key_store_jks: str = None, key_store_password: str = DEFAULT_PASSWORD,
- trust_store_jks: str = DEFAULT_TRUSTSTORE, trust_store_password: str = DEFAULT_PASSWORD):
+ def __init__(self, cluster, ssl_params=None, username=None, password=DEFAULT_AUTH_PASSWORD):
self._cluster = cluster
self.logger = cluster.context.logger
- if cluster.context.globals.get("use_ssl", False):
- admin_dict = cluster.globals.get("admin", dict())
-
- self.key_store_path = admin_dict.get("key_store_path",
- self.jks_path(admin_dict.get('key_store_jks', DEFAULT_ADMIN_KEYSTORE)))
- self.key_store_password = admin_dict.get('key_store_password', DEFAULT_PASSWORD)
- self.trust_store_path = admin_dict.get("trust_store_path",
- self.jks_path(admin_dict.get('trust_store_jks', DEFAULT_TRUSTSTORE)))
- self.trust_store_password = admin_dict.get('trust_store_password', DEFAULT_PASSWORD)
-
- elif key_store_jks is not None:
- self.key_store_path = self.jks_path(key_store_jks)
- self.key_store_password = key_store_password
- self.trust_store_path = self.jks_path(trust_store_jks)
- self.trust_store_password = trust_store_password
+ if not ssl_params:
+ self.ssl_params = get_ssl_params_from_globals(cluster.context.globals, 'admin')
+ else:
+ self.ssl_params = ssl_params
- def jks_path(self, jks_name: str):
- """
- :return Path to jks file.
- """
- return os.path.join(self._cluster.certificate_dir, jks_name)
+ if not username:
+ self.username, self.password = get_credentials_from_globals(cluster.context.globals, 'admin')
Review comment:
This user was previously added for ssl (not in this change)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] Sega76 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
Sega76 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r578752775
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -34,27 +36,11 @@ class ControlUtility:
BASE_COMMAND = "control.sh"
# pylint: disable=R0913
- def __init__(self, cluster,
- key_store_jks: str = None, key_store_password: str = DEFAULT_PASSWORD,
- trust_store_jks: str = DEFAULT_TRUSTSTORE, trust_store_password: str = DEFAULT_PASSWORD):
+ def __init__(self, cluster, **kwargs):
self._cluster = cluster
Review comment:
we can immediately receive SslContextFactory and CredsProvider
it's easier to use them
```
def __init__(self, cluster, ssl_context: SslContextFactory = None, creds: CredsProvider = None):
self._cluster = cluster
self.logger = cluster.context.logger
self.ssl_context = get_ssl_context_with_globals(cluster.context.globals, "admin", ssl_context)
self.creds = get_creds_provider_with_globals(cluster.context.globals, "admin", creds)
```
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -313,6 +305,41 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _parse_ssl_params(self, user, globals_dict, **kwargs):
Review comment:
can be simplified like this
```
def get_creds_provider_with_globals(globals: dict, user: str, creds_dflt: CredsProvider) -> CredsProvider:
"""
:param globals Globals
:param user User
:param creds_dflt Default CredsProvider
:return CredsProvider
"""
if globals.get('use_auth'):
if user in globals and 'creds' in globals[user]:
creds_provider = CredsProvider(**globals[user]['creds'])
else:
creds_provider = creds_dflt if creds_dflt else CredsProvider()
else:
creds_provider = creds_dflt
return creds_provider
def get_ssl_context_with_globals(globals: dict, user: str, ssl_ctx_dflt: SslContextFactory) -> SslContextFactory:
"""
:param globals Globals
:param user User
:param ssl_ctx_dflt Default SslContextFactory
:return SslContextFactory
"""
root_dir = globals.get("install_root", "/opt")
if globals.get('use_ssl'):
if user in globals and 'ssl' in globals[user]:
ssl_context_factory = SslContextFactory(root_dir, **globals[user]['ssl'])
else:
ssl_context_factory = ssl_ctx_dflt if ssl_ctx_dflt else SslContextFactory(root_dir, DEFAULT_ADMIN_KEYSTORE)
else:
ssl_context_factory = ssl_ctx_dflt
return ssl_context_factory
```
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/creds_provider.py
##########
@@ -0,0 +1,31 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License
+
+"""
+This module contains classes and utilities for Authentication Provider.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_LOGIN = "ignite"
+
+
+class CredsProvider:
+ """
+ Credentials provider.
+ """
+
+ def __init__(self, login=None, password=None):
Review comment:
we may use default value in __init__
##########
File path: modules/ducktests/tests/ignitetest/tests/auth_test.py
##########
@@ -0,0 +1,90 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains password based authentication tests
+"""
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.utils.control_utility import ControlUtility, ControlUtilityError
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.utils import ignite_versions, cluster, ignore_if
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+
+# pylint: disable=W0223
+class AuthenticationTests(IgniteTest):
+ """
+ Tests Ignite Authentication
+ """
+ NUM_NODES = 3
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_good_user(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ Positive case
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(name='persistent', persistent=True),
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES, startup_timeout_sec=60)
+
+ servers.start()
+
+ control_utility = ControlUtility(cluster=servers, login="ignite", password="ignite")
+ control_utility.activate()
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignore_if(lambda version, globals_dict: globals_dict.get("use_auth", False)) # Globals overrides test params
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_bad_user(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ Negative case
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(name='persistent', persistent=True),
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES, startup_timeout_sec=60)
+
+ servers.start()
+
+ control_utility = ControlUtility(cluster=servers, login="bad_person", password="wrong_password")
Review comment:
if you use the suggested above
```
control_utility = ControlUtility(cluster=servers,
creds=CredsProvider(login="bad_person", password="wrong_password"))
##########
File path: modules/ducktests/tests/ignitetest/tests/auth_test.py
##########
@@ -0,0 +1,90 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains password based authentication tests
+"""
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.utils.control_utility import ControlUtility, ControlUtilityError
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.utils import ignite_versions, cluster, ignore_if
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+
+# pylint: disable=W0223
+class AuthenticationTests(IgniteTest):
+ """
+ Tests Ignite Authentication
+ """
+ NUM_NODES = 3
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_good_user(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ Positive case
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(name='persistent', persistent=True),
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES, startup_timeout_sec=60)
+
+ servers.start()
+
+ control_utility = ControlUtility(cluster=servers, login="ignite", password="ignite")
Review comment:
if you use the suggested above
```
control_utility = ControlUtility(cluster=servers, creds=CredsProvider())
```
##########
File path: modules/ducktests/tests/ignitetest/tests/auth_test.py
##########
@@ -0,0 +1,90 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains password based authentication tests
+"""
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.utils.control_utility import ControlUtility, ControlUtilityError
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.utils import ignite_versions, cluster, ignore_if
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+
+# pylint: disable=W0223
+class AuthenticationTests(IgniteTest):
+ """
+ Tests Ignite Authentication
+ """
+ NUM_NODES = 3
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_good_user(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ Positive case
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(name='persistent', persistent=True),
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES, startup_timeout_sec=60)
+
+ servers.start()
+
+ control_utility = ControlUtility(cluster=servers, login="ignite", password="ignite")
+ control_utility.activate()
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignore_if(lambda version, globals_dict: globals_dict.get("use_auth", False)) # Globals overrides test params
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_bad_user(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ Negative case
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(name='persistent', persistent=True),
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES, startup_timeout_sec=60)
+
Review comment:
startup_timeout_sec=60 is default value
##########
File path: modules/ducktests/tests/ignitetest/tests/auth_test.py
##########
@@ -0,0 +1,90 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains password based authentication tests
+"""
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.utils.control_utility import ControlUtility, ControlUtilityError
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.utils import ignite_versions, cluster, ignore_if
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+
+# pylint: disable=W0223
+class AuthenticationTests(IgniteTest):
+ """
+ Tests Ignite Authentication
+ """
+ NUM_NODES = 3
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_good_user(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ Positive case
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(name='persistent', persistent=True),
Review comment:
the name parameter has a default value
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,250 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params pasrsing
+"""
+
+import pytest
+from ignitetest.services.utils.auth.creds_provider import CredsProvider, DEFAULT_AUTH_LOGIN, DEFAULT_AUTH_PASSWORD
+from ignitetest.services.utils.ssl.ssl_factory import SslContextFactory, DEFAULT_PASSWORD, DEFAULT_TRUSTSTORE, \
+ DEFAULT_ADMIN_KEYSTORE
+from ignitetest.services.utils.control_utility import ControlUtility
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.certificate_dir = '/opt/certs/'
+
+
+class Context:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.logger = ''
+ self.globals = globals_dict
+
+
+def compare_ssl(class1, class2):
+ """
+ Compare two SslContextFactory objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, SslContextFactory) and isinstance(class2, SslContextFactory):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+def compare_creds(class1, class2):
+ """
+ Compare two CredsProvider objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, CredsProvider) and isinstance(class2, CredsProvider):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+class TestParams:
+ """
+ Globals and Kwargs for tests
+ """
+
+ test_ssl_jks = {'key_store_jks': 'admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_jks': 'truststore.jks', 'trust_store_password': 'qwe123'}
+ test_ssl_path = {'key_store_path': '/opt/certs/admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_path': '/opt/certs/truststore.jks',
+ 'trust_store_password': 'qwe123'}
+ test_ssl_only_key = {'key_store_jks': 'admin1.jks'}
+
+ test_creds = {'login': 'admin1', 'password': 'qwe123'}
+ test_creds_only_login = {'login': 'admin1'}
+
+ expected_ssl = SslContextFactory(key_store_path='/opt/certs/admin1.jks',
+ key_store_password='qwe123',
+ trust_store_path='/opt/certs/truststore.jks',
+ trust_store_password='qwe123')
+ expected_ssl_default = SslContextFactory(key_store_path='/opt/certs/'+DEFAULT_ADMIN_KEYSTORE,
+ key_store_password=DEFAULT_PASSWORD,
+ trust_store_path='/opt/certs/'+DEFAULT_TRUSTSTORE,
+ trust_store_password=DEFAULT_PASSWORD)
+ expected_ssl_only_key = SslContextFactory(key_store_path='/opt/certs/admin1.jks',
+ key_store_password=DEFAULT_PASSWORD,
+ trust_store_path='/opt/certs/'+DEFAULT_TRUSTSTORE,
+ trust_store_password=DEFAULT_PASSWORD)
+
+ expected_creds = CredsProvider(login='admin1',
+ password='qwe123')
+ expected_creds_default = CredsProvider(login=DEFAULT_AUTH_LOGIN,
+ password=DEFAULT_AUTH_PASSWORD)
+ expected_creds_only_login = CredsProvider(login='admin1',
+ password=DEFAULT_AUTH_PASSWORD)
+
+
+class CheckCaseGlobalsSetKwargsNotSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_jks}}, {}, TestParams.expected_ssl),
+ ({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_path}}, {}, TestParams.expected_ssl),
+ ({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_only_key}}, {}, TestParams.expected_ssl_only_key),
+ ({'admin': {'ssl': TestParams.test_ssl_jks}}, {}, None), ])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ assert compare_ssl(ControlUtility(Cluster(test_globals), **test_kwargs).ssl_context, expected)
+
+
+class CheckCaseGlobalsNotSetKwargsSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from kwargs
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({}, TestParams.test_ssl_jks, TestParams.expected_ssl),
+ ({}, TestParams.test_ssl_path, TestParams.expected_ssl),
+ ({}, TestParams.test_ssl_only_key, TestParams.expected_ssl_only_key)])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse SSL params from kwargs
+ """
+
+ assert compare_ssl(ControlUtility(Cluster(test_globals), **test_kwargs).ssl_context, expected)
+
+
+class CheckCaseGlobalsSetKwargsSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_jks}}, TestParams.test_ssl_only_key,
+ TestParams.expected_ssl),
+ ({'admin': {'ssl': TestParams.test_ssl_jks}}, TestParams.test_ssl_only_key,
+ TestParams.expected_ssl_only_key)])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse SSL params
+ """
+
+ assert compare_ssl(ControlUtility(Cluster(test_globals), **test_kwargs).ssl_context, expected)
+
+
+class CheckCaseGlobalsSetKwargsNotSetCreds:
+ """
+ Check that control_utulity.py correctly parse credentials from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({'use_auth': True,
+ 'admin': {'creds': TestParams.test_creds}}, {}, TestParams.expected_creds),
+ ({'use_auth': True,
+ 'admin': {'creds': TestParams.test_creds_only_login}}, {},
+ TestParams.expected_creds_only_login),
+ ({'use_auth': True}, {},
+ TestParams.expected_creds_default)])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse credentials from globals
+ """
+ print(expected.__dict__)
Review comment:
for debugging?
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,250 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params pasrsing
+"""
+
+import pytest
+from ignitetest.services.utils.auth.creds_provider import CredsProvider, DEFAULT_AUTH_LOGIN, DEFAULT_AUTH_PASSWORD
+from ignitetest.services.utils.ssl.ssl_factory import SslContextFactory, DEFAULT_PASSWORD, DEFAULT_TRUSTSTORE, \
+ DEFAULT_ADMIN_KEYSTORE
+from ignitetest.services.utils.control_utility import ControlUtility
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.certificate_dir = '/opt/certs/'
+
+
+class Context:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.logger = ''
+ self.globals = globals_dict
+
+
+def compare_ssl(class1, class2):
+ """
+ Compare two SslContextFactory objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, SslContextFactory) and isinstance(class2, SslContextFactory):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+def compare_creds(class1, class2):
+ """
+ Compare two CredsProvider objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, CredsProvider) and isinstance(class2, CredsProvider):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+class TestParams:
+ """
+ Globals and Kwargs for tests
+ """
+
+ test_ssl_jks = {'key_store_jks': 'admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_jks': 'truststore.jks', 'trust_store_password': 'qwe123'}
+ test_ssl_path = {'key_store_path': '/opt/certs/admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_path': '/opt/certs/truststore.jks',
+ 'trust_store_password': 'qwe123'}
+ test_ssl_only_key = {'key_store_jks': 'admin1.jks'}
+
+ test_creds = {'login': 'admin1', 'password': 'qwe123'}
+ test_creds_only_login = {'login': 'admin1'}
+
+ expected_ssl = SslContextFactory(key_store_path='/opt/certs/admin1.jks',
+ key_store_password='qwe123',
+ trust_store_path='/opt/certs/truststore.jks',
+ trust_store_password='qwe123')
+ expected_ssl_default = SslContextFactory(key_store_path='/opt/certs/'+DEFAULT_ADMIN_KEYSTORE,
+ key_store_password=DEFAULT_PASSWORD,
+ trust_store_path='/opt/certs/'+DEFAULT_TRUSTSTORE,
+ trust_store_password=DEFAULT_PASSWORD)
+ expected_ssl_only_key = SslContextFactory(key_store_path='/opt/certs/admin1.jks',
+ key_store_password=DEFAULT_PASSWORD,
+ trust_store_path='/opt/certs/'+DEFAULT_TRUSTSTORE,
+ trust_store_password=DEFAULT_PASSWORD)
+
+ expected_creds = CredsProvider(login='admin1',
+ password='qwe123')
+ expected_creds_default = CredsProvider(login=DEFAULT_AUTH_LOGIN,
+ password=DEFAULT_AUTH_PASSWORD)
+ expected_creds_only_login = CredsProvider(login='admin1',
+ password=DEFAULT_AUTH_PASSWORD)
+
+
+class CheckCaseGlobalsSetKwargsNotSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
Review comment:
test_kwargs not used on this test
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,250 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params pasrsing
+"""
+
+import pytest
+from ignitetest.services.utils.auth.creds_provider import CredsProvider, DEFAULT_AUTH_LOGIN, DEFAULT_AUTH_PASSWORD
+from ignitetest.services.utils.ssl.ssl_factory import SslContextFactory, DEFAULT_PASSWORD, DEFAULT_TRUSTSTORE, \
+ DEFAULT_ADMIN_KEYSTORE
+from ignitetest.services.utils.control_utility import ControlUtility
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.certificate_dir = '/opt/certs/'
+
+
+class Context:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.logger = ''
+ self.globals = globals_dict
+
+
+def compare_ssl(class1, class2):
+ """
+ Compare two SslContextFactory objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, SslContextFactory) and isinstance(class2, SslContextFactory):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+def compare_creds(class1, class2):
+ """
+ Compare two CredsProvider objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, CredsProvider) and isinstance(class2, CredsProvider):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+class TestParams:
+ """
+ Globals and Kwargs for tests
+ """
+
+ test_ssl_jks = {'key_store_jks': 'admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_jks': 'truststore.jks', 'trust_store_password': 'qwe123'}
+ test_ssl_path = {'key_store_path': '/opt/certs/admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_path': '/opt/certs/truststore.jks',
+ 'trust_store_password': 'qwe123'}
+ test_ssl_only_key = {'key_store_jks': 'admin1.jks'}
+
+ test_creds = {'login': 'admin1', 'password': 'qwe123'}
+ test_creds_only_login = {'login': 'admin1'}
+
+ expected_ssl = SslContextFactory(key_store_path='/opt/certs/admin1.jks',
+ key_store_password='qwe123',
+ trust_store_path='/opt/certs/truststore.jks',
+ trust_store_password='qwe123')
+ expected_ssl_default = SslContextFactory(key_store_path='/opt/certs/'+DEFAULT_ADMIN_KEYSTORE,
+ key_store_password=DEFAULT_PASSWORD,
+ trust_store_path='/opt/certs/'+DEFAULT_TRUSTSTORE,
+ trust_store_password=DEFAULT_PASSWORD)
+ expected_ssl_only_key = SslContextFactory(key_store_path='/opt/certs/admin1.jks',
+ key_store_password=DEFAULT_PASSWORD,
+ trust_store_path='/opt/certs/'+DEFAULT_TRUSTSTORE,
+ trust_store_password=DEFAULT_PASSWORD)
+
+ expected_creds = CredsProvider(login='admin1',
+ password='qwe123')
+ expected_creds_default = CredsProvider(login=DEFAULT_AUTH_LOGIN,
+ password=DEFAULT_AUTH_PASSWORD)
+ expected_creds_only_login = CredsProvider(login='admin1',
+ password=DEFAULT_AUTH_PASSWORD)
+
+
+class CheckCaseGlobalsSetKwargsNotSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_jks}}, {}, TestParams.expected_ssl),
+ ({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_path}}, {}, TestParams.expected_ssl),
+ ({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_only_key}}, {}, TestParams.expected_ssl_only_key),
+ ({'admin': {'ssl': TestParams.test_ssl_jks}}, {}, None), ])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ assert compare_ssl(ControlUtility(Cluster(test_globals), **test_kwargs).ssl_context, expected)
+
+
+class CheckCaseGlobalsNotSetKwargsSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from kwargs
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({}, TestParams.test_ssl_jks, TestParams.expected_ssl),
+ ({}, TestParams.test_ssl_path, TestParams.expected_ssl),
+ ({}, TestParams.test_ssl_only_key, TestParams.expected_ssl_only_key)])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse SSL params from kwargs
+ """
+
+ assert compare_ssl(ControlUtility(Cluster(test_globals), **test_kwargs).ssl_context, expected)
+
+
+class CheckCaseGlobalsSetKwargsSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_jks}}, TestParams.test_ssl_only_key,
+ TestParams.expected_ssl),
+ ({'admin': {'ssl': TestParams.test_ssl_jks}}, TestParams.test_ssl_only_key,
+ TestParams.expected_ssl_only_key)])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse SSL params
+ """
+
+ assert compare_ssl(ControlUtility(Cluster(test_globals), **test_kwargs).ssl_context, expected)
+
+
+class CheckCaseGlobalsSetKwargsNotSetCreds:
+ """
+ Check that control_utulity.py correctly parse credentials from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({'use_auth': True,
+ 'admin': {'creds': TestParams.test_creds}}, {}, TestParams.expected_creds),
Review comment:
test_kwargs not used on this test
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,250 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params pasrsing
+"""
+
+import pytest
+from ignitetest.services.utils.auth.creds_provider import CredsProvider, DEFAULT_AUTH_LOGIN, DEFAULT_AUTH_PASSWORD
+from ignitetest.services.utils.ssl.ssl_factory import SslContextFactory, DEFAULT_PASSWORD, DEFAULT_TRUSTSTORE, \
+ DEFAULT_ADMIN_KEYSTORE
+from ignitetest.services.utils.control_utility import ControlUtility
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.certificate_dir = '/opt/certs/'
+
+
+class Context:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.logger = ''
+ self.globals = globals_dict
+
+
+def compare_ssl(class1, class2):
+ """
+ Compare two SslContextFactory objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, SslContextFactory) and isinstance(class2, SslContextFactory):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+def compare_creds(class1, class2):
+ """
+ Compare two CredsProvider objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, CredsProvider) and isinstance(class2, CredsProvider):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+class TestParams:
+ """
+ Globals and Kwargs for tests
+ """
+
+ test_ssl_jks = {'key_store_jks': 'admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_jks': 'truststore.jks', 'trust_store_password': 'qwe123'}
+ test_ssl_path = {'key_store_path': '/opt/certs/admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_path': '/opt/certs/truststore.jks',
+ 'trust_store_password': 'qwe123'}
+ test_ssl_only_key = {'key_store_jks': 'admin1.jks'}
+
+ test_creds = {'login': 'admin1', 'password': 'qwe123'}
+ test_creds_only_login = {'login': 'admin1'}
+
+ expected_ssl = SslContextFactory(key_store_path='/opt/certs/admin1.jks',
+ key_store_password='qwe123',
+ trust_store_path='/opt/certs/truststore.jks',
+ trust_store_password='qwe123')
+ expected_ssl_default = SslContextFactory(key_store_path='/opt/certs/'+DEFAULT_ADMIN_KEYSTORE,
+ key_store_password=DEFAULT_PASSWORD,
+ trust_store_path='/opt/certs/'+DEFAULT_TRUSTSTORE,
+ trust_store_password=DEFAULT_PASSWORD)
+ expected_ssl_only_key = SslContextFactory(key_store_path='/opt/certs/admin1.jks',
+ key_store_password=DEFAULT_PASSWORD,
+ trust_store_path='/opt/certs/'+DEFAULT_TRUSTSTORE,
+ trust_store_password=DEFAULT_PASSWORD)
+
+ expected_creds = CredsProvider(login='admin1',
+ password='qwe123')
+ expected_creds_default = CredsProvider(login=DEFAULT_AUTH_LOGIN,
+ password=DEFAULT_AUTH_PASSWORD)
+ expected_creds_only_login = CredsProvider(login='admin1',
+ password=DEFAULT_AUTH_PASSWORD)
+
+
+class CheckCaseGlobalsSetKwargsNotSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_jks}}, {}, TestParams.expected_ssl),
+ ({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_path}}, {}, TestParams.expected_ssl),
+ ({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_only_key}}, {}, TestParams.expected_ssl_only_key),
+ ({'admin': {'ssl': TestParams.test_ssl_jks}}, {}, None), ])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ assert compare_ssl(ControlUtility(Cluster(test_globals), **test_kwargs).ssl_context, expected)
+
+
+class CheckCaseGlobalsNotSetKwargsSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from kwargs
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({}, TestParams.test_ssl_jks, TestParams.expected_ssl),
+ ({}, TestParams.test_ssl_path, TestParams.expected_ssl),
+ ({}, TestParams.test_ssl_only_key, TestParams.expected_ssl_only_key)])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse SSL params from kwargs
+ """
+
+ assert compare_ssl(ControlUtility(Cluster(test_globals), **test_kwargs).ssl_context, expected)
+
+
+class CheckCaseGlobalsSetKwargsSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_jks}}, TestParams.test_ssl_only_key,
+ TestParams.expected_ssl),
+ ({'admin': {'ssl': TestParams.test_ssl_jks}}, TestParams.test_ssl_only_key,
+ TestParams.expected_ssl_only_key)])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse SSL params
+ """
+
+ assert compare_ssl(ControlUtility(Cluster(test_globals), **test_kwargs).ssl_context, expected)
+
+
+class CheckCaseGlobalsSetKwargsNotSetCreds:
+ """
+ Check that control_utulity.py correctly parse credentials from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({'use_auth': True,
+ 'admin': {'creds': TestParams.test_creds}}, {}, TestParams.expected_creds),
+ ({'use_auth': True,
+ 'admin': {'creds': TestParams.test_creds_only_login}}, {},
+ TestParams.expected_creds_only_login),
+ ({'use_auth': True}, {},
+ TestParams.expected_creds_default)])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse credentials from globals
+ """
+ print(expected.__dict__)
+ print(ControlUtility(Cluster(test_globals), **test_kwargs).creds_prover.__dict__)
+
+ assert compare_creds(ControlUtility(Cluster(test_globals), **test_kwargs).creds_prover, expected)
+
+
+class CheckCaseGlobalsNotSetKwargsSetCreds:
+ """
+ Check that control_utulity.py correctly parse credentials from kwargs
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({}, TestParams.test_creds, TestParams.expected_creds),
+ ({}, TestParams.test_creds_only_login, TestParams.expected_creds_only_login)])
+ def check_parse(test_globals, test_kwargs, expected):
Review comment:
test_globals does not change in the test
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,250 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params pasrsing
+"""
+
+import pytest
+from ignitetest.services.utils.auth.creds_provider import CredsProvider, DEFAULT_AUTH_LOGIN, DEFAULT_AUTH_PASSWORD
+from ignitetest.services.utils.ssl.ssl_factory import SslContextFactory, DEFAULT_PASSWORD, DEFAULT_TRUSTSTORE, \
+ DEFAULT_ADMIN_KEYSTORE
+from ignitetest.services.utils.control_utility import ControlUtility
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.certificate_dir = '/opt/certs/'
+
+
+class Context:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.logger = ''
+ self.globals = globals_dict
+
+
+def compare_ssl(class1, class2):
+ """
+ Compare two SslContextFactory objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, SslContextFactory) and isinstance(class2, SslContextFactory):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+def compare_creds(class1, class2):
+ """
+ Compare two CredsProvider objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, CredsProvider) and isinstance(class2, CredsProvider):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+class TestParams:
+ """
+ Globals and Kwargs for tests
+ """
+
+ test_ssl_jks = {'key_store_jks': 'admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_jks': 'truststore.jks', 'trust_store_password': 'qwe123'}
+ test_ssl_path = {'key_store_path': '/opt/certs/admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_path': '/opt/certs/truststore.jks',
+ 'trust_store_password': 'qwe123'}
+ test_ssl_only_key = {'key_store_jks': 'admin1.jks'}
+
+ test_creds = {'login': 'admin1', 'password': 'qwe123'}
+ test_creds_only_login = {'login': 'admin1'}
+
+ expected_ssl = SslContextFactory(key_store_path='/opt/certs/admin1.jks',
+ key_store_password='qwe123',
+ trust_store_path='/opt/certs/truststore.jks',
+ trust_store_password='qwe123')
+ expected_ssl_default = SslContextFactory(key_store_path='/opt/certs/'+DEFAULT_ADMIN_KEYSTORE,
+ key_store_password=DEFAULT_PASSWORD,
+ trust_store_path='/opt/certs/'+DEFAULT_TRUSTSTORE,
+ trust_store_password=DEFAULT_PASSWORD)
+ expected_ssl_only_key = SslContextFactory(key_store_path='/opt/certs/admin1.jks',
+ key_store_password=DEFAULT_PASSWORD,
+ trust_store_path='/opt/certs/'+DEFAULT_TRUSTSTORE,
+ trust_store_password=DEFAULT_PASSWORD)
+
+ expected_creds = CredsProvider(login='admin1',
+ password='qwe123')
+ expected_creds_default = CredsProvider(login=DEFAULT_AUTH_LOGIN,
+ password=DEFAULT_AUTH_PASSWORD)
+ expected_creds_only_login = CredsProvider(login='admin1',
+ password=DEFAULT_AUTH_PASSWORD)
+
+
+class CheckCaseGlobalsSetKwargsNotSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_jks}}, {}, TestParams.expected_ssl),
+ ({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_path}}, {}, TestParams.expected_ssl),
+ ({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_only_key}}, {}, TestParams.expected_ssl_only_key),
+ ({'admin': {'ssl': TestParams.test_ssl_jks}}, {}, None), ])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ assert compare_ssl(ControlUtility(Cluster(test_globals), **test_kwargs).ssl_context, expected)
+
+
+class CheckCaseGlobalsNotSetKwargsSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from kwargs
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({}, TestParams.test_ssl_jks, TestParams.expected_ssl),
+ ({}, TestParams.test_ssl_path, TestParams.expected_ssl),
+ ({}, TestParams.test_ssl_only_key, TestParams.expected_ssl_only_key)])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse SSL params from kwargs
+ """
+
+ assert compare_ssl(ControlUtility(Cluster(test_globals), **test_kwargs).ssl_context, expected)
Review comment:
test_globals does not change in the test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r579017427
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -313,6 +305,41 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _parse_ssl_params(self, user, globals_dict, **kwargs):
Review comment:
@Sega76, Are you sure that writing something twice simplifies it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r579316125
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -376,6 +366,41 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _parse_ssl_params(self, user, globals_dict, **kwargs):
Review comment:
Why you always use dict in variable? Everyone knows that globals is dict
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8686: Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r573613247
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,156 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params pasrsing
+"""
+
+import pytest
+from ignitetest.services.utils.control_utility import ControlUtility
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.certificate_dir = ''
+
+
+class Context:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.logger = ''
+ self.globals = globals_dict
+
+
+class CheckControlUtility:
+ """
+ Check that control_utulity.py correctly parse globals
+ """
+
+ test_admin_data_full = {"login": "admin1", "password": "qwe123",
+ "ssl": {"key_store_jks": "admin1.jks", "key_store_password": "qwe123",
+ "trust_store_jks": "truststore.jks", "trust_store_password": "qwe123"}}
+
+ test_admin_data_ssl = {
+ "ssl": {"key_store_jks": "admin1.jks", "key_store_password": "qwe123", "trust_store_jks": "truststore.jks",
+ "trust_store_password": "qwe123"}}
+ test_admin_data_auth = {"login": "admin1", "password": "qwe123"}
+
+ globals_full = {
+ 'use_ssl': True,
+ 'use_auth': True,
+ 'admin': test_admin_data_full
+ }
+ creds_full = {'key_store_path': 'admin1.jks', 'key_store_password': 'qwe123',
Review comment:
Is it possible to use Creds class instead.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r573697254
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,156 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params pasrsing
+"""
+
+import pytest
+from ignitetest.services.utils.control_utility import ControlUtility
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.certificate_dir = ''
+
+
+class Context:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.logger = ''
+ self.globals = globals_dict
+
+
+class CheckControlUtility:
+ """
+ Check that control_utulity.py correctly parse globals
+ """
+
+ test_admin_data_full = {"login": "admin1", "password": "qwe123",
+ "ssl": {"key_store_jks": "admin1.jks", "key_store_password": "qwe123",
+ "trust_store_jks": "truststore.jks", "trust_store_password": "qwe123"}}
+
+ test_admin_data_ssl = {
+ "ssl": {"key_store_jks": "admin1.jks", "key_store_password": "qwe123", "trust_store_jks": "truststore.jks",
+ "trust_store_password": "qwe123"}}
+ test_admin_data_auth = {"login": "admin1", "password": "qwe123"}
+
+ globals_full = {
+ 'use_ssl': True,
+ 'use_auth': True,
+ 'admin': test_admin_data_full
+ }
+ creds_full = {'key_store_path': 'admin1.jks', 'key_store_password': 'qwe123',
Review comment:
yes. Done
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586318885
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_context.py
##########
@@ -19,20 +19,24 @@
import os
DEFAULT_PASSWORD = "123456"
-DEFAULT_SERVER_KEYSTORE = "server.jks"
-DEFAULT_CLIENT_KEYSTORE = "client.jks"
-DEFAULT_ADMIN_KEYSTORE = "admin.jks"
DEFAULT_TRUSTSTORE = "truststore.jks"
+DEFAULT_ROOT = "/opt/"
+default_keystore = {
+ 'server': 'server.jks',
+ 'client': 'client.jks',
Review comment:
Something is used when IDEA may see the usages.
Duplication != Usage
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r594290070
##########
File path: modules/ducktests/tests/ignitetest/tests/auth_test.py
##########
@@ -0,0 +1,147 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains password based authentication tests
+"""
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.auth import DEFAULT_AUTH_PASSWORD, DEFAULT_AUTH_USERNAME
+from ignitetest.services.utils.control_utility import ControlUtility, ControlUtilityError
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.utils import ignite_versions, cluster
+from ignitetest.services.utils.ignite_configuration.discovery import from_ignite_cluster
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+WRONG_PASSWORD = "wrong_password"
+TEST_USERNAME = "admin"
+TEST_PASSWORD = "qwe123"
+TEST_PASSWORD2 = "123qwe"
+
+ADD_USER = 'adduser'
+UPDATE_USER = 'updateuser'
+REMOVE_USER = 'removeuser'
+
+
+# pylint: disable=W0223
+class AuthenticationTests(IgniteTest):
+ """
+ Tests Ignite Control Utility Activation command with enabled Authentication
+ """
+ NUM_NODES = 2
+
+ @cluster(num_nodes=NUM_NODES - 1)
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_with_authentication(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth_enabled=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(persistent=True)
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES - 1)
+
+ servers.start()
+
+ ControlUtility(cluster=servers, username=DEFAULT_AUTH_USERNAME, password=DEFAULT_AUTH_PASSWORD).activate()
+
+ # Run command with right password
+ check_authenticate(servers, DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD)
+
+ # Run command with wrong password
+ check_authenticate(servers, DEFAULT_AUTH_USERNAME, WRONG_PASSWORD, True)
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignite_versions(str(DEV_BRANCH))
+ def test_change_users(self, ignite_version):
+ """
+ Test add, update and remove user
+ """
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth_enabled=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(persistent=True),
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES - 1)
+
+ servers.start()
+
+ ControlUtility(cluster=servers, username=DEFAULT_AUTH_USERNAME, password=DEFAULT_AUTH_PASSWORD).activate()
+
+ client_cfg = config._replace(client_mode=True, discovery_spi=from_ignite_cluster(servers))
+
+ # Add new user
+ check_authenticate(servers, TEST_USERNAME, TEST_PASSWORD, True)
+ self.run_with_creds(client_cfg, ADD_USER, TEST_USERNAME, TEST_PASSWORD)
+ check_authenticate(servers, TEST_USERNAME, TEST_PASSWORD)
+
+ # Update user password
+ self.run_with_creds(client_cfg, UPDATE_USER, TEST_USERNAME, TEST_PASSWORD2)
+ check_authenticate(servers, TEST_USERNAME, TEST_PASSWORD2)
+
+ # Remove user
+ self.run_with_creds(client_cfg, REMOVE_USER, TEST_USERNAME, free=False)
+ check_authenticate(servers, TEST_USERNAME, TEST_PASSWORD, True)
Review comment:
Please, add negative tests - check that command wouldn't be executed with the wrong
* test user password
* wrong username
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov merged pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
nizhikov merged pull request #8686:
URL: https://github.com/apache/ignite/pull/8686
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r582896801
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,44 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict = None, user: str = 'server'):
+ """
+ Parse globals with structure like that.
Review comment:
test added
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586528697
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_auth": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"]
+ }
+ }
+ """
+ username, password = None, None
+ if _globals.get('use_auth'):
Review comment:
String `'use_auth'` - should be a constant somewhere(you also may want to replace all usages of 'use_auth' through other code).
To ensure we always use the right dictionary key(e.g 'use_aut' - because this can lead to hardly discovered bugs).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586198421
##########
File path: modules/ducktests/tests/checks/utils/check_get_credentials_from_globals.py
##########
@@ -0,0 +1,59 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Check that get_credentials_from_globals correctly parse Credentials from globals
+"""
+
+import pytest
+from ignitetest.services.utils.auth import get_credentials_from_globals, DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
+
+
+class TestParams:
+ """
+ Constant variables for test
+ """
+
+ test_globals = {
+ "use_auth": "True",
+ "client": {
+ "credentials": ["client", "qwe123"]}}
+ test_globals_default = {
+ "use_auth": "True"}
+ test_globals_no_auth = {}
+
+ expected_username = 'client'
+ expected_password = 'qwe123'
+ expected_username_default = DEFAULT_AUTH_USERNAME
+ expected_password_default = DEFAULT_AUTH_PASSWORD
Review comment:
1) Explicitly duplicated password.
2) Variables with single usage.
Should be inlined.
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_auth": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"]
+ }
+ }
+ """
+ username, password = None, None
+ if _globals.get('use_auth'):
+ if user in _globals and 'credentials' in _globals[user]:
+ username, password = _globals[user]['credentials']
Review comment:
`credentials` duplicated
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_auth": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"]
+ }
+ }
+ """
+ username, password = None, None
+ if _globals.get('use_auth'):
+ if user in _globals and 'credentials' in _globals[user]:
+ username, password = _globals[user]['credentials']
+ else:
+ username, password = DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
Review comment:
This looks extremely weird when we ask credentials for some user and got another user and its password instead.
The default user's password should be returned when (and when) password for default is user requested.
User not found exception, or something like this should be generated if the user was not found at the config.
This method should never return the username.
##########
File path: modules/ducktests/tests/checks/utils/check_get_credentials_from_globals.py
##########
@@ -0,0 +1,59 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Check that get_credentials_from_globals correctly parse Credentials from globals
+"""
+
+import pytest
+from ignitetest.services.utils.auth import get_credentials_from_globals, DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
+
+
+class TestParams:
+ """
+ Constant variables for test
+ """
+
+ test_globals = {
+ "use_auth": "True",
+ "client": {
+ "credentials": ["client", "qwe123"]}}
Review comment:
As for me, this looks extremely odd to define username at credentials when we already have mapping based on usernames.
BTW, should we have `auth` section, similarly to `ssl` here instead of credentials?
For example, two-factor authentication may require sections like token-provider or cell, which are not `credentials`.
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -416,20 +416,17 @@ def __update_node_log_file(self, node):
self.logger.debug(f"rotating {node.log_file} to {rotated_log} on {node.name}")
node.account.ssh(f"mv {node.log_file} {rotated_log}")
- def _update_ssl_config_with_globals(self, dict_name: str, default_jks: str):
+ def _update_ssl_config_with_globals(self, dict_name: str):
"""
Update ssl configuration.
"""
- _dict = self.globals.get(dict_name)
-
- if _dict is not None:
- ssl_context_factory = SslContextFactory(self.install_root, **_dict)
- else:
- ssl_context_factory = SslContextFactory(self.install_root, default_jks)
-
- self.config = self.config._replace(ssl_context_factory=ssl_context_factory)
- self.config = self.config._replace(connector_configuration=ConnectorConfiguration(
- ssl_enabled=True, ssl_context_factory=ssl_context_factory))
+ ssl_params = None
+ if self.config.ssl_params is None:
+ ssl_params = get_ssl_params_from_globals(self.globals, dict_name)
+ if ssl_params:
+ self.config = self.config._replace(ssl_params=ssl_params)
+ self.config = self.config._replace(connector_configuration=ConnectorConfiguration(
+ ssl_enabled=True, ssl_params=ssl_params))
Review comment:
Why we set `ssl-params` to two places?
##########
File path: modules/ducktests/tests/ignitetest/services/ignite.py
##########
@@ -63,8 +62,7 @@ def pids(self, node):
return []
def update_config_with_globals(self):
- if self.globals.get("use_ssl", False):
- self._update_ssl_config_with_globals("server", DEFAULT_SERVER_KEYSTORE)
+ self._update_ssl_config_with_globals("server")
Review comment:
IgniteService may also be client,
as well as IgniteApplicationService may be a server.
##########
File path: modules/ducktests/tests/checks/utils/check_get_credentials_from_globals.py
##########
@@ -0,0 +1,59 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Check that get_credentials_from_globals correctly parse Credentials from globals
+"""
+
+import pytest
+from ignitetest.services.utils.auth import get_credentials_from_globals, DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
+
+
+class TestParams:
+ """
+ Constant variables for test
+ """
+
+ test_globals = {
+ "use_auth": "True",
+ "client": {
+ "credentials": ["client", "qwe123"]}}
+ test_globals_default = {
+ "use_auth": "True"}
+ test_globals_no_auth = {}
+
+ expected_username = 'client'
+ expected_password = 'qwe123'
+ expected_username_default = DEFAULT_AUTH_USERNAME
+ expected_password_default = DEFAULT_AUTH_PASSWORD
+
+
+class CheckCaseJks:
+ """
+ Check that get_credentials_from_globals correctly parse Credentials from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, expected_username, expected_password',
+ [(TestParams.test_globals, TestParams.expected_username, TestParams.expected_password),
+ (TestParams.test_globals_default, TestParams.expected_username_default,
+ TestParams.expected_password_default),
+ (TestParams.test_globals_no_auth, None, None)])
+ def check_parse(test_globals, expected_username, expected_password):
+ """
+ Check function for pytest
+ """
+
+ assert (expected_username, expected_password) == get_credentials_from_globals(test_globals, 'client')
Review comment:
`expected_username` variable explicitly duplicated by 'client' string.
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_params.py
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License
+
+"""
+This module contains classes and utilities for Ignite SslContextFactory.
+
+This file contains three user presets:
+1. server for Ignite(clientMode=False)
+2. client for Ignte(clientMode=True) node and ThinClient
+3. admin for GridClient and console utils (control.sh)
+
+Keystores for this presets are generated automaticaly on creating envoriment
+If you like to specify different certificate for preset you can pass them throw globals
+If you specyfy ssl_params in test, you override globals
+"""
+import os
+
+
+DEFAULT_KEYSTORE = 'server.jks'
+DEFAULT_CLIENT_KEYSTORE = 'client.jks'
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
+DEFAULT_PASSWORD = "123456"
+DEFAULT_TRUSTSTORE = "truststore.jks"
+DEFAULT_ROOT = "/opt/"
+
+default_keystore = {
+ 'server': DEFAULT_KEYSTORE,
+ 'client': DEFAULT_CLIENT_KEYSTORE,
+ 'admin': DEFAULT_ADMIN_KEYSTORE
+}
Review comment:
We agreed that comments will be provided here, but nothing changed.
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_params.py
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License
+
+"""
+This module contains classes and utilities for Ignite SslContextFactory.
+
+This file contains three user presets:
+1. server for Ignite(clientMode=False)
+2. client for Ignte(clientMode=True) node and ThinClient
+3. admin for GridClient and console utils (control.sh)
+
+Keystores for this presets are generated automaticaly on creating envoriment
+If you like to specify different certificate for preset you can pass them throw globals
+If you specyfy ssl_params in test, you override globals
+"""
+import os
+
+
+DEFAULT_KEYSTORE = 'server.jks'
+DEFAULT_CLIENT_KEYSTORE = 'client.jks'
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
+DEFAULT_PASSWORD = "123456"
Review comment:
Should be used only for default keystores.
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_params.py
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License
+
+"""
+This module contains classes and utilities for Ignite SslContextFactory.
+
+This file contains three user presets:
+1. server for Ignite(clientMode=False)
+2. client for Ignte(clientMode=True) node and ThinClient
+3. admin for GridClient and console utils (control.sh)
+
+Keystores for this presets are generated automaticaly on creating envoriment
+If you like to specify different certificate for preset you can pass them throw globals
+If you specyfy ssl_params in test, you override globals
+"""
+import os
+
+
+DEFAULT_KEYSTORE = 'server.jks'
+DEFAULT_CLIENT_KEYSTORE = 'client.jks'
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
+DEFAULT_PASSWORD = "123456"
+DEFAULT_TRUSTSTORE = "truststore.jks"
+DEFAULT_ROOT = "/opt/"
+
+default_keystore = {
+ 'server': DEFAULT_KEYSTORE,
+ 'client': DEFAULT_CLIENT_KEYSTORE,
+ 'admin': DEFAULT_ADMIN_KEYSTORE
+}
+
+
+class SslParams:
+ """
+ Params for Ignite SslContextFactory.
+ """
+
+ # pylint: disable=R0913
+ def __init__(self, root_dir: str = DEFAULT_ROOT,
+ key_store_jks: str = DEFAULT_KEYSTORE, key_store_password: str = DEFAULT_PASSWORD,
+ trust_store_jks: str = DEFAULT_TRUSTSTORE, trust_store_password: str = DEFAULT_PASSWORD,
+ key_store_path: str = None, trust_store_path: str = None):
+ certificate_dir = os.path.join(root_dir, "ignite-dev", "modules", "ducktests", "tests", "certs")
+
+ self.key_store_path = key_store_path if key_store_path is not None \
+ else os.path.join(certificate_dir, key_store_jks)
+ self.key_store_password = key_store_password
+ self.trust_store_path = trust_store_path if trust_store_path is not None \
+ else os.path.join(certificate_dir, trust_store_jks)
+ self.trust_store_password = trust_store_password
+
+
+def get_ssl_params_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_ssl": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"],
+ "ssl": {
+ "key_store_jks": ....
+ }
+ }
+
+ }
+ """
+ root_dir = _globals.get("install_root", DEFAULT_ROOT)
+ ssl_param = None
+ if _globals.get('use_ssl'):
+ if user in _globals and 'ssl' in _globals[user]:
+ ssl_param = _globals[user]['ssl']
+ else:
+ ssl_param = {'key_store_jks': default_keystore[user]}
Review comment:
Case, when the user was not found at default Keystore, should be also handled.
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_params.py
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License
+
+"""
+This module contains classes and utilities for Ignite SslContextFactory.
+
+This file contains three user presets:
+1. server for Ignite(clientMode=False)
+2. client for Ignte(clientMode=True) node and ThinClient
+3. admin for GridClient and console utils (control.sh)
+
+Keystores for this presets are generated automaticaly on creating envoriment
+If you like to specify different certificate for preset you can pass them throw globals
+If you specyfy ssl_params in test, you override globals
+"""
+import os
+
+
+DEFAULT_KEYSTORE = 'server.jks'
+DEFAULT_CLIENT_KEYSTORE = 'client.jks'
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
+DEFAULT_PASSWORD = "123456"
+DEFAULT_TRUSTSTORE = "truststore.jks"
+DEFAULT_ROOT = "/opt/"
+
+default_keystore = {
+ 'server': DEFAULT_KEYSTORE,
+ 'client': DEFAULT_CLIENT_KEYSTORE,
+ 'admin': DEFAULT_ADMIN_KEYSTORE
+}
+
+
+class SslParams:
+ """
+ Params for Ignite SslContextFactory.
+ """
+
+ # pylint: disable=R0913
+ def __init__(self, root_dir: str = DEFAULT_ROOT,
+ key_store_jks: str = DEFAULT_KEYSTORE, key_store_password: str = DEFAULT_PASSWORD,
+ trust_store_jks: str = DEFAULT_TRUSTSTORE, trust_store_password: str = DEFAULT_PASSWORD,
+ key_store_path: str = None, trust_store_path: str = None):
+ certificate_dir = os.path.join(root_dir, "ignite-dev", "modules", "ducktests", "tests", "certs")
+
+ self.key_store_path = key_store_path if key_store_path is not None \
+ else os.path.join(certificate_dir, key_store_jks)
+ self.key_store_password = key_store_password
+ self.trust_store_path = trust_store_path if trust_store_path is not None \
+ else os.path.join(certificate_dir, trust_store_jks)
+ self.trust_store_password = trust_store_password
+
+
+def get_ssl_params_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_ssl": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"],
+ "ssl": {
+ "key_store_jks": ....
+ }
+ }
+
+ }
+ """
+ root_dir = _globals.get("install_root", DEFAULT_ROOT)
+ ssl_param = None
+ if _globals.get('use_ssl'):
+ if user in _globals and 'ssl' in _globals[user]:
+ ssl_param = _globals[user]['ssl']
Review comment:
`ssl` duplicated
##########
File path: modules/ducktests/tests/checks/utils/check_get_ssl_params_from_globals.py
##########
@@ -0,0 +1,100 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Check that SslParams correctly parse SSL params from globals
+"""
+
+import pytest
+from ignitetest.services.utils.ssl.ssl_params import get_ssl_params_from_globals, SslParams, DEFAULT_TRUSTSTORE, \
+ DEFAULT_CLIENT_KEYSTORE, DEFAULT_PASSWORD
+
+INSTALL_ROOT = '/opt'
+CERTIFICATE_DIR = '/opt/ignite-dev/modules/ducktests/tests/certs/'
+
+
+def _compare(expected, actual):
+ """
+ Compare SslParams object with dictionary
+ """
+
+ if expected is None and actual is None:
+ return True
+ if isinstance(expected, dict) and isinstance(actual, SslParams):
+ return expected == actual.__dict__
+ return False
+
+
+class TestParams:
+ """
+ Globals for tests
+ """
+
+ test_globals_jks = {
+ "install_root": INSTALL_ROOT,
+ "use_ssl": "True",
+ "client": {
+ "ssl": {
+ "key_store_jks": "client1.jks",
+ "key_store_password": "qwe123",
+ "trust_store_jks": "truststore.jks",
+ "trust_store_password": "qwe123"}}}
+ test_globals_path = {
+ "install_root": INSTALL_ROOT,
+ "use_ssl": "True",
+ "client": {
+ "ssl": {
+ "key_store_path": "/opt/certs/client1.jks",
+ "key_store_password": "qwe123",
+ "trust_store_path": "/opt/certs/truststore.jks",
+ "trust_store_password": "qwe123"}}}
+ test_globals_default = {
+ "install_root": INSTALL_ROOT,
+ "use_ssl": "True"}
+ test_globals_no_ssl = {
+ "install_root": INSTALL_ROOT}
+
+ expected_ssl_params_jks = {'key_store_path': CERTIFICATE_DIR + 'client1.jks',
+ 'key_store_password': 'qwe123',
+ 'trust_store_path': CERTIFICATE_DIR + 'truststore.jks',
+ 'trust_store_password': 'qwe123'}
+ expected_ssl_params_path = {'key_store_path': '/opt/certs/client1.jks',
+ 'key_store_password': 'qwe123',
+ 'trust_store_path': '/opt/certs/truststore.jks',
+ 'trust_store_password': 'qwe123'}
+ expected_ssl_params_default = {'key_store_path': CERTIFICATE_DIR + DEFAULT_CLIENT_KEYSTORE,
+ 'key_store_password': DEFAULT_PASSWORD,
+ 'trust_store_path': CERTIFICATE_DIR + DEFAULT_TRUSTSTORE,
+ 'trust_store_password': DEFAULT_PASSWORD}
Review comment:
1) Explicitly duplicated password.
2) Variables with single usage.
Should be inlined.
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_configuration/__init__.py
##########
@@ -43,8 +43,10 @@ class IgniteConfiguration(NamedTuple):
data_storage: DataStorageConfiguration = None
caches: list = []
local_host: str = None
- ssl_context_factory: SslContextFactory = None
+ ssl_params: SslParams = None
connector_configuration: ConnectorConfiguration = None
+ auth: bool = False
Review comment:
auth_enabled?
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -37,33 +37,19 @@ class ControlUtility:
BASE_COMMAND = "control.sh"
# pylint: disable=R0913
- def __init__(self, cluster,
- key_store_jks: str = None, key_store_password: str = DEFAULT_PASSWORD,
- trust_store_jks: str = DEFAULT_TRUSTSTORE, trust_store_password: str = DEFAULT_PASSWORD):
+ def __init__(self, cluster, ssl_params=None, username=None, password=DEFAULT_AUTH_PASSWORD):
self._cluster = cluster
self.logger = cluster.context.logger
- if cluster.context.globals.get("use_ssl", False):
- admin_dict = cluster.globals.get("admin", dict())
-
- self.key_store_path = admin_dict.get("key_store_path",
- self.jks_path(admin_dict.get('key_store_jks', DEFAULT_ADMIN_KEYSTORE)))
- self.key_store_password = admin_dict.get('key_store_password', DEFAULT_PASSWORD)
- self.trust_store_path = admin_dict.get("trust_store_path",
- self.jks_path(admin_dict.get('trust_store_jks', DEFAULT_TRUSTSTORE)))
- self.trust_store_password = admin_dict.get('trust_store_password', DEFAULT_PASSWORD)
-
- elif key_store_jks is not None:
- self.key_store_path = self.jks_path(key_store_jks)
- self.key_store_password = key_store_password
- self.trust_store_path = self.jks_path(trust_store_jks)
- self.trust_store_password = trust_store_password
+ if not ssl_params:
+ self.ssl_params = get_ssl_params_from_globals(cluster.context.globals, 'admin')
+ else:
+ self.ssl_params = ssl_params
- def jks_path(self, jks_name: str):
- """
- :return Path to jks file.
- """
- return os.path.join(self._cluster.certificate_dir, jks_name)
+ if not username:
+ self.username, self.password = get_credentials_from_globals(cluster.context.globals, 'admin')
Review comment:
Why credentials for `admin` requested? Do we have such user by default?
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_auth": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"]
+ }
+ }
+ """
+ username, password = None, None
+ if _globals.get('use_auth'):
Review comment:
Should be constant to be used from other files.
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_params.py
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License
+
+"""
+This module contains classes and utilities for Ignite SslContextFactory.
+
+This file contains three user presets:
+1. server for Ignite(clientMode=False)
+2. client for Ignte(clientMode=True) node and ThinClient
+3. admin for GridClient and console utils (control.sh)
+
+Keystores for this presets are generated automaticaly on creating envoriment
+If you like to specify different certificate for preset you can pass them throw globals
+If you specyfy ssl_params in test, you override globals
+"""
+import os
+
+
+DEFAULT_KEYSTORE = 'server.jks'
+DEFAULT_CLIENT_KEYSTORE = 'client.jks'
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
+DEFAULT_PASSWORD = "123456"
+DEFAULT_TRUSTSTORE = "truststore.jks"
+DEFAULT_ROOT = "/opt/"
+
+default_keystore = {
Review comment:
Default names must be constants to be used from other files without duplication.
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_auth": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"]
+ }
+ }
+ """
+ username, password = None, None
+ if _globals.get('use_auth'):
+ if user in _globals and 'credentials' in _globals[user]:
+ username, password = _globals[user]['credentials']
+ else:
+ username, password = DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
+ return username, password
Review comment:
Should we got an exception instead?
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict, user: str):
Review comment:
Should be refactored to *_auth_data() or something like this.
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_params.py
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License
+
+"""
+This module contains classes and utilities for Ignite SslContextFactory.
+
+This file contains three user presets:
+1. server for Ignite(clientMode=False)
+2. client for Ignte(clientMode=True) node and ThinClient
+3. admin for GridClient and console utils (control.sh)
+
+Keystores for this presets are generated automaticaly on creating envoriment
+If you like to specify different certificate for preset you can pass them throw globals
+If you specyfy ssl_params in test, you override globals
+"""
+import os
+
+
+DEFAULT_KEYSTORE = 'server.jks'
+DEFAULT_CLIENT_KEYSTORE = 'client.jks'
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
Review comment:
Should this be generated from names?
See no rear reason to have this as consts.
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -374,6 +397,7 @@ class ControlUtilityError(RemoteCommandError):
"""
Error is raised when control utility failed.
"""
+
Review comment:
The question is about this change!
##########
File path: modules/ducktests/tests/ignitetest/tests/auth_test.py
##########
@@ -0,0 +1,92 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains password based authentication tests
+"""
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.utils.control_utility import ControlUtility, ControlUtilityError
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.utils import ignite_versions, cluster, ignore_if
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+from ignitetest.services.utils.auth import DEFAULT_AUTH_PASSWORD, DEFAULT_AUTH_USERNAME
+
+
+# pylint: disable=W0223
+class AuthenticationTests(IgniteTest):
+ """
+ Tests Ignite Authentication
+ """
+ NUM_NODES = 3
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_correct_password(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ Positive case
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(name='persistent', persistent=True),
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES)
+
+ servers.start()
+
+ control_utility = ControlUtility(cluster=servers,
+ username=DEFAULT_AUTH_USERNAME, password=DEFAULT_AUTH_PASSWORD)
+ control_utility.activate()
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignore_if(lambda version, globals_dict: globals_dict.get("use_auth", False)) # Globals overrides test params
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_wrong_password(self, ignite_version):
Review comment:
This method code duplicates `test_activate_correct_password` for 80%.
We should avoid such duplications.
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_params.py
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License
+
+"""
+This module contains classes and utilities for Ignite SslContextFactory.
+
+This file contains three user presets:
+1. server for Ignite(clientMode=False)
+2. client for Ignte(clientMode=True) node and ThinClient
+3. admin for GridClient and console utils (control.sh)
+
+Keystores for this presets are generated automaticaly on creating envoriment
+If you like to specify different certificate for preset you can pass them throw globals
+If you specyfy ssl_params in test, you override globals
+"""
+import os
+
+
+DEFAULT_KEYSTORE = 'server.jks'
+DEFAULT_CLIENT_KEYSTORE = 'client.jks'
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
+DEFAULT_PASSWORD = "123456"
+DEFAULT_TRUSTSTORE = "truststore.jks"
+DEFAULT_ROOT = "/opt/"
+
+default_keystore = {
+ 'server': DEFAULT_KEYSTORE,
+ 'client': DEFAULT_CLIENT_KEYSTORE,
+ 'admin': DEFAULT_ADMIN_KEYSTORE
+}
+
+
+class SslParams:
+ """
+ Params for Ignite SslContextFactory.
+ """
+
+ # pylint: disable=R0913
+ def __init__(self, root_dir: str = DEFAULT_ROOT,
+ key_store_jks: str = DEFAULT_KEYSTORE, key_store_password: str = DEFAULT_PASSWORD,
+ trust_store_jks: str = DEFAULT_TRUSTSTORE, trust_store_password: str = DEFAULT_PASSWORD,
+ key_store_path: str = None, trust_store_path: str = None):
+ certificate_dir = os.path.join(root_dir, "ignite-dev", "modules", "ducktests", "tests", "certs")
+
+ self.key_store_path = key_store_path if key_store_path is not None \
+ else os.path.join(certificate_dir, key_store_jks)
+ self.key_store_password = key_store_password
+ self.trust_store_path = trust_store_path if trust_store_path is not None \
+ else os.path.join(certificate_dir, trust_store_jks)
+ self.trust_store_password = trust_store_password
+
+
+def get_ssl_params_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_ssl": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"],
+ "ssl": {
+ "key_store_jks": ....
+ }
+ }
+
+ }
+ """
+ root_dir = _globals.get("install_root", DEFAULT_ROOT)
+ ssl_param = None
+ if _globals.get('use_ssl'):
Review comment:
Should be constant to be used at other files.
##########
File path: modules/ducktests/tests/ignitetest/services/ignite_app.py
##########
@@ -109,5 +108,4 @@ def extract_results(self, name):
return res
def update_config_with_globals(self):
- if self.globals.get("use_ssl", False):
- self._update_ssl_config_with_globals("client", DEFAULT_CLIENT_KEYSTORE)
+ self._update_ssl_config_with_globals("client")
Review comment:
Why "client"?
##########
File path: modules/ducktests/tests/ignitetest/tests/auth_test.py
##########
@@ -0,0 +1,92 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains password based authentication tests
+"""
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.utils.control_utility import ControlUtility, ControlUtilityError
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.utils import ignite_versions, cluster, ignore_if
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+from ignitetest.services.utils.auth import DEFAULT_AUTH_PASSWORD, DEFAULT_AUTH_USERNAME
+
+
+# pylint: disable=W0223
+class AuthenticationTests(IgniteTest):
+ """
+ Tests Ignite Authentication
+ """
+ NUM_NODES = 3
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_correct_password(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ Positive case
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(name='persistent', persistent=True),
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES)
+
+ servers.start()
+
+ control_utility = ControlUtility(cluster=servers,
+ username=DEFAULT_AUTH_USERNAME, password=DEFAULT_AUTH_PASSWORD)
+ control_utility.activate()
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignore_if(lambda version, globals_dict: globals_dict.get("use_auth", False)) # Globals overrides test params
Review comment:
It's a good case to ignore some versions where some feature not supported, but this seems to be illegal usage.
Do you expect that someone will explicitly set "`use_auth`: False" at globals, while this should be false by default?
Sound odd to expect such config.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r577512953
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -313,6 +305,33 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _parse_ssl_params(self, user, globals_dict, **kwargs):
+ ssl_dict = None
+ if globals_dict.get('use_ssl', False):
+ ssl_dict = globals_dict.get(user, {}).get('ssl', {})
+ elif kwargs.get('key_store_jks') is not None or kwargs.get('key_store_path') is not None:
+ ssl_dict = kwargs
+ return None if ssl_dict is None else \
+ SslContextFactory(key_store_path=ssl_dict.get("key_store_path",
+ self.jks_path(
+ ssl_dict.get('key_store_jks', DEFAULT_ADMIN_KEYSTORE))),
+ key_store_password=ssl_dict.get('key_store_password', DEFAULT_PASSWORD),
+ trust_store_path=ssl_dict.get("trust_store_path",
+ self.jks_path(
+ ssl_dict.get('trust_store_jks', DEFAULT_TRUSTSTORE))),
+ trust_store_password=ssl_dict.get('trust_store_password', DEFAULT_PASSWORD))
+
+ @staticmethod
+ def _parse_creds(user, globals_dict, **kwargs):
+ creds_dict = None
+ if globals_dict.get('use_auth', False):
+ creds_dict = globals_dict.get(user, {}).get('creds', {})
Review comment:
Please, never, never write like that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r591412016
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_auth": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"]
+ }
+ }
+ """
+ username, password = None, None
+ if _globals.get('use_auth'):
+ if user in _globals and 'credentials' in _globals[user]:
+ username, password = _globals[user]['credentials']
+ else:
+ username, password = DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
Review comment:
Once again!!!
This method should never return the username.
You already know the username!
And you should not provide defaults when you asked for something else.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r579619603
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -376,6 +366,41 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _parse_ssl_params(self, user, globals_dict, **kwargs):
+ ssl_dict = None
+ if globals_dict.get('use_ssl'):
+ if user in globals_dict and 'ssl' in globals_dict[user]:
+ ssl_dict = globals_dict[user]['ssl']
+ else:
+ ssl_dict = {}
+ elif kwargs.get('key_store_jks') or kwargs.get('key_store_path'):
+ ssl_dict = kwargs
+ return None if ssl_dict is None else \
+ SslContextFactory(key_store_path=self.__get_store_path('key_store', ssl_dict),
+ key_store_password=ssl_dict.get('key_store_password', DEFAULT_PASSWORD),
+ trust_store_path=self.__get_store_path('trust_store', ssl_dict),
+ trust_store_password=ssl_dict.get('trust_store_password', DEFAULT_PASSWORD))
+
+ def __get_store_path(self, store_type, ssl_dict):
+ path_key = f'{store_type}_path'
+ store_name = f'{store_type}_jks'
+ default_name = DEFAULT_TRUSTSTORE if store_type == 'trust_store' else DEFAULT_ADMIN_KEYSTORE
+ return ssl_dict.get(path_key, self.jks_path(ssl_dict.get(store_name, default_name)))
+
+ @staticmethod
+ def _parse_creds(user, globals_dict, **kwargs):
+ creds_dict = None
+ if globals_dict.get('use_auth'):
+ if user in globals_dict and 'creds' in globals_dict[user]:
+ creds_dict = globals_dict[user]['creds']
+ else:
+ creds_dict = {}
+ elif kwargs.get('login'):
+ creds_dict = kwargs
+ return None if creds_dict is None else \
+ (creds_dict.get("login", DEFAULT_AUTH_LOGIN),
Review comment:
There are 8 named variables to take from kwargs. (6 for ssl and 2 for creds)
I think that 8 additional named variables in the constructor is too much.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586443585
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_context.py
##########
@@ -19,20 +19,24 @@
import os
DEFAULT_PASSWORD = "123456"
-DEFAULT_SERVER_KEYSTORE = "server.jks"
-DEFAULT_CLIENT_KEYSTORE = "client.jks"
-DEFAULT_ADMIN_KEYSTORE = "admin.jks"
DEFAULT_TRUSTSTORE = "truststore.jks"
+DEFAULT_ROOT = "/opt/"
+default_keystore = {
+ 'server': 'server.jks',
+ 'client': 'client.jks',
Review comment:
I believe that if something is used, it is enough to say "It is used"
Where is duplication?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r591329040
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_auth": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"]
+ }
+ }
+ """
+ username, password = None, None
+ if _globals.get('use_auth'):
+ if user in _globals and 'credentials' in _globals[user]:
+ username, password = _globals[user]['credentials']
+ else:
+ username, password = DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
+ return username, password
Review comment:
So, what about an exception?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r582901451
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_context.py
##########
@@ -19,20 +19,24 @@
import os
DEFAULT_PASSWORD = "123456"
-DEFAULT_SERVER_KEYSTORE = "server.jks"
-DEFAULT_CLIENT_KEYSTORE = "client.jks"
-DEFAULT_ADMIN_KEYSTORE = "admin.jks"
DEFAULT_TRUSTSTORE = "truststore.jks"
+DEFAULT_ROOT = "/opt/"
+default_keystore = {
+ 'server': 'server.jks',
+ 'client': 'client.jks',
+ 'admin': 'admin.jks'
+}
-class SslContextFactory:
+
+class SslContext:
"""
Ignite SslContextFactory.
"""
# pylint: disable=R0913
- def __init__(self, root_dir: str = "/opt",
- key_store_jks: str = DEFAULT_SERVER_KEYSTORE, key_store_password: str = DEFAULT_PASSWORD,
+ def __init__(self, root_dir: str = DEFAULT_ROOT,
+ key_store_jks: str = default_keystore['server'], key_store_password: str = DEFAULT_PASSWORD,
Review comment:
changed to DEFAULT_KEYSTORE
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r594317935
##########
File path: modules/ducktests/tests/ignitetest/services/utils/jmx_utils.py
##########
@@ -96,7 +96,7 @@ def mbean_attribute(self, mbean, attr):
return iter(s.strip() for s in self.__run_cmd(cmd))
def __run_cmd(self, cmd):
- return self.node.account.ssh_capture(cmd, allow_fail=False, callback=str)
+ return self.node.account.ssh_capture(cmd, allow_fail=False, callback=str, combine_stderr=False)
Review comment:
This is just a "easy fix" to run this function with JDK>=9.
This function takes the first line from the console output.
If we use JDK> = 9 then the first line will be a warning from JDK in stderr
Since we don't really need stderr, we can ignore it and workaround this situation
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586468907
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_params.py
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License
+
+"""
+This module contains classes and utilities for Ignite SslContextFactory.
+
+This file contains three user presets:
+1. server for Ignite(clientMode=False)
+2. client for Ignte(clientMode=True) node and ThinClient
+3. admin for GridClient and console utils (control.sh)
+
+Keystores for this presets are generated automaticaly on creating envoriment
+If you like to specify different certificate for preset you can pass them throw globals
+If you specyfy ssl_params in test, you override globals
+"""
+import os
+
+
+DEFAULT_KEYSTORE = 'server.jks'
+DEFAULT_CLIENT_KEYSTORE = 'client.jks'
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
+DEFAULT_PASSWORD = "123456"
+DEFAULT_TRUSTSTORE = "truststore.jks"
+DEFAULT_ROOT = "/opt/"
+
+default_keystore = {
+ 'server': DEFAULT_KEYSTORE,
+ 'client': DEFAULT_CLIENT_KEYSTORE,
+ 'admin': DEFAULT_ADMIN_KEYSTORE
+}
+
+
+class SslParams:
+ """
+ Params for Ignite SslContextFactory.
+ """
+
+ # pylint: disable=R0913
+ def __init__(self, root_dir: str = DEFAULT_ROOT,
+ key_store_jks: str = DEFAULT_KEYSTORE, key_store_password: str = DEFAULT_PASSWORD,
+ trust_store_jks: str = DEFAULT_TRUSTSTORE, trust_store_password: str = DEFAULT_PASSWORD,
+ key_store_path: str = None, trust_store_path: str = None):
+ certificate_dir = os.path.join(root_dir, "ignite-dev", "modules", "ducktests", "tests", "certs")
+
+ self.key_store_path = key_store_path if key_store_path is not None \
+ else os.path.join(certificate_dir, key_store_jks)
+ self.key_store_password = key_store_password
+ self.trust_store_path = trust_store_path if trust_store_path is not None \
+ else os.path.join(certificate_dir, trust_store_jks)
+ self.trust_store_password = trust_store_password
+
+
+def get_ssl_params_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_ssl": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"],
+ "ssl": {
+ "key_store_jks": ....
+ }
+ }
+
+ }
+ """
+ root_dir = _globals.get("install_root", DEFAULT_ROOT)
+ ssl_param = None
+ if _globals.get('use_ssl'):
+ if user in _globals and 'ssl' in _globals[user]:
+ ssl_param = _globals[user]['ssl']
Review comment:
it' not a variable or pameter. this is dictionary key
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r594279002
##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/authentication/UserModifyingApplication.java
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.ducktest.tests.authentication;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.ducktest.utils.IgniteAwareApplication;
+import org.apache.ignite.internal.processors.authentication.AuthorizationContext;
+import org.apache.ignite.internal.processors.authentication.IgniteAuthenticationProcessor;
+import org.apache.ignite.internal.processors.rest.GridRestCommand;
+
+/**
+ * Simple application that modify users
Review comment:
Dot in the end
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r577513245
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -313,6 +305,33 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _parse_ssl_params(self, user, globals_dict, **kwargs):
+ ssl_dict = None
+ if globals_dict.get('use_ssl', False):
+ ssl_dict = globals_dict.get(user, {}).get('ssl', {})
Review comment:
Please, never, never write like that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r572904731
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,148 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params pasrsing
+"""
+
+import pytest
+from ignitetest.services.utils.control_utility import ControlUtility
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.certificate_dir = ''
+
+
+class Context:
+ """
+ Need this to initialise ControlUtility
+ """
+ def __init__(self, globals_dict):
+ self.logger = ''
+ self.globals = globals_dict
+
+
+class CheckControlUtility:
+ """
+ Check that control_utulity.py correctly parse globals
+ """
+
+ test_admin_data_full = {"login": "admin1", "password": "qwe123",
+ "ssl": {"key_store_jks": "admin1.jks", "key_store_password": "qwe123",
+ "trust_store_jks": "truststore.jks", "trust_store_password": "qwe123"}}
+
+ test_admin_data_ssl = {
+ "ssl": {"key_store_jks": "admin1.jks", "key_store_password": "qwe123", "trust_store_jks": "truststore.jks",
+ "trust_store_password": "qwe123"}}
+ test_admin_data_auth = {"login": "admin1", "password": "qwe123"}
+
+ globals_full = {
+ 'use_ssl': True,
+ 'use_auth': True,
+ 'admin': test_admin_data_full
+ }
+
+ globals_auth = {
+ 'use_ssl': True,
+ 'use_auth': True,
+ 'admin': test_admin_data_auth
+ }
+
+ globals_ssl = {
+ 'use_ssl': True,
+ 'use_auth': True,
+ 'admin': test_admin_data_ssl
+ }
+
+ globals_no_ssl = {
+ 'use_auth': True,
+ 'admin': test_admin_data_full
+ }
+
+ globals_no_auth = {
+ 'use_ssl': True,
+ 'admin': test_admin_data_full
+ }
+
+ globals_no_ssl_no_auth = {
+ 'admin': test_admin_data_full
+ }
+
+ globals_ssl_auth = {
+ 'use_ssl': True,
+ 'use_auth': True
+ }
+
+ globals_nil = {}
+
+ @staticmethod
+ @pytest.mark.parametrize("test_globals, test_kwargs, expected",
+ [(globals_full, {}, {'key_store_path': 'admin1.jks', 'key_store_password': 'qwe123',
Review comment:
I put the results in a separate variable, but they are all different, so it's still hard to read
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r594281830
##########
File path: modules/ducktests/tests/checks/utils/check_get_credentials_from_globals.py
##########
@@ -0,0 +1,49 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Check that get_credentials correctly parse Credentials from globals
+"""
+
+import pytest
+from ignitetest.services.utils.auth import get_credentials, DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD, \
+ DEFAULT_CREDENTIALS_KEY, AUTHENTICATION_KEY, ENABLED_KEY
+
+TEST_USERNAME = "admin"
+TEST_PASSWORD = "qwe123"
+
+
+class CheckCaseJks:
+ """
+ Check that get_credentials correctly parse Credentials from globals
+ Posible structure is:
+ {"authentication": {
+ "enabled": true,
+ "default_credentials": ["admin","qwe123"]}}
Review comment:
Why this key named "default_credentials"?
Do we have not default credentials support?
Can we just make "login" and "password" attributes?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8686: Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r573615239
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -292,11 +276,17 @@ def __run(self, cmd):
def __form_cmd(self, node, cmd):
ssl = ""
- if hasattr(self, 'key_store_path'):
- ssl = f" --keystore {self.key_store_path} --keystore-password {self.key_store_password} " \
- f"--truststore {self.trust_store_path} --truststore-password {self.trust_store_password}"
-
- return self._cluster.script(f"{self.BASE_COMMAND} --host {node.account.externally_routable_ip} {cmd} {ssl}")
+ if hasattr(self.creds, 'key_store_path'):
Review comment:
As it self.creds is a class now, then it does have 'key_store_path' attr, doesn't it? Should it to be replaced with check for None?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586566939
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_auth": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"]
+ }
+ }
+ """
+ username, password = None, None
+ if _globals.get('use_auth'):
Review comment:
ok
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r581934175
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,44 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict = None, user: str = 'server'):
+ """
+ Parse globals with structure like that.
+ {
+ "use_ssl": True,
Review comment:
use_auth?
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,44 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict = None, user: str = 'server'):
Review comment:
any reason to create an empty package with this method?
should this be inlined to ControlUtility init?
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_context.py
##########
@@ -19,20 +19,24 @@
import os
DEFAULT_PASSWORD = "123456"
-DEFAULT_SERVER_KEYSTORE = "server.jks"
-DEFAULT_CLIENT_KEYSTORE = "client.jks"
-DEFAULT_ADMIN_KEYSTORE = "admin.jks"
DEFAULT_TRUSTSTORE = "truststore.jks"
+DEFAULT_ROOT = "/opt/"
+default_keystore = {
+ 'server': 'server.jks',
+ 'client': 'client.jks',
+ 'admin': 'admin.jks'
+}
-class SslContextFactory:
+
+class SslContext:
"""
Ignite SslContextFactory.
"""
# pylint: disable=R0913
- def __init__(self, root_dir: str = "/opt",
- key_store_jks: str = DEFAULT_SERVER_KEYSTORE, key_store_password: str = DEFAULT_PASSWORD,
+ def __init__(self, root_dir: str = DEFAULT_ROOT,
+ key_store_jks: str = default_keystore['server'], key_store_password: str = DEFAULT_PASSWORD,
Review comment:
reason to make 'server' default?
##########
File path: modules/ducktests/tests/ignitetest/tests/auth_test.py
##########
@@ -0,0 +1,90 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains password based authentication tests
+"""
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.utils.control_utility import ControlUtility, ControlUtilityError
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.utils import ignite_versions, cluster, ignore_if
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+
+# pylint: disable=W0223
+class AuthenticationTests(IgniteTest):
+ """
+ Tests Ignite Authentication
+ """
+ NUM_NODES = 3
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_correct_password(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ Positive case
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(name='persistent', persistent=True),
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES)
+
+ servers.start()
+
+ control_utility = ControlUtility(cluster=servers, username="ignite", password="ignite")
Review comment:
it looks like you're checking connection using default user/pass but writing them not as a reference to constant.
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/connector_configuration.py
##########
@@ -28,4 +28,4 @@ class ConnectorConfiguration(NamedTuple):
Used to connect from ControlUtility (control.sh).
"""
ssl_enabled: bool = False
- ssl_context_factory: SslContextFactory = None
+ ssl_context: SslContext = None
Review comment:
why get rid of the factory since it's the factory?
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,44 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict = None, user: str = 'server'):
+ """
+ Parse globals with structure like that.
Review comment:
should be explained by proper test, not by such comment
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_context.py
##########
@@ -43,3 +47,28 @@ def __init__(self, root_dir: str = "/opt",
self.trust_store_path = trust_store_path if trust_store_path is not None \
else os.path.join(certificate_dir, trust_store_jks)
self.trust_store_password = trust_store_password
+
+
+def get_ssl_context_from_globals(_globals: dict, user: str = 'server'):
Review comment:
reason to male 'server' default?
##########
File path: modules/ducktests/tests/ignitetest/services/ignite.py
##########
@@ -63,8 +62,7 @@ def pids(self, node):
return []
def update_config_with_globals(self):
- if self.globals.get("use_ssl", False):
- self._update_ssl_config_with_globals("server", DEFAULT_SERVER_KEYSTORE)
+ self._update_ssl_config_with_globals("server")
Review comment:
why server? Is it restricted to create IgniteApp as a client?
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_context.py
##########
@@ -19,20 +19,24 @@
import os
DEFAULT_PASSWORD = "123456"
-DEFAULT_SERVER_KEYSTORE = "server.jks"
-DEFAULT_CLIENT_KEYSTORE = "client.jks"
-DEFAULT_ADMIN_KEYSTORE = "admin.jks"
DEFAULT_TRUSTSTORE = "truststore.jks"
+DEFAULT_ROOT = "/opt/"
+default_keystore = {
+ 'server': 'server.jks',
+ 'client': 'client.jks',
Review comment:
is not used
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_context.py
##########
@@ -19,20 +19,24 @@
import os
DEFAULT_PASSWORD = "123456"
-DEFAULT_SERVER_KEYSTORE = "server.jks"
-DEFAULT_CLIENT_KEYSTORE = "client.jks"
-DEFAULT_ADMIN_KEYSTORE = "admin.jks"
DEFAULT_TRUSTSTORE = "truststore.jks"
+DEFAULT_ROOT = "/opt/"
+default_keystore = {
+ 'server': 'server.jks',
+ 'client': 'client.jks',
+ 'admin': 'admin.jks'
+}
-class SslContextFactory:
+
+class SslContext:
"""
Ignite SslContextFactory.
"""
Review comment:
why changed the class name but keep comment and semantic?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r579621038
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -376,6 +366,41 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _parse_ssl_params(self, user, globals_dict, **kwargs):
+ ssl_dict = None
+ if globals_dict.get('use_ssl'):
+ if user in globals_dict and 'ssl' in globals_dict[user]:
+ ssl_dict = globals_dict[user]['ssl']
+ else:
+ ssl_dict = {}
+ elif kwargs.get('key_store_jks') or kwargs.get('key_store_path'):
+ ssl_dict = kwargs
+ return None if ssl_dict is None else \
+ SslContextFactory(key_store_path=self.__get_store_path('key_store', ssl_dict),
+ key_store_password=ssl_dict.get('key_store_password', DEFAULT_PASSWORD),
+ trust_store_path=self.__get_store_path('trust_store', ssl_dict),
+ trust_store_password=ssl_dict.get('trust_store_password', DEFAULT_PASSWORD))
+
+ def __get_store_path(self, store_type, ssl_dict):
+ path_key = f'{store_type}_path'
+ store_name = f'{store_type}_jks'
+ default_name = DEFAULT_TRUSTSTORE if store_type == 'trust_store' else DEFAULT_ADMIN_KEYSTORE
+ return ssl_dict.get(path_key, self.jks_path(ssl_dict.get(store_name, default_name)))
+
+ @staticmethod
+ def _parse_creds(user, globals_dict, **kwargs):
+ creds_dict = None
+ if globals_dict.get('use_auth'):
+ if user in globals_dict and 'creds' in globals_dict[user]:
+ creds_dict = globals_dict[user]['creds']
+ else:
+ creds_dict = {}
+ elif kwargs.get('login'):
+ creds_dict = kwargs
+ return None if creds_dict is None else \
+ (creds_dict.get("login", DEFAULT_AUTH_LOGIN),
Review comment:
Too much -- this function. It is impossible even to undestand what it does.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r577513730
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -313,6 +305,33 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _parse_ssl_params(self, user, globals_dict, **kwargs):
+ ssl_dict = None
+ if globals_dict.get('use_ssl', False):
+ ssl_dict = globals_dict.get(user, {}).get('ssl', {})
+ elif kwargs.get('key_store_jks') is not None or kwargs.get('key_store_path') is not None:
+ ssl_dict = kwargs
Review comment:
just `'key_store_jks' in kwargs`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r594279099
##########
File path: modules/ducktests/src/main/java/org/apache/ignite/internal/ducktest/tests/authentication/UserModifyingApplication.java
##########
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.ducktest.tests.authentication;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.ducktest.utils.IgniteAwareApplication;
+import org.apache.ignite.internal.processors.authentication.AuthorizationContext;
+import org.apache.ignite.internal.processors.authentication.IgniteAuthenticationProcessor;
+import org.apache.ignite.internal.processors.rest.GridRestCommand;
+
+/**
+ * Simple application that modify users
+ * Posible operations: 0 - Add User 1 - Update User 2 - Remove User
Review comment:
Dot in the end
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r577509176
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -420,10 +420,10 @@ def _update_ssl_config_with_globals(self, dict_name: str, default_jks: str):
"""
Update ssl configuration.
"""
- _dict = self.globals.get(dict_name)
- if _dict is not None:
- ssl_context_factory = SslContextFactory(self.install_root, **_dict)
+ _dict = self.globals.get(dict_name)
+ if _dict is not None and "ssl" in _dict:
+ ssl_context_factory = SslContextFactory(self.install_root, **_dict.get("ssl"))
else:
Review comment:
just `**_dict['ssl']`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586456671
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_context.py
##########
@@ -19,20 +19,24 @@
import os
DEFAULT_PASSWORD = "123456"
-DEFAULT_SERVER_KEYSTORE = "server.jks"
-DEFAULT_CLIENT_KEYSTORE = "client.jks"
-DEFAULT_ADMIN_KEYSTORE = "admin.jks"
DEFAULT_TRUSTSTORE = "truststore.jks"
+DEFAULT_ROOT = "/opt/"
+default_keystore = {
+ 'server': 'server.jks',
+ 'client': 'client.jks',
Review comment:
This function trying to get SSL params from globals, if globals don't have SSL params, then it use defalts
if we run update_ssl_config_with_globals("client") then we get client.jks as keystore
if we run update_ssl_config_with_globals("server") then we get server.jks as keystore
Used in
modules/ducktests/tests/ignitetest/services/ignite.py:65
modules/ducktests/tests/ignitetest/services/ignite_app.py:111
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r579019160
##########
File path: modules/ducktests/tests/ignitetest/tests/auth_test.py
##########
@@ -0,0 +1,90 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains password based authentication tests
+"""
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.utils.control_utility import ControlUtility, ControlUtilityError
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.utils import ignite_versions, cluster, ignore_if
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+
+# pylint: disable=W0223
+class AuthenticationTests(IgniteTest):
+ """
+ Tests Ignite Authentication
+ """
+ NUM_NODES = 3
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_good_user(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ Positive case
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(name='persistent', persistent=True),
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES, startup_timeout_sec=60)
+
+ servers.start()
+
+ control_utility = ControlUtility(cluster=servers, login="ignite", password="ignite")
+ control_utility.activate()
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignore_if(lambda version, globals_dict: globals_dict.get("use_auth", False)) # Globals overrides test params
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_bad_user(self, ignite_version):
Review comment:
what is good or bad?
should we use more clear phrases like "wrong password" or successful/unsuccessful?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r572907721
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -374,6 +397,7 @@ class ControlUtilityError(RemoteCommandError):
"""
Error is raised when control utility failed.
"""
+
Review comment:
Are you toking about whole PR meaning?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r579312412
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -376,6 +366,41 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _parse_ssl_params(self, user, globals_dict, **kwargs):
+ ssl_dict = None
+ if globals_dict.get('use_ssl'):
+ if user in globals_dict and 'ssl' in globals_dict[user]:
+ ssl_dict = globals_dict[user]['ssl']
+ else:
+ ssl_dict = {}
+ elif kwargs.get('key_store_jks') or kwargs.get('key_store_path'):
+ ssl_dict = kwargs
+ return None if ssl_dict is None else \
+ SslContextFactory(key_store_path=self.__get_store_path('key_store', ssl_dict),
+ key_store_password=ssl_dict.get('key_store_password', DEFAULT_PASSWORD),
+ trust_store_path=self.__get_store_path('trust_store', ssl_dict),
+ trust_store_password=ssl_dict.get('trust_store_password', DEFAULT_PASSWORD))
+
+ def __get_store_path(self, store_type, ssl_dict):
+ path_key = f'{store_type}_path'
+ store_name = f'{store_type}_jks'
+ default_name = DEFAULT_TRUSTSTORE if store_type == 'trust_store' else DEFAULT_ADMIN_KEYSTORE
+ return ssl_dict.get(path_key, self.jks_path(ssl_dict.get(store_name, default_name)))
+
+ @staticmethod
+ def _parse_creds(user, globals_dict, **kwargs):
+ creds_dict = None
+ if globals_dict.get('use_auth'):
+ if user in globals_dict and 'creds' in globals_dict[user]:
+ creds_dict = globals_dict[user]['creds']
+ else:
+ creds_dict = {}
+ elif kwargs.get('login'):
+ creds_dict = kwargs
+ return None if creds_dict is None else \
+ (creds_dict.get("login", DEFAULT_AUTH_LOGIN),
Review comment:
Its more convenient to return None, None, and you should not user braces here
Could you explain what the purpose of this function? When you use 'login' as arg?
Why you use kwargs var, if you use only one named variable?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r577506126
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -313,6 +305,33 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _parse_ssl_params(self, user, globals_dict, **kwargs):
+ ssl_dict = None
+ if globals_dict.get('use_ssl', False):
+ ssl_dict = globals_dict.get(user, {}).get('ssl', {})
+ elif kwargs.get('key_store_jks') is not None or kwargs.get('key_store_path') is not None:
+ ssl_dict = kwargs
+ return None if ssl_dict is None else \
+ SslContextFactory(key_store_path=ssl_dict.get("key_store_path",
+ self.jks_path(
+ ssl_dict.get('key_store_jks', DEFAULT_ADMIN_KEYSTORE))),
+ key_store_password=ssl_dict.get('key_store_password', DEFAULT_PASSWORD),
+ trust_store_path=ssl_dict.get("trust_store_path",
+ self.jks_path(
+ ssl_dict.get('trust_store_jks', DEFAULT_TRUSTSTORE))),
+ trust_store_password=ssl_dict.get('trust_store_password', DEFAULT_PASSWORD))
+
+ @staticmethod
+ def _parse_creds(user, globals_dict, **kwargs):
+ creds_dict = None
+ if globals_dict.get('use_auth', False):
Review comment:
You don't need here default key, just
```
if globals_dict.get('use_auth'):
.....
elif 'login' in kwargs
....
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586317906
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,44 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict = None, user: str = 'server'):
+ """
+ Parse globals with structure like that.
Review comment:
But we still have a comment, which will be broken on first refactoring.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586438315
##########
File path: modules/ducktests/tests/checks/utils/check_get_ssl_params_from_globals.py
##########
@@ -0,0 +1,100 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Check that SslParams correctly parse SSL params from globals
+"""
+
+import pytest
+from ignitetest.services.utils.ssl.ssl_params import get_ssl_params_from_globals, SslParams, DEFAULT_TRUSTSTORE, \
+ DEFAULT_CLIENT_KEYSTORE, DEFAULT_PASSWORD
+
+INSTALL_ROOT = '/opt'
+CERTIFICATE_DIR = '/opt/ignite-dev/modules/ducktests/tests/certs/'
+
+
+def _compare(expected, actual):
+ """
+ Compare SslParams object with dictionary
+ """
+
+ if expected is None and actual is None:
+ return True
+ if isinstance(expected, dict) and isinstance(actual, SslParams):
+ return expected == actual.__dict__
+ return False
+
+
+class TestParams:
+ """
+ Globals for tests
+ """
+
+ test_globals_jks = {
+ "install_root": INSTALL_ROOT,
+ "use_ssl": "True",
+ "client": {
+ "ssl": {
+ "key_store_jks": "client1.jks",
+ "key_store_password": "qwe123",
+ "trust_store_jks": "truststore.jks",
+ "trust_store_password": "qwe123"}}}
+ test_globals_path = {
+ "install_root": INSTALL_ROOT,
+ "use_ssl": "True",
+ "client": {
+ "ssl": {
+ "key_store_path": "/opt/certs/client1.jks",
+ "key_store_password": "qwe123",
+ "trust_store_path": "/opt/certs/truststore.jks",
+ "trust_store_password": "qwe123"}}}
+ test_globals_default = {
+ "install_root": INSTALL_ROOT,
+ "use_ssl": "True"}
+ test_globals_no_ssl = {
+ "install_root": INSTALL_ROOT}
+
+ expected_ssl_params_jks = {'key_store_path': CERTIFICATE_DIR + 'client1.jks',
+ 'key_store_password': 'qwe123',
+ 'trust_store_path': CERTIFICATE_DIR + 'truststore.jks',
+ 'trust_store_password': 'qwe123'}
+ expected_ssl_params_path = {'key_store_path': '/opt/certs/client1.jks',
+ 'key_store_password': 'qwe123',
+ 'trust_store_path': '/opt/certs/truststore.jks',
+ 'trust_store_password': 'qwe123'}
+ expected_ssl_params_default = {'key_store_path': CERTIFICATE_DIR + DEFAULT_CLIENT_KEYSTORE,
+ 'key_store_password': DEFAULT_PASSWORD,
+ 'trust_store_path': CERTIFICATE_DIR + DEFAULT_TRUSTSTORE,
+ 'trust_store_password': DEFAULT_PASSWORD}
Review comment:
Duplicates removed. I left some variables outside because inlining would decrease readability
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r582903727
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/connector_configuration.py
##########
@@ -28,4 +28,4 @@ class ConnectorConfiguration(NamedTuple):
Used to connect from ControlUtility (control.sh).
"""
ssl_enabled: bool = False
- ssl_context_factory: SslContextFactory = None
+ ssl_context: SslContext = None
Review comment:
It's not a factory, it just params really
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r579620114
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -37,27 +39,11 @@ class ControlUtility:
BASE_COMMAND = "control.sh"
# pylint: disable=R0913
- def __init__(self, cluster,
- key_store_jks: str = None, key_store_password: str = DEFAULT_PASSWORD,
- trust_store_jks: str = DEFAULT_TRUSTSTORE, trust_store_password: str = DEFAULT_PASSWORD):
+ def __init__(self, cluster, **kwargs):
self._cluster = cluster
self.logger = cluster.context.logger
-
- if cluster.context.globals.get("use_ssl", False):
- admin_dict = cluster.globals.get("admin", dict())
-
- self.key_store_path = admin_dict.get("key_store_path",
- self.jks_path(admin_dict.get('key_store_jks', DEFAULT_ADMIN_KEYSTORE)))
- self.key_store_password = admin_dict.get('key_store_password', DEFAULT_PASSWORD)
- self.trust_store_path = admin_dict.get("trust_store_path",
- self.jks_path(admin_dict.get('trust_store_jks', DEFAULT_TRUSTSTORE)))
- self.trust_store_password = admin_dict.get('trust_store_password', DEFAULT_PASSWORD)
-
- elif key_store_jks is not None:
- self.key_store_path = self.jks_path(key_store_jks)
- self.key_store_password = key_store_password
- self.trust_store_path = self.jks_path(trust_store_jks)
- self.trust_store_password = trust_store_password
+ self.ssl_context = self._parse_ssl_params("admin", cluster.context.globals, **kwargs)
+ self.creds = self._parse_creds("admin", cluster.context.globals, **kwargs)
Review comment:
username and password are always used together, i don't see any good reason to separate them?
And we have 2 another passwords here (for keystore and for truststore)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r582055983
##########
File path: modules/ducktests/tests/ignitetest/services/ignite.py
##########
@@ -63,8 +62,7 @@ def pids(self, node):
return []
def update_config_with_globals(self):
- if self.globals.get("use_ssl", False):
- self._update_ssl_config_with_globals("server", DEFAULT_SERVER_KEYSTORE)
+ self._update_ssl_config_with_globals("server")
Review comment:
We have 2 classes for Ignite node
IgniteService for server node and IgniteApplicationService for client
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r577514294
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,248 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params pasrsing
+"""
+
+import pytest
+from ignitetest.services.utils.auth.creds_provider import CredsProvider, DEFAULT_AUTH_LOGIN, DEFAULT_AUTH_PASSWORD
+from ignitetest.services.utils.ssl.ssl_factory import SslContextFactory, DEFAULT_PASSWORD, DEFAULT_TRUSTSTORE, \
+ DEFAULT_ADMIN_KEYSTORE
+from ignitetest.services.utils.control_utility import ControlUtility
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.certificate_dir = '/opt/certs/'
+
+
+class Context:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.logger = ''
+ self.globals = globals_dict
+
+
+def compare_ssl(class1, class2):
+ """
+ Compare two SslContextFactory objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, SslContextFactory) and isinstance(class2, SslContextFactory):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+def compare_creds(class1, class2):
+ """
+ Compare two CredsProvider objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, CredsProvider) and isinstance(class2, CredsProvider):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+class TestParams:
+ """
+ Globals and Kwargs for tests
+ """
+
+ test_ssl_jks = {'key_store_jks': 'admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_jks': 'truststore.jks', 'trust_store_password': 'qwe123'}
+ test_ssl_path = {'key_store_path': '/opt/certs/admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_path': '/opt/certs/truststore.jks',
+ 'trust_store_password': 'qwe123'}
+ test_ssl_only_key = {'key_store_jks': 'admin1.jks'}
+
+ test_creds = {'login': 'admin1', 'password': 'qwe123'}
+ test_creds_only_login = {'login': 'admin1'}
+
+ expected_ssl = SslContextFactory(key_store_path='/opt/certs/admin1.jks',
+ key_store_password='qwe123',
+ trust_store_path='/opt/certs/truststore.jks',
+ trust_store_password='qwe123')
+ expected_ssl_default = SslContextFactory(key_store_path='/opt/certs/'+DEFAULT_ADMIN_KEYSTORE,
+ key_store_password=DEFAULT_PASSWORD,
+ trust_store_path='/opt/certs/'+DEFAULT_TRUSTSTORE,
+ trust_store_password=DEFAULT_PASSWORD)
+ expected_ssl_only_key = SslContextFactory(key_store_path='/opt/certs/admin1.jks',
+ key_store_password=DEFAULT_PASSWORD,
+ trust_store_path='/opt/certs/'+DEFAULT_TRUSTSTORE,
+ trust_store_password=DEFAULT_PASSWORD)
+
+ expected_creds = CredsProvider(login='admin1',
+ password='qwe123')
+ expected_creds_default = CredsProvider(login=DEFAULT_AUTH_LOGIN,
+ password=DEFAULT_AUTH_PASSWORD)
+ expected_creds_only_login = CredsProvider(login='admin1',
+ password=DEFAULT_AUTH_PASSWORD)
+
+
+class CheckCaseGlobalsSetKwargsNotSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_jks}}, {}, TestParams.expected_ssl),
+ ({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_path}}, {}, TestParams.expected_ssl),
+ ({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_only_key}}, {}, TestParams.expected_ssl_only_key),
+ ({'admin': {'ssl': TestParams.test_ssl_jks}}, {}, None), ])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ assert compare_ssl(ControlUtility(Cluster(test_globals), **test_kwargs).ssl_context, expected)
+
+
+class CheckCaseGlobalsNotSetKwargsSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from kwargs
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({}, TestParams.test_ssl_jks, TestParams.expected_ssl),
+ ({}, TestParams.test_ssl_path, TestParams.expected_ssl),
+ ({}, TestParams.test_ssl_only_key, TestParams.expected_ssl_only_key)])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse SSL params from kwargs
+ """
+
+ assert compare_ssl(ControlUtility(Cluster(test_globals), **test_kwargs).ssl_context, expected)
+
+
+class CheckCaseGlobalsSetKwargsSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_jks}}, TestParams.test_ssl_only_key,
+ TestParams.expected_ssl),
+ ({'admin': {'ssl': TestParams.test_ssl_jks}}, TestParams.test_ssl_only_key,
+ TestParams.expected_ssl_only_key)])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse SSL params
+ """
+
+ assert compare_ssl(ControlUtility(Cluster(test_globals), **test_kwargs).ssl_context, expected)
+
+
+class CheckCaseGlobalsSetKwargsNotSetCreds:
+ """
+ Check that control_utulity.py correctly parse credentials from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({'use_auth': True,
+ 'admin': {'creds': TestParams.test_creds}}, {}, TestParams.expected_creds),
+ ({'use_auth': True,
+ 'admin': {'creds': TestParams.test_creds_only_login}}, {},
+ TestParams.expected_creds_only_login),
+ ({'use_auth': True}, {},
+ TestParams.expected_creds_default)])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse credentials from globals
+ """
+
+ assert compare_creds(ControlUtility(Cluster(test_globals), **test_kwargs).creds_prover, expected)
+
+
+class CheckCaseGlobalsNotSetKwargsSetCreds:
+ """
+ Check that control_utulity.py correctly parse credentials from kwargs
+ """
Review comment:
Seems that here is a problem with identation
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r582895811
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,44 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict = None, user: str = 'server'):
Review comment:
Globals parsing logic looks redundant for the ControlUtility constructor
This function can be used for a ThinClient, JDBC and for ignite node with third-party authentication implementation
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r579020003
##########
File path: modules/ducktests/tests/ignitetest/tests/auth_test.py
##########
@@ -0,0 +1,90 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains password based authentication tests
+"""
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.utils.control_utility import ControlUtility, ControlUtilityError
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.utils import ignite_versions, cluster, ignore_if
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+
+# pylint: disable=W0223
+class AuthenticationTests(IgniteTest):
+ """
+ Tests Ignite Authentication
+ """
+ NUM_NODES = 3
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_good_user(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ Positive case
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(name='persistent', persistent=True),
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES, startup_timeout_sec=60)
+
+ servers.start()
+
+ control_utility = ControlUtility(cluster=servers, login="ignite", password="ignite")
+ control_utility.activate()
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignore_if(lambda version, globals_dict: globals_dict.get("use_auth", False)) # Globals overrides test params
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_bad_user(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ Negative case
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(name='persistent', persistent=True),
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES, startup_timeout_sec=60)
+
+ servers.start()
+
+ control_utility = ControlUtility(cluster=servers, login="bad_person", password="wrong_password")
Review comment:
any reason to have a bad and good person when we just able to use a single person with a correct and wrong password?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] timoninmaxim commented on a change in pull request #8686: Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
timoninmaxim commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r573616067
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -374,10 +395,24 @@ class ControlUtilityError(RemoteCommandError):
"""
Error is raised when control utility failed.
"""
+
def __init__(self, account, cmd, exit_status, output):
super().__init__(account, cmd, exit_status, "".join(output))
+class Creds():
+ """
+ POJO for SSL and Authentication params
+ """
+
+ key_store_path: str
Review comment:
All vars in this class are static. Is it expected behavior?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r594317935
##########
File path: modules/ducktests/tests/ignitetest/services/utils/jmx_utils.py
##########
@@ -96,7 +96,7 @@ def mbean_attribute(self, mbean, attr):
return iter(s.strip() for s in self.__run_cmd(cmd))
def __run_cmd(self, cmd):
- return self.node.account.ssh_capture(cmd, allow_fail=False, callback=str)
+ return self.node.account.ssh_capture(cmd, allow_fail=False, callback=str, combine_stderr=False)
Review comment:
This is just a "easy fix" to run this function from the JDK.
This function takes the first line from the console output.
If we use JDK> = 9 then the first line will be a warning from JDK in stderr
Since we don't really need stderr, we can ignore it and workaround this situation
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586480277
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -374,6 +397,7 @@ class ControlUtilityError(RemoteCommandError):
"""
Error is raised when control utility failed.
"""
+
Review comment:
Added ability to specify a username and password for control_utility,
Added ability to specify plugins in configuration
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r591412016
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_auth": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"]
+ }
+ }
+ """
+ username, password = None, None
+ if _globals.get('use_auth'):
+ if user in _globals and 'credentials' in _globals[user]:
+ username, password = _globals[user]['credentials']
+ else:
+ username, password = DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
Review comment:
Once again!!!
This method should never return the username.
You already know the username!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] Sega76 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
Sega76 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r578752775
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -34,27 +36,11 @@ class ControlUtility:
BASE_COMMAND = "control.sh"
# pylint: disable=R0913
- def __init__(self, cluster,
- key_store_jks: str = None, key_store_password: str = DEFAULT_PASSWORD,
- trust_store_jks: str = DEFAULT_TRUSTSTORE, trust_store_password: str = DEFAULT_PASSWORD):
+ def __init__(self, cluster, **kwargs):
self._cluster = cluster
Review comment:
we can immediately receive SslContextFactory and CredsProvider
it's easier to use them
```
def __init__(self, cluster, ssl_context: SslContextFactory = None, creds: CredsProvider = None):
self._cluster = cluster
self.logger = cluster.context.logger
self.ssl_context = get_ssl_context_with_globals(cluster.context.globals, "admin", ssl_context)
self.creds = get_creds_provider_with_globals(cluster.context.globals, "admin", creds)
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r594286839
##########
File path: modules/ducktests/tests/ignitetest/tests/auth_test.py
##########
@@ -0,0 +1,147 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains password based authentication tests
+"""
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.auth import DEFAULT_AUTH_PASSWORD, DEFAULT_AUTH_USERNAME
+from ignitetest.services.utils.control_utility import ControlUtility, ControlUtilityError
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.utils import ignite_versions, cluster
+from ignitetest.services.utils.ignite_configuration.discovery import from_ignite_cluster
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+WRONG_PASSWORD = "wrong_password"
+TEST_USERNAME = "admin"
+TEST_PASSWORD = "qwe123"
+TEST_PASSWORD2 = "123qwe"
+
+ADD_USER = 'adduser'
+UPDATE_USER = 'updateuser'
+REMOVE_USER = 'removeuser'
+
+
+# pylint: disable=W0223
+class AuthenticationTests(IgniteTest):
+ """
+ Tests Ignite Control Utility Activation command with enabled Authentication
+ """
+ NUM_NODES = 2
Review comment:
Why do we need 2 nodes?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586448579
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,44 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict = None, user: str = 'server'):
+ """
+ Parse globals with structure like that.
Review comment:
replace with reference to unit test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586424553
##########
File path: modules/ducktests/tests/ignitetest/services/ignite_app.py
##########
@@ -109,5 +108,4 @@ def extract_results(self, name):
return res
def update_config_with_globals(self):
- if self.globals.get("use_ssl", False):
- self._update_ssl_config_with_globals("client", DEFAULT_CLIENT_KEYSTORE)
+ self._update_ssl_config_with_globals("client")
Review comment:
Because currently it's done like that. I just modify function parsing this parameter from globals.
May be this parameter should be changed in different task.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r577505127
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -313,6 +305,33 @@ def __parse_output(raw_output):
def __alives(self):
return [node for node in self._cluster.nodes if self._cluster.alive(node)]
+ def _parse_ssl_params(self, user, globals_dict, **kwargs):
+ ssl_dict = None
+ if globals_dict.get('use_ssl', False):
+ ssl_dict = globals_dict.get(user, {}).get('ssl', {})
+ elif kwargs.get('key_store_jks') is not None or kwargs.get('key_store_path') is not None:
+ ssl_dict = kwargs
+ return None if ssl_dict is None else \
+ SslContextFactory(key_store_path=ssl_dict.get("key_store_path",
Review comment:
I suppose,that some of these lines of code can be extracted to method.
You can define it in __parse_ssl_params.
```
def get_store_path(type, ssl_dict):
path_key = f'{type}_path'
store_name = f'{type}_jks'
default_name = DEFAULT_TRUSTSTORE if type == 'trust_store' else DEFAULT_ADMIN_KEYSTORE
return ssl_dict.get(path_key, self.jks_path(ssl_disct.get(store_name, default_name)))
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586418374
##########
File path: modules/ducktests/tests/ignitetest/tests/auth_test.py
##########
@@ -0,0 +1,92 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains password based authentication tests
+"""
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.utils.control_utility import ControlUtility, ControlUtilityError
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.utils import ignite_versions, cluster, ignore_if
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+from ignitetest.services.utils.auth import DEFAULT_AUTH_PASSWORD, DEFAULT_AUTH_USERNAME
+
+
+# pylint: disable=W0223
+class AuthenticationTests(IgniteTest):
+ """
+ Tests Ignite Authentication
+ """
+ NUM_NODES = 3
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_correct_password(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ Positive case
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(name='persistent', persistent=True),
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES)
+
+ servers.start()
+
+ control_utility = ControlUtility(cluster=servers,
+ username=DEFAULT_AUTH_USERNAME, password=DEFAULT_AUTH_PASSWORD)
+ control_utility.activate()
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignore_if(lambda version, globals_dict: globals_dict.get("use_auth", False)) # Globals overrides test params
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_wrong_password(self, ignite_version):
Review comment:
password are now parameter passed thru matrix
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r579019160
##########
File path: modules/ducktests/tests/ignitetest/tests/auth_test.py
##########
@@ -0,0 +1,90 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains password based authentication tests
+"""
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.utils.control_utility import ControlUtility, ControlUtilityError
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.utils import ignite_versions, cluster, ignore_if
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+
+# pylint: disable=W0223
+class AuthenticationTests(IgniteTest):
+ """
+ Tests Ignite Authentication
+ """
+ NUM_NODES = 3
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_good_user(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ Positive case
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(name='persistent', persistent=True),
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES, startup_timeout_sec=60)
+
+ servers.start()
+
+ control_utility = ControlUtility(cluster=servers, login="ignite", password="ignite")
+ control_utility.activate()
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignore_if(lambda version, globals_dict: globals_dict.get("use_auth", False)) # Globals overrides test params
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_bad_user(self, ignite_version):
Review comment:
what is good or bad?
should we use more clear phrases like "wring password" or successful/unsuccessful?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r577514522
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,248 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params pasrsing
+"""
+
+import pytest
+from ignitetest.services.utils.auth.creds_provider import CredsProvider, DEFAULT_AUTH_LOGIN, DEFAULT_AUTH_PASSWORD
+from ignitetest.services.utils.ssl.ssl_factory import SslContextFactory, DEFAULT_PASSWORD, DEFAULT_TRUSTSTORE, \
+ DEFAULT_ADMIN_KEYSTORE
+from ignitetest.services.utils.control_utility import ControlUtility
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.certificate_dir = '/opt/certs/'
+
+
+class Context:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.logger = ''
+ self.globals = globals_dict
+
+
+def compare_ssl(class1, class2):
+ """
+ Compare two SslContextFactory objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, SslContextFactory) and isinstance(class2, SslContextFactory):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+def compare_creds(class1, class2):
+ """
+ Compare two CredsProvider objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, CredsProvider) and isinstance(class2, CredsProvider):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+class TestParams:
+ """
+ Globals and Kwargs for tests
+ """
+
+ test_ssl_jks = {'key_store_jks': 'admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_jks': 'truststore.jks', 'trust_store_password': 'qwe123'}
+ test_ssl_path = {'key_store_path': '/opt/certs/admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_path': '/opt/certs/truststore.jks',
+ 'trust_store_password': 'qwe123'}
+ test_ssl_only_key = {'key_store_jks': 'admin1.jks'}
+
+ test_creds = {'login': 'admin1', 'password': 'qwe123'}
+ test_creds_only_login = {'login': 'admin1'}
+
+ expected_ssl = SslContextFactory(key_store_path='/opt/certs/admin1.jks',
+ key_store_password='qwe123',
+ trust_store_path='/opt/certs/truststore.jks',
+ trust_store_password='qwe123')
+ expected_ssl_default = SslContextFactory(key_store_path='/opt/certs/'+DEFAULT_ADMIN_KEYSTORE,
+ key_store_password=DEFAULT_PASSWORD,
+ trust_store_path='/opt/certs/'+DEFAULT_TRUSTSTORE,
+ trust_store_password=DEFAULT_PASSWORD)
+ expected_ssl_only_key = SslContextFactory(key_store_path='/opt/certs/admin1.jks',
+ key_store_password=DEFAULT_PASSWORD,
+ trust_store_path='/opt/certs/'+DEFAULT_TRUSTSTORE,
+ trust_store_password=DEFAULT_PASSWORD)
+
+ expected_creds = CredsProvider(login='admin1',
+ password='qwe123')
+ expected_creds_default = CredsProvider(login=DEFAULT_AUTH_LOGIN,
+ password=DEFAULT_AUTH_PASSWORD)
+ expected_creds_only_login = CredsProvider(login='admin1',
+ password=DEFAULT_AUTH_PASSWORD)
+
+
+class CheckCaseGlobalsSetKwargsNotSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, test_kwargs, expected',
+ [({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_jks}}, {}, TestParams.expected_ssl),
+ ({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_path}}, {}, TestParams.expected_ssl),
+ ({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_only_key}}, {}, TestParams.expected_ssl_only_key),
+ ({'admin': {'ssl': TestParams.test_ssl_jks}}, {}, None), ])
+ def check_parse(test_globals, test_kwargs, expected):
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ assert compare_ssl(ControlUtility(Cluster(test_globals), **test_kwargs).ssl_context, expected)
+
+
+class CheckCaseGlobalsNotSetKwargsSetSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from kwargs
+ """
Review comment:
Some problem with identation
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586507210
##########
File path: modules/ducktests/tests/ignitetest/services/utils/auth/__init__.py
##########
@@ -0,0 +1,40 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains authentication classes and utilities.
+"""
+
+DEFAULT_AUTH_PASSWORD = "ignite"
+DEFAULT_AUTH_USERNAME = "ignite"
+
+
+def get_credentials_from_globals(_globals: dict, user: str):
+ """
+ Parse globals with structure like that.
+ {
+ "use_auth": True,
+ "admin": {
+ "credentials": ["username", "qwerty_123"]
+ }
+ }
+ """
+ username, password = None, None
+ if _globals.get('use_auth'):
+ if user in _globals and 'credentials' in _globals[user]:
+ username, password = _globals[user]['credentials']
+ else:
+ username, password = DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
Review comment:
This function is not meant to be run from tests.
If we specify "use_auth = true" in globals, then this function will return default login and password.
If we specify the username and password through globals, then it will return them.
This change is needed to run tests on a cluster with authentication enabled, without having to change anything in the tests.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586438613
##########
File path: modules/ducktests/tests/checks/utils/check_get_credentials_from_globals.py
##########
@@ -0,0 +1,59 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Check that get_credentials_from_globals correctly parse Credentials from globals
+"""
+
+import pytest
+from ignitetest.services.utils.auth import get_credentials_from_globals, DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
+
+
+class TestParams:
+ """
+ Constant variables for test
+ """
+
+ test_globals = {
+ "use_auth": "True",
+ "client": {
+ "credentials": ["client", "qwe123"]}}
+ test_globals_default = {
+ "use_auth": "True"}
+ test_globals_no_auth = {}
+
+ expected_username = 'client'
+ expected_password = 'qwe123'
+ expected_username_default = DEFAULT_AUTH_USERNAME
+ expected_password_default = DEFAULT_AUTH_PASSWORD
+
+
+class CheckCaseJks:
+ """
+ Check that get_credentials_from_globals correctly parse Credentials from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, expected_username, expected_password',
+ [(TestParams.test_globals, TestParams.expected_username, TestParams.expected_password),
+ (TestParams.test_globals_default, TestParams.expected_username_default,
+ TestParams.expected_password_default),
+ (TestParams.test_globals_no_auth, None, None)])
+ def check_parse(test_globals, expected_username, expected_password):
+ """
+ Check function for pytest
+ """
+
+ assert (expected_username, expected_password) == get_credentials_from_globals(test_globals, 'client')
Review comment:
fixed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r594290070
##########
File path: modules/ducktests/tests/ignitetest/tests/auth_test.py
##########
@@ -0,0 +1,147 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains password based authentication tests
+"""
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.ignite_app import IgniteApplicationService
+from ignitetest.services.utils.auth import DEFAULT_AUTH_PASSWORD, DEFAULT_AUTH_USERNAME
+from ignitetest.services.utils.control_utility import ControlUtility, ControlUtilityError
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.utils import ignite_versions, cluster
+from ignitetest.services.utils.ignite_configuration.discovery import from_ignite_cluster
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+
+WRONG_PASSWORD = "wrong_password"
+TEST_USERNAME = "admin"
+TEST_PASSWORD = "qwe123"
+TEST_PASSWORD2 = "123qwe"
+
+ADD_USER = 'adduser'
+UPDATE_USER = 'updateuser'
+REMOVE_USER = 'removeuser'
+
+
+# pylint: disable=W0223
+class AuthenticationTests(IgniteTest):
+ """
+ Tests Ignite Control Utility Activation command with enabled Authentication
+ """
+ NUM_NODES = 2
+
+ @cluster(num_nodes=NUM_NODES - 1)
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_with_authentication(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth_enabled=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(persistent=True)
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES - 1)
+
+ servers.start()
+
+ ControlUtility(cluster=servers, username=DEFAULT_AUTH_USERNAME, password=DEFAULT_AUTH_PASSWORD).activate()
+
+ # Run command with right password
+ check_authenticate(servers, DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD)
+
+ # Run command with wrong password
+ check_authenticate(servers, DEFAULT_AUTH_USERNAME, WRONG_PASSWORD, True)
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignite_versions(str(DEV_BRANCH))
+ def test_change_users(self, ignite_version):
+ """
+ Test add, update and remove user
+ """
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth_enabled=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(persistent=True),
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES - 1)
+
+ servers.start()
+
+ ControlUtility(cluster=servers, username=DEFAULT_AUTH_USERNAME, password=DEFAULT_AUTH_PASSWORD).activate()
+
+ client_cfg = config._replace(client_mode=True, discovery_spi=from_ignite_cluster(servers))
+
+ # Add new user
+ check_authenticate(servers, TEST_USERNAME, TEST_PASSWORD, True)
+ self.run_with_creds(client_cfg, ADD_USER, TEST_USERNAME, TEST_PASSWORD)
+ check_authenticate(servers, TEST_USERNAME, TEST_PASSWORD)
+
+ # Update user password
+ self.run_with_creds(client_cfg, UPDATE_USER, TEST_USERNAME, TEST_PASSWORD2)
+ check_authenticate(servers, TEST_USERNAME, TEST_PASSWORD2)
+
+ # Remove user
+ self.run_with_creds(client_cfg, REMOVE_USER, TEST_USERNAME, free=False)
+ check_authenticate(servers, TEST_USERNAME, TEST_PASSWORD, True)
Review comment:
Please, add negative tests - check that command wouldn't be executed with the wrong
* default user password
* test user password
* wrong username
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r582909603
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,203 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params parsing
+"""
+
+import pytest
+from ignitetest.services.utils.auth import DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
+from ignitetest.services.utils.ssl.ssl_context import SslContext
+from ignitetest.services.utils.control_utility import ControlUtility
+
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
+INSTALL_ROOT = '/opt'
+CERTIFICATE_DIR = '/opt/ignite-dev/modules/ducktests/tests/certs/'
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.instal_root = CERTIFICATE_DIR
+
+
+class Context:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.logger = ''
+ self.globals = globals_dict
+ self.globals['install_root'] = INSTALL_ROOT
+
+
+def compare_ssl(class1, class2):
+ """
+ Compare two SslContext objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, SslContext) and isinstance(class2, SslContext):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+class TestParams:
+ """
+ Globals and Kwargs for tests
+ """
+
+ test_ssl_jks = {'key_store_jks': 'admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_jks': 'truststore.jks', 'trust_store_password': 'qwe123'}
+ test_ssl_path = {'key_store_path': '/opt/certs/admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_path': '/opt/certs/truststore.jks',
+ 'trust_store_password': 'qwe123'}
+
+ test_ssl_context_jks = SslContext(root_dir=INSTALL_ROOT,
+ key_store_jks='admin1.jks', key_store_password='qwe123',
+ trust_store_jks='truststore.jks', trust_store_password='qwe123')
+ test_ssl_context_path = SslContext(key_store_path='/opt/certs/admin1.jks', key_store_password='qwe123',
+ trust_store_path='/opt/certs/truststore.jks', trust_store_password='qwe123')
+ test_ssl_context = SslContext(root_dir=INSTALL_ROOT, key_store_jks='admin1.jks')
+ test_ssl_context_default = SslContext(root_dir=INSTALL_ROOT,
+ key_store_jks='admin.jks', key_store_password='123456',
+ trust_store_jks='truststore.jks', trust_store_password='123456')
+
+ test_username = 'admin1'
+ test_password = 'qwe123'
+ test_username2 = 'admin1'
+
+
+class CheckCaseGlobalsSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, expected',
+ [({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_jks}}, TestParams.test_ssl_context_jks),
+ ({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_path}}, TestParams.test_ssl_context_path),
Review comment:
I removed this unclear test and added two independent tests for functions get_ssl_params_from_globals and get_credentials functions from _globals
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586420543
##########
File path: modules/ducktests/tests/ignitetest/tests/auth_test.py
##########
@@ -0,0 +1,92 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This module contains password based authentication tests
+"""
+
+from ignitetest.services.ignite import IgniteService
+from ignitetest.services.utils.control_utility import ControlUtility, ControlUtilityError
+from ignitetest.services.utils.ignite_configuration import IgniteConfiguration, DataStorageConfiguration
+from ignitetest.services.utils.ignite_configuration.data_storage import DataRegionConfiguration
+from ignitetest.utils import ignite_versions, cluster, ignore_if
+from ignitetest.utils.ignite_test import IgniteTest
+from ignitetest.utils.version import DEV_BRANCH, LATEST, IgniteVersion
+from ignitetest.services.utils.auth import DEFAULT_AUTH_PASSWORD, DEFAULT_AUTH_USERNAME
+
+
+# pylint: disable=W0223
+class AuthenticationTests(IgniteTest):
+ """
+ Tests Ignite Authentication
+ """
+ NUM_NODES = 3
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignite_versions(str(DEV_BRANCH), str(LATEST))
+ def test_activate_correct_password(self, ignite_version):
+ """
+ Test activate cluster.
+ Authentication enabled
+ Positive case
+ """
+
+ config = IgniteConfiguration(
+ cluster_state="INACTIVE",
+ auth=True,
+ version=IgniteVersion(ignite_version),
+ data_storage=DataStorageConfiguration(
+ default=DataRegionConfiguration(name='persistent', persistent=True),
+ )
+ )
+
+ servers = IgniteService(self.test_context, config=config, num_nodes=self.NUM_NODES)
+
+ servers.start()
+
+ control_utility = ControlUtility(cluster=servers,
+ username=DEFAULT_AUTH_USERNAME, password=DEFAULT_AUTH_PASSWORD)
+ control_utility.activate()
+
+ @cluster(num_nodes=NUM_NODES)
+ @ignore_if(lambda version, globals_dict: globals_dict.get("use_auth", False)) # Globals overrides test params
Review comment:
This test can run in both cases.
I removed this "if" expression
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586485120
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ssl/ssl_params.py
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License
+
+"""
+This module contains classes and utilities for Ignite SslContextFactory.
+
+This file contains three user presets:
+1. server for Ignite(clientMode=False)
+2. client for Ignte(clientMode=True) node and ThinClient
+3. admin for GridClient and console utils (control.sh)
+
+Keystores for this presets are generated automaticaly on creating envoriment
+If you like to specify different certificate for preset you can pass them throw globals
+If you specyfy ssl_params in test, you override globals
+"""
+import os
+
+
+DEFAULT_KEYSTORE = 'server.jks'
+DEFAULT_CLIENT_KEYSTORE = 'client.jks'
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
Review comment:
I like your proposal, but currently it's done like that. I just modify parsing from globals
We should change this later
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r594285835
##########
File path: modules/ducktests/tests/ignitetest/services/utils/jmx_utils.py
##########
@@ -96,7 +96,7 @@ def mbean_attribute(self, mbean, attr):
return iter(s.strip() for s in self.__run_cmd(cmd))
def __run_cmd(self, cmd):
- return self.node.account.ssh_capture(cmd, allow_fail=False, callback=str)
+ return self.node.account.ssh_capture(cmd, allow_fail=False, callback=str, combine_stderr=False)
Review comment:
Why this change?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r579313320
##########
File path: modules/ducktests/tests/ignitetest/services/utils/control_utility.py
##########
@@ -37,27 +39,11 @@ class ControlUtility:
BASE_COMMAND = "control.sh"
# pylint: disable=R0913
- def __init__(self, cluster,
- key_store_jks: str = None, key_store_password: str = DEFAULT_PASSWORD,
- trust_store_jks: str = DEFAULT_TRUSTSTORE, trust_store_password: str = DEFAULT_PASSWORD):
+ def __init__(self, cluster, **kwargs):
self._cluster = cluster
self.logger = cluster.context.logger
-
- if cluster.context.globals.get("use_ssl", False):
- admin_dict = cluster.globals.get("admin", dict())
-
- self.key_store_path = admin_dict.get("key_store_path",
- self.jks_path(admin_dict.get('key_store_jks', DEFAULT_ADMIN_KEYSTORE)))
- self.key_store_password = admin_dict.get('key_store_password', DEFAULT_PASSWORD)
- self.trust_store_path = admin_dict.get("trust_store_path",
- self.jks_path(admin_dict.get('trust_store_jks', DEFAULT_TRUSTSTORE)))
- self.trust_store_password = admin_dict.get('trust_store_password', DEFAULT_PASSWORD)
-
- elif key_store_jks is not None:
- self.key_store_path = self.jks_path(key_store_jks)
- self.key_store_password = key_store_password
- self.trust_store_path = self.jks_path(trust_store_jks)
- self.trust_store_password = trust_store_password
+ self.ssl_context = self._parse_ssl_params("admin", cluster.context.globals, **kwargs)
+ self.creds = self._parse_creds("admin", cluster.context.globals, **kwargs)
Review comment:
just use
```
self.username, self.password = self._parse_creds()
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] ivandasch commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
ivandasch commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r577507704
##########
File path: modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
##########
@@ -420,10 +420,10 @@ def _update_ssl_config_with_globals(self, dict_name: str, default_jks: str):
"""
Update ssl configuration.
"""
- _dict = self.globals.get(dict_name)
- if _dict is not None:
- ssl_context_factory = SslContextFactory(self.install_root, **_dict)
+ _dict = self.globals.get(dict_name)
+ if _dict is not None and "ssl" in _dict:
Review comment:
None is Falsy value. Empty dict also is Falsy value
```
if _dict and "ssl" in _dict:
....
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] map7000 commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
map7000 commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r586438911
##########
File path: modules/ducktests/tests/checks/utils/check_get_credentials_from_globals.py
##########
@@ -0,0 +1,59 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Check that get_credentials_from_globals correctly parse Credentials from globals
+"""
+
+import pytest
+from ignitetest.services.utils.auth import get_credentials_from_globals, DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
+
+
+class TestParams:
+ """
+ Constant variables for test
+ """
+
+ test_globals = {
+ "use_auth": "True",
+ "client": {
+ "credentials": ["client", "qwe123"]}}
+ test_globals_default = {
+ "use_auth": "True"}
+ test_globals_no_auth = {}
+
+ expected_username = 'client'
+ expected_password = 'qwe123'
+ expected_username_default = DEFAULT_AUTH_USERNAME
+ expected_password_default = DEFAULT_AUTH_PASSWORD
Review comment:
Duplicates removed. I left some variables outside because inlining would decrease readability
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] anton-vinogradov commented on a change in pull request #8686: IGNITE-14023 Password based authentication support in ducktape tests
Posted by GitBox <gi...@apache.org>.
anton-vinogradov commented on a change in pull request #8686:
URL: https://github.com/apache/ignite/pull/8686#discussion_r581907886
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,203 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params parsing
+"""
+
+import pytest
+from ignitetest.services.utils.auth import DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
+from ignitetest.services.utils.ssl.ssl_context import SslContext
+from ignitetest.services.utils.control_utility import ControlUtility
+
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
+INSTALL_ROOT = '/opt'
+CERTIFICATE_DIR = '/opt/ignite-dev/modules/ducktests/tests/certs/'
Review comment:
is not used
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,203 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params parsing
+"""
+
+import pytest
+from ignitetest.services.utils.auth import DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
+from ignitetest.services.utils.ssl.ssl_context import SslContext
+from ignitetest.services.utils.control_utility import ControlUtility
+
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
+INSTALL_ROOT = '/opt'
+CERTIFICATE_DIR = '/opt/ignite-dev/modules/ducktests/tests/certs/'
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.instal_root = CERTIFICATE_DIR
+
+
+class Context:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.logger = ''
+ self.globals = globals_dict
+ self.globals['install_root'] = INSTALL_ROOT
+
+
+def compare_ssl(class1, class2):
+ """
+ Compare two SslContext objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, SslContext) and isinstance(class2, SslContext):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+class TestParams:
+ """
+ Globals and Kwargs for tests
+ """
+
+ test_ssl_jks = {'key_store_jks': 'admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_jks': 'truststore.jks', 'trust_store_password': 'qwe123'}
+ test_ssl_path = {'key_store_path': '/opt/certs/admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_path': '/opt/certs/truststore.jks',
+ 'trust_store_password': 'qwe123'}
+
+ test_ssl_context_jks = SslContext(root_dir=INSTALL_ROOT,
+ key_store_jks='admin1.jks', key_store_password='qwe123',
+ trust_store_jks='truststore.jks', trust_store_password='qwe123')
+ test_ssl_context_path = SslContext(key_store_path='/opt/certs/admin1.jks', key_store_password='qwe123',
+ trust_store_path='/opt/certs/truststore.jks', trust_store_password='qwe123')
+ test_ssl_context = SslContext(root_dir=INSTALL_ROOT, key_store_jks='admin1.jks')
+ test_ssl_context_default = SslContext(root_dir=INSTALL_ROOT,
+ key_store_jks='admin.jks', key_store_password='123456',
+ trust_store_jks='truststore.jks', trust_store_password='123456')
+
+ test_username = 'admin1'
+ test_password = 'qwe123'
+ test_username2 = 'admin1'
Review comment:
1) duplication (eg. between test_ssl_jks and test_ssl_context_jks).
each string should be defined only once.
2) test_username and test_username2 are the same
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,203 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params parsing
+"""
+
+import pytest
+from ignitetest.services.utils.auth import DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
+from ignitetest.services.utils.ssl.ssl_context import SslContext
+from ignitetest.services.utils.control_utility import ControlUtility
+
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
+INSTALL_ROOT = '/opt'
+CERTIFICATE_DIR = '/opt/ignite-dev/modules/ducktests/tests/certs/'
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.instal_root = CERTIFICATE_DIR
Review comment:
1) typo.
2) is not used
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,203 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params parsing
+"""
+
+import pytest
+from ignitetest.services.utils.auth import DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
+from ignitetest.services.utils.ssl.ssl_context import SslContext
+from ignitetest.services.utils.control_utility import ControlUtility
+
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
+INSTALL_ROOT = '/opt'
+CERTIFICATE_DIR = '/opt/ignite-dev/modules/ducktests/tests/certs/'
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.instal_root = CERTIFICATE_DIR
+
+
+class Context:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.logger = ''
+ self.globals = globals_dict
+ self.globals['install_root'] = INSTALL_ROOT
+
+
+def compare_ssl(class1, class2):
+ """
+ Compare two SslContext objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, SslContext) and isinstance(class2, SslContext):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+class TestParams:
+ """
+ Globals and Kwargs for tests
+ """
+
+ test_ssl_jks = {'key_store_jks': 'admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_jks': 'truststore.jks', 'trust_store_password': 'qwe123'}
+ test_ssl_path = {'key_store_path': '/opt/certs/admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_path': '/opt/certs/truststore.jks',
+ 'trust_store_password': 'qwe123'}
+
+ test_ssl_context_jks = SslContext(root_dir=INSTALL_ROOT,
+ key_store_jks='admin1.jks', key_store_password='qwe123',
+ trust_store_jks='truststore.jks', trust_store_password='qwe123')
+ test_ssl_context_path = SslContext(key_store_path='/opt/certs/admin1.jks', key_store_password='qwe123',
+ trust_store_path='/opt/certs/truststore.jks', trust_store_password='qwe123')
+ test_ssl_context = SslContext(root_dir=INSTALL_ROOT, key_store_jks='admin1.jks')
+ test_ssl_context_default = SslContext(root_dir=INSTALL_ROOT,
+ key_store_jks='admin.jks', key_store_password='123456',
+ trust_store_jks='truststore.jks', trust_store_password='123456')
+
+ test_username = 'admin1'
+ test_password = 'qwe123'
+ test_username2 = 'admin1'
+
+
+class CheckCaseGlobalsSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, expected',
+ [({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_jks}}, TestParams.test_ssl_context_jks),
Review comment:
test_ssl_jks always used as a part of `({'use_ssl': True...`, any reason to have it like a variable?
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,203 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params parsing
+"""
+
+import pytest
+from ignitetest.services.utils.auth import DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
+from ignitetest.services.utils.ssl.ssl_context import SslContext
+from ignitetest.services.utils.control_utility import ControlUtility
+
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
+INSTALL_ROOT = '/opt'
+CERTIFICATE_DIR = '/opt/ignite-dev/modules/ducktests/tests/certs/'
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.instal_root = CERTIFICATE_DIR
+
+
+class Context:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.logger = ''
+ self.globals = globals_dict
+ self.globals['install_root'] = INSTALL_ROOT
+
+
+def compare_ssl(class1, class2):
+ """
+ Compare two SslContext objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, SslContext) and isinstance(class2, SslContext):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+class TestParams:
+ """
+ Globals and Kwargs for tests
+ """
+
+ test_ssl_jks = {'key_store_jks': 'admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_jks': 'truststore.jks', 'trust_store_password': 'qwe123'}
+ test_ssl_path = {'key_store_path': '/opt/certs/admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_path': '/opt/certs/truststore.jks',
+ 'trust_store_password': 'qwe123'}
+
+ test_ssl_context_jks = SslContext(root_dir=INSTALL_ROOT,
+ key_store_jks='admin1.jks', key_store_password='qwe123',
+ trust_store_jks='truststore.jks', trust_store_password='qwe123')
+ test_ssl_context_path = SslContext(key_store_path='/opt/certs/admin1.jks', key_store_password='qwe123',
+ trust_store_path='/opt/certs/truststore.jks', trust_store_password='qwe123')
+ test_ssl_context = SslContext(root_dir=INSTALL_ROOT, key_store_jks='admin1.jks')
+ test_ssl_context_default = SslContext(root_dir=INSTALL_ROOT,
+ key_store_jks='admin.jks', key_store_password='123456',
+ trust_store_jks='truststore.jks', trust_store_password='123456')
+
+ test_username = 'admin1'
+ test_password = 'qwe123'
+ test_username2 = 'admin1'
+
+
+class CheckCaseGlobalsSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, expected',
+ [({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_jks}}, TestParams.test_ssl_context_jks),
+ ({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_path}}, TestParams.test_ssl_context_path),
Review comment:
test_ssl_path always used as a part of ({'use_ssl': True..., any reason to have it like a variable?
##########
File path: modules/ducktests/tests/checks/utils/check_controls_util_parameters.py
##########
@@ -0,0 +1,203 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Checks Control Utility params parsing
+"""
+
+import pytest
+from ignitetest.services.utils.auth import DEFAULT_AUTH_USERNAME, DEFAULT_AUTH_PASSWORD
+from ignitetest.services.utils.ssl.ssl_context import SslContext
+from ignitetest.services.utils.control_utility import ControlUtility
+
+DEFAULT_ADMIN_KEYSTORE = 'admin.jks'
+INSTALL_ROOT = '/opt'
+CERTIFICATE_DIR = '/opt/ignite-dev/modules/ducktests/tests/certs/'
+
+
+class Cluster:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.context = Context(globals_dict)
+ self.instal_root = CERTIFICATE_DIR
+
+
+class Context:
+ """
+ Need this to initialise ControlUtility
+ """
+
+ def __init__(self, globals_dict):
+ self.logger = ''
+ self.globals = globals_dict
+ self.globals['install_root'] = INSTALL_ROOT
+
+
+def compare_ssl(class1, class2):
+ """
+ Compare two SslContext objects
+ """
+
+ if class1 is None and class2 is None:
+ return True
+ if isinstance(class1, SslContext) and isinstance(class2, SslContext):
+ return class1.__dict__ == class2.__dict__
+ return False
+
+
+class TestParams:
+ """
+ Globals and Kwargs for tests
+ """
+
+ test_ssl_jks = {'key_store_jks': 'admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_jks': 'truststore.jks', 'trust_store_password': 'qwe123'}
+ test_ssl_path = {'key_store_path': '/opt/certs/admin1.jks', 'key_store_password': 'qwe123',
+ 'trust_store_path': '/opt/certs/truststore.jks',
+ 'trust_store_password': 'qwe123'}
+
+ test_ssl_context_jks = SslContext(root_dir=INSTALL_ROOT,
+ key_store_jks='admin1.jks', key_store_password='qwe123',
+ trust_store_jks='truststore.jks', trust_store_password='qwe123')
+ test_ssl_context_path = SslContext(key_store_path='/opt/certs/admin1.jks', key_store_password='qwe123',
+ trust_store_path='/opt/certs/truststore.jks', trust_store_password='qwe123')
+ test_ssl_context = SslContext(root_dir=INSTALL_ROOT, key_store_jks='admin1.jks')
+ test_ssl_context_default = SslContext(root_dir=INSTALL_ROOT,
+ key_store_jks='admin.jks', key_store_password='123456',
+ trust_store_jks='truststore.jks', trust_store_password='123456')
+
+ test_username = 'admin1'
+ test_password = 'qwe123'
+ test_username2 = 'admin1'
+
+
+class CheckCaseGlobalsSsl:
+ """
+ Check that control_utulity.py correctly parse SSL params from globals
+ """
+
+ @staticmethod
+ @pytest.mark.parametrize('test_globals, expected',
+ [({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_jks}}, TestParams.test_ssl_context_jks),
+ ({'use_ssl': True,
+ 'admin': {'ssl': TestParams.test_ssl_path}}, TestParams.test_ssl_context_path),
+ ({'use_ssl': True}, TestParams.test_ssl_context_default)])
Review comment:
Could we make "test_globals, expected" pair more readable? eg. test_globals at the first line and expected at the second where newline required.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org