Implement the Gateway API support to configurate an ACME issuer #36

lonelyCZ · 2022-08-24T05:46:49Z

Signed-off-by: lonelyCZ 531187475@qq.com

Fixes #33

lonelyCZ · 2022-08-24T05:50:45Z

cm-gateway/common.tfvars

+
+# The following parameters must be used in combination
+# 1. issuer_type is "SelfSigned" and envoy.service.type is "NodePort", which can be tested in a local kind environment.
+# 2. issuer_type is "ACME" and envoy.service.type is "LoadBalancer", which can be tested in a cloud environment such as GKE.


Now it is only tested, if there will not problem, I will do other works, like update readme.

lonelyCZ · 2022-08-24T05:52:03Z

Hi @irbekrm, could you please help me to test this pr in GKE environment.

/assign @irbekrm

irbekrm

Thanks for working on this @lonelyCZ !

I've tried the module, but am getting error during ACME issuer creation. I can reproduce this on kind cluster as well

│ Error: Plugin did not respond
│ 
│   with kubernetes_manifest.gateway-issuer-acme[0],
│   on main.tf line 121, in resource "kubernetes_manifest" "gateway-issuer-acme":
│  121: resource "kubernetes_manifest" "gateway-issuer-acme" {
│ 
│ The plugin encountered an error, and failed to respond to the
│ plugin.(*GRPCProvider).ApplyResourceChange call. The plugin logs may
│ contain more details.
╵

Stack trace from the terraform-provider-kubernetes_v2.12.1_x5 plugin:

panic: interface conversion: interface {} is nil, not map[string]interface {}
...

(As a sidenote, we probably want to add a dependency on gateway API install to cert-manager install module as the cert-manager pod crashes if the Gateway feature is enabled, but it does not find the Gateway CRDs)

irbekrm · 2022-08-24T06:33:02Z

cm-gateway/cm-config/variables.tf

@@ -13,3 +13,11 @@ variable "issuer_name" {
  default = "gateway-issuer"
 }

+variable "issuer_type" {
+  type = string


Here we should validate that it is one of the accepted types (similar to what you did for vault auth mechanisms https://github.com/cert-manager/testing-addons/blob/master/cm-vault/vault-config/variables.tf#L10

irbekrm · 2022-08-24T06:36:25Z

cm-gateway/cm-config/main.tf

 resource "kubernetes_manifest" "selfsigned-issuer" {
+  count = var.issuer_type == "SelfSigned" ? 1 : 0


I think this issuer type should be called something like SelfSignedCA as it is a CA issuer that is actually used to sign the Gateway certs not a self-signed one

cm-gateway/common.tfvars

lonelyCZ · 2022-08-24T11:41:50Z

│ Error: Plugin did not respond
│ 
│   with kubernetes_manifest.gateway-issuer-acme[0],
│   on main.tf line 121, in resource "kubernetes_manifest" "gateway-issuer-acme":
│  121: resource "kubernetes_manifest" "gateway-issuer-acme" {
│ 
│ The plugin encountered an error, and failed to respond to the
│ plugin.(*GRPCProvider).ApplyResourceChange call. The plugin logs may
│ contain more details.
╵

Stack trace from the terraform-provider-kubernetes_v2.12.1_x5 plugin:

panic: interface conversion: interface {} is nil, not map[string]interface {}
...

I can reproduce this on kind cluster as well

I guess this is error coming from Kubernetes Provider, because it can run in my kind cluster environment.

The plugin encountered an error, and failed to respond to the plugin.(*GRPCProvider).ApplyResourceChange call

It seems that the previous resources haven't removed completely. Could you please help to retest it in your env.

But now there are some problems certainly, for example, if there are some errors occurring when executing terragrunt run-all destroy, we also can't run it secondly, I will open an issue to trace this problem and improve its stability.

As a sidenote, we probably want to add a dependency on gateway API install to cert-manager install module as the cert-manager pod crashes if the Gateway feature is enabled, but it does not find the Gateway CRDs.

From the Dependencies Graph, currently, the cert-manager should be deployed after gateway-api installed. Could you describe your encountered problems?

lonelyCZ · 2022-08-24T14:03:42Z

I also reproduced this error in my kind cluster.

Error: Plugin did not respond
│
│   with kubernetes_manifest.gateway-issuer-acme[0],
│   on main.tf line 121, in resource "kubernetes_manifest" "gateway-issuer-acme":
│  121: resource "kubernetes_manifest" "gateway-issuer-acme" {

I found the gateway-issuer was not ready, I guess the above error is caused by it.

[root@master68 cm-gateway]# kubectl get issuer -A
NAMESPACE      NAME             READY   AGE
gateway-demo   gateway-issuer   False   97m

status:
  acme: {}
  conditions:
  - lastTransitionTime: "2022-08-24T12:21:44Z"
    message: 'Failed to register ACME account: 400 urn:ietf:params:acme:error:invalidEmail:
      Error creating new account :: invalid contact domain. Contact emails @example.com
      are forbidden'
    observedGeneration: 1
    reason: ErrRegisterACMEAccount
    status: "False"
    type: Ready

lonelyCZ · 2022-08-24T14:25:04Z

cm-gateway/cm-config/main.tf

+    }
+    "spec" = {
+      "acme" = {
+        "email" : var.acme_email


I guess the problem is the configuration of acme, could you please help me to deploy it alone to debug it?

Ahh yes you're right there is a problem with the example email

status: acme: {} conditions: - lastTransitionTime: "2022-08-24T15:12:58Z" message: 'Failed to register ACME account: 400 urn:ietf:params:acme:error:invalidEmail: Error creating new account :: invalid contact domain. Contact emails @example.com are forbidden' observedGeneration: 1 reason: ErrRegisterACMEAccount status: "False" type: Read

... however I see that even with the correct email when the issuer does become ready it nevertheless crashes.

We should investigate for a bit more, if it doesn't help, we can try to temporarily remove the wait condition.

I see that even with the correct email when the issuer does become ready it nevertheless crashes.

Is it the reason that is timeout? The current creation time is only 1 minute.

testing-addons/cm-gateway/cm-config/main.tf

Line 43 in 8322d6a

create = "1m"

lonelyCZ · 2022-08-29T05:59:41Z

Hi, @irbekrm does it work if remove the wait condition? Or is it the reason that is timeout? The current creation time is only 1 minute.

/assign @irbekrm

irbekrm · 2022-08-31T04:03:28Z

Hi, @irbekrm does it work if remove the wait condition?

Hi @lonelyCZ yes, if the wait condition is removed the ACME issuer gets created correctly 👍🏼
I think it is a problem with how the conditions get parsed in the upstream terraform module. Perhaps we can investigate and try to open an issue upstream later, but for now we could just remove the wait condition.

I now see another error where the ACME challenge does not get solved correctly, looking into it..

irbekrm · 2022-08-31T04:21:18Z

I now see another error where the ACME challenge does not get solved correctly, looking into it..

I think we need another Gateway that would forward plain HTTP traffic from all routes and that cert-manager could use to attach the HTTPRoute it creates for the challenge. At the moment we configure the issuer to attach the challenge route to the same gateway that we are issuing the cert for, but this Gateway will only become Ready when the cert is issued, so it cannnot be used to route the traffik to the challenge pod. So, we need a plain HTTP Gateway like this as well

Let me know if that makes sense

lonelyCZ · 2022-08-31T04:26:11Z

I think we need another Gateway that would forward plain HTTP traffic from all routes and that cert-manager could use to attach the HTTPRoute it creates for the challenge. At the moment we configure the issuer to attach the challenge route to the same gateway that we are issuing the cert for, but this Gateway will only become Ready when the cert is issued, so it cannnot be used to route the traffik to the challenge pod. So, we need a plain HTTP Gateway like this as well

Thanks for your investigations, I will do it as your suggestions today.

irbekrm · 2022-08-31T05:00:09Z

Thanks for your investigations, I will do it as your suggestions today.

Thank you @lonelyCZ 😄

lonelyCZ · 2022-08-31T12:59:07Z

Please take a look. The dependencies graph also has been updated.

/assign @irbekrm

irbekrm · 2022-09-02T07:15:48Z

@lonelyCZ I think we still need to remove that wait condition from the issuer #36 (comment)

lonelyCZ · 2022-09-02T07:19:22Z

I think we still need to remove that wait condition from the issuer #36 (comment)

Ok, any other problems? If no, I am going to update the readme file.

/hold

irbekrm · 2022-09-02T10:55:57Z

Ok, any other problems?

I think after that this should be good 👍🏼

lonelyCZ · 2022-09-03T07:07:55Z

/unhold

/assign @irbekrm

Signed-off-by: lonelyCZ <531187475@qq.com>

irbekrm · 2022-09-07T07:39:13Z

Thanks @lonelyCZ!

/lgtm
/approve

jetstack-bot · 2022-09-07T07:39:18Z

[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:

~~OWNERS~~ [irbekrm]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

jetstack-bot added dco-signoff: yes size/L labels Aug 24, 2022

lonelyCZ force-pushed the pr-issuer-acme branch from 21a1e20 to 18cca64 Compare August 24, 2022 05:49

lonelyCZ commented Aug 24, 2022

View reviewed changes

jetstack-bot assigned irbekrm Aug 24, 2022

irbekrm suggested changes Aug 24, 2022

View reviewed changes

irbekrm assigned lonelyCZ and unassigned irbekrm Aug 24, 2022

lonelyCZ force-pushed the pr-issuer-acme branch 2 times, most recently from fec73f8 to 8322d6a Compare August 24, 2022 14:22

lonelyCZ commented Aug 24, 2022

View reviewed changes

lonelyCZ removed their assignment Aug 29, 2022

jetstack-bot assigned irbekrm Aug 29, 2022

irbekrm assigned lonelyCZ and unassigned irbekrm Aug 31, 2022

lonelyCZ force-pushed the pr-issuer-acme branch from 8322d6a to 2999ed9 Compare August 31, 2022 12:57

lonelyCZ removed their assignment Aug 31, 2022

jetstack-bot assigned irbekrm Aug 31, 2022

jetstack-bot added the do-not-merge/hold label Sep 2, 2022

irbekrm assigned lonelyCZ and unassigned irbekrm Sep 2, 2022

lonelyCZ force-pushed the pr-issuer-acme branch from 2999ed9 to 5324095 Compare September 3, 2022 07:06

lonelyCZ removed their assignment Sep 3, 2022

jetstack-bot assigned irbekrm Sep 3, 2022

jetstack-bot removed the do-not-merge/hold label Sep 3, 2022

lonelyCZ added 2 commits September 4, 2022 17:13

Implement the Gateway API support to configurate an ACME issuer

a5f62eb

Signed-off-by: lonelyCZ <531187475@qq.com>

Update the readme file of Gateway API workflow

cbfe992

Signed-off-by: lonelyCZ <531187475@qq.com>

lonelyCZ force-pushed the pr-issuer-acme branch from 5324095 to cbfe992 Compare September 4, 2022 09:13

jetstack-bot added the lgtm label Sep 7, 2022

jetstack-bot added the approved label Sep 7, 2022

jetstack-bot merged commit 519ae43 into cert-manager:master Sep 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement the Gateway API support to configurate an ACME issuer #36

Implement the Gateway API support to configurate an ACME issuer #36

		resource "kubernetes_manifest" "selfsigned-issuer" {
		count = var.issuer_type == "SelfSigned" ? 1 : 0

Implement the Gateway API support to configurate an ACME issuer #36

Implement the Gateway API support to configurate an ACME issuer #36

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment