CSS-4442 Adds nginx relation to the juju-jimm-k8s charm. #950
Conversation
alesstimec
force-pushed
the
juju-jimm-k8s-nginx-relation
branch
from
d9b6a99 to
c9144bf
Compare
If Vault is not present, we cannot create a JWKS service and JIMM would panic starting the JWKS rotator.
alesstimec
changed the title
alesstimec
force-pushed
the
juju-jimm-k8s-nginx-relation
branch
from
c9144bf to
7180912
Compare
charms/jimm-k8s/tox.ini
Outdated
| @@ -55,6 +55,7 @@ commands = | |||
| [testenv:unit] | |||
| description = Run unit tests | |||
| deps = | |||
| ipdb | |||
There was a problem hiding this comment.
Ideally remove this or comment it out, with a explanation as to why it's there.
charms/jimm-k8s/src/charm.py
Outdated
| @@ -27,6 +27,7 @@ | |||
| DatabaseEvent, | |||
| DatabaseRequires, | |||
| ) | |||
| from charms.nginx_ingress_integrator.v0.ingress import IngressRequires | |||
There was a problem hiding this comment.
Sorry to be the bearer of more changes, but it's recommended with the nginx_ingress_charm to use the new nginx-route rather than the ingress relation. Though to be honest, this is just meant to be a band-aid fix till we can use traefik so I think it's fine as is.
charms/jimm-k8s/src/charm.py
Outdated
| except RelationNotReadyError: | ||
| event.defer() | ||
| self.nginx_ingress.update_config({ | ||
| "service-hostname": self.config.get("dns-name", ""), |
There was a problem hiding this comment.
It's confusing that here we get the dns-name from config and then two lines below we get it from the function. A comment explaining the difference would be helpful. I assume it is because the nginx ingress would always be the external DNS but JIMM can accept a local cluster DNS?
charms/jimm-k8s/config.yaml
Outdated
| @@ -60,3 +60,6 @@ options: | |||
| private-key: | |||
| type: string | |||
| description: The private part of JIMM's macaroon bakery keypair. | |||
| dns-name: | |||
| type: string | |||
| description: Public DNS hostname that JIMM is being served from. | |||
There was a problem hiding this comment.
This doesn't necessarily have to be public right? Just any old DNS that resolves to JIMM?
| OPENFGA_STORE_ID = "openfga-store-id" | ||
| OPENFGA_TOKEN = "openfga-token" | ||
| OPENFGA_ADDRESS = "openfga-address" | ||
| OPENFGA_PORT = "openfga-port" |
charms/jimm-k8s/src/state.py
Outdated
| Returns: | ||
| A boolean representing whether the relation is ready to be used or not. | ||
| """ | ||
| return bool(self._get_relation()) |
| class JimmOperatorCharm(CharmBase): | ||
| """JIMM Operator Charm.""" | ||
|
|
||
| def __init__(self, *args): | ||
| super().__init__(*args) | ||
|
|
||
| self._state = State(self.app, lambda: self.model.get_relation("jimm")) |
There was a problem hiding this comment.
I like this, it's clever, so all we need to do is interact with _state now and not care about the peer data explicitly. Much cleaner
alesstimec
force-pushed
the
juju-jimm-k8s-nginx-relation
branch
from
7d0ff8f to
e2814d3
Compare
charms/jimm-k8s/lib/charms/nginx_ingress_integrator/v0/ingress.py
Outdated
Show resolved
Hide resolved
| ) | ||
|
|
||
| # Nginx ingress relation | ||
| require_nginx_route( |
There was a problem hiding this comment.
One thing that still confuses me about this new relation is how will the service_hostname change if you change your config value. But perhaps it doesn't.
There was a problem hiding this comment.
currently the nginx route docs do not describe how this is done.. so i expect config value for the dns-name will just have to be provided at deploy time
K8s charm now has two separate relations: - traefik_ingress, which can be used to relate to traefik - ingress, which can be used to relate to nginx-ingress-integrator
alesstimec
force-pushed
the
juju-jimm-k8s-nginx-relation
branch
from
228193b to
61a3558
Compare
Description
K8s charm now has two separate relations:
Engineering checklist
Check only items that apply
Test instructions
Notes for code reviewers