[WF-291] Added secret info in config

[jananisenthilkumar291]

Description

Related Issue:
Link

Added configs:

• api.secret_key -> CSRF Protection,
• api_auth.jwt_secret -> related to JWT token validity

---

Testing instructions

Notes for Reviewers (optional)

---

Checklist (all that apply)

• Unit tests added or updated
• Integration tests added or updated
• Documentation updated

[shayancanonical]

i prefer if we did not store the keys in the peer relation databag in plaintext. rather, the charm can create a juju application secret (backed by the peer relation) that contains these values. the secret id can then be stored in the peer relation to retrieve values at runtime

[jananisenthilkumar291]

Good Point! Here's an update 098b8b9

[shayancanonical]

nit: simplification: "secret-key": peer_data.get("secret_key", secrets.token_hex(32))

can use the default value of dict.get() instead of using or (condition structure)

[jananisenthilkumar291]

Good point, Addressed in abefe8a

[shayancanonical]

any particular reason to use a staticmethod? will it ever be called outside of the charm object's context?
suggestion: we should make this a normal method instead

suggestion: if this is a method instead, then a class-level property peer_app_data can be implemented to access the application databag of the peer relation. this way, no args will need to be passed into the method

[jananisenthilkumar291]

Makes sense, addressed in abefe8a

[shayancanonical]

the two calls to model.get_secret() are redundant here

suggestion:

• create property for peer_application_data
• refactor get_keys_secret() to be idempotent to retrieve the secret. this method can be called in config_generator.py when rendering the airflow_config.j2
• make sure to call _ensure_airflow_keys_generated() in the reconcile method

constants.AIRFLOW_KEYS_SECRET = "airflow_keys_secret_id"
constants.AIRFLOW_KEYS_SECRET_LABEL = "airflow_keys_secret"

@property
def peer_application_data(self) -> ops.RelationData:
    return self._peer_relation.data[self.app]

def get_keys_secret(self) -> ops.Secret:
    keys_secret_id = self.peer_application_data.get(constants.AIRFLOW_KEYS_SECRET)
    if keys_secret_id:
        try:
            return self.model.get_secret(id=keys_secret_id, label=constants.AIRFLOW_KEYS_SECRET_LABEL)
        except (ops.SecretNotFoundError, ops.ModelError):
            logger.error("Issue retrieving secret hosting airflow keys")   
            raise ExitWithStatusError("Issue retrieving secret hosting airflow keys", ops.BlockedStatus)

    try:
        keys_secret = self.appl.add_secret(content, label=constants.AIRFLOW_KEYS_SECRET_LABEL)
        self.peer_application_data[constants.AIRFLOW_KEYS_SECRET] = keys_secret.id
    except ops.ValueError:
        logger.error("Issue adding secret for airflow keys")
        raise ExitWithStatusError("Issue adding secret for airflow keys", ops.BlockedStatus)

def _reconcile(self, event) -> None:
    ...
    self._ensure_airlfow_keys_generated()
    ...

[jananisenthilkumar291]

Agreed, addressed in commit abefe8a

[shayancanonical]

looks great! 2 more minor nits - ready to approve after these are addressed

[shayancanonical]

nit: please update the remaining template variables in this file to follow the same format (we don't want 2 disjoint jinja variable naming conventions)

[jananisenthilkumar291]

Done. 2c782e0

[shayancanonical]

nit, prefer the other way

if not keys_secret_id:
    raise ExitWithStatusError

<code that assumes valid keys_secret_id>

it is far easier to trace the code as the consequence of not having the keys_secret_id is right next to the check for if it exists (rather than after an if block)

[jananisenthilkumar291]

Good point! Here's an update 2c782e0