-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DPE-5830] Bump to 24.04 #495
Changes from all commits
83ed30c
772f0a5
5d27abd
53897c2
b8cb252
0668479
337b880
64cb585
3f3c999
cf24129
3a3c019
25aad14
1dcaa19
d121e8d
f4aecff
68fb074
eec8311
2633109
4a6aa94
28aaaaf
cfce9ca
272761e
4a4c8dc
cacae9c
1065ff7
7b1ac8f
754b6e4
e07cd94
2b96dd6
5f47f14
494cbfa
4349a7b
b616601
e75277b
1445650
9821ba6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
@@ -2,7 +2,7 @@ | ||
# See LICENSE file for licensing details. | ||
|
||
type: charm | ||
-base: [email protected] | ||
+base: [email protected] | ||
platforms: | ||
amd64: | ||
parts: |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,13 +2,18 @@ | |
# See LICENSE file for licensing details. | ||
|
||
type: charm | ||
bases: | ||
- build-on: | ||
- name: "ubuntu" | ||
channel: "22.04" | ||
run-on: | ||
- name: "ubuntu" | ||
channel: "22.04" | ||
# base: [email protected] | ||
# platforms: | ||
# amd64: | ||
# Use upcoming ST124 syntax | ||
# To pack this charm, a temporary compatibility wrapper https://github.com/canonical/charmcraftst124 | ||
# is required until ST124 support is added to charmcraft | ||
# (ST124 syntax is needed to enable multi-base charms with Ubuntu 24.04. We use ST124 syntax across | ||
# all of our charms [even those that aren't multi base] for consistency and to simplify CI/CD | ||
# maintenance & tooling) | ||
platforms: | ||
[email protected]:amd64: | ||
[email protected]:amd64: | ||
parts: | ||
files: | ||
plugin: dump | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -197,23 +197,23 @@ def is_node_up(self, host: Optional[str] = None) -> bool: | |
return False | ||
|
||
def run_bin(self, bin_script_name: str, args: str = None, stdin: str = None) -> str: | ||
"""Run opensearch provided bin command, relative to OPENSEARCH_BIN. | ||
"""Run opensearch provided bin command, through the snap. | ||
|
||
Args: | ||
bin_script_name: opensearch script located in OPENSEARCH_BIN to be executed | ||
args: arguments passed to the script | ||
stdin: string input to be passed on the standard input of the subprocess. | ||
""" | ||
script_path = f"{self.paths.bin}/{bin_script_name}" | ||
return self._run_cmd(script_path, args, stdin=stdin) | ||
opensearch_command = f"opensearch.{bin_script_name}" | ||
return self._run_cmd(opensearch_command, args, stdin=stdin) | ||
|
||
def run_script(self, script_name: str, args: str = None): | ||
"""Run script provided by Opensearch in another directory, relative to OPENSEARCH_HOME.""" | ||
script_path = f"{self.paths.home}/{script_name}" | ||
if not os.access(script_path, os.X_OK): | ||
self._run_cmd(f"chmod a+x {script_path}") | ||
|
||
self._run_cmd(f"{script_path}", args) | ||
self._run_cmd(f"snap run --shell opensearch.daemon -- {script_path}", args) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice, makes a lot of sense to run these scripts under the snap context. TBH we only use this logic here. |
||
|
||
def request( # noqa | ||
self, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -79,7 +79,7 @@ def __init__( | |
self.peer_relation = peer_relation | ||
self.jdk_path = jdk_path | ||
self.certs_path = certs_path | ||
self.keytool = self.jdk_path + "/bin/keytool" | ||
self.keytool = "opensearch.keytool" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that change and that this is now provided by the snap! |
||
self.certs = TLSCertificatesRequiresV3(charm, TLS_RELATION, expiry_notification_time=23) | ||
|
||
self.framework.observe( | ||
|
@@ -513,8 +513,6 @@ def _create_keystore_pwd_if_not_exists(self, scope: Scope, cert_type: CertType, | |
|
||
def store_new_ca(self, secrets: Dict[str, Any]) -> bool: # noqa: C901 | ||
"""Add new CA cert to trust store.""" | ||
keytool = f"sudo {self.jdk_path}/bin/keytool" | ||
|
||
if not (deployment_desc := self.charm.opensearch_peer_cm.deployment_desc()): | ||
return False | ||
|
||
|
@@ -532,7 +530,7 @@ def store_new_ca(self, secrets: Dict[str, Any]) -> bool: # noqa: C901 | |
|
||
try: | ||
run_cmd( | ||
f"""{keytool} -changealias \ | ||
f"""{self.keytool} -changealias \ | ||
-alias {alias} \ | ||
-destalias old-{alias} \ | ||
-keystore {store_path} \ | ||
|
@@ -549,13 +547,15 @@ def store_new_ca(self, secrets: Dict[str, Any]) -> bool: # noqa: C901 | |
): | ||
raise | ||
|
||
with tempfile.NamedTemporaryFile(mode="w+t") as ca_tmp_file: | ||
with tempfile.NamedTemporaryFile( | ||
mode="w+t", dir=self.charm.opensearch.paths.conf | ||
) as ca_tmp_file: | ||
Comment on lines
+550
to
+552
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 good change. Just for consideration: there is also a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I was between that one or |
||
ca_tmp_file.write(secrets.get("ca-cert")) | ||
ca_tmp_file.flush() | ||
|
||
try: | ||
run_cmd( | ||
f"""{keytool} -importcert \ | ||
f"""{self.keytool} -importcert \ | ||
-trustcacerts \ | ||
-noprompt \ | ||
-alias {alias} \ | ||
|
@@ -604,7 +604,6 @@ def read_stored_ca(self, alias: str = "ca") -> Optional[str]: | |
|
||
def remove_old_ca(self) -> None: | ||
"""Remove old CA cert from trust store.""" | ||
keytool = f"sudo {self.jdk_path}/bin/keytool" | ||
ca_trust_store = f"{self.certs_path}/ca.p12" | ||
old_alias = "old-ca" | ||
|
||
|
@@ -613,7 +612,7 @@ def remove_old_ca(self) -> None: | |
|
||
try: | ||
run_cmd( | ||
f"""{keytool} \ | ||
f"""{self.keytool} \ | ||
-list \ | ||
-keystore {ca_trust_store} \ | ||
-storepass {store_pwd} \ | ||
|
@@ -628,7 +627,7 @@ def remove_old_ca(self) -> None: | |
old_ca_content = self.read_stored_ca(alias=old_alias) | ||
|
||
run_cmd( | ||
f"""{keytool} \ | ||
f"""{self.keytool} \ | ||
-delete \ | ||
-keystore {ca_trust_store} \ | ||
-storepass {store_pwd} \ | ||
|
@@ -672,12 +671,16 @@ def store_new_tls_resources(self, cert_type: CertType, secrets: Dict[str, Any]): | |
except OSError: | ||
pass | ||
|
||
tmp_key = tempfile.NamedTemporaryFile(mode="w+t", suffix=".pem") | ||
tmp_key = tempfile.NamedTemporaryFile( | ||
mode="w+t", suffix=".pem", dir=self.charm.opensearch.paths.conf | ||
) | ||
tmp_key.write(secrets.get("key")) | ||
tmp_key.flush() | ||
tmp_key.seek(0) | ||
|
||
tmp_cert = tempfile.NamedTemporaryFile(mode="w+t", suffix=".cert") | ||
tmp_cert = tempfile.NamedTemporaryFile( | ||
mode="w+t", suffix=".cert", dir=self.charm.opensearch.paths.conf | ||
) | ||
tmp_cert.write(secrets.get("cert")) | ||
tmp_cert.flush() | ||
tmp_cert.seek(0) | ||
|
@@ -714,7 +717,7 @@ def all_tls_resources_stored(self, only_unit_resources: bool = False) -> bool: | |
return False | ||
|
||
# to make sure the content is processed correctly by openssl, temporary store it in a file | ||
tmp_ca_file = tempfile.NamedTemporaryFile(mode="w+t") | ||
tmp_ca_file = tempfile.NamedTemporaryFile(mode="w+t", dir=self.charm.opensearch.paths.conf) | ||
tmp_ca_file.write(current_ca) | ||
tmp_ca_file.flush() | ||
tmp_ca_file.seek(0) | ||
|
@@ -819,12 +822,12 @@ def reload_tls_certificates(self): | |
# using the SSL API requires authentication with app-admin cert and key | ||
admin_secret = self.charm.secrets.get_object(Scope.APP, CertType.APP_ADMIN.val) | ||
|
||
tmp_cert = tempfile.NamedTemporaryFile(mode="w+t") | ||
tmp_cert = tempfile.NamedTemporaryFile(mode="w+t", dir=self.charm.opensearch.paths.conf) | ||
tmp_cert.write(admin_secret["cert"]) | ||
tmp_cert.flush() | ||
tmp_cert.seek(0) | ||
|
||
tmp_key = tempfile.NamedTemporaryFile(mode="w+t") | ||
tmp_key = tempfile.NamedTemporaryFile(mode="w+t", dir=self.charm.opensearch.paths.conf) | ||
tmp_key.write(admin_secret["key"]) | ||
tmp_key.flush() | ||
tmp_key.seek(0) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add some
# todo
comments here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The big "TODO" is on
charmcraft.yaml
explaining why all of this is needed. Once we generally update how charmcraft is done, all of this other places will be forced to change by necessity.