-
Notifications
You must be signed in to change notification settings - Fork 1
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
Initialize Cert-manager and Gateway API workflow #31
Conversation
/assign @irbekrm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @lonelyCZ I think this is great work and good practices like using variables and parameterizing values instead of hardcoding - well done 👍🏼 👍🏼
I have left a bunch of suggestions- let me know what you think. I will try out this feature on GKE once it is possible to use an ACME issuer with LetsEncrypt - I left a comment for that, let me know what you think!
Again- well done 👏🏼
cm-gateway/gateway-controller-install/projectcontour/contour.yaml
Outdated
Show resolved
Hide resolved
I've tried to use the resources deployed in this PR together with an HTTPRoute, Service and a Deployment similar to https://raw.githubusercontent.com/projectcontour/contour/v1.22.0/examples/example-workload/gatewayapi/kuard/kuard.yaml on GKE with the Gateway exposed on a public IP- after a few modification all worked as expected and I saw that Gateway controller uses the newly provisioned cert from
I think we want to make the envoy service type configurable to allow accessing it from outside if needed. This might be easier to do if we are able to use the Helm chart |
Thanks for your tests! I will fix these comments ASAP.
Indeed, but the Helm chart doesn't seem to work very well for gateway. What should we do? |
I only looked at this very briefly- I do see that it does not work out of the box (it does not pick up the GatewayClass that we deploy). I think the reason for this is that we don't have the Configmap with these options deployed by default with the Helm chart. I see that it is possible to configure it with the Helm chart via this param. I haven't actually tried it, but I believe it should work, I see that support for Gateway API was added to this chart in bitnami/charts#9029
I think we don't need the Gateway Provisioner, it belongs to the 'dynamic' Contour configuration, but we use static https://projectcontour.io/guides/gateway-api/ I think we should try to set the missing configmap values. Let me know what you think- I can look at this some more tomorrow as well |
Thanks for your guidences, I found it can work fine after configurating the |
Althought bitnami/charts#9029 has been merged, but the latest version of the chart hasn't released, how should we do? It is not this problem, I continue to research it.
It seemingly lacks |
2c14589
to
1dcd186
Compare
fd500b8
to
3e96a58
Compare
/assign @irbekrm |
/cc @irbekrm |
Signed-off-by: lonelyCZ <531187475@qq.com>
Signed-off-by: lonelyCZ <531187475@qq.com>
3e96a58
to
8f54bc8
Compare
/cc @irbekrm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this great work!
I've ran the latest version of the PR locally and saw the certs get created successfully 👍🏼
It would be awesome if we can also do #33 then we'll be able to test this with and actual e2e flow easier, but let me know if you think there will be issues with testing the ACME issuer.
/lgtm
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: irbekrm, lonelyCZ The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fixes #25
The result resources are below: