Skip to content
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

CSS-5106 Makes macaroon expiry duration configurable. #1029

Merged
merged 1 commit into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions charms/jimm-k8s/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,7 @@ options:
dns-name:
type: string
description: DNS hostname that JIMM is being served from.
macaroon-expiry-duration:
type: string
default: 24h
description: Expiry duration for authentication macaroons.
1 change: 1 addition & 0 deletions charms/jimm-k8s/src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ def _update_workload(self, event):
"JIMM_UUID": self.config.get("uuid", ""),
"JIMM_DASHBOARD_LOCATION": self.config.get("juju-dashboard-location", "https://jaas.ai/models"),
"JIMM_LISTEN_ADDR": ":8080",
"JIMM_MACAROON_EXPIRY_DURATION": self.config.get("macaroon-expiry-duration", "24h"),
}
if self._state.dsn:
config_values["JIMM_DSN"] = self._state.dsn
Expand Down
4 changes: 4 additions & 0 deletions charms/jimm-k8s/tests/unit/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
MINIMAL_CONFIG = {
"uuid": "1234567890",
"candid-url": "test-candid-url",
"macaroon-expiry-duration": "48h",
}


Expand Down Expand Up @@ -72,6 +73,7 @@ def test_on_pebble_ready(self):
"JIMM_DNS_NAME": "juju-jimm-k8s-0.juju-jimm-k8s-endpoints.None.svc.cluster.local",
"JIMM_LISTEN_ADDR": ":8080",
"JIMM_LOG_LEVEL": "info",
"JIMM_MACAROON_EXPIRY_DURATION": "48h",
"JIMM_UUID": "1234567890",
"JIMM_WATCH_CONTROLLERS": "1",
},
Expand Down Expand Up @@ -107,6 +109,7 @@ def test_on_config_changed(self):
"JIMM_DNS_NAME": "juju-jimm-k8s-0.juju-jimm-k8s-endpoints.None.svc.cluster.local",
"JIMM_LISTEN_ADDR": ":8080",
"JIMM_LOG_LEVEL": "info",
"JIMM_MACAROON_EXPIRY_DURATION": "48h",
"JIMM_UUID": "1234567890",
"JIMM_WATCH_CONTROLLERS": "1",
},
Expand Down Expand Up @@ -150,6 +153,7 @@ def test_bakery_configuration(self):
"JIMM_DNS_NAME": "juju-jimm-k8s-0.juju-jimm-k8s-endpoints.None.svc.cluster.local",
"JIMM_LISTEN_ADDR": ":8080",
"JIMM_LOG_LEVEL": "info",
"JIMM_MACAROON_EXPIRY_DURATION": "24h",
"JIMM_UUID": "1234567890",
"JIMM_WATCH_CONTROLLERS": "1",
},
Expand Down
1 change: 0 additions & 1 deletion charms/jimm/actions.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
# Copyright 2021 Canonical Ltd
# See LICENSE file for licensing details.

4 changes: 4 additions & 0 deletions charms/jimm/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,7 @@ options:
type: string
default: https://jaas.ai/models
description: URL of the Juju Dashboard for this controller.
macaroon-expiry-duration:
type: string
default: 24h
description: Expiry duration for authentication macaroons.
1 change: 1 addition & 0 deletions charms/jimm/src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ def _on_config_changed(self, _):
"log_level": self.config.get("log-level"),
"uuid": self.config.get("uuid"),
"dashboard_location": self.config.get("juju-dashboard-location"),
"macaroon_expiry_duration": self.config.get("macaroon-expiry-duration"),
}

with open(self._env_filename(), "wt") as f:
Expand Down
1 change: 1 addition & 0 deletions charms/jimm/templates/jimm.env
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ JIMM_DNS_NAME={{dns_name}}
{% endif %}
JIMM_LOG_LEVEL={{log_level}}
JIMM_UUID={{uuid}}
JIMM_MACAROON_EXPIRY_DURATION={{macaroon_expiry_duration}}
18 changes: 13 additions & 5 deletions charms/jimm/tests/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,14 @@ def test_config_changed(self):
"dns-name": "jimm.example.com",
"log-level": "debug",
"uuid": "caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa",
"macaroon-expiry-duration": "48h",
}
)
self.assertTrue(os.path.exists(config_file))
with open(config_file) as f:
lines = f.readlines()
os.unlink(config_file)
self.assertEqual(len(lines), 11)
self.assertEqual(len(lines), 12)
self.assertEqual(lines[0].strip(), "BAKERY_AGENT_FILE=")
self.assertEqual(lines[1].strip(), "CANDID_URL=https://candid.example.com")
self.assertEqual(lines[2].strip(), "JIMM_ADMINS=user1 user2 group1")
Expand All @@ -132,6 +133,7 @@ def test_config_changed(self):
self.assertEqual(lines[7].strip(), "JIMM_DNS_NAME=" + "jimm.example.com")
self.assertEqual(lines[9].strip(), "JIMM_LOG_LEVEL=debug")
self.assertEqual(lines[10].strip(), "JIMM_UUID=caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa")
self.assertEqual(lines[11].strip(), "JIMM_MACAROON_EXPIRY_DURATION=48h")

def test_config_changed_redirect_to_dashboard(self):
config_file = os.path.join(self.harness.charm.charm_dir, "juju-jimm.env")
Expand All @@ -143,13 +145,14 @@ def test_config_changed_redirect_to_dashboard(self):
"log-level": "debug",
"uuid": "caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa",
"juju-dashboard-location": "https://test.jaas.ai/models",
"macaroon-expiry-duration": "48h",
}
)
self.assertTrue(os.path.exists(config_file))
with open(config_file) as f:
lines = f.readlines()
os.unlink(config_file)
self.assertEqual(len(lines), 11)
self.assertEqual(len(lines), 12)
self.assertEqual(lines[0].strip(), "BAKERY_AGENT_FILE=")
self.assertEqual(lines[1].strip(), "CANDID_URL=https://candid.example.com")
self.assertEqual(lines[2].strip(), "JIMM_ADMINS=user1 user2 group1")
Expand All @@ -160,6 +163,7 @@ def test_config_changed_redirect_to_dashboard(self):
self.assertEqual(lines[7].strip(), "JIMM_DNS_NAME=" + "jimm.example.com")
self.assertEqual(lines[9].strip(), "JIMM_LOG_LEVEL=debug")
self.assertEqual(lines[10].strip(), "JIMM_UUID=caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa")
self.assertEqual(lines[11].strip(), "JIMM_MACAROON_EXPIRY_DURATION=48h")

def test_config_changed_ready(self):
config_file = os.path.join(self.harness.charm.charm_dir, "juju-jimm.env")
Expand All @@ -170,13 +174,14 @@ def test_config_changed_ready(self):
"candid-url": "https://candid.example.com",
"controller-admins": "user1 user2 group1",
"uuid": "caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa",
"macaroon-expiry-duration": "48h",
}
)
self.assertTrue(os.path.exists(config_file))
with open(config_file) as f:
lines = f.readlines()
os.unlink(config_file)
self.assertEqual(len(lines), 9)
self.assertEqual(len(lines), 10)
self.assertEqual(lines[0].strip(), "BAKERY_AGENT_FILE=")
self.assertEqual(lines[1].strip(), "CANDID_URL=https://candid.example.com")
self.assertEqual(lines[2].strip(), "JIMM_ADMINS=user1 user2 group1")
Expand All @@ -186,6 +191,7 @@ def test_config_changed_ready(self):
)
self.assertEqual(lines[7].strip(), "JIMM_LOG_LEVEL=info")
self.assertEqual(lines[8].strip(), "JIMM_UUID=caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa")
self.assertEqual(lines[9].strip(), "JIMM_MACAROON_EXPIRY_DURATION=48h")

def test_config_changed_with_agent(self):
config_file = os.path.join(self.harness.charm.charm_dir, "juju-jimm.env")
Expand All @@ -210,7 +216,7 @@ def test_config_changed_with_agent(self):

with open(config_file) as f:
lines = f.readlines()
self.assertEqual(len(lines), 9)
self.assertEqual(len(lines), 10)
self.assertEqual(
lines[0].strip(),
"BAKERY_AGENT_FILE=" + self.harness.charm._agent_filename,
Expand All @@ -220,6 +226,7 @@ def test_config_changed_with_agent(self):
self.assertEqual(lines[4].strip(), "JIMM_DASHBOARD_LOCATION=https://jaas.ai/models")
self.assertEqual(lines[7].strip(), "JIMM_LOG_LEVEL=info")
self.assertEqual(lines[8].strip(), "JIMM_UUID=caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa")
self.assertEqual(lines[9].strip(), "JIMM_MACAROON_EXPIRY_DURATION=24h")

self.harness.charm._agent_filename = os.path.join(self.tempdir.name, "no-such-dir", "agent.json")
self.harness.update_config(
Expand All @@ -234,13 +241,14 @@ def test_config_changed_with_agent(self):
)
with open(config_file) as f:
lines = f.readlines()
self.assertEqual(len(lines), 9)
self.assertEqual(len(lines), 10)
self.assertEqual(lines[0].strip(), "BAKERY_AGENT_FILE=")
self.assertEqual(lines[1].strip(), "CANDID_URL=https://candid.example.com")
self.assertEqual(lines[2].strip(), "JIMM_ADMINS=user1 user2 group1")
self.assertEqual(lines[4].strip(), "JIMM_DASHBOARD_LOCATION=https://jaas.ai/models")
self.assertEqual(lines[7].strip(), "JIMM_LOG_LEVEL=info")
self.assertEqual(lines[8].strip(), "JIMM_UUID=caaa4ba4-e2b5-40dd-9bf3-2bd26d6e17aa")
self.assertEqual(lines[9].strip(), "JIMM_MACAROON_EXPIRY_DURATION=24h")

def test_leader_elected(self):
leader_file = os.path.join(self.harness.charm.charm_dir, "juju-jimm-leader.env")
Expand Down
34 changes: 22 additions & 12 deletions cmd/jimmsrv/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,29 @@ func start(ctx context.Context, s *service.Service) error {
}
}

