[bitnami/wordpress] Add secondary ingress to Wordpress for /wp-admin

[djjudas21]

Description of the change

This change adds a secondary Ingress to the Wordpress helm chart. The intention is that a secondary Ingress may be used to serve the /wp-admin endpoint and add Ingress annotations for authentication, IP restriction, etc. Annotations are per-ingress, not per-path so a separate Ingress is necessary. There is some relevant discussion on #17007.

Benefits

This change allows increased security by allowing the user to protect the /wp-admin management endpoint and keeping it separate from the public-facing Wordpress site.

Possible drawbacks

Ideally we would implement an arbitrary number of Ingresses, as in bjw-s/common. However this would be a breaking change for every chart that uses bitnami/common. As a compromise, we add this secondary Ingress.

There may also be a smarter way to handle /wp-admin by creating Ingress resources with the paths already filled in.

Applicable issues

• partial fix for [bitnami/keycloak] Support Multiple Ingress #17007

Checklist

• Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
• Variables are documented in the values.yaml and added to the README.md using readme-generator-for-helm
• Title of the pull request follows this pattern [bitnami/<name_of_the_chart>] Descriptive title
• All commits signed off and in agreement of Developer Certificate of Origin (DCO)

I'll see if I can figure out the readme generator and the signing later. For now I wanted to create this PR to start a discussion about it.

[carrodher]

Thank you for initiating this pull request. We appreciate your effort. Just a friendly reminder that it's important to sign your commits. Adding your signature certifies that you either authored the patch or have the necessary rights to contribute the changes. You can find detailed information on how to do this in the “Sign your work” section of our contributing guidelines.

Feel free to reach out if you have any questions or need assistance with the signing process.

Apart from that, please note there are some issues related to the metadata that should appear in the values.yaml to properly generate the table in the README describing the different configuration parameters:

ERROR: Missing metadata for key: secondaryIngress.enabled
ERROR: Missing metadata for key: secondaryIngress.pathType
ERROR: Missing metadata for key: secondaryIngress.apiVersion
ERROR: Missing metadata for key: secondaryIngress.ingressClassName
ERROR: Missing metadata for key: secondaryIngress.hostname
ERROR: Missing metadata for key: secondaryIngress.path
ERROR: Missing metadata for key: secondaryIngress.annotations
ERROR: Missing metadata for key: secondaryIngress.tls
ERROR: Missing metadata for key: secondaryIngress.tlsWwwPrefix
ERROR: Missing metadata for key: secondaryIngress.selfSigned
ERROR: Missing metadata for key: secondaryIngress.extraHosts
ERROR: Missing metadata for key: secondaryIngress.extraPaths
ERROR: Missing metadata for key: secondaryIngress.extraTls
ERROR: Missing metadata for key: secondaryIngress.secrets
ERROR: Missing metadata for key: secondaryIngress.extraRules

[djjudas21]

Thanks, I've fixed the comments in values.yaml, and added signed commits

[djjudas21]

Hey, could I get some help merging this please? It's all good from my side but I'm not sure what the procedure is. Thanks

[andresbono]

I left some minor review comments, thank you very much for your contribution!!

But before we continue with the review process and merge of this PR, I think we should check if the proposed implementation fits in the use case you described, for example restrict the allowed IPs with access to /wp-admin.

What would be the values a user would pass to the Helm chart installation to achieve that?

Wouldn't the main ingress still serve /wp-admin anyways?

Please, let's clarify that. Thank you again for your contribution!

[djjudas21]

Hi @andresbono. I'll have a look at your review comments next, but to answer your question about use case examples:

The main ingress

Yes, this will require user action to prevent exposing /wp-admin. There are several ways to achieve this, but the most common seems to be adding an annotation to deny a path. It's also possible to route the /wp-admin path to the defaultBackend service, which always returns 404.

ingress:
  annotations:
    nginx.ingress.kubernetes.io/configuration-snippet: |

    server_tokens off;
    location /wp-admin {
      deny all;
      return 403;
    }

The secondary ingress

Annotations can be used to secure the secondary ingress for /wp-admin. It seems a bit beyond the scope of this helm chart to describe ingress security methods, but here are a few common examples.

To restrict to a whitelist of IPs:

ingress:    
  annotations:
    nginx.ingress.kubernetes.io/whitelist-source-range: "192.168.0.0/24"

To apply basic auth from a secret:

ingress:    
  annotations:
    nginx.ingress.kubernetes.io/auth-type: basic
    # name of the secret that contains the user/password definitions
    nginx.ingress.kubernetes.io/auth-secret: basic-auth
    nginx.ingress.kubernetes.io/auth-realm: 'Authentication Required - foo'

To apply auth via oauth2-proxy (e.g. to GitHub or social login):

ingress:    
  annotations:
    nginx.ingress.kubernetes.io/auth-url: "https://auth.example.com/oauth2/auth"
    nginx.ingress.kubernetes.io/auth-signin: "https://auth.example.com/oauth2/start?rd=https%3A%2F%2F$host$request_uri"

[andresbono]

LGTM, thank you for addressing the issues and explaining the use case where this secondary ingress would be of help.

[andresbono]

Please, check the DCO action details. There are a couple of commits with missing sign-off. In that page you have instructions to fix it. Let us know if you need help.

[djjudas21]

Argh, I followed the instructions in https://github.com/bitnami/charts/pull/24311/checks?check_run_id=23918274064 and it has pulled in loads of commits. I'm probably going to have to make a new branch and cherry-pick my handful of commits into it.

[andresbono]

Argh, I followed the instructions in https://github.com/bitnami/charts/pull/24311/checks?check_run_id=23918274064 and it has pulled in loads of commits. I'm probably going to have to make a new branch and cherry-pick my handful of commits into it.

I think it should be good now

[andresbono]

Merged now, thank you!