wordpress-charmers team mailing list archive

[Tom Haddon <mp+414465@xxxxxxxxxxxxxxxxxx>]

Some questions/comments inline

Diff comments:

> diff --git a/config.yaml b/config.yaml
> index a043cad..6af47e2 100644
> --- a/config.yaml
> +++ b/config.yaml
> @@ -106,3 +106,8 @@ options:
>        YAML dictionary with keys named after WordPress settings and the desired values.
>        Please note that the settings will be reset to values provided every time hooks run.
>      default: ""
> +  use_nginx_ingress_modsec:
> +    type: boolean
> +    default: true

You were building this optionally in the docker image, but are now setting this to true by default. Is that really what we want here (that's not a leading question, I'm honestly not sure of the upsides/downsides)?

> +    description: >
> +      When set to true, the charm will configure the k8s ingress with modsec enabled.
> diff --git a/src/charm.py b/src/charm.py
> index cba136a..628677f 100755
> --- a/src/charm.py
> +++ b/src/charm.py
> @@ -261,6 +261,15 @@ class WordpressCharm(CharmBase):
>                  ]
>              },
>          }
> +        modsec_annotations = {
> +            "nginx.ingress.kubernetes.io/enable-modsecurity": "true",
> +            "nginx.ingress.kubernetes.io/enable-owasp-modsecurity-crs": "true",
> +            "nginx.ingress.kubernetes.io/modsecurity-snippet":
> +                ("SecRuleEngine On\n"
> +                 "Include /etc/nginx/owasp-modsecurity-crs/nginx-modsecurity.conf"),
> +        }

Worth moving this inside the `if` block so it's only defined if used?

> +        if self.model.config["use_nginx_ingress_modsec"]:
> +            resources["kubernetesResources"]["ingressResources"][0]["annotations"].update(modsec_annotations)
>  
>          if self.model.config["additional_hostnames"]:
>              additional_hostnames = juju_setting_to_list(self.model.config["additional_hostnames"])


-- 
https://code.launchpad.net/~sajoupa/charm-k8s-wordpress/+git/charm-k8s-wordpress/+merge/414465
Your team Wordpress Charmers is subscribed to branch charm-k8s-wordpress:master.