macaroonExpiryDuration := 24 * time.Hour
durationString := os.Getenv("JIMM_MACAROON_EXPIRY_DURATION")
if durationString != "" {
expiry, err := time.ParseDuration(durationString)
if err != nil {
zapctx.Error(ctx, "failed to parse macaroon expiry duration", zap.Error(err))
}
macaroonExpiryDuration = expiry
}
jimmsvc, err := jimm.NewService(ctx, jimm.Params{
ControllerUUID: os.Getenv("JIMM_UUID"),
DSN: os.Getenv("JIMM_DSN"),
CandidURL: os.Getenv("CANDID_URL"),
CandidPublicKey: os.Getenv("CANDID_PUBLIC_KEY"),
BakeryAgentFile: os.Getenv("BAKERY_AGENT_FILE"),
ControllerAdmins: strings.Fields(os.Getenv("JIMM_ADMINS")),
VaultSecretFile: os.Getenv("VAULT_SECRET_FILE"),
VaultAddress: os.Getenv("VAULT_ADDR"),
VaultAuthPath: os.Getenv("VAULT_AUTH_PATH"),
VaultPath: os.Getenv("VAULT_PATH"),
DashboardLocation: os.Getenv("JIMM_DASHBOARD_LOCATION"),
PublicDNSName: os.Getenv("JIMM_DNS_NAME"),
ControllerUUID: os.Getenv("JIMM_UUID"),
DSN: os.Getenv("JIMM_DSN"),
CandidURL: os.Getenv("CANDID_URL"),
CandidPublicKey: os.Getenv("CANDID_PUBLIC_KEY"),
BakeryAgentFile: os.Getenv("BAKERY_AGENT_FILE"),
ControllerAdmins: strings.Fields(os.Getenv("JIMM_ADMINS")),
VaultSecretFile: os.Getenv("VAULT_SECRET_FILE"),
VaultAddress: os.Getenv("VAULT_ADDR"),
VaultAuthPath: os.Getenv("VAULT_AUTH_PATH"),
VaultPath: os.Getenv("VAULT_PATH"),
DashboardLocation: os.Getenv("JIMM_DASHBOARD_LOCATION"),
PublicDNSName: os.Getenv("JIMM_DNS_NAME"),
MacaroonExpiryDuration: macaroonExpiryDuration,
})
if err != nil {
return err
Expand Down
17 changes: 14 additions & 3 deletions service.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ type Params struct {
// PublicDNSName is the name to advertise as the public address of
// the juju controller.
PublicDNSName string

// MacaroonExpiryDuration holds the expiry duration of authentication macaroons.
MacaroonExpiryDuration time.Duration
}

// A Service is the implementation of a JIMM server.
Expand Down Expand Up @@ -319,11 +322,19 @@ func newAuthenticator(ctx context.Context, db *db.Database, p Params) (jimm.Auth
if err != nil {
return nil, err
}

if p.MacaroonExpiryDuration == 0 {
p.MacaroonExpiryDuration = 24 * time.Hour
}

return auth.JujuAuthenticator{
Bakery: identchecker.NewBakery(identchecker.BakeryParams{
RootKeyStore: dbrootkeystore.NewRootKeys(100, nil).NewStore(db, dbrootkeystore.Policy{
ExpiryDuration: 24 * time.Hour,
}),
RootKeyStore: dbrootkeystore.NewRootKeys(100, nil).NewStore(
db,
dbrootkeystore.Policy{
ExpiryDuration: p.MacaroonExpiryDuration,
},
),
Locator: httpbakery.NewThirdPartyLocator(nil, tps),
Key: key,
IdentityClient: candidClient,
Expand Down