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.

Sign up for GitHub

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

[WIP] Proposal make oathkeeper fully configurable #96

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[WIP] Proposal make oathkeeper fully configurable #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
800ad1b
Support general configuration overrides for ory
Oct 29, 2021
a7de18b
Make all access rules configurable
Oct 29, 2021
0857102
Propagate new ory configuration variables up
Oct 29, 2021
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
0 ...ry/oathkeeper/access-rule-oathkeeper.yaml → default-access-rules.yaml
View file
Open in desktop
File renamed without changes.
8 changes: 7 additions & 1 deletion main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,10 @@ resource "kubernetes_namespace" "ory_namespace" {
}
}

locals {
default_access_rules = yamldecode("default-access-rules")
}

module "ory" {
count = var.enable_ory_authentication ? 1 : 0
source = "./modules/ory"
Expand All @@ -223,9 +227,11 @@ module "ory" {
smtp_from_address = var.smtp_from_address

access_rules_path = var.access_rules_path
access_rules = concat(local.default_access_rules, var.ory_additional_access_rules)
configuration_overrides = var.ory_configuration_overrides
Comment on lines +230 to +231
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, I like this

}

module "k8s_tools" {
source = "./modules/k8s_tools"
install_metrics_server = var.install_metrics_server
}
}
2 changes: 0 additions & 2 deletions modules/ory/oathkeeper/config-oathkeeper.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
---
log:
# Set a lower level when running in production
level: debug
Expand Down Expand Up @@ -108,4 +107,3 @@ mutators:
{
"session": {{ .Extra | toJson }}
}
---
45 changes: 28 additions & 17 deletions modules/ory/oathkeeper/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,45 @@ locals {
config_mount_path = "/etc/config"
secrets_mount_path = "/etc/secrets"
access_rules_path = "${path.module}/access-rule-oathkeeper.yaml"
}

data "template_file" "oathkeeper-access-rules"{
template = file("%{if var.access_rules_path == null}${path.module}/access-rule-oathkeeper.yaml%{else}${var.access_rules_path}%{ endif }")
vars = {
hostname = var.hostname
enable_registration = var.enable_registration
configuration_defaults = yamldecode(file("config-oathkeeper.yaml"))
configuration_default_overrides = {
errors={
handlers={
redirect={
config={
to="${var.protocol}://${var.hostname}/profile/auth/login"
}
}
}
}
mutators={
id_token={
config={
issuer_url="${var.protocol}://${var.hostname}"
jwk_urls="file://${local.secrets_mount_path}/id_token.jwks.json"
}
}
}
Comment on lines +6 to +23
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I don't really like this.

In my opinion, what would be ideal is to, when you need to override something, not touch OML code, but touch a separate yaml data file with the keys you want to override.
We could have support for "additional rules" and "override defaults", but ideally without needing to touch OML.
This because if someone is using release 0.3.0 and wants to upgrade to 0.4.0, it won't be easy with custom stuff written inside it's main.tf files

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All in all, if we can load an yaml for this local instead of writing into it, then it would be solved

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then that yaml can be somewhere in the user's project root and OML lives as a submodule, like we do for omigami.

Copy link
Author

@kayibal kayibal Oct 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't fully understand these comments....

In my opinion, what would be ideal is to, when you need to override something, not touch OML code, but touch a separate yaml data file with the keys you want to override.

This is exactly the point of this PR you have complete control over ory from outside OML now. E.g. if I wanted to touch some config other than hostname or protocol. I can do that like this:

module "oml" {
    ...
    ory_configuration_overrides = yamldecode("mycustom-oathkeeper-config.yaml")
    ...
}

I never have to touche OML's main.tf nor any other files to do that. I can even load that custom config from a yaml file if I already have an oathkeeper configuration and want to hand over controller of ory to OML.

I guess there are a few fine tuning decisions to make as in should we keep support for protocol and hostname? If yes should it take precedence over the overrides?

Copy link
Contributor

@bernardolk bernardolk Oct 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so as aligned in conversation, the user won't need to change main.tf code as I initially suspected. Great :)
Here we are basically "forcing" the values of these particular vars for proper OML functioning. It is to be decided whether we want to do it (if it is actually needed to enforce this) or not.

}
}

data "template_file" "oathkeeper-config"{
template = file("${path.module}/config-oathkeeper.yaml")
vars = {
hostname = var.hostname
protocol = var.protocol
config_path = local.config_mount_path
secret_path = local.secrets_mount_path
}
locals {
configuration = merge(
local.configuration_defaults, # lowest precedence
var.configuration_overrides,
local.configuration_default_overrides # highest precedence
)
Comment on lines +27 to +32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting

}


resource "kubernetes_config_map" "oathkeeper-configs" {
metadata {
name = "oathkeeper-config"
namespace = var.namespace
}
data = {
"access-rule-oathkeeper.yaml" = data.template_file.oathkeeper-access-rules.rendered
"config-oathkeeper.yaml" = data.template_file.oathkeeper-config.rendered
"access-rule-oathkeeper.yaml" = yamlencode(var.access_rules)
"config-oathkeeper.yaml" = yamlencode(local.configuration)
}
}

Expand Down
30 changes: 29 additions & 1 deletion modules/ory/oathkeeper/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,34 @@ variable "protocol" {
default = "http"
}

variable "configuration_overrides" {
description="Additional overrides should follow the config structure as specified here: https://www.ory.sh/oathkeeper/docs/reference/configuration/"
default={}
}

variable "access_rules" {
description = "Access rules"
type=list(object({
id=string
match=object({
url = string
methods = list(string)
})
authenticators = list(object({
handler = string
}))
authorizer=object({
handler=string
})
mutators = list(object({
handler = string
}))
credential_issuer=object({
handler=string
})
}))
}

variable "enable_registration" {
description = "Bool to set if registration page will or not be visible to users"
type = bool
Expand All @@ -18,4 +46,4 @@ variable "enable_registration" {
variable "access_rules_path" {
type = string
default = null
}
}
30 changes: 29 additions & 1 deletion modules/ory/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,34 @@ variable "protocol" {
default = "http"
}

variable "configuration_overrides" {
description="Additional overrides should follow the config structure as specified here: https://www.ory.sh/oathkeeper/docs/reference/configuration/"
default={}
}

variable "access_rules" {
description = "Access rules"
type=list(object({
id=string
match=object({
url = string
methods = list(string)
})
authenticators = list(object({
handler = string
}))
authorizer=object({
handler=string
})
mutators = list(object({
handler = string
}))
credential_issuer=object({
handler=string
})
}))
}

// KRATOS
variable "kratos_db_password"{
description = "PostgreSQL Database Password"
Expand Down Expand Up @@ -69,4 +97,4 @@ variable "oauth2_providers" {
variable "access_rules_path" {
type = string
default = null
}
}
28 changes: 28 additions & 0 deletions variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,34 @@ variable "ory_kratos_cookie_secret" {
sensitive = true
}

variable "ory_configuration_overrides" {
description="Additional overrides should follow the config structure as specified here: https://www.ory.sh/oathkeeper/docs/reference/configuration/"
default={}
}

variable "ory_additional_access_rules" {
description = "Additional access rules. Will be merged with access rules for OML services."
type=list(object({
id=string
match=object({
url = string
methods = list(string)
})
authenticators = list(object({
handler = string
}))
authorizer=object({
handler=string
})
mutators = list(object({
handler = string
}))
credential_issuer=object({
handler=string
})
}))
}

variable "oauth2_providers" {
// Configure multiple Oauth2 providers.
// example:
Expand Down