Skip to content

Commit

Permalink
CSS-4936 Postgres as secret backend (#1011)
Browse files Browse the repository at this point in the history 
* Added Postgres as secret backend

* Fix migrations and cleaned up secret implementation

* Added tests

* Removed env files

* Revert compose changes to use Vault

* Added godocs

* Update 1_2.sql

* PR tweaks and test fixes

* PR Tweaks

- Added logging
- Fixed broken test
- Fixed cleanup JWKS function

* Tweak - move line after check
  • Loading branch information
@kian99
kian99 authored Jul 26, 2023
1 parent 54bf1c0 commit e348f24
Show file tree
Hide file tree
Showing 22 changed files with 764 additions and 76 deletions.
4 changes: 4 additions & 0 deletions charms/jimm-k8s/config.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ options:
type: string
default: https://jaas.ai/models
description: URL of the Juju Dashboard for this controller.
postgres-secret-storage:
default: false
type: boolean
description: Enables the use of Postgres for secret storage if Vault is not related.
public-key:
type: string
description: The public part of JIMM's macaroon bakery keypair.
Expand Down
3 changes: 3 additions & 0 deletions charms/jimm-k8s/src/charm.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,9 @@ def _update_workload(self, event):
config_values["JIMM_WATCH_CONTROLLERS"] = "1"
config_values["JIMM_ENABLE_JWKS_ROTATOR"] = "1"

if self.config.get("postgres-secret-storage", False):
config_values["INSECURE_SECRET_STORAGE"] = "enabled" # Value doesn't matter, checks env var exists.

# remove empty configuration values
config_values = {key: value for key, value in config_values.items() if value}

Expand Down
119 changes: 77 additions & 42 deletions charms/jimm-k8s/tests/unit/test_charm.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,20 @@
"public-key": "izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=",
"private-key": "ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=",
"vault-access-address": "10.0.1.123",
"audit-log-retention-period-in-days": "10",
}

EXPECTED_ENV = {
"CANDID_URL": "test-candid-url",
"JIMM_DASHBOARD_LOCATION": "https://jaas.ai/models",
"JIMM_DNS_NAME": "juju-jimm-k8s-0.juju-jimm-k8s-endpoints.None.svc.cluster.local",
"JIMM_ENABLE_JWKS_ROTATOR": "1",
"JIMM_LISTEN_ADDR": ":8080",
"JIMM_LOG_LEVEL": "info",
"JIMM_UUID": "1234567890",
"JIMM_WATCH_CONTROLLERS": "1",
"PRIVATE_KEY": "ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=",
"PUBLIC_KEY": "izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=",
"JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS": "0",
}


Expand Down Expand Up @@ -71,19 +84,7 @@ def test_on_pebble_ready(self):
"startup": "disabled",
"override": "replace",
"command": "/root/jimmsrv",
"environment": {
"CANDID_URL": "test-candid-url",
"JIMM_DASHBOARD_LOCATION": "https://jaas.ai/models",
"JIMM_DNS_NAME": "juju-jimm-k8s-0.juju-jimm-k8s-endpoints.None.svc.cluster.local",
"JIMM_ENABLE_JWKS_ROTATOR": "1",
"JIMM_LISTEN_ADDR": ":8080",
"JIMM_LOG_LEVEL": "info",
"JIMM_UUID": "1234567890",
"JIMM_WATCH_CONTROLLERS": "1",
"JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS": "10",
"PRIVATE_KEY": "ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=",
"PUBLIC_KEY": "izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=",
},
"environment": EXPECTED_ENV,
}
}
},
Expand All @@ -110,19 +111,37 @@ def test_on_config_changed(self):
"startup": "disabled",
"override": "replace",
"command": "/root/jimmsrv",
"environment": {
"CANDID_URL": "test-candid-url",
"JIMM_DASHBOARD_LOCATION": "https://jaas.ai/models",
"JIMM_DNS_NAME": "juju-jimm-k8s-0.juju-jimm-k8s-endpoints.None.svc.cluster.local",
"JIMM_ENABLE_JWKS_ROTATOR": "1",
"JIMM_LISTEN_ADDR": ":8080",
"JIMM_LOG_LEVEL": "info",
"JIMM_UUID": "1234567890",
"JIMM_WATCH_CONTROLLERS": "1",
"JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS": "10",
"PRIVATE_KEY": "ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=",
"PUBLIC_KEY": "izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=",
},
"environment": EXPECTED_ENV,
}
}
},
)

def test_postgres_secret_storage_config(self):
container = self.harness.model.unit.get_container("jimm")
self.harness.charm.on.jimm_pebble_ready.emit(container)

self.harness.update_config(MINIMAL_CONFIG)
self.harness.update_config({"postgres-secret-storage": True})
self.harness.set_leader(True)

# Emit the pebble-ready event for jimm
self.harness.charm.on.jimm_pebble_ready.emit(container)

# Check the that the plan was updated
plan = self.harness.get_container_pebble_plan("jimm")
expected_env = EXPECTED_ENV.copy()
expected_env.update({"INSECURE_SECRET_STORAGE": "enabled"})
self.assertEqual(
plan.to_dict(),
{
"services": {
"jimm": {
"summary": "JAAS Intelligent Model Manager",
"startup": "disabled",
"override": "replace",
"command": "/root/jimmsrv",
"environment": expected_env,
}
}
},
Expand All @@ -146,7 +165,8 @@ def test_bakery_configuration(self):

# Emit the pebble-ready event for jimm
self.harness.charm.on.jimm_pebble_ready.emit(container)

expected_env = EXPECTED_ENV.copy()
expected_env.update({"BAKERY_AGENT_FILE": "/root/config/agent.json"})
# Check the that the plan was updated
plan = self.harness.get_container_pebble_plan("jimm")
self.assertEqual(
Expand All @@ -158,20 +178,7 @@ def test_bakery_configuration(self):
"startup": "disabled",
"override": "replace",
"command": "/root/jimmsrv",
"environment": {
"BAKERY_AGENT_FILE": "/root/config/agent.json",
"CANDID_URL": "test-candid-url",
"JIMM_DASHBOARD_LOCATION": "https://jaas.ai/models",
"JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS": "0",
"JIMM_DNS_NAME": "juju-jimm-k8s-0.juju-jimm-k8s-endpoints.None.svc.cluster.local",
"JIMM_ENABLE_JWKS_ROTATOR": "1",
"JIMM_LISTEN_ADDR": ":8080",
"JIMM_LOG_LEVEL": "info",
"JIMM_UUID": "1234567890",
"JIMM_WATCH_CONTROLLERS": "1",
"PRIVATE_KEY": "ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=",
"PUBLIC_KEY": "izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=",
},
"environment": expected_env,
}
}
},
Expand All @@ -189,6 +196,34 @@ def test_bakery_configuration(self):
},
)

def test_audit_log_retention_config(self):
container = self.harness.model.unit.get_container("jimm")
self.harness.charm.on.jimm_pebble_ready.emit(container)

self.harness.update_config(MINIMAL_CONFIG)
self.harness.update_config({"audit-log-retention-period-in-days": "10"})

# Emit the pebble-ready event for jimm
self.harness.charm.on.jimm_pebble_ready.emit(container)
expected_env = EXPECTED_ENV.copy()
expected_env.update({"JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS": "10"})
# Check the that the plan was updated
plan = self.harness.get_container_pebble_plan("jimm")
self.assertEqual(
plan.to_dict(),
{
"services": {
"jimm": {
"summary": "JAAS Intelligent Model Manager",
"startup": "disabled",
"override": "replace",
"command": "/root/jimmsrv",
"environment": expected_env,
}
}
},
)

def test_dashboard_relation_joined(self):
harness = Harness(JimmOperatorCharm)
self.addCleanup(harness.cleanup)
Expand Down
4 changes: 4 additions & 0 deletions charms/jimm/config.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ options:
type: string
default: https://jaas.ai/models
description: URL of the Juju Dashboard for this controller.
postgres-secret-storage:
default: false
type: boolean
description: Enables the use of Postgres for secret storage if Vault is not related.
public-key:
type: string
description: The public part of JIMM's macaroon bakery keypair.
Expand Down
3 changes: 3 additions & 0 deletions charms/jimm/src/charm.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@ def _on_config_changed(self, _):
"audit_retention_period": self.config.get("audit-log-retention-period-in-days", ""),
}

if self.config.get("postgres-secret-storage", False):
args["insecure_secret_storage"] = "enabled" # Value doesn't matter, only checks env var exists.

with open(self._env_filename(), "wt") as f:
f.write(self._render_template("jimm.env", **args))
if self._ready():
Expand Down
3 changes: 3 additions & 0 deletions charms/jimm/templates/jimm.env
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,6 @@ PRIVATE_KEY={{private_key}}
PUBLIC_KEY={{public_key}}
{% endif %}
JIMM_AUDIT_LOG_RETENTION_PERIOD_IN_DAYS={{audit_retention_period}}
{%- if insecure_secret_storage %}
INSECURE_SECRET_STORAGE=enabled
{% endif %}
28 changes: 28 additions & 0 deletions charms/jimm/tests/test_charm.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,34 @@ def test_openfga_relation_changed(self):
self.assertEqual(lines[3].strip(), "OPENFGA_STORE=test-store")
self.assertEqual(lines[4].strip(), "OPENFGA_TOKEN=test-secret-token")

