[DPE-2992] Part-1 Recieve keyfile from config-server #4
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MiaAltieri
commented
Nov 28, 2023
src/charm.py
Outdated
Comment on lines
128
to
252
| self_address = self._unit_ip(self.unit) | ||
| addresses = [] | ||
| if peer_addresses: | ||
| addresses.extend(peer_addresses) | ||
| addresses.append(self_address) | ||
| return addresses | ||
|
|
||
| @property | ||
| def _peers(self) -> Optional[Relation]: | ||
| """Fetch the peer relation. | ||
|
|
||
| Returns: | ||
| An `ops.model.Relation` object representing the peer relation. | ||
| """ | ||
| return self.model.get_relation(Config.Relations.PEERS) | ||
|
|
||
| def get_secret(self, scope: str, key: str) -> Optional[str]: | ||
| """Get secret from the secret storage.""" | ||
| label = generate_secret_label(self, scope) | ||
| secret = self.secrets.get(label) | ||
| if not secret: | ||
| return | ||
|
|
||
| value = secret.get_content().get(key) | ||
| if value != Config.Secrets.SECRET_DELETED_LABEL: | ||
| return value | ||
|
|
||
| def set_secret(self, scope: str, key: str, value: Optional[str]) -> Optional[str]: | ||
| """Set secret in the secret storage. | ||
|
|
||
| Juju versions > 3.0 use `juju secrets`, this function first checks | ||
| which secret store is being used before setting the secret. | ||
| """ | ||
| if not value: | ||
| return self.remove_secret(scope, key) | ||
|
|
||
| label = generate_secret_label(self, scope) | ||
| secret = self.secrets.get(label) | ||
| if not secret: | ||
| self.secrets.add(label, {key: value}, scope) | ||
| else: | ||
| content = secret.get_content() | ||
| content.update({key: value}) | ||
| secret.set_content(content) | ||
| return label | ||
|
|
||
| def remove_secret(self, scope, key) -> None: | ||
| """Removing a secret.""" | ||
| label = generate_secret_label(self, scope) | ||
| secret = self.secrets.get(label) | ||
|
|
||
| if not secret: | ||
| return | ||
|
|
||
| content = secret.get_content() | ||
|
|
||
| if not content.get(key) or content[key] == Config.Secrets.SECRET_DELETED_LABEL: | ||
| logger.error( | ||
| f"Non-existing secret {scope}:{key} was attempted to be removed." | ||
| ) | ||
| return | ||
|
|
||
| content[key] = Config.Secrets.SECRET_DELETED_LABEL | ||
| secret.set_content(content) | ||
|
|
||
| def get_keyfile_contents(self) -> str: | ||
| """Retrieves the contents of the keyfile on host machine.""" | ||
| # wait for keyFile to be created by leader unit | ||
| if not self.get_secret(APP_SCOPE, Config.Secrets.SECRET_KEYFILE_NAME): | ||
| logger.debug("waiting for leader unit to generate keyfile contents") | ||
| return | ||
|
|
||
| key_file_path = f"{Config.MONGOD_CONF_DIR}/{KEY_FILE}" | ||
| key_file = Path(key_file_path) | ||
| print(key_file.is_file()) | ||
| if not key_file.is_file(): | ||
| logger.info("no keyfile present") | ||
| return | ||
|
|
||
| with open(key_file_path, "r") as file: | ||
| key = file.read() | ||
|
|
||
| return key | ||
|
|
||
| def push_file_to_unit(self, parent_dir, file_name, file_contents) -> None: | ||
| """K8s charms can push files to their containers easily, this is a vm charm workaround.""" | ||
| Path(parent_dir).mkdir(parents=True, exist_ok=True) | ||
| file_name = f"{parent_dir}/{file_name}" | ||
| with open(file_name, "w") as write_file: | ||
| write_file.write(file_contents) | ||
|
|
||
| # MongoDB limitation; it is needed 400 rights for keyfile and we need 440 rights on tls | ||
| # certs to be able to connect via MongoDB shell | ||
| if Config.TLS.KEY_FILE_NAME in file_name: | ||
| os.chmod(file_name, 0o400) | ||
| else: | ||
| os.chmod(file_name, 0o440) | ||
| mongodb_user = pwd.getpwnam(MONGO_USER) | ||
| os.chown(file_name, mongodb_user.pw_uid, ROOT_USER_GID) | ||
|
|
There was a problem hiding this comment.
copied from VM charm
MiaAltieri
commented
Nov 28, 2023
Comment on lines
+42
to
+74
| @parameterized.expand([("app"), ("unit")]) | ||
| def test_set_secret_returning_secret_id(self, scope): | ||
| secret_id = self.harness.charm.set_secret(scope, "somekey", "bla") | ||
| assert re.match(f"mongos.{scope}", secret_id) | ||
|
|
||
| @parameterized.expand([("app"), ("unit")]) | ||
| def test_set_reset_new_secret(self, scope): | ||
| """NOTE: currently ops.testing seems to allow for non-leader to set secrets too!""" | ||
| # Getting current password | ||
| self.harness.charm.set_secret(scope, "new-secret", "bla") | ||
| assert self.harness.charm.get_secret(scope, "new-secret") == "bla" | ||
|
|
||
| # Reset new secret | ||
| self.harness.charm.set_secret(scope, "new-secret", "blablabla") | ||
| assert self.harness.charm.get_secret(scope, "new-secret") == "blablabla" | ||
|
|
||
| # Set another new secret | ||
| self.harness.charm.set_secret(scope, "new-secret2", "blablabla") | ||
| assert self.harness.charm.get_secret(scope, "new-secret2") == "blablabla" | ||
|
|
||
| @parameterized.expand([("app"), ("unit")]) | ||
| def test_invalid_secret(self, scope): | ||
| with self.assertRaises(TypeError): | ||
| self.harness.charm.set_secret("unit", "somekey", 1) | ||
|
|
||
| self.harness.charm.set_secret("unit", "somekey", "") | ||
| assert self.harness.charm.get_secret(scope, "somekey") is None | ||
|
|
||
| @patch("charm.MongosOperatorCharm.get_secret", return_value=None) | ||
| def test_get_keyfile_contents_no_secret(self, get_secret): | ||
| """Tests file isn't checked if secret isn't set.""" | ||
| self.assertEqual(self.harness.charm.get_keyfile_contents(), None) | ||
|
|
There was a problem hiding this comment.
copied from VM charm
MiaAltieri
changed the title
delgod
requested changes
Nov 29, 2023
src/charm.py
Outdated
| @property | ||
| def mongos_config(self) -> MongoDBConfiguration: | ||
| """Generates a MongoDBConfiguration object for mongos in the deployment of MongoDB.""" | ||
| return self._get_mongos_config_for_user(OperatorUser, set(self._unit_ips)) |
There was a problem hiding this comment.
in the case of subordinate charm, we should connect only to local mongos
Suggested change
| return self._get_mongos_config_for_user(OperatorUser, set(self._unit_ips)) | |
| return self._get_mongos_config_for_user(OperatorUser, {"/tmp/mongos.sock"}) |
mongos should be bond to socket mongos --bind_ip /tmp/mongos.sock
There was a problem hiding this comment.
At the risk of sounding naive, why not mongos --bind_ip localhost
delgod
approved these changes
Nov 30, 2023
dmitry-ratushnyy
approved these changes
Nov 30, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue
DPE-2992 is to start the mongos subordinate charm with the provided config server. In order for the config server and for mongos to be connected they both must have the same keyfile
Solution
Recieve the keyfile
Follow up PRs
Testing