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

Allow configuration of nginx ingress controller #216

Merged

Conversation

mueller-ma
Copy link
Contributor

Description

Allow configuration of default nginx certificate.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Small minor change not affecting the Ansible Role code (GitHub Actions Workflow, Documentation etc.)

How Has This Been Tested?

I executed the role on my cluster and set rke2_ingress_nginx_default_certificate: "cert-manager/default-cert". With cert-manager I created a certificate in the secret cert-manager/default-cert.

@MonolithProjects MonolithProjects self-assigned this May 23, 2024
@MonolithProjects MonolithProjects added the enhancement New feature or request label May 24, 2024
tasks/ingress-nginx.yml Show resolved Hide resolved
tasks/ingress-nginx.yml Outdated Show resolved Hide resolved
@mueller-ma mueller-ma force-pushed the feat/configure-nginx-default-cert branch from 72bb0d1 to 1dd3ae9 Compare May 27, 2024 09:59
@mueller-ma mueller-ma changed the title Allow configuration of default nginx certificate Allow configuration of nginx ingress controller May 27, 2024
@mueller-ma
Copy link
Contributor Author

I updated the PR to make all values in this file configurable.

Copy link
Collaborator

@MonolithProjects MonolithProjects left a comment

Choose a reason for hiding this comment

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

LGTM

@MonolithProjects MonolithProjects merged commit 939ebd7 into lablabs:main May 28, 2024
5 checks passed
@mueller-ma mueller-ma deleted the feat/configure-nginx-default-cert branch May 28, 2024 05:34
@elbvdk
Copy link

elbvdk commented Jun 3, 2024

I think this should be marked as a Breaking Change; if you previously used the custom manifest deployment method to deploy those nginx configuration options, this will overwrite that custom manifest.

In our case, our deployment was failing because use-forwarded-hearders: true was removed from the nginx config which caused our Rancher to get stuck in an HTTP 308 redirect loop.

@MonolithProjects
Copy link
Collaborator

That's a good point @elbvdk. Thanks. Will add that to the release notes

@MonolithProjects
Copy link
Collaborator

@elbvdk if you want to keep using custom manifest for nginx config, use release 1.34.1 where this issue is fixed (rke2_ingress_nginx_values will be used only if it's not empty)

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

Successfully merging this pull request may close these issues.

None yet

3 participants