def test_insecure_secret_storage(self):
"""Test that the flag for insecure secret storage is only generated when explictly requested."""
config_file = os.path.join(self.harness.charm.charm_dir, "juju-jimm.env")
self.harness.update_config(
{
"candid-url": "https://candid.example.com",
"controller-admins": "user1 user2 group1",
"dns-name": "jimm.example.com",
"log-level": "debug",
"uuid": "caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa",
"public-key": "izcYsQy3TePp6bLjqOo3IRPFvkQd2IKtyODGqC6SdFk=",
"private-key": "ly/dzsI9Nt/4JxUILQeAX79qZ4mygDiuYGqc2ZEiDEc=",
}
)
self.assertTrue(os.path.exists(config_file))
with open(config_file) as f:
lines = f.readlines()
os.unlink(config_file)
self.assertEqual(len(lines), 18)
self.assertEqual(len([match for match in lines if "INSECURE_SECRET_STORAGE" in match]), 0)
self.harness.update_config({"postgres-secret-storage": True})
self.assertTrue(os.path.exists(config_file))
with open(config_file) as f:
lines = f.readlines()
os.unlink(config_file)
self.assertEqual(len(lines), 19)
self.assertEqual(len([match for match in lines if "INSECURE_SECRET_STORAGE" in match]), 1)


class VersionHTTPRequestHandler(BaseHTTPRequestHandler):
def __init__(self, *args, **kwargs):
Expand Down
4 changes: 3 additions & 1 deletion cmd/jimmsrv/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ func start(ctx context.Context, s *service.Service) error {
zapctx.Info(ctx, "attempting to start JWKS rotator")
s.Go(func() error {
err := jimmsvc.StartJWKSRotator(ctx, time.NewTicker(time.Hour).C, time.Now().UTC().AddDate(0, 3, 0))
zapctx.Error(ctx, "failed to start JWKS rotator", zap.Error(err))
if err != nil {
zapctx.Error(ctx, "failed to start JWKS rotator", zap.Error(err))
}
return err
})
}
Expand Down
3 changes: 3 additions & 0 deletions docker-compose.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,13 @@ services:
# Not needed for local test (yet).
# BAKERY_AGENT_FILE: ""
JIMM_ADMINS: "[email protected]"
# Note: You can comment out the Vault ENV vars below and instead use INSECURE_SECRET_STORAGE to place secrets in Postgres.
VAULT_ADDR: "http://vault:8200"
VAULT_PATH: "/jimm-kv/"
VAULT_SECRET_FILE: "/vault/approle.json"
VAULT_AUTH_PATH: "/auth/approle/login"
# Note: By default we should use Vault as that is the primary means of secret storage.
# INSECURE_SECRET_STORAGE: "enabled"
# JIMM_DASHBOARD_LOCATION: ""
JIMM_DNS_NAME: "jimm.localhost"
JIMM_WATCH_CONTROLLERS: ""
Expand Down
5 changes: 3 additions & 2 deletions internal/db/db.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (d *Database) Migrate(ctx context.Context, force bool) error {
}

for {
v := dbmodel.Version{Component: dbmodel.Component}
v := dbmodel.Version{Component: dbmodel.Component, Major: 1, Minor: 0}
if err := db.FirstOrCreate(&v).Error; err != nil {
return errors.E(op, dbError(err))
}
Expand All @@ -80,10 +80,11 @@ func (d *Database) Migrate(ctx context.Context, force bool) error {
atomic.StoreUint32(&d.migrated, 1)
return nil
}
if v.Major != dbmodel.Major && !force && v.Major != 0 {
if v.Major != dbmodel.Major && !force {
return errors.E(op, errors.CodeServerConfiguration, fmt.Sprintf("database has incompatible version %d.%d", v.Major, v.Minor))
}
// The major versions are unchanged, the database can be migrated.
v.Minor += 1
schema, err := dbmodel.SQL.ReadFile(path.Join("sql", db.Name(), fmt.Sprintf("%d_%d.sql", v.Major, v.Minor)))
if err != nil {
return errors.E(op, err)
Expand Down
10 changes: 10 additions & 0 deletions internal/db/export_test.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Copyright 2023 Canonical Ltd.

package db

var (
JwksKind = jwksKind
JwksPublicKeyTag = jwksPublicKeyTag
JwksPrivateKeyTag = jwksPrivateKeyTag
JwksExpiryTag = jwksExpiryTag
)
Loading

0 comments on commit e348f24

Please sign in to comment.