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

Replace lua scripting for x-robots header with annotation on ingress objects #2865

Closed
twardnw opened this issue Oct 14, 2021 · 8 comments · Fixed by #2867
Closed

Replace lua scripting for x-robots header with annotation on ingress objects #2865

twardnw opened this issue Oct 14, 2021 · 8 comments · Fixed by #2867

Comments

@twardnw
Copy link
Contributor

twardnw commented Oct 14, 2021

We could get rid of https://github.com/uselagoon/lagoon-images/blob/main/images/nginx/helpers/100_x-robots-header-development.conf by bringing this logic into the build and adding this annotation to the ingress object

nginx.ingress.kubernetes.io/server-snippet: |-
       add_header X-Robots-Tag "noindex, nofollow";
@Schnitzel
Copy link
Contributor

as we want these headers only for autogenerated routes and we know exactly which autogenerated routes are created from the helmcharts (basically all ingress objects, except the one for the custom-ingress), we could just add it here directly into the ingress object of all autogenerated routes, example for nginx:
https://github.com/uselagoon/lagoon/blob/main/images/kubectl-build-deploy-dind/helmcharts/nginx/templates/ingress.yaml#L11

@twardnw
Copy link
Contributor Author

twardnw commented Oct 14, 2021

We do need to inject it on custom-ingress of development environments though, so for those we will need to make sure we're not obliterating any custom annotations that customers might be setting

@tobybellwood
Copy link
Member

I'm just testing this now

    {{- if eq .Values.environmentType "development"}}
    nginx.ingress.kubernetes.io/server-snippet: |-
       add_header X-Robots-Tag "noindex, nofollow";
    {{- end }}

I can't see anywhere else that we use server-snippet (it only allows one - https://kubernetes.github.io/ingress-nginx/user-guide/nginx-configuration/annotations/#server-snippet)

@twardnw
Copy link
Contributor Author

twardnw commented Oct 14, 2021

We don't, but clients can via their .lagoon.yml, and there is no limit on which annotations they can add, so it is feasible that some are adding their own server-snippet

https://docs.lagoon.sh/lagoon/using-lagoon-the-basics/lagoon-yml#ingress-annotations

@Schnitzel
Copy link
Contributor

@twardnw
but that's only for custom ingress/routes.
customers cannot modify autogenerated ingresses and we want the headers only with these?

@twardnw
Copy link
Contributor Author

twardnw commented Oct 14, 2021

On development environment we set the x-robots header on all ingresses

@Schnitzel
Copy link
Contributor

Schnitzel commented Oct 14, 2021

oooh, disregard, I found that we add the headers in two cases:

  1. all autogenerated routes
  2. all development environments

Edit: what @twardnw said

@Schnitzel
Copy link
Contributor

so yea, it's possible for the customer to add this to their .lagoon.yml

        - "example.de":
            annotations:
              nginx.ingress.kubernetes.io/server-snippet: return 301 https://m.example.com;

which then would overwrite all server-snippets that lagoon has added.

I could imagine two possibilites:

  1. we document that if a customer uses nginx.ingress.kubernetes.io/server-snippet in the .lagoon.yml that they need to add the add_header X-Robots-Tag "noindex, nofollow"; by themselves
  2. we merge any stuff set in .lagoon.yml nginx.ingress.kubernetes.io/server-snippet with our own server-snippet.

I'm ok with both. Nr 1 would be easier and not many customers are using the route annotation anyways?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants