diff --git a/pkg/internal/ingress/BUILD.bazel b/pkg/internal/ingress/BUILD.bazel index 3df865a22..cc61fb136 100644 --- a/pkg/internal/ingress/BUILD.bazel +++ b/pkg/internal/ingress/BUILD.bazel @@ -29,7 +29,10 @@ go_library( go_test( name = "go_default_test", - srcs = ["ingress_test.go"], + srcs = [ + "convert_test.go", + "ingress_test.go", + ], embed = [":go_default_library"], deps = [ "//pkg/controller:go_default_library", @@ -40,6 +43,7 @@ go_test( "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library", "@io_k8s_apimachinery//pkg/labels:go_default_library", "@io_k8s_apimachinery//pkg/runtime/schema:go_default_library", + "@io_k8s_apimachinery//pkg/util/intstr:go_default_library", "@io_k8s_client_go//informers:go_default_library", "@io_k8s_client_go//kubernetes/fake:go_default_library", "@io_k8s_utils//pointer:go_default_library", diff --git a/pkg/internal/ingress/convert.go b/pkg/internal/ingress/convert.go index 0f9751ea5..aef164390 100644 --- a/pkg/internal/ingress/convert.go +++ b/pkg/internal/ingress/convert.go @@ -45,8 +45,30 @@ See the License for the specific language governing permissions and limitations under the License. */ +// Convert_networking_Ingress_To_v1beta1_Ingress uses unsafe pointer manipulation to manipulate a +// *networkingv1beta1.Ingress into pointing at the same underlying data as the input *networkingv1.Ingress. +// Both the `in` and `out` Object's data will be manipulated by this function. +// +// Recommended usage: +// // as in and out will point to the same data, make sure any manipulation doesn't affect the original Ingress +// in := myIngress.DeepCopy() +// out := new(networkingv1beta1.Ingress) +// err := Convert_networking_Ingress_To_v1beta1_Ingress(in, out, nil) func Convert_networking_Ingress_To_v1beta1_Ingress(in *networkingv1.Ingress, out *networkingv1beta1.Ingress, s conversion.Scope) error { - return autoConvert_networking_Ingress_To_v1beta1_Ingress(in, out, s) + err := autoConvert_networking_Ingress_To_v1beta1_Ingress(in, out, s) + if err != nil { + return err + } + // v1beta1 Ingresses should not have IngressClassName set but instead use the deprecated annotation. + // Move the ingress class to the annotations and then zero the IngressClassName field + if out.Spec.IngressClassName != nil { + if out.Annotations == nil { + out.Annotations = make(map[string]string) + } + out.Annotations["kubernetes.io/ingress.class"] = *out.Spec.IngressClassName + out.Spec.IngressClassName = nil + } + return nil } func autoConvert_networking_Ingress_To_v1beta1_Ingress(in *networkingv1.Ingress, out *networkingv1beta1.Ingress, s conversion.Scope) error { @@ -193,8 +215,35 @@ func autoConvert_networking_IngressBackend_To_v1beta1_IngressBackend(in *network return nil } +// Convert_v1beta1_Ingress_To_networking_Ingress uses unsafe pointer manipulation to manipulate a +// *networkingv1.Ingress into pointing at the same underlying data as the input *networkingv1beta1.Ingress. +// Both the `in` and `out` Object's data will be manipulated by this function. +// +// Recommended usage: +// // as in and out will point to the same data, make sure any manipulation doesn't affect the original Ingress +// in := myIngress.DeepCopy() +// out := new(networkingv1.Ingress) +// err := Convert_v1beta1_Ingress_To_networking_Ingress(in, out, nil) func Convert_v1beta1_Ingress_To_networking_Ingress(in *networkingv1beta1.Ingress, out *networkingv1.Ingress, s conversion.Scope) error { - return autoConvert_v1beta1_Ingress_To_networking_Ingress(in, out, s) + err := autoConvert_v1beta1_Ingress_To_networking_Ingress(in, out, s) + if err != nil { + return err + } + // v1beta1 Ingresses should not have IngressClassName set but instead use the deprecated annotation. + // Move the ingress class from the annotations to the Spec + if in.Annotations == nil { + return nil + } + if ingressClass, found := in.Annotations["kubernetes.io/ingress.class"]; found { + out.Spec.IngressClassName = &ingressClass + // HERE BE DRAGONS: + // in.Annotations and out.Annotations point to the same map. + // This mutates in as well as out, so make sure in is not an object in + // client-go's cache, for example by only passing DeepCopy()d objects + // to Convert_v1beta1_Ingress_To_networking_Ingress + delete(out.Annotations, "kubernetes.io/ingress.class") + } + return nil } func autoConvert_v1beta1_Ingress_To_networking_Ingress(in *networkingv1beta1.Ingress, out *networkingv1.Ingress, s conversion.Scope) error { diff --git a/pkg/internal/ingress/convert_test.go b/pkg/internal/ingress/convert_test.go new file mode 100644 index 000000000..fb09d391e --- /dev/null +++ b/pkg/internal/ingress/convert_test.go @@ -0,0 +1,157 @@ +/* +Copyright 2021 The cert-manager Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package ingress + +import ( + "testing" + + "github.com/stretchr/testify/assert" + networkingv1 "k8s.io/api/networking/v1" + networkingv1beta1 "k8s.io/api/networking/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/pointer" +) + +var v1TestIngress = &networkingv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-networkingv1-ingress", + Namespace: "test-networkingv1-namespace", + Annotations: map[string]string{ + "test.key": "test.value", + }, + Labels: map[string]string{ + "labelkey": "labelvalue", + }, + }, + Spec: networkingv1.IngressSpec{ + IngressClassName: pointer.String("bogus-ingress-class"), + DefaultBackend: &networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "default-backend-svc", + Port: networkingv1.ServiceBackendPort{ + Number: 1234, + }, + }, + }, + TLS: []networkingv1.IngressTLS{ + { + Hosts: []string{"aaa.", "bbb.", "ccc.ddd"}, + SecretName: "test-secret-1", + }, + { + Hosts: []string{"eee"}, + SecretName: "test-secret-2", + }, + }, + Rules: []networkingv1.IngressRule{ + { + Host: "aaa", + IngressRuleValue: networkingv1.IngressRuleValue{ + HTTP: &networkingv1.HTTPIngressRuleValue{ + Paths: []networkingv1.HTTPIngressPath{ + { + Path: "/.well-known/acme-challenge", + PathType: func() *networkingv1.PathType { p := networkingv1.PathTypeImplementationSpecific; return &p }(), + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: "test-solver-backend", + Port: networkingv1.ServiceBackendPort{Number: 80}, + }, + }, + }, + }, + }, + }, + }, + }, + }, +} + +var v1beta1TestIngress = &networkingv1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-networkingv1-ingress", + Namespace: "test-networkingv1-namespace", + Annotations: map[string]string{ + "test.key": "test.value", + "kubernetes.io/ingress.class": "bogus-ingress-class", + }, + Labels: map[string]string{ + "labelkey": "labelvalue", + }, + }, + Spec: networkingv1beta1.IngressSpec{ + Backend: &networkingv1beta1.IngressBackend{ + ServiceName: "default-backend-svc", + ServicePort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 1234, + }, + }, + Rules: []networkingv1beta1.IngressRule{ + { + Host: "aaa", + IngressRuleValue: networkingv1beta1.IngressRuleValue{ + HTTP: &networkingv1beta1.HTTPIngressRuleValue{ + Paths: []networkingv1beta1.HTTPIngressPath{ + { + Path: "/.well-known/acme-challenge", + PathType: func() *networkingv1beta1.PathType { p := networkingv1beta1.PathTypeImplementationSpecific; return &p }(), + Backend: networkingv1beta1.IngressBackend{ + ServiceName: "test-solver-backend", + ServicePort: intstr.IntOrString{ + Type: intstr.Int, + IntVal: 80, + }, + }, + }, + }, + }, + }, + }, + }, + TLS: []networkingv1beta1.IngressTLS{ + { + Hosts: []string{"aaa.", "bbb.", "ccc.ddd"}, + SecretName: "test-secret-1", + }, + { + Hosts: []string{"eee"}, + SecretName: "test-secret-2", + }, + }, + }, + Status: networkingv1beta1.IngressStatus{}, +} + +func TestConvert_networking_Ingress_To_v1beta1_Ingress(t *testing.T) { + in := v1TestIngress.DeepCopy() + out := &networkingv1beta1.Ingress{} + err := Convert_networking_Ingress_To_v1beta1_Ingress(in, out, nil) + assert.NoError(t, err, "converting networking v1 to networking v1beta1 Ingress should not fail") + expected := v1beta1TestIngress.DeepCopy() + assert.Equal(t, expected, out, "Conversion from networking v1 to networking v1beta1 Ingress was not as expected") +} + +func TestConvert_v1beta1_Ingress_To_networking_Ingress(t *testing.T) { + in := v1beta1TestIngress.DeepCopy() + out := &networkingv1.Ingress{} + err := Convert_v1beta1_Ingress_To_networking_Ingress(in, out, nil) + assert.NoError(t, err, "converting networking v1beta1 to networking v1 Ingress should not fail") + expected := v1TestIngress.DeepCopy() + assert.Equal(t, expected, out, "Conversion from networking v1beta1 to networking v1 Ingress was not as expected") +} diff --git a/pkg/internal/ingress/ingress.go b/pkg/internal/ingress/ingress.go index 65977815e..6b8d01542 100644 --- a/pkg/internal/ingress/ingress.go +++ b/pkg/internal/ingress/ingress.go @@ -14,7 +14,13 @@ See the License for the specific language governing permissions and limitations under the License. */ -// Package ingress lets us use an internal type for supporting multiple kinds of ingresses +// Package ingress lets us use an internal type for supporting multiple kinds of ingresses. +// +// This package's sole reason for existence is for compatibility with Kubernetes API servers +// below v1.18. However, our client-go library is already beyond the supported version skew +// (https://kubernetes.io/releases/version-skew-policy/) so it is not guaranteed to continue +// working in the future, and will definitely be removed once cert-manager no longer supports +// Kubernetes 1.17. package ingress import ( diff --git a/pkg/internal/ingress/ingress_test.go b/pkg/internal/ingress/ingress_test.go index ec931aa32..8fb1fc2d0 100644 --- a/pkg/internal/ingress/ingress_test.go +++ b/pkg/internal/ingress/ingress_test.go @@ -36,6 +36,8 @@ import ( discoveryfake "github.com/jetstack/cert-manager/test/unit/discovery" ) +// Important: these tests cannot run in parallel as the cache holds internal state at the package level. + func TestFunctionalityAgainstV1(t *testing.T) { // wipe known versions cache cacheLock.Lock() diff --git a/pkg/issuer/acme/http/ingress.go b/pkg/issuer/acme/http/ingress.go index 803c90d87..c94ddc384 100644 --- a/pkg/issuer/acme/http/ingress.go +++ b/pkg/issuer/acme/http/ingress.go @@ -29,7 +29,6 @@ import ( utilerrors "k8s.io/apimachinery/pkg/util/errors" cmacme "github.com/jetstack/cert-manager/pkg/apis/acme/v1" - cmapi "github.com/jetstack/cert-manager/pkg/apis/certmanager/v1" "github.com/jetstack/cert-manager/pkg/issuer/acme/http/solver" logf "github.com/jetstack/cert-manager/pkg/logs" ) @@ -73,7 +72,7 @@ func (s *Solver) getIngressesForChallenge(ctx context.Context, ch *cmacme.Challe // that the ingress has an appropriate challenge path configured func (s *Solver) ensureIngress(ctx context.Context, ch *cmacme.Challenge, svcName string) (ing *networkingv1.Ingress, err error) { log := logf.FromContext(ctx).WithName("ensureIngress") - httpDomainCfg, err := httpDomainCfgForChallenge(ch) + httpDomainCfg, err := http01IngressCfgForChallenge(ch) if err != nil { return nil, err } @@ -134,13 +133,13 @@ func (s *Solver) createIngress(ctx context.Context, ch *cmacme.Challenge, svcNam } func buildIngressResource(ch *cmacme.Challenge, svcName string) (*networkingv1.Ingress, error) { - httpDomainCfg, err := httpDomainCfgForChallenge(ch) + http01IngressCfg, err := http01IngressCfgForChallenge(ch) if err != nil { return nil, err } var ingClass *string - if httpDomainCfg.Class != nil { - ingClass = httpDomainCfg.Class + if http01IngressCfg.Class != nil { + ingClass = http01IngressCfg.Class } podLabels := podLabels(ch) @@ -150,10 +149,6 @@ func buildIngressResource(ch *cmacme.Challenge, svcName string) (*networkingv1.I // 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" - if ingClass != nil { - ingAnnotations[cmapi.IngressClassAnnotationKey] = *ingClass - } - ingPathToAdd := ingressPath(ch.Spec.Token, svcName) httpHost := ch.Spec.DNSName @@ -170,6 +165,7 @@ func buildIngressResource(ch *cmacme.Challenge, svcName string) (*networkingv1.I OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(ch, challengeGvk)}, }, Spec: networkingv1.IngressSpec{ + IngressClassName: ingClass, Rules: []networkingv1.IngressRule{ { Host: httpHost, @@ -212,7 +208,7 @@ func (s *Solver) mergeIngressObjectMetaWithIngressResourceTemplate(ingress *netw } func (s *Solver) addChallengePathToIngress(ctx context.Context, ch *cmacme.Challenge, svcName string) (*networkingv1.Ingress, error) { - httpDomainCfg, err := httpDomainCfgForChallenge(ch) + httpDomainCfg, err := http01IngressCfgForChallenge(ch) if err != nil { return nil, err } @@ -270,7 +266,7 @@ func (s *Solver) cleanupIngresses(ctx context.Context, ch *cmacme.Challenge) err return nil } - httpDomainCfg, err := httpDomainCfgForChallenge(ch) + httpDomainCfg, err := http01IngressCfgForChallenge(ch) if err != nil { return err } diff --git a/pkg/issuer/acme/http/ingress_test.go b/pkg/issuer/acme/http/ingress_test.go index 7453ccb05..e601fee5b 100644 --- a/pkg/issuer/acme/http/ingress_test.go +++ b/pkg/issuer/acme/http/ingress_test.go @@ -530,7 +530,6 @@ func TestMergeIngressObjectMetaWithIngressResourceTemplate(t *testing.T) { cmacme.SolverIdentificationLabelKey: "true", } expectedIngress.Annotations = map[string]string{ - "kubernetes.io/ingress.class": "nginx", "nginx.ingress.kubernetes.io/whitelist-source-range": "0.0.0.0/0,::/0", "nginx.org/mergeable-ingress-type": "minion", "traefik.ingress.kubernetes.io/frontend-entry-points": "http",