Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
kelkawi-a committed Jul 26, 2024
1 parent c36333e commit 65e9238
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 91 deletions.
32 changes: 16 additions & 16 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,17 @@ container.
The Charmed Temporal Worker allows the user to configure multiple sources of
environment variables and secrets to be injected into the workload container and
consumed by the user's workflow definitions. These sources can be configured
through the `secrets` config parameter of the charm. Below are the three sources
of environment variables and secrets currently supported. A user may choose to
use one, all or none of them. Once the `secrets.yaml` file is ready, it can be
configured into the charm as follows:
through the `environment` config parameter of the charm. Below are the three
sources of environment variables and secrets currently supported. A user may
choose to use one, all or none of them. Once the `environment.yaml` file is
ready, it can be configured into the charm as follows:

```bash
juju config temporal-worker-k8s secrets=@/path/to/secrets.yaml
juju config temporal-worker-k8s environment=@/path/to/environment.yaml
```

These secrets can then be injested by the workflows by using the `os` package as
follows:
These environment variables can then be retrieved by the workflows by using the
`os` package as follows:

```python
import os
Expand All @@ -88,10 +88,10 @@ These are usually values that are not secret and can be stored as plaintext. An
example is setting the application environment to `staging` or `production`.
They can be set as follows:

##### **`secrets.yaml`**
##### **`environment.yaml`**

```yaml
secrets:
environment:
env:
- key1: value1
- key2: value2
Expand All @@ -111,12 +111,12 @@ juju add-secret my-secret key1=value1 key2=value2
juju grant-secret my-secret temporal-worker-k8s
```

The secrets can then be configured into the charm as follows:
The environment variables can then be configured into the charm as follows:

##### **`secrets.yaml`**
##### **`environment.yaml`**

```yaml
secrets:
environment:
juju:
- secret-id: <secret_id>
key: key1
Expand All @@ -129,13 +129,13 @@ secrets:
The Vault section below outlines how the Charmed Temporal Worker can be related
to the [Vault operator charm](https://charmhub.io/vault-k8s) for storing secrets
securely. Once done, the charm can be configured to fetch secrets from Vault and
inject them as variables into the workload container. The secrets can be
configured into the charm as follows:
inject them as environment variables into the workload container. The secrets
can be configured into the charm as follows:
##### **`secrets.yaml`**
##### **`environment.yaml`**

```yaml
secrets:
environment:
vault:
- path: my-secrets
key: key1
Expand Down
2 changes: 1 addition & 1 deletion charmcraft.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ bases:
parts:
charm:
charm-binary-python-packages:
- hvac
- hvac==2.3.0
10 changes: 5 additions & 5 deletions config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -147,26 +147,26 @@ options:
default: ""
type: string

secrets:
environment:
description: |
This configuration is used to manage and retrieve sensitive information required
by the application from different sources. The `secrets` configuration supports
by the application from different sources. The `environment` configuration supports
the following sources:
- **Environment Variables**: Secrets can be provided directly as environment variables.
- **Environment Variables**: Plaintext environment variables.
- **Juju**: Secrets can be managed and retrieved using Juju's secret storage capabilities.
- **Vault**: Secrets can be securely stored and accessed from a HashiCorp Vault instance.
The application will prioritize these sources in the following order: Vault, Juju,
and then environment variables. If a secret is not found in the higher priority
and then environment variables. If a variable is not found in the higher priority
sources, it will fallback to the next available source. This ensures that the
application can function correctly in various deployment scenarios while maintaining
security and flexibility.
Sample structure:
```yaml
secrets:
environment:
env:
- key1: value1
- key2: value2
Expand Down
11 changes: 5 additions & 6 deletions lib/charms/vault_k8s/v0/vault_kv.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ def _on_update_status(self, event):
binding = self.model.get_binding("vault-kv")
if binding is not None:
egress_subnet = str(binding.network.interfaces[0].subnet)
self.interface.request_credentials(event.relation, egress_subnet, self.get_nonce())
relation = self.model.get_relation(relation_name="vault-kv")
self.interface.request_credentials(relation, egress_subnet, self.get_nonce())
def get_nonce(self):
secret = self.model.get_secret(label=NONCE_SECRET_LABEL)
Expand Down Expand Up @@ -132,7 +133,7 @@ def get_nonce(self):

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 7
LIBPATCH = 9

PYDEPS = ["pydantic", "pytest-interface-tester"]

Expand Down Expand Up @@ -163,9 +164,7 @@ class VaultKvProviderSchema(BaseModel):
ca_certificate: str = Field(
description="The CA certificate to use when validating the Vault server's certificate."
)
egress_subnet: str = Field(
description="The CIDR allowed by the role."
)
egress_subnet: str = Field(description="The CIDR allowed by the role.")
credentials: Json[Mapping[str, str]] = Field(
description=(
"Mapping of unit name and credentials for that unit."
Expand Down Expand Up @@ -408,7 +407,7 @@ def get_outstanding_kv_requests(self, relation_id: Optional[int] = None) -> List
kv_requests = self.get_kv_requests(relation_id=relation_id)
for request in kv_requests:
if not self._credentials_issued_for_request(
nonce=request.nonce, relation_id=relation_id
nonce=request.nonce, relation_id=request.relation_id
):
outstanding_requests.append(request)
return outstanding_requests
Expand Down
30 changes: 16 additions & 14 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from ops.charm import CharmBase
from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus

import secret_processors
import environment_processors
from literals import (
LOG_FILE,
PROMETHEUS_PORT,
Expand Down Expand Up @@ -173,20 +173,20 @@ def _validate_pebble_plan(self, container):
except pebble.ConnectionError:
return False

def create_env(self):
def create_env(self) -> dict:
"""Create an environment dictionary with secrets from the parsed secrets data.
Returns:
dict: A dictionary containing environment variables.
"""
self.vault_relation.update_vault_relation()

secrets_config = self.config.get("secrets")
parsed_secrets_data = secret_processors.parse_secrets(secrets_config)
environment_config = self.config.get("environment")
parsed_environment_data = environment_processors.parse_environment(environment_config)

env_variables = secret_processors.process_env_variables(parsed_secrets_data)
juju_variables = secret_processors.process_juju_secrets(self, parsed_secrets_data)
vault_variables = secret_processors.process_vault_secrets(self, parsed_secrets_data)
env_variables = environment_processors.process_env_variables(parsed_environment_data)
juju_variables = environment_processors.process_juju_variables(self, parsed_environment_data)
vault_variables = environment_processors.process_vault_variables(self, parsed_environment_data)

charm_env = {**env_variables, **juju_variables, **vault_variables}
return charm_env
Expand Down Expand Up @@ -235,12 +235,12 @@ def _validate(self, event): # noqa: C901
if self.config["sentry-dsn"] and (sample_rate < 0 or sample_rate > 1):
raise ValueError("Invalid config: sentry-sample-rate must be between 0 and 1")

secrets_config = self.config.get("secrets")
if secrets_config:
environment_config = self.config.get("environment")
if environment_config:
try:
yaml.safe_load(secrets_config)
yaml.safe_load(environment_config)
except (yaml.parser.ParserError, yaml.scanner.ScannerError) as e:
raise ValueError(f"Incorrectly formatted `secrets` config: {e}") from e
raise ValueError(f"Incorrectly formatted `environment` config: {e}") from e

def _update(self, event): # noqa: C901
"""Update the Temporal worker configuration and replan its execution.
Expand All @@ -257,8 +257,8 @@ def _update(self, event): # noqa: C901
context = {}
try:
self._validate(event)
secrets_config = self.config.get("secrets")
if secrets_config:
environment_config = self.config.get("environment")
if environment_config:
charm_config_env = self.create_env()
context.update(charm_config_env)
except ValueError as err:
Expand All @@ -278,7 +278,9 @@ def _update(self, event): # noqa: C901
if value:
context.update({key: value})

context.update({convert_env_var(key): value for key, value in self.config.items() if key not in ["secrets"]})
context.update(
{convert_env_var(key): value for key, value in self.config.items() if key not in ["environment"]}
)
context.update({"TWC_PROMETHEUS_PORT": PROMETHEUS_PORT})

pebble_layer = {
Expand Down
54 changes: 28 additions & 26 deletions src/secret_processors.py → src/environment_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,25 @@
logger = logging.getLogger(__name__)


def process_env_variables(parsed_secrets_data):
def process_env_variables(parsed_environment_data):
"""Process environment variables from the parsed secrets data.
Args:
parsed_secrets_data: Parsed secrets data.
parsed_environment_data: Parsed secrets data.
Returns:
dict: A dictionary containing environment variables.
"""
env_variables = parsed_secrets_data.get("env", {})
env_variables = parsed_environment_data.get("env", {})
return env_variables


def process_juju_secrets(charm, parsed_secrets_data):
def process_juju_variables(charm, parsed_environment_data):
"""Process Juju secrets from the parsed secrets data.
Args:
charm: The charm to perform operations on.
parsed_secrets_data: Parsed secrets data.
parsed_environment_data: Parsed secrets data.
Returns:
dict: A dictionary containing Juju secrets.
Expand All @@ -41,10 +41,10 @@ def process_juju_secrets(charm, parsed_secrets_data):
or if the charm does not have permission to access the specified Juju secret.
"""
charm_env = {}
if parsed_secrets_data.get("juju") and not JujuVersion.from_environ().has_secrets:
if parsed_environment_data.get("juju") and not JujuVersion.from_environ().has_secrets:
raise ValueError("Juju version does not support Juju user secrets")

juju_variables = parsed_secrets_data.get("juju", [])
juju_variables = parsed_environment_data.get("juju", [])
for juju_secret in juju_variables:
try:
secret_id = juju_secret.get("secret-id")
Expand All @@ -63,12 +63,12 @@ def process_juju_secrets(charm, parsed_secrets_data):
return charm_env


def process_vault_secrets(charm, parsed_secrets_data):
def process_vault_variables(charm, parsed_environment_data):
"""Process Vault secrets from the parsed secrets data.
Args:
charm: The charm to perform operations on.
parsed_secrets_data: Parsed secrets data.
parsed_environment_data: Parsed secrets data.
Returns:
dict: A dictionary containing Vault secrets.
Expand All @@ -79,7 +79,7 @@ def process_vault_secrets(charm, parsed_secrets_data):
"""
# TODO (kelkawi-a): Convert to using structured config
charm_env = {}
vault_variables = parsed_secrets_data.get("vault", [])
vault_variables = parsed_environment_data.get("vault", [])

if vault_variables and not charm.model.relations["vault"]:
raise ValueError("No vault relation found to fetch secrets from")
Expand All @@ -103,8 +103,8 @@ def process_vault_secrets(charm, parsed_secrets_data):
return charm_env


def parse_secrets(yaml_string):
"""Parse a YAML string containing secrets and validates its structure.
def parse_environment(yaml_string):
"""Parse a YAML string containing environment variables and validates its structure.
The YAML string should contain a 'secrets' key with nested 'env', 'juju', and 'vault' keys.
Each nested key should follow a specific structure:
Expand All @@ -130,37 +130,39 @@ def parse_secrets(yaml_string):
data = yaml.safe_load(yaml_string)

# Validate the main structure
if not isinstance(data, dict) or "secrets" not in data:
raise ValueError("Invalid secrets structure: 'secrets' key not found")
if not isinstance(data, dict) or "environment" not in data:
raise ValueError("Invalid environment structure: 'environment' key not found")

secrets_key = data["secrets"]
if not isinstance(secrets_key, dict):
raise ValueError("Invalid secrets structure: 'secrets' should be a dictionary")
environment_key = data["environment"]
if not isinstance(environment_key, dict):
raise ValueError("Invalid environment structure: 'environment' should be a dictionary")

# Validate env key
env = secrets_key.get("env", [])
env = environment_key.get("env", [])
if not isinstance(env, list) or not all(isinstance(item, dict) and len(item) == 1 for item in env):
raise ValueError("Invalid secrets structure: 'env' should be a list of single-key dictionaries")
raise ValueError("Invalid environment structure: 'env' should be a list of single-key dictionaries")

# Validate juju key
juju = secrets_key.get("juju", [])
juju = environment_key.get("juju", [])
if not isinstance(juju, list) or not all(
isinstance(item, dict) and "key" in item and (("secret-id" in item) and len(item) == 2) for item in juju
):
raise ValueError(
"Invalid secrets structure: 'juju' should be a list of dictionaries with 'key' and 'secret-id'"
"Invalid environment structure: 'juju' should be a list of dictionaries with 'key' and 'secret-id'"
)

# Validate vault key
vault = secrets_key.get("vault", [])
vault = environment_key.get("vault", [])
if not isinstance(vault, list) or not all(
isinstance(item, dict) and "path" in item and "key" in item and len(item) == 2 for item in vault
):
raise ValueError("Invalid secrets structure: 'vault' should be a list of dictionaries with 'path' and 'key'")
raise ValueError(
"Invalid environment structure: 'vault' should be a list of dictionaries with 'path' and 'key'"
)

env = secrets_key.get("env", [])
juju = secrets_key.get("juju", [])
vault = secrets_key.get("vault", [])
env = environment_key.get("env", [])
juju = environment_key.get("juju", [])
vault = environment_key.get("vault", [])

parsed_data = {
"env": {list(item.keys())[0]: list(item.values())[0] for item in env},
Expand Down
13 changes: 8 additions & 5 deletions src/relations/vault.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,10 @@ def get_vault_config(self):
fd.write(ca_certificate)

return {
"address": vault_url,
"role_id": role_id,
"role_secret_id": role_secret_id,
"mount": mount,
"vault_address": vault_url,
"vault_role_id": role_id,
"vault_role_secret_id": role_secret_id,
"vault_mount": mount,
}

def get_vault_client(self):
Expand All @@ -140,7 +140,10 @@ def get_vault_client(self):
ca_certificate_path = self.get_ca_cert_location_in_charm()
vault_config = self.get_vault_config()
return VaultClient(
**vault_config,
address=vault_config["vault_address"],
role_id=vault_config["vault_role_id"],
role_secret_id=vault_config["vault_role_secret_id"],
mount_point=vault_config["vault_mount"],
cert_path=f"{ca_certificate_path}/{VAULT_CA_CERT_FILENAME}",
)

Expand Down
Loading

0 comments on commit 65e9238

Please sign in to comment.