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

Add support for ingress nginx and certbot to ingress.yml #5613

Merged
merged 2 commits into from Aug 10, 2022

Conversation

lotas
Copy link
Contributor

@lotas lotas commented Aug 10, 2022

This introduces few changes:

  • ingress.yml can be configured through dev-config.yml option ingressType: nginx to generate proper routes (/ instead of /*)
  • ingress.yml can include cert-manager annotations to provision certificates automatically
  • Improved dev-deployment.yml to include information on how to use those options

Fixes #4913

@lotas lotas requested a review from a team as a code owner August 10, 2022 10:36
@lotas lotas requested review from petemoore and matt-boris and removed request for a team August 10, 2022 10:36
@lotas lotas force-pushed the feat/4913-ingress-nginx-support branch from 1965883 to d273e88 Compare August 10, 2022 10:38
@@ -87,6 +87,7 @@ script/dev:db:downgrade) shift; exec yarn run dev:db:downgrade "${@}";;
script/dev:apply) shift; exec yarn run dev:apply "${@}";;
script/dev:delete) shift; exec yarn run dev:delete "${@}";;
script/dev:verify) shift; exec yarn run dev:verify "${@}";;
script/dev:templates) shift; exec yarn run dev:templates "${@}";;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is to dump k8s templates with substituted variables into a file for debug purposes

@@ -111,14 +115,23 @@ const actions = [
provides: ['target-verify'],
run: async (requirements, utils) => ({
'target-verify': await execCommand({
command: ['kubectl', 'apply', '--dry-run', '-f', '-'],
command: ['kubectl', 'apply', '--dry-run=client', '-f', '-'],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the preferred way now, old was deprecated

@@ -6,14 +6,24 @@ metadata:
annotations:
'kubernetes.io/ingress.global-static-ip-name': '{{ .Values.ingressStaticIpName }}'
'ingress.gcp.kubernetes.io/pre-shared-cert': '{{ .Values.ingressCertName }}'
'cert-manager.io/cluster-issuer': '{{ .Values.certManagerClusterIssuerName | default "" }}'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

those extra annotations will do no harm when left blank to any existing deployment.

Plus mozilla is overriding those for moz-specific deployments

Copy link
Member

@petemoore petemoore left a comment

Choose a reason for hiding this comment

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

Looks great, many thanks Yarik!

@Eijebong Do you have anything to add?

Copy link
Contributor

@Eijebong Eijebong left a comment

Choose a reason for hiding this comment

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

Got nothing to add, this looks good to me.

@lotas lotas merged commit 7e535b5 into main Aug 10, 2022
@lotas lotas deleted the feat/4913-ingress-nginx-support branch August 10, 2022 15:53
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 this pull request may close these issues.

Current ingress YAML does not work with nginx ingress
3 participants