diff --git a/cmd/controller/app/controller.go b/cmd/controller/app/controller.go index b8f37b7e5..61c006513 100644 --- a/cmd/controller/app/controller.go +++ b/cmd/controller/app/controller.go @@ -283,8 +283,7 @@ func buildControllerContextFactory(ctx context.Context, opts *options.Controller ACMEHTTP01SolverRunAsNonRoot: ACMEHTTP01SolverRunAsNonRoot, HTTP01SolverImage: opts.ACMEHTTP01SolverImage, // Allows specifying a list of custom nameservers to perform HTTP01 checks on. - HTTP01SolverNameservers: opts.ACMEHTTP01SolverNameservers, - HTTP01SolverUseIngressClass: opts.ACMEHTTP01SolverUseIngressClassName, + HTTP01SolverNameservers: opts.ACMEHTTP01SolverNameservers, DNS01Nameservers: nameservers, DNS01CheckRetryPeriod: opts.DNS01CheckRetryPeriod, diff --git a/cmd/controller/app/options/options.go b/cmd/controller/app/options/options.go index a06967438..5606f8839 100644 --- a/cmd/controller/app/options/options.go +++ b/cmd/controller/app/options/options.go @@ -82,8 +82,7 @@ type ControllerOptions struct { ACMEHTTP01SolverResourceLimitsMemory string ACMEHTTP01SolverRunAsNonRoot bool // Allows specifying a list of custom nameservers to perform HTTP01 checks on. - ACMEHTTP01SolverNameservers []string - ACMEHTTP01SolverUseIngressClassName bool + ACMEHTTP01SolverNameservers []string ClusterIssuerAmbientCredentials bool IssuerAmbientCredentials bool @@ -141,8 +140,6 @@ const ( defaultTLSACMEIssuerGroup = cm.GroupName defaultEnableCertificateOwnerRef = false - defaultACMEHTTP01SolverUseIngressClassName = false - defaultDNS01RecursiveNameserversOnly = false defaultMaxConcurrentChallenges = 60 @@ -324,10 +321,6 @@ func (s *ControllerOptions) AddFlags(fs *pflag.FlagSet) { "ACME HTTP01 check requests. This should be a list containing host and "+ "port, for example 8.8.8.8:53,8.8.4.4:53") - fs.BoolVar(&s.ACMEHTTP01SolverUseIngressClassName, "acme-http01-solver-use-ingress-class-name", defaultACMEHTTP01SolverUseIngressClassName, ""+ - "Whether ACME HTTP01 Ingress resources should use the new ingressClassName "+ - "or the deprecated annotation.") - fs.BoolVar(&s.ClusterIssuerAmbientCredentials, "cluster-issuer-ambient-credentials", defaultClusterIssuerAmbientCredentials, ""+ "Whether a cluster-issuer may make use of ambient credentials for issuers. 'Ambient Credentials' are credentials drawn from the environment, metadata services, or local files which are not explicitly configured in the ClusterIssuer API object. "+ "When this flag is enabled, the following sources for credentials are also used: "+ diff --git a/deploy/crds/crd-challenges.yaml b/deploy/crds/crd-challenges.yaml index 9b849a0b1..f5b1f0b10 100644 --- a/deploy/crds/crd-challenges.yaml +++ b/deploy/crds/crd-challenges.yaml @@ -444,7 +444,10 @@ spec: type: object properties: class: - description: The ingress class to use when creating Ingress resources to solve ACME challenges that use this challenge solver. Only one of 'class' or 'name' may be specified. + description: This field configures the annotation `kubernetes.io/ingress.class` when creating Ingress resources to solve ACME challenges that use this challenge solver. Only one of `class`, `name` or `ingressClassName` may be specified. + type: string + ingressClassName: + description: This field configures the field `ingressClassName` on the created Ingress resources used to solve ACME challenges that use this challenge solver. This is the recommended way of configuring the ingress class. Only one of `class`, `name` or `ingressClassName` may be specified. type: string ingressTemplate: description: Optional ingress template used to configure the ACME challenge solver ingress used for HTTP01 challenges. diff --git a/deploy/crds/crd-clusterissuers.yaml b/deploy/crds/crd-clusterissuers.yaml index ab2ec6152..55ed02a51 100644 --- a/deploy/crds/crd-clusterissuers.yaml +++ b/deploy/crds/crd-clusterissuers.yaml @@ -483,7 +483,10 @@ spec: type: object properties: class: - description: The ingress class to use when creating Ingress resources to solve ACME challenges that use this challenge solver. Only one of 'class' or 'name' may be specified. + description: This field configures the annotation `kubernetes.io/ingress.class` when creating Ingress resources to solve ACME challenges that use this challenge solver. Only one of `class`, `name` or `ingressClassName` may be specified. + type: string + ingressClassName: + description: This field configures the field `ingressClassName` on the created Ingress resources used to solve ACME challenges that use this challenge solver. This is the recommended way of configuring the ingress class. Only one of `class`, `name` or `ingressClassName` may be specified. type: string ingressTemplate: description: Optional ingress template used to configure the ACME challenge solver ingress used for HTTP01 challenges. diff --git a/deploy/crds/crd-issuers.yaml b/deploy/crds/crd-issuers.yaml index 042e1cb5d..93d422e94 100644 --- a/deploy/crds/crd-issuers.yaml +++ b/deploy/crds/crd-issuers.yaml @@ -483,7 +483,10 @@ spec: type: object properties: class: - description: The ingress class to use when creating Ingress resources to solve ACME challenges that use this challenge solver. Only one of 'class' or 'name' may be specified. + description: This field configures the annotation `kubernetes.io/ingress.class` when creating Ingress resources to solve ACME challenges that use this challenge solver. Only one of `class`, `name` or `ingressClassName` may be specified. + type: string + ingressClassName: + description: This field configures the field `ingressClassName` on the created Ingress resources used to solve ACME challenges that use this challenge solver. This is the recommended way of configuring the ingress class. Only one of `class`, `name` or `ingressClassName` may be specified. type: string ingressTemplate: description: Optional ingress template used to configure the ACME challenge solver ingress used for HTTP01 challenges. diff --git a/internal/apis/acme/types_issuer.go b/internal/apis/acme/types_issuer.go index 29f41c356..53989fdad 100644 --- a/internal/apis/acme/types_issuer.go +++ b/internal/apis/acme/types_issuer.go @@ -212,9 +212,16 @@ type ACMEChallengeSolverHTTP01Ingress struct { // +optional ServiceType corev1.ServiceType - // The ingress class to use when creating Ingress resources to solve ACME - // challenges that use this challenge solver. - // Only one of 'class' or 'name' may be specified. + // This field configures the `ingressClassName` when creating Ingress + // resources to solve ACME challenges that use this challenge solver. This + // is the recommended way of configuring the ingress class. Only one of + // `class`, `name` or `ingressClassName` may be specified. + IngressClassName *string + + // This field configures the annotation `kubernetes.io/ingress.class` when + // creating Ingress resources to solve ACME challenges that use this + // challenge solver. Only one of `class`, `name` or `ingressClassName` may + // be specified. Class *string // The name of the ingress resource that should have ACME challenge solving diff --git a/internal/apis/acme/v1/zz_generated.conversion.go b/internal/apis/acme/v1/zz_generated.conversion.go index e0b267287..df119898f 100644 --- a/internal/apis/acme/v1/zz_generated.conversion.go +++ b/internal/apis/acme/v1/zz_generated.conversion.go @@ -693,6 +693,7 @@ func Convert_acme_ACMEChallengeSolverHTTP01GatewayHTTPRoute_To_v1_ACMEChallengeS func autoConvert_v1_ACMEChallengeSolverHTTP01Ingress_To_acme_ACMEChallengeSolverHTTP01Ingress(in *v1.ACMEChallengeSolverHTTP01Ingress, out *acme.ACMEChallengeSolverHTTP01Ingress, s conversion.Scope) error { out.ServiceType = corev1.ServiceType(in.ServiceType) + out.IngressClassName = (*string)(unsafe.Pointer(in.IngressClassName)) out.Class = (*string)(unsafe.Pointer(in.Class)) out.Name = in.Name out.PodTemplate = (*acme.ACMEChallengeSolverHTTP01IngressPodTemplate)(unsafe.Pointer(in.PodTemplate)) @@ -707,6 +708,7 @@ func Convert_v1_ACMEChallengeSolverHTTP01Ingress_To_acme_ACMEChallengeSolverHTTP func autoConvert_acme_ACMEChallengeSolverHTTP01Ingress_To_v1_ACMEChallengeSolverHTTP01Ingress(in *acme.ACMEChallengeSolverHTTP01Ingress, out *v1.ACMEChallengeSolverHTTP01Ingress, s conversion.Scope) error { out.ServiceType = corev1.ServiceType(in.ServiceType) + out.IngressClassName = (*string)(unsafe.Pointer(in.IngressClassName)) out.Class = (*string)(unsafe.Pointer(in.Class)) out.Name = in.Name out.PodTemplate = (*v1.ACMEChallengeSolverHTTP01IngressPodTemplate)(unsafe.Pointer(in.PodTemplate)) diff --git a/internal/apis/acme/v1alpha2/types_issuer.go b/internal/apis/acme/v1alpha2/types_issuer.go index 64d0fa708..27e2730b2 100644 --- a/internal/apis/acme/v1alpha2/types_issuer.go +++ b/internal/apis/acme/v1alpha2/types_issuer.go @@ -232,9 +232,17 @@ type ACMEChallengeSolverHTTP01Ingress struct { // +optional ServiceType corev1.ServiceType `json:"serviceType,omitempty"` - // The ingress class to use when creating Ingress resources to solve ACME - // challenges that use this challenge solver. - // Only one of 'class' or 'name' may be specified. + // This field configures the field `ingressClassName` on the created Ingress + // resources used to solve ACME challenges that use this challenge solver. + // This is the recommended way of configuring the ingress class. Only one of + // `class`, `name` or `ingressClassName` may be specified. + // +optional + IngressClassName *string `json:"ingressClassName,omitempty"` + + // This field configures the annotation `kubernetes.io/ingress.class` when + // creating Ingress resources to solve ACME challenges that use this + // challenge solver. Only one of `class`, `name` or `ingressClassName` may + // be specified. // +optional Class *string `json:"class,omitempty"` diff --git a/internal/apis/acme/v1alpha2/zz_generated.conversion.go b/internal/apis/acme/v1alpha2/zz_generated.conversion.go index cb89590ae..7de51defd 100644 --- a/internal/apis/acme/v1alpha2/zz_generated.conversion.go +++ b/internal/apis/acme/v1alpha2/zz_generated.conversion.go @@ -692,6 +692,7 @@ func Convert_acme_ACMEChallengeSolverHTTP01GatewayHTTPRoute_To_v1alpha2_ACMEChal func autoConvert_v1alpha2_ACMEChallengeSolverHTTP01Ingress_To_acme_ACMEChallengeSolverHTTP01Ingress(in *ACMEChallengeSolverHTTP01Ingress, out *acme.ACMEChallengeSolverHTTP01Ingress, s conversion.Scope) error { out.ServiceType = v1.ServiceType(in.ServiceType) + out.IngressClassName = (*string)(unsafe.Pointer(in.IngressClassName)) out.Class = (*string)(unsafe.Pointer(in.Class)) out.Name = in.Name out.PodTemplate = (*acme.ACMEChallengeSolverHTTP01IngressPodTemplate)(unsafe.Pointer(in.PodTemplate)) @@ -706,6 +707,7 @@ func Convert_v1alpha2_ACMEChallengeSolverHTTP01Ingress_To_acme_ACMEChallengeSolv func autoConvert_acme_ACMEChallengeSolverHTTP01Ingress_To_v1alpha2_ACMEChallengeSolverHTTP01Ingress(in *acme.ACMEChallengeSolverHTTP01Ingress, out *ACMEChallengeSolverHTTP01Ingress, s conversion.Scope) error { out.ServiceType = v1.ServiceType(in.ServiceType) + out.IngressClassName = (*string)(unsafe.Pointer(in.IngressClassName)) out.Class = (*string)(unsafe.Pointer(in.Class)) out.Name = in.Name out.PodTemplate = (*ACMEChallengeSolverHTTP01IngressPodTemplate)(unsafe.Pointer(in.PodTemplate)) diff --git a/internal/apis/acme/v1alpha2/zz_generated.deepcopy.go b/internal/apis/acme/v1alpha2/zz_generated.deepcopy.go index 3b1ccf0c7..2efff739f 100644 --- a/internal/apis/acme/v1alpha2/zz_generated.deepcopy.go +++ b/internal/apis/acme/v1alpha2/zz_generated.deepcopy.go @@ -223,6 +223,11 @@ func (in *ACMEChallengeSolverHTTP01GatewayHTTPRoute) DeepCopy() *ACMEChallengeSo // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ACMEChallengeSolverHTTP01Ingress) DeepCopyInto(out *ACMEChallengeSolverHTTP01Ingress) { *out = *in + if in.IngressClassName != nil { + in, out := &in.IngressClassName, &out.IngressClassName + *out = new(string) + **out = **in + } if in.Class != nil { in, out := &in.Class, &out.Class *out = new(string) diff --git a/internal/apis/acme/v1alpha3/types_issuer.go b/internal/apis/acme/v1alpha3/types_issuer.go index 92ead697d..73f13748e 100644 --- a/internal/apis/acme/v1alpha3/types_issuer.go +++ b/internal/apis/acme/v1alpha3/types_issuer.go @@ -232,9 +232,17 @@ type ACMEChallengeSolverHTTP01Ingress struct { // +optional ServiceType corev1.ServiceType `json:"serviceType,omitempty"` - // The ingress class to use when creating Ingress resources to solve ACME - // challenges that use this challenge solver. - // Only one of 'class' or 'name' may be specified. + // This field configures the field `ingressClassName` on the created Ingress + // resources used to solve ACME challenges that use this challenge solver. + // This is the recommended way of configuring the ingress class. Only one of + // `class`, `name` or `ingressClassName` may be specified. + // +optional + IngressClassName *string `json:"ingressClassName,omitempty"` + + // This field configures the annotation `kubernetes.io/ingress.class` when + // creating Ingress resources to solve ACME challenges that use this + // challenge solver. Only one of `class`, `name` or `ingressClassName` may + // be specified. // +optional Class *string `json:"class,omitempty"` diff --git a/internal/apis/acme/v1alpha3/zz_generated.conversion.go b/internal/apis/acme/v1alpha3/zz_generated.conversion.go index 315c0412f..ef41efb6f 100644 --- a/internal/apis/acme/v1alpha3/zz_generated.conversion.go +++ b/internal/apis/acme/v1alpha3/zz_generated.conversion.go @@ -692,6 +692,7 @@ func Convert_acme_ACMEChallengeSolverHTTP01GatewayHTTPRoute_To_v1alpha3_ACMEChal func autoConvert_v1alpha3_ACMEChallengeSolverHTTP01Ingress_To_acme_ACMEChallengeSolverHTTP01Ingress(in *ACMEChallengeSolverHTTP01Ingress, out *acme.ACMEChallengeSolverHTTP01Ingress, s conversion.Scope) error { out.ServiceType = v1.ServiceType(in.ServiceType) + out.IngressClassName = (*string)(unsafe.Pointer(in.IngressClassName)) out.Class = (*string)(unsafe.Pointer(in.Class)) out.Name = in.Name out.PodTemplate = (*acme.ACMEChallengeSolverHTTP01IngressPodTemplate)(unsafe.Pointer(in.PodTemplate)) @@ -706,6 +707,7 @@ func Convert_v1alpha3_ACMEChallengeSolverHTTP01Ingress_To_acme_ACMEChallengeSolv func autoConvert_acme_ACMEChallengeSolverHTTP01Ingress_To_v1alpha3_ACMEChallengeSolverHTTP01Ingress(in *acme.ACMEChallengeSolverHTTP01Ingress, out *ACMEChallengeSolverHTTP01Ingress, s conversion.Scope) error { out.ServiceType = v1.ServiceType(in.ServiceType) + out.IngressClassName = (*string)(unsafe.Pointer(in.IngressClassName)) out.Class = (*string)(unsafe.Pointer(in.Class)) out.Name = in.Name out.PodTemplate = (*ACMEChallengeSolverHTTP01IngressPodTemplate)(unsafe.Pointer(in.PodTemplate)) diff --git a/internal/apis/acme/v1alpha3/zz_generated.deepcopy.go b/internal/apis/acme/v1alpha3/zz_generated.deepcopy.go index d09266b11..37c92df29 100644 --- a/internal/apis/acme/v1alpha3/zz_generated.deepcopy.go +++ b/internal/apis/acme/v1alpha3/zz_generated.deepcopy.go @@ -223,6 +223,11 @@ func (in *ACMEChallengeSolverHTTP01GatewayHTTPRoute) DeepCopy() *ACMEChallengeSo // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ACMEChallengeSolverHTTP01Ingress) DeepCopyInto(out *ACMEChallengeSolverHTTP01Ingress) { *out = *in + if in.IngressClassName != nil { + in, out := &in.IngressClassName, &out.IngressClassName + *out = new(string) + **out = **in + } if in.Class != nil { in, out := &in.Class, &out.Class *out = new(string) diff --git a/internal/apis/acme/v1beta1/types_issuer.go b/internal/apis/acme/v1beta1/types_issuer.go index ccf3fb694..f9463b406 100644 --- a/internal/apis/acme/v1beta1/types_issuer.go +++ b/internal/apis/acme/v1beta1/types_issuer.go @@ -231,9 +231,17 @@ type ACMEChallengeSolverHTTP01Ingress struct { // +optional ServiceType corev1.ServiceType `json:"serviceType,omitempty"` - // The ingress class to use when creating Ingress resources to solve ACME - // challenges that use this challenge solver. - // Only one of 'class' or 'name' may be specified. + // This field configures the field `ingressClassName` on the created Ingress + // resources used to solve ACME challenges that use this challenge solver. + // This is the recommended way of configuring the ingress class. Only one of + // `class`, `name` or `ingressClassName` may be specified. + // +optional + IngressClassName *string `json:"ingressClassName,omitempty"` + + // This field configures the annotation `kubernetes.io/ingress.class` when + // creating Ingress resources to solve ACME challenges that use this + // challenge solver. Only one of `class`, `name` or `ingressClassName` may + // be specified. // +optional Class *string `json:"class,omitempty"` diff --git a/internal/apis/acme/v1beta1/zz_generated.conversion.go b/internal/apis/acme/v1beta1/zz_generated.conversion.go index 8003d2d18..c9b4237ac 100644 --- a/internal/apis/acme/v1beta1/zz_generated.conversion.go +++ b/internal/apis/acme/v1beta1/zz_generated.conversion.go @@ -692,6 +692,7 @@ func Convert_acme_ACMEChallengeSolverHTTP01GatewayHTTPRoute_To_v1beta1_ACMEChall func autoConvert_v1beta1_ACMEChallengeSolverHTTP01Ingress_To_acme_ACMEChallengeSolverHTTP01Ingress(in *ACMEChallengeSolverHTTP01Ingress, out *acme.ACMEChallengeSolverHTTP01Ingress, s conversion.Scope) error { out.ServiceType = v1.ServiceType(in.ServiceType) + out.IngressClassName = (*string)(unsafe.Pointer(in.IngressClassName)) out.Class = (*string)(unsafe.Pointer(in.Class)) out.Name = in.Name out.PodTemplate = (*acme.ACMEChallengeSolverHTTP01IngressPodTemplate)(unsafe.Pointer(in.PodTemplate)) @@ -706,6 +707,7 @@ func Convert_v1beta1_ACMEChallengeSolverHTTP01Ingress_To_acme_ACMEChallengeSolve func autoConvert_acme_ACMEChallengeSolverHTTP01Ingress_To_v1beta1_ACMEChallengeSolverHTTP01Ingress(in *acme.ACMEChallengeSolverHTTP01Ingress, out *ACMEChallengeSolverHTTP01Ingress, s conversion.Scope) error { out.ServiceType = v1.ServiceType(in.ServiceType) + out.IngressClassName = (*string)(unsafe.Pointer(in.IngressClassName)) out.Class = (*string)(unsafe.Pointer(in.Class)) out.Name = in.Name out.PodTemplate = (*ACMEChallengeSolverHTTP01IngressPodTemplate)(unsafe.Pointer(in.PodTemplate)) diff --git a/internal/apis/acme/v1beta1/zz_generated.deepcopy.go b/internal/apis/acme/v1beta1/zz_generated.deepcopy.go index 2fc6ef38f..f4ab40930 100644 --- a/internal/apis/acme/v1beta1/zz_generated.deepcopy.go +++ b/internal/apis/acme/v1beta1/zz_generated.deepcopy.go @@ -223,6 +223,11 @@ func (in *ACMEChallengeSolverHTTP01GatewayHTTPRoute) DeepCopy() *ACMEChallengeSo // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ACMEChallengeSolverHTTP01Ingress) DeepCopyInto(out *ACMEChallengeSolverHTTP01Ingress) { *out = *in + if in.IngressClassName != nil { + in, out := &in.IngressClassName, &out.IngressClassName + *out = new(string) + **out = **in + } if in.Class != nil { in, out := &in.Class, &out.Class *out = new(string) diff --git a/internal/apis/acme/zz_generated.deepcopy.go b/internal/apis/acme/zz_generated.deepcopy.go index c6e3f2edf..23c3b8aec 100644 --- a/internal/apis/acme/zz_generated.deepcopy.go +++ b/internal/apis/acme/zz_generated.deepcopy.go @@ -223,6 +223,11 @@ func (in *ACMEChallengeSolverHTTP01GatewayHTTPRoute) DeepCopy() *ACMEChallengeSo // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ACMEChallengeSolverHTTP01Ingress) DeepCopyInto(out *ACMEChallengeSolverHTTP01Ingress) { *out = *in + if in.IngressClassName != nil { + in, out := &in.IngressClassName, &out.IngressClassName + *out = new(string) + **out = **in + } if in.Class != nil { in, out := &in.Class, &out.Class *out = new(string) diff --git a/internal/apis/certmanager/validation/issuer.go b/internal/apis/certmanager/validation/issuer.go index 953a4a729..5f34983a2 100644 --- a/internal/apis/certmanager/validation/issuer.go +++ b/internal/apis/certmanager/validation/issuer.go @@ -24,6 +24,7 @@ import ( admissionv1 "k8s.io/api/admission/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" cmacme "github.com/cert-manager/cert-manager/internal/apis/acme" @@ -195,9 +196,21 @@ func ValidateACMEIssuerChallengeSolverHTTP01Config(http01 *cmacme.ACMEChallengeS func ValidateACMEIssuerChallengeSolverHTTP01IngressConfig(ingress *cmacme.ACMEChallengeSolverHTTP01Ingress, fldPath *field.Path) field.ErrorList { el := field.ErrorList{} - if ingress.Class != nil && len(ingress.Name) > 0 { - el = append(el, field.Forbidden(fldPath, "only one of 'name' or 'class' should be specified")) + if ingress.Class != nil && ingress.IngressClassName != nil && len(ingress.Name) > 0 { + el = append(el, field.Forbidden(fldPath, "only one of 'ingressClassName', 'name' or 'class' should be specified")) } + + // Since "class" used to be a free string, let's have a stricter validation + // for "ingressClassName" since it is expected to be a valid resource name. + // A notable example is "azure/application-gateway" that is a valid value + // for "class" but not for "ingressClassName". + if ingress.IngressClassName != nil { + errs := validation.IsDNS1123Subdomain(*ingress.IngressClassName) + if len(errs) > 0 { + el = append(el, field.Invalid(fldPath.Child("ingressClassName"), *ingress.IngressClassName, "must be a valid IngressClass name: "+strings.Join(errs, ", "))) + } + } + switch ingress.ServiceType { case "", corev1.ServiceTypeClusterIP, corev1.ServiceTypeNodePort: default: diff --git a/internal/apis/certmanager/validation/issuer_test.go b/internal/apis/certmanager/validation/issuer_test.go index a86777a96..0a433cbe1 100644 --- a/internal/apis/certmanager/validation/issuer_test.go +++ b/internal/apis/certmanager/validation/issuer_test.go @@ -764,6 +764,11 @@ func TestValidateACMEIssuerHTTP01Config(t *testing.T) { Ingress: &cmacme.ACMEChallengeSolverHTTP01Ingress{Class: strPtr("abc")}, }, }, + "ingressClassName field specified": { + cfg: &cmacme.ACMEChallengeSolverHTTP01{ + Ingress: &cmacme.ACMEChallengeSolverHTTP01Ingress{IngressClassName: strPtr("abc")}, + }, + }, "neither field specified": { cfg: &cmacme.ACMEChallengeSolverHTTP01{ Ingress: &cmacme.ACMEChallengeSolverHTTP01Ingress{}, @@ -775,15 +780,26 @@ func TestValidateACMEIssuerHTTP01Config(t *testing.T) { field.Required(fldPath, "no HTTP01 solver type configured"), }, }, - "both fields specified": { + "all three fields specified": { cfg: &cmacme.ACMEChallengeSolverHTTP01{ Ingress: &cmacme.ACMEChallengeSolverHTTP01Ingress{ - Name: "abc", - Class: strPtr("abc"), + Name: "abc", + Class: strPtr("abc"), + IngressClassName: strPtr("abc"), }, }, errs: []*field.Error{ - field.Forbidden(fldPath.Child("ingress"), "only one of 'name' or 'class' should be specified"), + field.Forbidden(fldPath.Child("ingress"), "only one of 'ingressClassName', 'name' or 'class' should be specified"), + }, + }, + "ingressClassName is invalid": { + cfg: &cmacme.ACMEChallengeSolverHTTP01{ + Ingress: &cmacme.ACMEChallengeSolverHTTP01Ingress{ + IngressClassName: strPtr("azure/application-gateway"), + }, + }, + errs: []*field.Error{ + field.Invalid(fldPath.Child("ingress", "ingressClassName"), "azure/application-gateway", `must be a valid IngressClass name: a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')`), }, }, "acme issuer with valid http01 service config serviceType ClusterIP": { diff --git a/pkg/apis/acme/v1/types_issuer.go b/pkg/apis/acme/v1/types_issuer.go index ba38aa33e..0f75b40d4 100644 --- a/pkg/apis/acme/v1/types_issuer.go +++ b/pkg/apis/acme/v1/types_issuer.go @@ -233,9 +233,17 @@ type ACMEChallengeSolverHTTP01Ingress struct { // +optional ServiceType corev1.ServiceType `json:"serviceType,omitempty"` - // The ingress class to use when creating Ingress resources to solve ACME - // challenges that use this challenge solver. - // Only one of 'class' or 'name' may be specified. + // This field configures the field `ingressClassName` on the created Ingress + // resources used to solve ACME challenges that use this challenge solver. + // This is the recommended way of configuring the ingress class. Only one of + // `class`, `name` or `ingressClassName` may be specified. + // +optional + IngressClassName *string `json:"ingressClassName,omitempty"` + + // This field configures the annotation `kubernetes.io/ingress.class` when + // creating Ingress resources to solve ACME challenges that use this + // challenge solver. Only one of `class`, `name` or `ingressClassName` may + // be specified. // +optional Class *string `json:"class,omitempty"` diff --git a/pkg/apis/acme/v1/zz_generated.deepcopy.go b/pkg/apis/acme/v1/zz_generated.deepcopy.go index 3b103ee59..b5472216c 100644 --- a/pkg/apis/acme/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acme/v1/zz_generated.deepcopy.go @@ -223,6 +223,11 @@ func (in *ACMEChallengeSolverHTTP01GatewayHTTPRoute) DeepCopy() *ACMEChallengeSo // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ACMEChallengeSolverHTTP01Ingress) DeepCopyInto(out *ACMEChallengeSolverHTTP01Ingress) { *out = *in + if in.IngressClassName != nil { + in, out := &in.IngressClassName, &out.IngressClassName + *out = new(string) + **out = **in + } if in.Class != nil { in, out := &in.Class, &out.Class *out = new(string) diff --git a/pkg/controller/context.go b/pkg/controller/context.go index ea717ef9d..f3010faf6 100644 --- a/pkg/controller/context.go +++ b/pkg/controller/context.go @@ -177,9 +177,6 @@ type ACMEOptions struct { // for ACME HTTP01 validations. HTTP01SolverNameservers []string - // HTTP01SolverUseIngressClass indicates whether to use the ingressClassName of IngressV1 resources. - HTTP01SolverUseIngressClass bool - // DNS01CheckAuthoritative is a flag for controlling if auth nss are used // for checking propagation of an RR. This is the ideal scenario DNS01CheckAuthoritative bool diff --git a/pkg/issuer/acme/http/ingress.go b/pkg/issuer/acme/http/ingress.go index 4a23135d7..36a165a31 100644 --- a/pkg/issuer/acme/http/ingress.go +++ b/pkg/issuer/acme/http/ingress.go @@ -127,7 +127,7 @@ func ingressServiceName(ing *networkingv1.Ingress) string { // createIngress will create a challenge solving ingress for the given certificate, // domain, token and key. func (s *Solver) createIngress(ctx context.Context, ch *cmacme.Challenge, svcName string) (*networkingv1.Ingress, error) { - ing, err := buildIngressResource(ch, svcName, s.HTTP01SolverUseIngressClass) + ing, err := buildIngressResource(ch, svcName) if err != nil { return nil, err } @@ -141,7 +141,7 @@ func (s *Solver) createIngress(ctx context.Context, ch *cmacme.Challenge, svcNam return s.Client.NetworkingV1().Ingresses(ch.Namespace).Create(ctx, ing, metav1.CreateOptions{}) } -func buildIngressResource(ch *cmacme.Challenge, svcName string, useIngressClassName bool) (*networkingv1.Ingress, error) { +func buildIngressResource(ch *cmacme.Challenge, svcName string) (*networkingv1.Ingress, error) { http01IngressCfg, err := http01IngressCfgForChallenge(ch) if err != nil { return nil, err @@ -154,21 +154,18 @@ func buildIngressResource(ch *cmacme.Challenge, svcName string, useIngressClassN // TODO: Figure out how to remove this without breaking users who depend on it. ingAnnotations["nginx.ingress.kubernetes.io/whitelist-source-range"] = "0.0.0.0/0,::/0" - // Use the annotation based on whether the user wants to - // https://github.com/cert-manager/cert-manager/issues/4821 - var ingressClassName *string = nil - if useIngressClassName { - // https://github.com/cert-manager/cert-manager/issues/4537 - ingressClassName = http01IngressCfg.Class - } else { - // Use the Ingress Class annotation defined in networkingv1beta1 even though our Ingress objects - // are networkingv1, for maximum compatibility with all Ingress controllers. - // if the `kubernetes.io/ingress.class` annotation is present, it takes precedence over the - // `spec.IngressClassName` field. - // See discussion in https://github.com/cert-manager/cert-manager/issues/4537. - if http01IngressCfg.Class != nil { - ingAnnotations[annotationIngressClass] = *http01IngressCfg.Class - } + // The Kubernetes API won't allow both having the annotation and the field + // set. + if http01IngressCfg.Class != nil && http01IngressCfg.IngressClassName != nil { + return nil, fmt.Errorf("the fields ingressClassName and class cannot be set at the same time") + } + + var ingressClassName *string + if http01IngressCfg.Class != nil { + ingAnnotations[annotationIngressClass] = *http01IngressCfg.Class + } + if http01IngressCfg.IngressClassName != nil { + ingressClassName = http01IngressCfg.IngressClassName } ingPathToAdd := ingressPath(ch.Spec.Token, svcName) diff --git a/pkg/issuer/acme/http/ingress_test.go b/pkg/issuer/acme/http/ingress_test.go index 39d091633..e670ef6fc 100644 --- a/pkg/issuer/acme/http/ingress_test.go +++ b/pkg/issuer/acme/http/ingress_test.go @@ -22,6 +22,8 @@ import ( "reflect" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" networkingv1 "k8s.io/api/networking/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -473,6 +475,28 @@ func TestEnsureIngress(t *testing.T) { } }, }, + "class field is passed to ingress as the annotation kubernetes.io/ingress.class": { + Challenge: &cmacme.Challenge{Spec: cmacme.ChallengeSpec{Solver: cmacme.ACMEChallengeSolver{HTTP01: &cmacme.ACMEChallengeSolverHTTP01{ + Ingress: &cmacme.ACMEChallengeSolverHTTP01Ingress{ + Class: strPtr("nginx"), + }}}}, + }, + CheckFn: checkOneIngress(func(t *testing.T, ingress *networkingv1.Ingress) { + assert.Equal(t, "nginx", ingress.Annotations["kubernetes.io/ingress.class"]) + assert.Empty(t, ingress.Spec.IngressClassName) + }), + }, + "ingressClassName field is passed to the ingress": { + Challenge: &cmacme.Challenge{Spec: cmacme.ChallengeSpec{Solver: cmacme.ACMEChallengeSolver{HTTP01: &cmacme.ACMEChallengeSolverHTTP01{ + Ingress: &cmacme.ACMEChallengeSolverHTTP01Ingress{ + IngressClassName: strPtr("nginx"), + }}}}, + }, + CheckFn: checkOneIngress(func(t *testing.T, ingress *networkingv1.Ingress) { + assert.Empty(t, ingress.Annotations["kubernetes.io/ingress.class"]) + assert.Equal(t, strPtr("nginx"), ingress.Spec.IngressClassName) + }), + }, } for name, test := range tests { t.Run(name, func(t *testing.T) { @@ -489,6 +513,19 @@ func TestEnsureIngress(t *testing.T) { } } +func Test_createIngress(t *testing.T) { + +} + +func checkOneIngress(check func(*testing.T, *networkingv1.Ingress)) func(*testing.T, *solverFixture, ...interface{}) { + return func(t *testing.T, s *solverFixture, _ ...interface{}) { + ingresses, err := s.Solver.ingressLister.List(labels.NewSelector()) + assert.NoError(t, err) + require.Len(t, ingresses, 1) + check(t, ingresses[0]) + } +} + func TestMergeIngressObjectMetaWithIngressResourceTemplate(t *testing.T) { const createdIngressKey = "createdIngressKey" tests := map[string]solverFixture{ @@ -519,7 +556,7 @@ func TestMergeIngressObjectMetaWithIngressResourceTemplate(t *testing.T) { }, }, PreFn: func(t *testing.T, s *solverFixture) { - expectedIngress, err := buildIngressResource(s.Challenge, "fakeservice", false) + expectedIngress, err := buildIngressResource(s.Challenge, "fakeservice") if err != nil { t.Errorf("error preparing test: %v", err) } @@ -597,7 +634,7 @@ func TestOverrideNginxIngressWhitelistAnnotation(t *testing.T) { }, }, PreFn: func(t *testing.T, s *solverFixture) { - expectedIngress, err := buildIngressResource(s.Challenge, "fakeservice", false) + expectedIngress, err := buildIngressResource(s.Challenge, "fakeservice") if err != nil { t.Errorf("error preparing test: %v", err) }