nil pointer: the Gateway API is full of pointers

Signed-off-by: Maël Valais <mael@vls.dev>
This commit is contained in:
Maël Valais 2021-08-03 15:05:43 +02:00
parent 94d854c525
commit 30af205777
2 changed files with 169 additions and 26 deletions

View File

@ -136,17 +136,18 @@ func SyncFnFor(
rec.Eventf(ingLikeObj, corev1.EventTypeNormal, reasonUpdateCertificate, "Successfully updated Certificate %q", crt.Name)
}
unrequiredCrts, err := findUnrequiredCertificates(cmLister, ingLike)
certs, err := cmLister.Certificates(ingLike.GetNamespace()).List(labels.Everything())
if err != nil {
return err
}
unrequiredCertNames := findCertificatesToBeRemoved(certs, ingLike)
for _, crt := range unrequiredCrts {
err = cmClient.CertmanagerV1().Certificates(crt.Namespace).Delete(ctx, crt.Name, metav1.DeleteOptions{})
for _, certName := range unrequiredCertNames {
err = cmClient.CertmanagerV1().Certificates(ingLike.GetNamespace()).Delete(ctx, certName, metav1.DeleteOptions{})
if err != nil {
return err
}
rec.Eventf(ingLikeObj, corev1.EventTypeNormal, reasonDeleteCertificate, "Successfully deleted unrequired Certificate %q", crt.Name)
rec.Eventf(ingLikeObj, corev1.EventTypeNormal, reasonDeleteCertificate, "Successfully deleted unrequired Certificate %q", certName)
}
return nil
@ -395,42 +396,39 @@ func buildCertificates(
return newCrts, updateCrts, nil
}
func findUnrequiredCertificates(list cmlisters.CertificateLister, ingLike metav1.Object) ([]*cmapi.Certificate, error) {
crts, err := list.Certificates(ingLike.GetNamespace()).List(labels.Everything())
if err != nil {
return nil, err
}
var unrequired []*cmapi.Certificate
for _, crt := range crts {
if isUnrequiredCertificate(crt, ingLike) {
unrequired = append(unrequired, crt)
func findCertificatesToBeRemoved(certs []*cmapi.Certificate, ingLike metav1.Object) []string {
var toBeRemoved []string
for _, crt := range certs {
if !metav1.IsControlledBy(crt, ingLike) {
continue
}
if !secretNameUsedIn(crt.Spec.SecretName, ingLike) {
toBeRemoved = append(toBeRemoved, crt.Name)
}
}
return unrequired, nil
return toBeRemoved
}
func isUnrequiredCertificate(crt *cmapi.Certificate, ingLike metav1.Object) bool {
if !metav1.IsControlledBy(crt, ingLike) {
return false
}
func secretNameUsedIn(secretName string, ingLike metav1.Object) bool {
switch o := ingLike.(type) {
case *networkingv1.Ingress:
for _, tls := range o.Spec.TLS {
if crt.Spec.SecretName == tls.SecretName {
return false
if secretName == tls.SecretName {
return true
}
}
case *gwapi.Gateway:
for _, l := range o.Spec.Listeners {
if crt.Spec.SecretName == l.TLS.CertificateRef.Name {
return false
if l.TLS == nil || l.TLS.CertificateRef == nil {
continue
}
if secretName == l.TLS.CertificateRef.Name {
return true
}
}
}
return true
return false
}
// certNeedsUpdate checks and returns true if two Certificates differ.

View File

@ -2610,6 +2610,7 @@ func buildIngressOwnerReferences(name, namespace string) []metav1.OwnerReference
}
}
// The Gateway name and UID are set to the same.
func buildGatewayOwnerReferences(name, namespace string) []metav1.OwnerReference {
return []metav1.OwnerReference{
*metav1.NewControllerRef(buildIngress(name, namespace, nil), gatewayGVK),
@ -2720,3 +2721,147 @@ func Test_validateGatewayListenerBlock(t *testing.T) {
})
}
}
func Test_findCertificatesToBeRemoved(t *testing.T) {
tests := []struct {
name string
givenCerts []*cmapi.Certificate
ingLike metav1.Object
wantToBeRemoved []string
}{
{
name: "should not remove Certificate when not owned by the Ingress",
givenCerts: []*cmapi.Certificate{{
ObjectMeta: metav1.ObjectMeta{
Name: "cert-1",
Namespace: "default",
OwnerReferences: buildGatewayOwnerReferences("ingress-1", "default"),
}, Spec: cmapi.CertificateSpec{
SecretName: "secret-name",
}},
},
ingLike: &networkingv1.Ingress{
ObjectMeta: metav1.ObjectMeta{Name: "ingress-2", Namespace: "default", UID: "ingress-2"},
Spec: networkingv1.IngressSpec{TLS: []networkingv1.IngressTLS{{SecretName: "secret-name"}}},
},
wantToBeRemoved: nil,
},
{
name: "should not remove Certificate when Ingress references the secretName of the Certificate",
givenCerts: []*cmapi.Certificate{{
ObjectMeta: metav1.ObjectMeta{
Name: "cert-1",
Namespace: "default",
OwnerReferences: buildGatewayOwnerReferences("ingress-1", "default"),
}, Spec: cmapi.CertificateSpec{
SecretName: "secret-name",
}},
},
ingLike: &networkingv1.Ingress{
ObjectMeta: metav1.ObjectMeta{Name: "ingress-1", Namespace: "default", UID: "ingress-1"},
Spec: networkingv1.IngressSpec{TLS: []networkingv1.IngressTLS{{SecretName: "secret-name"}}},
},
wantToBeRemoved: nil,
},
{
name: "should remove Certificate when Ingress does not reference the secretName of the Certificate",
givenCerts: []*cmapi.Certificate{{
ObjectMeta: metav1.ObjectMeta{
Name: "cert-1",
Namespace: "default",
OwnerReferences: buildGatewayOwnerReferences("ingress-1", "default"),
}, Spec: cmapi.CertificateSpec{
SecretName: "secret-name",
}},
},
ingLike: &networkingv1.Ingress{
ObjectMeta: metav1.ObjectMeta{Name: "ingress-1", Namespace: "default", UID: "ingress-1"},
},
wantToBeRemoved: []string{"cert-1"},
},
{
name: "should not remove Certificate when not owned by the Gateway",
givenCerts: []*cmapi.Certificate{{
ObjectMeta: metav1.ObjectMeta{
Name: "cert-1",
Namespace: "default",
OwnerReferences: buildGatewayOwnerReferences("gw-1", "default"),
}, Spec: cmapi.CertificateSpec{
SecretName: "secret-name",
}},
},
ingLike: &gwapi.Gateway{
ObjectMeta: metav1.ObjectMeta{Name: "gw-2", Namespace: "default", UID: "gw-2"},
Spec: gwapi.GatewaySpec{Listeners: []gwapi.Listener{{
TLS: &gwapi.GatewayTLSConfig{CertificateRef: &gwapi.LocalObjectReference{Name: "secret-name"}},
}}},
},
wantToBeRemoved: nil,
},
{
name: "should remove Certificate when Gateway does not reference the secretName of the Certificate in one of its listers",
givenCerts: []*cmapi.Certificate{{
ObjectMeta: metav1.ObjectMeta{
Name: "cert-1",
Namespace: "default",
OwnerReferences: buildGatewayOwnerReferences("gw-1", "default"),
}, Spec: cmapi.CertificateSpec{
SecretName: "secret-name",
}},
},
ingLike: &gwapi.Gateway{
ObjectMeta: metav1.ObjectMeta{Name: "gw-1", Namespace: "default", UID: "gw-1"},
Spec: gwapi.GatewaySpec{Listeners: []gwapi.Listener{
{TLS: &gwapi.GatewayTLSConfig{CertificateRef: &gwapi.LocalObjectReference{Name: "not-secret-name"}}},
}},
},
wantToBeRemoved: []string{"cert-1"},
},
{
name: "should not remove Certificate when the Gateway references the secretName of the Certificate in one of its listers",
givenCerts: []*cmapi.Certificate{{
ObjectMeta: metav1.ObjectMeta{
Name: "cert-1",
Namespace: "default",
OwnerReferences: buildGatewayOwnerReferences("gw-1", "default"),
}, Spec: cmapi.CertificateSpec{
SecretName: "secret-name",
}},
},
ingLike: &gwapi.Gateway{
ObjectMeta: metav1.ObjectMeta{Name: "gw-1", Namespace: "default", UID: "gw-1"},
Spec: gwapi.GatewaySpec{Listeners: []gwapi.Listener{
{TLS: &gwapi.GatewayTLSConfig{CertificateRef: &gwapi.LocalObjectReference{Name: "secret-name"}}},
}},
},
wantToBeRemoved: nil,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
gotCerts := findCertificatesToBeRemoved(test.givenCerts, test.ingLike)
assert.Equal(t, test.wantToBeRemoved, gotCerts)
})
}
}
func Test_secretNameUsedIn_nilPointerGateway(t *testing.T) {
got := secretNameUsedIn("secret-name", &gwapi.Gateway{
ObjectMeta: metav1.ObjectMeta{Name: "gw-1", Namespace: "default", UID: "gw-1"},
Spec: gwapi.GatewaySpec{Listeners: []gwapi.Listener{
{TLS: nil},
{TLS: &gwapi.GatewayTLSConfig{CertificateRef: nil}},
{TLS: &gwapi.GatewayTLSConfig{CertificateRef: &gwapi.LocalObjectReference{Name: "secret-name"}}},
}},
})
assert.Equal(t, true, got)
got = secretNameUsedIn("secret-name", &gwapi.Gateway{
ObjectMeta: metav1.ObjectMeta{Name: "gw-1", Namespace: "default", UID: "gw-1"},
Spec: gwapi.GatewaySpec{Listeners: []gwapi.Listener{
{TLS: nil},
{TLS: &gwapi.GatewayTLSConfig{CertificateRef: nil}},
}},
})
assert.Equal(t, false, got)
}