fgerlits commented on code in PR #1882:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1882#discussion_r1853948908
##########
docker/test/integration/cluster/containers/CouchbaseServerContainer.py:
##########
@@ -12,19 +12,72 @@
# 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.
+import os
+import OpenSSL.crypto
+import tempfile
+import docker
+import requests
+import logging
+from requests.auth import HTTPBasicAuth
from .Container import Container
from utils import retry_check
+from ssl_utils.SSL_cert_utils import make_server_cert
class CouchbaseServerContainer(Container):
- def __init__(self, feature_context, name, vols, network, image_store,
command=None):
- super().__init__(feature_context, name, 'couchbase-server', vols,
network, image_store, command)
+ def __init__(self, feature_context, name, vols, network, image_store,
command=None, ssl=False):
+ self.ssl = ssl
+ engine = "couchbase-server" if not ssl else "couchbase-server-ssl"
+ super().__init__(feature_context, name, engine, vols, network,
image_store, command)
+ couchbase_cert, couchbase_key =
make_server_cert(f"couchbase-server-{feature_context.id}",
feature_context.root_ca_cert, feature_context.root_ca_key)
+
+ self.root_ca_file = tempfile.NamedTemporaryFile(delete=False)
+
self.root_ca_file.write(OpenSSL.crypto.dump_certificate(type=OpenSSL.crypto.FILETYPE_PEM,
cert=feature_context.root_ca_cert))
+ self.root_ca_file.close()
+ os.chmod(self.root_ca_file.name, 0o666)
+
+ self.couchbase_cert_file = tempfile.NamedTemporaryFile(delete=False)
+
self.couchbase_cert_file.write(OpenSSL.crypto.dump_certificate(type=OpenSSL.crypto.FILETYPE_PEM,
cert=couchbase_cert))
+ self.couchbase_cert_file.close()
+ os.chmod(self.couchbase_cert_file.name, 0o666)
+
+ self.couchbase_key_file = tempfile.NamedTemporaryFile(delete=False)
+
self.couchbase_key_file.write(OpenSSL.crypto.dump_privatekey(type=OpenSSL.crypto.FILETYPE_PEM,
pkey=couchbase_key))
+ self.couchbase_key_file.close()
+ os.chmod(self.couchbase_key_file.name, 0o666)
Review Comment:
can we skip this part if `ssl` is `False`?
##########
docker/test/integration/minifi/controllers/CouchbaseClusterService.py:
##########
@@ -18,10 +18,13 @@
class CouchbaseClusterService(ControllerService):
- def __init__(self, name, connection_string):
+ def __init__(self, name, connection_string, ssl_context_service=None):
super(CouchbaseClusterService, self).__init__(name=name)
self.service_class = 'CouchbaseClusterService'
self.properties['Connection String'] = connection_string
- self.properties['User Name'] = "Administrator"
- self.properties['User Password'] = "password123"
+ if ssl_context_service:
+ self.linked_services.append(ssl_context_service)
+ if not ssl_context_service or ssl_context_service and 'Client
Certificate' not in ssl_context_service.properties:
Review Comment:
not really a review comment, but I didn't know about `not in`, it's pretty
cool
##########
docker/test/integration/cluster/containers/CouchbaseServerContainer.py:
##########
@@ -12,19 +12,72 @@
# 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.
+import os
+import OpenSSL.crypto
+import tempfile
+import docker
+import requests
+import logging
+from requests.auth import HTTPBasicAuth
from .Container import Container
from utils import retry_check
+from ssl_utils.SSL_cert_utils import make_server_cert
class CouchbaseServerContainer(Container):
- def __init__(self, feature_context, name, vols, network, image_store,
command=None):
- super().__init__(feature_context, name, 'couchbase-server', vols,
network, image_store, command)
+ def __init__(self, feature_context, name, vols, network, image_store,
command=None, ssl=False):
+ self.ssl = ssl
+ engine = "couchbase-server" if not ssl else "couchbase-server-ssl"
+ super().__init__(feature_context, name, engine, vols, network,
image_store, command)
+ couchbase_cert, couchbase_key =
make_server_cert(f"couchbase-server-{feature_context.id}",
feature_context.root_ca_cert, feature_context.root_ca_key)
+
+ self.root_ca_file = tempfile.NamedTemporaryFile(delete=False)
+
self.root_ca_file.write(OpenSSL.crypto.dump_certificate(type=OpenSSL.crypto.FILETYPE_PEM,
cert=feature_context.root_ca_cert))
+ self.root_ca_file.close()
+ os.chmod(self.root_ca_file.name, 0o666)
+
+ self.couchbase_cert_file = tempfile.NamedTemporaryFile(delete=False)
+
self.couchbase_cert_file.write(OpenSSL.crypto.dump_certificate(type=OpenSSL.crypto.FILETYPE_PEM,
cert=couchbase_cert))
+ self.couchbase_cert_file.close()
+ os.chmod(self.couchbase_cert_file.name, 0o666)
+
+ self.couchbase_key_file = tempfile.NamedTemporaryFile(delete=False)
+
self.couchbase_key_file.write(OpenSSL.crypto.dump_privatekey(type=OpenSSL.crypto.FILETYPE_PEM,
pkey=couchbase_key))
+ self.couchbase_key_file.close()
+ os.chmod(self.couchbase_key_file.name, 0o666)
def get_startup_finished_log_entry(self):
# after startup the logs are only available in the container, only
this message is shown
return "logs available in"
+ @retry_check(12, 5)
+ def _run_couchbase_cli_command(self, command):
+ (code, _) = self.client.containers.get(self.name).exec_run(command)
+ if code != 0:
+ logging.error(f"Failed to run command '{command}', returned error
code: {code}")
+ return False
+ return True
+
+ def _run_couchbase_cli_commands(self, commands):
+ for command in commands:
+ if not self._run_couchbase_cli_command(command):
+ return False
+ return True
Review Comment:
just a stylistic thing, but I would prefer
```suggestion
return all(self._run_couchbase_cli_command(command) for command in
commands)
```
##########
extensions/couchbase/controllerservices/CouchbaseClusterService.cpp:
##########
@@ -50,6 +50,50 @@ CouchbaseErrorType getErrorType(const std::error_code&
error_code) {
} // namespace
+CouchbaseClient::CouchbaseClient(std::string connection_string, std::string
username, std::string password, minifi::controllers::SSLContextService*
ssl_context_service,
+ const std::shared_ptr<core::logging::Logger>& logger)
+ : connection_string_(std::move(connection_string)), logger_(logger),
cluster_options_(buildClusterOptions(std::move(username), std::move(password),
ssl_context_service)) {
+}
+
+::couchbase::cluster_options CouchbaseClient::buildClusterOptions(std::string
username, std::string password, minifi::controllers::SSLContextService*
ssl_context_service) {
+ if (username.empty() && (!ssl_context_service || (ssl_context_service &&
ssl_context_service->getCertificateFile().empty()))) {
+ throw minifi::Exception(ExceptionType::PROCESS_SCHEDULE_EXCEPTION,
"Neither username and password nor SSLContextService is provided for Couchbase
authentication");
+ }
+
+ if (!username.empty() && ssl_context_service &&
!ssl_context_service->getCertificateFile().empty()) {
+ throw minifi::Exception(ExceptionType::PROCESS_SCHEDULE_EXCEPTION,
"Username and password authentication or mTLS authentication using certificate
defined in SSLContextService "
+ "linked service should be provided exclusively for Couchbase");
Review Comment:
this phrasing is not clear to me, "Either username and password or mTLS
certificate authentication should be used in the SSLContextService for
Couchbase, but not both" would be better
##########
docker/test/integration/cluster/containers/CouchbaseServerContainer.py:
##########
@@ -12,19 +12,72 @@
# 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.
+import os
+import OpenSSL.crypto
+import tempfile
+import docker
+import requests
+import logging
+from requests.auth import HTTPBasicAuth
from .Container import Container
from utils import retry_check
+from ssl_utils.SSL_cert_utils import make_server_cert
class CouchbaseServerContainer(Container):
- def __init__(self, feature_context, name, vols, network, image_store,
command=None):
- super().__init__(feature_context, name, 'couchbase-server', vols,
network, image_store, command)
+ def __init__(self, feature_context, name, vols, network, image_store,
command=None, ssl=False):
+ self.ssl = ssl
+ engine = "couchbase-server" if not ssl else "couchbase-server-ssl"
+ super().__init__(feature_context, name, engine, vols, network,
image_store, command)
+ couchbase_cert, couchbase_key =
make_server_cert(f"couchbase-server-{feature_context.id}",
feature_context.root_ca_cert, feature_context.root_ca_key)
+
+ self.root_ca_file = tempfile.NamedTemporaryFile(delete=False)
+
self.root_ca_file.write(OpenSSL.crypto.dump_certificate(type=OpenSSL.crypto.FILETYPE_PEM,
cert=feature_context.root_ca_cert))
+ self.root_ca_file.close()
+ os.chmod(self.root_ca_file.name, 0o666)
+
+ self.couchbase_cert_file = tempfile.NamedTemporaryFile(delete=False)
+
self.couchbase_cert_file.write(OpenSSL.crypto.dump_certificate(type=OpenSSL.crypto.FILETYPE_PEM,
cert=couchbase_cert))
+ self.couchbase_cert_file.close()
+ os.chmod(self.couchbase_cert_file.name, 0o666)
+
+ self.couchbase_key_file = tempfile.NamedTemporaryFile(delete=False)
+
self.couchbase_key_file.write(OpenSSL.crypto.dump_privatekey(type=OpenSSL.crypto.FILETYPE_PEM,
pkey=couchbase_key))
+ self.couchbase_key_file.close()
+ os.chmod(self.couchbase_key_file.name, 0o666)
def get_startup_finished_log_entry(self):
# after startup the logs are only available in the container, only
this message is shown
return "logs available in"
+ @retry_check(12, 5)
Review Comment:
are keyword arguments allowed here?
```suggestion
@retry_check(max_tries=12, retry_interval=5)
```
would be easier to read
##########
docker/test/integration/features/steps/steps.py:
##########
@@ -1384,6 +1384,34 @@ def step_impl(context, service_name):
container.add_controller(couchbase_cluster_controller_service)
+@given("a CouchbaseClusterService is setup up with SSL connection with the
name \"{service_name}\"")
Review Comment:
typo:
```suggestion
@given("a CouchbaseClusterService is set up with SSL connection with the
name \"{service_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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
